fix(auth): revoke a user's sessions when their password is reset

Merged
JU
4d ago
+604-46

Description

A password reset left every existing session of the user valid, and an admin could reset their own password through the admin endpoint and stay logged in (from an external security report).

  • Admin reset (POST /api/password/reset_password) signs out all sessions of the target user. Web cookie and mobile bearer sessions share one token store, so both are covered.
  • Forgot-password email reset signs out all sessions.
  • Change password signs out the user's other sessions and keeps the caller's session.
  • Self-reset through the admin endpoint returns 400 and points to the change-password flow.
  • Route errors move to OnyxError (same status codes, same detail field the frontend reads). The broad except Exception that returned str(e) in a 500 is gone; a wrong current password is now a 400, not a 500.

Coverage per AUTH_BACKEND:

  • redis (default; cloud and standard self-hosted):
    • write_token / refresh_token add the token to a per-user set (fastapi_users_user_sessions:<user_id>) with the session TTL. destroy_token removes it.
    • Revocation tombstones each indexed token, so rejected requests show as SESSION_TERMINATED. There is no keyspace scan.
    • Sessions written before this deploy join the set on their next refresh. Until then they are not revoked (they expire within SESSION_EXPIRE_TIME_SECONDS).
  • postgres: delete the user's AccessToken rows, except the kept token.
  • jwt (opt-in, single-tenant only): stateless, so not revoked. This is the same as logout today; it logs a warning.

Not in scope:

  • Admins can still reset other users, including other admins.
  • The one-time password is still shown to the admin.
  • Forcing a password change at next login is a possible follow-up.

How Has This Been Tested?

  • tests/unit/onyx/auth + route contract tests: 500 passed. test_on_after_reset_password_emits_event now also asserts revocation.
  • New tests/external_dependency_unit/auth/test_session_revocation.py (6 tests):
    • Redis: other sessions revoked, kept session stays, another user's sessions untouched.
    • The index stays right across logout and refresh.
    • Admin reset revokes all.
    • A wrong old password revokes nothing.
    • Postgres: keep one session, then revoke all.
    • Not run locally (no local Postgres/Redis); CI runs them.
  • New integration test tests/integration/tests/security/test_session_revocation_on_reset.py:
    • Admin reset: both old cookies are rejected and the new password logs in.
    • Self-reset returns 400.
    • Change-password keeps the current session and rejects the other one.
    • Not run locally (shared stack); CI runs it.
  • Redis strategy flow checked against an in-memory fake (write, logout, keep, revoke all, refresh). pre-commit passes.

Additional Options

  • [Optional] Please cherry-pick this PR to the latest release version.
  • [Optional] Override Linear Check

🤖 Generated with Claude Code

Summary by cubic

Closes a security gap where a password reset or change left existing sessions valid, and an admin could reset their own password through the admin endpoint and stay logged in.

Now the admin reset (POST /password/reset_password) signs out all sessions of the target user — web cookie and mobile bearer sessions share one token store, so both are covered. The forgot-password email reset signs out all sessions, and changing a password signs out the user's other sessions while keeping the caller's session. An admin resetting their own password via the admin endpoint gets a 400 pointing to the change-password flow. Route errors now use OnyxError with the same status codes and detail field, and a wrong current password returns 400 instead of the previous 500. A refresh can no longer revive a revoked or logged-out session: a failed refresh returns SESSION_TERMINATED instead of the old logout/login fallback, which could mint a new session after a concurrent revoke.

Revocation coverage per AUTH_BACKEND:

  • redis (default): tokens are indexed in a per-user set; revocation tombstones each indexed token so rejected requests show as SESSION_TERMINATED, with no keyspace scan. Refresh extends under WATCH/MULTI, so a tombstone landing between read and write fails the extend and the session stays revoked. Sessions written before this deploy join the index on their next refresh.
  • postgres: deletes the user's AccessToken rows except the kept token; refresh on a deleted row raises SESSION_TERMINATED instead of minting a new token.
  • jwt (opt-in): stateless, so tokens stay valid until expiry — the same as logout today — and it logs a warning.

Summary by CodeRabbit

  • Security
    • Changing your password now signs out your other active sessions while keeping your current session active.
    • An admin password reset signs the affected user out of all sessions. Admins cannot reset their own password through this action.
    • If your current password is incorrect, the change is rejected and your sessions remain active.
    • Revoked or expired sessions can no longer be restored by refreshing them.
    • With JWT-based sessions, existing sessions may remain valid until their tokens expire.