diff --git a/awx/main/access.py b/awx/main/access.py index 49ced43142..1e9b061775 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -146,6 +146,7 @@ class UserAccess(BaseAccess): - I'm a superuser. - I'm that user. - I'm their org admin. + - I'm in an org with that user. - I'm on a team with that user. I can change some fields for a user (mainly password) when I am that user. I can change all fields for a user (admin access) or delete when: @@ -162,6 +163,7 @@ class UserAccess(BaseAccess): return qs.filter( Q(pk=self.user.pk) | Q(organizations__in=self.user.admin_of_organizations.all()) | + Q(organizations__in=self.user.organizations.all()) | Q(teams__in=self.user.teams.all()) ).distinct() @@ -521,6 +523,7 @@ class ProjectAccess(BaseAccess): I can see projects when: - I am a superuser. - I am an admin in an organization associated with the project. + - I am a user in an organization associated with the project. - I am on a team associated with the project. - I have been explicitly granted permission to run/check jobs using the project. @@ -529,7 +532,6 @@ class ProjectAccess(BaseAccess): - I am a superuser. - I am an admin in an organization associated with the project. ''' - # FIXME: Also just a user of the org, or not? model = Project @@ -541,6 +543,7 @@ class ProjectAccess(BaseAccess): return qs.filter( Q(created_by=self.user) | Q(organizations__admins__in=[self.user]) | + Q(organizations__users__in=[self.user]) | Q(teams__users__in=[self.user]) | Q(permissions__user=self.user, permissions__permission_type__in=allowed) | Q(permissions__team__users__in=[self.user], permissions__permission_type__in=allowed) diff --git a/awx/main/base_views.py b/awx/main/base_views.py index 629ea89fd9..756d6fb262 100644 --- a/awx/main/base_views.py +++ b/awx/main/base_views.py @@ -59,7 +59,7 @@ class SubListAPIView(ListAPIView): # relationship = 'rel_name_from_parent_to_model' # And optionally (user must have given access permission on parent object # to view sublist): - # parent_access = 'admin' + # parent_access = 'read' def get_description_vars(self): d = super(SubListAPIView, self).get_description_vars() @@ -81,7 +81,7 @@ class SubListAPIView(ListAPIView): def check_parent_access(self, parent=None): parent = parent or self.get_parent_object() - parent_access = getattr(self, 'parent_access', 'admin') + parent_access = getattr(self, 'parent_access', 'read') if parent_access in ('read', 'delete'): args = (self.parent_model, parent_access, parent) else: diff --git a/awx/main/tests/organizations.py b/awx/main/tests/organizations.py index ddae348d20..6359ab1bb7 100644 --- a/awx/main/tests/organizations.py +++ b/awx/main/tests/organizations.py @@ -42,11 +42,12 @@ class OrganizationsTest(BaseTest): # admin_user is an admin and regular user in all organizations # other_user is all organizations # normal_user is a user in organization 0, and an admin of organization 1 + # nobody_user is a user not a member of any organizations for x in self.organizations: - # NOTE: superuser does not have to be explicitly added to admin group - # x.admins.add(self.super_django_user) + x.admins.add(self.super_django_user) x.users.add(self.super_django_user) + x.users.add(self.other_django_user) self.organizations[0].users.add(self.normal_django_user) self.organizations[1].admins.add(self.normal_django_user) @@ -91,6 +92,11 @@ class OrganizationsTest(BaseTest): # no admin rights? get empty list with self.current_user(self.other_django_user): + response = self.get(url, expect=200) + self.check_pagination_and_size(response, self.other_django_user.organizations.count(), previous=None, next=None) + + # not a member of any orgs? get empty list + with self.current_user(self.nobody_django_user): response = self.get(url, expect=200) self.check_pagination_and_size(response, 0, previous=None, next=None) @@ -112,8 +118,11 @@ class OrganizationsTest(BaseTest): data = self.get(urls[1], expect=200, auth=self.get_normal_credentials()) data = self.get(urls[9], expect=403, auth=self.get_normal_credentials()) - # other user isn't a user or admin of anything, and similarly can't get in - data = self.get(urls[0], expect=403, auth=self.get_other_credentials()) + # other user is a member, but not admin, can access org + data = self.get(urls[0], expect=200, auth=self.get_other_credentials()) + + # nobody user is not a member, cannot access org + data = self.get(urls[0], expect=403, auth=self.get_nobody_credentials()) def test_get_item_subobjects_projects(self): @@ -128,20 +137,23 @@ class OrganizationsTest(BaseTest): self.get(projects0_url, expect=401, auth=None) self.get(projects0_url, expect=401, auth=self.get_invalid_credentials()) - # normal user is just a member of the first org, but can't see any projects under the org - projects0a = self.get(projects0_url, expect=403, auth=self.get_normal_credentials()) + # normal user is just a member of the first org, so can see all projects under the org + projects0a = self.get(projects0_url, expect=200, auth=self.get_normal_credentials()) # however in the second org, he's an admin and should see all of them projects1a = self.get(projects1_url, expect=200, auth=self.get_normal_credentials()) self.assertEquals(projects1a['count'], 5) # but the non-admin cannot access the list of projects in the org. He should use /projects/ instead! - projects1b = self.get(projects1_url, expect=403, auth=self.get_other_credentials()) + projects1b = self.get(projects1_url, expect=200, auth=self.get_other_credentials()) # superuser should be able to read anything projects9a = self.get(projects9_url, expect=200, auth=self.get_super_credentials()) self.assertEquals(projects9a['count'], 1) + # nobody user is not a member of any org, so can't see projects... + projects0a = self.get(projects0_url, expect=403, auth=self.get_nobody_credentials()) + projects1a = self.get(projects1_url, expect=403, auth=self.get_nobody_credentials()) def test_get_item_subobjects_users(self): @@ -149,9 +161,11 @@ class OrganizationsTest(BaseTest): orgs = self.get(self.collection(), expect=200, auth=self.get_super_credentials()) org1_users_url = orgs['results'][1]['related']['users'] org1_users = self.get(org1_users_url, expect=200, auth=self.get_normal_credentials()) - self.assertEquals(org1_users['count'], 1) + self.assertEquals(org1_users['count'], 2) org1_users = self.get(org1_users_url, expect=200, auth=self.get_super_credentials()) - self.assertEquals(org1_users['count'], 1) + self.assertEquals(org1_users['count'], 2) + org1_users = self.get(org1_users_url, expect=200, auth=self.get_other_credentials()) + self.assertEquals(org1_users['count'], 2) def test_get_item_subobjects_admins(self): @@ -159,9 +173,9 @@ class OrganizationsTest(BaseTest): orgs = self.get(self.collection(), expect=200, auth=self.get_super_credentials()) org1_users_url = orgs['results'][1]['related']['admins'] org1_users = self.get(org1_users_url, expect=200, auth=self.get_normal_credentials()) - self.assertEquals(org1_users['count'], 1) + self.assertEquals(org1_users['count'], 2) org1_users = self.get(org1_users_url, expect=200, auth=self.get_super_credentials()) - self.assertEquals(org1_users['count'], 1) + self.assertEquals(org1_users['count'], 2) def test_get_organization_inventories_list(self): pass @@ -268,13 +282,13 @@ class OrganizationsTest(BaseTest): url = reverse('main:organization_users_list', args=(self.organizations[1].pk,)) users = self.get(url, expect=200, auth=self.get_normal_credentials()) - self.assertEqual(users['count'], 1) + self.assertEqual(users['count'], 2) self.post(url, dict(id=self.normal_django_user.pk), expect=204, auth=self.get_normal_credentials()) users = self.get(url, expect=200, auth=self.get_normal_credentials()) - self.assertEqual(users['count'], 2) + self.assertEqual(users['count'], 3) self.post(url, dict(id=self.normal_django_user.pk, disassociate=True), expect=204, auth=self.get_normal_credentials()) users = self.get(url, expect=200, auth=self.get_normal_credentials()) - self.assertEqual(users['count'], 1) + self.assertEqual(users['count'], 2) # post a completely new user to verify we can add users to the subcollection directly new_user = dict(username='NewUser9000') @@ -283,19 +297,19 @@ class OrganizationsTest(BaseTest): posted = self.post(url, new_user, expect=201, auth=self.get_normal_credentials()) all_users = self.get(url, expect=200, auth=self.get_normal_credentials()) - self.assertEqual(all_users['count'], 2) + self.assertEqual(all_users['count'], 3) def test_post_item_subobjects_admins(self): url = reverse('main:organization_admins_list', args=(self.organizations[1].pk,)) admins = self.get(url, expect=200, auth=self.get_normal_credentials()) - self.assertEqual(admins['count'], 1) - self.post(url, dict(id=self.super_django_user.pk), expect=204, auth=self.get_normal_credentials()) + self.assertEqual(admins['count'], 2) + self.post(url, dict(id=self.other_django_user.pk), expect=204, auth=self.get_normal_credentials()) + admins = self.get(url, expect=200, auth=self.get_normal_credentials()) + self.assertEqual(admins['count'], 3) + self.post(url, dict(id=self.other_django_user.pk, disassociate=1), expect=204, auth=self.get_normal_credentials()) admins = self.get(url, expect=200, auth=self.get_normal_credentials()) self.assertEqual(admins['count'], 2) - self.post(url, dict(id=self.super_django_user.pk, disassociate=1), expect=204, auth=self.get_normal_credentials()) - admins = self.get(url, expect=200, auth=self.get_normal_credentials()) - self.assertEqual(admins['count'], 1) def _test_post_item_subobjects_tags(self): # FIXME: Update to support taggit! diff --git a/awx/main/tests/projects.py b/awx/main/tests/projects.py index dc8939b0f0..33979b80ec 100644 --- a/awx/main/tests/projects.py +++ b/awx/main/tests/projects.py @@ -333,7 +333,7 @@ class ProjectsTest(BaseTest): # ===================================================================== # TEAM PROJECTS - team = Team.objects.filter(organization__pk=self.organizations[1].pk)[0] + team = Team.objects.filter(active=True, organization__pk=self.organizations[1].pk)[0] team_projects = reverse('main:team_projects_list', args=(team.pk,)) p1 = self.projects[0] diff --git a/awx/main/tests/users.py b/awx/main/tests/users.py index cd6f3e5efd..6033404672 100644 --- a/awx/main/tests/users.py +++ b/awx/main/tests/users.py @@ -146,7 +146,7 @@ class UsersTest(BaseTest): data2 = self.get(url, expect=200, auth=self.get_normal_credentials()) self.assertEquals(data2['count'], 2) data1 = self.get(url, expect=200, auth=self.get_other_credentials()) - self.assertEquals(data1['count'], 1) + self.assertEquals(data1['count'], 2) def test_super_user_can_delete_a_user_but_only_marked_inactive(self): user_pk = self.normal_django_user.pk @@ -199,8 +199,10 @@ class UsersTest(BaseTest): # also accessible via superuser data = self.get(url, expect=200, auth=self.get_super_credentials()) self.assertEquals(data['count'], 1) - # but not by other user - data = self.get(url, expect=403, auth=self.get_other_credentials()) + # and also by other user... + data = self.get(url, expect=200, auth=self.get_other_credentials()) + # but not by nobody user + data = self.get(url, expect=403, auth=self.get_nobody_credentials()) # organizations the user is an admin of, should be 1 url = reverse('main:user_admin_of_organizations_list', @@ -210,8 +212,10 @@ class UsersTest(BaseTest): # also accessible via superuser data = self.get(url, expect=200, auth=self.get_super_credentials()) self.assertEquals(data['count'], 1) - # but not by other user - data = self.get(url, expect=403, auth=self.get_other_credentials()) + # and also by other user + data = self.get(url, expect=200, auth=self.get_other_credentials()) + # but not by nobody user + data = self.get(url, expect=403, auth=self.get_nobody_credentials()) # teams the user is on, should be 0 url = reverse('main:user_teams_list', args=(self.normal_django_user.pk,)) @@ -220,8 +224,10 @@ class UsersTest(BaseTest): # also accessible via superuser data = self.get(url, expect=200, auth=self.get_super_credentials()) self.assertEquals(data['count'], 0) - # but not by other user - data = self.get(url, expect=403, auth=self.get_other_credentials()) + # and also by other user + data = self.get(url, expect=200, auth=self.get_other_credentials()) + # but not by nobody user + data = self.get(url, expect=403, auth=self.get_nobody_credentials()) # verify org admin can still read other user data too url = reverse('main:user_organizations_list', diff --git a/awx/main/views.py b/awx/main/views.py index 5869cd6454..5decb37460 100644 --- a/awx/main/views.py +++ b/awx/main/views.py @@ -326,7 +326,6 @@ class UserTeamsList(SubListAPIView): serializer_class = TeamSerializer parent_model = User relationship = 'teams' - parent_access = 'read' class UserPermissionsList(SubListCreateAPIView): @@ -335,7 +334,6 @@ class UserPermissionsList(SubListCreateAPIView): parent_model = User relationship = 'permissions' parent_key = 'user' - parent_access = 'read' class UserProjectsList(SubListAPIView): @@ -343,7 +341,6 @@ class UserProjectsList(SubListAPIView): serializer_class = ProjectSerializer parent_model = User relationship = 'projects' - parent_access = 'read' def get_queryset(self): parent = self.get_parent_object() @@ -358,7 +355,6 @@ class UserCredentialsList(SubListCreateAPIView): parent_model = User relationship = 'credentials' parent_key = 'user' - parent_access = 'read' class UserOrganizationsList(SubListAPIView): @@ -366,7 +362,6 @@ class UserOrganizationsList(SubListAPIView): serializer_class = OrganizationSerializer parent_model = User relationship = 'organizations' - parent_access = 'read' class UserAdminOfOrganizationsList(SubListAPIView): @@ -374,7 +369,6 @@ class UserAdminOfOrganizationsList(SubListAPIView): serializer_class = OrganizationSerializer parent_model = User relationship = 'admin_of_organizations' - parent_access = 'read' class UserDetail(RetrieveUpdateDestroyAPIView): @@ -439,7 +433,6 @@ class InventoryHostsList(SubListCreateAPIView): serializer_class = HostSerializer parent_model = Inventory relationship = 'hosts' - parent_access = 'read' parent_key = 'inventory' class HostGroupsList(SubListCreateAPIView): @@ -449,7 +442,6 @@ class HostGroupsList(SubListCreateAPIView): serializer_class = GroupSerializer parent_model = Host relationship = 'groups' - parent_access = 'read' class HostAllGroupsList(SubListAPIView): ''' the list of all groups of which the host is directly or indirectly a member ''' @@ -458,7 +450,6 @@ class HostAllGroupsList(SubListAPIView): serializer_class = GroupSerializer parent_model = Host relationship = 'groups' - parent_access = 'read' def get_queryset(self): parent = self.get_parent_object() @@ -478,7 +469,6 @@ class GroupChildrenList(SubListCreateAPIView): serializer_class = GroupSerializer parent_model = Group relationship = 'children' - parent_access = 'read' class GroupHostsList(SubListCreateAPIView): ''' the list of hosts directly below a group ''' @@ -487,7 +477,6 @@ class GroupHostsList(SubListCreateAPIView): serializer_class = HostSerializer parent_model = Group relationship = 'hosts' - parent_access = 'read' class GroupAllHostsList(SubListAPIView): ''' the list of all hosts below a group, even including subgroups ''' @@ -496,7 +485,6 @@ class GroupAllHostsList(SubListAPIView): serializer_class = HostSerializer parent_model = Group relationship = 'hosts' - parent_access = 'read' def get_queryset(self): parent = self.get_parent_object() @@ -516,7 +504,6 @@ class InventoryGroupsList(SubListCreateAPIView): serializer_class = GroupSerializer parent_model = Inventory relationship = 'groups' - parent_access = 'read' parent_key = 'inventory' class InventoryRootGroupsList(SubListCreateAPIView): @@ -525,7 +512,6 @@ class InventoryRootGroupsList(SubListCreateAPIView): serializer_class = GroupSerializer parent_model = Inventory relationship = 'groups' - parent_access = 'read' parent_key = 'inventory' def get_queryset(self): @@ -837,7 +823,6 @@ class BaseJobHostSummariesList(SubListAPIView): serializer_class = JobHostSummarySerializer parent_model = None # Subclasses must define this attribute. relationship = 'job_host_summaries' - parent_access = 'read' view_name = 'Job Host Summary List' @@ -892,7 +877,6 @@ class BaseJobEventsList(SubListAPIView): serializer_class = JobEventSerializer parent_model = None # Subclasses must define this attribute. relationship = 'job_events' - parent_access = 'read' class HostJobEventsList(BaseJobEventsList):