[rbac] Fix permissions for high-privilege users - #1622
Conversation
## What this PR does This patch grants "admin" permissions to super-admins, "use" permissions to admins and super-admins, "view" permissions to "use"-privileged users, admins, and super-admins. Previously lower-privileged roles were not assigned to higher-privileged users, so a viewer could excercise their basic read-only permissions which were not available to high-privilege users. This patch corrects the template function used to generate subjects in rolebindings, fixing the issue. ### Release note ```release-note [rbac] Fix issue of privileged users not having low-privilege read-only permissions. ``` Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
Summary of ChangesHello Timofei Larkin (@lllamnyp), I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical flaw in the Role-Based Access Control (RBAC) configuration where users with higher administrative privileges were inadvertently lacking fundamental lower-level permissions. By updating the RBAC template function, the changes ensure that permissions are correctly inherited across different access levels, thereby allowing privileged users to perform all actions expected of their role, including those typically associated with less privileged roles. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
WalkthroughA template function call for RBAC subject generation is updated across multiple role bindings in the tenant template. The function Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
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 |
There was a problem hiding this comment.
Code Review
This pull request aims to fix RBAC permissions by ensuring higher-privileged users also receive lower-privilege permissions. This is done by switching to the cozy-lib.rbac.subjectsForTenantAndAccessLevel Helm template helper. While the intent is correct, this change introduces a critical issue. The cozy-lib.rbac.subjectsForTenantAndAccessLevel function incorrectly generates ServiceAccount subjects from parent tenants, which reside in different namespaces. Kubernetes RoleBindings are namespace-scoped and cannot reference subjects in other namespaces, which will cause the deployment to fail. I have added a detailed comment on the first occurrence of this issue, which applies to all changes in this pull request. This must be fixed before merging.
| namespace: {{ include "tenant.name" . }} | ||
| subjects: | ||
| {{ include "cozy-lib.rbac.subjectsForTenant" (list "view" (include "tenant.name" .)) | nindent 2 }} | ||
| {{ include "cozy-lib.rbac.subjectsForTenantAndAccessLevel" (list "view" (include "tenant.name" .)) | nindent 2 }} |
There was a problem hiding this comment.
The cozy-lib.rbac.subjectsForTenantAndAccessLevel template function introduces a critical issue by generating invalid subjects for this RoleBinding. A Kubernetes RoleBinding is a namespace-scoped resource and can only grant permissions to subjects (like ServiceAccounts) within its own namespace.
The subjectsForTenantAndAccessLevel template iterates through the current tenant and all its parent tenants, and for each, it creates a ServiceAccount subject. For any parent tenant, the ServiceAccount will be in a different namespace than this RoleBinding, making the generated resource invalid and causing the deployment to fail.
To fix this, the cozy-lib.rbac.subjectsForTenantAndAccessLevel template in packages/library/cozy-lib/templates/_rbac.tpl must be changed to not generate ServiceAccount subjects for parent tenants. It should only generate a ServiceAccount subject for the current tenant, if at all.
The problematic logic in cozy-lib.rbac.subjectsForTenantAndAccessLevel is the call to include "cozy-lib.rbac.serviceAccountSubject" inside the loop over all tenants ($tenants). This call should be moved outside the loop or removed if ServiceAccount subjects are handled elsewhere.
This issue applies to all four changes in this file.
There was a problem hiding this comment.
This is incorrect. Roles can reference serviceaccounts in other namespaces. To quote the k8s documentation:
You can use RBAC to allow service accounts in one namespace to perform actions on resources in a different namespace in the cluster.
|
Successfully created backport PR for |
What this PR does
This patch grants "admin" permissions to super-admins, "use" permissions to admins and super-admins, "view" permissions to "use"-privileged users, admins, and super-admins. Previously lower-privileged roles were not assigned to higher-privileged users, so a viewer could excercise their basic read-only permissions which were not available to high-privilege users. This patch corrects the template function used to generate subjects in rolebindings, fixing the issue.
Release note
Summary by CodeRabbit