From 1548ead8eb1a2514bbf3e5ab48677125e6a1003c Mon Sep 17 00:00:00 2001 From: Andrew Laskevych Date: Mon, 8 Jun 2026 20:53:45 +0300 Subject: [PATCH] fix(idp): invalidate other sessions on password reset and change --- packages/idp-better-auth/jest.setup.ts | 3 + .../auth-config.session-revocation.test.ts | 70 ++++++++++++++++ .../idp-better-auth/src/auth/auth-config.ts | 18 +++++ .../src/services/page-service.ts | 3 +- .../services/user-management-service.test.ts | 80 ++++++++++++++++++ .../src/services/user-management-service.ts | 16 +++- .../src/store/DatabaseStore.ts | 6 ++ .../src/store/MysqlDatabaseStore.ts | 12 +++ ...teDatabaseStore.session-revocation.test.ts | 56 +++++++++++++ .../src/store/SqliteDatabaseStore.ts | 11 +++ .../password-flow-controller.test.ts | 81 +++++++++++++++++++ .../controllers/password-flow-controller.ts | 18 ++++- .../src/owox-better-auth-idp.ts | 35 +++++++- 13 files changed, 405 insertions(+), 4 deletions(-) create mode 100644 packages/idp-better-auth/src/auth/auth-config.session-revocation.test.ts create mode 100644 packages/idp-better-auth/src/store/SqliteDatabaseStore.session-revocation.test.ts diff --git a/packages/idp-better-auth/jest.setup.ts b/packages/idp-better-auth/jest.setup.ts index af8bf7d1c5..f83711b6b1 100644 --- a/packages/idp-better-auth/jest.setup.ts +++ b/packages/idp-better-auth/jest.setup.ts @@ -17,4 +17,7 @@ jest.unstable_mockModule('@owox/internal-helpers', () => ({ // and must be present in the mock so Jest ESM can satisfy the named import binding disableConditionalCaching: jest.fn(), sendSecureHtml: jest.fn(), + // LogLevel is imported by auth-config.ts / page-service.ts; provide an + // enum-like object so the named import binding resolves under Jest ESM. + LogLevel: { ERROR: 'error', WARN: 'warn', INFO: 'info', DEBUG: 'debug' }, })); diff --git a/packages/idp-better-auth/src/auth/auth-config.session-revocation.test.ts b/packages/idp-better-auth/src/auth/auth-config.session-revocation.test.ts new file mode 100644 index 0000000000..0fca137095 --- /dev/null +++ b/packages/idp-better-auth/src/auth/auth-config.session-revocation.test.ts @@ -0,0 +1,70 @@ +import { afterEach, describe, expect, it } from '@jest/globals'; +import Database from 'better-sqlite3'; +import { getMigrations } from 'better-auth/db/migration'; +import { createBetterAuthConfig } from './auth-config.js'; +import type { BetterAuthConfig } from '../types/index.js'; + +/** + * Security regression test: the exposed Better Auth `/change-password` + * endpoint must revoke the user's other sessions even when the caller does not + * opt in via `revokeOtherSessions`. The auth config forces it on via a hook. + */ +describe('idp-better-auth — change-password session invalidation', () => { + const EMAIL = 'user@test.io'; + const PASSWORD = 'Passw0rd1'; + + let db: InstanceType; + + afterEach(() => { + db?.close(); + }); + + async function buildAuth() { + db = new Database(':memory:'); + const config = { + baseURL: 'http://localhost:3000', + secret: 'x'.repeat(40), + magicLinkTtl: 3600, + } as unknown as BetterAuthConfig; + + const auth = await createBetterAuthConfig(config, { adapter: db }); + const { runMigrations } = await getMigrations(auth.options); + await runMigrations(); + return auth; + } + + function sessionTokens(): string[] { + return (db.prepare('SELECT token FROM session').all() as Array<{ token: string }>).map( + r => r.token + ); + } + + it('revokes other sessions when the client omits revokeOtherSessions', async () => { + const auth = await buildAuth(); + + const signUpRes = await auth.api.signUpEmail({ + body: { email: EMAIL, password: PASSWORD, name: 'User' }, + asResponse: true, + }); + const setCookie = signUpRes.headers.get('set-cookie') ?? ''; + const tokenMatch = setCookie.match(/refreshToken=([^;]+)/); + expect(tokenMatch?.[1]).toBeTruthy(); + const sessionAToken = decodeURIComponent(tokenMatch![1]).split('.')[0]; + + // A second, independent session for the same user. + await auth.api.signInEmail({ body: { email: EMAIL, password: PASSWORD } }); + expect(sessionTokens()).toHaveLength(2); + + const resp = await auth.api.changePassword({ + // intentionally no `revokeOtherSessions` — the config hook forces it + body: { currentPassword: PASSWORD, newPassword: 'NewPassw0rd1' }, + headers: { cookie: `refreshToken=${tokenMatch![1]}` }, + asResponse: true, + }); + expect(resp.status).toBe(200); + + const remaining = sessionTokens(); + expect(remaining).toHaveLength(1); + expect(remaining).not.toContain(sessionAToken); + }); +}); diff --git a/packages/idp-better-auth/src/auth/auth-config.ts b/packages/idp-better-auth/src/auth/auth-config.ts index dfbfdf22ea..c89dfbca84 100644 --- a/packages/idp-better-auth/src/auth/auth-config.ts +++ b/packages/idp-better-auth/src/auth/auth-config.ts @@ -1,4 +1,5 @@ import { betterAuth } from 'better-auth'; +import { createAuthMiddleware } from 'better-auth/api'; import { magicLink, organization } from 'better-auth/plugins'; import { BetterAuthConfig } from '../types/index.js'; import { createAccessControl } from 'better-auth/plugins/access'; @@ -110,6 +111,23 @@ export async function createBetterAuthConfig( enabled: true, requireEmailVerification: false, }, + hooks: { + // Security: the Better Auth `/change-password` endpoint is exposed through + // the request handler. By default it only revokes other sessions when the + // caller opts in via `revokeOtherSessions`. Force it on so a password + // change always invalidates the user's other sessions while Better Auth + // issues a fresh session for the current one. + before: createAuthMiddleware(async ctx => { + if (ctx.path === '/change-password') { + return { + context: { + body: { ...ctx.body, revokeOtherSessions: true }, + }, + }; + } + return; + }), + }, advanced: { cookies: { session_token: { diff --git a/packages/idp-better-auth/src/services/page-service.ts b/packages/idp-better-auth/src/services/page-service.ts index 1d26348790..2e515d2f2f 100644 --- a/packages/idp-better-auth/src/services/page-service.ts +++ b/packages/idp-better-auth/src/services/page-service.ts @@ -442,7 +442,8 @@ export class PageService { const result = await this.userManagementService.resetUserPassword( validation.userId, - session.user.id + session.user.id, + session.session?.token ); res.json({ diff --git a/packages/idp-better-auth/src/services/user-management-service.test.ts b/packages/idp-better-auth/src/services/user-management-service.test.ts index 84a3e9c62c..3342582129 100644 --- a/packages/idp-better-auth/src/services/user-management-service.test.ts +++ b/packages/idp-better-auth/src/services/user-management-service.test.ts @@ -136,3 +136,83 @@ describe('UserManagementService — inviteAndCreateStub', () => { ); }); }); + +describe('UserManagementService — resetUserPassword (session revocation)', () => { + let store: jest.Mocked; + let magicLinkService: jest.Mocked; + let cryptoService: jest.Mocked; + + const ADMIN_ID = 'admin-1'; + const TARGET_ID = 'target-1'; + const CURRENT_TOKEN = 'current-session-token'; + + beforeEach(() => { + store = { + getUserRole: jest.fn(), + getUserById: jest.fn(), + revokeUserSessions: jest.fn(), + revokeOtherUserSessions: jest.fn(), + clearUserPassword: jest.fn(), + } as unknown as jest.Mocked; + magicLinkService = { + generateMagicLink: jest.fn(), + } as unknown as jest.Mocked; + cryptoService = {} as unknown as jest.Mocked; + + store.getUserById.mockResolvedValue({ id: TARGET_ID, email: 'target@x.io' } as never); + magicLinkService.generateMagicLink.mockResolvedValue('https://app/magic/tok'); + }); + + function build() { + const auth = { + options: { baseURL: 'http://localhost:3000' }, + handler: jest.fn(), + } as unknown as Parameters[0]; + return new UserManagementService(auth, magicLinkService, cryptoService, store); + } + + it('admin resetting ANOTHER user revokes ALL of that user’s sessions', async () => { + // store.getUserRole is called as (orgId, userId) — branch on the userId arg. + store.getUserRole.mockImplementation(async (_orgId, userId) => + userId === ADMIN_ID ? 'admin' : 'editor' + ); + + const service = build(); + await service.resetUserPassword(TARGET_ID, ADMIN_ID, CURRENT_TOKEN); + + expect(store.revokeUserSessions).toHaveBeenCalledWith(TARGET_ID); + expect(store.revokeOtherUserSessions).not.toHaveBeenCalled(); + expect(store.clearUserPassword).toHaveBeenCalledWith(TARGET_ID); + }); + + it('self-reset revokes OTHER sessions while preserving the current session', async () => { + store.getUserRole.mockResolvedValue('admin'); + + const service = build(); + await service.resetUserPassword(ADMIN_ID, ADMIN_ID, CURRENT_TOKEN); + + expect(store.revokeOtherUserSessions).toHaveBeenCalledWith(ADMIN_ID, CURRENT_TOKEN); + expect(store.revokeUserSessions).not.toHaveBeenCalled(); + }); + + it('self-reset without a known current session falls back to revoking all sessions', async () => { + store.getUserRole.mockResolvedValue('admin'); + + const service = build(); + await service.resetUserPassword(ADMIN_ID, ADMIN_ID); + + expect(store.revokeUserSessions).toHaveBeenCalledWith(ADMIN_ID); + expect(store.revokeOtherUserSessions).not.toHaveBeenCalled(); + }); + + it('rejects when the acting user is not an admin', async () => { + store.getUserRole.mockResolvedValue('editor'); + + const service = build(); + await expect(service.resetUserPassword(TARGET_ID, ADMIN_ID, CURRENT_TOKEN)).rejects.toThrow( + /administrators/ + ); + expect(store.revokeUserSessions).not.toHaveBeenCalled(); + expect(store.revokeOtherUserSessions).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/idp-better-auth/src/services/user-management-service.ts b/packages/idp-better-auth/src/services/user-management-service.ts index ceb5a559f9..6a9963106a 100644 --- a/packages/idp-better-auth/src/services/user-management-service.ts +++ b/packages/idp-better-auth/src/services/user-management-service.ts @@ -250,8 +250,18 @@ export class UserManagementService { /** * Reset user password (admin-only operation) * Clears existing password, revokes sessions, and generates new magic link + * + * Session handling: + * - Resetting another user's password revokes ALL of that user's sessions. + * - Resetting your own password (self-reset) revokes every OTHER session but + * keeps the acting session valid, provided `currentSessionToken` is given. + * Without it we fall back to revoking all sessions to stay fail-safe. */ - async resetUserPassword(userId: string, adminUserId: string): Promise<{ magicLink: string }> { + async resetUserPassword( + userId: string, + adminUserId: string, + currentSessionToken?: string + ): Promise<{ magicLink: string }> { try { const adminRole = await this.getUserRole(adminUserId); if (adminRole !== 'admin') { @@ -270,6 +280,10 @@ export class UserManagementService { if (userId !== adminUserId) { await this.store.revokeUserSessions(userId); + } else if (currentSessionToken) { + await this.store.revokeOtherUserSessions(userId, currentSessionToken); + } else { + await this.store.revokeUserSessions(userId); } await this.store.clearUserPassword(userId); diff --git a/packages/idp-better-auth/src/store/DatabaseStore.ts b/packages/idp-better-auth/src/store/DatabaseStore.ts index 6c9cd91c3f..ea27bb3ba9 100644 --- a/packages/idp-better-auth/src/store/DatabaseStore.ts +++ b/packages/idp-better-auth/src/store/DatabaseStore.ts @@ -20,6 +20,12 @@ export interface DatabaseStore { userHasPassword(userId: string): Promise; clearUserPassword(userId: string): Promise; revokeUserSessions(userId: string): Promise; + /** + * Revoke every session for the user except the one identified by + * `exceptSessionToken`. Used for self-service password resets where the + * acting session must stay valid while all other sessions are invalidated. + */ + revokeOtherUserSessions(userId: string, exceptSessionToken: string): Promise; /** * Pre-provision a user record for a pending invitation. If a user with the diff --git a/packages/idp-better-auth/src/store/MysqlDatabaseStore.ts b/packages/idp-better-auth/src/store/MysqlDatabaseStore.ts index 3122209c9a..2767cd0cfe 100644 --- a/packages/idp-better-auth/src/store/MysqlDatabaseStore.ts +++ b/packages/idp-better-auth/src/store/MysqlDatabaseStore.ts @@ -204,6 +204,18 @@ export class MysqlDatabaseStore implements DatabaseStore { } } + async revokeOtherUserSessions(userId: string, exceptSessionToken: string): Promise { + const pool = await this.getPool(); + try { + await pool.execute('DELETE FROM session WHERE userId = ? AND token != ?', [ + userId, + exceptSessionToken, + ]); + } catch { + // Non-fatal: sessions might not exist + } + } + async updateUserName(userId: string, name: string): Promise { const pool = await this.getPool(); const [res] = (await pool.execute('UPDATE user SET name = ? WHERE id = ?', [name, userId])) as [ diff --git a/packages/idp-better-auth/src/store/SqliteDatabaseStore.session-revocation.test.ts b/packages/idp-better-auth/src/store/SqliteDatabaseStore.session-revocation.test.ts new file mode 100644 index 0000000000..2aa8a572eb --- /dev/null +++ b/packages/idp-better-auth/src/store/SqliteDatabaseStore.session-revocation.test.ts @@ -0,0 +1,56 @@ +import { afterEach, beforeEach, describe, expect, it } from '@jest/globals'; +import { SqliteDatabaseStore } from './SqliteDatabaseStore.js'; + +/** + * Verifies the raw SQL of the session-revocation helpers against a real + * in-memory SQLite database. + */ +describe('SqliteDatabaseStore — session revocation SQL', () => { + let store: SqliteDatabaseStore; + + beforeEach(async () => { + store = new SqliteDatabaseStore(':memory:'); + await store.connect(); + const db = (await store.getAdapter()) as { + prepare: (sql: string) => { run: (...args: unknown[]) => unknown }; + }; + db.prepare('CREATE TABLE session (token TEXT PRIMARY KEY, userId TEXT)').run(); + db.prepare('INSERT INTO session (token, userId) VALUES (?, ?)').run('tok-current', 'user-1'); + db.prepare('INSERT INTO session (token, userId) VALUES (?, ?)').run('tok-other', 'user-1'); + db.prepare('INSERT INTO session (token, userId) VALUES (?, ?)').run('tok-elsewhere', 'user-2'); + }); + + afterEach(async () => { + await store.shutdown(); + }); + + function rows(): Array<{ token: string; userId: string }> { + const adapter = ( + store as unknown as { + db: { prepare: (sql: string) => { all: () => Array<{ token: string; userId: string }> } }; + } + ).db; + return adapter.prepare('SELECT token, userId FROM session').all(); + } + + function tokensFor(userId: string): string[] { + return rows() + .filter(r => r.userId === userId) + .map(r => r.token); + } + + it('revokeOtherUserSessions deletes all of the user’s sessions except the current one', async () => { + await store.revokeOtherUserSessions('user-1', 'tok-current'); + + expect(tokensFor('user-1')).toEqual(['tok-current']); + // Other users are untouched. + expect(tokensFor('user-2')).toEqual(['tok-elsewhere']); + }); + + it('revokeUserSessions deletes every session for the user', async () => { + await store.revokeUserSessions('user-1'); + + expect(tokensFor('user-1')).toEqual([]); + expect(tokensFor('user-2')).toEqual(['tok-elsewhere']); + }); +}); diff --git a/packages/idp-better-auth/src/store/SqliteDatabaseStore.ts b/packages/idp-better-auth/src/store/SqliteDatabaseStore.ts index 2d7c877575..fbb4b66e09 100644 --- a/packages/idp-better-auth/src/store/SqliteDatabaseStore.ts +++ b/packages/idp-better-auth/src/store/SqliteDatabaseStore.ts @@ -172,6 +172,17 @@ export class SqliteDatabaseStore implements DatabaseStore { } } + async revokeOtherUserSessions(userId: string, exceptSessionToken: string): Promise { + await this.connect(); + try { + this.getDb() + .prepare('DELETE FROM session WHERE userId = ? AND token != ?') + .run(userId, exceptSessionToken); + } catch { + // Non-fatal: sessions might not exist + } + } + async updateUserName(userId: string, name: string): Promise { await this.connect(); const stmt = this.getDb().prepare('UPDATE user SET name = ? WHERE id = ?'); diff --git a/packages/idp-owox-better-auth/src/controllers/password-flow-controller.test.ts b/packages/idp-owox-better-auth/src/controllers/password-flow-controller.test.ts index c2ef1d4232..6d7bff7d53 100644 --- a/packages/idp-owox-better-auth/src/controllers/password-flow-controller.test.ts +++ b/packages/idp-owox-better-auth/src/controllers/password-flow-controller.test.ts @@ -169,6 +169,87 @@ describe('PasswordFlowController.setPassword', () => { expect(res.redirect).toHaveBeenCalledWith(`${AUTH_BASE_PATH}/password/success`); }); + it('revokes the OWOX platform refresh token after a successful reset', async () => { + const resetPassword = jest.fn<(_params: ResetPasswordParams) => Promise>( + async () => ({}) + ); + const auth = { + api: { + getSession: jest.fn(async () => ({ + user: { id: 'user-id', email: 'user@example.com' }, + })), + resetPassword, + setPassword: jest.fn(), + signOut: jest.fn(), + }, + } as unknown as BetterAuthInstance; + const revokePlatformSession = jest.fn<(req: Request, res: Response) => Promise>( + async () => {} + ); + const service = new PasswordFlowController( + auth, + {} as BetterAuthSessionService, + {} as MagicLinkService, + undefined, + revokePlatformSession + ); + const req = { + body: { + password: 'NewPassw0rd', + intent: MAGIC_LINK_INTENT.RESET, + token: 'reset-token', + }, + headers: baseHeaders, + } as unknown as Request; + const res = createResponseMock(); + + await service.setPassword(req, res); + + expect(resetPassword).toHaveBeenCalled(); + expect(revokePlatformSession).toHaveBeenCalledWith(req, res); + expect(res.redirect).toHaveBeenCalledWith(`${AUTH_BASE_PATH}/password/success`); + }); + + it('does not revoke the platform refresh token when the reset fails', async () => { + const resetPassword = jest.fn<(_params: ResetPasswordParams) => Promise>(async () => { + throw new Error('invalid token'); + }); + const auth = { + api: { + getSession: jest.fn(async () => ({ + user: { id: 'user-id', email: 'user@example.com' }, + })), + resetPassword, + setPassword: jest.fn(), + signOut: jest.fn(), + }, + } as unknown as BetterAuthInstance; + const revokePlatformSession = jest.fn<(req: Request, res: Response) => Promise>( + async () => {} + ); + const service = new PasswordFlowController( + auth, + {} as BetterAuthSessionService, + {} as MagicLinkService, + undefined, + revokePlatformSession + ); + const req = { + body: { + password: 'NewPassw0rd', + intent: MAGIC_LINK_INTENT.RESET, + token: 'reset-token', + }, + headers: baseHeaders, + } as unknown as Request; + const res = createResponseMock(); + + await service.setPassword(req, res); + + expect(revokePlatformSession).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(400); + }); + it('uses resetPassword when token is provided even without intent', async () => { const resetPassword = jest.fn<(_params: ResetPasswordParams) => Promise>( async () => ({}) diff --git a/packages/idp-owox-better-auth/src/controllers/password-flow-controller.ts b/packages/idp-owox-better-auth/src/controllers/password-flow-controller.ts index 874baeded7..0ae12c15db 100644 --- a/packages/idp-owox-better-auth/src/controllers/password-flow-controller.ts +++ b/packages/idp-owox-better-auth/src/controllers/password-flow-controller.ts @@ -28,7 +28,19 @@ export class PasswordFlowController { private readonly auth: BetterAuthInstance, private readonly sessionService: BetterAuthSessionService, private readonly magicLinkService: MagicLinkService, - private readonly gtmContainerId?: string + private readonly gtmContainerId?: string, + /** + * Best-effort revocation of the OWOX platform refresh token carried by the + * request (the `refreshToken` cookie), invoked after a successful password + * reset so the acting device's platform session is revoked at the OWOX + * Identity level, not only the Better Auth UI session. No-op when the + * request carries no platform refresh token (e.g. reset from a logged-out + * browser). + */ + private readonly revokePlatformSession?: ( + req: ExpressRequest, + res: ExpressResponse + ) => Promise ) {} async sendMagicLink(req: ExpressRequest, res: ExpressResponse): Promise { @@ -161,6 +173,10 @@ export class PasswordFlowController { body: { newPassword: password, token: resetToken }, headers: convertExpressHeaders(req), }); + // 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); clearBetterAuthCookies(res, req); return res.redirect(`${AUTH_BASE_PATH}/password/success`); } catch (error) { diff --git a/packages/idp-owox-better-auth/src/owox-better-auth-idp.ts b/packages/idp-owox-better-auth/src/owox-better-auth-idp.ts index f1a3acf089..073179dccc 100644 --- a/packages/idp-owox-better-auth/src/owox-better-auth-idp.ts +++ b/packages/idp-owox-better-auth/src/owox-better-auth-idp.ts @@ -152,7 +152,8 @@ export class OwoxBetterAuthIdp implements IdpProvider { this.auth, this.betterAuthSessionService, this.magicLinkService, - this.config.gtmContainerId + this.config.gtmContainerId, + this.revokePlatformRefreshTokenFromRequest.bind(this) ); this.authFlowMiddleware = new AuthFlowMiddleware( this.pageController, @@ -621,6 +622,38 @@ export class OwoxBetterAuthIdp implements IdpProvider { await this.tokenFacade.revokeToken(token); } + /** + * Best-effort revocation of the OWOX platform refresh token carried by the + * current request (the `refreshToken` cookie), then clears that cookie. + * + * Used after a password reset so the platform session tied to THIS request + * is invalidated at the OWOX Identity level — not only the Better Auth UI + * 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 + * device). Invalidating a user's OTHER devices would require a + * revoke-by-userId capability on OWOX Identity, which does not exist yet. + */ + private async revokePlatformRefreshTokenFromRequest( + req: e.Request, + res: e.Response + ): Promise { + const refreshToken = extractRefreshToken(req); + if (refreshToken) { + try { + await this.revokeToken(refreshToken); + } catch (error) { + this.logger.warn( + 'Failed to revoke OWOX refresh token during password flow', + undefined, + error instanceof Error ? error : undefined + ); + } + } + clearCookie(res, CORE_REFRESH_TOKEN_COOKIE, req); + } + async accessTokenMiddleware( req: e.Request, res: e.Response,