-
Notifications
You must be signed in to change notification settings - Fork 663
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
MG-2201 - Add Policy to Fail Role Update if User Not on Platform #2234
base: main
Are you sure you want to change the base?
Conversation
1768f0a
to
3bf4593
Compare
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.
Since we check if the user is SuperAdmin first before updating the role, then they are certainly in the platform @arvindh123 what do you think?
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.
Since we check if the user is SuperAdmin first before updating the role, then they are certainly in the platform @arvindh123 what do you think?
@rodneyosodo
In previous part of the code, we are checking the Request User is SuperAdmin or Normal User.
we are not checking the role change user ID .
It is like super admin can make other user as super admin or normal user
In role change API endpoint, SuperAdmin will be sending the user id of the other user to change the role.
So we should check the user id given by super admin in request , is exists in platform or not.
6c58f06
to
d519516
Compare
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.
Please update tests.
3137712
to
8c077be
Compare
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
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.
@JeffMboya Please resolve conflicts.
Signed-off-by: JeffMboya <jangina.mboya@gmail.com> refactor: add memberRelation policy Signed-off-by: JeffMboya <jangina.mboya@gmail.com> refactor: add memberRelation policy Signed-off-by: JeffMboya <jangina.mboya@gmail.com> refactor:Add memberRelation policy Signed-off-by: JeffMboya <jangina.mboya@gmail.com> refactor:Add memberRelation policy Signed-off-by: JeffMboya <jangina.mboya@gmail.com> refactor:Add memberRelation policy Signed-off-by: JeffMboya <jangina.mboya@gmail.com> refactor:Add memberRelation policy Signed-off-by: JeffMboya <jangina.mboya@gmail.com> refactor:Add memberRelation policy Signed-off-by: JeffMboya <jangina.mboya@gmail.com> refactor:Add memberRelation policy Signed-off-by: JeffMboya <jangina.mboya@gmail.com> refactor: Revert changes in grpcServer struct Signed-off-by: JeffMboya <jangina.mboya@gmail.com> refactor: Revert changes in grpcServer struct Signed-off-by: JeffMboya <jangina.mboya@gmail.com> refactor: Fix authorization error in updateClientPolicy Signed-off-by: JeffMboya <jangina.mboya@gmail.com> refactor: Fix authorization error in updateClientPolicy Signed-off-by: JeffMboya <jangina.mboya@gmail.com> refactor: Fix authorization error in updateClientPolicy Signed-off-by: JeffMboya <jangina.mboya@gmail.com>
Signed-off-by: JeffMboya <jangina.mboya@gmail.com>
Signed-off-by: JeffMboya <jangina.mboya@gmail.com>
Signed-off-by: JeffMboya <jangina.mboya@gmail.com>
Signed-off-by: JeffMboya <jangina.mboya@gmail.com>
Signed-off-by: JeffMboya <jangina.mboya@gmail.com> fix: failing tests Signed-off-by: JeffMboya <jangina.mboya@gmail.com> fix: failing tests Signed-off-by: JeffMboya <jangina.mboya@gmail.com> fix: failing tests Signed-off-by: JeffMboya <jangina.mboya@gmail.com> fix: failing tests Signed-off-by: JeffMboya <jangina.mboya@gmail.com>
Signed-off-by: JeffMboya <jangina.mboya@gmail.com>
Signed-off-by: JeffMboya <jangina.mboya@gmail.com>
Signed-off-by: JeffMboya <jangina.mboya@gmail.com>
60f460d
What type of PR is this?
This is a feature because it adds the following functionality: client check check in policy layer when updating client role.
What does this do?
This PR modifies the update client role functionality to check for client existence in the policy layer (spiceDB) instead of the repo layer. This prevents the system from adding a policy for a non-existent user and then having to roll it back.
Which issue(s) does this PR fix/relate to?
Have you included tests for your changes?
No
Did you document any new/modified feature?
No