security: revoke sessions on self-service password reset#669
Merged
Conversation
ResetPassword updated the credential but never dropped the user's existing sessions and refresh tokens. An attacker holding a live session before the reset kept full access after it, defeating the purpose of resetting a compromised password (CWE-613). Terminate all sessions after the password update via MemoryStoreProvider.DeleteAllUserSessions, matching the existing UpdateProfile and DeactivateAccount pattern (fire-and-forget, does not block or fail the reset). Add a regression test asserting the user's sessions are gone from the store and the pre-existing refresh token is rejected by /oauth/token after a reset.
2 tasks
pull Bot
pushed a commit
to bhardwajRahul/authorizer
that referenced
this pull request
Jul 11, 2026
Follow-up to authorizerdev#669 (already merged): session revocation on self-service password reset was fire-and-forget (a goroutine dispatched before the response returned), leaving a small window where an attacker holding a pre-existing session/refresh token could still use it between the response going out and the goroutine actually running. Made it a synchronous call - the revocation is a single cheap memory-store operation, there's no latency reason for it to be async - and logged its error instead of silently discarding it, since a failure here means old sessions may still be live even though the reset "succeeded". Also tightened the regression test to assert immediately rather than polling with require.Eventually, since revocation is no longer eventual. Note: an in-memory single-map-delete goroutine typically completes before the surrounding code (audit log, metrics, building the response) finishes anyway, so this assertion alone doesn't reliably distinguish sync from fire-and-forget in practice — verified empirically by reverting the fix and confirming the test still passed. The guarantee here comes from the source (no goroutine, no race, full stop), documented in the test so a future reader doesn't over-trust this assertion's discriminating power.
pull Bot
pushed a commit
to bhardwajRahul/authorizer
that referenced
this pull request
Jul 22, 2026
Added to README: - WebAuthn / passkey login - Enterprise SSO (SAML 2.0 IdP & SP, OIDC broker, verified domains, home realm discovery) - SCIM 2.0 provisioning - Multi-tenant / org-scoped admin - Migration guides (planned) Added to CHANGELOG [Unreleased]: - Service accounts as FGA subjects (authorizerdev#665) - SAML ACS CSRF exemption fix (authorizerdev#666) - Trusted-issuer token review config (authorizerdev#667) - Session revocation on password reset (authorizerdev#669, authorizerdev#673) - Multi-tenant SSO phases (authorizerdev#672, authorizerdev#674, authorizerdev#675) - OIDC discovery caching + Twitter PKCE fix (authorizerdev#668) - WebAuthn passkey support (authorizerdev#671) - Per-user TOTP lockout + hashed recovery codes (authorizerdev#670) - Server-side user search + org membership (authorizerdev#678, authorizerdev#680) - Per-method MFA availability signals (authorizerdev#681) - Consolidated MFA redesign narrative (authorizerdev#682-686) - SAML 2.0 IdP role (authorizerdev#691) - OAuth 2.1 / MCP hardening (authorizerdev#693) - SCIM group provisioning + FGA + SAML assertions (authorizerdev#694) - Provider template parity (authorizerdev#695) - Recent fixes (async tracking, AGENTS.md rules, security fixes, storage parity, error typing, panics, frontend fixes) (authorizerdev#696-702)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap
The self-service password reset flow (
internal/service/reset_password.go) updated the user's password but never revoked their existing sessions or refresh tokens. Anyone holding a live session before the reset (e.g. the attacker whose access you're resetting the password to kill) kept full access afterward — the reset was cosmetic against a live intruder (CWE-613: Insufficient Session Expiration).Two sibling flows already do the right thing on a similar sensitive change:
internal/service/update_profile.go(on email change)internal/service/deactivate_account.goBoth call
MemoryStoreProvider.DeleteAllUserSessions(user.ID). Password reset was the odd one out.The fix
Add the same
DeleteAllUserSessionscall after the password update succeeds, matching the existing pattern exactly — fire-and-forget in a goroutine, so a memory-store hiccup logs and continues rather than failing the reset:This covers both token and OTP reset paths (single shared code path in
ResetPassword).Test
Extended
TestResetPasswordwithshould revoke existing sessions after reset:offline_accessrefresh token).401) by/oauth/token.The test discriminates: it fails on
main(session survives) and passes with the fix. Refresh tokens are single-use (rotated on success), so the test polls the non-mutating memory store to detect revocation rather than polling the refresh endpoint (which would self-invalidate the token and mask the bug).make test-sqlitepasses clean.