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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Jakob Wennberg
Claude Fable 5
parent
47fe193c48
commit
52bfd7a399
@@ -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 ────────────────────────────────
|
||||
|
||||
@@ -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')
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user