From 885b9cafdc5264345c344bfba10c83d45b0ad6b9 Mon Sep 17 00:00:00 2001 From: Martin Donadieu Date: Wed, 11 Mar 2026 13:01:05 +0100 Subject: [PATCH 1/3] fix(sql): split platform admin check --- .../_backend/private/admin_credits.ts | 4 +- .../functions/_backend/private/admin_stats.ts | 2 +- supabase/functions/_backend/private/log_as.ts | 2 +- .../functions/_backend/public/replication.ts | 2 +- ...11000000_split_is_admin_platform_admin.sql | 109 ++++++++++++++++++ 5 files changed, 114 insertions(+), 5 deletions(-) create mode 100644 supabase/migrations/20260311000000_split_is_admin_platform_admin.sql diff --git a/supabase/functions/_backend/private/admin_credits.ts b/supabase/functions/_backend/private/admin_credits.ts index d8dc9a3116..32cb9989fa 100644 --- a/supabase/functions/_backend/private/admin_credits.ts +++ b/supabase/functions/_backend/private/admin_credits.ts @@ -37,8 +37,8 @@ async function verifyAdmin(c: AppContext): Promise<{ isAdmin: boolean, userId: s const userId = auth.userId const userSupabase = supabaseClient(c, auth.jwt) - // is_admin() is MFA-aware and must run with the caller JWT context. - const { data: isAdmin, error: adminError } = await userSupabase.rpc('is_admin') + // is_platform_admin() is MFA-aware and must run with the caller JWT context. + const { data: isAdmin, error: adminError } = await userSupabase.rpc('is_platform_admin') if (adminError) { cloudlog({ requestId: c.get('requestId'), message: 'is_admin_error', error: adminError }) diff --git a/supabase/functions/_backend/private/admin_stats.ts b/supabase/functions/_backend/private/admin_stats.ts index fec2b899a4..69e730c1c1 100644 --- a/supabase/functions/_backend/private/admin_stats.ts +++ b/supabase/functions/_backend/private/admin_stats.ts @@ -80,7 +80,7 @@ app.post('/', middlewareAuth, async (c) => { // Verify user is admin const supabaseClient = useSupabaseClient(c, authToken) - const { data: isAdmin, error: adminError } = await supabaseClient.rpc('is_admin') + const { data: isAdmin, error: adminError } = await supabaseClient.rpc('is_platform_admin') if (adminError) { cloudlog({ requestId: c.get('requestId'), message: 'is_admin_error', error: adminError }) diff --git a/supabase/functions/_backend/private/log_as.ts b/supabase/functions/_backend/private/log_as.ts index 16cf9715a3..1116c8e01d 100644 --- a/supabase/functions/_backend/private/log_as.ts +++ b/supabase/functions/_backend/private/log_as.ts @@ -27,7 +27,7 @@ app.post('/', middlewareAuth, async (c) => { const supabaseAdmin = await useSupabaseAdmin(c) const supabaseClient = useSupabaseClient(c, authToken) - const { data: isAdmin, error: adminError } = await supabaseClient.rpc('is_admin') + const { data: isAdmin, error: adminError } = await supabaseClient.rpc('is_platform_admin') if (adminError) { throw simpleError('is_admin_error', 'Is admin error', { adminError }) } diff --git a/supabase/functions/_backend/public/replication.ts b/supabase/functions/_backend/public/replication.ts index 8c433a3341..ed0d3f4e8e 100644 --- a/supabase/functions/_backend/public/replication.ts +++ b/supabase/functions/_backend/public/replication.ts @@ -175,7 +175,7 @@ async function validateReplicationAccess(c: ReplicationContext) { }) const userClient = supabaseClient(c, authorization) - const { data: isAdmin, error: adminError } = await userClient.rpc('is_admin') + const { data: isAdmin, error: adminError } = await userClient.rpc('is_platform_admin') if (adminError) { cloudlogErr({ requestId: c.get('requestId'), message: 'replication_is_admin_error', error: adminError }) throw quickError(500, 'is_admin_error', 'Unable to verify admin rights') diff --git a/supabase/migrations/20260311000000_split_is_admin_platform_admin.sql b/supabase/migrations/20260311000000_split_is_admin_platform_admin.sql new file mode 100644 index 0000000000..10e597e854 --- /dev/null +++ b/supabase/migrations/20260311000000_split_is_admin_platform_admin.sql @@ -0,0 +1,109 @@ +-- Split platform admin detection from is_admin +CREATE OR REPLACE FUNCTION public.is_platform_admin(userid uuid) +RETURNS boolean +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + admin_ids_jsonb jsonb; + is_admin_legacy boolean := false; + mfa_verified boolean; + rbac_enabled boolean; + has_platform_admin boolean := false; +BEGIN + SELECT public.verify_mfa() INTO mfa_verified; + IF NOT mfa_verified THEN + RETURN false; + END IF; + + SELECT decrypted_secret::jsonb INTO admin_ids_jsonb + FROM vault.decrypted_secrets + WHERE name = 'admin_users'; + + is_admin_legacy := COALESCE(admin_ids_jsonb ? userid::text, false); + + SELECT use_new_rbac INTO rbac_enabled + FROM public.rbac_settings + WHERE id = 1; + + IF COALESCE(rbac_enabled, false) THEN + SELECT EXISTS ( + SELECT 1 + FROM public.role_bindings rb + JOIN public.roles r ON r.id = rb.role_id + WHERE rb.principal_type = public.rbac_principal_user() + AND rb.principal_id = userid + AND rb.scope_type = public.rbac_scope_platform() + AND r.name = public.rbac_role_platform_super_admin() + ) INTO has_platform_admin; + END IF; + + RETURN is_admin_legacy OR has_platform_admin; +END; +$$; + +ALTER FUNCTION public.is_platform_admin(userid uuid) OWNER TO "postgres"; + +CREATE OR REPLACE FUNCTION public.is_platform_admin() +RETURNS boolean +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +BEGIN + RETURN public.is_platform_admin((SELECT auth.uid())); +END; +$$; + +ALTER FUNCTION public.is_platform_admin() OWNER TO "postgres"; + +GRANT ALL ON FUNCTION public.is_platform_admin(userid uuid) TO "anon"; +GRANT ALL ON FUNCTION public.is_platform_admin(userid uuid) TO "authenticated"; +GRANT ALL ON FUNCTION public.is_platform_admin(userid uuid) TO "service_role"; +GRANT ALL ON FUNCTION public.is_platform_admin() TO "anon"; +GRANT ALL ON FUNCTION public.is_platform_admin() TO "authenticated"; +GRANT ALL ON FUNCTION public.is_platform_admin() TO "service_role"; + +COMMENT ON FUNCTION public.is_platform_admin(uuid) IS 'Checks if a user is a platform admin. In RBAC mode, accepts legacy vault list or platform_super_admin role. In legacy mode, only accepts legacy vault list. Always requires MFA.'; + +CREATE OR REPLACE FUNCTION public.is_admin(userid uuid) +RETURNS boolean +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + admin_ids_jsonb jsonb; + is_admin_legacy boolean; + mfa_verified boolean; +BEGIN + SELECT public.verify_mfa() INTO mfa_verified; + IF NOT mfa_verified THEN + RETURN false; + END IF; + + SELECT decrypted_secret::jsonb INTO admin_ids_jsonb + FROM vault.decrypted_secrets + WHERE name = 'admin_users'; + + is_admin_legacy := COALESCE(admin_ids_jsonb ? userid::text, false); + + RETURN is_admin_legacy; +END; +$$; + +COMMENT ON FUNCTION public.is_admin(uuid) IS 'Checks if a user is listed in legacy platform admin secret. Always requires MFA.'; + +CREATE OR REPLACE FUNCTION public.is_admin() +RETURNS boolean +LANGUAGE plpgsql +SET + search_path = '' +AS $$ +BEGIN + RETURN public.is_admin((SELECT auth.uid())); +END; +$$; + +COMMENT ON FUNCTION public.is_admin() IS 'Legacy platform admin helper. Checks if the current user is listed in admin_users.'; From 2525a13ae4b348fd2e2c1e45e10d66849551ab52 Mon Sep 17 00:00:00 2001 From: Martin Donadieu Date: Wed, 11 Mar 2026 13:08:02 +0100 Subject: [PATCH 2/3] test(db): split is_admin/platform_admin behavior --- tests/is-admin-functions.test.ts | 115 +++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 tests/is-admin-functions.test.ts diff --git a/tests/is-admin-functions.test.ts b/tests/is-admin-functions.test.ts new file mode 100644 index 0000000000..e0bdb0f8ab --- /dev/null +++ b/tests/is-admin-functions.test.ts @@ -0,0 +1,115 @@ +import { randomUUID } from 'node:crypto' +import { Pool, type PoolClient } from 'pg' +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest' +import { POSTGRES_URL } from './test-utils.ts' + +describe('is_admin / is_platform_admin SQL functions', () => { + let pool: Pool + let client: PoolClient + + const query = (text: string, values: Array = []) => { + return client.query(text, values) + } + + beforeAll(() => { + pool = new Pool({ connectionString: POSTGRES_URL }) + }) + + beforeEach(async () => { + client = await pool.connect() + await query('BEGIN') + }) + + afterEach(async () => { + if (!client) + return + + try { + await query('ROLLBACK') + } + finally { + client.release() + } + }) + + afterAll(async () => { + await pool.end() + }) + + it('keeps is_admin legacy-only even when RBAC is enabled', async () => { + const legacyAdmin = randomUUID() + const nonAdmin = randomUUID() + + await query(` + UPDATE public.rbac_settings + SET use_new_rbac = false + WHERE id = 1; + `) + + await query(` + UPDATE vault.decrypted_secrets + SET decrypted_secret = ( + COALESCE(decrypted_secret::jsonb, '{}'::jsonb) + || $1::jsonb + )::text + WHERE name = 'admin_users'; + `, [JSON.stringify({ [legacyAdmin]: true })]) + + const legacy = await query( + 'SELECT public.is_admin($1::uuid) as is_admin, public.is_platform_admin($1::uuid) as is_platform_admin', + [legacyAdmin], + ) + + const regular = await query( + 'SELECT public.is_admin($1::uuid) as is_admin, public.is_platform_admin($1::uuid) as is_platform_admin', + [nonAdmin], + ) + + expect(legacy.rows[0].is_admin).toBe(true) + expect(legacy.rows[0].is_platform_admin).toBe(true) + expect(regular.rows[0].is_admin).toBe(false) + expect(regular.rows[0].is_platform_admin).toBe(false) + }) + + it('moves RBAC platform super-admin checks into is_platform_admin only', async () => { + const rbacUserId = randomUUID() + const normalUserId = randomUUID() + + await query(` + UPDATE public.rbac_settings + SET use_new_rbac = true + WHERE id = 1; + `) + + await query(` + INSERT INTO public.role_bindings ( + principal_type, + principal_id, + role_id, + scope_type, + granted_by + ) VALUES ( + public.rbac_principal_user(), + $1::uuid, + (SELECT id FROM public.roles WHERE name = public.rbac_role_platform_super_admin()), + public.rbac_scope_platform(), + $1::uuid + ); + `, [rbacUserId]) + + const rbacUserResults = await query( + 'SELECT public.is_admin($1::uuid) as is_admin, public.is_platform_admin($1::uuid) as is_platform_admin', + [rbacUserId], + ) + + const regularUserResults = await query( + 'SELECT public.is_admin($1::uuid) as is_admin, public.is_platform_admin($1::uuid) as is_platform_admin', + [normalUserId], + ) + + expect(rbacUserResults.rows[0].is_admin).toBe(false) + expect(rbacUserResults.rows[0].is_platform_admin).toBe(true) + expect(regularUserResults.rows[0].is_admin).toBe(false) + expect(regularUserResults.rows[0].is_platform_admin).toBe(false) + }) +}) From fbec0534ee8dddad7299eb7ddfb90b7fdb9805bd Mon Sep 17 00:00:00 2001 From: Martin Donadieu Date: Wed, 11 Mar 2026 13:09:26 +0100 Subject: [PATCH 3/3] test(db): split is_admin/platform_admin behavior --- tests/is-admin-functions.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/is-admin-functions.test.ts b/tests/is-admin-functions.test.ts index e0bdb0f8ab..786df35402 100644 --- a/tests/is-admin-functions.test.ts +++ b/tests/is-admin-functions.test.ts @@ -1,5 +1,6 @@ +import type { PoolClient } from 'pg' import { randomUUID } from 'node:crypto' -import { Pool, type PoolClient } from 'pg' +import { Pool } from 'pg' import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest' import { POSTGRES_URL } from './test-utils.ts'