diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 75457c1eb6..038c607579 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1553,7 +1553,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') @@ -1568,21 +1568,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/api/views.py b/awx/api/views.py index a3c2280c7d..53ca2ef7f5 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -820,11 +820,11 @@ class TeamActivityStreamList(SubListAPIView): def get_queryset(self): parent = self.get_parent_object() self.check_parent_access(parent) + qs = self.request.user.get_queryset(self.model) return qs.filter(Q(team=parent) | - Q(project__in=parent.projects.all()) | - Q(credential__in=parent.credentials.all()) | - Q(permission__in=parent.permissions.all())) + Q(project__in=Project.accessible_objects(parent, {'read':True})) | + Q(credential__in=Credential.accessible_objects(parent, {'read':True}))) class TeamAccessList(ResourceAccessList): diff --git a/awx/main/migrations/0007_v300_rbac_changes.py b/awx/main/migrations/0007_v300_rbac_changes.py index e7e8624780..cfe8432da9 100644 --- a/awx/main/migrations/0007_v300_rbac_changes.py +++ b/awx/main/migrations/0007_v300_rbac_changes.py @@ -225,4 +225,33 @@ class Migration(migrations.Migration): name='organization', field=models.ForeignKey(related_name='projects', to='main.Organization', blank=True, null=True), ), + migrations.RenameField( + 'Credential', + 'team', + 'deprecated_team', + ), + migrations.RenameField( + 'Credential', + 'user', + 'deprecated_user', + ), + 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([]), + ), ] diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index 3823dff1b3..e28d642d7b 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/models/mixins.py b/awx/main/models/mixins.py index 639611fbca..0160ca9be5 100644 --- a/awx/main/models/mixins.py +++ b/awx/main/models/mixins.py @@ -2,11 +2,14 @@ from django.db import models from django.db.models.aggregates import Max from django.contrib.contenttypes.fields import GenericRelation +from django.contrib.contenttypes.models import ContentType +from django.contrib.auth.models import User # noqa # AWX from awx.main.models.rbac import ( get_user_permissions_on_resource, get_role_permissions_on_resource, + Role, ) @@ -20,7 +23,7 @@ class ResourceMixin(models.Model): role_permissions = GenericRelation('main.RolePermission') @classmethod - def accessible_objects(cls, user, permissions): + def accessible_objects(cls, accessor, permissions): ''' Use instead of `MyModel.objects` when you want to only consider resources that a user has specific permissions for. For example: @@ -32,13 +35,22 @@ class ResourceMixin(models.Model): performant to resolve the resource in question then call `myresource.get_permissions(user)`. ''' - return ResourceMixin._accessible_objects(cls, user, permissions) + return ResourceMixin._accessible_objects(cls, accessor, permissions) @staticmethod - def _accessible_objects(cls, user, permissions): - qs = cls.objects.filter( - role_permissions__role__ancestors__members=user - ) + def _accessible_objects(cls, accessor, permissions): + if type(accessor) == User: + qs = cls.objects.filter( + role_permissions__role__ancestors__members=accessor + ) + else: + accessor_type = ContentType.objects.get_for_model(accessor) + roles = Role.objects.filter(content_type__pk=accessor_type.id, + object_id=accessor.id) + qs = cls.objects.filter( + role_permissions__role__ancestors__in=roles + ) + for perm in permissions: qs = qs.annotate(**{'max_' + perm: Max('role_permissions__' + perm)}) qs = qs.filter(**{'max_' + perm: int(permissions[perm])}) diff --git a/awx/main/models/rbac.py b/awx/main/models/rbac.py index 644ffa1315..b1f3ca0a57 100644 --- a/awx/main/models/rbac.py +++ b/awx/main/models/rbac.py @@ -16,6 +16,7 @@ from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.fields import GenericForeignKey # AWX +from django.contrib.auth.models import User # noqa from awx.main.models.base import * # noqa __all__ = [ @@ -195,10 +196,17 @@ def get_user_permissions_on_resource(resource, user): access. ''' + if type(user) == User: + roles = user.roles.all() + else: + accessor_type = ContentType.objects.get_for_model(user) + roles = Role.objects.filter(content_type__pk=accessor_type.id, + object_id=user.id) + qs = RolePermission.objects.filter( content_type=ContentType.objects.get_for_model(resource), object_id=resource.id, - role__ancestors__in=user.roles.all() + role__ancestors__in=roles, ) res = qs = qs.aggregate( 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}) diff --git a/awx/main/tests/functional/test_rbac_team.py b/awx/main/tests/functional/test_rbac_team.py index 0c4ed86b34..a6ad507e22 100644 --- a/awx/main/tests/functional/test_rbac_team.py +++ b/awx/main/tests/functional/test_rbac_team.py @@ -1,6 +1,7 @@ import pytest from awx.main.access import TeamAccess +from awx.main.models import Project @pytest.mark.django_db def test_team_access_superuser(team, user): @@ -48,3 +49,25 @@ def test_team_access_member(organization, team, user): assert len(t.member_role.members.all()) == 1 assert len(t.organization.admin_role.members.all()) == 0 +@pytest.mark.django_db +def test_team_accessible_by(team, user, project): + u = user('team_member', False) + + team.member_role.children.add(project.member_role) + assert project.accessible_by(team, {'read':True}) + assert not project.accessible_by(u, {'read':True}) + + team.member_role.members.add(u) + assert project.accessible_by(u, {'read':True}) + +@pytest.mark.django_db +def test_team_accessible_objects(team, user, project): + u = user('team_member', False) + + team.member_role.children.add(project.member_role) + assert len(Project.accessible_objects(team, {'read':True})) == 1 + assert not Project.accessible_objects(u, {'read':True}) + + team.member_role.members.add(u) + assert len(Project.accessible_objects(u, {'read':True})) == 1 + diff --git a/awx/main/tests/job_base.py b/awx/main/tests/job_base.py index 81f33e2671..165193971b 100644 --- a/awx/main/tests/job_base.py +++ b/awx/main/tests/job_base.py @@ -264,17 +264,21 @@ class BaseJobTestMixin(BaseTestMixin): from awx.main.tests.data.ssh import (TEST_SSH_KEY_DATA, TEST_SSH_KEY_DATA_LOCKED, TEST_SSH_KEY_DATA_UNLOCK) - self.cred_sue = self.user_sue.credentials.create( + self.cred_sue = Credential.objects.create( username='sue', password=TEST_SSH_KEY_DATA, created_by=self.user_sue, ) - self.cred_sue_ask = self.user_sue.credentials.create( + self.cred_sue.owner_role.members.add(self.user_sue) + + self.cred_sue_ask = Credential.objects.create( username='sue', password='ASK', created_by=self.user_sue, ) - self.cred_sue_ask_many = self.user_sue.credentials.create( + self.cred_sue_ask.owner_role.members.add(self.user_sue) + + self.cred_sue_ask_many = Credential.objects.create( username='sue', password='ASK', become_method='sudo', @@ -284,23 +288,31 @@ class BaseJobTestMixin(BaseTestMixin): ssh_key_unlock='ASK', created_by=self.user_sue, ) - self.cred_bob = self.user_bob.credentials.create( + self.cred_sue_ask_many.owner_role.members.add(self.user_sue) + + self.cred_bob = Credential.objects.create( username='bob', password='ASK', created_by=self.user_sue, ) - self.cred_chuck = self.user_chuck.credentials.create( + self.cred_bob.usage_role.members.add(self.user_bob) + + self.cred_chuck = Credential.objects.create( username='chuck', ssh_key_data=TEST_SSH_KEY_DATA, created_by=self.user_sue, ) - self.cred_doug = self.user_doug.credentials.create( + self.cred_chuck.usage_role.members.add(self.user_chuck) + + self.cred_doug = Credential.objects.create( username='doug', password='doug doesn\'t mind his password being saved. this ' 'is why we dont\'t let doug actually run jobs.', created_by=self.user_sue, ) - self.cred_eve = self.user_eve.credentials.create( + self.cred_doug.usage_role.members.add(self.user_doug) + + self.cred_eve = Credential.objects.create( username='eve', password='ASK', become_method='sudo', @@ -308,40 +320,52 @@ class BaseJobTestMixin(BaseTestMixin): become_password='ASK', created_by=self.user_sue, ) - self.cred_frank = self.user_frank.credentials.create( + self.cred_eve.usage_role.members.add(self.user_eve) + + self.cred_frank = Credential.objects.create( username='frank', password='fr@nk the t@nk', created_by=self.user_sue, ) - self.cred_greg = self.user_greg.credentials.create( + self.cred_frank.usage_role.members.add(self.user_frank) + + self.cred_greg = Credential.objects.create( username='greg', ssh_key_data=TEST_SSH_KEY_DATA_LOCKED, ssh_key_unlock='ASK', created_by=self.user_sue, ) - self.cred_holly = self.user_holly.credentials.create( + self.cred_greg.usage_role.members.add(self.user_greg) + + self.cred_holly = Credential.objects.create( username='holly', password='holly rocks', created_by=self.user_sue, ) - self.cred_iris = self.user_iris.credentials.create( + self.cred_holly.usage_role.memebers.add(self.user_holly) + + self.cred_iris = Credential.objects.create( username='iris', password='ASK', created_by=self.user_sue, ) + self.cred_iris.usage_role.members.add(self.user_iris) # Each operations team also has shared credentials they can use. - self.cred_ops_east = self.team_ops_east.credentials.create( + self.cred_ops_east = Credential.objects.create( username='east', ssh_key_data=TEST_SSH_KEY_DATA_LOCKED, ssh_key_unlock=TEST_SSH_KEY_DATA_UNLOCK, created_by = self.user_sue, ) - self.cred_ops_west = self.team_ops_west.credentials.create( + self.team_ops_east.member_role.children.add(self.cred_ops_east.usage_role) + + self.cred_ops_west = Credential.objects.create( username='west', password='Heading270', created_by = self.user_sue, ) + self.team_ops_west.member_role.children.add(self.cred_ops_west.usage_role) # FIXME: This code can be removed (probably) @@ -355,17 +379,19 @@ class BaseJobTestMixin(BaseTestMixin): # created_by = self.user_sue, #) - self.cred_ops_north = self.team_ops_north.credentials.create( + self.cred_ops_north = Credential.objects.create( username='north', password='Heading0', created_by = self.user_sue, ) + self.team_ops_north.member_role.children.add(self.cred_ops_north.usage_role) - self.cred_ops_test = self.team_ops_testers.credentials.create( + self.cred_ops_test = Credential.objects.create( username='testers', password='HeadingNone', created_by = self.user_sue, ) + self.team_ops_testers.member_role.children(self.cred_ops_test.usage_role) self.ops_east_permission = Permission.objects.create( inventory = self.inv_ops_east,