mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(auth): tolerate refresh-token rotation races
Parallel refresh requests from a single client (multi-tab SPA, mobile tab restore) tripped reuse-detection and logged users out every few hours. Track rotation provenance via new rotated_at + replaced_by_token_hash columns and accept revoked- token reuse only inside a configurable grace window with a chain that reaches a live, non-logout link. Scope the rotate UPDATE with revoked_at IS NULL so a concurrent logout or admin revoke fails the racing refresh instead of handing the user a fresh access token. Closes #120
This commit is contained in:
@@ -66,10 +66,7 @@ export function useAuth() {
|
||||
async function init(): Promise<void> {
|
||||
isLoading.value = true
|
||||
try {
|
||||
const res = await fetch('/api/v1/auth/refresh', { method: 'POST', credentials: 'include' })
|
||||
if (!res.ok) return
|
||||
const { accessToken } = await res.json()
|
||||
setAccessToken(accessToken)
|
||||
await refreshAccessToken()
|
||||
await me()
|
||||
startSessionRefresh()
|
||||
} catch {
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { api, refreshAccessToken, setAccessToken, setOnAuthFailure } from '@/lib/api'
|
||||
|
||||
describe('api wrapper', () => {
|
||||
const originalFetch = globalThis.fetch
|
||||
|
||||
beforeEach(() => {
|
||||
setAccessToken(null)
|
||||
setOnAuthFailure(() => {})
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
globalThis.fetch = originalFetch
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
it('coalesces concurrent refreshes into a single network call (single-flight)', async () => {
|
||||
let refreshResolvers: ((value: Response) => void)[] = []
|
||||
const fetchMock = vi.fn<typeof fetch>((input: RequestInfo | URL) => {
|
||||
const url = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url
|
||||
if (url.endsWith('/api/v1/auth/refresh')) {
|
||||
return new Promise<Response>((resolve) => {
|
||||
refreshResolvers.push(resolve)
|
||||
})
|
||||
}
|
||||
return Promise.resolve(new Response(JSON.stringify({ ok: true }), { status: 200 }))
|
||||
})
|
||||
globalThis.fetch = fetchMock as never
|
||||
|
||||
const p1 = refreshAccessToken().catch(() => null)
|
||||
const p2 = refreshAccessToken().catch(() => null)
|
||||
const p3 = refreshAccessToken().catch(() => null)
|
||||
|
||||
// All three callers share one in-flight request.
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1)
|
||||
|
||||
refreshResolvers[0]!(new Response(JSON.stringify({ accessToken: 'new-token' }), { status: 200 }))
|
||||
const results = await Promise.all([p1, p2, p3])
|
||||
expect(results).toEqual(['new-token', 'new-token', 'new-token'])
|
||||
|
||||
// After the in-flight promise settles, the next call should open a new flight.
|
||||
refreshResolvers = []
|
||||
const p4 = refreshAccessToken().catch(() => null)
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2)
|
||||
refreshResolvers[0]!(new Response(JSON.stringify({ accessToken: 'next-token' }), { status: 200 }))
|
||||
await p4
|
||||
})
|
||||
|
||||
it('on 401, refreshes once and retries the original request with the new token', async () => {
|
||||
const calls: { url: string; auth: string | null }[] = []
|
||||
let refreshed = false
|
||||
const fetchMock = vi.fn<typeof fetch>((input: RequestInfo | URL, init?: RequestInit) => {
|
||||
const url = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url
|
||||
const headers = new Headers(init?.headers)
|
||||
calls.push({ url, auth: headers.get('Authorization') })
|
||||
if (url.endsWith('/api/v1/auth/refresh')) {
|
||||
refreshed = true
|
||||
return Promise.resolve(new Response(JSON.stringify({ accessToken: 'fresh-token' }), { status: 200 }))
|
||||
}
|
||||
if (!refreshed) return Promise.resolve(new Response('', { status: 401 }))
|
||||
return Promise.resolve(new Response(JSON.stringify({ ok: true }), { status: 200 }))
|
||||
})
|
||||
globalThis.fetch = fetchMock as never
|
||||
setAccessToken('stale-token')
|
||||
|
||||
const res = await api('/api/v1/books/1')
|
||||
expect(res.status).toBe(200)
|
||||
// 1st: original with stale token -> 401, 2nd: refresh, 3rd: retry with fresh token
|
||||
expect(calls.map((c) => c.url)).toEqual(['/api/v1/books/1', '/api/v1/auth/refresh', '/api/v1/books/1'])
|
||||
expect(calls[0]!.auth).toBe('Bearer stale-token')
|
||||
expect(calls[2]!.auth).toBe('Bearer fresh-token')
|
||||
})
|
||||
|
||||
it('invokes onAuthFailure when refresh itself fails', async () => {
|
||||
const fetchMock = vi.fn<typeof fetch>((input: RequestInfo | URL) => {
|
||||
const url = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url
|
||||
if (url.endsWith('/api/v1/auth/refresh')) return Promise.resolve(new Response('', { status: 401 }))
|
||||
return Promise.resolve(new Response('', { status: 401 }))
|
||||
})
|
||||
globalThis.fetch = fetchMock as never
|
||||
|
||||
const onFail = vi.fn<() => void>()
|
||||
setOnAuthFailure(onFail)
|
||||
await expect(api('/api/v1/books/1')).rejects.toThrow('Session expired')
|
||||
expect(onFail).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
@@ -71,6 +71,7 @@ describe('config', () => {
|
||||
jwtExpiresIn: '15m',
|
||||
jwtRefreshExpiresIn: '7d',
|
||||
setupBootstrapToken: '',
|
||||
refreshRotationGraceMs: 30_000,
|
||||
});
|
||||
expect(emailConfig().encryptionKey).toBe('');
|
||||
expect(migrationConfig().encryptionKey).toBe('');
|
||||
|
||||
@@ -17,6 +17,7 @@ export const authConfig = registerAs('auth', () => ({
|
||||
jwtExpiresIn: process.env.JWT_EXPIRES_IN ?? '15m',
|
||||
jwtRefreshExpiresIn: process.env.JWT_REFRESH_EXPIRES_IN ?? '7d',
|
||||
setupBootstrapToken: process.env.SETUP_BOOTSTRAP_TOKEN ?? '',
|
||||
refreshRotationGraceMs: parsePositiveInteger(process.env.AUTH_REFRESH_ROTATION_GRACE_MS, 30_000),
|
||||
}));
|
||||
|
||||
export const storageConfig = registerAs('storage', () => ({
|
||||
|
||||
@@ -0,0 +1,2 @@
|
||||
ALTER TABLE "refresh_tokens" ADD COLUMN "rotated_at" timestamp with time zone;--> statement-breakpoint
|
||||
ALTER TABLE "refresh_tokens" ADD COLUMN "replaced_by_token_hash" varchar(64);
|
||||
File diff suppressed because it is too large
Load Diff
@@ -57,6 +57,13 @@
|
||||
"when": 1779391608076,
|
||||
"tag": "0007_add_content_filter_tables",
|
||||
"breakpoints": true
|
||||
},
|
||||
{
|
||||
"idx": 8,
|
||||
"version": "7",
|
||||
"when": 1779941161092,
|
||||
"tag": "0008_auth_refresh_rotation_metadata",
|
||||
"breakpoints": true
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
@@ -99,6 +99,8 @@ export const refreshTokens = pgTable(
|
||||
tokenHash: varchar('token_hash', { length: 255 }).notNull().unique(),
|
||||
expiresAt: timestamp('expires_at', { withTimezone: true }).notNull(),
|
||||
revokedAt: timestamp('revoked_at', { withTimezone: true }),
|
||||
rotatedAt: timestamp('rotated_at', { withTimezone: true }),
|
||||
replacedByTokenHash: varchar('replaced_by_token_hash', { length: 64 }),
|
||||
createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(),
|
||||
},
|
||||
(t) => [
|
||||
|
||||
@@ -27,7 +27,7 @@ function makeDb(overrides?: Record<string, unknown>) {
|
||||
from: vi.fn().mockResolvedValue([{ total: 0 }]),
|
||||
update: vi.fn().mockReturnThis(),
|
||||
set: vi.fn().mockReturnThis(),
|
||||
where: vi.fn().mockResolvedValue(undefined),
|
||||
where: vi.fn().mockResolvedValue({ rowCount: 1 }),
|
||||
delete: vi.fn().mockReturnThis(),
|
||||
transaction: vi.fn().mockImplementation(async (fn: (tx: unknown) => Promise<unknown>) => fn(db)),
|
||||
};
|
||||
@@ -81,6 +81,7 @@ function makeService(dbOverrides?: Record<string, unknown>) {
|
||||
if (key === 'auth.jwtExpiresIn') return '15m';
|
||||
if (key === 'app.nodeEnv') return 'test';
|
||||
if (key === 'auth.setupBootstrapToken') return 'bootstrap-token';
|
||||
if (key === 'auth.refreshRotationGraceMs') return 30_000;
|
||||
return undefined;
|
||||
}),
|
||||
};
|
||||
@@ -452,7 +453,80 @@ describe('AuthService', () => {
|
||||
await expect(service.refresh(makeRequest({ refresh_token: 'unknown-token' }), makeReply())).rejects.toThrow(UnauthorizedException);
|
||||
});
|
||||
|
||||
it('throws UnauthorizedException and revokes all sessions when revoked token is reused', async () => {
|
||||
it('returns access-only success for a recently rotated token reuse', async () => {
|
||||
const { service, db, jwtService } = makeService();
|
||||
const reply = makeReply();
|
||||
const rotatedAt = new Date();
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).refreshTokens.findFirst
|
||||
.mockResolvedValueOnce({
|
||||
id: 1,
|
||||
userId: 5,
|
||||
revokedAt: rotatedAt,
|
||||
rotatedAt,
|
||||
replacedByTokenHash: 'replacement-hash',
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
id: 2,
|
||||
userId: 5,
|
||||
revokedAt: null,
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
});
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).users.findFirst.mockResolvedValue({
|
||||
id: 5,
|
||||
tokenVersion: 7,
|
||||
active: true,
|
||||
provisioningMethod: 'local',
|
||||
});
|
||||
|
||||
await expect(service.refresh(makeRequest({ refresh_token: 'recently-rotated-token' }), reply)).resolves.toEqual({ accessToken: 'signed-jwt' });
|
||||
|
||||
expect(jwtService.sign).toHaveBeenCalledWith({ sub: 5, ver: 7 });
|
||||
expect(db.transaction).not.toHaveBeenCalled();
|
||||
expect(db.update).not.toHaveBeenCalled();
|
||||
expect(db.delete).not.toHaveBeenCalled();
|
||||
|
||||
const setCookieCalls = (reply as unknown as { setCookie: vi.Mock }).setCookie.mock.calls;
|
||||
expect(setCookieCalls.map(([name]) => name)).toEqual(['access_token']);
|
||||
expect(setCookieCalls[0]).toEqual(['access_token', 'signed-jwt', expect.objectContaining({ path: '/api', maxAge: 900 })]);
|
||||
});
|
||||
|
||||
it('does not resurrect a recently rotated token after its replacement session is revoked', async () => {
|
||||
const { service, db } = makeService();
|
||||
const reply = makeReply();
|
||||
const rotatedAt = new Date();
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).refreshTokens.findFirst
|
||||
.mockResolvedValueOnce({
|
||||
id: 1,
|
||||
userId: 5,
|
||||
revokedAt: rotatedAt,
|
||||
rotatedAt,
|
||||
replacedByTokenHash: 'replacement-hash',
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
id: 2,
|
||||
userId: 5,
|
||||
revokedAt: new Date(),
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
});
|
||||
|
||||
await expect(service.refresh(makeRequest({ refresh_token: 'recently-rotated-token' }), reply)).rejects.toThrow(UnauthorizedException);
|
||||
expect(db.transaction).toHaveBeenCalled();
|
||||
expect(db.delete).toHaveBeenCalled();
|
||||
expect((reply as unknown as { setCookie: vi.Mock }).setCookie).toHaveBeenCalledWith(
|
||||
'refresh_token',
|
||||
'',
|
||||
expect.objectContaining({ path: '/api/v1/auth', maxAge: 0 }),
|
||||
);
|
||||
expect((reply as unknown as { setCookie: vi.Mock }).setCookie).toHaveBeenCalledWith(
|
||||
'access_token',
|
||||
'',
|
||||
expect.objectContaining({ path: '/api', maxAge: 0 }),
|
||||
);
|
||||
});
|
||||
|
||||
it('does not accept a recently revoked token unless it was revoked by refresh rotation', async () => {
|
||||
const { service, db } = makeService();
|
||||
const reply = makeReply();
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).refreshTokens.findFirst.mockResolvedValue({
|
||||
@@ -477,6 +551,317 @@ describe('AuthService', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('revokes all sessions when a rotated token is reused after the grace window', async () => {
|
||||
const { service, db } = makeService();
|
||||
const reply = makeReply();
|
||||
const rotatedAt = new Date(Date.now() - 31_000);
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).refreshTokens.findFirst.mockResolvedValue({
|
||||
id: 1,
|
||||
userId: 5,
|
||||
revokedAt: rotatedAt,
|
||||
rotatedAt,
|
||||
replacedByTokenHash: 'replacement-hash',
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
});
|
||||
|
||||
await expect(service.refresh(makeRequest({ refresh_token: 'old-rotated-token' }), reply)).rejects.toThrow(UnauthorizedException);
|
||||
expect(db.transaction).toHaveBeenCalled();
|
||||
expect(db.delete).toHaveBeenCalled();
|
||||
expect((reply as unknown as { setCookie: vi.Mock }).setCookie).toHaveBeenCalledWith(
|
||||
'refresh_token',
|
||||
'',
|
||||
expect.objectContaining({ path: '/api/v1/auth', maxAge: 0 }),
|
||||
);
|
||||
expect((reply as unknown as { setCookie: vi.Mock }).setCookie).toHaveBeenCalledWith(
|
||||
'access_token',
|
||||
'',
|
||||
expect.objectContaining({ path: '/api', maxAge: 0 }),
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects a recently rotated token reuse for disabled users and clears cookies', async () => {
|
||||
const { service, db } = makeService();
|
||||
const reply = makeReply();
|
||||
const rotatedAt = new Date();
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).refreshTokens.findFirst
|
||||
.mockResolvedValueOnce({
|
||||
id: 1,
|
||||
userId: 5,
|
||||
revokedAt: rotatedAt,
|
||||
rotatedAt,
|
||||
replacedByTokenHash: 'replacement-hash',
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
id: 2,
|
||||
userId: 5,
|
||||
revokedAt: null,
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
});
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).users.findFirst.mockResolvedValue({
|
||||
id: 5,
|
||||
tokenVersion: 7,
|
||||
active: false,
|
||||
provisioningMethod: 'local',
|
||||
});
|
||||
|
||||
await expect(service.refresh(makeRequest({ refresh_token: 'recently-rotated-token' }), reply)).rejects.toThrow(UnauthorizedException);
|
||||
expect((reply as unknown as { setCookie: vi.Mock }).setCookie).toHaveBeenCalledWith(
|
||||
'refresh_token',
|
||||
'',
|
||||
expect.objectContaining({ path: '/api/v1/auth', maxAge: 0 }),
|
||||
);
|
||||
expect((reply as unknown as { setCookie: vi.Mock }).setCookie).toHaveBeenCalledWith(
|
||||
'access_token',
|
||||
'',
|
||||
expect.objectContaining({ path: '/api', maxAge: 0 }),
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects a recently rotated shared-user token reuse when all magic links are revoked', async () => {
|
||||
const { service, db, magicLinkRepo } = makeService();
|
||||
const reply = makeReply();
|
||||
const rotatedAt = new Date();
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).refreshTokens.findFirst
|
||||
.mockResolvedValueOnce({
|
||||
id: 1,
|
||||
userId: 5,
|
||||
revokedAt: rotatedAt,
|
||||
rotatedAt,
|
||||
replacedByTokenHash: 'replacement-hash',
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
id: 2,
|
||||
userId: 5,
|
||||
revokedAt: null,
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
});
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).users.findFirst.mockResolvedValue({
|
||||
id: 5,
|
||||
tokenVersion: 7,
|
||||
active: true,
|
||||
provisioningMethod: 'shared',
|
||||
});
|
||||
magicLinkRepo.hasActiveByUserId.mockResolvedValue(false);
|
||||
|
||||
await expect(service.refresh(makeRequest({ refresh_token: 'recently-rotated-token' }), reply)).rejects.toThrow(UnauthorizedException);
|
||||
expect(db.transaction).toHaveBeenCalled();
|
||||
expect((reply as unknown as { setCookie: vi.Mock }).setCookie).toHaveBeenCalledWith(
|
||||
'refresh_token',
|
||||
'',
|
||||
expect.objectContaining({ path: '/api/v1/auth', maxAge: 0 }),
|
||||
);
|
||||
expect((reply as unknown as { setCookie: vi.Mock }).setCookie).toHaveBeenCalledWith(
|
||||
'access_token',
|
||||
'',
|
||||
expect.objectContaining({ path: '/api', maxAge: 0 }),
|
||||
);
|
||||
});
|
||||
|
||||
it('returns access-only success across a chained rotation (N -> N+1 -> live N+2) within grace', async () => {
|
||||
const { service, db, jwtService } = makeService();
|
||||
const reply = makeReply();
|
||||
const rotatedAt = new Date(Date.now() - 5_000);
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).refreshTokens.findFirst
|
||||
// 1st findFirst: the presented (oldest) token, already rotated to hash-1
|
||||
.mockResolvedValueOnce({
|
||||
id: 1,
|
||||
userId: 5,
|
||||
revokedAt: rotatedAt,
|
||||
rotatedAt,
|
||||
replacedByTokenHash: 'hash-1',
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
})
|
||||
// 2nd findFirst: intermediate link, itself rotated to hash-2 (revoked-but-rotated => keep walking)
|
||||
.mockResolvedValueOnce({
|
||||
id: 2,
|
||||
userId: 5,
|
||||
revokedAt: new Date(Date.now() - 1_000),
|
||||
rotatedAt: new Date(Date.now() - 1_000),
|
||||
replacedByTokenHash: 'hash-2',
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
})
|
||||
// 3rd findFirst: final live row
|
||||
.mockResolvedValueOnce({
|
||||
id: 3,
|
||||
userId: 5,
|
||||
revokedAt: null,
|
||||
rotatedAt: null,
|
||||
replacedByTokenHash: null,
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
});
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).users.findFirst.mockResolvedValue({
|
||||
id: 5,
|
||||
tokenVersion: 9,
|
||||
active: true,
|
||||
provisioningMethod: 'local',
|
||||
});
|
||||
|
||||
await expect(service.refresh(makeRequest({ refresh_token: 'oldest-token' }), reply)).resolves.toEqual({ accessToken: 'signed-jwt' });
|
||||
expect(jwtService.sign).toHaveBeenCalledWith({ sub: 5, ver: 9 });
|
||||
expect(db.transaction).not.toHaveBeenCalled();
|
||||
const setCookieCalls = (reply as unknown as { setCookie: vi.Mock }).setCookie.mock.calls;
|
||||
expect(setCookieCalls.map(([name]) => name)).toEqual(['access_token']);
|
||||
});
|
||||
|
||||
it('treats a chain that ends in a logout-revoked link (no rotatedAt) as theft', async () => {
|
||||
const { service, db } = makeService();
|
||||
const reply = makeReply();
|
||||
const rotatedAt = new Date(Date.now() - 5_000);
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).refreshTokens.findFirst
|
||||
.mockResolvedValueOnce({
|
||||
id: 1,
|
||||
userId: 5,
|
||||
revokedAt: rotatedAt,
|
||||
rotatedAt,
|
||||
replacedByTokenHash: 'hash-1',
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
id: 2,
|
||||
userId: 5,
|
||||
revokedAt: new Date(),
|
||||
rotatedAt: null, // <- explicitly logged out, not rotated
|
||||
replacedByTokenHash: null,
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
});
|
||||
|
||||
await expect(service.refresh(makeRequest({ refresh_token: 'oldest-token' }), reply)).rejects.toThrow(UnauthorizedException);
|
||||
expect(db.transaction).toHaveBeenCalled();
|
||||
expect(db.delete).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('falls back to access-only when the rotation UPDATE matches zero rows AND re-fetch confirms rotation', async () => {
|
||||
const { service, db, jwtService } = makeService();
|
||||
const reply = makeReply();
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).refreshTokens.findFirst
|
||||
// 1st: refresh() initial lookup - presented token still looks live in our snapshot.
|
||||
.mockResolvedValueOnce({
|
||||
id: 1,
|
||||
userId: 5,
|
||||
revokedAt: null,
|
||||
rotatedAt: null,
|
||||
replacedByTokenHash: null,
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
})
|
||||
// 2nd: re-fetch after rowCount=0 — concurrent rotation winner already flipped the row.
|
||||
.mockResolvedValueOnce({
|
||||
id: 1,
|
||||
userId: 5,
|
||||
revokedAt: new Date(),
|
||||
rotatedAt: new Date(),
|
||||
replacedByTokenHash: 'replacement-hash',
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
})
|
||||
// 3rd: chain walk inside isRecentRefreshRotationReuse — replacement is live.
|
||||
.mockResolvedValueOnce({
|
||||
id: 2,
|
||||
userId: 5,
|
||||
revokedAt: null,
|
||||
rotatedAt: null,
|
||||
replacedByTokenHash: null,
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
});
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).users.findFirst.mockResolvedValue({
|
||||
id: 5,
|
||||
tokenVersion: 3,
|
||||
active: true,
|
||||
provisioningMethod: 'local',
|
||||
});
|
||||
// Simulate the racing competitor: our UPDATE matches 0 rows.
|
||||
(db.where as vi.Mock).mockResolvedValueOnce({ rowCount: 0 });
|
||||
|
||||
await expect(service.refresh(makeRequest({ refresh_token: 'racing-token' }), reply)).resolves.toEqual({ accessToken: 'signed-jwt' });
|
||||
expect(jwtService.sign).toHaveBeenCalledWith({ sub: 5, ver: 3 });
|
||||
// No refresh cookie was set on the race-loser path
|
||||
const setCookieCalls = (reply as unknown as { setCookie: vi.Mock }).setCookie.mock.calls;
|
||||
expect(setCookieCalls.map(([name]) => name)).toEqual(['access_token']);
|
||||
});
|
||||
|
||||
it('rejects (and does NOT issue an access token) when the concurrent revoke was a logout, not a rotation', async () => {
|
||||
const { service, db, jwtService } = makeService();
|
||||
const reply = makeReply();
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).refreshTokens.findFirst
|
||||
// 1st: initial lookup - token still appears live to us.
|
||||
.mockResolvedValueOnce({
|
||||
id: 1,
|
||||
userId: 5,
|
||||
revokedAt: null,
|
||||
rotatedAt: null,
|
||||
replacedByTokenHash: null,
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
})
|
||||
// 2nd: re-fetch after rowCount=0 — concurrent LOGOUT revoked the row (rotatedAt is null).
|
||||
.mockResolvedValueOnce({
|
||||
id: 1,
|
||||
userId: 5,
|
||||
revokedAt: new Date(),
|
||||
rotatedAt: null,
|
||||
replacedByTokenHash: null,
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
});
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).users.findFirst.mockResolvedValue({
|
||||
id: 5,
|
||||
tokenVersion: 3,
|
||||
active: true,
|
||||
provisioningMethod: 'local',
|
||||
});
|
||||
(db.where as vi.Mock).mockResolvedValueOnce({ rowCount: 0 });
|
||||
|
||||
await expect(service.refresh(makeRequest({ refresh_token: 'racing-token' }), reply)).rejects.toThrow(UnauthorizedException);
|
||||
// CRITICAL: no access token cookie set, both cookies cleared (maxAge: 0).
|
||||
// (jwtService.sign may have been called speculatively before the transaction lost the race,
|
||||
// but the resulting token must NEVER be sent to the client.)
|
||||
const setCookieCalls = (reply as unknown as { setCookie: vi.Mock }).setCookie.mock.calls;
|
||||
expect(setCookieCalls.map(([name]) => name)).toEqual(['refresh_token', 'access_token']);
|
||||
expect(setCookieCalls[0]).toEqual(['refresh_token', '', expect.objectContaining({ path: '/api/v1/auth', maxAge: 0 })]);
|
||||
expect(setCookieCalls[1]).toEqual(['access_token', '', expect.objectContaining({ path: '/api', maxAge: 0 })]);
|
||||
// No setCookie call ever emitted a non-empty access token value.
|
||||
const accessTokenSetCalls = setCookieCalls.filter(([name, value]) => name === 'access_token' && value !== '');
|
||||
expect(accessTokenSetCalls).toHaveLength(0);
|
||||
void jwtService;
|
||||
});
|
||||
|
||||
it('honours refreshRotationGraceMs from config when widened', async () => {
|
||||
const { service, db, config, jwtService } = makeService();
|
||||
(config.get as vi.Mock).mockImplementation((key: string) => {
|
||||
if (key === 'auth.jwtRefreshExpiresIn') return '7d';
|
||||
if (key === 'auth.jwtExpiresIn') return '15m';
|
||||
if (key === 'auth.refreshRotationGraceMs') return 120_000; // widen to 2m
|
||||
if (key === 'app.nodeEnv') return 'test';
|
||||
return undefined;
|
||||
});
|
||||
const reply = makeReply();
|
||||
const rotatedAt = new Date(Date.now() - 90_000); // outside default 30s window, inside widened 120s
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).refreshTokens.findFirst
|
||||
.mockResolvedValueOnce({
|
||||
id: 1,
|
||||
userId: 5,
|
||||
revokedAt: rotatedAt,
|
||||
rotatedAt,
|
||||
replacedByTokenHash: 'replacement-hash',
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
id: 2,
|
||||
userId: 5,
|
||||
revokedAt: null,
|
||||
rotatedAt: null,
|
||||
replacedByTokenHash: null,
|
||||
expiresAt: new Date(Date.now() + 100_000),
|
||||
});
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).users.findFirst.mockResolvedValue({
|
||||
id: 5,
|
||||
tokenVersion: 4,
|
||||
active: true,
|
||||
provisioningMethod: 'local',
|
||||
});
|
||||
|
||||
await expect(service.refresh(makeRequest({ refresh_token: 'widened-grace-token' }), reply)).resolves.toEqual({ accessToken: 'signed-jwt' });
|
||||
expect(jwtService.sign).toHaveBeenCalledWith({ sub: 5, ver: 4 });
|
||||
});
|
||||
|
||||
it('throws UnauthorizedException when token is expired', async () => {
|
||||
const { service, db } = makeService();
|
||||
(db.query as never as Record<string, Record<string, vi.Mock>>).refreshTokens.findFirst.mockResolvedValue({
|
||||
@@ -506,7 +891,11 @@ describe('AuthService', () => {
|
||||
|
||||
const result = await service.refresh(makeRequest({ refresh_token: 'ok-token' }), reply);
|
||||
expect(result).toEqual({ accessToken: 'signed-jwt' });
|
||||
expect(db.update).toHaveBeenCalled();
|
||||
expect(db.transaction).toHaveBeenCalled();
|
||||
const rotationSetArg = (db as unknown as Record<string, vi.Mock>).set.mock.calls[0]?.[0] as { revokedAt?: Date; rotatedAt?: Date };
|
||||
expect(rotationSetArg.revokedAt).toBeInstanceOf(Date);
|
||||
expect(rotationSetArg.rotatedAt).toBe(rotationSetArg.revokedAt);
|
||||
expect(rotationSetArg).toEqual(expect.objectContaining({ replacedByTokenHash: expect.any(String) }));
|
||||
expect(oidcSessionRepo.touchActiveByUserId).toHaveBeenCalledWith(5, expect.any(Date));
|
||||
expect((reply as unknown as { setCookie: vi.Mock }).setCookie).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -56,8 +56,11 @@ function sha256(value: string): string {
|
||||
|
||||
const LOGIN_LOCKOUT_THRESHOLD = 5;
|
||||
const LOGIN_LOCKOUT_DURATION_MS = 15 * 60_000;
|
||||
const ROTATION_CHAIN_MAX_HOPS = 5;
|
||||
const DUMMY_HASH = '$2a$12$LJ3m4ys3Lk0TSwHBbqP8b.3bFfR1oVDMhPzX8KPrPeuMEJBJJPa.G';
|
||||
|
||||
class ConcurrentRotationError extends Error {}
|
||||
|
||||
function maskEmail(email: string): string {
|
||||
const at = email.indexOf('@');
|
||||
if (at <= 0) return '***';
|
||||
@@ -289,9 +292,13 @@ export class AuthService {
|
||||
throw new UnauthorizedException();
|
||||
}
|
||||
|
||||
// Reuse of a revoked token indicates possible theft.
|
||||
// Revoke refresh sessions and bump tokenVersion to invalidate active access tokens.
|
||||
if (row.revokedAt) {
|
||||
if (await this.isRecentRefreshRotationReuse(row)) {
|
||||
return this.issueAccessOnlyForRefreshReuse(row.userId, reply);
|
||||
}
|
||||
|
||||
// Reuse of a revoked token outside a fresh rotation indicates possible theft.
|
||||
// Revoke refresh sessions and bump tokenVersion to invalidate active access tokens.
|
||||
this.logger.warn(
|
||||
`[auth.refresh] [fail] userId=${row.userId} errorClass=UnauthorizedException error="token revoked - reuse attempt" - refresh failed`,
|
||||
);
|
||||
@@ -313,33 +320,52 @@ export class AuthService {
|
||||
throw new UnauthorizedException();
|
||||
}
|
||||
|
||||
const userForToken = await this.db.query.users.findFirst({ where: eq(schema.users.id, row.userId) });
|
||||
if (!userForToken) throw new UnauthorizedException();
|
||||
if (!userForToken.active) {
|
||||
this.logger.warn(`[auth.refresh] [fail] userId=${row.userId} errorClass=UnauthorizedException error="account disabled" - refresh failed`);
|
||||
this.clearRefreshCookie(reply);
|
||||
this.clearAccessCookie(reply);
|
||||
throw new UnauthorizedException('Account disabled');
|
||||
}
|
||||
const userForToken = await this.assertUserCanRefresh(row.userId, reply);
|
||||
|
||||
if (userForToken.provisioningMethod === 'shared') {
|
||||
const hasActive = await this.magicLinkRepo.hasActiveByUserId(row.userId);
|
||||
// Rotate: revoke old, issue new. Guard against the SPA race where two requests
|
||||
// present the same (still-valid) refresh: the loser's UPDATE matches zero rows
|
||||
// and we treat it as a benign reuse of an already-rotated token.
|
||||
const { accessToken, rawRefreshToken, refreshExpiresAt, refreshTokenHash } = this.createTokenPair(row.userId, userForToken.tokenVersion);
|
||||
|
||||
if (!hasActive) {
|
||||
const rotatedAt = new Date();
|
||||
try {
|
||||
await this.db.transaction(async (tx) => {
|
||||
const updateResult = await tx
|
||||
.update(schema.refreshTokens)
|
||||
.set({ revokedAt: rotatedAt, rotatedAt, replacedByTokenHash: refreshTokenHash })
|
||||
.where(and(eq(schema.refreshTokens.id, row.id), isNull(schema.refreshTokens.revokedAt)));
|
||||
if ((updateResult.rowCount ?? 0) === 0) {
|
||||
throw new ConcurrentRotationError();
|
||||
}
|
||||
await tx.insert(schema.refreshTokens).values({ userId: row.userId, tokenHash: refreshTokenHash, expiresAt: refreshExpiresAt });
|
||||
});
|
||||
} catch (err) {
|
||||
if (err instanceof ConcurrentRotationError) {
|
||||
// rowCount=0 only proves "someone else changed this row first" — it could be a
|
||||
// concurrent rotation OR a concurrent logout/security revoke. Re-fetch and only
|
||||
// accept the race as benign if the revoke came from rotation. Otherwise the user
|
||||
// (or admin) explicitly killed this session and we must NOT resurrect it.
|
||||
const refreshedRow = await this.db.query.refreshTokens.findFirst({
|
||||
where: eq(schema.refreshTokens.id, row.id),
|
||||
});
|
||||
if (refreshedRow && (await this.isRecentRefreshRotationReuse(refreshedRow))) {
|
||||
this.logger.log(`[auth.refresh] [end] userId=${row.userId} reason="rotation-race-lost" - access-only refresh issued`);
|
||||
return this.issueAccessOnlyForRefreshReuse(row.userId, reply);
|
||||
}
|
||||
// Concurrent non-rotation revoke = the user (logout) or an admin (disable/security)
|
||||
// just killed this specific session. Honor that intent: fail this refresh and clear
|
||||
// cookies, but do NOT touch other devices' sessions (logout deliberately only revoked
|
||||
// this token; admin actions have their own scope).
|
||||
this.logger.warn(
|
||||
`[auth.refresh] [fail] userId=${row.userId} errorClass=UnauthorizedException error="all magic links revoked" - refresh failed`,
|
||||
`[auth.refresh] [fail] userId=${row.userId} errorClass=UnauthorizedException error="concurrent non-rotation revoke" - refresh failed`,
|
||||
);
|
||||
await this.revokeAllUserSessions(row.userId);
|
||||
this.clearRefreshCookie(reply);
|
||||
this.clearAccessCookie(reply);
|
||||
throw new UnauthorizedException();
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
// Rotate: revoke old, issue new
|
||||
await this.db.update(schema.refreshTokens).set({ revokedAt: new Date() }).where(eq(schema.refreshTokens.id, row.id));
|
||||
|
||||
const { accessToken, rawRefreshToken, refreshExpiresAt } = await this.issueTokenPair(row.userId, userForToken.tokenVersion);
|
||||
// touchActive must run outside the transaction (oidc sessions are best-effort sliding TTL).
|
||||
await this.oidcSessionRepo.touchActiveByUserId(row.userId, refreshExpiresAt);
|
||||
this.setRefreshCookie(reply, rawRefreshToken);
|
||||
this.setAccessCookie(reply, accessToken);
|
||||
@@ -594,15 +620,85 @@ export class AuthService {
|
||||
}
|
||||
|
||||
private async issueTokenPair(userId: number, tokenVersion: number) {
|
||||
const pair = this.createTokenPair(userId, tokenVersion);
|
||||
|
||||
await this.db.insert(schema.refreshTokens).values({ userId, tokenHash: pair.refreshTokenHash, expiresAt: pair.refreshExpiresAt });
|
||||
|
||||
return pair;
|
||||
}
|
||||
|
||||
private createTokenPair(userId: number, tokenVersion: number) {
|
||||
const accessToken = this.jwtService.sign({ sub: userId, ver: tokenVersion });
|
||||
|
||||
const rawRefreshToken = randomBytes(32).toString('hex');
|
||||
const tokenHash = sha256(rawRefreshToken);
|
||||
const expiresAt = this.getRefreshTokenExpiryDate();
|
||||
|
||||
await this.db.insert(schema.refreshTokens).values({ userId, tokenHash, expiresAt });
|
||||
return { accessToken, rawRefreshToken, refreshExpiresAt: expiresAt, refreshTokenHash: tokenHash };
|
||||
}
|
||||
|
||||
return { accessToken, rawRefreshToken, refreshExpiresAt: expiresAt };
|
||||
private getRefreshRotationGraceMs(): number {
|
||||
return this.config.get<number>('auth.refreshRotationGraceMs') ?? 30_000;
|
||||
}
|
||||
|
||||
/**
|
||||
* Distinguishes a benign rotation race (or a chained rotation) from refresh-token theft.
|
||||
*
|
||||
* Accepts the presented (revoked) token only when:
|
||||
* - it was revoked by rotation (rotatedAt + replacedByTokenHash set)
|
||||
* - the rotation happened within the configured grace window
|
||||
* - walking the rotation chain (up to ROTATION_CHAIN_MAX_HOPS) reaches a live row
|
||||
* for the same user. A revoked link in the chain is OK only if it was itself rotated
|
||||
* (so we don't resurrect a session that was explicitly logged out).
|
||||
*/
|
||||
private async isRecentRefreshRotationReuse(row: typeof schema.refreshTokens.$inferSelect) {
|
||||
if (!row.rotatedAt || !row.replacedByTokenHash) return false;
|
||||
if (row.expiresAt < new Date()) return false;
|
||||
const elapsedMs = Date.now() - row.rotatedAt.getTime();
|
||||
if (elapsedMs < 0 || elapsedMs > this.getRefreshRotationGraceMs()) return false;
|
||||
|
||||
let nextHash: string | null = row.replacedByTokenHash;
|
||||
for (let hop = 0; hop < ROTATION_CHAIN_MAX_HOPS && nextHash; hop++) {
|
||||
const next: typeof schema.refreshTokens.$inferSelect | undefined = await this.db.query.refreshTokens.findFirst({
|
||||
where: eq(schema.refreshTokens.tokenHash, nextHash),
|
||||
});
|
||||
if (!next || next.userId !== row.userId || next.expiresAt < new Date()) return false;
|
||||
if (!next.revokedAt) return true;
|
||||
// Revoked-but-rotated link: keep walking. Revoked-without-rotation means logout/security revoke.
|
||||
if (!next.rotatedAt || !next.replacedByTokenHash) return false;
|
||||
nextHash = next.replacedByTokenHash;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private async assertUserCanRefresh(userId: number, reply: FastifyReply) {
|
||||
const userForToken = await this.db.query.users.findFirst({ where: eq(schema.users.id, userId) });
|
||||
if (!userForToken) throw new UnauthorizedException();
|
||||
if (!userForToken.active) {
|
||||
this.logger.warn(`[auth.refresh] [fail] userId=${userId} errorClass=UnauthorizedException error="account disabled" - refresh failed`);
|
||||
this.clearRefreshCookie(reply);
|
||||
this.clearAccessCookie(reply);
|
||||
throw new UnauthorizedException('Account disabled');
|
||||
}
|
||||
if (userForToken.provisioningMethod === 'shared') {
|
||||
const hasActive = await this.magicLinkRepo.hasActiveByUserId(userId);
|
||||
if (!hasActive) {
|
||||
this.logger.warn(`[auth.refresh] [fail] userId=${userId} errorClass=UnauthorizedException error="all magic links revoked" - refresh failed`);
|
||||
await this.revokeAllUserSessions(userId);
|
||||
this.clearRefreshCookie(reply);
|
||||
this.clearAccessCookie(reply);
|
||||
throw new UnauthorizedException();
|
||||
}
|
||||
}
|
||||
return userForToken;
|
||||
}
|
||||
|
||||
private async issueAccessOnlyForRefreshReuse(userId: number, reply: FastifyReply) {
|
||||
const userForToken = await this.assertUserCanRefresh(userId, reply);
|
||||
const accessToken = this.jwtService.sign({ sub: userId, ver: userForToken.tokenVersion });
|
||||
this.setAccessCookie(reply, accessToken);
|
||||
this.logger.log(`[auth.refresh] [end] userId=${userId} reason="rotation-race" - access-only refresh issued`);
|
||||
return { accessToken };
|
||||
}
|
||||
|
||||
private assertSetupToken(setupToken: string | undefined) {
|
||||
|
||||
@@ -412,7 +412,7 @@ describe('Auth session security (e2e)', () => {
|
||||
});
|
||||
|
||||
describe('revoked refresh token reuse', () => {
|
||||
it('revokes all sessions and invalidates access tokens when a revoked token is reused', async () => {
|
||||
it('treats immediate rotated token reuse as benign and keeps the replacement session active', async () => {
|
||||
const credentials = await createLocalUser(context.db);
|
||||
const initialLogin = await login(context.app, credentials.username, credentials.password);
|
||||
const revokedRefreshToken = initialLogin.refreshToken;
|
||||
@@ -426,7 +426,7 @@ describe('Auth session security (e2e)', () => {
|
||||
});
|
||||
|
||||
expect(rotateResponse.statusCode).toBe(200);
|
||||
const rotatedAccessToken = (rotateResponse.json() as { accessToken: string }).accessToken;
|
||||
mergeCookieJar(initialLogin.jar, getSetCookieLines(rotateResponse.headers));
|
||||
|
||||
const reuseResponse = await context.app.inject({
|
||||
method: 'POST',
|
||||
@@ -436,33 +436,41 @@ describe('Auth session security (e2e)', () => {
|
||||
},
|
||||
});
|
||||
|
||||
expect(reuseResponse.statusCode).toBe(401);
|
||||
const reuseCookies = getSetCookieLines(reuseResponse.headers);
|
||||
expect(cookieValue(reuseCookies, 'refresh_token')).toBe('');
|
||||
expect(cookieValue(reuseCookies, 'access_token')).toBe('');
|
||||
expect(reuseResponse.statusCode).toBe(200);
|
||||
const replayAccessToken = (reuseResponse.json() as { accessToken: string }).accessToken;
|
||||
expect(replayAccessToken).toEqual(expect.any(String));
|
||||
|
||||
const remainingSessions = await context.db
|
||||
.select({ id: schema.refreshTokens.id })
|
||||
.from(schema.refreshTokens)
|
||||
.where(eq(schema.refreshTokens.userId, credentials.userId));
|
||||
expect(remainingSessions).toHaveLength(0);
|
||||
const reuseCookies = getSetCookieLines(reuseResponse.headers);
|
||||
expect(cookieValue(reuseCookies, 'refresh_token')).toBeNull();
|
||||
expect(cookieValue(reuseCookies, 'access_token')).toEqual(expect.any(String));
|
||||
|
||||
expect(await activeSessionCount(context.db, credentials.userId)).toBe(1);
|
||||
|
||||
const [userAfterReuse] = await context.db
|
||||
.select({ tokenVersion: schema.users.tokenVersion })
|
||||
.from(schema.users)
|
||||
.where(eq(schema.users.id, credentials.userId))
|
||||
.limit(1);
|
||||
expect(userAfterReuse?.tokenVersion).toBeGreaterThan(1);
|
||||
expect(userAfterReuse?.tokenVersion).toBe(1);
|
||||
|
||||
const meResponse = await context.app.inject({
|
||||
method: 'GET',
|
||||
url: '/api/v1/auth/me',
|
||||
headers: {
|
||||
authorization: `Bearer ${rotatedAccessToken}`,
|
||||
authorization: `Bearer ${replayAccessToken}`,
|
||||
},
|
||||
});
|
||||
expect(meResponse.statusCode).toBe(200);
|
||||
|
||||
const nextRefreshResponse = await context.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/auth/refresh',
|
||||
headers: {
|
||||
cookie: cookieHeader(initialLogin.jar),
|
||||
},
|
||||
});
|
||||
|
||||
expect(meResponse.statusCode).toBe(401);
|
||||
expect(nextRefreshResponse.statusCode).toBe(200);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user