-
Notifications
You must be signed in to change notification settings - Fork 126
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
New permission ClientsRead and new router middleware RequireAtLeastOnePermission #324
Conversation
…, revoke ClientsManage from role User
…at a requester has at least one listed permission in order to access a given resource
Nothing stands out to me. There's potential for some DRY work here but I feel like it's squeezing for the sake of abstraction so I'm happy to approve this as-is but feel free to refactor based on other comments. |
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.
This is missing the criteria for Upload Only role currently. Not sure if we want to put that into this PR or open a new PR with that change in it. Otherwise, just a couple small housekeeping items and I think this looks good.
@superlinkx It looks like this requested change was already made in #131 |
Oh, awesome! I'll go ahead and give this the thumbs up as is then |
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.
LGTM
Description
A new permission,
ClientsRead
, is introduced and assigned to theUser
role. Additionally, theClientsManage
permission has been revoked from theUser
role.A new router middleware,
RequireAtLeastOnePermission
, is introduced. This middleware forbids access to a resource unless the requester has at least one of the listed permissions. This is in contrast toRequirePermissions
which forbids access to a resource unless the requester has all of the listed permissions.Motivation and Context
This change enables desired behavior in BloodHound Enterprise.
How Has This Been Tested?
Existing unit tests have been updated and new unit tests have been added.
Types of changes
Checklist: