Skip to content
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

RBAC: Adjust filter for acl list to check for permissions on service accounts #78681

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

kalleep
Copy link
Contributor

@kalleep kalleep commented Nov 27, 2023

What is this feature?
When we added support for service account in acl list we never adjusted the permissions filter to look for correct permissions for service accounts. Because of this a user could only list service account if they could read all users or could read a service account with a user scopes (this should not be possible).

I adjusted the filter to look for different permissions depending on if the user is marked as a service account or not.
This query is quite large and complex and I wonder if we should move out the filter from the query and check it on the service layer manually instead.

Which issue(s) does this PR fix?:
Fixes https://github.com/grafana/identity-access-team/issues/487

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@kalleep kalleep added this to the 10.3.x milestone Nov 27, 2023
@kalleep kalleep requested a review from a team as a code owner November 27, 2023 11:46
Copy link
Contributor

@IevaVasiljeva IevaVasiljeva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this! I agree that the query is getting pretty long, but I think it's still quite readable, so I think I vote for sticking with this approach for now (it also feels consistent with how we filter resources in most other places).

@kalleep kalleep force-pushed the rbac/service-account-filtering branch from 334187b to 1f39736 Compare November 27, 2023 12:13
Copy link
Contributor

@gamab gamab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@kalleep kalleep merged commit 1c270b1 into main Nov 27, 2023
14 checks passed
@kalleep kalleep deleted the rbac/service-account-filtering branch November 27, 2023 12:37
@aangelisc aangelisc modified the milestones: 10.3.x, 10.2.3 Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants