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

Enable RBAC support for public API endpoints #5211

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

matiasb
Copy link
Contributor

@matiasb matiasb commented Oct 25, 2024

@matiasb matiasb added pr:no public docs Added to a PR that does not require public documentation updates release:patch PR will be added to "Other Changes" section of release notes labels Oct 25, 2024
@matiasb matiasb requested a review from a team October 25, 2024 17:15
Copy link
Contributor

@joeyorlando joeyorlando left a comment

Choose a reason for hiding this comment

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

this is amazing! 🔥

@@ -52,10 +52,8 @@ def authenticate(self, request):
auth = get_authorization_header(request).decode("utf-8")
user, auth_token = self.authenticate_credentials(auth)

if not user.is_active or not user_is_authorized(user, [RBACPermission.Permissions.API_KEYS_WRITE]):
Copy link
Contributor

Choose a reason for hiding this comment

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

nice 👍 (this always felt like a hack)

Copy link
Contributor

@joeyorlando joeyorlando left a comment

Choose a reason for hiding this comment

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

I'm guessing docs + (maybe?) deprecating creation of new API keys (UI + API) will be done in separate PRs?

@matiasb
Copy link
Contributor Author

matiasb commented Oct 25, 2024

I'm guessing docs + (maybe?) deprecating creation of new API keys (UI + API) will be done in separate PRs?

Yeah, there will be more PRs coming (this is one first step from #5212); in practice this is still not really useful (because only admins can issue tokens for now).

OTOH, not sure about deprecating our API keys, something to discuss probably (with these changes any user could have their own token, with limited perms, which could be still useful to perform user-related actions: ack, resolve, etc., while service accounts are not tied to a user).

@joeyorlando
Copy link
Contributor

in practice this is still not really useful (because only admins can issue tokens for now)

you don't need to be an Admin per se. You could have Viewer users that you assign some of these RBAC roles to be able to create service accounts + tokens (but right, I guess this doesn't apply to OSS where RBAC in Grafana isn't available).

@matiasb matiasb added this pull request to the merge queue Oct 30, 2024
Merged via the queue into dev with commit 91b67b9 Oct 30, 2024
28 checks passed
@matiasb matiasb deleted the matiasb/public-api-rbac-checks branch October 30, 2024 10:05
github-merge-queue bot pushed a commit that referenced this pull request Nov 19, 2024
Related to grafana/oncall-private#2826

Continuing work started in #5211,
this adds support for Grafana service accounts tokens for API
authentication (except alert group actions which will still require a
user behind). Next steps would be updating the go client and the
terraform provider to allow service account token auth for OnCall
resources.

Following proposal 1.1 from
[doc](https://docs.google.com/document/d/1I3nFbsUEkiNPphBXT-kWefIeramTY71qqZ1OA06Kmls/edit?usp=sharing).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates release:patch PR will be added to "Other Changes" section of release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants