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

feat: rbac middleware #26159

Merged
merged 38 commits into from
Nov 21, 2024
Merged

feat: rbac middleware #26159

merged 38 commits into from
Nov 21, 2024

Conversation

zlwaterfield
Copy link
Contributor

@zlwaterfield zlwaterfield commented Nov 12, 2024

Changes

Adding middleware for RBAC. This is mostly pulling the logic from @benjackwhite previous PR: #20864 but updating a bunch more tests and a few logic pieces.

See #24512 for more information.

Key additions:

  • ee
    • AccessControlSerializer - for the AccessControl model
    • AccessControlViewSetMixin - Adds an "access_controls" action to the viewset that handles access control for the given resource
  • posthog
    • AccessControlPermission - Unified permissions access - controls access to any object based on the user's access controls
    • UserAccessControlSerializerMixin - Mixin for serializers to add user access control fields
    • UserAccessControl - UserAccessControl provides functions for checking unified access to all resources and objects from a Project level downwards.

Many of the changes are test updates for the number of queries. Then we've also added the middleware to the viewsets of Dashboard, Projects, Organizations, Insights, Notebooks, Feature Flags, etc.

More details in Readme to come.

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Does this work well for both Cloud and self-hosted?

How did you test this code?

Updated existing tests
Added new tests

@zlwaterfield zlwaterfield marked this pull request as ready for review November 14, 2024 23:27
Copy link
Contributor

@benjackwhite benjackwhite left a comment

Choose a reason for hiding this comment

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

This looks solid! (If i do say so myself).

Should be totally safe but we might want to be ready for a qucik revert just in case

queryset = self.safely_get_queryset(queryset)
except NotImplementedError:
pass
self._in_get_queryset = True
Copy link
Contributor

Choose a reason for hiding this comment

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

Ooh this is nice

posthog/permissions.py Outdated Show resolved Hide resolved
@zlwaterfield zlwaterfield merged commit 7b261b9 into master Nov 21, 2024
91 checks passed
@zlwaterfield zlwaterfield deleted the zach/rbac/3 branch November 21, 2024 20:36
Copy link

sentry-io bot commented Nov 21, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ AssertionError /api/feature_flag/local_evaluation/ View Issue

Did you find this useful? React with a 👍 or 👎

zlwaterfield added a commit that referenced this pull request Nov 21, 2024
zlwaterfield added a commit that referenced this pull request Nov 21, 2024
zlwaterfield added a commit that referenced this pull request Nov 21, 2024
thmsobrmlr pushed a commit that referenced this pull request Nov 25, 2024
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Bryan Ciaraldi <[email protected]>
thmsobrmlr pushed a commit that referenced this pull request Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants