From 02b7149cadd71a317f34303f68c1abca2cb63175 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 23 Mar 2016 17:13:43 -0400 Subject: [PATCH 1/8] Org counts in detail view and test --- awx/api/views.py | 25 +++++++++++++++++++ .../api/test_organization_counts.py | 19 ++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/awx/api/views.py b/awx/api/views.py index b50ba0497c..a243fa16e7 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -691,6 +691,31 @@ class OrganizationDetail(RetrieveUpdateDestroyAPIView): model = Organization serializer_class = OrganizationSerializer + def get_serializer_context(self, *args, **kwargs): + full_context = super(OrganizationDetail, self).get_serializer_context(*args, **kwargs) + + if not hasattr(self, 'kwargs'): + return full_context + org_id = int(self.kwargs['pk']) + + org_counts = {} + user_qs = self.request.user.get_queryset(User) + org_counts['users'] = user_qs.filter(organizations__id=org_id).count() + org_counts['admins'] = user_qs.filter(admin_of_organizations__id=org_id).count() + org_counts['inventories'] = self.request.user.get_queryset(Inventory).filter( + organization__id=org_id).count() + org_counts['teams'] = self.request.user.get_queryset(Team).filter( + organization__id=org_id).count() + org_counts['projects'] = self.request.user.get_queryset(Project).filter( + organizations__id=org_id).count() + org_counts['job_templates'] = self.request.user.get_queryset(JobTemplate).filter( + inventory__organization__id=org_id).count() + + full_context['related_field_counts'] = {} + full_context['related_field_counts'][org_id] = org_counts + + return full_context + class OrganizationInventoriesList(SubListAPIView): model = Inventory diff --git a/awx/main/tests/functional/api/test_organization_counts.py b/awx/main/tests/functional/api/test_organization_counts.py index 8d881fe8a0..6d016decd0 100644 --- a/awx/main/tests/functional/api/test_organization_counts.py +++ b/awx/main/tests/functional/api/test_organization_counts.py @@ -20,6 +20,25 @@ def resourced_organization(organization, project, team, inventory, user): return organization + +@pytest.mark.django_db +def test_org_counts_detail_view(resourced_organization, user, get): + # Check that all types of resources are counted by a superuser + external_admin = user('admin', True) + response = get(reverse('api:organization_detail', args=[resourced_organization.pk]), + external_admin) + assert response.status_code == 200 + + counts = response.data['summary_fields']['related_field_counts'] + assert counts == { + 'users': 1, + 'admins': 1, + 'job_templates': 1, + 'projects': 1, + 'inventories': 1, + 'teams': 1 + } + @pytest.mark.django_db def test_org_counts_admin(resourced_organization, user, get): # Check that all types of resources are counted by a superuser From 39f444836b4c354404edb51a0b8c683025ad008c Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 23 Mar 2016 17:25:27 -0400 Subject: [PATCH 2/8] flake8 fix --- awx/main/tests/functional/api/test_organization_counts.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/awx/main/tests/functional/api/test_organization_counts.py b/awx/main/tests/functional/api/test_organization_counts.py index 6d016decd0..6f785cf25f 100644 --- a/awx/main/tests/functional/api/test_organization_counts.py +++ b/awx/main/tests/functional/api/test_organization_counts.py @@ -25,8 +25,8 @@ def resourced_organization(organization, project, team, inventory, user): def test_org_counts_detail_view(resourced_organization, user, get): # Check that all types of resources are counted by a superuser external_admin = user('admin', True) - response = get(reverse('api:organization_detail', args=[resourced_organization.pk]), - external_admin) + response = get(reverse('api:organization_detail', + args=[resourced_organization.pk]), external_admin) assert response.status_code == 200 counts = response.data['summary_fields']['related_field_counts'] From 1dea6610a7409f73c7e0c56490c8b5d278b66227 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 24 Mar 2016 15:35:37 -0400 Subject: [PATCH 3/8] Fixed up tasks.py for credential changes --- awx/main/tests/old/tasks.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/awx/main/tests/old/tasks.py b/awx/main/tests/old/tasks.py index 30f58353c2..28e586376a 100644 --- a/awx/main/tests/old/tasks.py +++ b/awx/main/tests/old/tasks.py @@ -279,7 +279,10 @@ class RunJobTest(BaseJobExecutionTest): 'password': '', } opts.update(kwargs) + user = opts['user'] + del opts['user'] self.cloud_credential = Credential.objects.create(**opts) + self.cloud_credential.owner_role.members.add(user) return self.cloud_credential def create_test_project(self, playbook_content, role_playbooks=None): From f9a1e373718a525dea625ac117a99b14969dfab8 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 24 Mar 2016 16:04:22 -0400 Subject: [PATCH 4/8] projects.py test fixes --- awx/main/tests/old/projects.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/awx/main/tests/old/projects.py b/awx/main/tests/old/projects.py index 1d2816d6b6..b6b75ecd4b 100644 --- a/awx/main/tests/old/projects.py +++ b/awx/main/tests/old/projects.py @@ -236,6 +236,7 @@ class ProjectsTest(BaseTransactionTest): 'scm_update_on_launch': '', 'scm_delete_on_update': None, 'scm_clean': False, + 'organization': self.organizations[0].pk, } # Adding a project with scm_type=None should work, but scm_type will be # changed to an empty string. Other boolean fields should accept null @@ -502,7 +503,10 @@ class ProjectUpdatesTest(BaseTransactionTest): kw[field.replace('scm_key_', 'ssh_key_')] = kwargs.pop(field) else: kw[field.replace('scm_', '')] = kwargs.pop(field) + u = kw['user'] + del kw['user'] credential = Credential.objects.create(**kw) + credential.owner_role.members.add(u) kwargs['credential'] = credential project = Project.objects.create(**kwargs) project_path = project.get_project_path(check_if_exists=False) @@ -952,11 +956,13 @@ class ProjectUpdatesTest(BaseTransactionTest): self.skipTest('no public git repo defined for https!') projects_url = reverse('api:project_list') credentials_url = reverse('api:credential_list') + org = self.make_organizations(self.super_django_user, 1)[0] # Test basic project creation without a credential. project_data = { 'name': 'my public git project over https', 'scm_type': 'git', 'scm_url': scm_url, + 'organization': org.id, } with self.current_user(self.super_django_user): self.post(projects_url, project_data, expect=201) @@ -965,6 +971,7 @@ class ProjectUpdatesTest(BaseTransactionTest): 'name': 'my local git project', 'scm_type': 'git', 'scm_url': 'file:///path/to/repo.git', + 'organization': org.id, } with self.current_user(self.super_django_user): self.post(projects_url, project_data, expect=400) @@ -984,6 +991,7 @@ class ProjectUpdatesTest(BaseTransactionTest): 'scm_type': 'git', 'scm_url': scm_url, 'credential': credential_id, + 'organization': org.id, } with self.current_user(self.super_django_user): self.post(projects_url, project_data, expect=201) @@ -1004,6 +1012,7 @@ class ProjectUpdatesTest(BaseTransactionTest): 'scm_type': 'git', 'scm_url': scm_url, 'credential': ssh_credential_id, + 'organization': org.id, } with self.current_user(self.super_django_user): self.post(projects_url, project_data, expect=400) @@ -1013,6 +1022,7 @@ class ProjectUpdatesTest(BaseTransactionTest): 'scm_type': 'git', 'scm_url': 'ssh://git@github.com/ansible/ansible.github.com.git', 'credential': credential_id, + 'organization': org.id, } with self.current_user(self.super_django_user): self.post(projects_url, project_data, expect=201) @@ -1023,12 +1033,13 @@ class ProjectUpdatesTest(BaseTransactionTest): if not all([scm_url]): self.skipTest('no public git repo defined for https!') projects_url = reverse('api:project_list') + org = self.make_organizations(self.super_django_user, 1)[0] project_data = { 'name': 'my public git project over https', 'scm_type': 'git', 'scm_url': scm_url, + 'organization': org.id, } - org = self.make_organizations(self.super_django_user, 1)[0] org.admin_role.members.add(self.normal_django_user) with self.current_user(self.super_django_user): del_proj = self.post(projects_url, project_data, expect=201) @@ -1406,8 +1417,8 @@ class ProjectUpdatesTest(BaseTransactionTest): self.group = self.inventory.groups.create(name='test-group', inventory=self.inventory) self.group.hosts.add(self.host) - self.credential = Credential.objects.create(name='test-creds', - user=self.super_django_user) + self.credential = Credential.objects.create(name='test-creds') + self.credential.owner_role.members.add(self.super_django_user) self.project = self.create_project( name='my public git project over https', scm_type='git', @@ -1442,8 +1453,8 @@ class ProjectUpdatesTest(BaseTransactionTest): self.group = self.inventory.groups.create(name='test-group', inventory=self.inventory) self.group.hosts.add(self.host) - self.credential = Credential.objects.create(name='test-creds', - user=self.super_django_user) + self.credential = Credential.objects.create(name='test-creds') + self.credential.owner_role.members.add(self.super_django_user) self.project = self.create_project( name='my private git project over https', scm_type='git', From f8656b38e705c9324e39a513ac9f785a5dd158b6 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Thu, 24 Mar 2016 16:38:45 -0400 Subject: [PATCH 5/8] implemented RBAC version of org detail counts, test passes --- awx/api/views.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/awx/api/views.py b/awx/api/views.py index 4742170f8f..abcd06fc85 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -650,17 +650,21 @@ class OrganizationDetail(RetrieveUpdateDestroyAPIView): org_id = int(self.kwargs['pk']) org_counts = {} - user_qs = self.request.user.get_queryset(User) - org_counts['users'] = user_qs.filter(organizations__id=org_id).count() - org_counts['admins'] = user_qs.filter(admin_of_organizations__id=org_id).count() - org_counts['inventories'] = self.request.user.get_queryset(Inventory).filter( + access_kwargs = {'accessor': self.request.user, 'permissions': {"read": True}} + direct_counts = Organization.objects.filter(id=org_id).annotate( + users=Count('member_role__members', distinct=True), + admins=Count('admin_role__members', distinct=True) + ).values('users', 'admins') + + org_counts = direct_counts[0] + org_counts['inventories'] = Inventory.accessible_objects(**access_kwargs).filter( organization__id=org_id).count() - org_counts['teams'] = self.request.user.get_queryset(Team).filter( + org_counts['teams'] = Team.accessible_objects(**access_kwargs).filter( organization__id=org_id).count() - org_counts['projects'] = self.request.user.get_queryset(Project).filter( - organizations__id=org_id).count() - org_counts['job_templates'] = self.request.user.get_queryset(JobTemplate).filter( - inventory__organization__id=org_id).count() + org_counts['projects'] = Project.accessible_objects(**access_kwargs).filter( + organization__id=org_id).count() + org_counts['job_templates'] = JobTemplate.accessible_objects(**access_kwargs).filter( + project__organization__id=org_id).count() full_context['related_field_counts'] = {} full_context['related_field_counts'][org_id] = org_counts From c900c9126c77335687b425eb364b57b8b3e5049c Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 24 Mar 2016 18:34:24 -0400 Subject: [PATCH 6/8] missing .add --- awx/main/tests/job_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/tests/job_base.py b/awx/main/tests/job_base.py index 9312acd48c..2640d29365 100644 --- a/awx/main/tests/job_base.py +++ b/awx/main/tests/job_base.py @@ -391,7 +391,7 @@ class BaseJobTestMixin(BaseTestMixin): password='HeadingNone', created_by = self.user_sue, ) - self.team_ops_testers.member_role.children(self.cred_ops_test.usage_role) + self.team_ops_testers.member_role.children.add(self.cred_ops_test.usage_role) self.ops_east_permission = Permission.objects.create( inventory = self.inv_ops_east, From c89a549cbdd7a8d63c90a1460b2ec2059cc4dc85 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 24 Mar 2016 20:31:01 -0400 Subject: [PATCH 7/8] Removed deprecated user/team from select_related CredentialAccess queryset --- awx/main/access.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/access.py b/awx/main/access.py index 7b7cb70660..79f70032df 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -547,7 +547,7 @@ class CredentialAccess(BaseAccess): permitted to see. """ qs = self.model.accessible_objects(self.user, {'read':True}) - qs = qs.select_related('created_by', 'modified_by', 'user', 'team') + qs = qs.select_related('created_by', 'modified_by') return qs def can_add(self, data): From 453772f62c7c8a99ee13c78822eede368f1a43fa Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Thu, 24 Mar 2016 21:16:15 -0400 Subject: [PATCH 8/8] Fixed up credential viewability expectations in jobs_monlithic tests Super users can now see all credentials always.. Adjusted test to test for this, as well as original test intent which was to test to ensure after removing a team which has access to a credential, members of that team no longer have access to the credential. --- awx/main/tests/job_base.py | 2 +- awx/main/tests/old/jobs/jobs_monolithic.py | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/awx/main/tests/job_base.py b/awx/main/tests/job_base.py index 2640d29365..9cea21e2cd 100644 --- a/awx/main/tests/job_base.py +++ b/awx/main/tests/job_base.py @@ -384,7 +384,7 @@ class BaseJobTestMixin(BaseTestMixin): password='Heading0', created_by = self.user_sue, ) - self.team_ops_north.member_role.children.add(self.cred_ops_north.usage_role) + self.team_ops_north.member_role.children.add(self.cred_ops_north.owner_role) self.cred_ops_test = Credential.objects.create( username='testers', diff --git a/awx/main/tests/old/jobs/jobs_monolithic.py b/awx/main/tests/old/jobs/jobs_monolithic.py index 6b6d25681e..e174fd55d5 100644 --- a/awx/main/tests/old/jobs/jobs_monolithic.py +++ b/awx/main/tests/old/jobs/jobs_monolithic.py @@ -281,11 +281,17 @@ class JobTemplateTest(BaseJobTestMixin, django.test.TransactionTestCase): self.assertFalse('south' in [x['username'] for x in all_credentials['results']]) url2 = reverse('api:team_detail', args=(self.team_ops_north.id,)) - # Sue shouldn't be able to see the north credential once deleting its team - with self.current_user(self.user_sue): + # Greg shouldn't be able to see the north credential once deleting its team + with self.current_user(self.user_greg): + all_credentials = self.get(url, expect=200) + self.assertTrue('north' in [x['username'] for x in all_credentials['results']]) self.delete(url2, expect=204) all_credentials = self.get(url, expect=200) self.assertFalse('north' in [x['username'] for x in all_credentials['results']]) + # Sue can still see the credential, she's a super user + with self.current_user(self.user_sue): + all_credentials = self.get(url, expect=200) + self.assertTrue('north' in [x['username'] for x in all_credentials['results']]) def test_post_job_template_list(self): self.skipTest('This test makes assumptions about projects being multi-org and needs to be updated/rewritten')