diff --git a/supabase/config.toml b/supabase/config.toml index 38d3f7667b..bce3d7dae6 100644 --- a/supabase/config.toml +++ b/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] diff --git a/supabase/migrations/20260512003000_require_aal2_for_platform_admin_mfa.sql b/supabase/migrations/20260512003000_require_aal2_for_platform_admin_mfa.sql new file mode 100644 index 0000000000..3fb1f76211 --- /dev/null +++ b/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"; diff --git a/supabase/tests/07_auth_functions.sql b/supabase/tests/07_auth_functions.sql index 9f8483f703..f4055793da 100644 --- a/supabase/tests/07_auth_functions.sql +++ b/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) diff --git a/tests/admin-credits.test.ts b/tests/admin-credits.test.ts index 6d09bcff42..1da993a5a0 100644 --- a/tests/admin-credits.test.ts +++ b/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 } diff --git a/tests/admin-stats.test.ts b/tests/admin-stats.test.ts index 366067fa44..ef7612bba1 100644 --- a/tests/admin-stats.test.ts +++ b/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(), diff --git a/tests/audit-logs.test.ts b/tests/audit-logs.test.ts index 0f65f544ae..019578f845 100644 --- a/tests/audit-logs.test.ts +++ b/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).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).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, }), }) diff --git a/tests/is-admin-functions.test.ts b/tests/is-admin-functions.test.ts index 2bcf02bc80..10e3ddedc2 100644 --- a/tests/is-admin-functions.test.ts +++ b/tests/is-admin-functions.test.ts @@ -63,6 +63,20 @@ describe('is_platform_admin SQL function', () => { } } + async function setAuthClaims(query: QueryFn, claims: Record) { + 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) + }) + }) }) diff --git a/tests/test-utils.ts b/tests/test-utils.ts index 39ecd7707d..28f6eb9e3e 100644 --- a/tests/test-utils.ts +++ b/tests/test-utils.ts @@ -1,6 +1,8 @@ import type { SupabaseClient } from '@supabase/supabase-js' import type { Database } from '../src/types/supabase.types' +import { Buffer } from 'node:buffer' import { spawnSync } from 'node:child_process' +import { createHmac } from 'node:crypto' import process, { env } from 'node:process' import { createClient } from '@supabase/supabase-js' import { Pool } from 'pg' @@ -331,6 +333,152 @@ export async function getAuthHeaders(): Promise> { export async function getAuthHeadersForCredentials(email: string, password: string): Promise> { return signInAndBuildAuthHeaders(email, password) } + +function decodeBase32(value: string): Buffer { + const alphabet = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ234567' + let bits = '' + + for (const char of value.toUpperCase().replace(/[\s=]/g, '')) { + const index = alphabet.indexOf(char) + if (index < 0) + throw new Error('Invalid TOTP secret') + bits += index.toString(2).padStart(5, '0') + } + + const bytes: number[] = [] + for (let offset = 0; offset + 8 <= bits.length; offset += 8) + bytes.push(Number.parseInt(bits.slice(offset, offset + 8), 2)) + + return Buffer.from(bytes) +} + +function generateTotp(secret: string, now = Date.now()): string { + const key = decodeBase32(secret) + const counter = Math.floor(now / 30000) + const buffer = Buffer.alloc(8) + buffer.writeBigUInt64BE(BigInt(counter)) + + const digest = createHmac('sha1', key).update(buffer).digest() + const offset = digest[digest.length - 1] & 0x0F + const binary = ((digest[offset] & 0x7F) << 24) + | ((digest[offset + 1] & 0xFF) << 16) + | ((digest[offset + 2] & 0xFF) << 8) + | (digest[offset + 3] & 0xFF) + + return (binary % 1_000_000).toString().padStart(6, '0') +} + +const MFA_TEST_FACTOR_PREFIX = 'Capgo test MFA ' +const MFA_TEST_FACTOR_LIKE = `${MFA_TEST_FACTOR_PREFIX}%` + +export async function getAal2AuthHeadersForCredentials(email: string, password: string): Promise> { + hydrateLocalSupabaseEnvFromStatus() + + const supabaseBaseUrl = normalizeLocalhostUrl(env.SUPABASE_URL) ?? SUPABASE_BASE_URL + const supabaseAnonKey = env.SUPABASE_ANON_KEY ?? SUPABASE_ANON_KEY + + if (!supabaseBaseUrl || !supabaseAnonKey) { + throw new Error('SUPABASE_URL or SUPABASE_ANON_KEY is missing for MFA auth headers') + } + + const supabase = createClient(supabaseBaseUrl, supabaseAnonKey, { + auth: { + persistSession: false, + autoRefreshToken: false, + detectSessionInUrl: false, + }, + }) + + const { data: signInData, error: signInError } = await supabase.auth.signInWithPassword({ + email, + password, + }) + + if (signInError || !signInData.session?.access_token || !signInData.user?.id) { + throw signInError ?? new Error('Unable to obtain JWT for MFA tests') + } + + // Keep MFA cleanup scoped to factors enrolled by this helper. + try { + await executeSQL(` + DELETE FROM auth.mfa_factors + WHERE user_id = $1::uuid + AND friendly_name LIKE $2 + AND created_at < NOW() - INTERVAL '10 minutes'; + `, [signInData.user.id, MFA_TEST_FACTOR_LIKE]) + } + catch (cleanupError) { + console.warn('Failed to clean up old MFA test factors', { + cleanupError, + userId: signInData.user.id, + friendlyNameLike: MFA_TEST_FACTOR_LIKE, + }) + } + + await executeSQL(` + 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(); + `, [signInData.user.id]) + + let enrolledFactorId: string | undefined + try { + const { data: enrollData, error: enrollError } = await supabase.auth.mfa.enroll({ + factorType: 'totp', + friendlyName: `${MFA_TEST_FACTOR_PREFIX}${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}`, + }) + + enrolledFactorId = enrollData?.id + + const totpSecret = enrollData?.totp?.secret + if (enrollError || !enrollData?.id || !totpSecret) { + throw enrollError ?? new Error('Unable to enroll TOTP factor for MFA tests') + } + + const { data: challengeData, error: challengeError } = await supabase.auth.mfa.challenge({ + factorId: enrollData.id, + }) + + if (challengeError || !challengeData?.id) { + throw challengeError ?? new Error('Unable to create TOTP challenge for MFA tests') + } + + const { data: verifyData, error: verifyError } = await supabase.auth.mfa.verify({ + factorId: enrollData.id, + challengeId: challengeData.id, + code: generateTotp(totpSecret), + }) + + if (verifyError) + throw verifyError + + const { data: sessionData } = await supabase.auth.getSession() + const accessToken = (verifyData as any)?.access_token + ?? (verifyData as any)?.session?.access_token + ?? sessionData.session?.access_token + if (!accessToken) + throw new Error('Unable to obtain aal2 JWT for MFA tests') + + return { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${accessToken}`, + } + } + finally { + if (enrolledFactorId) { + await executeSQL(` + DELETE FROM auth.mfa_factors + WHERE id = $1::uuid + AND user_id = $2::uuid + AND friendly_name LIKE $3; + `, [enrolledFactorId, signInData.user.id, MFA_TEST_FACTOR_LIKE]) + } + } +} + export const headersStats = { 'Content-Type': 'application/json', 'Authorization': APIKEY_STATS,