From 3fa0122750bff5430e30f0798e1ee03f54e56944 Mon Sep 17 00:00:00 2001 From: Martin Donadieu Date: Thu, 30 Apr 2026 17:01:56 +0200 Subject: [PATCH 1/4] fix(api): enforce create_device app org scope --- .../_backend/private/create_device.ts | 6 +- ...enforce_check_min_rights_app_org_scope.sql | 139 ++++++++++++++++++ tests/private-error-cases.test.ts | 60 +++++++- 3 files changed, 203 insertions(+), 2 deletions(-) create mode 100644 supabase/migrations/20260430145518_enforce_check_min_rights_app_org_scope.sql diff --git a/supabase/functions/_backend/private/create_device.ts b/supabase/functions/_backend/private/create_device.ts index 825b8b225d..02bcfbf42d 100644 --- a/supabase/functions/_backend/private/create_device.ts +++ b/supabase/functions/_backend/private/create_device.ts @@ -59,7 +59,7 @@ app.post('/', middlewareV2(['all', 'write']), async (c) => { return quickError(401, 'not_authorized', 'Not authorized', { userId, appId: safeBody.app_id }) } - const { error: appError } = await supabase.from('apps') + const { data: app, error: appError } = await supabase.from('apps') .select('owner_org') .eq('app_id', safeBody.app_id) .single() @@ -68,6 +68,10 @@ app.post('/', middlewareV2(['all', 'write']), async (c) => { return quickError(404, 'app_not_found', 'App not found', { app_id: safeBody.app_id }) } + if (app.owner_org !== safeBody.org_id) { + return quickError(401, 'not_authorized', 'Not authorized', { userId, appId: safeBody.app_id, orgId: safeBody.org_id }) + } + await createStatsDevices(c, { app_id: safeBody.app_id, device_id: safeBody.device_id, diff --git a/supabase/migrations/20260430145518_enforce_check_min_rights_app_org_scope.sql b/supabase/migrations/20260430145518_enforce_check_min_rights_app_org_scope.sql new file mode 100644 index 0000000000..1a03f7ed72 --- /dev/null +++ b/supabase/migrations/20260430145518_enforce_check_min_rights_app_org_scope.sql @@ -0,0 +1,139 @@ +-- Enforce that app-scoped permission checks cannot be authorized through a foreign org_id. + +CREATE OR REPLACE FUNCTION "public"."check_min_rights"( + "min_right" "public"."user_min_right", + "user_id" "uuid", + "org_id" "uuid", + "app_id" character varying, + "channel_id" bigint +) RETURNS boolean + LANGUAGE "plpgsql" SECURITY DEFINER + SET "search_path" TO '' + AS $$ +DECLARE + v_allowed boolean := false; + v_perm text; + v_scope text; + v_apikey text; + v_apikey_principal uuid; + v_use_rbac boolean; + v_effective_org_id uuid := org_id; + v_app_owner_org uuid; + v_org_enforcing_2fa boolean; + v_password_policy_ok boolean; + api_key record; +BEGIN + -- Existing apps are always authorized in the app owner's org scope. + -- Keep nonexistent apps on the caller org so API handlers can still return their + -- own not-found errors after a valid org-level check. + IF app_id IS NOT NULL THEN + SELECT owner_org INTO v_app_owner_org + FROM public.apps + WHERE public.apps.app_id = check_min_rights.app_id + LIMIT 1; + + IF v_app_owner_org IS NOT NULL THEN + IF v_effective_org_id IS NOT NULL AND v_effective_org_id IS DISTINCT FROM v_app_owner_org THEN + PERFORM public.pg_log('deny: CHECK_MIN_RIGHTS_APP_ORG_MISMATCH', jsonb_build_object( + 'org_id', v_effective_org_id, + 'app_owner_org', v_app_owner_org, + 'app_id', app_id, + 'channel_id', channel_id, + 'min_right', min_right::text, + 'user_id', user_id + )); + RETURN false; + END IF; + + v_effective_org_id := v_app_owner_org; + END IF; + END IF; + + -- Derive org from channel when not provided to honor org-level flag and scoping. + IF v_effective_org_id IS NULL AND channel_id IS NOT NULL THEN + SELECT owner_org INTO v_effective_org_id FROM public.channels WHERE public.channels.id = channel_id LIMIT 1; + END IF; + + -- Enforce 2FA if the org requires it. + IF v_effective_org_id IS NOT NULL THEN + SELECT enforcing_2fa INTO v_org_enforcing_2fa FROM public.orgs WHERE id = v_effective_org_id; + IF v_org_enforcing_2fa = true AND (user_id IS NULL OR NOT public.has_2fa_enabled(user_id)) THEN + PERFORM public.pg_log('deny: CHECK_MIN_RIGHTS_2FA_ENFORCEMENT', jsonb_build_object( + 'org_id', COALESCE(org_id, v_effective_org_id), + 'app_id', app_id, + 'channel_id', channel_id, + 'min_right', min_right::text, + 'user_id', user_id + )); + RETURN false; + END IF; + END IF; + + -- Enforce password policy if enabled for the org. + IF v_effective_org_id IS NOT NULL THEN + v_password_policy_ok := public.user_meets_password_policy(user_id, v_effective_org_id); + IF v_password_policy_ok = false THEN + PERFORM public.pg_log('deny: CHECK_MIN_RIGHTS_PASSWORD_POLICY_ENFORCEMENT', jsonb_build_object( + 'org_id', COALESCE(org_id, v_effective_org_id), + 'app_id', app_id, + 'channel_id', channel_id, + 'min_right', min_right::text, + 'user_id', user_id + )); + RETURN false; + END IF; + END IF; + + v_use_rbac := public.rbac_is_enabled_for_org(v_effective_org_id); + IF NOT v_use_rbac THEN + RETURN public.check_min_rights_legacy(min_right, user_id, COALESCE(org_id, v_effective_org_id), app_id, channel_id); + END IF; + + IF channel_id IS NOT NULL THEN + v_scope := public.rbac_scope_channel(); + ELSIF app_id IS NOT NULL THEN + v_scope := public.rbac_scope_app(); + ELSE + v_scope := public.rbac_scope_org(); + END IF; + + v_perm := public.rbac_permission_for_legacy(min_right, v_scope); + + IF user_id IS NOT NULL THEN + v_allowed := public.rbac_has_permission(public.rbac_principal_user(), user_id, v_perm, v_effective_org_id, app_id, channel_id); + END IF; + + -- Also consider apikey principal when RBAC is enabled (API keys can hold roles directly). + IF NOT v_allowed THEN + SELECT public.get_apikey_header() INTO v_apikey; + IF v_apikey IS NOT NULL THEN + -- Enforce org/app scoping before using the apikey RBAC principal. + SELECT * FROM public.find_apikey_by_value(v_apikey) INTO api_key; + IF api_key.id IS NOT NULL THEN + IF public.is_apikey_expired(api_key.expires_at) THEN + PERFORM public.pg_log('deny: API_KEY_EXPIRED', jsonb_build_object('key_id', api_key.id, 'org_id', v_effective_org_id, 'app_id', app_id)); + ELSIF v_effective_org_id IS NULL THEN + PERFORM public.pg_log('deny: CHECK_MIN_RIGHTS_APIKEY_NO_ORG', jsonb_build_object('app_id', app_id)); + ELSIF COALESCE(array_length(api_key.limited_to_orgs, 1), 0) > 0 AND NOT (v_effective_org_id = ANY(api_key.limited_to_orgs)) THEN + PERFORM public.pg_log('deny: CHECK_MIN_RIGHTS_APIKEY_ORG_RESTRICT', jsonb_build_object('org_id', v_effective_org_id, 'app_id', app_id)); + ELSIF app_id IS NOT NULL AND api_key.limited_to_apps IS DISTINCT FROM '{}' AND NOT (app_id = ANY(api_key.limited_to_apps)) THEN + PERFORM public.pg_log('deny: CHECK_MIN_RIGHTS_APIKEY_APP_RESTRICT', jsonb_build_object('org_id', v_effective_org_id, 'app_id', app_id)); + ELSE + v_apikey_principal := api_key.rbac_id; + IF v_apikey_principal IS NOT NULL THEN + v_allowed := public.rbac_has_permission(public.rbac_principal_apikey(), v_apikey_principal, v_perm, v_effective_org_id, app_id, channel_id); + END IF; + END IF; + END IF; + END IF; + END IF; + + IF NOT v_allowed THEN + PERFORM public.pg_log('deny: CHECK_MIN_RIGHTS_RBAC', jsonb_build_object('org_id', COALESCE(org_id, v_effective_org_id), 'app_id', app_id, 'channel_id', channel_id, 'min_right', min_right::text, 'user_id', user_id, 'scope', v_scope, 'perm', v_perm)); + END IF; + + RETURN v_allowed; +END; +$$; + +ALTER FUNCTION "public"."check_min_rights"("min_right" "public"."user_min_right", "user_id" "uuid", "org_id" "uuid", "app_id" character varying, "channel_id" bigint) OWNER TO "postgres"; diff --git a/tests/private-error-cases.test.ts b/tests/private-error-cases.test.ts index e96c690d45..b41c6fe034 100644 --- a/tests/private-error-cases.test.ts +++ b/tests/private-error-cases.test.ts @@ -1,6 +1,6 @@ import { randomUUID } from 'node:crypto' import { afterAll, beforeAll, describe, expect, it } from 'vitest' -import { APIKEY_STATS, getEndpointUrl, getSupabaseClient, headers, NON_OWNER_ORG_ID, resetAndSeedAppData, resetAppData, USER_ID } from './test-utils.ts' +import { APIKEY_STATS, getEndpointUrl, getSupabaseClient, headers, NON_OWNER_ORG_ID, ORG_ID, resetAndSeedAppData, resetAppData, USER_ID } from './test-utils.ts' const id = randomUUID() const APPNAME = `com.private.error.${id}` @@ -59,6 +59,64 @@ afterAll(async () => { }) describe('[POST] /private/create_device - Error Cases', () => { + it('should reject app/org scope mismatch in permission checks', async () => { + const { data, error } = await getSupabaseClient().rpc('check_min_rights', { + min_right: 'write', + org_id: testOrgId, + user_id: USER_ID, + channel_id: null as any, + app_id: APPNAME, + }) + + expect(error).toBeNull() + expect(data).toBe(false) + }) + + it('should return 401 when org_id does not own the app', async () => { + const deviceId = randomUUID() + const response = await fetch(getEndpointUrl('/private/create_device'), { + method: 'POST', + headers, + body: JSON.stringify({ + app_id: APPNAME, + org_id: testOrgId, + device_id: deviceId, + platform: 'android', + version_name: '1.0.0', + }), + }) + + expect(response.status).toBe(401) + const data = await response.json() as { error: string } + expect(data.error).toBe('not_authorized') + + const { data: device, error: deviceError } = await getSupabaseClient().from('devices') + .select('device_id') + .eq('app_id', APPNAME) + .eq('device_id', deviceId) + .maybeSingle() + expect(deviceError).toBeNull() + expect(device).toBeNull() + }) + + it('should allow device creation when org_id owns the app', async () => { + const response = await fetch(getEndpointUrl('/private/create_device'), { + method: 'POST', + headers, + body: JSON.stringify({ + app_id: APPNAME, + org_id: ORG_ID, + device_id: randomUUID(), + platform: 'android', + version_name: '1.0.0', + }), + }) + + expect(response.status).toBe(200) + const data = await response.json() as { status: string } + expect(data.status).toBe('ok') + }) + it('should return 401 when not authorized', async () => { const response = await fetch(getEndpointUrl('/private/create_device'), { method: 'POST', From 9424b8f90bae6a3de4f833da823095a97fe216b9 Mon Sep 17 00:00:00 2001 From: Martin Donadieu Date: Thu, 30 Apr 2026 17:23:39 +0200 Subject: [PATCH 2/4] fix(api): address create_device review feedback --- ...0430145518_enforce_check_min_rights_app_org_scope.sql | 4 ++++ tests/private-error-cases.test.ts | 9 +++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/supabase/migrations/20260430145518_enforce_check_min_rights_app_org_scope.sql b/supabase/migrations/20260430145518_enforce_check_min_rights_app_org_scope.sql index 1a03f7ed72..60c2b8df1d 100644 --- a/supabase/migrations/20260430145518_enforce_check_min_rights_app_org_scope.sql +++ b/supabase/migrations/20260430145518_enforce_check_min_rights_app_org_scope.sql @@ -137,3 +137,7 @@ END; $$; ALTER FUNCTION "public"."check_min_rights"("min_right" "public"."user_min_right", "user_id" "uuid", "org_id" "uuid", "app_id" character varying, "channel_id" bigint) OWNER TO "postgres"; +REVOKE ALL ON FUNCTION "public"."check_min_rights"("min_right" "public"."user_min_right", "user_id" "uuid", "org_id" "uuid", "app_id" character varying, "channel_id" bigint) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION "public"."check_min_rights"("min_right" "public"."user_min_right", "user_id" "uuid", "org_id" "uuid", "app_id" character varying, "channel_id" bigint) TO "anon"; +GRANT EXECUTE ON FUNCTION "public"."check_min_rights"("min_right" "public"."user_min_right", "user_id" "uuid", "org_id" "uuid", "app_id" character varying, "channel_id" bigint) TO "authenticated"; +GRANT EXECUTE ON FUNCTION "public"."check_min_rights"("min_right" "public"."user_min_right", "user_id" "uuid", "org_id" "uuid", "app_id" character varying, "channel_id" bigint) TO "service_role"; diff --git a/tests/private-error-cases.test.ts b/tests/private-error-cases.test.ts index b41c6fe034..7e92edb7b6 100644 --- a/tests/private-error-cases.test.ts +++ b/tests/private-error-cases.test.ts @@ -59,7 +59,7 @@ afterAll(async () => { }) describe('[POST] /private/create_device - Error Cases', () => { - it('should reject app/org scope mismatch in permission checks', async () => { + it.concurrent('should reject app/org scope mismatch in permission checks', async () => { const { data, error } = await getSupabaseClient().rpc('check_min_rights', { min_right: 'write', org_id: testOrgId, @@ -72,7 +72,7 @@ describe('[POST] /private/create_device - Error Cases', () => { expect(data).toBe(false) }) - it('should return 401 when org_id does not own the app', async () => { + it.concurrent('should return 401 when org_id does not own the app', async () => { const deviceId = randomUUID() const response = await fetch(getEndpointUrl('/private/create_device'), { method: 'POST', @@ -90,7 +90,8 @@ describe('[POST] /private/create_device - Error Cases', () => { const data = await response.json() as { error: string } expect(data.error).toBe('not_authorized') - const { data: device, error: deviceError } = await getSupabaseClient().from('devices') + const { data: device, error: deviceError } = await getSupabaseClient() + .from('devices') .select('device_id') .eq('app_id', APPNAME) .eq('device_id', deviceId) @@ -99,7 +100,7 @@ describe('[POST] /private/create_device - Error Cases', () => { expect(device).toBeNull() }) - it('should allow device creation when org_id owns the app', async () => { + it.concurrent('should allow device creation when org_id owns the app', async () => { const response = await fetch(getEndpointUrl('/private/create_device'), { method: 'POST', headers, From a6425bb771687435903388a15c63d3322397c766 Mon Sep 17 00:00:00 2001 From: Martin Donadieu Date: Thu, 30 Apr 2026 17:39:36 +0200 Subject: [PATCH 3/4] fix(api): address create_device review scope --- .../_backend/private/create_device.ts | 38 ++++++++++++++----- tests/private-error-cases.test.ts | 4 +- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/supabase/functions/_backend/private/create_device.ts b/supabase/functions/_backend/private/create_device.ts index 02bcfbf42d..8ad40ea31c 100644 --- a/supabase/functions/_backend/private/create_device.ts +++ b/supabase/functions/_backend/private/create_device.ts @@ -1,16 +1,19 @@ import type { MiddlewareKeyVariables } from '../utils/hono.ts' import { type } from 'arktype' +import { eq } from 'drizzle-orm' import { Hono } from 'hono/tiny' import { safeParseSchema } from '../utils/ark_validation.ts' import { BRES, parseBody, quickError, simpleError, useCors } from '../utils/hono.ts' import { middlewareV2 } from '../utils/hono_middleware.ts' +import { closeClient, getDrizzleClient, getPgClient } from '../utils/pg.ts' +import { schema } from '../utils/postgres_schema.ts' import { createStatsDevices } from '../utils/stats.ts' import { supabaseWithAuth } from '../utils/supabase.ts' const bodySchema = type({ device_id: 'string.uuid', app_id: 'string', - org_id: 'string', + org_id: 'string.uuid', platform: '"ios" | "android"', version_name: 'string', }) @@ -37,6 +40,7 @@ app.post('/', middlewareV2(['all', 'write']), async (c) => { } const safeBody = parsedBodyResult.data + const normalizedOrgId = safeBody.org_id.toLowerCase() // Use authenticated client for data queries - RLS will enforce access const supabase = supabaseWithAuth(c, auth) @@ -48,7 +52,7 @@ app.post('/', middlewareV2(['all', 'write']), async (c) => { user_id: userId, channel_id: null as any, app_id: safeBody.app_id, - org_id: safeBody.org_id, + org_id: normalizedOrgId, }) if (userRight.error) { @@ -59,17 +63,33 @@ app.post('/', middlewareV2(['all', 'write']), async (c) => { return quickError(401, 'not_authorized', 'Not authorized', { userId, appId: safeBody.app_id }) } - const { data: app, error: appError } = await supabase.from('apps') - .select('owner_org') - .eq('app_id', safeBody.app_id) - .single() + let appOwnerOrg: string | null = null + let pgClient: ReturnType | undefined + try { + pgClient = getPgClient(c) + const drizzleClient = getDrizzleClient(pgClient) + const appResult = await drizzleClient + .select({ ownerOrg: schema.apps.owner_org }) + .from(schema.apps) + .where(eq(schema.apps.app_id, safeBody.app_id)) + .limit(1) + appOwnerOrg = appResult[0]?.ownerOrg ?? null + } + catch (error) { + throw simpleError('internal_db_error', 'Cannot get app', { error, appId: safeBody.app_id }) + } + finally { + if (pgClient) { + closeClient(c, pgClient) + } + } - if (appError) { + if (!appOwnerOrg) { return quickError(404, 'app_not_found', 'App not found', { app_id: safeBody.app_id }) } - if (app.owner_org !== safeBody.org_id) { - return quickError(401, 'not_authorized', 'Not authorized', { userId, appId: safeBody.app_id, orgId: safeBody.org_id }) + if (appOwnerOrg.toLowerCase() !== normalizedOrgId) { + return quickError(401, 'not_authorized', 'Not authorized', { userId, appId: safeBody.app_id, orgId: normalizedOrgId }) } await createStatsDevices(c, { diff --git a/tests/private-error-cases.test.ts b/tests/private-error-cases.test.ts index 7e92edb7b6..32b7183dbb 100644 --- a/tests/private-error-cases.test.ts +++ b/tests/private-error-cases.test.ts @@ -100,13 +100,13 @@ describe('[POST] /private/create_device - Error Cases', () => { expect(device).toBeNull() }) - it.concurrent('should allow device creation when org_id owns the app', async () => { + it.concurrent('should allow device creation when normalized org_id owns the app', async () => { const response = await fetch(getEndpointUrl('/private/create_device'), { method: 'POST', headers, body: JSON.stringify({ app_id: APPNAME, - org_id: ORG_ID, + org_id: ORG_ID.toUpperCase(), device_id: randomUUID(), platform: 'android', version_name: '1.0.0', From a58c0e7a8fb5fddbc2d93c62996bd7ac9c6a39f7 Mon Sep 17 00:00:00 2001 From: Martin Donadieu Date: Fri, 1 May 2026 14:05:24 +0200 Subject: [PATCH 4/4] fix(test): use arktype validation helper --- tests/updates.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/updates.test.ts b/tests/updates.test.ts index 3f63d6d7be..b3d1916cf4 100644 --- a/tests/updates.test.ts +++ b/tests/updates.test.ts @@ -632,7 +632,7 @@ describe('[POST] /updates parallel tests', () => { expect(response.status).toBe(200) const json = await response.json() - expect(() => updateNewScheme.parse(json)).not.toThrow() + expect(() => parseSchema(updateNewScheme, json)).not.toThrow() expect(json.version).toBe(versionName) expect(json.error).toBeUndefined() })