mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(auth): start no new session for a password change over a Bearer token
A password change ends every session of the user and re-issues one for the device it was made on, handed out as the session cookie. A caller that authenticated with a Bearer token never reads that cookie, so the new session sat in the session list as a live device nobody held until it expired. The controller now asks for the replacement session only when the request came with the session cookie, the same check the sliding renewal makes. The response body is unchanged.
This commit is contained in:
@@ -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<string, string> | 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<string, string> | 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!);
|
||||
}
|
||||
|
||||
@@ -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 };
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -352,10 +352,20 @@ describe('AuthController (authenticated)', () => {
|
||||
const changePassword = vi.fn().mockReturnValue({ token: 'tk3' });
|
||||
const c = ac(asvc({ changePassword, setAuthCookie } as Partial<AuthService>), 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<AuthService>), 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<AuthService>), rl()).deleteAccount(user, req))).toEqual({ status: 403, body: { error: 'Last admin' } });
|
||||
expect(await ac(asvc({ deleteAccount: vi.fn().mockReturnValue({}) } as Partial<AuthService>), rl()).deleteAccount(user, req)).toEqual({ success: true });
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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. |
|
||||
|
||||
Reference in New Issue
Block a user