feat(audit): enrich device sign-out audit logs

[appleboy]

Summary

Device sign-out on /account/devices already wrote an audit record, but the single-device record logged only a bare session UUID, and the legacy-cookie path of "sign out other devices" produced no record at all. This PR enriches the single-device SESSION_REVOKED entry with the revoked device's browser/OS/IP, and makes the refused empty-SID "sign out others" attempt emit a WARNING / success=false SESSION_REVOKED_ALL entry instead of silently doing nothing. No revoke behavior changes; this is additive, fail-open audit enrichment.

Architecture / flow

flowchart TD
    ReqOne[POST /account/devices/:id/revoke] --> RBID["RevokeByID()"]
    ReqOthers[POST /account/devices/revoke-others] --> ROTH["RevokeOthers()"]

    RBID --> Fetch["GetLoginSessionByID(id)<br/>best-effort prefetch"]
    Fetch --> Parse["ParseUserAgent(ls.UserAgent)<br/>build Details: browser, os, ip"]
    Fetch -->|fetch error, fail open| Revoke
    Parse --> Revoke["RevokeLoginSessionByID"]
    Revoke --> LogOne["auditService.Log<br/>SESSION_REVOKED + Details"]

    ROTH --> Empty{"currentRawSID empty?"}
    Empty -->|no, normal| RevokeMany["RevokeLoginSessionsByUserExceptSIDHash"]
    RevokeMany --> LogAll["auditService.Log<br/>SESSION_REVOKED_ALL + revoked_count"]
    Empty -->|yes, legacy cookie| LogRefused["auditService.Log<br/>WARNING, success=false<br/>reason=missing_current_sid"]
    LogRefused --> Noop["return 0, nil"]

    LogOne --> ADB[(audit_logs)]
    LogAll --> ADB
    LogRefused --> ADB

    style Fetch fill:#dff0d8,stroke:#3c763d
    style Parse fill:#dff0d8,stroke:#3c763d
    style LogOne fill:#dff0d8,stroke:#3c763d
    style LogRefused fill:#dff0d8,stroke:#3c763d

Loading

Green = new/changed call sites. Everything else already existed and is unchanged.

AI Authorship

• AI was used. Details:
  • Tool / model: Claude Opus 4.8 (1M context) via Claude Code
  • AI-authored files: internal/services/login_session.go, internal/services/login_session_test.go
  • Human line-by-line reviewed: TODO — pending. An AI review (/code-review max) was run and its findings applied, but a human line-by-line pass is recommended before merge given the auth/session context.

Change classification

• Core code (lives in the auth/session security service — LoginSessionService)
  • The change itself is additive observability with fail-open semantics (a prefetch error never blocks the revoke), but it sits in a security-sensitive path, so it warrants line-by-line review.

Plan reference

plan.md — Enrich device sign-out audit logs.

• Goal: single-device sign-out records the revoked device's browser/OS/IP; the refused empty-SID "sign out others" attempt records a WARNING entry instead of silently no-op'ing.
• Scope (respected): may modify internal/services/login_session.go + its test; must-not-modify list (store, handlers, audit service, models, migrations, templates) untouched. No new event types, no DB migration, no new dependencies.

Verification

• Unit tests (service-level, real in-memory SQLite store)
• At least 3 e2e tests (1 happy path + 2 errors):
  • TestLoginSession_RevokeByID_AuditEnriched — asserts Details{browser:"Chrome", os:"macOS", ip:"192.0.2.10"}
  • TestLoginSession_RevokeByID_MissingRowDegradesGracefully — missing row still revokes + logs, omits device Details
  • TestLoginSession_RevokeOthers_EmptySIDIsNoop (extended) — still revokes nothing and records WARNING/success=false/reason=missing_current_sid
• Stress / soak: N/A (synchronous, user-initiated action)
• make generate && make fmt && make lint && make test all pass.
• Manual verification: sign in from two browsers, sign one out from /account/devices, open /admin/audit, confirm the SESSION_REVOKED row shows the revoked browser/OS/IP.

Verifiability check

• Inputs and outputs documented (audit Details keys: browser, os, ip, reason)
• Failures surface in monitoring (audit log + existing metrics)
• Reviewer can judge most of the change from the tests; the fail-open prefetch path still warrants a line-by-line look.

Risk & rollback

• Risk: the extra best-effort GetLoginSessionByID read or a parse error could in principle break a previously-working revoke. Mitigated: the prefetch is fail-open — errors are swallowed and the revoke proceeds (covered by the missing-row test). A second, lower risk: the empty-SID RevokeOthers refusal now writes an audit row on a route that is not rate-limited (pre-existing infra); surfacing the refusal is intentional, but heavy abuse of a SID-less cookie could add WARNING rows.
• Rollback: revert the single commit. No schema/migration/config change; existing audit rows are unaffected.

Reviewer guide

• Read carefully: RevokeByID / RevokeOthers in internal/services/login_session.go — the fail-open prefetch and the nil-Details path.
• Spot-check OK: internal/services/login_session_test.go — assertions are exact (entry counts, severity, success flag).

---

🤖 Generated with Claude Code

[appleboy]

@codex review

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: f5de3c9e5b

ℹ️ 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".