fix(mail): request gmail.readonly alone, mailbox address via Gmail profile (Google verification) (#2301)
* fix(mail): request gmail.readonly alone and read the mailbox address from Gmail's profile Google's restricted-scope review (2026-08-31) bounced the Gmail connector on a "scope discrepancy": the authorization URL asked for `openid email` on top of gmail.readonly, while the Cloud Console declares gmail.readonly only, and the review string-matches the two. The extra scopes existed solely to learn the mailbox address from the id_token. Gmail's users.getProfile returns that address under gmail.readonly, so the consent request now carries exactly one scope and the callback reads the address from the profile. Also adds `app_metadata.mfa_exempt === true` to shouldEnforceMfa. Google's reviewers log in with credentials we hand them and treat a second factor as an "authentication blocker"; app_metadata is service-role only, so this is an operator switch for demo accounts, never a user-reachable setting. Tests: scope pinned in google-oauth.test.ts, profile read in gmail-client.test.ts, callback path in oauth-callback.test.ts, flag shape in mfa.test.ts. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UD3HsDX8hnJEqpt35azxBJ * fix(auth): time-box the reviewer MFA exemption instead of a boolean flag Superagent's P1 on the first shape was fair: a boolean app_metadata.mfa_exempt relied on someone remembering to clear it. The exemption is now app_metadata.mfa_exempt_until, an ISO timestamp honoured only while it lies in the future, so a forgotten flag dies on its own. Anything malformed or non-string enforces MFA. Still service-role only, still meant for the one demo account Google's OAuth reviewers log in with. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UD3HsDX8hnJEqpt35azxBJ --------- Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5.1
Jakob Wennberg
parent
0ad83b8d71
commit
971952fe19
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect, vi, afterEach } from 'vitest'
|
||||
import { isMfaRequired, shouldEnforceMfa } from '../mfa'
|
||||
import { isMfaExemptionActive, isMfaRequired, shouldEnforceMfa } from '../mfa'
|
||||
|
||||
describe('mfa helpers', () => {
|
||||
afterEach(() => {
|
||||
@@ -38,10 +38,41 @@ describe('mfa helpers', () => {
|
||||
expect(shouldEnforceMfa({ app_metadata: {} })).toBe(true)
|
||||
})
|
||||
|
||||
it('skips MFA while a service-role exemption is still in the future', () => {
|
||||
vi.stubEnv('NEXT_PUBLIC_SELF_HOSTED', 'false')
|
||||
vi.stubEnv('NEXT_PUBLIC_REQUIRE_MFA', 'true')
|
||||
const future = new Date(Date.now() + 60 * 60 * 1000).toISOString()
|
||||
expect(shouldEnforceMfa({ app_metadata: { mfa_exempt_until: future } })).toBe(false)
|
||||
})
|
||||
|
||||
it('enforces MFA again once the exemption has expired', () => {
|
||||
vi.stubEnv('NEXT_PUBLIC_SELF_HOSTED', 'false')
|
||||
vi.stubEnv('NEXT_PUBLIC_REQUIRE_MFA', 'true')
|
||||
const past = new Date(Date.now() - 60 * 1000).toISOString()
|
||||
expect(shouldEnforceMfa({ app_metadata: { mfa_exempt_until: past } })).toBe(true)
|
||||
})
|
||||
|
||||
it('treats a malformed or non-string exemption as no exemption', () => {
|
||||
vi.stubEnv('NEXT_PUBLIC_SELF_HOSTED', 'false')
|
||||
vi.stubEnv('NEXT_PUBLIC_REQUIRE_MFA', 'true')
|
||||
expect(shouldEnforceMfa({ app_metadata: { mfa_exempt_until: 'soon' } })).toBe(true)
|
||||
expect(shouldEnforceMfa({ app_metadata: { mfa_exempt_until: true } })).toBe(true)
|
||||
expect(shouldEnforceMfa({ app_metadata: { mfa_exempt_until: 4102444800000 } })).toBe(true)
|
||||
expect(shouldEnforceMfa({ app_metadata: { mfa_exempt: true } })).toBe(true)
|
||||
})
|
||||
|
||||
it('returns true when app_metadata is undefined', () => {
|
||||
vi.stubEnv('NEXT_PUBLIC_SELF_HOSTED', 'false')
|
||||
vi.stubEnv('NEXT_PUBLIC_REQUIRE_MFA', 'true')
|
||||
expect(shouldEnforceMfa({})).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('isMfaExemptionActive', () => {
|
||||
it('compares against the clock it is given', () => {
|
||||
const user = { app_metadata: { mfa_exempt_until: '2026-10-01T00:00:00Z' } }
|
||||
expect(isMfaExemptionActive(user, new Date('2026-09-30T23:59:59Z'))).toBe(true)
|
||||
expect(isMfaExemptionActive(user, new Date('2026-10-01T00:00:00Z'))).toBe(false)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -12,12 +12,37 @@ export function isMfaRequired(): boolean {
|
||||
return flagEnabled(process.env.NEXT_PUBLIC_REQUIRE_MFA)
|
||||
}
|
||||
|
||||
/**
|
||||
* A time-boxed exemption: `app_metadata.mfa_exempt_until` holds an ISO
|
||||
* timestamp, and the gate is skipped only while that instant is in the
|
||||
* future. app_metadata is written only through the service role (never from a
|
||||
* browser session), so this is an operator switch for the one kind of account
|
||||
* that must be usable by someone who cannot enrol an authenticator: Google's
|
||||
* OAuth verification reviewers, who log in with credentials we hand them and
|
||||
* treat any second factor as an "authentication blocker".
|
||||
*
|
||||
* Time-boxed rather than a boolean so a forgotten flag cannot outlive the
|
||||
* review: the exemption dies on its own. Anything malformed enforces MFA.
|
||||
*/
|
||||
export function isMfaExemptionActive(
|
||||
user: { app_metadata?: Record<string, unknown> },
|
||||
now: Date = new Date(),
|
||||
): boolean {
|
||||
const until = user.app_metadata?.mfa_exempt_until
|
||||
if (typeof until !== 'string') return false
|
||||
const expires = Date.parse(until)
|
||||
if (Number.isNaN(expires)) return false
|
||||
return expires > now.getTime()
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if MFA should be enforced for a specific user.
|
||||
* BankID-linked users skip TOTP because BankID is inherently 2FA.
|
||||
* A live, time-boxed exemption (see isMfaExemptionActive) also skips it.
|
||||
*/
|
||||
export function shouldEnforceMfa(user: { app_metadata?: Record<string, unknown> }): boolean {
|
||||
if (!isMfaRequired()) return false
|
||||
if (user.app_metadata?.bankid_linked) return false
|
||||
if (isMfaExemptionActive(user)) return false
|
||||
return true
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user