From c21d560cfd63123431d31c6e205065b0eec1c44b Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Tue, 5 Jul 2016 12:33:19 -0400 Subject: [PATCH 1/3] Credential can_change updates for organization related field --- awx/main/access.py | 18 +++++++-- awx/main/tests/functional/conftest.py | 3 +- .../tests/functional/test_rbac_credential.py | 40 ++++++++++++++++++- 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 442d8d5230..6faeac9ee9 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 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..7511f0ef78 100644 --- a/awx/main/tests/functional/conftest.py +++ b/awx/main/tests/functional/conftest.py @@ -159,8 +159,7 @@ def machine_credential(): @pytest.fixture def org_credential(organization, credential): - credential.admin_role.parents.add(organization.admin_role) - return credential + 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..d521c055f6 100644 --- a/awx/main/tests/functional/test_rbac_credential.py +++ b/awx/main/tests/functional/test_rbac_credential.py @@ -112,7 +112,45 @@ 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(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): From ddbe54f841b03c4540af97fd3ccb449797dd72c1 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Tue, 5 Jul 2016 15:06:40 -0400 Subject: [PATCH 2/3] allow null data cases in credential can_change --- awx/main/access.py | 2 +- awx/main/tests/functional/test_rbac_credential.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/awx/main/access.py b/awx/main/access.py index 6faeac9ee9..0d4842a10c 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -616,7 +616,7 @@ class CredentialAccess(BaseAccess): # Check access to organizations organization_pk = get_pk_from_dict(data, 'organization') - if organization_pk != getattr(obj, 'organization_id', None): + 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) diff --git a/awx/main/tests/functional/test_rbac_credential.py b/awx/main/tests/functional/test_rbac_credential.py index d521c055f6..95a6610a23 100644 --- a/awx/main/tests/functional/test_rbac_credential.py +++ b/awx/main/tests/functional/test_rbac_credential.py @@ -125,6 +125,8 @@ def test_org_credential_access_member(alice, org_credential, credential): 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}) From 5df846eb0a60d38471bf354f86a47670ec4b87ff Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 6 Jul 2016 15:21:57 -0400 Subject: [PATCH 3/3] remove unnecessary fixture from org_credential --- awx/main/tests/functional/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index 7511f0ef78..076f368631 100644 --- a/awx/main/tests/functional/conftest.py +++ b/awx/main/tests/functional/conftest.py @@ -158,7 +158,7 @@ def machine_credential(): return Credential.objects.create(name='machine-cred', kind='ssh', username='test_user', password='pas4word') @pytest.fixture -def org_credential(organization, credential): +def org_credential(organization): return Credential.objects.create(kind='aws', name='test-cred', organization=organization) @pytest.fixture