From d1ea8708ad1d1145a0c15beb26877f160a4a9c9f Mon Sep 17 00:00:00 2001 From: Chris Church Date: Mon, 27 Apr 2015 12:42:40 -0400 Subject: [PATCH] Make sure credential can only be assigned to a user OR team, but never both. Fixes https://trello.com/c/yzlAEfAN --- awx/api/generics.py | 4 ++-- awx/api/serializers.py | 10 ++++++++++ awx/main/models/credential.py | 17 +++++++++++++++++ awx/main/tests/projects.py | 27 ++++++++++++++++++++++++++- 4 files changed, 55 insertions(+), 3 deletions(-) diff --git a/awx/api/generics.py b/awx/api/generics.py index 78071f2222..1ac01a3eb6 100644 --- a/awx/api/generics.py +++ b/awx/api/generics.py @@ -365,7 +365,7 @@ class SubListCreateAPIView(SubListAPIView, ListCreateAPIView): data[parent_key] = self.kwargs['pk'] # attempt to deserialize the object - serializer = self.serializer_class(data=data) + serializer = self.get_serializer(data=data) if not serializer.is_valid(): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) @@ -377,7 +377,7 @@ class SubListCreateAPIView(SubListAPIView, ListCreateAPIView): # save the object through the serializer, reload and returned the saved # object deserialized obj = serializer.save() - serializer = self.serializer_class(obj) + serializer = self.get_serializer(instance=obj) headers = {'Location': obj.get_absolute_url()} return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 5bf92ab2b3..6e361724ea 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1295,6 +1295,16 @@ class CredentialSerializer(BaseSerializer): for field in Credential.PASSWORD_FIELDS: if unicode(attrs.get(field, '')).startswith('$encrypted$'): attrs.pop(field, None) + + # If creating a credential from a view that automatically sets the + # parent_key (user or team), set the other value to None. + view = self.context.get('view', None) + parent_key = getattr(view, 'parent_key', None) + if parent_key == 'user': + attrs['team'] = None + if parent_key == 'team': + attrs['user'] = None + instance = super(CredentialSerializer, self).restore_object(attrs, instance) return instance diff --git a/awx/main/models/credential.py b/awx/main/models/credential.py index 1bce98d643..2975baf6d2 100644 --- a/awx/main/models/credential.py +++ b/awx/main/models/credential.py @@ -379,6 +379,23 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique): # If update_fields has been specified, add our field names to it, # if hit hasn't been specified, then we're just doing a normal save. update_fields = kwargs.get('update_fields', []) + # If updating a credential, make sure that we only allow user OR team + # to be set, and clear out the other field based on which one has + # changed. + if self.pk: + cred_before = Credential.objects.get(pk=self.pk) + if self.user and self.team: + # If the user changed, remove the previously assigned team. + if cred_before.user != self.user: + self.team = None + if 'team' not in update_fields: + update_fields.append('team') + # If the team changed, remove the previously assigned user. + elif cred_before.team != self.team: + self.user = None + if 'user' not in update_fields: + update_fields.append('user') + # Set cloud flag based on credential kind. cloud = self.kind in CLOUD_PROVIDERS + ('aws',) if self.cloud != cloud: self.cloud = cloud diff --git a/awx/main/tests/projects.py b/awx/main/tests/projects.py index b27e4ac710..3792db497b 100644 --- a/awx/main/tests/projects.py +++ b/awx/main/tests/projects.py @@ -486,8 +486,11 @@ class ProjectsTest(BaseTransactionTest): # can add credentials to a user (if user or org admin or super user) self.post(other_creds, data=new_credentials, expect=401) self.post(other_creds, data=new_credentials, expect=401, auth=self.get_invalid_credentials()) + new_credentials['team'] = team.pk result = self.post(other_creds, data=new_credentials, expect=201, auth=self.get_super_credentials()) cred_user = result['id'] + self.assertEqual(result['team'], None) + del new_credentials['team'] new_credentials['name'] = 'credential2' self.post(other_creds, data=new_credentials, expect=201, auth=self.get_normal_credentials()) new_credentials['name'] = 'credential3' @@ -497,9 +500,12 @@ class ProjectsTest(BaseTransactionTest): # can add credentials to a team new_credentials['name'] = 'credential' + new_credentials['user'] = other.pk self.post(team_creds, data=new_credentials, expect=401) self.post(team_creds, data=new_credentials, expect=401, auth=self.get_invalid_credentials()) - self.post(team_creds, data=new_credentials, expect=201, auth=self.get_super_credentials()) + result = self.post(team_creds, data=new_credentials, expect=201, auth=self.get_super_credentials()) + self.assertEqual(result['user'], None) + del new_credentials['user'] new_credentials['name'] = 'credential2' result = self.post(team_creds, data=new_credentials, expect=201, auth=self.get_normal_credentials()) new_credentials['name'] = 'credential3' @@ -611,6 +617,25 @@ class ProjectsTest(BaseTransactionTest): cred_put_t = self.put(edit_creds2, data=d_cred_team, expect=200, auth=self.get_normal_credentials()) self.put(edit_creds2, data=d_cred_team, expect=403, auth=self.get_other_credentials()) + # Reassign credential between team and user. + with self.current_user(self.super_django_user): + self.post(team_creds, data=dict(id=cred_user.pk), expect=204) + response = self.get(edit_creds1) + self.assertEqual(response['team'], team.pk) + self.assertEqual(response['user'], None) + self.post(other_creds, data=dict(id=cred_user.pk), expect=204) + response = self.get(edit_creds1) + self.assertEqual(response['team'], None) + self.assertEqual(response['user'], other.pk) + self.post(other_creds, data=dict(id=cred_team.pk), expect=204) + response = self.get(edit_creds2) + self.assertEqual(response['team'], None) + self.assertEqual(response['user'], other.pk) + self.post(team_creds, data=dict(id=cred_team.pk), expect=204) + response = self.get(edit_creds2) + self.assertEqual(response['team'], team.pk) + self.assertEqual(response['user'], None) + cred_put_t['disassociate'] = 1 team_url = reverse('api:team_credentials_list', args=(cred_put_t['team'],)) self.post(team_url, data=cred_put_t, expect=204, auth=self.get_normal_credentials())