Harden RPC access and redact PII logs#1562
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughSingle SQL migration introduces security-hardening measures: adds three SECURITY DEFINER functions for account deletion checks, organization resolution, and user invitations, along with restrictive privilege revocation and explicit role-based access grants to service_role and authenticated users only. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06db5108bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| SELECT public.get_identity_org_appid('{read,upload,write,all}'::public.key_mode[], org_id, get_user_main_org_id_by_app_id.app_id) INTO api_user_id; | ||
| IF api_user_id IS NULL THEN | ||
| RETURN NULL; |
There was a problem hiding this comment.
When auth.uid() is NULL and there are no request headers (e.g., migrations/seed.sql or other SQL-run admin tasks), get_identity_org_appid(...) returns NULL and this function returns NULL as well. That breaks triggers like auto_owner_org_by_app_id (used by app_versions, channels, etc.), because owner_org is NOT NULL and the trigger overwrites it with this NULL value, causing inserts during bunx supabase db reset/seed or SQL maintenance to fail. Consider allowing the postgres/no-JWT context (or bypassing the trigger when owner_org is already set) so non-request inserts still work.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@supabase/migrations/20260203140000_security_hardening.sql`:
- Around line 106-186: The function public.invite_user_to_org lacks explicit
GRANT EXECUTE statements so after revoking default privileges no role can call
it; add GRANT EXECUTE ON FUNCTION public.invite_user_to_org(varchar, uuid,
public.user_min_right) TO the appropriate roles (e.g., authenticated, anon and
service_role or whichever roles your project uses to invoke invites) immediately
after the function definition so callers regain permission to execute
invite_user_to_org.
- Around line 58-101: Add explicit GRANT EXECUTE for the new function so
authenticated users can call it: after the function definition, add a statement
like GRANT EXECUTE ON FUNCTION public.get_user_main_org_id_by_app_id(text) TO
authenticated; (match the exact signature "get_user_main_org_id_by_app_id(text)"
and place this alongside the other hardened functions' GRANTs).
| CREATE OR REPLACE FUNCTION "public"."get_user_main_org_id_by_app_id"("app_id" "text") RETURNS "uuid" | ||
| LANGUAGE "plpgsql" SECURITY DEFINER | ||
| SET "search_path" TO '' | ||
| AS $$ | ||
| DECLARE | ||
| org_id uuid; | ||
| auth_uid uuid; | ||
| auth_role text; | ||
| api_user_id uuid; | ||
| BEGIN | ||
| SELECT apps.owner_org INTO org_id | ||
| FROM public.apps | ||
| WHERE ((apps.app_id)::text = (get_user_main_org_id_by_app_id.app_id)::text) | ||
| LIMIT 1; | ||
|
|
||
| IF org_id IS NULL THEN | ||
| RETURN NULL; | ||
| END IF; | ||
|
|
||
| SELECT auth.uid() INTO auth_uid; | ||
| IF auth_uid IS NOT NULL THEN | ||
| IF public.check_min_rights('read'::public.user_min_right, auth_uid, org_id, get_user_main_org_id_by_app_id.app_id, NULL::bigint) THEN | ||
| RETURN org_id; | ||
| END IF; | ||
| RETURN NULL; | ||
| END IF; | ||
|
|
||
| SELECT auth.role() INTO auth_role; | ||
| IF auth_role = 'service_role' THEN | ||
| RETURN org_id; | ||
| END IF; | ||
|
|
||
| SELECT public.get_identity_org_appid('{read,upload,write,all}'::public.key_mode[], org_id, get_user_main_org_id_by_app_id.app_id) INTO api_user_id; | ||
| IF api_user_id IS NULL THEN | ||
| RETURN NULL; | ||
| END IF; | ||
|
|
||
| IF public.check_min_rights('read'::public.user_min_right, api_user_id, org_id, get_user_main_org_id_by_app_id.app_id, NULL::bigint) THEN | ||
| RETURN org_id; | ||
| END IF; | ||
|
|
||
| RETURN NULL; | ||
| END; | ||
| $$; |
There was a problem hiding this comment.
Missing GRANT statements for get_user_main_org_id_by_app_id.
The function is created but no explicit GRANT EXECUTE statements follow it. After the default privilege revocation at lines 191-192, this function will be inaccessible to authenticated users unless explicitly granted. This is inconsistent with the other hardened functions and likely breaks intended functionality.
🔧 Proposed fix: Add GRANT statements after line 101
RETURN NULL;
END;
$$;
+
+REVOKE EXECUTE ON FUNCTION "public"."get_user_main_org_id_by_app_id"(text) FROM "anon";
+GRANT EXECUTE ON FUNCTION "public"."get_user_main_org_id_by_app_id"(text) TO "authenticated";
+GRANT EXECUTE ON FUNCTION "public"."get_user_main_org_id_by_app_id"(text) TO "service_role";🤖 Prompt for AI Agents
In `@supabase/migrations/20260203140000_security_hardening.sql` around lines 58 -
101, Add explicit GRANT EXECUTE for the new function so authenticated users can
call it: after the function definition, add a statement like GRANT EXECUTE ON
FUNCTION public.get_user_main_org_id_by_app_id(text) TO authenticated; (match
the exact signature "get_user_main_org_id_by_app_id(text)" and place this
alongside the other hardened functions' GRANTs).
| CREATE OR REPLACE FUNCTION "public"."invite_user_to_org" ( | ||
| "email" varchar, | ||
| "org_id" uuid, | ||
| "invite_type" public.user_min_right | ||
| ) RETURNS varchar LANGUAGE plpgsql SECURITY DEFINER | ||
| SET search_path = '' AS $$ | ||
| DECLARE | ||
| org record; | ||
| invited_user record; | ||
| current_record record; | ||
| current_tmp_user record; | ||
| calling_user_id uuid; | ||
| BEGIN | ||
| -- Get the calling user's ID | ||
| SELECT public.get_identity_org_allowed('{read,upload,write,all}'::public.key_mode[], invite_user_to_org.org_id) | ||
| INTO calling_user_id; | ||
|
|
||
| -- Check if org exists | ||
| SELECT * INTO org FROM public.orgs WHERE public.orgs.id=invite_user_to_org.org_id; | ||
| IF org IS NULL THEN | ||
| RETURN 'NO_ORG'; | ||
| END IF; | ||
|
|
||
| -- Check if user has at least public.rbac_right_admin() rights | ||
| IF NOT public.check_min_rights(public.rbac_right_admin()::public.user_min_right, calling_user_id, invite_user_to_org.org_id, NULL::varchar, NULL::bigint) THEN | ||
| PERFORM public.pg_log('deny: NO_RIGHTS_ADMIN', jsonb_build_object('org_id', invite_user_to_org.org_id, 'invite_type', invite_user_to_org.invite_type)); | ||
| RETURN 'NO_RIGHTS'; | ||
| END IF; | ||
|
|
||
| -- If inviting as super_admin, caller must be super_admin | ||
| IF (invite_type = public.rbac_right_super_admin()::public.user_min_right OR invite_type = public.rbac_right_invite_super_admin()::public.user_min_right) THEN | ||
| IF NOT public.check_min_rights(public.rbac_right_super_admin()::public.user_min_right, calling_user_id, invite_user_to_org.org_id, NULL::varchar, NULL::bigint) THEN | ||
| PERFORM public.pg_log('deny: NO_RIGHTS_SUPER_ADMIN', jsonb_build_object('org_id', invite_user_to_org.org_id, 'invite_type', invite_user_to_org.invite_type)); | ||
| RETURN 'NO_RIGHTS'; | ||
| END IF; | ||
| END IF; | ||
|
|
||
| -- Check if user already exists | ||
| SELECT public.users.id INTO invited_user FROM public.users WHERE public.users.email=invite_user_to_org.email; | ||
|
|
||
| IF invited_user IS NOT NULL THEN | ||
| -- User exists, check if already in org | ||
| SELECT public.org_users.id INTO current_record | ||
| FROM public.org_users | ||
| WHERE public.org_users.user_id=invited_user.id | ||
| AND public.org_users.org_id=invite_user_to_org.org_id; | ||
|
|
||
| IF current_record IS NOT NULL THEN | ||
| RETURN 'ALREADY_INVITED'; | ||
| ELSE | ||
| -- Add user to org | ||
| INSERT INTO public.org_users (user_id, org_id, user_right) | ||
| VALUES (invited_user.id, invite_user_to_org.org_id, invite_type); | ||
| RETURN 'OK'; | ||
| END IF; | ||
| ELSE | ||
| -- User doesn't exist, check tmp_users for pending invitations | ||
| SELECT * INTO current_tmp_user | ||
| FROM public.tmp_users | ||
| WHERE public.tmp_users.email=invite_user_to_org.email | ||
| AND public.tmp_users.org_id=invite_user_to_org.org_id; | ||
|
|
||
| IF current_tmp_user IS NOT NULL THEN | ||
| -- Invitation already exists | ||
| IF current_tmp_user.cancelled_at IS NOT NULL THEN | ||
| -- Invitation was cancelled, check if recent | ||
| IF current_tmp_user.cancelled_at > (CURRENT_TIMESTAMP - INTERVAL '3 hours') THEN | ||
| RETURN 'TOO_RECENT_INVITATION_CANCELATION'; | ||
| ELSE | ||
| RETURN 'NO_EMAIL'; | ||
| END IF; | ||
| ELSE | ||
| RETURN 'ALREADY_INVITED'; | ||
| END IF; | ||
| ELSE | ||
| -- No invitation exists, need to create one (handled elsewhere) | ||
| RETURN 'NO_EMAIL'; | ||
| END IF; | ||
| END IF; | ||
| END; | ||
| $$; |
There was a problem hiding this comment.
Missing GRANT statements for invite_user_to_org.
Similar to get_user_main_org_id_by_app_id, this function lacks explicit GRANT EXECUTE statements. After default privilege revocation, no role will be able to invoke this function.
🔧 Proposed fix: Add GRANT statements after line 186
END;
$$;
+
+REVOKE EXECUTE ON FUNCTION "public"."invite_user_to_org"(varchar, uuid, public.user_min_right) FROM "anon";
+GRANT EXECUTE ON FUNCTION "public"."invite_user_to_org"(varchar, uuid, public.user_min_right) TO "authenticated";
+GRANT EXECUTE ON FUNCTION "public"."invite_user_to_org"(varchar, uuid, public.user_min_right) TO "service_role";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| CREATE OR REPLACE FUNCTION "public"."invite_user_to_org" ( | |
| "email" varchar, | |
| "org_id" uuid, | |
| "invite_type" public.user_min_right | |
| ) RETURNS varchar LANGUAGE plpgsql SECURITY DEFINER | |
| SET search_path = '' AS $$ | |
| DECLARE | |
| org record; | |
| invited_user record; | |
| current_record record; | |
| current_tmp_user record; | |
| calling_user_id uuid; | |
| BEGIN | |
| -- Get the calling user's ID | |
| SELECT public.get_identity_org_allowed('{read,upload,write,all}'::public.key_mode[], invite_user_to_org.org_id) | |
| INTO calling_user_id; | |
| -- Check if org exists | |
| SELECT * INTO org FROM public.orgs WHERE public.orgs.id=invite_user_to_org.org_id; | |
| IF org IS NULL THEN | |
| RETURN 'NO_ORG'; | |
| END IF; | |
| -- Check if user has at least public.rbac_right_admin() rights | |
| IF NOT public.check_min_rights(public.rbac_right_admin()::public.user_min_right, calling_user_id, invite_user_to_org.org_id, NULL::varchar, NULL::bigint) THEN | |
| PERFORM public.pg_log('deny: NO_RIGHTS_ADMIN', jsonb_build_object('org_id', invite_user_to_org.org_id, 'invite_type', invite_user_to_org.invite_type)); | |
| RETURN 'NO_RIGHTS'; | |
| END IF; | |
| -- If inviting as super_admin, caller must be super_admin | |
| IF (invite_type = public.rbac_right_super_admin()::public.user_min_right OR invite_type = public.rbac_right_invite_super_admin()::public.user_min_right) THEN | |
| IF NOT public.check_min_rights(public.rbac_right_super_admin()::public.user_min_right, calling_user_id, invite_user_to_org.org_id, NULL::varchar, NULL::bigint) THEN | |
| PERFORM public.pg_log('deny: NO_RIGHTS_SUPER_ADMIN', jsonb_build_object('org_id', invite_user_to_org.org_id, 'invite_type', invite_user_to_org.invite_type)); | |
| RETURN 'NO_RIGHTS'; | |
| END IF; | |
| END IF; | |
| -- Check if user already exists | |
| SELECT public.users.id INTO invited_user FROM public.users WHERE public.users.email=invite_user_to_org.email; | |
| IF invited_user IS NOT NULL THEN | |
| -- User exists, check if already in org | |
| SELECT public.org_users.id INTO current_record | |
| FROM public.org_users | |
| WHERE public.org_users.user_id=invited_user.id | |
| AND public.org_users.org_id=invite_user_to_org.org_id; | |
| IF current_record IS NOT NULL THEN | |
| RETURN 'ALREADY_INVITED'; | |
| ELSE | |
| -- Add user to org | |
| INSERT INTO public.org_users (user_id, org_id, user_right) | |
| VALUES (invited_user.id, invite_user_to_org.org_id, invite_type); | |
| RETURN 'OK'; | |
| END IF; | |
| ELSE | |
| -- User doesn't exist, check tmp_users for pending invitations | |
| SELECT * INTO current_tmp_user | |
| FROM public.tmp_users | |
| WHERE public.tmp_users.email=invite_user_to_org.email | |
| AND public.tmp_users.org_id=invite_user_to_org.org_id; | |
| IF current_tmp_user IS NOT NULL THEN | |
| -- Invitation already exists | |
| IF current_tmp_user.cancelled_at IS NOT NULL THEN | |
| -- Invitation was cancelled, check if recent | |
| IF current_tmp_user.cancelled_at > (CURRENT_TIMESTAMP - INTERVAL '3 hours') THEN | |
| RETURN 'TOO_RECENT_INVITATION_CANCELATION'; | |
| ELSE | |
| RETURN 'NO_EMAIL'; | |
| END IF; | |
| ELSE | |
| RETURN 'ALREADY_INVITED'; | |
| END IF; | |
| ELSE | |
| -- No invitation exists, need to create one (handled elsewhere) | |
| RETURN 'NO_EMAIL'; | |
| END IF; | |
| END IF; | |
| END; | |
| $$; | |
| CREATE OR REPLACE FUNCTION "public"."invite_user_to_org" ( | |
| "email" varchar, | |
| "org_id" uuid, | |
| "invite_type" public.user_min_right | |
| ) RETURNS varchar LANGUAGE plpgsql SECURITY DEFINER | |
| SET search_path = '' AS $$ | |
| DECLARE | |
| org record; | |
| invited_user record; | |
| current_record record; | |
| current_tmp_user record; | |
| calling_user_id uuid; | |
| BEGIN | |
| -- Get the calling user's ID | |
| SELECT public.get_identity_org_allowed('{read,upload,write,all}'::public.key_mode[], invite_user_to_org.org_id) | |
| INTO calling_user_id; | |
| -- Check if org exists | |
| SELECT * INTO org FROM public.orgs WHERE public.orgs.id=invite_user_to_org.org_id; | |
| IF org IS NULL THEN | |
| RETURN 'NO_ORG'; | |
| END IF; | |
| -- Check if user has at least public.rbac_right_admin() rights | |
| IF NOT public.check_min_rights(public.rbac_right_admin()::public.user_min_right, calling_user_id, invite_user_to_org.org_id, NULL::varchar, NULL::bigint) THEN | |
| PERFORM public.pg_log('deny: NO_RIGHTS_ADMIN', jsonb_build_object('org_id', invite_user_to_org.org_id, 'invite_type', invite_user_to_org.invite_type)); | |
| RETURN 'NO_RIGHTS'; | |
| END IF; | |
| -- If inviting as super_admin, caller must be super_admin | |
| IF (invite_type = public.rbac_right_super_admin()::public.user_min_right OR invite_type = public.rbac_right_invite_super_admin()::public.user_min_right) THEN | |
| IF NOT public.check_min_rights(public.rbac_right_super_admin()::public.user_min_right, calling_user_id, invite_user_to_org.org_id, NULL::varchar, NULL::bigint) THEN | |
| PERFORM public.pg_log('deny: NO_RIGHTS_SUPER_ADMIN', jsonb_build_object('org_id', invite_user_to_org.org_id, 'invite_type', invite_user_to_org.invite_type)); | |
| RETURN 'NO_RIGHTS'; | |
| END IF; | |
| END IF; | |
| -- Check if user already exists | |
| SELECT public.users.id INTO invited_user FROM public.users WHERE public.users.email=invite_user_to_org.email; | |
| IF invited_user IS NOT NULL THEN | |
| -- User exists, check if already in org | |
| SELECT public.org_users.id INTO current_record | |
| FROM public.org_users | |
| WHERE public.org_users.user_id=invited_user.id | |
| AND public.org_users.org_id=invite_user_to_org.org_id; | |
| IF current_record IS NOT NULL THEN | |
| RETURN 'ALREADY_INVITED'; | |
| ELSE | |
| -- Add user to org | |
| INSERT INTO public.org_users (user_id, org_id, user_right) | |
| VALUES (invited_user.id, invite_user_to_org.org_id, invite_type); | |
| RETURN 'OK'; | |
| END IF; | |
| ELSE | |
| -- User doesn't exist, check tmp_users for pending invitations | |
| SELECT * INTO current_tmp_user | |
| FROM public.tmp_users | |
| WHERE public.tmp_users.email=invite_user_to_org.email | |
| AND public.tmp_users.org_id=invite_user_to_org.org_id; | |
| IF current_tmp_user IS NOT NULL THEN | |
| -- Invitation already exists | |
| IF current_tmp_user.cancelled_at IS NOT NULL THEN | |
| -- Invitation was cancelled, check if recent | |
| IF current_tmp_user.cancelled_at > (CURRENT_TIMESTAMP - INTERVAL '3 hours') THEN | |
| RETURN 'TOO_RECENT_INVITATION_CANCELATION'; | |
| ELSE | |
| RETURN 'NO_EMAIL'; | |
| END IF; | |
| ELSE | |
| RETURN 'ALREADY_INVITED'; | |
| END IF; | |
| ELSE | |
| -- No invitation exists, need to create one (handled elsewhere) | |
| RETURN 'NO_EMAIL'; | |
| END IF; | |
| END IF; | |
| END; | |
| $$; | |
| REVOKE EXECUTE ON FUNCTION "public"."invite_user_to_org"(varchar, uuid, public.user_min_right) FROM "anon"; | |
| GRANT EXECUTE ON FUNCTION "public"."invite_user_to_org"(varchar, uuid, public.user_min_right) TO "authenticated"; | |
| GRANT EXECUTE ON FUNCTION "public"."invite_user_to_org"(varchar, uuid, public.user_min_right) TO "service_role"; |
🤖 Prompt for AI Agents
In `@supabase/migrations/20260203140000_security_hardening.sql` around lines 106 -
186, The function public.invite_user_to_org lacks explicit GRANT EXECUTE
statements so after revoking default privileges no role can call it; add GRANT
EXECUTE ON FUNCTION public.invite_user_to_org(varchar, uuid,
public.user_min_right) TO the appropriate roles (e.g., authenticated, anon and
service_role or whichever roles your project uses to invoke invites) immediately
after the function definition so callers regain permission to execute
invite_user_to_org.
There was a problem hiding this comment.
Pull request overview
This migration tightens database-level security around several RPC functions and logging behaviors, focusing on restricting sensitive function execution and reducing log exposure of PII.
Changes:
- Revokes
EXECUTEonpublic.find_apikey_by_value(text)fromanon/authenticated, limiting it toservice_role. - Hardens
public.get_account_removal_date(uuid)to allow only the owning user orservice_roleto access the removal date, and adjusts grants to disallowanon. - Reimplements
public.get_user_main_org_id_by_app_id(text)with auth/rights checks to mitigate org-id enumeration, and updatespublic.invite_user_to_orgto remove PII (email,calling_user) from failure logs while leaving behavior otherwise unchanged. - Changes default privileges so new
publicfunctions owned bypostgresno longer auto-grantEXECUTEtoanonorauthenticated.
| SELECT auth.uid() INTO auth_uid; | ||
| IF auth_uid IS NOT NULL THEN | ||
| IF public.check_min_rights('read'::public.user_min_right, auth_uid, org_id, get_user_main_org_id_by_app_id.app_id, NULL::bigint) THEN | ||
| RETURN org_id; | ||
| END IF; | ||
| RETURN NULL; | ||
| END IF; | ||
|
|
||
| SELECT auth.role() INTO auth_role; | ||
| IF auth_role = 'service_role' THEN | ||
| RETURN org_id; | ||
| END IF; | ||
|
|
||
| SELECT public.get_identity_org_appid('{read,upload,write,all}'::public.key_mode[], org_id, get_user_main_org_id_by_app_id.app_id) INTO api_user_id; | ||
| IF api_user_id IS NULL THEN | ||
| RETURN NULL; | ||
| END IF; | ||
|
|
||
| IF public.check_min_rights('read'::public.user_min_right, api_user_id, org_id, get_user_main_org_id_by_app_id.app_id, NULL::bigint) THEN | ||
| RETURN org_id; | ||
| END IF; | ||
|
|
||
| RETURN NULL; |
There was a problem hiding this comment.
In the new implementation of get_user_main_org_id_by_app_id, the api_user_id path (the call to public.get_identity_org_appid and subsequent check_min_rights check) is effectively unreachable because all control-flow paths return before reaching it: authenticated users return inside the auth_uid IS NOT NULL block, and service_role returns inside the auth_role = 'service_role' block. As written, this dead code will never execute and API key–based authorization logic here cannot take effect. Consider either removing this unused block or restructuring the earlier authentication/authorization flow so that the API-key branch can actually run if it is intended to be supported.
| auth_uid uuid; | ||
| auth_role text; | ||
| api_user_id uuid; | ||
| BEGIN | ||
| SELECT apps.owner_org INTO org_id | ||
| FROM public.apps | ||
| WHERE ((apps.app_id)::text = (get_user_main_org_id_by_app_id.app_id)::text) | ||
| LIMIT 1; | ||
|
|
||
| IF org_id IS NULL THEN | ||
| RETURN NULL; | ||
| END IF; | ||
|
|
||
| SELECT auth.uid() INTO auth_uid; | ||
| IF auth_uid IS NOT NULL THEN | ||
| IF public.check_min_rights('read'::public.user_min_right, auth_uid, org_id, get_user_main_org_id_by_app_id.app_id, NULL::bigint) THEN | ||
| RETURN org_id; | ||
| END IF; | ||
| RETURN NULL; | ||
| END IF; | ||
|
|
||
| SELECT auth.role() INTO auth_role; | ||
| IF auth_role = 'service_role' THEN | ||
| RETURN org_id; | ||
| END IF; | ||
|
|
||
| SELECT public.get_identity_org_appid('{read,upload,write,all}'::public.key_mode[], org_id, get_user_main_org_id_by_app_id.app_id) INTO api_user_id; | ||
| IF api_user_id IS NULL THEN | ||
| RETURN NULL; | ||
| END IF; | ||
|
|
||
| IF public.check_min_rights('read'::public.user_min_right, api_user_id, org_id, get_user_main_org_id_by_app_id.app_id, NULL::bigint) THEN | ||
| RETURN org_id; | ||
| END IF; | ||
|
|
||
| RETURN NULL; |
There was a problem hiding this comment.
The updated get_user_main_org_id_by_app_id now returns NULL for unauthorized callers, which changes the behavior of existing RLS policies that pass its result into public.get_identity_org_appid and public.check_min_rights. When those policies call public.get_identity_org_appid with a NULL org_id, the limited_to_orgs restriction on API keys is no longer enforced (because the comparison org_id = ANY(limited_to_orgs) becomes NULL), while public.check_min_rights still re-derives the real owner_org from public.apps, allowing an API key that is scoped to one org but whose user has broader RBAC rights to access devices in other orgs. To fix this, keep org_id non-NULL in all code paths used by RLS (for example by separating the “org lookup” from the authorization decision), or update public.get_identity_org_appid to treat NULL org_id as a hard deny for keys with limited_to_orgs, so that per-org API key scoping cannot be bypassed.
| auth_uid uuid; | |
| auth_role text; | |
| api_user_id uuid; | |
| BEGIN | |
| SELECT apps.owner_org INTO org_id | |
| FROM public.apps | |
| WHERE ((apps.app_id)::text = (get_user_main_org_id_by_app_id.app_id)::text) | |
| LIMIT 1; | |
| IF org_id IS NULL THEN | |
| RETURN NULL; | |
| END IF; | |
| SELECT auth.uid() INTO auth_uid; | |
| IF auth_uid IS NOT NULL THEN | |
| IF public.check_min_rights('read'::public.user_min_right, auth_uid, org_id, get_user_main_org_id_by_app_id.app_id, NULL::bigint) THEN | |
| RETURN org_id; | |
| END IF; | |
| RETURN NULL; | |
| END IF; | |
| SELECT auth.role() INTO auth_role; | |
| IF auth_role = 'service_role' THEN | |
| RETURN org_id; | |
| END IF; | |
| SELECT public.get_identity_org_appid('{read,upload,write,all}'::public.key_mode[], org_id, get_user_main_org_id_by_app_id.app_id) INTO api_user_id; | |
| IF api_user_id IS NULL THEN | |
| RETURN NULL; | |
| END IF; | |
| IF public.check_min_rights('read'::public.user_min_right, api_user_id, org_id, get_user_main_org_id_by_app_id.app_id, NULL::bigint) THEN | |
| RETURN org_id; | |
| END IF; | |
| RETURN NULL; | |
| BEGIN | |
| SELECT apps.owner_org INTO org_id | |
| FROM public.apps | |
| WHERE ((apps.app_id)::text = (get_user_main_org_id_by_app_id.app_id)::text) | |
| LIMIT 1; | |
| -- Pure lookup: return the main org for this app (may be NULL if app_id is unknown) | |
| RETURN org_id; |
|
Summary (AI generated)
Test plan (AI generated)
Screenshots (AI generated)
Checklist (AI generated)
.
accordingly.
my tests
Summary by CodeRabbit