Make sure credential can only be assigned to a user OR team, but never both. Fixes https://trello.com/c/yzlAEfAN

This commit is contained in:
Chris Church
2015-04-27 12:42:40 -04:00
parent 0f30ccc9b7
commit d1ea8708ad
4 changed files with 55 additions and 3 deletions

View File

@@ -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)

View File

@@ -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

View File

@@ -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

View File

@@ -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())