From 695df551e7e8ba5c578d01e71d5115f6a002e29e Mon Sep 17 00:00:00 2001 From: Matthew Jones Date: Thu, 24 Jul 2014 15:19:53 -0400 Subject: [PATCH] Handle cascading mark inactive in better ways as per rbac spec specifically around Teams and Credentials --- awx/main/access.py | 4 +-- awx/main/models/organization.py | 7 ++++++ awx/main/tasks.py | 2 -- awx/main/tests/jobs.py | 44 +++++++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 4 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 91076e8f72..187247416e 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -258,8 +258,8 @@ class InventoryAccess(BaseAccess): qs = qs.select_related('created_by', 'organization') if self.user.is_superuser: return qs - admin_of = qs.filter(organization__admins__in=[self.user], - organization__active=True).distinct() + qs = qs.filter(organization__active=True) + admin_of = qs.filter(organization__admins__in=[self.user]).distinct() has_user_perms = qs.filter( permissions__user__in=[self.user], permissions__permission_type__in=allowed, diff --git a/awx/main/models/organization.py b/awx/main/models/organization.py index bb9cfbed0e..46e7e401b9 100644 --- a/awx/main/models/organization.py +++ b/awx/main/models/organization.py @@ -83,6 +83,13 @@ class Team(CommonModelNameNotUnique): def get_absolute_url(self): return reverse('api:team_detail', args=(self.pk,)) + def mark_inactive(self, save=True): + ''' + When marking a team inactive we'll wipe out its credentials also + ''' + for cred in self.credentials.all(): + cred.mark_inactive() + super(Team, self).mark_inactive(save=save) class Permission(CommonModelNameNotUnique): ''' diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 9a947bb5d4..de0d1c7528 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -46,8 +46,6 @@ HIDDEN_PASSWORD = '**********' logger = logging.getLogger('awx.main.tasks') -# FIXME: Cleanly cancel task when celery worker is stopped. - @task() def bulk_inventory_element_delete(inventory, hosts=[], groups=[]): from awx.main.signals import disable_activity_stream diff --git a/awx/main/tests/jobs.py b/awx/main/tests/jobs.py index d6271a2ba9..b195294123 100644 --- a/awx/main/tests/jobs.py +++ b/awx/main/tests/jobs.py @@ -251,6 +251,23 @@ class BaseJobTestMixin(BaseTestMixin): self.team_ops_west.users.add(self.user_greg) self.team_ops_west.users.add(self.user_iris) + # The south team is no longer active having been folded into the east team + self.team_ops_south = self.org_ops.teams.create( + name='southerners', + created_by=self.user_sue, + active=False, + ) + self.team_ops_south.projects.add(self.proj_prod) + self.team_ops_south.users.add(self.user_greg) + + # The north team is going to be deleted + self.team_ops_north = self.org_ops.teams.create( + name='northerners', + created_by=self.user_sue, + ) + self.team_ops_north.projects.add(self.proj_prod) + self.team_ops_north.users.add(self.user_greg) + # Each user has his/her own set of credentials. from awx.main.tests.tasks import (TEST_SSH_KEY_DATA, TEST_SSH_KEY_DATA_LOCKED, @@ -312,6 +329,17 @@ class BaseJobTestMixin(BaseTestMixin): password='Heading270', created_by = self.user_sue, ) + self.cred_ops_south = self.team_ops_south.credentials.create( + username='south', + password='Heading180', + created_by = self.user_sue, + ) + + self.cred_ops_north = self.team_ops_north.credentials.create( + username='north', + password='Heading0', + created_by = self.user_sue, + ) # FIXME: Define explicit permissions for tests. # other django user is on the project team and can deploy @@ -502,6 +530,20 @@ class JobTemplateTest(BaseJobTestMixin, django.test.TestCase): # FIXME: Check with other credentials. + def test_credentials_list(self): + url = reverse('api:credential_list') + # Greg can't see the 'south' credential because the 'southerns' team is inactive + with self.current_user(self.user_greg): + all_credentials = self.get(url, expect=200) + 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): + self.delete(url2, expect=204) + all_credentials = self.get(url, expect=200) + self.assertFalse('north' in [x['username'] for x in all_credentials['results']]) + def test_post_job_template_list(self): url = reverse('api:job_template_list') data = dict( @@ -686,6 +728,8 @@ class JobTest(BaseJobTestMixin, django.test.TestCase): with self.current_user(self.user_sue): response = self.post(url, data, expect=201) + # sue can't create a job when it is hidden due to inactive team + # FIXME: Check with other credentials and optional fields. def test_get_job_detail(self):