feat: rbac client side code #26694
feat: rbac client side code #26694
Conversation
|
Size Change: +64 B (+0.01%) Total Size: 1.11 MB ℹ️ View Unchanged
|
📸 UI snapshots have been updated8 snapshot changes in total. 0 added, 8 modified, 0 deleted:
Triggered by this commit. |
zlwaterfield
requested review from
benjackwhite,
Twixes and
raquelmsmith
and removed request for
benjackwhite and
Twixes
zlwaterfield
changed the title
📸 UI snapshots have been updated14 snapshot changes in total. 0 added, 14 modified, 0 deleted:
Triggered by this commit. |
There was a problem hiding this comment.
I love this PR so much. No idea if this is because I'm seeing a lot of my own code and having an incredibly vain moment or if it's because your modifications / rewrite is so fantastic. I'm gonna call this a fine cocktail of the both of us called the Ben-Zachary 😍
All of my comments outside the activity scope stuff can be follow up rather than blocking 🙌
frontend/src/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject.tsx
Outdated
Show resolved
Hide resolved
| <div className="my-1"> | ||
| <SimplLevelComponent | ||
| size="small" | ||
| level={access_level} |
There was a problem hiding this comment.
One thing missing here potentially is a notice to indicate if the default access level is higher than the selected level (making it clear that the default level of "editor" overrides "viewer" for example
| @@ -518,6 +528,11 @@ export const notebookLogic = kea<notebookLogicType>([ | |||
| ) | |||
| }, | |||
| setLocalContent: async ({ updateEditor, jsonContent }, breakpoint) => { | |||
| if (values.notebook?.user_access_level !== 'editor') { | |||
There was a problem hiding this comment.
Edge case - If user A has the notebook open and then user B swaps the permissions to view only user A can get stuck in a state where the local content is there but the API requests failed... We probs need an alert if the API response indicates no permissions and then some sort of refresh.
There was a problem hiding this comment.
Yeah good point, we probably need a permission probe across all resources tbh.
frontend/src/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject.tsx
Show resolved
Hide resolved
| <div> | ||
| <h3>Access control</h3> | ||
| <LemonBanner type="info" className="mb-4"> | ||
| Permissions have moved! We're rolling out our new access control system. Click below to open it. |
There was a problem hiding this comment.
This is a nice system
There was a problem hiding this comment.
Although we may need to consider still showing the old UI if there are some configurations set. Depends on whether we will go for an all-in-one migration or not
There was a problem hiding this comment.
This only renders when the flag is on so my idea was we migrate users and then turn on the flag but that might be too simplistic so I'll keep that in mind.
Changes
See issue: #24512
This PR is adding the main UI for RBAC behind a feature flag. This is all client side and only available for those with the feature flag. This is just the first PR, more to come. This should not affect any existing users.
Added
See slack #project-rbac for next steps.
Role management:
Project access:
Resource access control:
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Does this work well for both Cloud and self-hosted?
It doesn't have an impact.
How did you test this code?
Manually tested