From a4b35676192d8d236b372d1ad1349d17a2dcc5fe Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Mon, 29 Feb 2016 15:34:57 -0500 Subject: [PATCH 1/7] Added migration to remove users/admins FK from Org/Teams --- awx/main/migrations/0005_rbac_remove_users.py | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 awx/main/migrations/0005_rbac_remove_users.py diff --git a/awx/main/migrations/0005_rbac_remove_users.py b/awx/main/migrations/0005_rbac_remove_users.py new file mode 100644 index 0000000000..535e78a73a --- /dev/null +++ b/awx/main/migrations/0005_rbac_remove_users.py @@ -0,0 +1,26 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0004_rbac_migrations'), + ] + + operations = [ + migrations.RemoveField( + model_name='organization', + name='admins', + ), + migrations.RemoveField( + model_name='organization', + name='users', + ), + migrations.RemoveField( + model_name='team', + name='users', + ), + ] From 380ccec687dc739d460d215d3f40279176e31c4d Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Mon, 29 Feb 2016 15:36:51 -0500 Subject: [PATCH 2/7] started access refactoring, added UserAccess and updated how ALL permissions is checked --- awx/main/access.py | 41 ++++++++++++++---------- awx/main/models/mixins.py | 2 +- awx/main/models/organization.py | 10 ++++-- awx/main/models/rbac.py | 17 ++-------- awx/main/signals.py | 55 --------------------------------- 5 files changed, 35 insertions(+), 90 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 5a7ec03263..cf41ac08af 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -16,9 +16,9 @@ from rest_framework.exceptions import ParseError, PermissionDenied # AWX from awx.main.utils import * # noqa from awx.main.models import * # noqa +from awx.main.models.rbac import ALL_PERMISSIONS from awx.api.license import LicenseForbids from awx.main.task_engine import TaskSerializer -from awx.main.conf import tower_settings __all__ = ['get_user_queryset', 'check_user_access'] @@ -52,6 +52,21 @@ access_registry = { # ... } + +def user_or_team(data): + try: + if 'user' in data: + pk = get_pk_from_dict(data, 'user') + return get_object_or_400(User, pk=pk), None + elif 'team' in data: + pk = get_pk_from_dict(data, 'team') + return None, get_object_or_400(Team, pk=pk) + else: + return None, None + except ParseError: + return None, None + + def register_access(model_class, access_class): access_classes = access_registry.setdefault(model_class, []) access_classes.append(access_class) @@ -193,24 +208,16 @@ class UserAccess(BaseAccess): model = User def get_queryset(self): - qs = self.model.objects.filter(is_active=True).distinct() - if self.user.is_superuser: - return qs - if tower_settings.ORG_ADMINS_CAN_SEE_ALL_USERS and self.user.admin_of_organizations.filter(active=True).exists(): - return qs - return qs.filter( - Q(pk=self.user.pk) | - Q(organizations__in=self.user.admin_of_organizations.filter(active=True)) | - Q(organizations__in=self.user.organizations.filter(active=True)) | - Q(teams__in=self.user.teams.filter(active=True)) - ).distinct() + qs = self.model.accessible_objects(self.user, {'read':True}) + return qs def can_add(self, data): if data is not None and 'is_superuser' in data: if to_python_boolean(data['is_superuser'], allow_none=True) and not self.user.is_superuser: return False - return bool(self.user.is_superuser or - self.user.admin_of_organizations.filter(active=True).exists()) + if self.user.is_superuser: + return True + return Organization.accessible_objects(self.user, ALL_PERMISSIONS).filter(active=True).exists() def can_change(self, obj, data): if data is not None and 'is_superuser' in data: @@ -225,7 +232,7 @@ class UserAccess(BaseAccess): # Admin implies changing all user fields. if self.user.is_superuser: return True - return bool(obj.organizations.filter(active=True, admins__in=[self.user]).exists()) + return obj.accessible_by(self.user, {'create': True, 'write':True, 'update':True, 'read':True}) def can_delete(self, obj): if obj == self.user: @@ -235,8 +242,8 @@ class UserAccess(BaseAccess): if obj.is_superuser and super_users.count() == 1: # cannot delete the last active superuser return False - return bool(self.user.is_superuser or - obj.organizations.filter(active=True, admins__in=[self.user]).exists()) + return obj.accessible_by(self.user, {'delete': True}) + class OrganizationAccess(BaseAccess): ''' diff --git a/awx/main/models/mixins.py b/awx/main/models/mixins.py index 6d069ed3d4..d6bb10754d 100644 --- a/awx/main/models/mixins.py +++ b/awx/main/models/mixins.py @@ -51,7 +51,7 @@ class ResourceMixin(models.Model): ''' perms = self.get_permissions(user) - if not perms: + if perms is None: return False for k in permissions: if k not in perms or perms[k] < permissions[k]: diff --git a/awx/main/models/organization.py b/awx/main/models/organization.py index 0cd50d9dcc..9709c2d3d0 100644 --- a/awx/main/models/organization.py +++ b/awx/main/models/organization.py @@ -18,7 +18,11 @@ from django.utils.translation import ugettext_lazy as _ # AWX from awx.main.fields import AutoOneToOneField, ImplicitRoleField from awx.main.models.base import * # noqa -from awx.main.models.rbac import ROLE_SINGLETON_SYSTEM_ADMINISTRATOR, ROLE_SINGLETON_SYSTEM_AUDITOR +from awx.main.models.rbac import ( + ALL_PERMISSIONS, + ROLE_SINGLETON_SYSTEM_ADMINISTRATOR, + ROLE_SINGLETON_SYSTEM_AUDITOR, +) from awx.main.models.mixins import ResourceMixin from awx.main.conf import tower_settings @@ -52,7 +56,7 @@ class Organization(CommonModel, ResourceMixin): admin_role = ImplicitRoleField( role_name='Organization Administrator', parent_role='singleton:' + ROLE_SINGLETON_SYSTEM_ADMINISTRATOR, - permissions = {'all': True} + permissions = ALL_PERMISSIONS, ) auditor_role = ImplicitRoleField( role_name='Organization Auditor', @@ -108,7 +112,7 @@ class Team(CommonModelNameNotUnique, ResourceMixin): admin_role = ImplicitRoleField( role_name='Team Administrator', parent_role='organization.admin_role', - permissions = {'all': True} + permissions = ALL_PERMISSIONS, ) auditor_role = ImplicitRoleField( role_name='Team Auditor', diff --git a/awx/main/models/rbac.py b/awx/main/models/rbac.py index 396dcd71c3..b7b6d50f3e 100644 --- a/awx/main/models/rbac.py +++ b/awx/main/models/rbac.py @@ -31,7 +31,8 @@ ROLE_SINGLETON_SYSTEM_AUDITOR='System Auditor' role_rebuilding_paused = False roles_needing_rebuilding = set() - +ALL_PERMISSIONS = {'create': True, 'read': True, 'update': True, 'delete': True, + 'write': True, 'scm_update': True, 'use': True, 'execute': True} class Role(CommonModelNameNotUnique): ''' @@ -122,18 +123,6 @@ class Role(CommonModelNameNotUnique): def grant(self, resource, permissions): # take either the raw Resource or something that includes the ResourceMixin resource = resource if type(resource) is Resource else resource.resource - - if 'all' in permissions and permissions['all']: - del permissions['all'] - permissions['create'] = True - permissions['read'] = True - permissions['write'] = True - permissions['update'] = True - permissions['delete'] = True - permissions['scm_update'] = True - permissions['use'] = True - permissions['execute'] = True - permission = RolePermission(role=self, resource=resource) for k in permissions: setattr(permission, k, int(permissions[k])) @@ -256,8 +245,8 @@ class RolePermission(CreatedModifiedModel): create = models.IntegerField(default = 0) read = models.IntegerField(default = 0) write = models.IntegerField(default = 0) - update = models.IntegerField(default = 0) delete = models.IntegerField(default = 0) + update = models.IntegerField(default = 0) execute = models.IntegerField(default = 0) scm_update = models.IntegerField(default = 0) use = models.IntegerField(default = 0) diff --git a/awx/main/signals.py b/awx/main/signals.py index 6451da0fe6..01b2bb9d34 100644 --- a/awx/main/signals.py +++ b/awx/main/signals.py @@ -130,58 +130,6 @@ def sync_superuser_status_to_rbac(sender, instance, **kwargs): else: Role.singleton(ROLE_SINGLETON_SYSTEM_ADMINISTRATOR).members.remove(instance) -def sync_user_to_team_members_role(sender, reverse, model, instance, pk_set, action, **kwargs): - 'When a user is added or removed from Team.users, ensure that is reflected in Team.member_role' - if action == 'post_add' or action == 'pre_remove': - if reverse: - for team in Team.objects.filter(id__in=pk_set).all(): - if action == 'post_add': - team.member_role.members.add(instance) - if action == 'pre_remove': - team.member_role.members.remove(instance) - else: - for user in User.objects.filter(id__in=pk_set).all(): - if action == 'post_add': - instance.member_role.members.add(user) - if action == 'pre_remove': - instance.member_role.members.remove(user) - -def sync_admin_to_org_admin_role(sender, reverse, model, instance, pk_set, action, **kwargs): - 'When a user is added or removed from Organization.admins, ensure that is reflected in Organization.admin_role' - if action == 'post_add' or action == 'pre_remove': - if reverse: - for org in Organization.objects.filter(id__in=pk_set).all(): - if action == 'post_add': - org.admin_role.members.add(instance) - if action == 'pre_remove': - org.admin_role.members.remove(instance) - else: - for user in User.objects.filter(id__in=pk_set).all(): - if action == 'post_add': - instance.admin_role.members.add(user) - if action == 'pre_remove': - instance.admin_role.members.remove(user) - -def sync_user_to_org_members_role(sender, reverse, model, instance, pk_set, action, **kwargs): - 'When a user is added or removed from Organization.users, ensure that is reflected in Organization.member_role' - if action == 'post_add' or action == 'pre_remove': - if reverse: - for org in Organization.objects.filter(id__in=pk_set).all(): - if action == 'post_add': - org.member_role.members.add(instance) - org.admin_role.children.add(instance.resource.admin_role) - if action == 'pre_remove': - org.member_role.members.remove(instance) - org.admin_role.children.remove(instance.resource.admin_role) - else: - for user in User.objects.filter(id__in=pk_set).all(): - if action == 'post_add': - instance.member_role.members.add(user) - instance.admin_role.children.add(user.resource.admin_role) - if action == 'pre_remove': - instance.member_role.members.remove(user) - instance.admin_role.children.remove(user.resource.admin_role) - def create_user_resource(sender, **kwargs): instance = kwargs['instance'] try: @@ -210,10 +158,7 @@ post_save.connect(emit_job_event_detail, sender=JobEvent) post_save.connect(emit_ad_hoc_command_event_detail, sender=AdHocCommandEvent) m2m_changed.connect(rebuild_role_ancestor_list, Role.parents.through) post_save.connect(sync_superuser_status_to_rbac, sender=User) -m2m_changed.connect(sync_user_to_team_members_role, Team.users.through) post_save.connect(create_user_resource, sender=User) -m2m_changed.connect(sync_user_to_org_members_role, Organization.users.through) -m2m_changed.connect(sync_admin_to_org_admin_role, Organization.admins.through) # Migrate hosts, groups to parent group(s) whenever a group is deleted or # marked as inactive. From 1d179574af403a063430d0b3528436ea851fe4a8 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Mon, 29 Feb 2016 15:37:59 -0500 Subject: [PATCH 3/7] Updated Organization and Credential access --- awx/main/access.py | 61 ++++++------------- .../tests/functional/test_rbac_credential.py | 18 +++--- .../tests/functional/test_rbac_inventory.py | 36 +++++++++++ .../functional/test_rbac_organization.py | 14 ++--- 4 files changed, 71 insertions(+), 58 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index cf41ac08af..3870b1012e 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -258,15 +258,14 @@ class OrganizationAccess(BaseAccess): model = Organization def get_queryset(self): - qs = self.model.objects.filter(active=True).distinct() + qs = self.model.accessible_objects(self.user, {'read':True}) qs = qs.select_related('created_by', 'modified_by') - if self.user.is_superuser: - return qs - return qs.filter(Q(admins__in=[self.user]) | Q(users__in=[self.user])) + return qs def can_change(self, obj, data): - return bool(self.user.is_superuser or - self.user in obj.admins.all()) + if self.user.is_superuser: + return True + return obj.accessible_by(self.user, ALL_PERMISSIONS) def can_delete(self, obj): self.check_license(feature='multiple_organizations', check_expiration=False) @@ -567,55 +566,29 @@ class CredentialAccess(BaseAccess): """Return the queryset for credentials, based on what the user is permitted to see. """ - # Create a base queryset. - # If the user is a superuser, and therefore can see everything, this - # is also sufficient, and we are done. - qs = self.model.objects.filter(active=True).distinct() + qs = self.model.accessible_objects(self.user, {'read':True}) qs = qs.select_related('created_by', 'modified_by', 'user', 'team') - if self.user.is_superuser: - return qs - - # Get the list of organizations for which the user is an admin - orgs_as_admin_ids = set(self.user.admin_of_organizations.filter(active=True).values_list('id', flat=True)) - return qs.filter( - Q(user=self.user) | - Q(user__organizations__id__in=orgs_as_admin_ids) | - Q(user__admin_of_organizations__id__in=orgs_as_admin_ids) | - Q(team__organization__id__in=orgs_as_admin_ids, team__active=True) | - Q(team__users__in=[self.user], team__active=True) - ) + return qs def can_add(self, data): 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 self.user.can_access(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 self.user.can_access(Team, 'change', team_obj, None) - return False + + user, team = user_or_team(data) + if user is None and team is None: + return False + + if user is not None: + return user.resource.accessible_by(self.user, {'write': True}) + if team is not None: + return team.accessible_by(self.user, {'write':True}) def can_change(self, obj, data): if self.user.is_superuser: return True if not self.can_add(data): return False - if self.user == obj.created_by: - return True - if obj.user: - if self.user == obj.user: - return True - if obj.user.organizations.filter(active=True, admins__in=[self.user]).exists(): - return True - if obj.user.admin_of_organizations.filter(active=True, admins__in=[self.user]).exists(): - return True - if obj.team: - if self.user in obj.team.organization.admins.filter(is_active=True): - return True - return False + return obj.accessible_by(self.user, {'read':True, 'update': True, 'delete':True}) def can_delete(self, obj): # Unassociated credentials may be marked deleted by anyone, though we diff --git a/awx/main/tests/functional/test_rbac_credential.py b/awx/main/tests/functional/test_rbac_credential.py index e2febb456d..ec990472a1 100644 --- a/awx/main/tests/functional/test_rbac_credential.py +++ b/awx/main/tests/functional/test_rbac_credential.py @@ -69,11 +69,9 @@ def test_credential_access_superuser(): assert access.can_delete(credential) @pytest.mark.django_db -def test_credential_access_admin(user, organization, team, credential): +def test_credential_access_admin(user, team, credential): u = user('org-admin', False) - organization.admins.add(u) - team.organization = organization - team.save() + team.organization.admin_role.members.add(u) access = CredentialAccess(u) @@ -85,10 +83,16 @@ def test_credential_access_admin(user, organization, team, credential): # unowned credential can be deleted assert access.can_delete(credential) - team.users.add(u) - assert not access.can_change(credential, {'user': u.pk}) - + # credential is now part of a team + # that is part of an organization + # that I am an admin for credential.team = team credential.save() + credential.owner_role.rebuild_role_ancestor_list() + cred = Credential.objects.create(kind='aws', name='test-cred') + cred.team = team + cred.save() + + # should have can_change access as org-admin assert access.can_change(credential, {'user': u.pk}) diff --git a/awx/main/tests/functional/test_rbac_inventory.py b/awx/main/tests/functional/test_rbac_inventory.py index 8834545140..d4440f6902 100644 --- a/awx/main/tests/functional/test_rbac_inventory.py +++ b/awx/main/tests/functional/test_rbac_inventory.py @@ -195,3 +195,39 @@ def test_group_parent_admin(group, permissions, user): parent2.admin_role.members.add(u) assert childA.accessible_by(u, permissions['admin']) + +@pytest.mark.django_db +def test_access_admin(organization, inventory, user): + a = user('admin', False) + inventory.organization = organization + organization.admin_role.members.add(a) + + access = InventoryAccess(a) + assert access.can_read(inventory) + assert access.can_add(None) + assert access.can_add({'organization': organization.id}) + assert access.can_change(inventory, None) + assert access.can_change(inventory, {'organization': organization.id}) + assert access.can_admin(inventory, None) + assert access.can_admin(inventory, {'organization': organization.id}) + assert access.can_delete(inventory) + assert access.can_run_ad_hoc_commands(inventory) + +@pytest.mark.django_db +def test_access_auditor(organization, inventory, user): + u = user('admin', False) + inventory.organization = organization + organization.auditor_role.members.add(u) + + access = InventoryAccess(u) + assert access.can_read(inventory) + assert not access.can_add(None) + assert not access.can_add({'organization': organization.id}) + assert not access.can_change(inventory, None) + assert not access.can_change(inventory, {'organization': organization.id}) + assert not access.can_admin(inventory, None) + assert not access.can_admin(inventory, {'organization': organization.id}) + assert not access.can_delete(inventory) + assert not access.can_run_ad_hoc_commands(inventory) + + diff --git a/awx/main/tests/functional/test_rbac_organization.py b/awx/main/tests/functional/test_rbac_organization.py index c8f1d709a2..cf3f5f6cdf 100644 --- a/awx/main/tests/functional/test_rbac_organization.py +++ b/awx/main/tests/functional/test_rbac_organization.py @@ -57,27 +57,27 @@ def test_organization_access_superuser(cl, organization, user): def test_organization_access_admin(cl, organization, user): '''can_change because I am an admin of that org''' a = user('admin', False) - organization.admins.add(a) - organization.users.add(user('user', False)) + organization.admin_role.members.add(a) + organization.member_role.members.add(user('user', False)) access = OrganizationAccess(a) assert access.can_change(organization, None) assert access.can_delete(organization) org = access.get_queryset()[0] - assert len(org.admins.all()) == 1 - assert len(org.users.all()) == 1 + assert len(org.admin_role.members.all()) == 1 + assert len(org.member_role.members.all()) == 1 @mock.patch.object(BaseAccess, 'check_license', return_value=None) @pytest.mark.django_db def test_organization_access_user(cl, organization, user): access = OrganizationAccess(user('user', False)) - organization.users.add(user('user', False)) + organization.member_role.members.add(user('user', False)) assert not access.can_change(organization, None) assert not access.can_delete(organization) org = access.get_queryset()[0] - assert len(org.admins.all()) == 0 - assert len(org.users.all()) == 1 + assert len(org.admin_role.members.all()) == 0 + assert len(org.member_role.members.all()) == 1 From 70335429105641ccc65902917af6f365afe8cf75 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Tue, 1 Mar 2016 15:00:36 -0500 Subject: [PATCH 4/7] Updated Invetory access --- awx/main/access.py | 56 +++++-------------- .../tests/functional/test_rbac_inventory.py | 1 + 2 files changed, 14 insertions(+), 43 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 3870b1012e..231836115e 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -294,53 +294,23 @@ class InventoryAccess(BaseAccess): model = Inventory def get_queryset(self, allowed=None, ad_hoc=None): - allowed = allowed or PERMISSION_TYPES_ALLOWING_INVENTORY_READ - qs = Inventory.objects.filter(active=True).distinct() + qs = self.model.accessible_objects(self.user) qs = qs.select_related('created_by', 'modified_by', 'organization') - if self.user.is_superuser: - return qs - qs = qs.filter(organization__active=True) - admin_of = qs.filter(organization__admins__in=[self.user]).distinct() - has_user_kw = dict( - permissions__user__in=[self.user], - permissions__permission_type__in=allowed, - permissions__active=True, - ) - if ad_hoc is not None: - has_user_kw['permissions__run_ad_hoc_commands'] = ad_hoc - has_user_perms = qs.filter(**has_user_kw).distinct() - has_team_kw = dict( - permissions__team__users__in=[self.user], - permissions__team__active=True, - permissions__permission_type__in=allowed, - permissions__active=True, - ) - if ad_hoc is not None: - has_team_kw['permissions__run_ad_hoc_commands'] = ad_hoc - has_team_perms = qs.filter(**has_team_kw).distinct() - return admin_of | has_user_perms | has_team_perms - - def has_permission_types(self, obj, allowed, ad_hoc=None): - return bool(obj and self.get_queryset(allowed, ad_hoc).filter(pk=obj.pk).exists()) + return qs def can_read(self, obj): - return self.has_permission_types(obj, PERMISSION_TYPES_ALLOWING_INVENTORY_READ) + return obj.accessible_by(self.user, {'read': True}) def can_add(self, data): # If no data is specified, just checking for generic add permission? if not data: - return bool(self.user.is_superuser or - self.user.admin_of_organizations.filter(active=True).exists()) - # Otherwise, verify that the user has access to change the parent - # organization of this inventory. + return Organization.accessible_objects(self.user, ALL_PERMISSIONS).exists() if self.user.is_superuser: return True - else: - org_pk = get_pk_from_dict(data, 'organization') - org = get_object_or_400(Organization, pk=org_pk) - if self.user.can_access(Organization, 'change', org, None): - return True - return False + + org_pk = get_pk_from_dict(data, 'organization') + org = get_object_or_400(Organization, pk=org_pk) + return org.accessible_by(self.user, {'read': True, 'create':True, 'update': True, 'delete': True}) def can_change(self, obj, data): # Verify that the user has access to the new organization if moving an @@ -348,10 +318,10 @@ class InventoryAccess(BaseAccess): org_pk = get_pk_from_dict(data, 'organization') if obj and org_pk and obj.organization.pk != org_pk: org = get_object_or_400(Organization, pk=org_pk) - if not self.user.can_access(Organization, 'change', org, None): + if not org.accessible_by(self.user, {'read': True, 'create':True, 'update': True, 'delete': True}): return False # Otherwise, just check for write permission. - return self.has_permission_types(obj, PERMISSION_TYPES_ALLOWING_INVENTORY_WRITE) + return obj.accessible_by(self.user, {'read': True, 'create':True, 'update': True, 'delete': True}) def can_admin(self, obj, data): # Verify that the user has access to the new organization if moving an @@ -359,16 +329,16 @@ class InventoryAccess(BaseAccess): org_pk = get_pk_from_dict(data, 'organization') if obj and org_pk and obj.organization.pk != org_pk: org = get_object_or_400(Organization, pk=org_pk) - if not self.user.can_access(Organization, 'change', org, None): + if not org.accessible_by(self.user, ALL_PERMISSIONS): return False # Otherwise, just check for admin permission. - return self.has_permission_types(obj, PERMISSION_TYPES_ALLOWING_INVENTORY_ADMIN) + return obj.accessible_by(self.user, ALL_PERMISSIONS) def can_delete(self, obj): return self.can_admin(obj, None) def can_run_ad_hoc_commands(self, obj): - return self.has_permission_types(obj, PERMISSION_TYPES_ALLOWING_INVENTORY_READ, True) + return obj.accessible_by(self.user, {'execute': True}) class HostAccess(BaseAccess): ''' diff --git a/awx/main/tests/functional/test_rbac_inventory.py b/awx/main/tests/functional/test_rbac_inventory.py index d4440f6902..3faf66fb91 100644 --- a/awx/main/tests/functional/test_rbac_inventory.py +++ b/awx/main/tests/functional/test_rbac_inventory.py @@ -2,6 +2,7 @@ import pytest from awx.main.migrations import _rbac as rbac from awx.main.models import Permission +from awx.main.access import InventoryAccess from django.apps import apps @pytest.mark.django_db From a1cc5b06b81de0baf6f3af3cf57ae36cb84a4113 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Tue, 1 Mar 2016 17:30:32 -0500 Subject: [PATCH 5/7] Remove can_access methods and registration --- awx/main/access.py | 217 ++++++----------------------------- awx/main/migrations/_rbac.py | 21 ++-- awx/main/models/__init__.py | 1 - 3 files changed, 49 insertions(+), 190 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 231836115e..d3fd865de2 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -20,7 +20,7 @@ from awx.main.models.rbac import ALL_PERMISSIONS from awx.api.license import LicenseForbids from awx.main.task_engine import TaskSerializer -__all__ = ['get_user_queryset', 'check_user_access'] +__all__ = ['get_user_queryset'] PERMISSION_TYPES = [ PERM_INVENTORY_ADMIN, @@ -90,25 +90,6 @@ def get_user_queryset(user, model_class): queryset = queryset.filter(pk__in=qs.values_list('pk', flat=True)) return queryset -def check_user_access(user, model_class, action, *args, **kwargs): - ''' - Return True if user can perform action against model_class with the - provided parameters. - ''' - for access_class in access_registry.get(model_class, []): - access_instance = access_class(user) - access_method = getattr(access_instance, 'can_%s' % action, None) - if not access_method: - logger.debug('%s.%s not found', access_instance.__class__.__name__, - 'can_%s' % action) - continue - result = access_method(*args, **kwargs) - logger.debug('%s.%s %r returned %r', access_instance.__class__.__name__, - access_method.__name__, args, result) - if result: - return result - return False - class BaseAccess(object): ''' @@ -156,7 +137,7 @@ class BaseAccess(object): return self.can_change(obj, None) else: return bool(self.can_change(obj, None) and - self.user.can_access(type(sub_obj), 'read', sub_obj)) + sub_obj.accessible_by(self.user, {'read':True})) def can_unattach(self, obj, sub_obj, relationship): return self.can_change(obj, None) @@ -358,7 +339,7 @@ class HostAccess(BaseAccess): return qs.filter(inventory_id__in=inventory_ids) def can_read(self, obj): - return obj and self.user.can_access(Inventory, 'read', obj.inventory) + return obj and obj.inventory.accessible_by(self.user, {'read':True}) def can_add(self, data): if not data or 'inventory' not in data: @@ -367,7 +348,7 @@ class HostAccess(BaseAccess): # Checks for admin or change permission on inventory. inventory_pk = get_pk_from_dict(data, 'inventory') inventory = get_object_or_400(Inventory, pk=inventory_pk) - if not self.user.can_access(Inventory, 'change', inventory, None): + if not inventory.accessible_by(self.user, {'read':True, 'create':True}): return False # Check to see if we have enough licenses @@ -381,7 +362,7 @@ class HostAccess(BaseAccess): raise PermissionDenied('Unable to change inventory on a host') # Checks for admin or change permission on inventory, controls whether # the user can edit variable data. - return obj and self.user.can_access(Inventory, 'change', obj.inventory, None) + return obj and obj.inventory.accessible_by(self.user, {'read':True, 'update':True, 'write':True}) def can_attach(self, obj, sub_obj, relationship, data, skip_sub_obj_read_check=False): @@ -394,7 +375,7 @@ class HostAccess(BaseAccess): return True def can_delete(self, obj): - return obj and self.user.can_access(Inventory, 'delete', obj.inventory) + return obj and obj.inventory.accessible_by(self.user, {'delete':True}) class GroupAccess(BaseAccess): ''' @@ -412,7 +393,7 @@ class GroupAccess(BaseAccess): return qs.filter(inventory_id__in=inventory_ids) def can_read(self, obj): - return obj and self.user.can_access(Inventory, 'read', obj.inventory) + return obj and obj.inventory.accessible_by(self.user, {'read':True}) def can_add(self, data): if not data or 'inventory' not in data: @@ -420,7 +401,7 @@ class GroupAccess(BaseAccess): # Checks for admin or change permission on inventory. inventory_pk = get_pk_from_dict(data, 'inventory') inventory = get_object_or_400(Inventory, pk=inventory_pk) - return self.user.can_access(Inventory, 'change', inventory, None) + return inventory.accessible_by(self.user, {'read':True, 'create':True}) def can_change(self, obj, data): # Prevent moving a group to a different inventory. @@ -429,7 +410,7 @@ class GroupAccess(BaseAccess): raise PermissionDenied('Unable to change inventory on a group') # Checks for admin or change permission on inventory, controls whether # the user can attach subgroups or edit variable data. - return obj and self.user.can_access(Inventory, 'change', obj.inventory, None) + return obj and obj.inventory.accessible_by(self.user, {'read':True, 'update':True, 'write':True}) def can_attach(self, obj, sub_obj, relationship, data, skip_sub_obj_read_check=False): @@ -453,8 +434,7 @@ class GroupAccess(BaseAccess): return True def can_delete(self, obj): - return obj and self.user.can_access(Inventory, 'delete', obj.inventory) - + return obj and obj.inventory.accessible_by(self.user, {'delete':True}) class InventorySourceAccess(BaseAccess): ''' @@ -473,9 +453,9 @@ class InventorySourceAccess(BaseAccess): def can_read(self, obj): if obj and obj.group: - return self.user.can_access(Group, 'read', obj.group) + return obj.group.accessible_by(self.user, {'read':True}) elif obj and obj.inventory: - return self.user.can_access(Inventory, 'read', obj.inventory) + return obj.inventory.accessible_by(self.user, {'read':True}) else: return False @@ -486,7 +466,7 @@ class InventorySourceAccess(BaseAccess): def can_change(self, obj, data): # Checks for admin or change permission on group. if obj and obj.group: - return self.user.can_access(Group, 'change', obj.group, None) + return obj.group.accessible_by(self.user, {'read':True, 'update':True, 'write':True}) # Can't change inventory sources attached to only the inventory, since # these are created automatically from the management command. else: @@ -596,7 +576,7 @@ class TeamAccess(BaseAccess): else: org_pk = get_pk_from_dict(data, 'organization') org = get_object_or_400(Organization, pk=org_pk) - if self.user.can_access(Organization, 'change', org, None): + if org.accessible_by(self.user, {'read':True, 'update':True, 'write':True}): return True return False @@ -701,100 +681,7 @@ class ProjectUpdateAccess(BaseAccess): return self.can_change(obj, {}) and obj.can_cancel def can_delete(self, obj): - return obj and self.user.can_access(Project, 'delete', obj.project) - -class PermissionAccess(BaseAccess): - ''' - I can see a permission when: - - I'm a superuser. - - I'm an org admin and it's for a user in my org. - - I'm an org admin and it's for a team in my org. - - I'm a user and it's assigned to me. - - I'm a member of a team and it's assigned to the team. - I can create/change/delete when: - - I'm a superuser. - - I'm an org admin and the team/user is in my org and the inventory is in - my org and the project is in my org. - ''' - - model = Permission - - def get_queryset(self): - qs = self.model.objects.filter(active=True).distinct() - qs = qs.select_related('created_by', 'modified_by', 'user', 'team', 'inventory', - 'project') - if self.user.is_superuser: - return qs - orgs_as_admin_ids = set(self.user.admin_of_organizations.filter(active=True).values_list('id', flat=True)) - return qs.filter( - Q(user__organizations__in=orgs_as_admin_ids) | - Q(user__admin_of_organizations__in=orgs_as_admin_ids) | - Q(team__organization__in=orgs_as_admin_ids, team__active=True) | - Q(user=self.user) | - Q(team__users__in=[self.user], team__active=True) - ) - - def can_add(self, data): - if not data: - return True # generic add permission check - user_pk = get_pk_from_dict(data, 'user') - team_pk = get_pk_from_dict(data, 'team') - if user_pk: - user = get_object_or_400(User, pk=user_pk) - if not self.user.can_access(User, 'admin', user, None): - return False - elif team_pk: - team = get_object_or_400(Team, pk=team_pk) - if not self.user.can_access(Team, 'admin', team, None): - return False - else: - return False - inventory_pk = get_pk_from_dict(data, 'inventory') - if inventory_pk: - inventory = get_object_or_400(Inventory, pk=inventory_pk) - if not self.user.can_access(Inventory, 'admin', inventory, None): - return False - project_pk = get_pk_from_dict(data, 'project') - if project_pk: - project = get_object_or_400(Project, pk=project_pk) - if not self.user.can_access(Project, 'admin', project, None): - return False - # FIXME: user/team, inventory and project should probably all be part - # of the same organization. - return True - - def can_change(self, obj, data): - # Prevent assigning a permission to a different user. - user_pk = get_pk_from_dict(data, 'user') - if obj and user_pk and obj.user and obj.user.pk != user_pk: - raise PermissionDenied('Unable to change user on a permission') - # Prevent assigning a permission to a different team. - team_pk = get_pk_from_dict(data, 'team') - if obj and team_pk and obj.team and obj.team.pk != team_pk: - raise PermissionDenied('Unable to change team on a permission') - if self.user.is_superuser: - return True - # If changing inventory, verify access to the new inventory. - new_inventory_pk = get_pk_from_dict(data, 'inventory') - if obj and new_inventory_pk and obj.inventory and obj.inventory.pk != new_inventory_pk: - inventory = get_object_or_400(Inventory, pk=new_inventory_pk) - if not self.user.can_access(Inventory, 'admin', inventory, None): - return False - # If changing project, verify access to the new project. - new_project = get_pk_from_dict(data, 'project') - if obj and new_project and obj.project and obj.project.pk != new_project: - project = get_object_or_400(Project, pk=new_project) - if not self.user.can_access(Project, 'admin', project, None): - return False - # Check for admin access to the user or team. - if obj.user and self.user.can_access(User, 'admin', obj.user, None): - return True - if obj.team and self.user.can_access(Team, 'admin', obj.team, None): - return True - return False - - def can_delete(self, obj): - return self.can_change(obj, None) + return obj and obj.project.accessible_by(self.user, {'delete':True}) class JobTemplateAccess(BaseAccess): ''' @@ -895,7 +782,7 @@ class JobTemplateAccess(BaseAccess): credential_pk = get_pk_from_dict(data, 'credential') if credential_pk: credential = get_object_or_400(Credential, pk=credential_pk) - if not self.user.can_access(Credential, 'read', credential): + if not credential.accessible_by(self.user, {'read':True}): return False # If a cloud credential is provided, the user should have read access. @@ -903,7 +790,7 @@ class JobTemplateAccess(BaseAccess): if cloud_credential_pk: cloud_credential = get_object_or_400(Credential, pk=cloud_credential_pk) - if not self.user.can_access(Credential, 'read', cloud_credential): + if not cloud_credential.accessible_by(self.user, {'read':True}): return False # Check that the given inventory ID is valid. @@ -914,48 +801,19 @@ class JobTemplateAccess(BaseAccess): project_pk = get_pk_from_dict(data, 'project') if 'job_type' in data and data['job_type'] == PERM_INVENTORY_SCAN: - if not project_pk and self.user.can_access(Organization, 'change', inventory[0].organization, None): + org = inventory[0].organization + accessible = org.accessible_by(self.user, {'read':True, 'update':True, 'write':True}) + if not project_pk and accessible: return True - elif not self.user.can_access(Organization, "change", inventory[0].organization, None): + elif not accessible: return False # If the user has admin access to the project (as an org admin), should # be able to proceed without additional checks. project = get_object_or_400(Project, pk=project_pk) - if self.user.can_access(Project, 'admin', project, None): + if project.accessible_by(self.user, ALL_PERMISSIONS): return True - # Otherwise, check for explicitly granted permissions to create job templates - # for the project and inventory. - permission_qs = Permission.objects.filter( - Q(user=self.user) | Q(team__users__in=[self.user]), - inventory=inventory, - project=project, - active=True, - #permission_type__in=[PERM_INVENTORY_CHECK, PERM_INVENTORY_DEPLOY], - permission_type=PERM_JOBTEMPLATE_CREATE, - ) - if permission_qs.exists(): - return True - return False - - # job_type = data.get('job_type', None) - - # for perm in permission_qs: - # # if you have run permissions, you can also create check jobs - # if job_type == PERM_INVENTORY_CHECK: - # has_perm = True - # # you need explicit run permissions to make run jobs - # elif job_type == PERM_INVENTORY_DEPLOY and perm.permission_type == PERM_INVENTORY_DEPLOY: - # has_perm = True - # if not has_perm: - # return False - # return True - - # shouldn't really matter with permissions given, but make sure the user - # is also currently on the team in case they were added a per-user permission and then removed - # from the project. - #if not project.teams.filter(users__in=[self.user]).count(): - # return False + return project.accessible_by(self.user, ALL_PERMISSIONS) and inventory.accessible_by(self.user, {'read':True}) def can_start(self, obj, validate_license=True): # Check license. @@ -973,14 +831,14 @@ class JobTemplateAccess(BaseAccess): if obj.inventory is None: return False if obj.job_type == PERM_INVENTORY_SCAN: - if obj.project is None and self.user.can_access(Organization, 'change', obj.inventory.organization, None): + if obj.project is None and obj.inventory.organization.accessible_by(self.user, {'read':True, 'update':True, 'write':True}): return True - if not self.user.can_access(Organization, 'change', obj.inventory.organization, None): + if not obj.inventory.organization.accessible_by(self.user, {'read':True, 'update':True, 'write':True}): return False if obj.project is None: return False # If the user has admin access to the project they can start a job - if self.user.can_access(Project, 'admin', obj.project, None): + if obj.project.accessible_by(self.user, ALL_PERMISSIONS): return True # Otherwise check for explicitly granted permissions @@ -1004,7 +862,7 @@ class JobTemplateAccess(BaseAccess): obj.job_type == PERM_INVENTORY_CHECK: has_perm = True - dep_access = self.user.can_access(Inventory, 'read', obj.inventory) and self.user.can_access(Project, 'read', obj.project) + dep_access = obj.inventory.accessible_by(self.user, {'read':True}) and obj.project.accessible_by(self.user, {'read':True}) return dep_access and has_perm def can_change(self, obj, data): @@ -1119,10 +977,10 @@ class JobAccess(BaseAccess): # If a user can launch the job template then they can relaunch a job from that # job template has_perm = False - if obj.job_template is not None and self.user.can_access(JobTemplate, 'start', obj.job_template): + if obj.job_template is not None and obj.job_template.accessible_by(self.user, {'execute':True}): has_perm = True - dep_access_inventory = self.user.can_access(Inventory, 'read', obj.inventory) - dep_access_project = obj.project is None or self.user.can_access(Project, 'read', obj.project) + dep_access_inventory = obj.inventory.accessible_by(self.user, {'read':True}) + dep_access_project = obj.project is None or obj.project.accessible_by(self.user, {'read':True}) return self.can_read(obj) and dep_access_inventory and dep_access_project and has_perm def can_cancel(self, obj): @@ -1192,7 +1050,7 @@ class AdHocCommandAccess(BaseAccess): credential_pk = get_pk_from_dict(data, 'credential') if credential_pk: credential = get_object_or_400(Credential, pk=credential_pk, active=True) - if not self.user.can_access(Credential, 'read', credential): + if not credential.accessible_by(self.user, {'read':True}): return False # Check that the user has the run ad hoc command permission on the @@ -1200,7 +1058,7 @@ class AdHocCommandAccess(BaseAccess): inventory_pk = get_pk_from_dict(data, 'inventory') if inventory_pk: inventory = get_object_or_400(Inventory, pk=inventory_pk, active=True) - if not self.user.can_access(Inventory, 'run_ad_hoc_commands', inventory): + if not inventory.accessible_by(self.user, {'execute': True}): return False return True @@ -1403,8 +1261,7 @@ class ScheduleAccess(BaseAccess): if self.user.is_superuser: return True if obj and obj.unified_job_template: - job_class = obj.unified_job_template - return self.user.can_access(type(job_class), 'read', obj.unified_job_template) + return obj.unified_job_template.accessible_by(self.user, {'read':True}) else: return False @@ -1414,7 +1271,7 @@ class ScheduleAccess(BaseAccess): pk = get_pk_from_dict(data, 'unified_job_template') obj = get_object_or_400(UnifiedJobTemplate, pk=pk) if obj: - return self.user.can_access(type(obj), 'change', obj, None) + return obj.accessible_by(self.user, {'read':True, 'update':True, 'write':True}) else: return False @@ -1422,8 +1279,7 @@ class ScheduleAccess(BaseAccess): if self.user.is_superuser: return True if obj and obj.unified_job_template: - job_class = obj.unified_job_template - return self.user.can_access(type(job_class), 'change', job_class, None) + return obj.unified_job_template.accessible_by(self.user, {'read':True, 'update':True, 'write':True}) else: return False @@ -1431,8 +1287,7 @@ class ScheduleAccess(BaseAccess): if self.user.is_superuser: return True if obj and obj.unified_job_template: - job_class = obj.unified_job_template - return self.user.can_access(type(job_class), 'change', job_class, None) + return obj.unified_job_template.accessible_by(self.user, {'read':True, 'update':True, 'write':True}) else: return False diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index 546721c4f6..f26970f9ba 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -195,19 +195,24 @@ def migrate_job_templates(apps, schema_editor): Permission = apps.get_model('main', 'Permission') for jt in JobTemplate.objects.all(): + permission = Permission.objects.filter( + inventory=jt.inventory, + project=jt.project, + active=True, + permission_type__in=['create', 'check', 'run'] if jt.job_type == 'check' else ['create', 'run'], + ) + for team in Team.objects.all(): - if Permission.objects.filter( - team=team, - inventory=jt.inventory, - project=jt.project, - active=True, - permission_type__in=['create', 'check', 'run'] if jt.job_type == 'check' else ['create', 'run'] - ): - team.member_role.children.add(jt.executor_role); + if permission.filter(team=team).exists(): + team.member_role.children.add(jt.executor_role) migrations[jt.name]['teams'].add(team) for user in User.objects.all(): + if permission.filter(user=user).exists(): + jt.exector_role.members.add(user) + migrations[jt.name]['users'].add(user) + if jt.accessible_by(user, {'execute': True}): # If the job template is already accessible by the user, because they # are a sytem, organization, or project admin, then don't add an explicit diff --git a/awx/main/models/__init__.py b/awx/main/models/__init__.py index 85476a19e7..41b866f78c 100644 --- a/awx/main/models/__init__.py +++ b/awx/main/models/__init__.py @@ -37,7 +37,6 @@ _PythonSerializer.handle_m2m_field = _new_handle_m2m_field from django.contrib.auth.models import User # noqa from awx.main.access import * # noqa User.add_to_class('get_queryset', get_user_queryset) -User.add_to_class('can_access', check_user_access) # Import signal handlers only after models have been defined. import awx.main.signals # noqa From fd2212dd33f38111204d1fb073003e318eac1c85 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Tue, 8 Mar 2016 13:47:11 -0500 Subject: [PATCH 6/7] More updates to access, use RBAC for permission checks --- awx/main/access.py | 269 ++++++++++----------------------------------- 1 file changed, 57 insertions(+), 212 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 092652acc2..c8d0ea5047 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -330,13 +330,12 @@ class HostAccess(BaseAccess): model = Host def get_queryset(self): - qs = self.model.objects.filter(active=True).distinct() + qs = self.model.accessible_objects(self.user, {'read':True}) qs = qs.select_related('created_by', 'modified_by', 'inventory', 'last_job__job_template', 'last_job_host_summary__job') qs = qs.prefetch_related('groups') - inventory_ids = set(self.user.get_queryset(Inventory).values_list('id', flat=True)) - return qs.filter(inventory_id__in=inventory_ids) + return qs def can_read(self, obj): return obj and obj.inventory.accessible_by(self.user, {'read':True}) @@ -386,11 +385,10 @@ class GroupAccess(BaseAccess): model = Group def get_queryset(self): - qs = self.model.objects.filter(active=True).distinct() + qs = self.model.accessible_objects(self.user, {'read':True}) qs = qs.select_related('created_by', 'modified_by', 'inventory') qs = qs.prefetch_related('parents', 'children', 'inventory_source') - inventory_ids = set(self.user.get_queryset(Inventory).values_list('id', flat=True)) - return qs.filter(inventory_id__in=inventory_ids) + return qs def can_read(self, obj): return obj and obj.inventory.accessible_by(self.user, {'read':True}) @@ -417,9 +415,6 @@ class GroupAccess(BaseAccess): if not super(GroupAccess, self).can_attach(obj, sub_obj, relationship, data, skip_sub_obj_read_check): return False - # Don't allow attaching if the sub obj is not active - if not obj.active: - return False # Prevent assignments between different inventories. if obj.inventory != sub_obj.inventory: raise ParseError('Cannot associate two items from different inventories') @@ -445,11 +440,9 @@ class InventorySourceAccess(BaseAccess): model = InventorySource def get_queryset(self): - qs = self.model.objects.filter(active=True).distinct() + qs = self.model.accessible_by(self.user, {'read':True}) qs = qs.select_related('created_by', 'modified_by', 'group', 'inventory') - inventory_ids = set(self.user.get_queryset(Inventory).values_list('id', flat=True)) - return qs.filter(Q(inventory_id__in=inventory_ids) | - Q(group__inventory_id__in=inventory_ids)) + return qs def can_read(self, obj): if obj and obj.group: @@ -561,14 +554,9 @@ class TeamAccess(BaseAccess): model = Team def get_queryset(self): - qs = self.model.objects.filter(active=True).distinct() + qs = self.model.accessible_objects(self.user, {'read':True}) qs = qs.select_related('created_by', 'modified_by', 'organization') - if self.user.is_superuser: - return qs - return qs.filter( - Q(organization__admins__in=[self.user], organization__active=True) | - Q(users__in=[self.user]) - ) + return qs def can_add(self, data): if self.user.is_superuser: @@ -585,11 +573,7 @@ class TeamAccess(BaseAccess): org_pk = get_pk_from_dict(data, 'organization') if obj and org_pk and obj.organization.pk != org_pk: raise PermissionDenied('Unable to change organization on a team') - if self.user.is_superuser: - return True - if self.user in obj.organization.admins.all(): - return True - return False + return obj.organization.accessible_by(self.user, ALL_PERMISSIONS) def can_delete(self, obj): return self.can_change(obj, None) @@ -613,48 +597,20 @@ class ProjectAccess(BaseAccess): model = Project def get_queryset(self): - qs = Project.objects.filter(active=True).distinct() + qs = self.model.accessible_objects(self.user, {'read':True}) qs = qs.select_related('modified_by', 'credential', 'current_job', 'last_job') - if self.user.is_superuser: - return qs - team_ids = set(Team.objects.filter(users__in=[self.user]).values_list('id', flat=True)) - qs = qs.filter(Q(created_by=self.user, organizations__isnull=True) | - Q(organizations__admins__in=[self.user], organizations__active=True) | - Q(organizations__users__in=[self.user], organizations__active=True) | - Q(teams__in=team_ids)) - allowed_deploy = [PERM_JOBTEMPLATE_CREATE, PERM_INVENTORY_DEPLOY] - allowed_check = [PERM_JOBTEMPLATE_CREATE, PERM_INVENTORY_DEPLOY, PERM_INVENTORY_CHECK] - - deploy_permissions_ids = set(Permission.objects.filter( - Q(user=self.user) | Q(team_id__in=team_ids), - active=True, - permission_type__in=allowed_deploy, - ).values_list('id', flat=True)) - check_permissions_ids = set(Permission.objects.filter( - Q(user=self.user) | Q(team_id__in=team_ids), - active=True, - permission_type__in=allowed_check, - ).values_list('id', flat=True)) - - perm_deploy_qs = qs.filter(permissions__in=deploy_permissions_ids) - perm_check_qs = qs.filter(permissions__in=check_permissions_ids) - return qs | perm_deploy_qs | perm_check_qs + return qs def can_add(self, data): if self.user.is_superuser: return True - if self.user.admin_of_organizations.filter(active=True).exists(): - return True - return False + qs = Organization.accessible_objects(self.uesr, ALL_PERMISSIONS) + return bool(qs.count() > 0) def can_change(self, obj, data): if self.user.is_superuser: return True - if obj.created_by == self.user and not obj.organizations.filter(active=True).count(): - return True - if obj.organizations.filter(active=True, admins__in=[self.user]).exists(): - return True - return False + return obj.accessible_by(self.user, ALL_PERMISSIONS) def can_delete(self, obj): return self.can_change(obj, None) @@ -699,60 +655,10 @@ class JobTemplateAccess(BaseAccess): model = JobTemplate def get_queryset(self): - qs = self.model.objects.filter(active=True).distinct() + qs = self.model.accessible_objects(self.user, {'read':True}) qs = qs.select_related('created_by', 'modified_by', 'inventory', 'project', 'credential', 'cloud_credential', 'next_schedule') - if self.user.is_superuser: - return qs - credential_ids = self.user.get_queryset(Credential) - inventory_ids = self.user.get_queryset(Inventory) - base_qs = qs.filter( - Q(credential_id__in=credential_ids) | Q(credential__isnull=True), - Q(cloud_credential_id__in=credential_ids) | Q(cloud_credential__isnull=True), - ) - org_admin_ids = base_qs.filter( - Q(project__organizations__admins__in=[self.user]) | - (Q(project__isnull=True) & Q(job_type=PERM_INVENTORY_SCAN) & Q(inventory__organization__admins__in=[self.user])) - ) - - allowed_deploy = [PERM_JOBTEMPLATE_CREATE, PERM_INVENTORY_DEPLOY] - allowed_check = [PERM_JOBTEMPLATE_CREATE, PERM_INVENTORY_DEPLOY, PERM_INVENTORY_CHECK] - - team_ids = Team.objects.filter(users__in=[self.user]) - - # TODO: I think the below queries can be combined - deploy_permissions_ids = Permission.objects.filter( - Q(user=self.user) | Q(team_id__in=team_ids), - active=True, - permission_type__in=allowed_deploy, - ) - check_permissions_ids = Permission.objects.filter( - Q(user=self.user) | Q(team_id__in=team_ids), - active=True, - permission_type__in=allowed_check, - ) - - perm_deploy_ids = base_qs.filter( - job_type=PERM_INVENTORY_DEPLOY, - inventory__permissions__in=deploy_permissions_ids, - project__permissions__in=deploy_permissions_ids, - inventory__permissions__pk=F('project__permissions__pk'), - inventory_id__in=inventory_ids, - ) - - perm_check_ids = base_qs.filter( - job_type=PERM_INVENTORY_CHECK, - inventory__permissions__in=check_permissions_ids, - project__permissions__in=check_permissions_ids, - inventory__permissions__pk=F('project__permissions__pk'), - inventory_id__in=inventory_ids, - ) - - return base_qs.filter( - Q(id__in=org_admin_ids) | - Q(id__in=perm_deploy_ids) | - Q(id__in=perm_check_ids) - ) + return qs def can_read(self, obj): # you can only see the job templates that you have permission to launch. @@ -841,29 +747,7 @@ class JobTemplateAccess(BaseAccess): if obj.project.accessible_by(self.user, ALL_PERMISSIONS): return True - # Otherwise check for explicitly granted permissions - permission_qs = Permission.objects.filter( - Q(user=self.user) | Q(team__users__in=[self.user]), - inventory=obj.inventory, - project=obj.project, - active=True, - permission_type__in=[PERM_JOBTEMPLATE_CREATE, PERM_INVENTORY_CHECK, PERM_INVENTORY_DEPLOY], - ) - - has_perm = False - for perm in permission_qs: - # If you have job template create permission that implies both CHECK and DEPLOY - # If you have DEPLOY permissions you can run both CHECK and DEPLOY - if perm.permission_type in [PERM_JOBTEMPLATE_CREATE, PERM_INVENTORY_DEPLOY] and \ - obj.job_type == PERM_INVENTORY_DEPLOY: - has_perm = True - # If you only have CHECK permission then you can only run CHECK - if perm.permission_type in [PERM_JOBTEMPLATE_CREATE, PERM_INVENTORY_DEPLOY, PERM_INVENTORY_CHECK] and \ - obj.job_type == PERM_INVENTORY_CHECK: - has_perm = True - - dep_access = obj.inventory.accessible_by(self.user, {'read':True}) and obj.project.accessible_by(self.user, {'read':True}) - return dep_access and has_perm + return obj.inventory.accessible_by(self.user, {'read':True}) and obj.project.accessible_by(self.user, {'read':True}) def can_change(self, obj, data): data_for_change = data @@ -1115,7 +999,7 @@ class JobHostSummaryAccess(BaseAccess): model = JobHostSummary def get_queryset(self): - qs = self.model.objects.distinct() + qs = self.model.accessible_objects(self.user, {'read':True}) qs = qs.select_related('job', 'job__job_template', 'host') if self.user.is_superuser: return qs @@ -1140,7 +1024,7 @@ class JobEventAccess(BaseAccess): model = JobEvent def get_queryset(self): - qs = self.model.objects.distinct() + qs = self.model.accessible_objects(self.user, {'read':True}) qs = qs.select_related('job', 'job__job_template', 'host', 'parent') qs = qs.prefetch_related('hosts', 'children') @@ -1177,7 +1061,7 @@ class UnifiedJobTemplateAccess(BaseAccess): model = UnifiedJobTemplate def get_queryset(self): - qs = self.model.objects.filter(active=True).distinct() + qs = self.model.accessible_objects(self.user, {'read':True}) project_qs = self.user.get_queryset(Project).filter(scm_type__in=[s[0] for s in Project.SCM_TYPE_CHOICES]) inventory_source_qs = self.user.get_queryset(InventorySource).filter(source__in=CLOUD_INVENTORY_SOURCES) job_template_qs = self.user.get_queryset(JobTemplate) @@ -1187,15 +1071,17 @@ class UnifiedJobTemplateAccess(BaseAccess): qs = qs.select_related( 'created_by', 'modified_by', - #'project', - #'inventory', - #'credential', - #'cloud_credential', 'next_schedule', 'last_job', 'current_job', ) - # FIXME: Figure out how to do select/prefetch on related project/inventory/credential/cloud_credential. + qs = qs.prefetch_related( + 'project', + 'inventory', + 'credential', + 'cloud_credential', + ) + return qs class UnifiedJobAccess(BaseAccess): @@ -1207,7 +1093,7 @@ class UnifiedJobAccess(BaseAccess): model = UnifiedJob def get_queryset(self): - qs = self.model.objects.filter(active=True).distinct() + qs = self.model.accessible_objects(self.user, {'read':True}) project_update_qs = self.user.get_queryset(ProjectUpdate) inventory_update_qs = self.user.get_queryset(InventoryUpdate).filter(source__in=CLOUD_INVENTORY_SOURCES) job_qs = self.user.get_queryset(Job) @@ -1221,19 +1107,23 @@ class UnifiedJobAccess(BaseAccess): qs = qs.select_related( 'created_by', 'modified_by', - #'project', - #'inventory', - #'credential', - #'project___credential', - #'inventory_source___credential', - #'inventory_source___inventory', - #'job_template___inventory', - #'job_template___project', - #'job_template___credential', - #'job_template___cloud_credential', ) - qs = qs.prefetch_related('unified_job_template') - # FIXME: Figure out how to do select/prefetch on related project/inventory/credential/cloud_credential. + qs = qs.prefetch_related( + 'unified_job_template', + 'project', + 'inventory', + 'credential', + 'job_template', + 'inventory_source', + 'cloud_credential', + 'project___credential', + 'inventory_source___credential', + 'inventory_source___inventory', + 'job_template__inventory', + 'job_template__project', + 'job_template__credential', + 'job_template__cloud_credential', + ) return qs class ScheduleAccess(BaseAccess): @@ -1244,7 +1134,7 @@ class ScheduleAccess(BaseAccess): model = Schedule def get_queryset(self): - qs = self.model.objects.filter(active=True).distinct() + qs = self.model.objects.all() qs = qs.select_related('created_by', 'modified_by') qs = qs.prefetch_related('unified_job_template') if self.user.is_superuser: @@ -1298,7 +1188,7 @@ class NotifierAccess(BaseAccess): model = Notifier def get_queryset(self): - qs = self.model.objects.filter(active=True).distinct() + qs = self.model.objects.distinct() if self.user.is_superuser: return qs return qs @@ -1324,7 +1214,7 @@ class ActivityStreamAccess(BaseAccess): model = ActivityStream def get_queryset(self): - qs = self.model.objects.distinct() + qs = self.model.accessible_objects(self.user, {'read':True}) qs = qs.select_related('actor') qs = qs.prefetch_related('organization', 'user', 'inventory', 'host', 'group', 'inventory_source', 'inventory_update', 'credential', 'team', 'project', 'project_update', @@ -1332,17 +1222,6 @@ class ActivityStreamAccess(BaseAccess): if self.user.is_superuser: return qs - user_admin_orgs = self.user.admin_of_organizations.all() - user_orgs = self.user.organizations.all() - - #Organization filter - qs = qs.filter(Q(organization__admins__in=[self.user]) | Q(organization__users__in=[self.user])) - - #User filter - qs = qs.filter(Q(user__pk=self.user.pk) | - Q(user__organizations__in=user_admin_orgs) | - Q(user__organizations__in=user_orgs)) - #Inventory filter inventory_qs = self.user.get_queryset(Inventory) qs.filter(inventory__in=inventory_qs) @@ -1362,15 +1241,12 @@ class ActivityStreamAccess(BaseAccess): Q(inventory_update__inventory_source__group__inventory__in=inventory_qs)) #Credential Update Filter - qs.filter(Q(credential__user=self.user) | - Q(credential__user__organizations__in=user_admin_orgs) | - Q(credential__user__admin_of_organizations__in=user_admin_orgs) | - Q(credential__team__organization__in=user_admin_orgs) | - Q(credential__team__users__in=[self.user])) + credential_qs = self.user.get_queryset(Credential) + qs.filter(credential__in=credential_qs) #Team Filter - qs.filter(Q(team__organization__admins__in=[self.user]) | - Q(team__users__in=[self.user])) + team_qs = self.user.get_queryset(Team) + qs.filter(team__in=team_qs) #Project Filter project_qs = self.user.get_queryset(Project) @@ -1395,33 +1271,6 @@ class ActivityStreamAccess(BaseAccess): ad_hoc_command_qs = self.user.get_queryset(AdHocCommand) qs.filter(ad_hoc_command__in=ad_hoc_command_qs) - # organization_qs = self.user.get_queryset(Organization) - # user_qs = self.user.get_queryset(User) - # inventory_qs = self.user.get_queryset(Inventory) - # host_qs = self.user.get_queryset(Host) - # group_qs = self.user.get_queryset(Group) - # inventory_source_qs = self.user.get_queryset(InventorySource) - # inventory_update_qs = self.user.get_queryset(InventoryUpdate) - # credential_qs = self.user.get_queryset(Credential) - # team_qs = self.user.get_queryset(Team) - # project_qs = self.user.get_queryset(Project) - # project_update_qs = self.user.get_queryset(ProjectUpdate) - # permission_qs = self.user.get_queryset(Permission) - # job_template_qs = self.user.get_queryset(JobTemplate) - # job_qs = self.user.get_queryset(Job) - # qs = qs.filter(Q(organization__in=organization_qs) | - # Q(user__in=user_qs) | - # Q(inventory__in=inventory_qs) | - # Q(host__in=host_qs) | - # Q(group__in=group_qs) | - # Q(inventory_source__in=inventory_source_qs) | - # Q(credential__in=credential_qs) | - # Q(team__in=team_qs) | - # Q(project__in=project_qs) | - # Q(project_update__in=project_update_qs) | - # Q(permission__in=permission_qs) | - # Q(job_template__in=job_template_qs) | - # Q(job__in=job_qs)) return qs def can_add(self, data): @@ -1438,17 +1287,14 @@ class CustomInventoryScriptAccess(BaseAccess): model = CustomInventoryScript def get_queryset(self): - qs = self.model.objects.filter(active=True).distinct() - if not self.user.is_superuser: - qs = qs.filter(Q(organization__admins__in=[self.user]) | Q(organization__users__in=[self.user])) - return qs + if self.user.is_superuser: + return self.model.objects.distinct() + return self.model.accessible_by(self.user, {'read':True}) def can_read(self, obj): if self.user.is_superuser: return True - if not obj.active: - return False - return bool(obj.organization in self.user.organizations.all() or obj.organization in self.user.admin_of_organizations.all()) + return obj.accessible_by(self.user, {'read':True}) def can_add(self, data): if self.user.is_superuser: @@ -1500,7 +1346,7 @@ class RoleAccess(BaseAccess): def get_queryset(self): if self.user.is_superuser: return self.model.objects.all() - return self.model.visible_roles(self.user) + return self.model.accessible_objects(self.user, {'read':True}) def can_change(self, obj, data): return self.user.is_superuser @@ -1519,7 +1365,7 @@ class RoleAccess(BaseAccess): if obj.object_id and \ isinstance(obj.content_object, ResourceMixin) and \ obj.content_object.accessible_by(self.user, {'write': True}): - return True + return True return False def can_delete(self, obj): @@ -1566,7 +1412,6 @@ register_access(Credential, CredentialAccess) register_access(Team, TeamAccess) register_access(Project, ProjectAccess) register_access(ProjectUpdate, ProjectUpdateAccess) -register_access(Permission, PermissionAccess) register_access(JobTemplate, JobTemplateAccess) register_access(Job, JobAccess) register_access(JobHostSummary, JobHostSummaryAccess) From 3df4c4ec9318f080ba5eaffd884fb688dfa69461 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Tue, 8 Mar 2016 14:09:16 -0500 Subject: [PATCH 7/7] fixing tests and migration issues --- awx/main/migrations/_rbac.py | 2 +- awx/main/tests/functional/test_rbac_team.py | 18 +++++++++--------- .../tests/functional/test_rbac_userresource.py | 12 ++++++------ 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index f26970f9ba..2cf0b6e775 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -210,7 +210,7 @@ def migrate_job_templates(apps, schema_editor): for user in User.objects.all(): if permission.filter(user=user).exists(): - jt.exector_role.members.add(user) + jt.executor_role.members.add(user) migrations[jt.name]['users'].add(user) if jt.accessible_by(user, {'execute': True}): diff --git a/awx/main/tests/functional/test_rbac_team.py b/awx/main/tests/functional/test_rbac_team.py index ad10351fa9..4ba4218c01 100644 --- a/awx/main/tests/functional/test_rbac_team.py +++ b/awx/main/tests/functional/test_rbac_team.py @@ -22,7 +22,7 @@ def test_team_migration_user(team, user, permissions): @pytest.mark.django_db def test_team_access_superuser(team, user): - team.users.add(user('member', False)) + team.member_role.members.add(user('member', False)) access = TeamAccess(user('admin', True)) @@ -31,13 +31,13 @@ def test_team_access_superuser(team, user): assert access.can_delete(team) t = access.get_queryset()[0] - assert len(t.users.all()) == 1 - assert len(t.organization.admins.all()) == 0 + assert len(t.member_role.members.all()) == 1 + assert len(t.organization.admin_role.members.all()) == 0 @pytest.mark.django_db def test_team_access_org_admin(organization, team, user): a = user('admin', False) - organization.admins.add(a) + organization.admin_role.members.add(a) team.organization = organization team.save() @@ -47,13 +47,13 @@ def test_team_access_org_admin(organization, team, user): assert access.can_delete(team) t = access.get_queryset()[0] - assert len(t.users.all()) == 0 - assert len(t.organization.admins.all()) == 1 + assert len(t.member_role.members.all()) == 0 + assert len(t.organization.admin_role.members.all()) == 1 @pytest.mark.django_db def test_team_access_member(organization, team, user): u = user('member', False) - team.users.add(u) + team.member_role.members.add(u) team.organization = organization team.save() @@ -63,6 +63,6 @@ def test_team_access_member(organization, team, user): assert not access.can_delete(team) t = access.get_queryset()[0] - assert len(t.users.all()) == 1 - assert len(t.organization.admins.all()) == 0 + assert len(t.member_role.members.all()) == 1 + assert len(t.organization.admin_role.members.all()) == 0 diff --git a/awx/main/tests/functional/test_rbac_userresource.py b/awx/main/tests/functional/test_rbac_userresource.py index f5e83438df..517b297298 100644 --- a/awx/main/tests/functional/test_rbac_userresource.py +++ b/awx/main/tests/functional/test_rbac_userresource.py @@ -19,13 +19,13 @@ def test_org_user_admin(user, organization): admin = user('orgadmin') member = user('orgmember') - organization.users.add(member) + organization.member_role.members.add(member) assert not member.resource.accessible_by(admin, {'write':True}) - organization.admins.add(admin) + organization.admin_role.members.add(admin) assert member.resource.accessible_by(admin, {'write':True}) - organization.admins.remove(admin) + organization.admin_role.members.remove(admin) assert not member.resource.accessible_by(admin, {'write':True}) @pytest.mark.django_db @@ -33,10 +33,10 @@ def test_org_user_removed(user, organization): admin = user('orgadmin') member = user('orgmember') - organization.admins.add(admin) - organization.users.add(member) + organization.admin_role.members.add(admin) + organization.member_role.members.add(member) assert member.resource.accessible_by(admin, {'write':True}) - organization.users.remove(member) + organization.member_role.members.remove(member) assert not member.resource.accessible_by(admin, {'write':True})