diff --git a/awx/main/access.py b/awx/main/access.py index 97a64ae0c6..08b1b7b10f 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -611,10 +611,20 @@ class CredentialAccess(BaseAccess): @check_superuser def can_change(self, obj, data): - if data is not None: - keys = data.keys() - if 'user' in keys or 'team' in keys or 'organization' in keys: - if not self.can_add(data): + if not obj: + return False + + # Check access to organizations + organization_pk = get_pk_from_dict(data, 'organization') + if data and 'organization' in data and organization_pk != getattr(obj, 'organization_id', None): + if organization_pk: + # admin permission to destination organization is mandatory + new_organization_obj = get_object_or_400(Organization, pk=organization_pk) + if self.user not in new_organization_obj.admin_role: + return False + # admin permission to existing organization is also mandatory + if obj.organization: + if self.user not in obj.organization.admin_role: return False if obj.organization: diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index 5e6ef333bb..076f368631 100644 --- a/awx/main/tests/functional/conftest.py +++ b/awx/main/tests/functional/conftest.py @@ -158,9 +158,8 @@ def machine_credential(): return Credential.objects.create(name='machine-cred', kind='ssh', username='test_user', password='pas4word') @pytest.fixture -def org_credential(organization, credential): - credential.admin_role.parents.add(organization.admin_role) - return credential +def org_credential(organization): + return Credential.objects.create(kind='aws', name='test-cred', organization=organization) @pytest.fixture def inventory(organization): diff --git a/awx/main/tests/functional/test_rbac_credential.py b/awx/main/tests/functional/test_rbac_credential.py index 4a05403233..95a6610a23 100644 --- a/awx/main/tests/functional/test_rbac_credential.py +++ b/awx/main/tests/functional/test_rbac_credential.py @@ -112,7 +112,47 @@ def test_credential_access_admin(user, team, credential): cred.save() # should have can_change access as org-admin - assert access.can_change(credential, {'user': u.pk}) + assert access.can_change(credential, {'description': 'New description.'}) + +@pytest.mark.django_db +def test_org_credential_access_member(alice, org_credential, credential): + org_credential.admin_role.members.add(alice) + credential.admin_role.members.add(alice) + + access = CredentialAccess(alice) + + # Alice should be able to PATCH if organization is not changed + assert access.can_change(org_credential, { + 'description': 'New description.', + 'organization': org_credential.organization.pk}) + assert access.can_change(org_credential, { + 'description': 'New description.'}) + assert access.can_change(credential, { + 'description': 'New description.', + 'organization': None}) + +@pytest.mark.django_db +def test_credential_access_org_permissions( + org_admin, org_member, organization, org_credential, credential): + credential.admin_role.members.add(org_admin) + credential.admin_role.members.add(org_member) + org_credential.admin_role.members.add(org_member) + + access = CredentialAccess(org_admin) + member_access = CredentialAccess(org_member) + + # Org admin can move their own credential into their org + assert access.can_change(credential, {'organization': organization.pk}) + # Org member can not + assert not member_access.can_change(credential, { + 'organization': organization.pk}) + + # Org admin can remove a credential from their org + assert access.can_change(org_credential, {'organization': None}) + # Org member can not + assert not member_access.can_change(org_credential, {'organization': None}) + assert not member_access.can_change(org_credential, { + 'user': org_member.pk, 'organization': None}) @pytest.mark.django_db def test_cred_job_template_xfail(user, deploy_jobtemplate):