From 52bfd7a399b681ab6b82c66cf5503cc6be7c33ce Mon Sep 17 00:00:00 2001 From: Jakob Wennberg Date: Wed, 26 Aug 2026 14:15:22 +0200 Subject: [PATCH] perf(auth): skip the MFA factor lookup once the session is at AAL2 (#1933) The enforced-MFA branch of the proxy called supabase.auth.mfa.listFactors() on every page, RSC and prefetch request for every MFA-verified user with a company. auth-js implements listFactors() as a getUser() network round trip, so hosted page requests paid two Supabase Auth calls in sequence. At AAL2 a verified factor exists by construction (the session got there by verifying a challenge on one), so the lookup only runs on the aal1/aal1 path (users mid-enrolment), where it still gates exactly as before. The one case deferred is a user who unenrols their last factor mid-session: the JWT keeps aal2 until the next refresh, so the enrolment bounce lands on the refresh instead of the next click. Tests: aal1/aal1 still asks for the factor list and bounces to /mfa/enroll; at aal2 listFactors is never called (even with a factor list that would read as empty) on page, RSC and prefetch requests; the step-up bounce and the no-company skip are unchanged. Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 --- lib/supabase/__tests__/middleware.test.ts | 32 ++++++++++++++++++++++- lib/supabase/middleware.ts | 30 ++++++++++++++------- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/lib/supabase/__tests__/middleware.test.ts b/lib/supabase/__tests__/middleware.test.ts index aac4a9f8..c9418388 100644 --- a/lib/supabase/__tests__/middleware.test.ts +++ b/lib/supabase/__tests__/middleware.test.ts @@ -15,6 +15,7 @@ const state = vi.hoisted(() => ({ authError: null as unknown, aal: null as null | { currentLevel: string; nextLevel: string }, factors: null as null | { totp: Array<{ id: string; status: string }> }, + listFactors: vi.fn(async () => ({ data: state.factors })), company: { data: [{ company_id: 'company-1', locale: 'sv', used_fallback: false }], error: null as unknown, @@ -45,7 +46,7 @@ vi.mock('@supabase/ssr', () => ({ signOut: state.signOut, mfa: { getAuthenticatorAssuranceLevel: vi.fn(async () => ({ data: state.aal })), - listFactors: vi.fn(async () => ({ data: state.factors })), + listFactors: (...args: unknown[]) => state.listFactors(...args), }, }, rpc: vi.fn(async () => state.company), @@ -117,6 +118,7 @@ describe('updateSession redirect destinations', () => { beforeEach(() => { vi.clearAllMocks() logState.info.mockClear() + state.listFactors.mockClear() state.user = null state.sessionId = 'session-1' state.authError = null @@ -545,6 +547,34 @@ describe('updateSession redirect destinations', () => { expect(response.status).toBe(200) }) + + it('still asks for the factor list at aal1/aal1 before bouncing', async () => { + const response = await run('/invoices/new') + + expect(new URL(locationOf(response)!).pathname).toBe('/mfa/enroll') + expect(state.listFactors).toHaveBeenCalledTimes(1) + }) + + it('never calls listFactors once the session is at aal2 (a verified factor is implied)', async () => { + state.aal = { currentLevel: 'aal2', nextLevel: 'aal2' } + // Even a factor list that would read as "none" must not matter here: + // the call is skipped, not just its result ignored. + state.factors = { totp: [] } + + const response = await run('/invoices/new') + + expect(response.status).toBe(200) + expect(state.listFactors).not.toHaveBeenCalled() + }) + + it('does not spend an MFA lookup on RSC and prefetch requests at aal2 either', async () => { + state.aal = { currentLevel: 'aal2', nextLevel: 'aal2' } + + await run('/invoices', { headers: { rsc: '1' } }) + await run('/invoices', { headers: { 'next-router-prefetch': '1', rsc: '1' } }) + + expect(state.listFactors).not.toHaveBeenCalled() + }) }) // ── MFA semantics that must not change ──────────────────────────────── diff --git a/lib/supabase/middleware.ts b/lib/supabase/middleware.ts index 146912dd..88ff9943 100644 --- a/lib/supabase/middleware.ts +++ b/lib/supabase/middleware.ts @@ -379,16 +379,28 @@ async function updateSessionInner( } // MFA required but user has no factor enrolled yet → force enrollment - // Skip for users with no companies (still setting up) - const { companyId: companyIdForMfa } = await resolveCompanyOnce() - if (companyIdForMfa) { - const { data: factors } = await timed(timing, 'mfaMs', () => - supabase.auth.mfa.listFactors(), - ) - const hasVerifiedFactor = factors?.totp?.some(f => f.status === 'verified') + // Skip for users with no companies (still setting up). + // + // Only worth asking when the session is NOT at AAL2: reaching AAL2 + // requires having verified a challenge on a verified factor, so the + // factor list cannot be empty there. auth-js implements listFactors() + // as a getUser() network round trip, and running it here on every + // page, RSC and prefetch request for every MFA-verified user was the + // second Supabase Auth call per request (measured via mw-mfa, PR #1922). + // The narrow case this defers is a user who unenrols their last factor + // mid-session: the JWT keeps aal2 until the next token refresh, so the + // enrolment bounce lands on the refresh instead of the next click. + if (aal?.currentLevel !== 'aal2') { + const { companyId: companyIdForMfa } = await resolveCompanyOnce() + if (companyIdForMfa) { + const { data: factors } = await timed(timing, 'mfaMs', () => + supabase.auth.mfa.listFactors(), + ) + const hasVerifiedFactor = factors?.totp?.some(f => f.status === 'verified') - if (!hasVerifiedFactor) { - return bounceToAuth(request, '/mfa/enroll') + if (!hasVerifiedFactor) { + return bounceToAuth(request, '/mfa/enroll') + } } } }