From 2c05df064bab68145ff31fff2aa7289cca5c5063 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Fri, 10 Jun 2016 13:23:02 -0400 Subject: [PATCH 1/4] adjusting API for new Credential.organization --- awx/api/serializers.py | 7 +++++-- awx/api/urls.py | 2 +- awx/api/views.py | 20 ++------------------ 3 files changed, 8 insertions(+), 21 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 7c4afff005..3d7508a1fa 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1619,7 +1619,7 @@ class CredentialSerializer(BaseSerializer): model = Credential fields = ('*', 'kind', 'cloud', 'host', 'username', 'password', 'security_token', 'project', 'domain', - 'ssh_key_data', 'ssh_key_unlock', + 'ssh_key_data', 'ssh_key_unlock', 'organization', 'become_method', 'become_username', 'become_password', 'vault_password', 'subscription', 'tenant', 'secret', 'client', 'authorize', 'authorize_password') @@ -1634,13 +1634,16 @@ class CredentialSerializer(BaseSerializer): def get_related(self, obj): res = super(CredentialSerializer, self).get_related(obj) + + if obj.organization: + res['organization'] = reverse('api:organization_detail', args=(obj.organization.pk,)) + res.update(dict( activity_stream = reverse('api:credential_activity_stream_list', args=(obj.pk,)), access_list = reverse('api:credential_access_list', args=(obj.pk,)), object_roles = reverse('api:credential_object_roles_list', args=(obj.pk,)), owner_users = reverse('api:credential_owner_users_list', args=(obj.pk,)), owner_teams = reverse('api:credential_owner_teams_list', args=(obj.pk,)), - owner_organizations = reverse('api:credential_owner_organizations_list', args=(obj.pk,)), )) parents = obj.owner_role.parents.exclude(object_id__isnull=True) diff --git a/awx/api/urls.py b/awx/api/urls.py index c9a516cfd9..28f7f8744a 100644 --- a/awx/api/urls.py +++ b/awx/api/urls.py @@ -168,7 +168,7 @@ credential_urls = patterns('awx.api.views', url(r'^(?P[0-9]+)/object_roles/$', 'credential_object_roles_list'), url(r'^(?P[0-9]+)/owner/users/$', 'credential_owner_users_list'), url(r'^(?P[0-9]+)/owner/teams/$', 'credential_owner_teams_list'), - url(r'^(?P[0-9]+)/owner/organizations/$', 'credential_owner_organizations_list'), + url(r'^(?P[0-9]+)/organization/$', 'credential_owner_teams_list'), # See also credentials resources on users/teams. ) diff --git a/awx/api/views.py b/awx/api/views.py index 734640577b..28935358cc 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -1358,7 +1358,8 @@ class CredentialList(ListCreateAPIView): if 'team' in request.data: credential.owner_role.parents.add(team.member_role) if 'organization' in request.data: - credential.owner_role.parents.add(organization.admin_role) + credential.organization = organization + credential.save() return ret @@ -1388,23 +1389,6 @@ class CredentialOwnerTeamsList(SubListAPIView): return self.model.objects.filter(pk__in=teams) -class CredentialOwnerOrganizationsList(SubListAPIView): - model = Organization - serializer_class = OrganizationSerializer - parent_model = Credential - new_in_300 = True - - def get_queryset(self): - credential = get_object_or_404(self.parent_model, pk=self.kwargs['pk']) - if not self.request.user.can_access(Credential, 'read', None): - raise PermissionDenied() - - content_type = ContentType.objects.get_for_model(self.model) - orgs = [c.content_object.pk for c in credential.owner_role.parents.filter(content_type=content_type)] - - return self.model.objects.filter(pk__in=orgs) - - class UserCredentialsList(CredentialList): model = Credential From 5754b4bb2c9f8970087014cea47c7d32b2ad80dc Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Fri, 10 Jun 2016 13:23:32 -0400 Subject: [PATCH 2/4] adjusting Credential model and migrations --- awx/main/access.py | 4 ++ awx/main/migrations/0008_v300_rbac_changes.py | 6 ++- awx/main/migrations/_rbac.py | 45 ++++++++++++++----- awx/main/models/credential.py | 13 +++++- 4 files changed, 55 insertions(+), 13 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 231ece8042..90a9002259 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -600,6 +600,10 @@ class CredentialAccess(BaseAccess): if not self.can_add(data): return False + if obj.organization: + if self.user in obj.organization.admin_role: + return True + return self.user in obj.owner_role def can_delete(self, obj): diff --git a/awx/main/migrations/0008_v300_rbac_changes.py b/awx/main/migrations/0008_v300_rbac_changes.py index 9c4b193d47..50ce375f5f 100644 --- a/awx/main/migrations/0008_v300_rbac_changes.py +++ b/awx/main/migrations/0008_v300_rbac_changes.py @@ -86,7 +86,11 @@ class Migration(migrations.Migration): name='credential', unique_together=set([]), ), - + migrations.AddField( + model_name='credential', + name='organization', + field=models.ForeignKey(related_name='credentials', default=None, blank=True, to='main.Organization', null=True), + ), # # New RBAC models and fields diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index b7933f1a11..34f57ed773 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -123,10 +123,10 @@ def attrfunc(attr_path): return attr def _update_credential_parents(org, cred): - org.admin_role.children.add(cred.owner_role) + cred.organization = org cred.save() -def _discover_credentials(instances, cred, orgfunc): +def _discover_credentials(apps, instances, cred, orgfunc): '''_discover_credentials will find shared credentials across organizations. If a shared credential is found, it will duplicate the credential, ensure the proper role permissions are added to the new @@ -139,6 +139,8 @@ def _discover_credentials(instances, cred, orgfunc): orgfunc is a function that when called with an instance from instances will produce an Organization object. ''' + Credential = apps.get_model('main', "Credential") + orgs = defaultdict(list) for inst in instances: try: @@ -161,17 +163,38 @@ def _discover_credentials(instances, cred, orgfunc): _update_credential_parents(org, cred) else: # Create a new credential - cred.pk = None - cred.save() - - # Unlink the old information from the new credential - cred.owner_role, cred.use_role = None, None - cred.save() + new_cred = Credential.objects.create( + kind = cred.kind, + cloud = cred.cloud, + host = cred.host, + username = cred.username, + password = cred.password, + security_token = cred.security_token, + project = cred.project, + domain = cred.domain, + ssh_key_data = cred.ssh_key_data, + ssh_key_unlock = cred.ssh_key_unlock, + become_method = cred.become_method, + become_username = cred.become_username, + become_password = cred.become_password, + vault_password = cred.vault_password, + authorize = cred.authorize, + authorize_password = cred.authorize_password, + client = cred.client, + secret = cred.secret, + subscription = cred.subscription, + tenant = cred.tenant, + created = cred.created, + modified = cred.modified, + created_by_id = cred.created_by_id, + modified_by_id = cred.modified_by_id, + ) for i in orgs[org]: - i.credential = cred + i.credential = new_cred i.save() - _update_credential_parents(org, cred) + + _update_credential_parents(org, new_cred) @log_migration def migrate_credential(apps, schema_editor): @@ -187,7 +210,7 @@ def migrate_credential(apps, schema_editor): if len(results) == 1: _update_credential_parents(results[0].inventory.organization, cred) else: - _discover_credentials(results, cred, attrfunc('inventory.organization')) + _discover_credentials(apps, results, cred, attrfunc('inventory.organization')) logger.info(smart_text(u"added Credential(name={}, kind={}, host={}) at organization level".format(cred.name, cred.kind, cred.host))) projs = Project.objects.filter(credential=cred).all() diff --git a/awx/main/models/credential.py b/awx/main/models/credential.py index d1e6e91d93..07cf77e97d 100644 --- a/awx/main/models/credential.py +++ b/awx/main/models/credential.py @@ -78,6 +78,14 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin): on_delete=models.CASCADE, related_name='deprecated_credentials', ) + organization = models.ForeignKey( + 'Organization', + null=True, + default=None, + blank=True, + on_delete=models.CASCADE, + related_name='credentials', + ) kind = models.CharField( max_length=32, choices=KIND_CHOICES, @@ -209,7 +217,10 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin): ], ) use_role = ImplicitRoleField( - parent_role=['owner_role'] + parent_role=[ + 'organization.admin_role', + 'owner_role', + ] ) read_role = ImplicitRoleField(parent_role=[ 'singleton:' + ROLE_SINGLETON_SYSTEM_AUDITOR, From e243c8319ddfa5016a79d359175094894382f7b1 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Fri, 10 Jun 2016 13:33:23 -0400 Subject: [PATCH 3/4] fix tests with refreshes --- awx/api/urls.py | 1 - awx/main/migrations/_rbac.py | 40 ++++--------------- .../tests/functional/test_rbac_credential.py | 8 ++++ 3 files changed, 16 insertions(+), 33 deletions(-) diff --git a/awx/api/urls.py b/awx/api/urls.py index 28f7f8744a..e22bc61e63 100644 --- a/awx/api/urls.py +++ b/awx/api/urls.py @@ -168,7 +168,6 @@ credential_urls = patterns('awx.api.views', url(r'^(?P[0-9]+)/object_roles/$', 'credential_object_roles_list'), url(r'^(?P[0-9]+)/owner/users/$', 'credential_owner_users_list'), url(r'^(?P[0-9]+)/owner/teams/$', 'credential_owner_teams_list'), - url(r'^(?P[0-9]+)/organization/$', 'credential_owner_teams_list'), # See also credentials resources on users/teams. ) diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index 34f57ed773..0b3c3ccc37 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -126,7 +126,7 @@ def _update_credential_parents(org, cred): cred.organization = org cred.save() -def _discover_credentials(apps, instances, cred, orgfunc): +def _discover_credentials(instances, cred, orgfunc): '''_discover_credentials will find shared credentials across organizations. If a shared credential is found, it will duplicate the credential, ensure the proper role permissions are added to the new @@ -139,8 +139,6 @@ def _discover_credentials(apps, instances, cred, orgfunc): orgfunc is a function that when called with an instance from instances will produce an Organization object. ''' - Credential = apps.get_model('main', "Credential") - orgs = defaultdict(list) for inst in instances: try: @@ -163,38 +161,16 @@ def _discover_credentials(apps, instances, cred, orgfunc): _update_credential_parents(org, cred) else: # Create a new credential - new_cred = Credential.objects.create( - kind = cred.kind, - cloud = cred.cloud, - host = cred.host, - username = cred.username, - password = cred.password, - security_token = cred.security_token, - project = cred.project, - domain = cred.domain, - ssh_key_data = cred.ssh_key_data, - ssh_key_unlock = cred.ssh_key_unlock, - become_method = cred.become_method, - become_username = cred.become_username, - become_password = cred.become_password, - vault_password = cred.vault_password, - authorize = cred.authorize, - authorize_password = cred.authorize_password, - client = cred.client, - secret = cred.secret, - subscription = cred.subscription, - tenant = cred.tenant, - created = cred.created, - modified = cred.modified, - created_by_id = cred.created_by_id, - modified_by_id = cred.modified_by_id, - ) + cred.pk = None + cred.save() + + cred.owner_role, cred.use_role, cred.organization = None, None, None for i in orgs[org]: - i.credential = new_cred + i.credential = cred i.save() - _update_credential_parents(org, new_cred) + _update_credential_parents(org, cred) @log_migration def migrate_credential(apps, schema_editor): @@ -210,7 +186,7 @@ def migrate_credential(apps, schema_editor): if len(results) == 1: _update_credential_parents(results[0].inventory.organization, cred) else: - _discover_credentials(apps, results, cred, attrfunc('inventory.organization')) + _discover_credentials(results, cred, attrfunc('inventory.organization')) logger.info(smart_text(u"added Credential(name={}, kind={}, host={}) at organization level".format(cred.name, cred.kind, cred.host))) projs = Project.objects.filter(credential=cred).all() diff --git a/awx/main/tests/functional/test_rbac_credential.py b/awx/main/tests/functional/test_rbac_credential.py index 75bcffecb6..a42c5b489d 100644 --- a/awx/main/tests/functional/test_rbac_credential.py +++ b/awx/main/tests/functional/test_rbac_credential.py @@ -118,6 +118,9 @@ def test_cred_job_template(user, team, deploy_jobtemplate): access = CredentialAccess(a) rbac.migrate_credential(apps, None) + + cred.refresh_from_db() + assert access.can_change(cred, {'organization': org.pk}) org.admin_role.members.remove(a) @@ -135,6 +138,8 @@ def test_cred_multi_job_template_single_org_xfail(user, deploy_jobtemplate): access = CredentialAccess(a) rbac.migrate_credential(apps, None) + cred.refresh_from_db() + assert not access.can_change(cred, {'organization': org.pk}) @pytest.mark.django_db @@ -149,6 +154,8 @@ def test_cred_multi_job_template_single_org(user, team, deploy_jobtemplate): access = CredentialAccess(a) rbac.migrate_credential(apps, None) + cred.refresh_from_db() + assert access.can_change(cred, {'organization': org.pk}) org.admin_role.members.remove(a) @@ -180,6 +187,7 @@ def test_single_cred_multi_job_template_multi_org(user, organizations, credentia for jt in jts: jt.refresh_from_db() + credential.refresh_from_db() assert jts[0].credential != jts[1].credential assert access.can_change(jts[0].credential, {'organization': org.pk}) From 358998c4b4b221c469671f29ae080c7fc083d344 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Fri, 10 Jun 2016 14:08:52 -0400 Subject: [PATCH 4/4] fix api test --- awx/main/tests/functional/api/test_credential.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/awx/main/tests/functional/api/test_credential.py b/awx/main/tests/functional/api/test_credential.py index afe3bb7f29..e594c95c1d 100644 --- a/awx/main/tests/functional/api/test_credential.py +++ b/awx/main/tests/functional/api/test_credential.py @@ -147,8 +147,7 @@ def test_credential_detail(post, get, organization, org_admin): response = get(reverse('api:credential_detail', args=(response.data['id'],)), org_admin) assert response.status_code == 200 summary_fields = response.data['summary_fields'] - assert 'owners' in summary_fields - assert summary_fields['owners'][0]['id'] == organization.id + assert 'organization' in summary_fields related_fields = response.data['related'] assert 'organization' in related_fields