diff --git a/awx/api/serializers.py b/awx/api/serializers.py index bd0d3d1ea3..c9e084c193 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1547,7 +1547,7 @@ class CredentialSerializer(BaseSerializer): class Meta: model = Credential - fields = ('*', 'user', 'team', 'kind', 'cloud', 'host', 'username', + fields = ('*', 'deprecated_user', 'deprecated_team', 'kind', 'cloud', 'host', 'username', 'password', 'security_token', 'project', 'ssh_key_data', 'ssh_key_unlock', 'become_method', 'become_username', 'become_password', 'vault_password') @@ -1562,21 +1562,16 @@ class CredentialSerializer(BaseSerializer): def to_representation(self, obj): ret = super(CredentialSerializer, self).to_representation(obj) - if obj is not None and 'user' in ret and not obj.user: - ret['user'] = None - if obj is not None and 'team' in ret and not obj.team: - ret['team'] = None + if obj is not None and 'deprecated_user' in ret and not obj.deprecated_user: + ret['deprecated_user'] = None + if obj is not None and 'deprecated_team' in ret and not obj.deprecated_team: + ret['deprecated_team'] = None return ret def validate(self, attrs): - # 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 + # Ensure old style assignment for user/team is always None + attrs['deprecated_user'] = None + attrs['deprecated_team'] = None return super(CredentialSerializer, self).validate(attrs) diff --git a/awx/main/migrations/0007_v300_rbac_changes.py b/awx/main/migrations/0007_v300_rbac_changes.py index c05a1ea4ff..b847b7eff5 100644 --- a/awx/main/migrations/0007_v300_rbac_changes.py +++ b/awx/main/migrations/0007_v300_rbac_changes.py @@ -220,4 +220,41 @@ class Migration(migrations.Migration): name='organization', field=models.ForeignKey(related_name='projects', to='main.Organization', blank=True, null=True), ), + migrations.AddField( + model_name='credential', + name='deprecated_team', + field=models.ForeignKey(related_name='deprecated_credentials', default=None, blank=True, to='main.Team', null=True), + ), + migrations.AddField( + model_name='credential', + name='deprecated_user', + field=models.ForeignKey(related_name='deprecated_credentials', default=None, blank=True, to=settings.AUTH_USER_MODEL, null=True), + ), + migrations.AlterField( + model_name='organization', + name='deprecated_admins', + field=models.ManyToManyField(related_name='deprecated_admin_of_organizations', to=settings.AUTH_USER_MODEL, blank=True), + ), + migrations.AlterField( + model_name='organization', + name='deprecated_users', + field=models.ManyToManyField(related_name='deprecated_organizations', to=settings.AUTH_USER_MODEL, blank=True), + ), + migrations.AlterField( + model_name='team', + name='deprecated_users', + field=models.ManyToManyField(related_name='deprecated_teams', to=settings.AUTH_USER_MODEL, blank=True), + ), + migrations.AlterUniqueTogether( + name='credential', + unique_together=set([]), + ), + migrations.RemoveField( + model_name='credential', + name='team', + ), + migrations.RemoveField( + model_name='credential', + name='user', + ), ] diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index a333ff0233..4731df886b 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -70,7 +70,7 @@ def attrfunc(attr_path): def _update_credential_parents(org, cred): org.admin_role.children.add(cred.owner_role) org.member_role.children.add(cred.usage_role) - cred.user, cred.team = None, None + cred.deprecated_user, cred.deprecated_team = None, None cred.save() def _discover_credentials(instances, cred, orgfunc): @@ -102,7 +102,7 @@ def _discover_credentials(instances, cred, orgfunc): cred.save() # Unlink the old information from the new credential - cred.user, cred.team = None, None + cred.deprecated_user, cred.deprecated_team = None, None cred.owner_role, cred.usage_role = None, None cred.save() @@ -138,15 +138,15 @@ def migrate_credential(apps, schema_editor): _discover_credentials(projs, cred, attrfunc('organization')) continue - if cred.team is not None: - cred.team.admin_role.children.add(cred.owner_role) - cred.team.member_role.children.add(cred.usage_role) - cred.user, cred.team = None, None + if cred.deprecated_team is not None: + cred.deprecated_team.admin_role.children.add(cred.owner_role) + cred.deprecated_team.member_role.children.add(cred.usage_role) + cred.deprecated_user, cred.deprecated_team = None, None cred.save() - elif cred.user is not None: - cred.user.admin_role.children.add(cred.owner_role) - cred.user, cred.team = None, None + elif cred.deprecated_user is not None: + cred.deprecated_user.admin_role.children.add(cred.owner_role) + cred.deprecated_user, cred.deprecated_team = None, None cred.save() # no match found, log diff --git a/awx/main/models/credential.py b/awx/main/models/credential.py index 1d59049326..6d4325f1d9 100644 --- a/awx/main/models/credential.py +++ b/awx/main/models/credential.py @@ -7,7 +7,7 @@ import re # Django from django.db import models from django.utils.translation import ugettext_lazy as _ -from django.core.exceptions import ValidationError, NON_FIELD_ERRORS +from django.core.exceptions import ValidationError from django.core.urlresolvers import reverse # AWX @@ -56,24 +56,23 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin): class Meta: app_label = 'main' - unique_together = [('user', 'team', 'kind', 'name')] ordering = ('kind', 'name') - user = models.ForeignKey( + deprecated_user = models.ForeignKey( 'auth.User', null=True, default=None, blank=True, on_delete=models.CASCADE, - related_name='credentials', + related_name='deprecated_credentials', ) - team = models.ForeignKey( + deprecated_team = models.ForeignKey( 'Team', null=True, default=None, blank=True, on_delete=models.CASCADE, - related_name='credentials', + related_name='deprecated_credentials', ) kind = models.CharField( max_length=32, @@ -294,57 +293,9 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin): return self.ssh_key_unlock def clean(self): - if self.user and self.team: + if self.deprecated_user and self.deprecated_team: raise ValidationError('Credential cannot be assigned to both a user and team') - def _validate_unique_together_with_null(self, unique_check, exclude=None): - # Based on existing Django model validation code, except it doesn't - # skip the check for unique violations when a field is None. See: - # https://github.com/django/django/blob/stable/1.5.x/django/db/models/base.py#L792 - errors = {} - model_class = self.__class__ - if set(exclude or []) & set(unique_check): - return - lookup_kwargs = {} - for field_name in unique_check: - f = self._meta.get_field(field_name) - lookup_value = getattr(self, f.attname) - if f.primary_key and not self._state.adding: - # no need to check for unique primary key when editing - continue - lookup_kwargs[str(field_name)] = lookup_value - if len(unique_check) != len(lookup_kwargs): - return - qs = model_class._default_manager.filter(**lookup_kwargs) - # Exclude the current object from the query if we are editing an - # instance (as opposed to creating a new one) - # Note that we need to use the pk as defined by model_class, not - # self.pk. These can be different fields because model inheritance - # allows single model to have effectively multiple primary keys. - # Refs #17615. - model_class_pk = self._get_pk_val(model_class._meta) - if not self._state.adding and model_class_pk is not None: - qs = qs.exclude(pk=model_class_pk) - if qs.exists(): - key = NON_FIELD_ERRORS - errors.setdefault(key, []).append(self.unique_error_message(model_class, unique_check)) - if errors: - raise ValidationError(errors) - - def validate_unique(self, exclude=None): - errors = {} - try: - super(Credential, self).validate_unique(exclude) - except ValidationError, e: - errors = e.update_error_dict(errors) - try: - unique_fields = ('user', 'team', 'kind', 'name') - self._validate_unique_together_with_null(unique_fields, exclude) - except ValidationError, e: - errors = e.update_error_dict(errors) - if errors: - raise ValidationError(errors) - def _password_field_allows_ask(self, field): return bool(self.kind == 'ssh' and field != 'ssh_key_data') @@ -357,17 +308,17 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin): # changed. if self.pk: cred_before = Credential.objects.get(pk=self.pk) - if self.user and self.team: + if self.deprecated_user and self.deprecated_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') + self.deprecated_team = None + if 'deprecated_team' not in update_fields: + update_fields.append('deprecated_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') + elif cred_before.deprecated_team != self.deprecated_team: + self.deprecated_user = None + if 'deprecated_user' not in update_fields: + update_fields.append('deprecated_user') # Set cloud flag based on credential kind. cloud = self.kind in CLOUD_PROVIDERS + ('aws',) if self.cloud != cloud: diff --git a/awx/main/tests/functional/test_rbac_credential.py b/awx/main/tests/functional/test_rbac_credential.py index f4ea00e68c..a63e3ba888 100644 --- a/awx/main/tests/functional/test_rbac_credential.py +++ b/awx/main/tests/functional/test_rbac_credential.py @@ -11,7 +11,7 @@ from django.contrib.auth.models import User @pytest.mark.django_db def test_credential_migration_user(credential, user, permissions): u = user('user', False) - credential.user = u + credential.deprecated_user = u credential.save() migrated = rbac.migrate_credential(apps, None) @@ -29,7 +29,7 @@ def test_credential_usage_role(credential, user, permissions): def test_credential_migration_team_member(credential, team, user, permissions): u = user('user', False) team.admin_role.members.add(u) - credential.team = team + credential.deprecated_team = team credential.save() @@ -48,7 +48,7 @@ def test_credential_migration_team_member(credential, team, user, permissions): def test_credential_migration_team_admin(credential, team, user, permissions): u = user('user', False) team.member_role.members.add(u) - credential.team = team + credential.deprecated_team = team credential.save() assert not credential.accessible_by(u, permissions['usage']) @@ -88,7 +88,7 @@ def test_credential_access_admin(user, team, credential): credential.owner_role.rebuild_role_ancestor_list() cred = Credential.objects.create(kind='aws', name='test-cred') - cred.team = team + cred.deprecated_team = team cred.save() # should have can_change access as org-admin @@ -101,7 +101,7 @@ def test_cred_job_template(user, deploy_jobtemplate): org.admin_role.members.add(a) cred = deploy_jobtemplate.credential - cred.user = user('john', False) + cred.deprecated_user = user('john', False) cred.save() access = CredentialAccess(a) @@ -118,7 +118,7 @@ def test_cred_multi_job_template_single_org(user, deploy_jobtemplate): org.admin_role.members.add(a) cred = deploy_jobtemplate.credential - cred.user = user('john', False) + cred.deprecated_user = user('john', False) cred.save() access = CredentialAccess(a) @@ -197,7 +197,7 @@ def test_cred_no_org(user, credential): def test_cred_team(user, team, credential): u = user('a', False) team.member_role.members.add(u) - credential.team = team + credential.deprecated_team = team credential.save() assert not credential.accessible_by(u, {'use':True})