-
Notifications
You must be signed in to change notification settings - Fork 164
[PLAT-191] resource level sharing #8314
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
Conversation
|
@begelundmuller Going through the comments wanted to discuss few high level things-
|
begelundmuller
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would also be nice with a test case that adds/removes resource restrictions, and parses the generated JWT and checks if the security rules match the expecations
begelundmuller
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good for initial merge once the final comments have been addressed
* resource level sharing * full role check * fixes * fxi issue with resources in project permissions * remove meta tag * comment * defensive handling of existing invites * fix condition * minor refactor * remove project invite on all resource removal * review comments * api args * refactor * check * clean up * clean up * not fonud status * backwards compatibility * review comments * fix dto * fix test * review comments * review comment * sort resources before storing * some tests * fixes and test * handle reports/alerts with restricted resource access * lint * review comments * fix test * don't filter managed groups * minor refactor
Allows user to have access to only certain resources, for now limited to explore and canvas. Resources can be shared directly for a user or a usergroup.
Resource restriction works independently of the roles i.e. user or usergroup role can change but resource restrictions remains the same. This also allows to add user with a role with access to no resources at all. So this is possible
Similar cmds for usergroups.
Important Note
If user has any role on org level apart from
guestthen they automatically become part of system managed group namedautogroup:membersand thus resource restrictions won't apply unless one explicitly sets resources restrictions onautogroup:membersgroup (which you should think twice before doing).Existing APIs like
AddProjectMemberUserandSetProjectMemberUserRole(and similar for usergroups) now accepts additional resource names and restrict-resources bool flag, if there are more than 0 resources in the call then restrict-resources is automatically set to true. The APIs works in PUT style i.e. with each call full list of resources should be provided and it just replaces the existing list, this means if resources are not set then resource list will be empty or if restrict resources flag is not set then essentially it give access to everything.Permission Resolution
Resource restrictions only apply If all roles for the user/group have restrict resources set to true (AND of all flags); the effective resources will be union of resources from role and groups.
Backwards compatibility
Since the APIs now accept additional params, UI needs to populate them always by first fetching current resource list and restrict resource flag otherwise it may unintentionally give access to everything. If application changes cannot go in with these changes then we may change the flag to be optional and if not set then fetch current resources/flag and use those. Also restrict APIs being called from older versions of CLI.
Follow up (Component level sharing)
An interesting follow up could be have component level sharing so that specific components from the canvas can be shared, this will require refactoring component's
ResolveTransitiveAccessand then at canvas level it will just merge component level transitive accesses.Checklist: