diff --git a/package.json b/package.json index a38faf99..738e6835 100644 --- a/package.json +++ b/package.json @@ -132,6 +132,8 @@ "node": ">=22.0.0" }, "overrides": { - "lodash-es": "^4.18.1" + "lodash": "^4.18.1", + "lodash-es": "^4.18.1", + "brace-expansion": "^2.0.3" } } diff --git a/src/api/routers/users.ts b/src/api/routers/users.ts index b81ab6af..cb26009c 100644 --- a/src/api/routers/users.ts +++ b/src/api/routers/users.ts @@ -4,6 +4,7 @@ import { z } from 'zod'; import { createUser, deleteUser, + deleteUserSessions, getUserById, listOrgUsers, updateUser, @@ -122,6 +123,12 @@ export const usersRouter = router({ } await updateUser(input.id, updates); + + // Invalidate all sessions for the target user when their password changes. + // This prevents stale sessions from remaining valid after a password reset. + if (updates.passwordHash !== undefined) { + await deleteUserSessions(input.id); + } }), delete: adminProcedure.input(z.object({ id: z.string() })).mutation(async ({ ctx, input }) => { diff --git a/src/db/repositories/usersRepository.ts b/src/db/repositories/usersRepository.ts index a7cddeb7..f927ae8f 100644 --- a/src/db/repositories/usersRepository.ts +++ b/src/db/repositories/usersRepository.ts @@ -84,6 +84,21 @@ export async function deleteExpiredSessions(): Promise { await db.delete(sessions).where(lt(sessions.expiresAt, new Date())); } +/** + * Delete all sessions for a given user. Optionally exclude a specific token + * (e.g. to preserve the caller's own session when they change their own password). + */ +export async function deleteUserSessions(userId: string, excludeToken?: string): Promise { + const db = getDb(); + if (excludeToken !== undefined) { + await db + .delete(sessions) + .where(and(eq(sessions.userId, userId), ne(sessions.token, excludeToken))); + } else { + await db.delete(sessions).where(eq(sessions.userId, userId)); + } +} + // ============================================================================ // CRUD for users (org-scoped) // ============================================================================ diff --git a/tests/unit/api/routers/users.test.ts b/tests/unit/api/routers/users.test.ts index 11b6493a..5cd943e3 100644 --- a/tests/unit/api/routers/users.test.ts +++ b/tests/unit/api/routers/users.test.ts @@ -8,6 +8,7 @@ const { mockUpdateUser, mockDeleteUser, mockGetUserById, + mockDeleteUserSessions, mockBcryptHash, } = vi.hoisted(() => ({ mockListOrgUsers: vi.fn(), @@ -15,6 +16,7 @@ const { mockUpdateUser: vi.fn(), mockDeleteUser: vi.fn(), mockGetUserById: vi.fn(), + mockDeleteUserSessions: vi.fn(), mockBcryptHash: vi.fn(), })); @@ -24,6 +26,7 @@ vi.mock('../../../../src/db/repositories/usersRepository.js', () => ({ updateUser: mockUpdateUser, deleteUser: mockDeleteUser, getUserById: mockGetUserById, + deleteUserSessions: mockDeleteUserSessions, })); vi.mock('bcrypt', () => ({ @@ -43,6 +46,7 @@ const mockMember = createMockUser({ id: 'member-1', role: 'member' }); describe('usersRouter', () => { beforeEach(() => { mockBcryptHash.mockResolvedValue('hashed-password'); + mockDeleteUserSessions.mockResolvedValue(undefined); }); describe('list', () => { @@ -406,6 +410,36 @@ describe('usersRouter', () => { code: 'FORBIDDEN', }); }); + + it('invalidates all sessions when password is changed', async () => { + mockGetUserById.mockResolvedValue({ id: 'user-2', orgId: 'org-1', role: 'member' }); + mockUpdateUser.mockResolvedValue(undefined); + const caller = createCaller({ user: mockAdminUser, effectiveOrgId: mockAdminUser.orgId }); + + await caller.update({ id: 'user-2', password: 'newpassword12' }); + + expect(mockDeleteUserSessions).toHaveBeenCalledWith('user-2'); + }); + + it('does not invalidate sessions when password is not changed', async () => { + mockGetUserById.mockResolvedValue({ id: 'user-2', orgId: 'org-1', role: 'member' }); + mockUpdateUser.mockResolvedValue(undefined); + const caller = createCaller({ user: mockAdminUser, effectiveOrgId: mockAdminUser.orgId }); + + await caller.update({ id: 'user-2', name: 'New Name' }); + + expect(mockDeleteUserSessions).not.toHaveBeenCalled(); + }); + + it('does not invalidate sessions when only role/email are changed', async () => { + mockGetUserById.mockResolvedValue({ id: 'user-2', orgId: 'org-1', role: 'member' }); + mockUpdateUser.mockResolvedValue(undefined); + const caller = createCaller({ user: mockAdminUser, effectiveOrgId: mockAdminUser.orgId }); + + await caller.update({ id: 'user-2', email: 'new@example.com' }); + + expect(mockDeleteUserSessions).not.toHaveBeenCalled(); + }); }); describe('delete', () => { diff --git a/tests/unit/db/repositories/usersRepository.test.ts b/tests/unit/db/repositories/usersRepository.test.ts index 61c37f97..ccb1b546 100644 --- a/tests/unit/db/repositories/usersRepository.test.ts +++ b/tests/unit/db/repositories/usersRepository.test.ts @@ -29,6 +29,7 @@ import { deleteExpiredSessions, deleteSession, deleteUser, + deleteUserSessions, getSessionByToken, getUserByEmail, getUserById, @@ -288,4 +289,34 @@ describe('usersRepository', () => { expect(mockDb.db.delete).toHaveBeenCalledTimes(1); }); }); + + describe('deleteUserSessions', () => { + it('deletes all sessions for a user', async () => { + mockDb.chain.where.mockResolvedValueOnce(undefined); + + await deleteUserSessions('user-1'); + + expect(mockDb.db.delete).toHaveBeenCalledTimes(1); + }); + + it('deletes all sessions when excludeToken is not provided', async () => { + mockDb.chain.where.mockResolvedValueOnce(undefined); + + await deleteUserSessions('user-1'); + + // Without excludeToken the where clause uses a single eq condition (no and/ne) + expect(mockDb.db.delete).toHaveBeenCalledTimes(1); + expect(mockDb.chain.where).toHaveBeenCalledTimes(1); + }); + + it('excludes a specific token when provided', async () => { + mockDb.chain.where.mockResolvedValueOnce(undefined); + + await deleteUserSessions('user-1', 'keep-this-token'); + + // With excludeToken the where clause uses an and(eq, ne) condition + expect(mockDb.db.delete).toHaveBeenCalledTimes(1); + expect(mockDb.chain.where).toHaveBeenCalledTimes(1); + }); + }); });