Skip to content

fix(idp): invalidate other sessions on password reset and change#1297

Open
laskevych wants to merge 1 commit into
mainfrom
idp/invalidate-other-sessions-on-password-reset-and-change
Open

fix(idp): invalidate other sessions on password reset and change#1297
laskevych wants to merge 1 commit into
mainfrom
idp/invalidate-other-sessions-on-password-reset-and-change

Conversation

@laskevych

Copy link
Copy Markdown
Contributor

Why

A security assessment found that some password reset/change paths left a user's
other active sessions valid. After a successful password reset or change, every
other session for that user should be invalidated (the session performing the
change may remain valid). Scope: idp-better-auth and idp-owox-better-auth.
First-time password setup is out of scope.

What changed

idp-better-auth — complete fix

Here a "session" is a Better Auth session: the refreshToken cookie is the
Better Auth session token, and access tokens are short-lived (≈1h), so deleting
session rows immediately cuts off other devices' refresh ability.

  • Forced revoke on the exposed /change-password — a hooks.before
    middleware sets revokeOtherSessions: true, so a password change always
    revokes the user's other sessions while Better Auth issues a fresh session for
    the current one (clients can no longer skip it by omitting the flag).
  • Admin self-reset now revokes other sessions, preserving the current one
    resetUserPassword(userId, adminUserId, currentSessionToken?):
    • admin resets another user → revoke ALL of that user's sessions (unchanged),
    • admin resets self → revoke every OTHER session, keep the acting one,
    • missing token → fail-safe revoke-all.
  • New store method revokeOtherUserSessions(userId, exceptSessionToken)
    (DELETE FROM session WHERE userId = ? AND token != ?) on the SQLite and
    MySQL stores + interface.

idp-owox-better-auth — partial, platform-level

Here a real session is an OWOX Identity token (issued by an external service,
30-day refresh), not a Better Auth session — so deleting Better Auth rows alone
does not cut platform access.

  • On a successful password reset, also revoke the OWOX platform refresh token
    carried by the request (extractRefreshToken → revokeToken, best-effort,
    non-fatal) and clear its cookie, via a callback injected into
    PasswordFlowController.

Known limitation: this revokes only the acting device's platform token.
Revoking a user's other devices requires a revoke-by-userId endpoint on
OWOX Identity, which does not exist yet (only single-token revocation is
exposed). Because the backend validates tokens via INTROSPECT by default,
such an endpoint would take effect on the next request once available. Tracked
separately for the OWOX Identity service.

Tests

  • idp-better-auth: integration tests on a real Better Auth + in-memory SQLite
    instance assert surviving session rows for change-password; unit tests cover
    all resetUserPassword branches and the new revoke SQL.
  • idp-owox-better-auth: controller tests assert the platform token is revoked
    on successful reset and NOT revoked when the reset fails.
  • Full suites green: idp-better-auth 125, idp-owox-better-auth 224.

Not in scope

  • First-time password setup (creates a credential, never updates an existing one).
  • Full multi-device revocation in idp-owox (blocked on the OWOX Identity
    backend endpoint above).

🤖 Generated with Claude Code

@laskevych laskevych requested a review from max-voloshyn June 8, 2026 17:54
@laskevych laskevych self-assigned this Jun 8, 2026
// Revoke the OWOX platform refresh token carried by this request (if
// any) so the acting device's platform session is invalidated at the
// OWOX Identity level, not only the Better Auth UI session.
await this.revokePlatformSession?.(req, res);

@max-voloshyn max-voloshyn Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: This reset flow still leaves the user’s other Better Auth sessions valid. better-auth@1.6.13 only deletes sessions during resetPassword when emailAndPassword.revokeSessionsOnPasswordReset is enabled; the OWOX config does not set it, and the new callback only revokes or clears the current request platform cookie. A reset from a logged-out browser or while another Better Auth session exists will keep that other session usable. Please enable Better Auth reset-session revocation or explicitly delete the user’s Better Auth sessions before redirecting.
🤖 Reviewed by Codex (GPT-5.5)

enabled: true,
requireEmailVerification: false,
},
hooks: {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: This hook only covers @owox/idp-better-auth. @owox/idp-owox-better-auth has a separate createBetterAuthConfig, and BetterAuthProxyHandler forwards every /auth/better-auth/* route there, so /auth/better-auth/change-password can still omit revokeOtherSessions and keep other sessions alive. Please add the same hook to the OWOX Better Auth config or centralize this config behavior.
🤖 Reviewed by Codex (GPT-5.5)

* session. Revocation failures are non-fatal (logged, not thrown) so they
* never block completing the password flow.
*
* Scope note: this only revokes the token present in the request (the acting

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: This scope note means password reset still does not invalidate other active OWOX platform sessions. Only the refresh token on the current request is revoked; another device refreshToken can still refresh through accessTokenMiddleware. Since this PR is meant to invalidate other sessions on reset, it needs a revoke-by-user or session-family path or another mechanism that prevents old platform refresh tokens from continuing after reset.
🤖 Reviewed by Codex (GPT-5.5)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants