fix(audit): 2FA login audit + 2FA enable/disable audit + dashboard recent-events#23
Conversation
…fix empty dashboard feed Two regressions on the deployed console: 1) Silent auth audit (#31). RecordLoginSucceeded gated on request()->path() === 'login'. With 2FA enabled the real Login event fires on the /two-factor-challenge POST, not /login, so every 2FA login's audit was dropped (StartIamSession has no such guard → the session row was still created, masking it). Accept both /login and /two-factor-challenge. Also add RecordTwoFactorEnabled/Disabled listeners (auth.2fa.enabled / auth.2fa.disabled) — 2FA changes were never audited at all. Audit failures now Log::warning, not silent. 2) Empty "Recent audit events" dashboard card (#32). The audit endpoint filters one stream at a time and defaults to 'global' (empty) when none is passed; the dashboard passed no stream while Audit Log passes 'auth' (where login/logout/2FA live). Dashboard now requests stream 'auth'. Tests: 21 green incl. new 2FA enable/disable audit test + the login-audit guard. pint + SPA lint/build green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84f4f8a933
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // where the real Login fires when 2FA is enabled. Checking only `/login` silently dropped every | ||
| // 2FA login's audit (the regression). (POST /login has no route name in Fortify, so match by path.) | ||
| if (! $event->user instanceof User | ||
| || ! in_array(request()->path(), ['login', 'two-factor-challenge'], true)) { |
There was a problem hiding this comment.
Require POST before auditing 2FA logins
When a browser still has a remember-me recaller cookie and requests GET /two-factor-challenge, the guest middleware can resolve the guard from that cookie and fire Laravel's Login event before redirecting; because this filter only checks the path, that non-interactive restore is now recorded as auth.login.succeeded. This inflates the login metrics the listener is trying to protect, for any remembered session that hits the challenge URL, so gate the accepted paths by the POST method as well.
Useful? React with 👍 / 👎.
| // The audit endpoint filters ONE stream at a time and defaults to 'global' (empty) when none is given — | ||
| // login/logout/2FA and most activity live in 'auth', so ask for that (matches the Audit Log default) | ||
| // instead of showing an always-empty 'global' feed. | ||
| const events = useCursorList<Record<string, unknown>>('audit/events', { stream: 'auth' }, 8) |
There was a problem hiding this comment.
Read actor_user_id for auth recent events
With the dashboard now querying the auth stream, the rows it displays are the auth audit records written by these listeners, which store the actor under actor_user_id; the table below still only checks actor, actor_id, subject_id, user_id, and causer. As a result, recent login/logout/2FA events show — in the Actor column even though the audit record has the operator ID; include actor_user_id in that pick when switching this feed to auth events.
Useful? React with 👍 / 👎.
Fixes #31 (silent auth audit: 2FA login dropped by a path guard; 2FA enable/disable never audited) and #32 (dashboard Recent audit events empty — was querying the empty 'global' stream instead of 'auth'). 21 tests green + new 2FA audit test.