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

chore: fixes to objects/queryset perm checks + type updates #27330

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

zlwaterfield
Copy link
Contributor

@zlwaterfield zlwaterfield commented Jan 7, 2025

Changes

Related to RBAC project, see more: #24512

This PR:

  • Checks for request in context when setting access control - sometimes this is called outside of a request and it's not available
  • Move the filter by access level into it's own function so it can be called when dangerously getting queryset - right now access levels are not being checked in the filters for listing dashboards and insights
  • Move around some UserAccessControlSerializerMixin usages so access levels are always available
  • Update insight view post permissions because this is a post allowed with view only permissions
  • Update dashboard retrieve to use the get object so permissions are checked - right now the get queryset is bypassing the object permissions
  • Update some access control types on the frontend

Changes pulled out of: #27174

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

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

Yes

How did you test this code?

Manuallu

@zlwaterfield zlwaterfield changed the title Check for request in context chore: fixes to objects/queryset perm checks + type updates Jan 7, 2025
Copy link
Contributor

github-actions bot commented Jan 7, 2025

Size Change: 0 B

Total Size: 1.11 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 1.11 MB

compressed-size-action

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@zlwaterfield
Copy link
Contributor Author

Working on fixing these tests. The fixes found some holes in the tests.

@zlwaterfield zlwaterfield self-assigned this Jan 7, 2025
@zlwaterfield zlwaterfield requested a review from a team January 8, 2025 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants