feat: update RBAC permissioning to support `grafana-irm-app` by joeyorlando · Pull Request #5149 · grafana/oncall

joeyorlando · 2024-10-09T21:33:20Z

What this PR does

Closes https://github.com/grafana/irm/issues/31 (and supersedes #4784)

Main changes:

updates apps.api.permissions.user_is_authorized to check the value of organization.is_grafana_irm_enabled. If it is, we check for the presence of grafana-irm-app prefixed RBAC permissions rather than grafana-oncall-app
cleans-up engine/apps/api/tests/test_permissions.py (bulk of the changes in the PR)
converts apps.user_management.models.User.build_permissions_query to a UserQuerySet method instead
- means we can now do things like this instead:
```
User.objects.filter_by_permission(RBACPermission.Permissions.NOTIFICATIONS_READ, organization)
```

Checklist

Unit, integration, and e2e (if applicable) tests updated
Documentation added (or pr:no public docs PR label added if not required)
Added the relevant release notes label (see labels prefixed w/ release:). These labels dictate how your PR will
show up in the autogenerated release notes.

joeyorlando · 2024-10-10T15:55:35Z

-    if organization.is_rbac_permissions_enabled:
-        # it is more efficient to check permissions on the subset of users filtered above
-        # than performing a regex query for the required permission
-        users_found_in_ical = [u for u in users_found_in_ical if {"action": required_permission.value} in u.permissions]
-    else:
-        users_found_in_ical = users_found_in_ical.filter(role__lte=required_permission.fallback_role.value)
-
-    return list(users_found_in_ical)


this makes things much cleaner. users_in_ical shouldn't need to know anything about how to conditionally check permissions (ie. is_rbac_permissions_enabled) or the structure of RBAC permissions ({"action": required_permission.value}).

See the new LegacyAccessControlCompatiblePermission.user_has_permission method

joeyorlando · 2024-10-10T15:56:10Z

        self.value = f"{prefix}.{resource.value}:{action.value}"
        self.fallback_role = fallback_role

+    def user_has_permission(self, user: "User") -> bool:


main changes

joeyorlando · 2024-10-10T15:57:33Z



-def get_permission_from_permission_string(perm: str) -> typing.Optional[LegacyAccessControlCompatiblePermission]:
+def get_permission_from_permission_string(


(see here for more context on how this is used; tldr; query filtering for the user's endpoint)

…rm-app

… github.com:grafana/oncall into jorlando/update-rbac-permissions-to-support-irm-app

matiasb

LGTM, minor comments added.

matiasb · 2024-10-10T16:45:25Z

+        permission_class_value = permission_class.value
+        irm_permission_value = convert_oncall_permission_to_irm(permission_class)
+
+        if permission_class_value == perm or organization.is_grafana_irm_enabled and irm_permission_value == perm:


Precedence works ok here, but maybe add parenthesis grouping the and conditions to make it easier to read?

decided to get rid of this method in favour of ALL_PERMISSION_NAME_TO_CLASS_MAP (dict lookup should be more performant than the for loop)

matiasb · 2024-10-10T16:53:24Z

@@ -90,14 +93,9 @@ def users_in_ical(
        (Q(username__in=usernames_from_ical) | Q(email__lower__in=emails_from_ical))
    ).distinct()


Wondering if we should do a select_related for organization here, since we will be accessing u.organization for each user when checking user_has_permission below.

good catch 👍 addressed in 6c9a29d

feat: update RBAC permissioning to support grafana-irm-app

a52e19f

joeyorlando 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 9, 2024

joeyorlando requested a review from a team October 9, 2024 21:33

joeyorlando temporarily deployed to github-pages October 9, 2024 21:33 — with GitHub Actions Inactive

joeyorlando marked this pull request as draft October 10, 2024 13:03

joeyorlando mentioned this pull request Oct 10, 2024

update backend authorization to support grafana-irm-app prefiex RBAC permissions #4784

Closed

3 tasks

joeyorlando added 2 commits October 10, 2024 10:30

wip

8effe17

linting

bb580bf

joeyorlando temporarily deployed to github-pages October 10, 2024 14:32 — with GitHub Actions Inactive

wip

2e25016

joeyorlando had a problem deploying to github-pages October 10, 2024 15:28 — with GitHub Actions Failure

wip

159f664

joeyorlando had a problem deploying to github-pages October 10, 2024 15:44 — with GitHub Actions Failure

wip

3a1f1bb

joeyorlando temporarily deployed to github-pages October 10, 2024 15:49 — with GitHub Actions Inactive

joeyorlando commented Oct 10, 2024

View reviewed changes

joeyorlando marked this pull request as ready for review October 10, 2024 16:01

Merge branch 'dev' into jorlando/update-rbac-permissions-to-support-i…

175a9b0

…rm-app

joeyorlando had a problem deploying to github-pages October 10, 2024 16:03 — with GitHub Actions Error

joeyorlando added 2 commits October 10, 2024 12:06

wip

248b0ca

Merge branch 'jorlando/update-rbac-permissions-to-support-irm-app' of…

df00e67

… github.com:grafana/oncall into jorlando/update-rbac-permissions-to-support-irm-app

joeyorlando temporarily deployed to github-pages October 10, 2024 16:07 — with GitHub Actions Inactive

matiasb approved these changes Oct 10, 2024

View reviewed changes

wip

d827d83

joeyorlando had a problem deploying to github-pages October 10, 2024 17:21 — with GitHub Actions Error

PR comment

6c9a29d

joeyorlando temporarily deployed to github-pages October 10, 2024 17:26 — with GitHub Actions Inactive

joeyorlando added 2 commits October 10, 2024 13:30

linting

da0c5b5

wrap up testing

85c5742

joeyorlando temporarily deployed to github-pages October 10, 2024 17:49 — with GitHub Actions Inactive

joeyorlando added this pull request to the merge queue Oct 10, 2024

Merged via the queue into dev with commit 04ab676 Oct 10, 2024

joeyorlando deleted the jorlando/update-rbac-permissions-to-support-irm-app branch October 10, 2024 19:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: update RBAC permissioning to support `grafana-irm-app`#5149

feat: update RBAC permissioning to support `grafana-irm-app`#5149
joeyorlando merged 13 commits into
devfrom
jorlando/update-rbac-permissions-to-support-irm-app

joeyorlando commented Oct 9, 2024 •

edited

Loading

Uh oh!

joeyorlando Oct 10, 2024

Uh oh!

joeyorlando Oct 10, 2024

Uh oh!

joeyorlando Oct 10, 2024

Uh oh!

matiasb left a comment

Uh oh!

matiasb Oct 10, 2024

Uh oh!

joeyorlando Oct 10, 2024 •

edited

Loading

Uh oh!

matiasb Oct 10, 2024

Uh oh!

joeyorlando Oct 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants



		def get_permission_from_permission_string(perm: str) -> typing.Optional[LegacyAccessControlCompatiblePermission]:
		def get_permission_from_permission_string(

		@@ -90,14 +93,9 @@ def users_in_ical(
		(Q(username__in=usernames_from_ical) \| Q(email__lower__in=emails_from_ical))
		).distinct()

Conversation

joeyorlando commented Oct 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Checklist

Uh oh!

joeyorlando Oct 10, 2024

Choose a reason for hiding this comment

Uh oh!

joeyorlando Oct 10, 2024

Choose a reason for hiding this comment

Uh oh!

joeyorlando Oct 10, 2024

Choose a reason for hiding this comment

Uh oh!

matiasb left a comment

Choose a reason for hiding this comment

Uh oh!

matiasb Oct 10, 2024

Choose a reason for hiding this comment

Uh oh!

joeyorlando Oct 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matiasb Oct 10, 2024

Choose a reason for hiding this comment

Uh oh!

joeyorlando Oct 10, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

joeyorlando commented Oct 9, 2024 •

edited

Loading

joeyorlando Oct 10, 2024 •

edited

Loading