diff --git a/apps/web/src/lib/server/auth/__tests__/hooks-callback-cleanup.test.ts b/apps/web/src/lib/server/auth/__tests__/hooks-callback-cleanup.test.ts index 810aa2830..7094b681e 100644 --- a/apps/web/src/lib/server/auth/__tests__/hooks-callback-cleanup.test.ts +++ b/apps/web/src/lib/server/auth/__tests__/hooks-callback-cleanup.test.ts @@ -31,6 +31,9 @@ const mockPrincipalDeleteWhere = vi.fn(async () => undefined) const mockDeleteSessionCookie = vi.fn() const mockGetPublicPortalConfig = vi.fn() const mockHasPlatformCredentials = vi.fn() +const mockGetSignedCookie = vi.fn() +const mockSetSignedCookie = vi.fn() +const mockSetCookie = vi.fn() vi.mock('@/lib/server/db', () => ({ db: { @@ -103,6 +106,7 @@ function ctxFor(opts: { userId?: string email?: string token?: string + secret?: string }) { return { path: opts.path, @@ -113,7 +117,11 @@ function ctxFor(opts: { user: opts.userId ? { id: opts.userId, email: opts.email } : undefined, session: opts.token ? { token: opts.token } : undefined, }, + secret: opts.secret, }, + getSignedCookie: mockGetSignedCookie, + setSignedCookie: mockSetSignedCookie, + setCookie: mockSetCookie, redirect: vi.fn((url: string) => new Error(`REDIRECT:${url}`)), } } @@ -127,6 +135,8 @@ beforeEach(() => { }) mockHasPlatformCredentials.mockResolvedValue(true) mockIsSsoActuallyRegistered.mockImplementation(async (sso) => sso?.enabled === true) + mockGetSignedCookie.mockResolvedValue(undefined) + mockSetSignedCookie.mockResolvedValue(undefined) }) // ============================================================ @@ -189,6 +199,67 @@ describe('handleCallbackPolicyCleanup — guards', () => { }) }) +describe('handleCallbackPolicyCleanup — two-factor deferred OAuth policy', () => { + it('remembers the first-factor provider when an OAuth callback defers final session issuance to 2FA', async () => { + mockPrincipalFindFirst.mockResolvedValue({ role: 'admin' }) + const ctx = ctxFor({ + path: '/oauth2/callback/:providerId', + providerParam: 'google', + userId: 'user_1', + email: 'a@external.com', + secret: 'test-secret', + }) + + await handleCallbackPolicyCleanup(ctx, tenantSettings({ googleEnabled: true })) + + expect(mockSetSignedCookie).toHaveBeenCalledWith( + 'quackback.2fa_policy_provider', + 'user_1:google', + 'test-secret', + expect.objectContaining({ httpOnly: true, maxAge: 600 }) + ) + }) + + it('applies the remembered OAuth provider policy on /two-factor/verify-totp session creation', async () => { + mockPrincipalFindFirst.mockResolvedValue({ role: 'admin' }) + mockUserFindFirst.mockResolvedValue({ createdAt: new Date(Date.now() - 60 * 60_000) }) + mockGetSignedCookie.mockResolvedValue('user_1:google') + const ctx = ctxFor({ + path: '/two-factor/verify-totp', + userId: 'user_1', + email: 'a@external.com', + token: 'tok', + secret: 'test-secret', + }) + + await expect( + handleCallbackPolicyCleanup(ctx, tenantSettings({ googleEnabled: false })) + ).rejects.toThrow(/\/admin\/login\?error=oauth_method_not_allowed/) + + expect(mockSetCookie).toHaveBeenCalledWith('quackback.2fa_policy_provider', '', { + path: '/', + maxAge: 0, + }) + expect(mockSessionDeleteWhere).toHaveBeenCalled() + expect(mockDeleteSessionCookie).toHaveBeenCalled() + }) + + it('skips two-factor verification without a remembered first-factor provider', async () => { + const ctx = ctxFor({ + path: '/two-factor/verify-totp', + userId: 'user_1', + email: 'a@external.com', + token: 'tok', + secret: 'test-secret', + }) + + await handleCallbackPolicyCleanup(ctx, tenantSettings({ googleEnabled: false })) + + expect(mockSessionDeleteWhere).not.toHaveBeenCalled() + expect(ctx.redirect).not.toHaveBeenCalled() + }) +}) + // ============================================================ // SSO callback — always allowed for team // ============================================================ diff --git a/apps/web/src/lib/server/auth/hooks.ts b/apps/web/src/lib/server/auth/hooks.ts index 0bc908be2..44ce5fe09 100644 --- a/apps/web/src/lib/server/auth/hooks.ts +++ b/apps/web/src/lib/server/auth/hooks.ts @@ -47,6 +47,13 @@ export type AuthProviderId = | 'sso' // genericOAuth provider id 'sso' | string // social ('google'|'github'|...) or other generic OAuth +const AUTH_PATH_PREFIX = '@opencoven-feedback' + +function normalizeAuthPath(path: string | undefined): string | undefined { + if (!path) return undefined + return path.startsWith(`${AUTH_PATH_PREFIX}/`) ? path.slice(AUTH_PATH_PREFIX.length) : path +} + /** * Map a Better-Auth `ctx.path` template to the conceptual provider id * the policy table operates on. Returns `null` for paths that aren't @@ -68,7 +75,7 @@ export function inferProvider(ctx: { params?: Record body?: Record }): AuthProviderId | null { - const p = ctx.path + const p = normalizeAuthPath(ctx.path) if (!p) return null switch (p) { case '/sign-in/email': @@ -115,12 +122,79 @@ export function inferProvider(ctx: { * session synchronously without going through `/callback/:id` — Layer * B can't see the email pre-session, so Layer C is the only gate. */ -export const SESSION_CREATING_CALLBACK_PATHS = new Set([ +const OAUTH_CALLBACK_POLICY_PATHS = new Set([ '/callback/:id', '/oauth2/callback/:providerId', '/sign-in/social', ]) +const TWO_FACTOR_SESSION_COMPLETION_PATHS = new Set([ + '/two-factor/verify', + '/two-factor/verify-totp', + '/two-factor/verify-otp', + '/two-factor/verify-backup-code', +]) + +export const SESSION_CREATING_CALLBACK_PATHS = new Set([ + ...OAUTH_CALLBACK_POLICY_PATHS, + ...TWO_FACTOR_SESSION_COMPLETION_PATHS, +]) + +const TWO_FACTOR_POLICY_PROVIDER_COOKIE = 'quackback.2fa_policy_provider' +const TWO_FACTOR_POLICY_COOKIE_MAX_AGE = 10 * 60 + +type TwoFactorPolicyCookieCtx = { + context?: { secret?: string } + getSignedCookie?: ( + name: string, + secret: string + ) => string | undefined | Promise + setSignedCookie?: ( + name: string, + value: string, + secret: string, + attrs?: Record + ) => unknown | Promise + setCookie?: (name: string, value: string, opts?: Record) => unknown +} + +async function rememberTwoFactorPolicyProvider( + ctx: TwoFactorPolicyCookieCtx, + userId: string, + provider: AuthProviderId +): Promise { + if (!ctx.setSignedCookie || typeof ctx.context?.secret !== 'string') return + await ctx.setSignedCookie( + TWO_FACTOR_POLICY_PROVIDER_COOKIE, + `${userId}:${provider}`, + ctx.context.secret, + { + httpOnly: true, + sameSite: 'lax', + secure: process.env.NODE_ENV === 'production', + path: '/', + maxAge: TWO_FACTOR_POLICY_COOKIE_MAX_AGE, + } + ) +} + +async function consumeTwoFactorPolicyProvider( + ctx: TwoFactorPolicyCookieCtx, + userId: string +): Promise { + if (!ctx.getSignedCookie || typeof ctx.context?.secret !== 'string') return null + const value = await ctx.getSignedCookie(TWO_FACTOR_POLICY_PROVIDER_COOKIE, ctx.context.secret) + if (!value) return null + ctx.setCookie?.(TWO_FACTOR_POLICY_PROVIDER_COOKIE, '', { path: '/', maxAge: 0 }) + + const separator = value.indexOf(':') + if (separator <= 0) return null + const cookieUserId = value.slice(0, separator) + const provider = value.slice(separator + 1) + if (cookieUserId !== userId || !provider) return null + return provider +} + /** * Paths where the email isn't in `ctx.body` — Layer B can't gate them * because there's no caller identity yet. Layer A (registration filter) @@ -522,13 +596,26 @@ export async function handleCallbackPolicyCleanup( ReturnType > ): Promise { - if (!SESSION_CREATING_CALLBACK_PATHS.has(ctx.path ?? '')) return + const path = normalizeAuthPath(ctx.path) + if (!SESSION_CREATING_CALLBACK_PATHS.has(path ?? '')) return const userId = ctx.context?.newSession?.user?.id const userEmail = ctx.context?.newSession?.user?.email const token = ctx.context?.newSession?.session?.token - if (typeof userId !== 'string' || typeof token !== 'string') return + if (typeof userId !== 'string') return - const provider = inferProvider(ctx as Parameters[0]) + let provider = inferProvider(ctx as Parameters[0]) + if (provider && path && OAUTH_CALLBACK_POLICY_PATHS.has(path) && typeof token !== 'string') { + // Better-Auth's 2FA plugin can turn a successful first-factor + // callback into a pending challenge by deleting the provisional + // session and issuing the final session later from `/two-factor/*`. + // Keep the first-factor provider in a signed, short-lived cookie + // so the completion endpoint is evaluated against the same policy. + await rememberTwoFactorPolicyProvider(ctx as TwoFactorPolicyCookieCtx, userId, provider) + } + + if (!provider && path && TWO_FACTOR_SESSION_COMPLETION_PATHS.has(path)) { + provider = await consumeTwoFactorPolicyProvider(ctx as TwoFactorPolicyCookieCtx, userId) + } if (!provider) return const { @@ -592,7 +679,7 @@ export async function handleCallbackPolicyCleanup( typeof userEmail === 'string' && isHardBound(provider, userEmail, role, tenant?.authConfig, verifiedDomains, ssoRegistered) ) { - await revokeSession(ctx as SessionCtx, token) + if (typeof token === 'string') await revokeSession(ctx as SessionCtx, token) await wipeBrandNewShellsIfFresh() throw blockedRedirect('verified_domain_requires_sso') } @@ -602,7 +689,7 @@ export async function handleCallbackPolicyCleanup( const result = await isAuthMethodAllowed(provider, role, tenant) if (result.allowed) return - await revokeSession(ctx as SessionCtx, token) + if (typeof token === 'string') await revokeSession(ctx as SessionCtx, token) await wipeBrandNewShellsIfFresh() throw blockedRedirect(result.error ?? 'auth_method_blocked') }