-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Prevent assigning credential to user of other org #15296
base: devel
Are you sure you want to change the base?
Conversation
@@ -315,6 +317,15 @@ def _get_dynamic_input(self, field_name): | |||
else: | |||
raise ValueError('{} is not a dynamic input field'.format(field_name)) | |||
|
|||
def validate_role_assignment(self, actor, role_definition): | |||
if isinstance(actor, User): | |||
if actor.is_superuser or self.organization in actor.organizations: |
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.
actor.organizations
wasn't correct, and it was my fault. #15298
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.
We should add a unit test for this. Considering it now has 2 dependent patches, I would not feel comfortable merging without that as added verification. Otherwise we should be good.
resp = post(url=url, data={"user": rando.id, "role_definition": rd.id, "object_id": credential.id}, user=admin_user, expect=400) | ||
assert "You cannot grant credential access to a User not in the credentials' organization" in str(resp.data) | ||
|
||
# can assign credential to superuser |
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.
Never really thought about this case, also don't think I care either way.
CI should be re-ran once ansible/django-ansible-base#490 is merged |
d861cfc
to
4076208
Compare
I previously rebased this (using github). Checks failed due to stale image. I rebuilt the image and restarted the check (without modifying patch) just now. I am expecting a pass, but we'll see. |
I know the image has rebuilt, but locally, I pulled it and got a version from 19 hours ago (which is too old) and confirmed it has this commit: ansible/django-ansible-base@8eab0c0 And if that's what the checks are using (which it could be) then that would be why it's failing. |
Utilizes the `validate_role_assignment` callback from dab (see dab PR ansible#490) to prevent granting credential access to a user of another organization. This logic will work for role_user_assignments and role_team_assignments endpoints. Signed-off-by: Seth Foster <[email protected]>
Signed-off-by: Seth Foster <[email protected]>
Signed-off-by: Seth Foster <[email protected]>
Signed-off-by: Seth Foster <[email protected]>
4076208
to
0c2f5a8
Compare
#15308 was merged to freshen up the images and address the issue with CI here. |
SUMMARY
AAP-24919
requires ansible/django-ansible-base#490
Utilizes the
validate_role_assignment
callback from dab to prevent granting credential access to a user of another organization.This prevention was already being handled correctly for the
RoleUsersList
view, but not through the newerrole assignment endpoints.
This change is for assignments that are made through the role_user_assignments and role_team_assignments endpoints.
ISSUE TYPE
COMPONENT NAME