Require AAL2 for platform admin MFA

supabase/config.toml

@@ -250,8 +250,8 @@ max_enrolled_factors = 10
 
 # Control MFA via App Authenticator (TOTP)
 [auth.mfa.totp]
-enroll_enabled = false
-verify_enabled = false
+enroll_enabled = true
+verify_enabled = true
 
 # Configure MFA via Phone Messaging
 [auth.mfa.phone]

supabase/migrations/20260512003000_require_aal2_for_platform_admin_mfa.sql

@@ -0,0 +1,53 @@
+CREATE OR REPLACE FUNCTION "public"."verify_mfa"()
+RETURNS boolean
+LANGUAGE "plpgsql"
+SECURITY DEFINER
+SET "search_path" TO ''
+AS $$
+BEGIN
+  -- Email OTP and magic-link first-factor sessions can carry amr.method = 'otp'
+  -- while remaining aal1, so MFA authorization must use the authoritative aal
+  -- claim instead of accepting OTP method metadata.
+  -- Zero-factor users are intentionally allowed at aal1 here; platform-admin
+  -- paths must independently require aal2 before calling the admin-secret check.
+  RETURN (
+    array[(SELECT coalesce(auth.jwt()->>'aal', 'aal1'))] <@ (
+      SELECT
+          CASE
+            WHEN count(id) > 0 THEN array['aal2']
+            ELSE array['aal1', 'aal2']
+          END AS aal
+        FROM auth.mfa_factors
+        WHERE (SELECT auth.uid()) = user_id AND status = 'verified'
+    )
+  );
+END;
+$$;
+
+ALTER FUNCTION "public"."verify_mfa"() OWNER TO "postgres";
+REVOKE ALL ON FUNCTION "public"."verify_mfa"() FROM PUBLIC;
+GRANT EXECUTE ON FUNCTION "public"."verify_mfa"() TO "anon";
+GRANT EXECUTE ON FUNCTION "public"."verify_mfa"() TO "authenticated";
+GRANT EXECUTE ON FUNCTION "public"."verify_mfa"() TO "service_role";
+
+CREATE OR REPLACE FUNCTION "public"."is_platform_admin"()
+RETURNS boolean
+LANGUAGE "plpgsql"
+SECURITY DEFINER
+SET "search_path" TO ''
+AS $$
+BEGIN
+  -- Platform-admin actions are privileged even for admins without an enrolled
+  -- factor row, so require an MFA-verified session before checking the secret.
+  IF coalesce(auth.jwt()->>'aal', 'aal1') <> 'aal2' THEN
+    RETURN false;
+  END IF;
+
+  RETURN public.is_platform_admin((SELECT auth.uid()));
+END;
+$$;
+
+ALTER FUNCTION "public"."is_platform_admin"() OWNER TO "postgres";
+REVOKE ALL ON FUNCTION "public"."is_platform_admin"() FROM PUBLIC;
+REVOKE EXECUTE ON FUNCTION "public"."is_platform_admin"() FROM "service_role";
+GRANT EXECUTE ON FUNCTION "public"."is_platform_admin"() TO "authenticated";

supabase/tests/07_auth_functions.sql

@@ -1,10 +1,15 @@
 BEGIN;
 
 
-SELECT plan(16);
+SELECT plan(18);
 
 -- Test is_platform_admin wrapper
 SELECT tests.authenticate_as('test_admin');
+SELECT set_config(
+    'request.jwt.claims',
+    (current_setting('request.jwt.claims', true)::jsonb || jsonb_build_object('aal', 'aal2'))::text,
+    true
+);
 
 SELECT
     is(
@@ -26,6 +31,28 @@ SELECT
 
 SELECT tests.clear_authentication();
 
+SELECT
+    is(
+        has_function_privilege(
+            'service_role'::name,
+            'public.is_platform_admin()'::regprocedure,
+            'EXECUTE'
+        ),
+        false,
+        'is_platform_admin wrapper test - service_role cannot execute user-context wrapper'
+    );
+
+SELECT
+    is(
+        has_function_privilege(
+            'service_role'::name,
+            'public.is_platform_admin(uuid)'::regprocedure,
+            'EXECUTE'
+        ),
+        true,
+        'is_platform_admin wrapper test - service_role can execute private uuid overload'
+    );
+
 -- Test split behavior when an RBAC role exists (RBAC roles should not affect is_platform_admin)
 SET LOCAL ROLE service_role;
 INSERT INTO public.orgs (id, created_by, name, management_email)

tests/admin-credits.test.ts

@@ -1,8 +1,6 @@
 import { randomUUID } from 'node:crypto'
-import { env } from 'node:process'
-import { createClient } from '@supabase/supabase-js'
 import { afterAll, beforeAll, describe, expect, it } from 'vitest'
-import { BASE_URL, getSupabaseClient, headers, ORG_ID, TEST_EMAIL, USER_ID } from './test-utils.ts'
+import { BASE_URL, getAal2AuthHeadersForCredentials, getSupabaseClient, headers, ORG_ID, TEST_EMAIL, USER_ID } from './test-utils.ts'
 
 // Test organization for admin credits tests
 const TEST_ORG_ID = randomUUID()
@@ -17,34 +15,7 @@ async function getAdminHeaders() {
   if (adminHeadersCache)
     return adminHeadersCache
 
-  const supabaseUrl = env.SUPABASE_URL
-  const supabaseAnonKey = env.SUPABASE_ANON_KEY
-
-  if (!supabaseUrl || !supabaseAnonKey) {
-    throw new Error('SUPABASE_URL or SUPABASE_ANON_KEY is missing for admin auth headers')
-  }
-
-  const supabase = createClient(supabaseUrl, supabaseAnonKey, {
-    auth: {
-      persistSession: false,
-      autoRefreshToken: false,
-      detectSessionInUrl: false,
-    },
-  })
-
-  const { data, error } = await supabase.auth.signInWithPassword({
-    email: ADMIN_EMAIL,
-    password: ADMIN_PASSWORD,
-  })
-
-  if (error || !data.session?.access_token) {
-    throw error ?? new Error('Unable to obtain admin JWT for tests')
-  }
-
-  adminHeadersCache = {
-    'Content-Type': 'application/json',
-    'Authorization': `Bearer ${data.session.access_token}`,
-  }
+  adminHeadersCache = await getAal2AuthHeadersForCredentials(ADMIN_EMAIL, ADMIN_PASSWORD)
 
   return adminHeadersCache
 }

tests/admin-stats.test.ts

@@ -1,6 +1,6 @@
 import { randomUUID } from 'node:crypto'
 import { afterAll, beforeAll, describe, expect, it } from 'vitest'
-import { BASE_URL, fetchWithRetry, getAuthHeadersForCredentials, getSupabaseClient, PRODUCT_ID, TEST_EMAIL, USER_ADMIN_EMAIL, USER_ID } from './test-utils.ts'
+import { BASE_URL, fetchWithRetry, getAal2AuthHeadersForCredentials, getSupabaseClient, PRODUCT_ID, TEST_EMAIL, USER_ADMIN_EMAIL, USER_ID } from './test-utils.ts'
 
 const DAY_IN_MS = 24 * 60 * 60 * 1000
 const NOW = Date.now()
@@ -55,7 +55,7 @@ let creatorUserCreatedAt = ''
 beforeAll(async () => {
   const supabase = getSupabaseClient()
 
-  adminHeaders = await getAuthHeadersForCredentials(USER_ADMIN_EMAIL, 'adminadmin')
+  adminHeaders = await getAal2AuthHeadersForCredentials(USER_ADMIN_EMAIL, 'adminadmin')
 
   const [{ data: planRow, error: planError }, { data: userRow, error: userError }] = await Promise.all([
     supabase.from('plans').select('name, price_m_id, price_y_id, stripe_id').eq('stripe_id', PRODUCT_ID).single(),

tests/audit-logs.test.ts

@@ -441,8 +441,8 @@ describe('audit log triggers', () => {
 // These tests verify that audit logs are created when using API key authentication
 // This was a bug where CLI/API users were not logged because get_identity() didn't check API keys
 describe('audit logs for app_versions via API key', () => {
-  const testVersionName = `99.0.0-audit-test-${randomUUID()}`
   let createdVersionId: number | null = null
+  let createdVersionName: string | null = null
 
   afterAll(async () => {
     // Clean up: delete the test version if it was created
@@ -454,6 +454,9 @@ describe('audit logs for app_versions via API key', () => {
   })
 
   it('app_version INSERT via API creates audit log with user_id from API key', async () => {
+    const testVersionName = `99.0.0-audit-test-${randomUUID()}`
+    createdVersionName = testVersionName
+
     // Create a bundle via the API (uses API key authentication)
     const response = await fetchWithRetry(`${BASE_URL}/bundle`, {
       method: 'POST',
@@ -475,34 +478,20 @@ describe('audit logs for app_versions via API key', () => {
     // Wait for the trigger to execute
     await new Promise(resolve => setTimeout(resolve, 200))
 
-    // Fetch audit logs for the dedicated test org
-    const auditResponse = await fetchWithRetry(`${BASE_URL}/organization/audit?orgId=${ORG_ID}&tableName=app_versions&operation=INSERT`, {
-      headers: authHeaders,
-    })
-    expect(auditResponse.status).toBe(200)
-    const auditData = await auditResponse.json()
-    const safe = parseAuditLogsResponse(auditData)
-    expect(safe.success).toBe(true)
-
-    if (safe.success) {
-      // Find the audit log for our created version
-      const versionAuditLog = safe.data.data.find(
-        log => log.record_id === createdVersionId?.toString(),
-      )
+    const versionAuditLog = await waitForAuditLog(
+      `${BASE_URL}/organization/audit?orgId=${ORG_ID}&tableName=app_versions&operation=INSERT`,
+      log => log.record_id === createdVersionId?.toString(),
+    )
 
-      expect(versionAuditLog).toBeTruthy()
-      if (versionAuditLog) {
-        expect(versionAuditLog.operation).toBe('INSERT')
-        expect(versionAuditLog.table_name).toBe('app_versions')
-        expect(versionAuditLog.org_id).toBe(ORG_ID)
-        // This is the key assertion: user_id should be set from the API key
-        expect(versionAuditLog.user_id).toBe(USER_ID)
-        expect(versionAuditLog.old_record).toBeNull()
-        expect(versionAuditLog.new_record).toBeTruthy()
-        if (versionAuditLog.new_record && typeof versionAuditLog.new_record === 'object') {
-          expect((versionAuditLog.new_record as Record<string, unknown>).name).toBe(testVersionName)
-        }
-      }
+    expect(versionAuditLog.operation).toBe('INSERT')
+    expect(versionAuditLog.table_name).toBe('app_versions')
+    expect(versionAuditLog.org_id).toBe(ORG_ID)
+    // This is the key assertion: user_id should be set from the API key
+    expect(versionAuditLog.user_id).toBe(USER_ID)
+    expect(versionAuditLog.old_record).toBeNull()
+    expect(versionAuditLog.new_record).toBeTruthy()
+    if (versionAuditLog.new_record && typeof versionAuditLog.new_record === 'object') {
+      expect((versionAuditLog.new_record as Record<string, unknown>).name).toBe(testVersionName)
     }
   })
 
@@ -545,7 +534,7 @@ describe('audit logs for app_versions via API key', () => {
 
   it('app_version soft-DELETE via API creates UPDATE audit log with user_id from API key', async () => {
     // Skip if we don't have a version to delete
-    if (!createdVersionId) {
+    if (!createdVersionId || !createdVersionName) {
       console.warn('Skipping DELETE test: no version was created')
       return
     }
@@ -558,7 +547,7 @@ describe('audit logs for app_versions via API key', () => {
       headers: apiKeyAuthHeaders,
       body: JSON.stringify({
         app_id: APIKEY_AUDIT_APP_ID,
-        version: testVersionName,
+        version: createdVersionName,
       }),
     })
 

tests/is-admin-functions.test.ts

@@ -63,6 +63,20 @@ describe('is_platform_admin SQL function', () => {
     }
   }
 
+  async function setAuthClaims(query: QueryFn, claims: Record<string, unknown>) {
+    const sub = typeof claims.sub === 'string' ? claims.sub : ''
+    const email = typeof claims.email === 'string' ? claims.email : ''
+
+    await query('SELECT set_config(\'role\', \'authenticated\', true)')
+    await query('SELECT set_config(\'request.jwt.claim.sub\', $1, true)', [sub])
+    await query('SELECT set_config(\'request.jwt.claim.email\', $1, true)', [email])
+    await query('SELECT set_config(\'request.jwt.claim.role\', \'authenticated\', true)')
+    await query('SELECT set_config(\'request.jwt.claims\', $1, true)', [JSON.stringify({
+      role: 'authenticated',
+      ...claims,
+    })])
+  }
+
   afterAll(async () => {
     await pool.end()
   })
@@ -145,4 +159,72 @@ describe('is_platform_admin SQL function', () => {
       expect(regularUserResults.rows[0].is_platform_admin).toBe(false)
     })
   })
+
+  it.concurrent('does not treat email OTP sessions as platform-admin MFA', async () => {
+    await withTransaction(async (query) => {
+      const legacyAdmin = await getAdminUserId(query)
+
+      expect(legacyAdmin).toBeTruthy()
+
+      await query(`
+        INSERT INTO public.user_security (user_id, email_otp_verified_at, created_at, updated_at)
+        VALUES ($1::uuid, NOW(), NOW(), NOW())
+        ON CONFLICT (user_id) DO UPDATE
+        SET
+          email_otp_verified_at = EXCLUDED.email_otp_verified_at,
+          updated_at = NOW();
+      `, [legacyAdmin])
+
+      await query(`
+        INSERT INTO auth.mfa_factors (id, user_id, friendly_name, factor_type, status, created_at, updated_at)
+        VALUES (gen_random_uuid(), $1::uuid, 'Admin TOTP', 'totp'::auth.factor_type, 'verified'::auth.factor_status, NOW(), NOW());
+      `, [legacyAdmin])
+
+      await setAuthClaims(query, {
+        sub: legacyAdmin,
+        email: 'admin@example.test',
+        aal: 'aal1',
+        amr: [{ method: 'otp' }],
+      })
+
+      const otpOnly = await query('SELECT public.is_platform_admin() as is_platform_admin')
+
+      expect(otpOnly.rows[0].is_platform_admin).toBe(false)
+
+      await setAuthClaims(query, {
+        sub: legacyAdmin,
+        email: 'admin@example.test',
+        aal: 'aal2',
+        amr: [{ method: 'password' }, { method: 'totp' }],
+      })
+
+      const totpVerified = await query('SELECT public.is_platform_admin() as is_platform_admin')
+
+      expect(totpVerified.rows[0].is_platform_admin).toBe(true)
+    })
+  })
+
+  it.concurrent('requires aal2 for platform admins without enrolled factors', async () => {
+    await withTransaction(async (query) => {
+      const legacyAdmin = await getAdminUserId(query)
+
+      expect(legacyAdmin).toBeTruthy()
+
+      await query(`
+        DELETE FROM auth.mfa_factors
+        WHERE user_id = $1::uuid;
+      `, [legacyAdmin])
+
+      await setAuthClaims(query, {
+        sub: legacyAdmin,
+        email: 'admin@example.test',
+        aal: 'aal1',
+        amr: [{ method: 'password' }],
+      })
+
+      const aal1Admin = await query('SELECT public.is_platform_admin() as is_platform_admin')
+
+      expect(aal1Admin.rows[0].is_platform_admin).toBe(false)
+    })
+  })
 })