From 29b55fa04dd899cf0e1c0c20f60eff719275af47 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Fri, 29 Apr 2016 17:27:14 -0400 Subject: [PATCH] Moved access control from credential add view to access.py as it should have always been. This messes up being able to post to api/v1/users/:n/credentials and api/v1/teams/:n/credentials without specifyign the user/team id in the post body, but looking at the old code it looks like this might have always been the case, so whatevs.. This fixes a old v new access.py test "failure", and is better anyways.. --- awx/api/views.py | 14 ++++++-------- awx/main/access.py | 18 ++++++++++++++++-- .../tests/functional/api/test_credential.py | 3 +++ 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/awx/api/views.py b/awx/api/views.py index 4ea256c246..8aebdbf9f1 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -1232,15 +1232,15 @@ class CredentialList(ListCreateAPIView): if 'user' in request.data: user = User.objects.get(pk=request.data['user']) - obj = user + can_add_params = {'user': user.id} if 'team' in request.data: team = Team.objects.get(pk=request.data['team']) - obj = team + can_add_params = {'team': team.id} if 'organization' in request.data: organization = Organization.objects.get(pk=request.data['organization']) - obj = organization + can_add_params = {'organization': organization.id} - if not self.request.user.can_access(type(obj), 'change', obj, request.data): + if not self.request.user.can_access(Credential, 'add', can_add_params): raise PermissionDenied() ret = super(CredentialList, self).post(request, *args, **kwargs) @@ -1270,8 +1270,7 @@ class UserCredentialsList(CredentialList): return user_creds & visible_creds def post(self, request, *args, **kwargs): - user = User.objects.get(pk=self.kwargs['pk']) - request.data['user'] = user.id + request.data['user'] = self.kwargs['pk'] # The following post takes care of ensuring the current user can add a cred to this user return super(UserCredentialsList, self).post(request, args, kwargs) @@ -1290,8 +1289,7 @@ class TeamCredentialsList(CredentialList): return team_creds & visible_creds def post(self, request, *args, **kwargs): - team = Team.objects.get(pk=self.kwargs['pk']) - request.data['team'] = team.id + request.data['team'] = self.kwargs['pk'] # The following post takes care of ensuring the current user can add a cred to this user return super(TeamCredentialsList, self).post(request, args, kwargs) diff --git a/awx/main/access.py b/awx/main/access.py index ec1ad16317..bd27bbd6a3 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -572,8 +572,22 @@ class CredentialAccess(BaseAccess): return self.user in obj.read_role def can_add(self, data): - # Access enforced in our view where we have context enough to make a decision - return True + if self.user.is_superuser: + return True + user_pk = get_pk_from_dict(data, 'user') + if user_pk: + user_obj = get_object_or_400(User, pk=user_pk) + return check_user_access(self.user, User, 'change', user_obj, None) + team_pk = get_pk_from_dict(data, 'team') + if team_pk: + team_obj = get_object_or_400(Team, pk=team_pk) + return check_user_access(self.user, Team, 'change', team_obj, None) + organization_pk = get_pk_from_dict(data, 'organization') + if organization_pk: + organization_obj = get_object_or_400(Organization, pk=organization_pk) + return check_user_access(self.user, Organization, 'change', organization_obj, None) + return False + @check_superuser def can_use(self, obj): diff --git a/awx/main/tests/functional/api/test_credential.py b/awx/main/tests/functional/api/test_credential.py index 24b653b863..fffab6f1a0 100644 --- a/awx/main/tests/functional/api/test_credential.py +++ b/awx/main/tests/functional/api/test_credential.py @@ -24,6 +24,7 @@ def test_create_user_credential_via_credentials_list(post, get, alice): @pytest.mark.django_db def test_create_user_credential_via_user_credentials_list(post, get, alice): response = post(reverse('api:user_credentials_list', args=(alice.pk,)), { + 'user': alice.pk, 'name': 'Some name', 'username': 'someusername', }, alice) @@ -45,6 +46,7 @@ def test_create_user_credential_via_credentials_list_xfail(post, alice, bob): @pytest.mark.django_db def test_create_user_credential_via_user_credentials_list_xfail(post, alice, bob): response = post(reverse('api:user_credentials_list', args=(bob.pk,)), { + 'user': bob.pk, 'name': 'Some name', 'username': 'someusername' }, alice) @@ -71,6 +73,7 @@ def test_create_team_credential(post, get, team, org_admin, team_member): @pytest.mark.django_db def test_create_team_credential_via_team_credentials_list(post, get, team, org_admin, team_member): response = post(reverse('api:team_credentials_list', args=(team.pk,)), { + 'team': team.pk, 'name': 'Some name', 'username': 'someusername', }, org_admin)