-
-
Notifications
You must be signed in to change notification settings - Fork 128
fix(api): RBAC follow-ups for PR #2403 #2548
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: codex/rbac-apikey-management-hardening
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,60 @@ DROP FUNCTION IF EXISTS public.rbac_rollback_org(uuid); | |
| ALTER TABLE public.orgs | ||
| DROP COLUMN IF EXISTS use_new_rbac; | ||
|
|
||
| CREATE OR REPLACE FUNCTION public.rbac_perm_org_manage_apikeys() RETURNS text | ||
| LANGUAGE sql | ||
| IMMUTABLE | ||
| PARALLEL SAFE | ||
| SET search_path = '' | ||
| AS $$ SELECT 'org.manage_apikeys'::text $$; | ||
|
|
||
| ALTER FUNCTION public.rbac_perm_org_manage_apikeys() OWNER TO postgres; | ||
|
|
||
| INSERT INTO public.permissions (key, scope_type, description) | ||
| VALUES ( | ||
| public.rbac_perm_org_manage_apikeys(), | ||
| public.rbac_scope_org(), | ||
| 'Create, list, update metadata, rotate, and delete API keys for the org without assigning user roles' | ||
| ) | ||
| ON CONFLICT (key) DO NOTHING; | ||
|
|
||
| CREATE OR REPLACE FUNCTION public.rbac_role_apikey_manager() RETURNS text | ||
| LANGUAGE sql | ||
| IMMUTABLE | ||
| PARALLEL SAFE | ||
| SET search_path = '' | ||
| AS $$ SELECT 'apikey_manager'::text $$; | ||
|
|
||
| ALTER FUNCTION public.rbac_role_apikey_manager() OWNER TO postgres; | ||
|
|
||
| INSERT INTO public.roles (name, scope_type, description, priority_rank, is_assignable, created_by) | ||
| VALUES ( | ||
| public.rbac_role_apikey_manager(), | ||
| public.rbac_scope_org(), | ||
| 'Manage API keys for CI/CD without org role assignment rights', | ||
| 78, | ||
| true, | ||
| NULL | ||
| ) | ||
| ON CONFLICT (name) DO UPDATE | ||
| SET | ||
| scope_type = EXCLUDED.scope_type, | ||
| description = EXCLUDED.description, | ||
| priority_rank = EXCLUDED.priority_rank, | ||
| is_assignable = EXCLUDED.is_assignable; | ||
|
|
||
| INSERT INTO public.role_permissions (role_id, permission_id) | ||
| SELECT roles.id, permissions.id | ||
| FROM public.roles | ||
| INNER JOIN public.permissions | ||
| ON permissions.key = public.rbac_perm_org_manage_apikeys() | ||
| WHERE roles.name IN ( | ||
| public.rbac_role_apikey_manager(), | ||
| public.rbac_role_org_admin(), | ||
| public.rbac_role_org_super_admin() | ||
|
Comment on lines
+60
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Callers that only have the new Useful? React with 👍 / 👎. |
||
| ) | ||
| ON CONFLICT DO NOTHING; | ||
|
|
||
| -- Direct channel table updates are intentionally admin/channel-admin only. | ||
| -- App developers can upload/promote bundles, but must not mutate channel settings | ||
| -- through the anon API-key RLS path. | ||
|
|
@@ -36,6 +90,18 @@ INNER JOIN public.permissions | |
| WHERE roles.name = public.rbac_role_app_reader() | ||
| ON CONFLICT DO NOTHING; | ||
|
|
||
| -- Upload-only principals can promote bundles to channels without channel settings writes. | ||
| INSERT INTO public.role_permissions (role_id, permission_id) | ||
| SELECT roles.id, permissions.id | ||
| FROM public.roles | ||
| INNER JOIN public.permissions | ||
| ON permissions.key IN ( | ||
| public.rbac_perm_channel_read(), | ||
| public.rbac_perm_channel_promote_bundle() | ||
| ) | ||
| WHERE roles.name = public.rbac_role_app_uploader() | ||
| ON CONFLICT DO NOTHING; | ||
|
|
||
| DROP POLICY IF EXISTS "Allow update for auth (admin+)" ON public.orgs; | ||
| DROP POLICY IF EXISTS "Allow org settings update via RBAC" ON public.orgs; | ||
| CREATE POLICY "Allow org settings update via RBAC" | ||
|
|
@@ -146,16 +212,6 @@ BEGIN | |
|
|
||
| v_effective_user_id := v_api_key.user_id; | ||
|
|
||
| IF (SELECT enforcing_2fa FROM public.orgs WHERE id = v_effective_org_id) | ||
| AND 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; | ||
|
|
||
| v_allowed := public.rbac_has_permission( | ||
| public.rbac_principal_apikey(), | ||
| v_api_key.rbac_id, | ||
|
|
@@ -312,12 +368,6 @@ BEGIN | |
|
|
||
| v_effective_user_id := v_api_key.user_id; | ||
|
|
||
| IF (SELECT enforcing_2fa FROM public.orgs WHERE id = v_effective_org_id) | ||
| AND NOT public.has_2fa_enabled(v_effective_user_id) | ||
| THEN | ||
| RETURN false; | ||
| END IF; | ||
|
|
||
| RETURN public.rbac_has_permission( | ||
| public.rbac_principal_apikey(), | ||
| v_api_key.rbac_id, | ||
|
|
@@ -640,11 +690,8 @@ ON public.app_versions | |
| FOR SELECT | ||
| TO anon, authenticated | ||
| USING ( | ||
| public.rbac_check_permission_request( | ||
| public.rbac_perm_app_read_bundles(), | ||
| owner_org, | ||
| app_id, | ||
| NULL::bigint | ||
| app_id = ANY( | ||
| COALESCE((SELECT public.app_versions_readable_app_ids()), '{}'::character varying[]) | ||
| ) | ||
| ); | ||
|
|
||
|
|
@@ -1249,11 +1296,8 @@ USING ( | |
| SELECT 1 | ||
| FROM public.app_versions av | ||
| WHERE av.id = manifest.app_version_id | ||
| AND public.rbac_check_permission_request( | ||
| public.rbac_perm_app_read_bundles(), | ||
| av.owner_org, | ||
| av.app_id, | ||
| NULL::bigint | ||
| AND av.app_id = ANY( | ||
| COALESCE((SELECT public.app_versions_readable_app_ids()), '{}'::character varying[]) | ||
| ) | ||
| ) | ||
| ); | ||
|
|
@@ -4349,10 +4393,10 @@ SET rbac_role_name = COALESCE( | |
| WHEN 'super_admin' THEN 'channel_admin' | ||
| WHEN 'invite_admin' THEN 'channel_admin' | ||
| WHEN 'admin' THEN 'channel_admin' | ||
| WHEN 'invite_write' THEN 'channel_developer' | ||
| WHEN 'write' THEN 'channel_developer' | ||
| WHEN 'invite_upload' THEN 'channel_uploader' | ||
| WHEN 'upload' THEN 'channel_uploader' | ||
| WHEN 'invite_write' THEN 'channel_admin' | ||
| WHEN 'write' THEN 'channel_admin' | ||
| WHEN 'invite_upload' THEN 'channel_admin' | ||
| WHEN 'upload' THEN 'channel_admin' | ||
|
Comment on lines
+4396
to
+4399
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For existing Useful? React with 👍 / 👎. |
||
| ELSE 'channel_reader' | ||
| END | ||
| WHEN app_id IS NOT NULL THEN | ||
|
|
@@ -5731,7 +5775,7 @@ BEGIN | |
| ON role_permissions.role_id = role_closure.effective_role_id | ||
| INNER JOIN public.permissions | ||
| ON permissions.id = role_permissions.permission_id | ||
| WHERE permissions.key = public.rbac_perm_app_read() | ||
| WHERE permissions.key = public.rbac_perm_app_read_bundles() | ||
| ), | ||
| scoped_apps AS ( | ||
| SELECT apps.app_id, apps.owner_org | ||
|
|
@@ -5758,11 +5802,11 @@ BEGIN | |
| SELECT orgs.id | ||
| FROM candidate_orgs | ||
| INNER JOIN public.orgs ON orgs.id = candidate_orgs.owner_org | ||
| WHERE ( | ||
| orgs.enforcing_2fa IS NOT TRUE | ||
| OR (v_user_id IS NOT NULL AND public.has_2fa_enabled(v_user_id)) | ||
| WHERE v_principal_type = public.rbac_principal_apikey() | ||
| OR ( | ||
| (orgs.enforcing_2fa IS NOT TRUE OR (v_user_id IS NOT NULL AND public.has_2fa_enabled(v_user_id))) | ||
| AND public.user_meets_password_policy(v_user_id, orgs.id) IS DISTINCT FROM false | ||
| ) | ||
| AND public.user_meets_password_policy(v_user_id, orgs.id) IS DISTINCT FROM false | ||
| ) | ||
| SELECT COALESCE(array_agg(DISTINCT scoped_apps.app_id), '{}'::character varying[]) | ||
| INTO v_allowed | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the caller is an API key bound only to the new
apikey_managerrole, this org-levelorg.manage_apikeyscheck is the only authorization before arbitrary requested bindings are created. The latercreateRoleBindingForPrincipalguard only comparespriority_rank, soapikey_manager(rank 78) can POST a lower-rankedapp_adminorchannel_adminbinding and mint a key that can update apps/channels even though the caller itself only had API-key-management rights. Please also verify the caller can grant each requested role/scope, or restrict the roles this manager can assign.Useful? React with 👍 / 👎.