diff --git a/server/src/nest/auth/auth.controller.ts b/server/src/nest/auth/auth.controller.ts index 325c9a0bf..efd36e733 100644 --- a/server/src/nest/auth/auth.controller.ts +++ b/server/src/nest/auth/auth.controller.ts @@ -111,10 +111,14 @@ export class AuthController { async changePassword(@CurrentUser() user: User, @Body() body: ChangePasswordDto, @Req() req: Request, @Res({ passthrough: true }) res: Response) { await this.limit('login', req, 5); // Carry the session's remember choice into the re-issued token/cookie so a - // "remember me" login survives a password change (#1927). Bearer callers - // have no cookie → undefined → the historical default duration. - const remember = decodeSessionClaims((req.cookies as Record | undefined)?.trek_session)?.remember; - const result = await this.auth.changePassword(user.id, user.email, body, remember, sessionClientFrom(req)); + // "remember me" login survives a password change (#1927). The guard + // verified the cookie whenever there is one (extractToken prefers it), the + // same check the renewal interceptor makes. A Bearer caller has none: it + // would never see a re-issued token, so no new session is started for it. + const cookie = (req.cookies as Record | undefined)?.trek_session; + const remember = decodeSessionClaims(cookie)?.remember; + const cookieSession = typeof cookie === 'string' && cookie.length > 0; + const result = await this.auth.changePassword(user.id, user.email, body, remember, sessionClientFrom(req), cookieSession); if (result.error) { throw new HttpException({ error: result.error }, result.status!); } diff --git a/server/src/nest/auth/auth.service.ts b/server/src/nest/auth/auth.service.ts index f95b9b56f..41d8481ca 100644 --- a/server/src/nest/auth/auth.service.ts +++ b/server/src/nest/auth/auth.service.ts @@ -624,6 +624,7 @@ export class AuthService { rawBody: unknown, remember?: boolean, client?: SessionClient, + issueSession = true, ): Promise<{ error?: string; status?: number; success?: boolean; token?: string }> { const body = rawBody as { current_password?: string; new_password?: string }; if (await this.isOidcOnlyMode()) { @@ -654,9 +655,7 @@ export class AuthService { // before sessions were tracked) and prunes the MCP static token and OAuth // bearer-token stores to match, the same set the password reset revokes. await this.mcpTokens.deleteAllForUser(userId); - try { - await this.oauthTokens.revokeAllForUser(userId); - } catch { /* oauth_tokens table may not exist in very old installs */ } + try { await this.oauthTokens.revokeAllForUser(userId); } catch { /* no oauth_tokens in very old installs */ } // Push devices keep receiving notifications without any session, so they // go too. The device the change was made on registers again right away: // the client reloads the user after the change, and that re-syncs its @@ -667,8 +666,9 @@ export class AuthService { try { revokeUserSessions?.(userId); } catch { /* best-effort */ } - // Re-issue a session for the current device only, keeping the login's remember - // choice instead of downgrading it to the default duration (#1927). + // Re-issue a session for the current device only, with the login's remember choice (#1927). + // Not for a Bearer caller (`issueSession` false): it never receives the token (see the controller). + if (!issueSession) return { success: true }; const token = await this.generateToken({ id: userId, password_version: newPv }, remember, client); return { success: true, token }; } diff --git a/server/tests/e2e/sessions.e2e.test.ts b/server/tests/e2e/sessions.e2e.test.ts index aa785f98c..ad798f6c7 100644 --- a/server/tests/e2e/sessions.e2e.test.ts +++ b/server/tests/e2e/sessions.e2e.test.ts @@ -213,6 +213,23 @@ describe('Sessions e2e (sign-in sessions that can be ended)', () => { expect(list.body.sessions).toEqual([expect.objectContaining({ id: sessionIdOf(next), user_agent: 'Here', current: true })]); }, 15000); + it('a password change over a Bearer token ends every session and starts none, since no cookie carries it', async () => { + const { user, password } = freshUser('sess-password-bearer'); + const cookie = await signIn(user.email, password, 'Api client'); + const bearer = cookie.slice('trek_session='.length); + + const change = await request(server) + .put('/api/auth/me/password') + .set('Authorization', `Bearer ${bearer}`) + .send({ current_password: password, new_password: 'New1234!x' }); + expect(change.status).toBe(200); + expect(change.body).toEqual({ success: true }); + expect(((change.headers['set-cookie'] ?? []) as unknown as string[]).some((c) => c.startsWith('trek_session='))).toBe(false); + + const rows = sessionRows(db as never, user.id); + expect(rows).toEqual([expect.objectContaining({ id: sessionIdOf(cookie), revoked_at: expect.any(String) })]); + }, 15000); + it('a token from before sessions were tracked keeps working, is listed as untracked, and survives "sign out others"', async () => { const { user, password } = freshUser('sess-legacy'); const legacy = sessionCookie(user.id); diff --git a/server/tests/unit/nest/auth.controller.test.ts b/server/tests/unit/nest/auth.controller.test.ts index b01291be5..5d60d96dd 100644 --- a/server/tests/unit/nest/auth.controller.test.ts +++ b/server/tests/unit/nest/auth.controller.test.ts @@ -352,10 +352,20 @@ describe('AuthController (authenticated)', () => { const changePassword = vi.fn().mockReturnValue({ token: 'tk3' }); const c = ac(asvc({ changePassword, setAuthCookie } as Partial), rl()); expect(await c.changePassword(user, anyBody(), reqCookie, res)).toEqual({ success: true }); - expect(changePassword).toHaveBeenCalledWith(1, 'u@example.test', anyBody(), true, { userAgent: null }); + expect(changePassword).toHaveBeenCalledWith(1, 'u@example.test', anyBody(), true, { userAgent: null }, true); expect(setAuthCookie).toHaveBeenCalledWith(res, 'tk3', reqCookie, true); }); + it('change-password starts no new session for a Bearer caller, who has no cookie to receive it', async () => { + const setAuthCookie = vi.fn(); + const changePassword = vi.fn().mockReturnValue({ success: true }); + const bearer = { ip: '9.9.9.9', headers: { authorization: 'Bearer abc' } } as unknown as Request; + const c = ac(asvc({ changePassword, setAuthCookie } as Partial), rl()); + expect(await c.changePassword(user, anyBody(), bearer, res)).toEqual({ success: true }); + expect(changePassword).toHaveBeenCalledWith(1, 'u@example.test', anyBody(), undefined, { userAgent: null }, false); + expect(setAuthCookie).not.toHaveBeenCalled(); + }); + it('delete-account maps error, else audits and succeeds', async () => { expect(await thrownAsync(() => ac(asvc({ deleteAccount: vi.fn().mockReturnValue({ error: 'Last admin', status: 403 }) } as Partial), rl()).deleteAccount(user, req))).toEqual({ status: 403, body: { error: 'Last admin' } }); expect(await ac(asvc({ deleteAccount: vi.fn().mockReturnValue({}) } as Partial), rl()).deleteAccount(user, req)).toEqual({ success: true }); diff --git a/server/tests/unit/nest/auth.service.test.ts b/server/tests/unit/nest/auth.service.test.ts index 36382fae7..f803b5987 100644 --- a/server/tests/unit/nest/auth.service.test.ts +++ b/server/tests/unit/nest/auth.service.test.ts @@ -528,6 +528,19 @@ describe('changePassword — session invalidation', () => { expect(await verifyJwtAndLoadUser(stolen, await createTestUsersRepo(testDb), await createTestUserSessionsRepo(testDb))).toBeNull(); // invalidated by the pv bump }); + it('AUTH-DB-036g: without issueSession it ends every session and starts none (a Bearer caller)', async () => { + const { user, password } = createUser(testDb); + await svc.generateToken({ id: user.id }); + + const result = await svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' }, undefined, {}, false); + + expect(result).toEqual({ success: true }); + expect(pvOf(user.id)).toBe(1); + const rows = sessionRows(testDb, user.id); + expect(rows).toHaveLength(1); + expect(rows.every((row) => row.revoked_at !== null)).toBe(true); + }); + it('AUTH-DB-036d: preserves the remember choice in the re-issued session (#1927)', async () => { // eslint-disable-next-line @typescript-eslint/no-require-imports const jwt = require('jsonwebtoken'); diff --git a/wiki/Security-Hardening.md b/wiki/Security-Hardening.md index 38e624c4f..295833505 100644 --- a/wiki/Security-Hardening.md +++ b/wiki/Security-Hardening.md @@ -42,7 +42,7 @@ A session ends when: | **Log out** (`POST /api/auth/logout`) | The one it is called with. Clearing the cookie alone used to leave the token valid. | | `DELETE /api/auth/sessions/{id}` | That one session of your own account. | | `POST /api/auth/sessions/revoke-others` | Every session of your account but the current one. | -| Password change | Every session; the device the change was made on gets a new one. | +| Password change | Every session. A browser that made the change with its session cookie gets a new one; an API client calling it with a Bearer token gets none, since it would never receive it. | | Password reset by email | Every session. | | Admin sets a new password for a user | Every session of that user. | | Recovery script `reset-admin.js` | Every session of the account it resets. |