Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/idp-better-auth/jest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
}));
Original file line number Diff line number Diff line change
@@ -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<typeof Database>;

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);
});
});
18 changes: 18 additions & 0 deletions packages/idp-better-auth/src/auth/auth-config.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -110,6 +111,23 @@ export async function createBetterAuthConfig(
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)

// 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: {
Expand Down
3 changes: 2 additions & 1 deletion packages/idp-better-auth/src/services/page-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,83 @@ describe('UserManagementService — inviteAndCreateStub', () => {
);
});
});

describe('UserManagementService — resetUserPassword (session revocation)', () => {
let store: jest.Mocked<DatabaseStore>;
let magicLinkService: jest.Mocked<MagicLinkService>;
let cryptoService: jest.Mocked<CryptoService>;

const ADMIN_ID = 'admin-1';
const TARGET_ID = 'target-1';
const CURRENT_TOKEN = 'current-session-token';

beforeEach(() => {
store = {
getUserRole: jest.fn<DatabaseStore['getUserRole']>(),
getUserById: jest.fn<DatabaseStore['getUserById']>(),
revokeUserSessions: jest.fn<DatabaseStore['revokeUserSessions']>(),
revokeOtherUserSessions: jest.fn<DatabaseStore['revokeOtherUserSessions']>(),
clearUserPassword: jest.fn<DatabaseStore['clearUserPassword']>(),
} as unknown as jest.Mocked<DatabaseStore>;
magicLinkService = {
generateMagicLink: jest.fn<MagicLinkService['generateMagicLink']>(),
} as unknown as jest.Mocked<MagicLinkService>;
cryptoService = {} as unknown as jest.Mocked<CryptoService>;

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<typeof UserManagementService>[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();
});
});
16 changes: 15 additions & 1 deletion packages/idp-better-auth/src/services/user-management-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand All @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions packages/idp-better-auth/src/store/DatabaseStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ export interface DatabaseStore {
userHasPassword(userId: string): Promise<boolean>;
clearUserPassword(userId: string): Promise<void>;
revokeUserSessions(userId: string): Promise<void>;
/**
* 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<void>;

/**
* Pre-provision a user record for a pending invitation. If a user with the
Expand Down
12 changes: 12 additions & 0 deletions packages/idp-better-auth/src/store/MysqlDatabaseStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,18 @@ export class MysqlDatabaseStore implements DatabaseStore {
}
}

async revokeOtherUserSessions(userId: string, exceptSessionToken: string): Promise<void> {
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<void> {
const pool = await this.getPool();
const [res] = (await pool.execute('UPDATE user SET name = ? WHERE id = ?', [name, userId])) as [
Expand Down
Original file line number Diff line number Diff line change
@@ -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']);
});
});
11 changes: 11 additions & 0 deletions packages/idp-better-auth/src/store/SqliteDatabaseStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,17 @@ export class SqliteDatabaseStore implements DatabaseStore {
}
}

async revokeOtherUserSessions(userId: string, exceptSessionToken: string): Promise<void> {
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<void> {
await this.connect();
const stmt = this.getDb().prepare('UPDATE user SET name = ? WHERE id = ?');
Expand Down
Loading
Loading