Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 31 additions & 7 deletions supabase/functions/_backend/private/create_device.ts
Original file line number Diff line number Diff line change
@@ -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',
})
Expand All @@ -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)
Expand All @@ -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) {
Expand All @@ -59,15 +63,35 @@ 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')
.select('owner_org')
.eq('app_id', safeBody.app_id)
.single()
let appOwnerOrg: string | null = null
let pgClient: ReturnType<typeof getPgClient> | 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 (appOwnerOrg.toLowerCase() !== normalizedOrgId) {
return quickError(401, 'not_authorized', 'Not authorized', { userId, appId: safeBody.app_id, orgId: normalizedOrgId })
}

await createStatsDevices(c, {
app_id: safeBody.app_id,
device_id: safeBody.device_id,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
-- 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,

Check failure on line 38 in supabase/migrations/20260430145518_enforce_check_min_rights_app_org_scope.sql

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of duplicating this literal 7 times.

See more on https://sonarcloud.io/project/issues?id=Cap-go_capgo&issues=AZ3e72gkHXSCddFQ_yMt&open=AZ3e72gkHXSCddFQ_yMt&pullRequest=1997
'app_owner_org', v_app_owner_org,
'app_id', app_id,

Check failure on line 40 in supabase/migrations/20260430145518_enforce_check_min_rights_app_org_scope.sql

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of duplicating this literal 8 times.

See more on https://sonarcloud.io/project/issues?id=Cap-go_capgo&issues=AZ3e72gkHXSCddFQ_yMs&open=AZ3e72gkHXSCddFQ_yMs&pullRequest=1997
'channel_id', channel_id,

Check failure on line 41 in supabase/migrations/20260430145518_enforce_check_min_rights_app_org_scope.sql

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of duplicating this literal 4 times.

See more on https://sonarcloud.io/project/issues?id=Cap-go_capgo&issues=AZ3e72gkHXSCddFQ_yMr&open=AZ3e72gkHXSCddFQ_yMr&pullRequest=1997
'min_right', min_right::text,

Check failure on line 42 in supabase/migrations/20260430145518_enforce_check_min_rights_app_org_scope.sql

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of duplicating this literal 4 times.

See more on https://sonarcloud.io/project/issues?id=Cap-go_capgo&issues=AZ3e72gkHXSCddFQ_yMq&open=AZ3e72gkHXSCddFQ_yMq&pullRequest=1997
'user_id', user_id

Check failure on line 43 in supabase/migrations/20260430145518_enforce_check_min_rights_app_org_scope.sql

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of duplicating this literal 4 times.

See more on https://sonarcloud.io/project/issues?id=Cap-go_capgo&issues=AZ3e72gkHXSCddFQ_yMu&open=AZ3e72gkHXSCddFQ_yMu&pullRequest=1997
));
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";
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";
61 changes: 60 additions & 1 deletion tests/private-error-cases.test.ts
Original file line number Diff line number Diff line change
@@ -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}`
Expand Down Expand Up @@ -59,6 +59,65 @@ afterAll(async () => {
})

describe('[POST] /private/create_device - Error Cases', () => {
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,
user_id: USER_ID,
channel_id: null as any,
app_id: APPNAME,
})

expect(error).toBeNull()
expect(data).toBe(false)
})

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',
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.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.toUpperCase(),
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',
Expand Down
2 changes: 1 addition & 1 deletion tests/updates.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ describe('[POST] /updates parallel tests', () => {
expect(response.status).toBe(200)

const json = await response.json<UpdateRes>()
expect(() => updateNewScheme.parse(json)).not.toThrow()
expect(() => parseSchema(updateNewScheme, json)).not.toThrow()
expect(json.version).toBe(versionName)
expect(json.error).toBeUndefined()
})
Expand Down
Loading