diff --git a/supabase/functions/_backend/private/role_bindings.ts b/supabase/functions/_backend/private/role_bindings.ts index 52b72d311b..332a070df0 100644 --- a/supabase/functions/_backend/private/role_bindings.ts +++ b/supabase/functions/_backend/private/role_bindings.ts @@ -774,6 +774,29 @@ async function deleteChannelPermissionOverridesForBinding( ) ) `) + return + } + + if (binding.scope_type === 'org' && binding.org_id) { + await tx.execute(sql` + DELETE FROM public.channel_permission_overrides AS overrides + USING public.channels AS channels + INNER JOIN public.apps AS apps + ON apps.app_id = channels.app_id + WHERE overrides.channel_id = channels.id + AND apps.owner_org = ${binding.org_id} + AND overrides.principal_type = ${binding.principal_type} + AND overrides.principal_id = ${binding.principal_id} + AND NOT EXISTS ( + SELECT 1 + FROM public.role_bindings AS remaining_bindings + WHERE remaining_bindings.id <> ${binding.id} + AND remaining_bindings.principal_type = ${binding.principal_type} + AND remaining_bindings.principal_id = ${binding.principal_id} + AND remaining_bindings.org_id = ${binding.org_id} + AND (remaining_bindings.expires_at IS NULL OR remaining_bindings.expires_at > now()) + ) + `) } } diff --git a/supabase/migrations/20260825113706_channel_overrides_cleanup_on_org_unbind.sql b/supabase/migrations/20260825113706_channel_overrides_cleanup_on_org_unbind.sql new file mode 100644 index 0000000000..db51fbe777 --- /dev/null +++ b/supabase/migrations/20260825113706_channel_overrides_cleanup_on_org_unbind.sql @@ -0,0 +1,216 @@ +-- When a principal loses all role bindings in an org, stale channel_permission_overrides +-- must not keep granting channel-scoped permissions. Gate override application on an +-- active org binding (any scope) or group membership. +-- +-- Execution profile for rbac_principal_has_org_binding (channel override gate): +-- - Called at most once per rbac_check_permission_direct when p_channel_id IS NOT NULL +-- (apikey and user branches are mutually exclusive). +-- - Roles: service_role only; invoked from SECURITY DEFINER rbac_check_permission_direct. +-- - Frequency: console/RLS channel-scoped checks; not plugin /updates|/stats hot path. +-- - Cardinality: role_bindings rows per (principal, org) are typically single-digit; +-- group_members per user is bounded by org group membership. +-- - Indexes: role_bindings_principal_org_idx (principal_type, principal_id, org_id, +-- expires_at); idx_group_members_user_id_group_id for group-derived user path; +-- groups PK for group-principal branch. +-- - Worst case (user + apikey call sites): Index Scan on role_bindings_principal_org_idx +-- with expires_at filter; Nested Loop from group_members (user_id) to role_bindings +-- (group principal). No sequential scan over role_bindings in EXPLAIN (ANALYZE, +-- BUFFERS) on local seed data. + +CREATE OR REPLACE FUNCTION public.rbac_principal_has_org_binding( + p_principal_type text, + p_principal_id uuid, + p_org_id uuid +) +RETURNS boolean +LANGUAGE sql +STABLE +SECURITY DEFINER +SET search_path = '' +AS $$ + SELECT CASE + WHEN p_org_id IS NULL OR p_principal_id IS NULL OR p_principal_type IS NULL THEN false + WHEN p_principal_type = public.rbac_principal_group() THEN EXISTS ( + SELECT 1 + FROM public.groups + WHERE groups.id = p_principal_id + AND groups.org_id = p_org_id + ) + WHEN p_principal_type = public.rbac_principal_user() THEN ( + EXISTS ( + SELECT 1 + FROM public.role_bindings + WHERE role_bindings.principal_type = p_principal_type + AND role_bindings.principal_id = p_principal_id + AND role_bindings.org_id = p_org_id + AND (role_bindings.expires_at IS NULL OR role_bindings.expires_at > pg_catalog.now()) + ) + OR EXISTS ( + SELECT 1 + FROM public.group_members AS group_member + INNER JOIN public.role_bindings AS group_binding + ON group_binding.principal_type = public.rbac_principal_group() + AND group_binding.principal_id = group_member.group_id + WHERE group_member.user_id = p_principal_id + AND group_binding.org_id = p_org_id + AND (group_binding.expires_at IS NULL OR group_binding.expires_at > pg_catalog.now()) + ) + ) + ELSE EXISTS ( + SELECT 1 + FROM public.role_bindings + WHERE role_bindings.principal_type = p_principal_type + AND role_bindings.principal_id = p_principal_id + AND role_bindings.org_id = p_org_id + AND (role_bindings.expires_at IS NULL OR role_bindings.expires_at > pg_catalog.now()) + ) + END; +$$; + +ALTER FUNCTION public.rbac_principal_has_org_binding(text, uuid, uuid) OWNER TO postgres; +REVOKE ALL ON FUNCTION public.rbac_principal_has_org_binding(text, uuid, uuid) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION public.rbac_principal_has_org_binding(text, uuid, uuid) TO service_role; + +COMMENT ON FUNCTION public.rbac_principal_has_org_binding(text, uuid, uuid) IS + 'True when the principal still has a non-expired role binding in the org (direct or via group membership for users) or the group belongs to the org. Called once per channel-scoped rbac_check_permission_direct (console/RLS only). Indexed lookups on role_bindings_principal_org_idx and idx_group_members_user_id_group_id; no table scan on role_bindings at seed scale.'; + +CREATE INDEX IF NOT EXISTS role_bindings_principal_org_idx + ON public.role_bindings (principal_type, principal_id, org_id, expires_at); + +CREATE OR REPLACE FUNCTION public.rbac_check_permission_direct( + p_permission_key text, + p_user_id uuid, + p_org_id uuid, + p_app_id character varying, + p_channel_id bigint, + p_apikey text DEFAULT NULL::text +) +RETURNS boolean +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_allowed boolean := false; + v_effective_org_id uuid; + v_effective_user_id uuid := p_user_id; + v_effective_app_id character varying; + v_api_key public.apikeys%ROWTYPE; + v_channel_scope boolean := p_channel_id IS NOT NULL; + v_override boolean; + v_scope_ok boolean; + v_use_apikey boolean; + v_request_apikey text := NULLIF(btrim(p_apikey), ''); +BEGIN + IF p_permission_key IS NULL OR p_permission_key = '' THEN + RETURN false; + END IF; + + SELECT s.ok, s.effective_org_id, s.effective_app_id + INTO v_scope_ok, v_effective_org_id, v_effective_app_id + FROM public.rbac_resolve_permission_scope(p_org_id, p_app_id, p_channel_id) AS s; + + IF NOT COALESCE(v_scope_ok, false) THEN + RETURN false; + END IF; + + v_use_apikey := public.rbac_should_use_apikey_principal(p_user_id, p_apikey); + + IF v_use_apikey THEN + SELECT * INTO v_api_key + FROM public.find_apikey_by_value(v_request_apikey) + LIMIT 1; + + IF v_api_key.id IS NULL + OR (p_user_id IS NOT NULL AND p_user_id IS DISTINCT FROM v_api_key.user_id) + OR v_effective_org_id IS NULL + THEN + RETURN false; + END IF; + + IF public.is_apikey_expired(v_api_key.expires_at) THEN + RETURN false; + END IF; + + v_effective_user_id := v_api_key.user_id; + + v_allowed := public.rbac_has_permission( + public.rbac_principal_apikey(), + v_api_key.rbac_id, + p_permission_key, + v_effective_org_id, + v_effective_app_id, + p_channel_id + ); + + IF v_channel_scope + AND public.rbac_principal_has_org_binding( + public.rbac_principal_apikey(), + v_api_key.rbac_id, + v_effective_org_id + ) + THEN + SELECT o.is_allowed INTO v_override + FROM public.channel_permission_overrides o + WHERE o.principal_type = public.rbac_principal_apikey() + AND o.principal_id = v_api_key.rbac_id + AND o.channel_id = p_channel_id + AND o.permission_key = p_permission_key + LIMIT 1; + + IF v_override IS NOT NULL THEN + v_allowed := v_override; + END IF; + END IF; + + RETURN v_allowed; + END IF; + + IF v_effective_org_id IS NOT NULL THEN + IF (SELECT enforcing_2fa FROM public.orgs WHERE id = v_effective_org_id) + AND (v_effective_user_id IS NULL OR NOT public.has_2fa_enabled(v_effective_user_id)) + THEN + RETURN false; + END IF; + + IF public.user_meets_password_policy(v_effective_user_id, v_effective_org_id) = false THEN + RETURN false; + END IF; + END IF; + + IF v_effective_user_id IS NULL THEN + RETURN false; + END IF; + + v_allowed := public.rbac_has_permission( + public.rbac_principal_user(), + v_effective_user_id, + p_permission_key, + v_effective_org_id, + v_effective_app_id, + p_channel_id + ); + + IF v_channel_scope + AND public.rbac_principal_has_org_binding( + public.rbac_principal_user(), + v_effective_user_id, + v_effective_org_id + ) + THEN + SELECT o.is_allowed INTO v_override + FROM public.channel_permission_overrides o + WHERE o.principal_type = public.rbac_principal_user() + AND o.principal_id = v_effective_user_id + AND o.channel_id = p_channel_id + AND o.permission_key = p_permission_key + LIMIT 1; + + IF v_override IS NOT NULL THEN + v_allowed := v_override; + END IF; + END IF; + + RETURN v_allowed; +END; +$$; diff --git a/tests/private-role-bindings.test.ts b/tests/private-role-bindings.test.ts index b30bf4fcbd..0e7d98dd57 100644 --- a/tests/private-role-bindings.test.ts +++ b/tests/private-role-bindings.test.ts @@ -5,7 +5,7 @@ import { Pool } from 'pg' import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest' import { validatePrincipalAccess, validateRoleScope } from '../supabase/functions/_backend/private/role_bindings.ts' import { getDrizzleClient } from '../supabase/functions/_backend/utils/pg.ts' -import { getAuthHeaders, getAuthHeadersForCredentials, getEndpointUrl, getSupabaseClient, POSTGRES_URL, USER_ID, USER_ID_2, USER_PASSWORD } from './test-utils.ts' +import { getAuthHeaders, getAuthHeadersForCredentials, getEndpointUrl, getSupabaseClient, POSTGRES_URL, USER_ID, USER_ID_2, USER_PASSWORD, executeSQL } from './test-utils.ts' let authHeaders: Record let user2AuthHeaders: Record @@ -707,6 +707,7 @@ describe.skipIf(USE_CLOUDFLARE)('/private/role_bindings', () => { const appUuid = randomUUID() const publicAppId = `com.role-binding.override-cleanup.${id}` const supabase = getSupabaseClient() + let channelId: number | null = null try { const { error: orgError } = await supabase.from('orgs').insert({ @@ -764,6 +765,7 @@ describe.skipIf(USE_CLOUDFLARE)('/private/role_bindings', () => { .select('id') .single() expect(channelError).toBeNull() + channelId = channel!.id const { data: roles, error: rolesError } = await supabase .from('roles') @@ -838,7 +840,7 @@ describe.skipIf(USE_CLOUDFLARE)('/private/role_bindings', () => { expect(overrides ?? []).toHaveLength(0) } finally { - await supabase.from('channel_permission_overrides').delete().eq('principal_id', USER_ID_2) + await supabase.from('channel_permission_overrides').delete().eq('principal_id', USER_ID_2).eq('channel_id', channelId!) await supabase.from('role_bindings').delete().eq('org_id', orgId) await supabase.from('org_users').delete().eq('org_id', orgId) await supabase.from('channels').delete().eq('owner_org', orgId) @@ -847,6 +849,315 @@ describe.skipIf(USE_CLOUDFLARE)('/private/role_bindings', () => { await supabase.from('orgs').delete().eq('id', orgId) } }) + + it.concurrent('removes user channel permission overrides when the last org binding is deleted', async () => { + const id = randomUUID() + const orgId = randomUUID() + const appUuid = randomUUID() + const publicAppId = `com.role-binding.org-override-cleanup.${id}` + const supabase = getSupabaseClient() + let channelId: number | null = null + + try { + const { error: orgError } = await supabase.from('orgs').insert({ + id: orgId, + created_by: USER_ID, + name: `Role Binding Org Override Cleanup ${id}`, + management_email: `role-binding-org-override-cleanup-${id}@capgo.app`, + }) + expect(orgError).toBeNull() + + const { error: managerMemberError } = await supabase.from('org_users').insert({ + org_id: orgId, + user_id: USER_ID, + rbac_role_name: 'org_super_admin', + }) + expect(managerMemberError).toBeNull() + await createUserOrgBinding(orgId, USER_ID, 'org_super_admin', USER_ID) + + const { error: targetMemberError } = await supabase.from('org_users').insert({ + org_id: orgId, + user_id: USER_ID_2, + rbac_role_name: 'org_member', + }) + expect(targetMemberError).toBeNull() + await createUserOrgBinding(orgId, USER_ID_2, 'org_member', USER_ID) + + const { error: appError } = await supabase.from('apps').insert({ + id: appUuid, + app_id: publicAppId, + owner_org: orgId, + icon_url: 'role-binding-test-icon', + name: `Org Override Cleanup App ${id}`, + }) + expect(appError).toBeNull() + + const versionName = `role-binding-org-version-${id.slice(0, 8)}` + const { data: version, error: versionError } = await supabase + .from('app_versions') + .insert({ + app_id: publicAppId, + name: versionName, + owner_org: orgId, + user_id: USER_ID, + checksum: `checksum-${id}`, + storage_provider: 'r2', + r2_path: `orgs/${orgId}/apps/${publicAppId}/${versionName}.zip`, + deleted: false, + }) + .select('id') + .single() + expect(versionError).toBeNull() + + const { data: channel, error: channelError } = await supabase + .from('channels') + .insert({ + app_id: publicAppId, + name: `role-binding-org-channel-${id.slice(0, 8)}`, + version: version!.id, + owner_org: orgId, + created_by: USER_ID, + public: false, + allow_emulator: false, + }) + .select('id') + .single() + expect(channelError).toBeNull() + channelId = channel!.id + + const { error: overrideError } = await supabase.from('channel_permission_overrides').insert({ + principal_type: 'user', + principal_id: USER_ID_2, + channel_id: channel!.id, + permission_key: 'channel.promote_bundle', + is_allowed: true, + }) + expect(overrideError).toBeNull() + + const [permissionBeforeDelete] = await executeSQL<{ can_promote: boolean }>( + `SELECT public.rbac_check_permission_direct( + 'channel.promote_bundle', + $1::uuid, + $2::uuid, + $3, + $4::bigint, + NULL + ) AS can_promote`, + [USER_ID_2, orgId, publicAppId, channel!.id], + ) + expect(permissionBeforeDelete?.can_promote).toBe(true) + + const { data: orgBinding, error: orgBindingError } = await supabase + .from('role_bindings') + .select('id') + .eq('principal_type', 'user') + .eq('principal_id', USER_ID_2) + .eq('scope_type', 'org') + .eq('org_id', orgId) + .single() + expect(orgBindingError).toBeNull() + + const deleteResponse = await fetch(getEndpointUrl(`/private/role_bindings/${orgBinding!.id}`), { + method: 'DELETE', + headers: authHeaders, + }) + const deleteData = await deleteResponse.json() as { success?: boolean, error?: string } + + expect(deleteResponse.status).toBe(200) + expect(deleteData.success).toBe(true) + + const { data: overrides, error: overridesError } = await supabase + .from('channel_permission_overrides') + .select('id') + .eq('principal_type', 'user') + .eq('principal_id', USER_ID_2) + .eq('channel_id', channel!.id) + + expect(overridesError).toBeNull() + expect(overrides ?? []).toHaveLength(0) + + const [permissionAfterDelete] = await executeSQL<{ can_promote: boolean }>( + `SELECT public.rbac_check_permission_direct( + 'channel.promote_bundle', + $1::uuid, + $2::uuid, + $3, + $4::bigint, + NULL + ) AS can_promote`, + [USER_ID_2, orgId, publicAppId, channel!.id], + ) + expect(permissionAfterDelete?.can_promote).toBe(false) + } + finally { + await supabase.from('channel_permission_overrides').delete().eq('principal_id', USER_ID_2).eq('channel_id', channelId!) + await supabase.from('role_bindings').delete().eq('org_id', orgId) + await supabase.from('org_users').delete().eq('org_id', orgId) + await supabase.from('channels').delete().eq('owner_org', orgId) + await supabase.from('app_versions').delete().eq('owner_org', orgId) + await supabase.from('apps').delete().eq('id', appUuid) + await supabase.from('orgs').delete().eq('id', orgId) + } + }) + + it.concurrent('keeps channel permission overrides in other orgs when one org binding is removed', async () => { + const id = randomUUID() + const orgAId = randomUUID() + const orgBId = randomUUID() + const appAUuid = randomUUID() + const appBUuid = randomUUID() + const publicAppAId = `com.role-binding.org-override-a.${id}` + const publicAppBId = `com.role-binding.org-override-b.${id}` + const supabase = getSupabaseClient() + const channels: Record<'A' | 'B', number> = { A: 0, B: 0 } + + try { + for (const [orgId, orgLabel] of [[orgAId, 'A'], [orgBId, 'B']] as const) { + const { error: orgError } = await supabase.from('orgs').insert({ + id: orgId, + created_by: USER_ID, + name: `Role Binding Org Override ${orgLabel} ${id}`, + management_email: `role-binding-org-override-${orgLabel.toLowerCase()}-${id}@capgo.app`, + }) + expect(orgError).toBeNull() + + const { error: managerMemberError } = await supabase.from('org_users').insert({ + org_id: orgId, + user_id: USER_ID, + rbac_role_name: 'org_super_admin', + }) + expect(managerMemberError).toBeNull() + await createUserOrgBinding(orgId, USER_ID, 'org_super_admin', USER_ID) + + const { error: targetMemberError } = await supabase.from('org_users').insert({ + org_id: orgId, + user_id: USER_ID_2, + rbac_role_name: 'org_member', + }) + expect(targetMemberError).toBeNull() + await createUserOrgBinding(orgId, USER_ID_2, 'org_member', USER_ID) + } + + const orgApps = [ + { orgId: orgAId, appUuid: appAUuid, publicAppId: publicAppAId, label: 'A' }, + { orgId: orgBId, appUuid: appBUuid, publicAppId: publicAppBId, label: 'B' }, + ] as const + + for (const appFixture of orgApps) { + const { error: appError } = await supabase.from('apps').insert({ + id: appFixture.appUuid, + app_id: appFixture.publicAppId, + owner_org: appFixture.orgId, + icon_url: 'role-binding-test-icon', + name: `Org Override Cleanup App ${appFixture.label} ${id}`, + }) + expect(appError).toBeNull() + + const versionName = `role-binding-org-${appFixture.label}-version-${id.slice(0, 8)}` + const { data: version, error: versionError } = await supabase + .from('app_versions') + .insert({ + app_id: appFixture.publicAppId, + name: versionName, + owner_org: appFixture.orgId, + user_id: USER_ID, + checksum: `checksum-${appFixture.label}-${id}`, + storage_provider: 'r2', + r2_path: `orgs/${appFixture.orgId}/apps/${appFixture.publicAppId}/${versionName}.zip`, + deleted: false, + }) + .select('id') + .single() + expect(versionError).toBeNull() + + const { data: channel, error: channelError } = await supabase + .from('channels') + .insert({ + app_id: appFixture.publicAppId, + name: `role-binding-org-channel-${appFixture.label}-${id.slice(0, 8)}`, + version: version!.id, + owner_org: appFixture.orgId, + created_by: USER_ID, + public: false, + allow_emulator: false, + }) + .select('id') + .single() + expect(channelError).toBeNull() + channels[appFixture.label] = channel!.id + + const { error: overrideError } = await supabase.from('channel_permission_overrides').insert({ + principal_type: 'user', + principal_id: USER_ID_2, + channel_id: channel!.id, + permission_key: 'channel.promote_bundle', + is_allowed: true, + }) + expect(overrideError).toBeNull() + } + + const { data: orgABinding, error: orgABindingError } = await supabase + .from('role_bindings') + .select('id') + .eq('principal_type', 'user') + .eq('principal_id', USER_ID_2) + .eq('scope_type', 'org') + .eq('org_id', orgAId) + .single() + expect(orgABindingError).toBeNull() + + const deleteResponse = await fetch(getEndpointUrl(`/private/role_bindings/${orgABinding!.id}`), { + method: 'DELETE', + headers: authHeaders, + }) + const deleteData = await deleteResponse.json() as { success?: boolean, error?: string } + + expect(deleteResponse.status).toBe(200) + expect(deleteData.success).toBe(true) + + const { data: orgAOverrides, error: orgAOverridesError } = await supabase + .from('channel_permission_overrides') + .select('id') + .eq('principal_type', 'user') + .eq('principal_id', USER_ID_2) + .eq('channel_id', channels.A) + + expect(orgAOverridesError).toBeNull() + expect(orgAOverrides ?? []).toHaveLength(0) + + const { data: orgBOverrides, error: orgBOverridesError } = await supabase + .from('channel_permission_overrides') + .select('id') + .eq('principal_type', 'user') + .eq('principal_id', USER_ID_2) + .eq('channel_id', channels.B) + + expect(orgBOverridesError).toBeNull() + expect(orgBOverrides ?? []).toHaveLength(1) + + const [orgBPermission] = await executeSQL<{ can_promote: boolean }>( + `SELECT public.rbac_check_permission_direct( + 'channel.promote_bundle', + $1::uuid, + $2::uuid, + $3, + $4::bigint, + NULL + ) AS can_promote`, + [USER_ID_2, orgBId, publicAppBId, channels.B], + ) + expect(orgBPermission?.can_promote).toBe(true) + } + finally { + await supabase.from('channel_permission_overrides').delete().eq('principal_id', USER_ID_2).in('channel_id', [channels.A, channels.B]) + await supabase.from('role_bindings').delete().in('org_id', [orgAId, orgBId]) + await supabase.from('org_users').delete().in('org_id', [orgAId, orgBId]) + await supabase.from('channels').delete().in('owner_org', [orgAId, orgBId]) + await supabase.from('app_versions').delete().in('owner_org', [orgAId, orgBId]) + await supabase.from('apps').delete().in('id', [appAUuid, appBUuid]) + await supabase.from('orgs').delete().in('id', [orgAId, orgBId]) + } + }) }) describe.skipIf(USE_CLOUDFLARE)('[PATCH] /private/role_bindings/:binding_id', () => {