Collapse the fake user layer #53

Closed
opened 2026-08-15 13:35:48 +02:00 by myrmidex · 3 comments
Owner

The app is single-user with no auth enforcement, yet carries a full user system that exists only to be bypassed.

Current state

User::default() (app/Models/User.php:40-48) invents a fake user@incr.local with a random bcrypt password, and every controller opens with User::default()->tracker plus a null-guard. Around it sits Authenticatable, Notifiable, hashed-password casts, users / sessions / password_reset_tokens tables, RegisteredUserController, routes/auth.php, LoginRequest, ProfileUpdateRequest — none of it enforcing anything.

Scope

  • Delete app/Http/Controllers/Auth/RegisteredUserController.php, routes/auth.php and its require in routes/web.php
  • Delete app/Http/Requests/Auth/LoginRequest.php, app/Http/Requests/Settings/ProfileUpdateRequest.php
  • Reduce User to nothing, or remove it entirely in favour of a singleton Tracker resolved by Tracker::firstOrCreate()
  • Drop users, sessions, password_reset_tokens if User goes

Open question — resolve during planning

Whether User disappears entirely or stays as a hollow owner record. Removing it is cleaner now; keeping it costs one table and makes multi-user a smaller change if that ever comes back. Recommend removing — it can be reintroduced properly if ever needed, and speculative retention is the habit this milestone is correcting.

Depends on

#48 and #50 — those remove most User::default()->tracker call sites, so this lands on a much smaller surface afterwards.

Acceptance criteria

  • No fake-user construction anywhere
  • Tracker resolves without going through a user
  • Migration runs clean forward on a fresh DB
  • Lint + PHPStan clean
The app is single-user with no auth enforcement, yet carries a full user system that exists only to be bypassed. ## Current state `User::default()` (`app/Models/User.php:40-48`) invents a fake `user@incr.local` with a random bcrypt password, and every controller opens with `User::default()->tracker` plus a null-guard. Around it sits `Authenticatable`, `Notifiable`, hashed-password casts, `users` / `sessions` / `password_reset_tokens` tables, `RegisteredUserController`, `routes/auth.php`, `LoginRequest`, `ProfileUpdateRequest` — none of it enforcing anything. ## Scope - Delete `app/Http/Controllers/Auth/RegisteredUserController.php`, `routes/auth.php` and its `require` in `routes/web.php` - Delete `app/Http/Requests/Auth/LoginRequest.php`, `app/Http/Requests/Settings/ProfileUpdateRequest.php` - Reduce `User` to nothing, or remove it entirely in favour of a singleton `Tracker` resolved by `Tracker::firstOrCreate()` - Drop `users`, `sessions`, `password_reset_tokens` if `User` goes ## Open question — resolve during planning Whether `User` disappears entirely or stays as a hollow owner record. Removing it is cleaner now; keeping it costs one table and makes multi-user a smaller change if that ever comes back. Recommend removing — it can be reintroduced properly if ever needed, and speculative retention is the habit this milestone is correcting. ## Depends on #48 and #50 — those remove most `User::default()->tracker` call sites, so this lands on a much smaller surface afterwards. ## Acceptance criteria - [ ] No fake-user construction anywhere - [ ] Tracker resolves without going through a user - [ ] Migration runs clean forward on a fresh DB - [ ] Lint + PHPStan clean
myrmidex added this to the (deleted) milestone 2026-08-15 13:35:48 +02:00
myrmidex added the
enhancement
label 2026-08-15 13:35:48 +02:00
myrmidex self-assigned this 2026-08-15 13:35:48 +02:00
myrmidex modified the milestone from (deleted) to v0.4.0 2026-08-15 13:42:27 +02:00
Author
Owner

Live bug to fix here: /register is already broken

Surfaced by a code-reviewer pass during #47. Not caused by #47 — traced to 04fbda4 ("43 - Delete dead Breeze auth/settings boilerplate, slim auth routes to register-only"), which removed the page component but left the controller and route behind.

Current state:

  • app/Http/Controllers/Auth/RegisteredUserController.php:28 returns Inertia::render('auth/register')
  • routes/auth.php:7 registers GET /register; routes/web.php:54 requires it, so the route is live
  • resources/js/pages/auth/register.tsx does not exist — confirmed absent since 04fbda4

Inertia resolves page components via import.meta.glob('./pages/**/*.tsx') (resources/js/app.tsx:12), so this fails at runtime, not build time — which is why no gate caught it.

Impact: on a fresh install User::exists() is false, so /register is the first-run setup path. It would fail for any new deployment.

Resolution: this ticket already deletes RegisteredUserController, routes/auth.php, LoginRequest and ProfileUpdateRequest, which removes the dangling route entirely. No need to restore the page component — just make sure the route/controller removal actually lands rather than only trimming the model.

Add to acceptance criteria:

  • GET /register returns 404 (route gone), not an Inertia resolution error
  • No Inertia::render() call anywhere references a non-existent page component
## Live bug to fix here: `/register` is already broken Surfaced by a `code-reviewer` pass during #47. Not caused by #47 — traced to `04fbda4` ("43 - Delete dead Breeze auth/settings boilerplate, slim auth routes to register-only"), which removed the page component but left the controller and route behind. Current state: - `app/Http/Controllers/Auth/RegisteredUserController.php:28` returns `Inertia::render('auth/register')` - `routes/auth.php:7` registers `GET /register`; `routes/web.php:54` requires it, so the route is live - `resources/js/pages/auth/register.tsx` **does not exist** — confirmed absent since `04fbda4` Inertia resolves page components via `import.meta.glob('./pages/**/*.tsx')` (`resources/js/app.tsx:12`), so this fails at runtime, not build time — which is why no gate caught it. **Impact:** on a fresh install `User::exists()` is false, so `/register` is the first-run setup path. It would fail for any new deployment. **Resolution:** this ticket already deletes `RegisteredUserController`, `routes/auth.php`, `LoginRequest` and `ProfileUpdateRequest`, which removes the dangling route entirely. No need to restore the page component — just make sure the route/controller removal actually lands rather than only trimming the model. Add to acceptance criteria: - [ ] `GET /register` returns 404 (route gone), not an Inertia resolution error - [ ] No `Inertia::render()` call anywhere references a non-existent page component
Author
Owner

PHPStan level 8 finding in code this ticket deletes

While setting up #55 I tried level 8 to see what it would take. One real null-safety bug turned up in a file this ticket removes:

app/Http/Requests/Settings/ProfileUpdateRequest.php:28
  Cannot access property $id on App\Models\User|null

$this->user()->id assumes an authenticated user, but the app has no auth — user() is always null here. The file is unreachable (nothing routes to it), so it is latent rather than live, but it is a genuine defect and not a false positive.

No action needed beyond deleting the file, which this ticket already scopes.

Consider raising to level 8 after this lands

The project is clean at level 7 with no baseline. Level 8 reports 8 errors across 4 files, and two of those files are deleted by this ticket and #52:

File Errors Fate
ProfileUpdateRequest.php 1 deleted here
HandleInertiaRequests.php 2 deleted by #52
CounterTest.php, TrackerTest.php, CountBackfillMigrationTest.php 5 Model::first() returning ?Model — one-line fixes

So after this ticket and #52, level 8 costs roughly five assertNotNull() calls in tests. Worth doing then rather than now, since fixing code scheduled for deletion is wasted effort.

## PHPStan level 8 finding in code this ticket deletes While setting up #55 I tried level 8 to see what it would take. One real null-safety bug turned up in a file this ticket removes: ``` app/Http/Requests/Settings/ProfileUpdateRequest.php:28 Cannot access property $id on App\Models\User|null ``` `$this->user()->id` assumes an authenticated user, but the app has no auth — `user()` is always null here. The file is unreachable (nothing routes to it), so it is latent rather than live, but it is a genuine defect and not a false positive. No action needed beyond deleting the file, which this ticket already scopes. ## Consider raising to level 8 after this lands The project is clean at level 7 with no baseline. Level 8 reports 8 errors across 4 files, and **two of those files are deleted by this ticket and #52**: | File | Errors | Fate | |---|---|---| | `ProfileUpdateRequest.php` | 1 | deleted here | | `HandleInertiaRequests.php` | 2 | deleted by #52 | | `CounterTest.php`, `TrackerTest.php`, `CountBackfillMigrationTest.php` | 5 | `Model::first()` returning `?Model` — one-line fixes | So after this ticket and #52, level 8 costs roughly five `assertNotNull()` calls in tests. Worth doing then rather than now, since fixing code scheduled for deletion is wasted effort.
Author
Owner

Done — c34269d

21 files, −292 lines. User is gone entirely.

Decision on the open question: remove User completely rather than keep a hollow owner record. With Tracker down to label/unit/count there was nothing meaningful being owned, and the belongsTo was pure ceremony.

Deleted: User, UserFactory, RegisteredUserController, LoginRequest, ProfileUpdateRequest, routes/auth.php, config/auth.php.

Tracker is standalone, resolved by Tracker::current() (explicit orderBy('id')->first()).

Migration 2026_08_15_000004_drop_users_and_sessions.php — drops trackers.user_id FK and column, then sessions, password_reset_tokens, users.

Verified live after migrating: GET /tracker returns {"exists":true,"tracker":{"id":1,"label":"Counter","unit":"units","count":1005,...}} — no user_id, no asset_id, count intact.

Required side effect: session driver

Changed databasecookie in .env, .env.example, and the config/session.php default.

This was not optional. With no auth, sessions exist only to carry the CSRF token — dropping the sessions table on the database driver would have broken every POST. The config default was changed too, so a missing env var cannot fall back to a table that no longer exists.

Tests run with SESSION_DRIVER=array and would never have caught this, so it was verified against the running app: POST /increment succeeded with a real CSRF token (1005 → 1006), PATCH /count restored it.

config/auth.php deleted

It still referenced User::class after the model was gone — a latent fatal that only fires when the auth provider resolves. Confirmed safe: no Auth::, auth(), config('auth, or auth middleware anywhere in the codebase.

Review found a real defect; testing the fix found the fix was wrong

Review correctly spotted that down() restored trackers.user_id as nullable() while the original (2026_05_02_000001:14) is non-nullable. Removing nullable() and testing the round trip failed:

SQLSTATE[23000]: Cannot add or update a child row:
a foreign key constraint fails (trackers_user_id_foreign)

A non-nullable FK cannot be restored here — existing trackers have no user to point at, so every row would get user_id = 0 and violate the constraint. nullable() is load-bearing. Reverted with a comment explaining why, so it does not get "corrected" again.

This is the third down() transcription mismatch in this milestone (#49, #48, #53). Practice changed: write the round-trip test alongside the migration rather than hand-checking the transcription.

New tests

DropUsersMigrationTest (5 tests): migrated schema has no user tables, down() restores them, restored user_id is deliberately nullable, up() re-drops respecting FKs, and the counter survives the round trip.

Also

  • Removed dead BCRYPT_ROUNDS from .env.example
  • Two earlier migration tests inserted a users row for the old FK; removed
  • 2026_08_15_000003's down() used ->after('user_id'), which no longer exists; positional hint dropped
  • Removed the always-null auth.user Inertia shared prop

Noted, not fixed

TrackerController::store() has a check-then-create race on the singleton invariant. Pre-existing, negligible for a single-user app, and #52 rewrites that controller.

Gates

PHPStan 0 errors (level 7) · Pint PASS 43 files · PHPUnit OK 36 tests, 93 assertions

Level 8 is now one file closer — only HandleInertiaRequests.php remains, which #52 deletes.

## Done — `c34269d` 21 files, −292 lines. `User` is gone entirely. **Decision on the open question:** remove `User` completely rather than keep a hollow owner record. With `Tracker` down to `label`/`unit`/`count` there was nothing meaningful being owned, and the `belongsTo` was pure ceremony. **Deleted:** `User`, `UserFactory`, `RegisteredUserController`, `LoginRequest`, `ProfileUpdateRequest`, `routes/auth.php`, `config/auth.php`. **`Tracker`** is standalone, resolved by `Tracker::current()` (explicit `orderBy('id')->first()`). **Migration** `2026_08_15_000004_drop_users_and_sessions.php` — drops `trackers.user_id` FK and column, then `sessions`, `password_reset_tokens`, `users`. Verified live after migrating: `GET /tracker` returns `{"exists":true,"tracker":{"id":1,"label":"Counter","unit":"units","count":1005,...}}` — no `user_id`, no `asset_id`, count intact. ### Required side effect: session driver Changed `database` → `cookie` in `.env`, `.env.example`, and the `config/session.php` default. This was **not optional**. With no auth, sessions exist only to carry the CSRF token — dropping the `sessions` table on the database driver would have broken every POST. The config default was changed too, so a missing env var cannot fall back to a table that no longer exists. Tests run with `SESSION_DRIVER=array` and would never have caught this, so it was verified against the running app: `POST /increment` succeeded with a real CSRF token (1005 → 1006), `PATCH /count` restored it. ### `config/auth.php` deleted It still referenced `User::class` after the model was gone — a latent fatal that only fires when the auth provider resolves. Confirmed safe: no `Auth::`, `auth()`, `config('auth`, or auth middleware anywhere in the codebase. ### Review found a real defect; testing the fix found the fix was wrong Review correctly spotted that `down()` restored `trackers.user_id` as `nullable()` while the original (`2026_05_02_000001:14`) is non-nullable. Removing `nullable()` and testing the round trip failed: ``` SQLSTATE[23000]: Cannot add or update a child row: a foreign key constraint fails (trackers_user_id_foreign) ``` A non-nullable FK **cannot** be restored here — existing trackers have no user to point at, so every row would get `user_id = 0` and violate the constraint. `nullable()` is load-bearing. Reverted with a comment explaining why, so it does not get "corrected" again. This is the third `down()` transcription mismatch in this milestone (#49, #48, #53). Practice changed: write the round-trip test alongside the migration rather than hand-checking the transcription. ### New tests `DropUsersMigrationTest` (5 tests): migrated schema has no user tables, `down()` restores them, restored `user_id` is deliberately nullable, `up()` re-drops respecting FKs, and the counter survives the round trip. ### Also - Removed dead `BCRYPT_ROUNDS` from `.env.example` - Two earlier migration tests inserted a `users` row for the old FK; removed - `2026_08_15_000003`'s `down()` used `->after('user_id')`, which no longer exists; positional hint dropped - Removed the always-null `auth.user` Inertia shared prop ### Noted, not fixed `TrackerController::store()` has a check-then-create race on the singleton invariant. Pre-existing, negligible for a single-user app, and #52 rewrites that controller. ### Gates PHPStan 0 errors (level 7) · Pint PASS 43 files · PHPUnit OK **36 tests, 93 assertions** Level 8 is now one file closer — only `HandleInertiaRequests.php` remains, which #52 deletes.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lvl0/incr#53
No description provided.