From b59e960b469ac0d1a1aa0ae27937b3af1e8f4996 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Thu, 17 Mar 2016 11:21:10 -0400 Subject: [PATCH 1/4] Credential migration and initial tests --- awx/main/access.py | 34 ++--- .../migrations/0007_v300_rbac_migrations.py | 2 +- awx/main/migrations/_rbac.py | 124 ++++++++++++++++-- .../tests/functional/test_rbac_credential.py | 82 ++++++++++++ 4 files changed, 207 insertions(+), 35 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 2446c94553..4a843520ae 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -58,20 +58,6 @@ 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) @@ -566,14 +552,16 @@ class CredentialAccess(BaseAccess): if self.user.is_superuser: return True - user, team = user_or_team(data) - if user is None and team is None: - return False - - if user is not None: + if 'user' in data: + pk = get_pk_from_dict(data, 'user') + user = get_object_or_400(User, pk=pk) return user.accessible_by(self.user, {'write': True}) - if team is not None: - return team.accessible_by(self.user, {'write':True}) + elif 'organization' in data: + pk = get_pk_from_dict(data, 'organization') + org = get_object_or_400(Organization, pk=pk) + return org.accessible_by(self.user, {'write': True}) + + return False def can_change(self, obj, data): if self.user.is_superuser: @@ -585,8 +573,8 @@ class CredentialAccess(BaseAccess): def can_delete(self, obj): # Unassociated credentials may be marked deleted by anyone, though we # shouldn't ever end up with those. - if obj.user is None and obj.team is None: - return True + #if obj.user is None and obj.team is None: + # return True return self.can_change(obj, None) class TeamAccess(BaseAccess): diff --git a/awx/main/migrations/0007_v300_rbac_migrations.py b/awx/main/migrations/0007_v300_rbac_migrations.py index d50069ab48..cd9d2515ac 100644 --- a/awx/main/migrations/0007_v300_rbac_migrations.py +++ b/awx/main/migrations/0007_v300_rbac_migrations.py @@ -14,8 +14,8 @@ class Migration(migrations.Migration): operations = [ migrations.RunPython(rbac.migrate_users), migrations.RunPython(rbac.migrate_organization), - migrations.RunPython(rbac.migrate_credential), migrations.RunPython(rbac.migrate_team), migrations.RunPython(rbac.migrate_inventory), migrations.RunPython(rbac.migrate_projects), + migrations.RunPython(rbac.migrate_credential), ] diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index 33bcb82b4a..cb677a35ff 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -1,4 +1,7 @@ +import logging + from django.contrib.contenttypes.models import ContentType +from django.db.models import Q from collections import defaultdict import _old_access as old_access @@ -52,18 +55,117 @@ def migrate_team(apps, schema_editor): migrations[t.name].append(user) return migrations +def _update_jt_creds(jts, cred): + for i, jt in enumerate(jts): + if i == 0: + jt.inventory.organization.admin_role.children.add(cred.owner_role) + continue + + cred.pk = None + cred.user = None + cred.save() + + jt.credential = cred + jt.inventory.organization.admin_role.children.add(cred.owner_role) + jt.save() + +def _update_proj_creds(projects, cred): + for i, proj in enumerate(projects): + if i == 0: + proj.organization.admin_role.children.add(cred.owner_role) + continue + + cred.pk = None + cred.user = None + cred.save() + + proj.credential = cred + proj.organization.admin_role.children.add(cred.owner_role) + proj.save() + +def _handle_single_cred(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.save() + +def _handle_multi_cred(insts, cred, org_path='organization'): + def get_org(inst): + fields = org_path.split('.') + for field in fields: + inst = getattr(inst, field) + return inst + + orgs = defaultdict(list) + for inst in insts: + orgs[get_org(inst)].append(inst) + + if len(orgs) == 1: + _handle_single_cred(insts[0].inventory.organization, cred) + else: + for pos, org in enumerate(orgs): + if pos == 0: + _handle_single_cred(org, cred) + else: + cred.pk, cred.user, cred.team = None, None, None + cred.save() + cred.owner_role, cred.usage_role = None, None + cred.save() + + for i in orgs[org]: + i.credential = cred + i.save() + + _handle_single_cred(org, cred) + + def migrate_credential(apps, schema_editor): - migrations = defaultdict(list) - credential = apps.get_model('main', "Credential") - for cred in credential.objects.all(): - if cred.user: - cred.owner_role.members.add(cred.user) - migrations[cred.name].append(cred.user) - elif cred.team: - cred.owner_role.parents.add(cred.team.admin_role) - cred.usage_role.parents.add(cred.team.member_role) - migrations[cred.name].append(cred.team) - return migrations + Credential = apps.get_model('main', "Credential") + JobTemplate = apps.get_model('main', 'JobTemplate') + Project = apps.get_model('main', 'Project') + InventorySource = apps.get_model('main', 'InventorySource') + + + for cred in Credential.objects.all(): + jts = JobTemplate.objects.filter(Q(credential=cred) | Q(cloud_credential=cred)).all() + if jts is not None: + if len(jts) == 1: + _handle_single_cred(jts[0].inventory.organization, cred) + else: + _handle_multi_cred(jts, cred, org_path='inventory.organization') + continue + + invs = InventorySource.objects.filter(credential=cred).all() + if invs is not None: + if len(invs) == 1: + _single_cred(invs[0].inventory.organization, cred) + else: + _multi_cred(invs, cred, org_path='inventory.organization') + continue + + projs = Project.objects.filter(credential=cred).all() + if projs is not None: + if len(projs) == 1: + _single_cred(projs[0].organization, cred) + else: + _multi_cred(projs, cred, org_path='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 = None + cred.team = None + cred.save() + + elif cred.user is not None: + cred.user.admin_role.children.add(cred.owner_role) + cred.user = None + cred.team = None + cred.save() + + # no match found, log + def migrate_inventory(apps, schema_editor): migrations = defaultdict(dict) diff --git a/awx/main/tests/functional/test_rbac_credential.py b/awx/main/tests/functional/test_rbac_credential.py index ec990472a1..efec822e57 100644 --- a/awx/main/tests/functional/test_rbac_credential.py +++ b/awx/main/tests/functional/test_rbac_credential.py @@ -2,6 +2,7 @@ import pytest from awx.main.access import CredentialAccess from awx.main.models.credential import Credential +from awx.main.models.jobs import JobTemplate from awx.main.migrations import _rbac as rbac from django.apps import apps from django.contrib.auth.models import User @@ -96,3 +97,84 @@ def test_credential_access_admin(user, team, credential): # should have can_change access as org-admin assert access.can_change(credential, {'user': u.pk}) + +@pytest.mark.django_db +def test_cred_job_template(user, deploy_jobtemplate): + a = user('admin', False) + org = deploy_jobtemplate.project.organization + org.admin_role.members.add(a) + + cred = deploy_jobtemplate.credential + cred.user = user('john', False) + cred.save() + + access = CredentialAccess(a) + rbac.migrate_credential(apps, None) + assert access.can_change(cred, {'organization': org.pk}) + + org.admin_role.members.remove(a) + assert not access.can_change(cred, {'organization': org.pk}) + +@pytest.mark.django_db +def test_cred_multi_job_template_single_org(user, deploy_jobtemplate): + a = user('admin', False) + org = deploy_jobtemplate.project.organization + org.admin_role.members.add(a) + + cred = deploy_jobtemplate.credential + cred.user = user('john', False) + cred.save() + + access = CredentialAccess(a) + rbac.migrate_credential(apps, None) + assert access.can_change(cred, {'organization': org.pk}) + + org.admin_role.members.remove(a) + assert not access.can_change(cred, {'organization': org.pk}) + +@pytest.mark.django_db +def test_single_cred_multi_job_template_multi_org(user, organizations, credential): + orgs = organizations(2) + jts = [] + for org in orgs: + inv = org.inventories.create(name="inv-%d" % org.pk) + jt = JobTemplate.objects.create( + inventory=inv, + credential=credential, + name="test-jt-org-%d" % org.pk, + job_type='check', + ) + jts.append(jt) + + a = user('admin', False) + orgs[0].admin_role.members.add(a) + orgs[1].admin_role.members.add(a) + + access = CredentialAccess(a) + rbac.migrate_credential(apps, None) + + for jt in jts: + jt.refresh_from_db() + + assert jts[0].credential != jts[1].credential + assert access.can_change(jts[0].credential, {'organization': org.pk}) + assert access.can_change(jts[1].credential, {'organization': org.pk}) + + orgs[0].admin_role.members.remove(a) + assert not access.can_change(jts[0].credential, {'organization': org.pk}) + +@pytest.mark.django_db +def test_cred_single_org(): + pass + +@pytest.mark.django_db +def test_cred_created_by_multi_org(): + pass + +@pytest.mark.django_db +def test_cred_no_org(): + pass + +@pytest.mark.django_db +def test_cred_team(): + pass From 74e2c440a5e9605f30a4fe2677679b65d69c2a7c Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Thu, 17 Mar 2016 11:37:59 -0400 Subject: [PATCH 2/4] Rename credential migration helpers --- awx/main/migrations/_rbac.py | 59 +++++++++++------------------------- 1 file changed, 17 insertions(+), 42 deletions(-) diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index cb677a35ff..d58de7300a 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -55,41 +55,19 @@ def migrate_team(apps, schema_editor): migrations[t.name].append(user) return migrations -def _update_jt_creds(jts, cred): - for i, jt in enumerate(jts): - if i == 0: - jt.inventory.organization.admin_role.children.add(cred.owner_role) - continue - - cred.pk = None - cred.user = None - cred.save() - - jt.credential = cred - jt.inventory.organization.admin_role.children.add(cred.owner_role) - jt.save() - -def _update_proj_creds(projects, cred): - for i, proj in enumerate(projects): - if i == 0: - proj.organization.admin_role.children.add(cred.owner_role) - continue - - cred.pk = None - cred.user = None - cred.save() - - proj.credential = cred - proj.organization.admin_role.children.add(cred.owner_role) - proj.save() - -def _handle_single_cred(org, cred): +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.save() -def _handle_multi_cred(insts, cred, org_path='organization'): +def _discover_credentials(insts, cred, org_path='organization'): + '''_discover_credentials will find shared credentials across + organizations. If a shared credential is found, it will duplicate + the credential, ensure the proper role permissions are added to the new + credential, and update any references from the old to the newly created + credential. + ''' def get_org(inst): fields = org_path.split('.') for field in fields: @@ -101,23 +79,20 @@ def _handle_multi_cred(insts, cred, org_path='organization'): orgs[get_org(inst)].append(inst) if len(orgs) == 1: - _handle_single_cred(insts[0].inventory.organization, cred) + _update_credential_parents(insts[0].inventory.organization, cred) else: for pos, org in enumerate(orgs): if pos == 0: - _handle_single_cred(org, cred) + _update_credential_parents(org, cred) else: cred.pk, cred.user, cred.team = None, None, None cred.save() cred.owner_role, cred.usage_role = None, None cred.save() - for i in orgs[org]: i.credential = cred i.save() - - _handle_single_cred(org, cred) - + _update_credential_parents(org, cred) def migrate_credential(apps, schema_editor): Credential = apps.get_model('main', "Credential") @@ -130,25 +105,25 @@ def migrate_credential(apps, schema_editor): jts = JobTemplate.objects.filter(Q(credential=cred) | Q(cloud_credential=cred)).all() if jts is not None: if len(jts) == 1: - _handle_single_cred(jts[0].inventory.organization, cred) + _update_credential_parents(jts[0].inventory.organization, cred) else: - _handle_multi_cred(jts, cred, org_path='inventory.organization') + _discover_credentials(jts, cred, org_path='inventory.organization') continue invs = InventorySource.objects.filter(credential=cred).all() if invs is not None: if len(invs) == 1: - _single_cred(invs[0].inventory.organization, cred) + _update_credential_parents(invs[0].inventory.organization, cred) else: - _multi_cred(invs, cred, org_path='inventory.organization') + _discover_credentials(invs, cred, org_path='inventory.organization') continue projs = Project.objects.filter(credential=cred).all() if projs is not None: if len(projs) == 1: - _single_cred(projs[0].organization, cred) + _update_credential_parents(projs[0].organization, cred) else: - _multi_cred(projs, cred, org_path='organization') + _discover_credentials(projs, cred, org_path='organization') continue if cred.team is not None: From 6d249f38a7b706df2d861b122e8d3b204524ae70 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Thu, 17 Mar 2016 15:25:09 -0400 Subject: [PATCH 3/4] Fix credential assertions and rename migration helpers --- awx/main/access.py | 2 - awx/main/migrations/_rbac.py | 61 +++++++++++-------- awx/main/models/credential.py | 2 - .../tests/functional/test_rbac_credential.py | 58 +++++++++++++----- 4 files changed, 76 insertions(+), 47 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 4a843520ae..9d67dd5295 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -566,8 +566,6 @@ class CredentialAccess(BaseAccess): def can_change(self, obj, data): if self.user.is_superuser: return True - if not self.can_add(data): - return False return obj.accessible_by(self.user, {'read':True, 'update': True, 'delete':True}) def can_delete(self, obj): diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index d58de7300a..2dc4c287b1 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -1,5 +1,3 @@ -import logging - from django.contrib.contenttypes.models import ContentType from django.db.models import Q @@ -55,31 +53,44 @@ def migrate_team(apps, schema_editor): migrations[t.name].append(user) return migrations +def attrfunc(attr_path): + '''attrfunc returns a function that will + attempt to use the attr_path to access the attribute + of an instance that is passed in to the returned function. + + Example: + get_org = attrfunc('inventory.organization') + org = get_org(JobTemplateInstance) + ''' + def attr(inst): + return reduce(getattr, [inst] + attr_path.split('.')) + return attr + 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.save() -def _discover_credentials(insts, cred, org_path='organization'): +def _discover_credentials(instances, cred, orgfunc): '''_discover_credentials will find shared credentials across organizations. If a shared credential is found, it will duplicate the credential, ensure the proper role permissions are added to the new credential, and update any references from the old to the newly created credential. - ''' - def get_org(inst): - fields = org_path.split('.') - for field in fields: - inst = getattr(inst, field) - return inst + instances is a list of all objects that were matched when filtered + with cred. + + orgfunc is a function that when called with an instance from instances + will produce an Organization object. + ''' orgs = defaultdict(list) - for inst in insts: - orgs[get_org(inst)].append(inst) + for inst in instances: + orgs[orgfunc(inst)].append(inst) if len(orgs) == 1: - _update_credential_parents(insts[0].inventory.organization, cred) + _update_credential_parents(instances[0].inventory.organization, cred) else: for pos, org in enumerate(orgs): if pos == 0: @@ -100,30 +111,25 @@ def migrate_credential(apps, schema_editor): Project = apps.get_model('main', 'Project') InventorySource = apps.get_model('main', 'InventorySource') - + migrated = [] for cred in Credential.objects.all(): - jts = JobTemplate.objects.filter(Q(credential=cred) | Q(cloud_credential=cred)).all() - if jts is not None: - if len(jts) == 1: - _update_credential_parents(jts[0].inventory.organization, cred) - else: - _discover_credentials(jts, cred, org_path='inventory.organization') - continue + migrated.append(cred) - invs = InventorySource.objects.filter(credential=cred).all() - if invs is not None: - if len(invs) == 1: - _update_credential_parents(invs[0].inventory.organization, cred) + results = (JobTemplate.objects.filter(Q(credential=cred) | Q(cloud_credential=cred)).all() or + InventorySource.objects.filter(credential=cred).all()) + if results: + if len(results) == 1: + _update_credential_parents(results[0].inventory.organization, cred) else: - _discover_credentials(invs, cred, org_path='inventory.organization') + _discover_credentials(results, cred, attrfunc('inventory.organization')) continue projs = Project.objects.filter(credential=cred).all() - if projs is not None: + if projs: if len(projs) == 1: _update_credential_parents(projs[0].organization, cred) else: - _discover_credentials(projs, cred, org_path='organization') + _discover_credentials(projs, cred, attrfunc('organization')) continue if cred.team is not None: @@ -140,6 +146,7 @@ def migrate_credential(apps, schema_editor): cred.save() # no match found, log + return migrated def migrate_inventory(apps, schema_editor): diff --git a/awx/main/models/credential.py b/awx/main/models/credential.py index 9ae6b47298..1d59049326 100644 --- a/awx/main/models/credential.py +++ b/awx/main/models/credential.py @@ -163,7 +163,6 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin): role_name='Credential Owner', role_description='Owner of the credential', parent_role=[ - 'team.admin_role', 'singleton:' + ROLE_SINGLETON_SYSTEM_ADMINISTRATOR, ], permissions = {'all': True} @@ -179,7 +178,6 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin): usage_role = ImplicitRoleField( role_name='Credential User', role_description='May use this credential, but not read sensitive portions or modify it', - parent_role= 'team.member_role', permissions = {'use': True} ) diff --git a/awx/main/tests/functional/test_rbac_credential.py b/awx/main/tests/functional/test_rbac_credential.py index efec822e57..f4ea00e68c 100644 --- a/awx/main/tests/functional/test_rbac_credential.py +++ b/awx/main/tests/functional/test_rbac_credential.py @@ -3,6 +3,7 @@ import pytest from awx.main.access import CredentialAccess from awx.main.models.credential import Credential from awx.main.models.jobs import JobTemplate +from awx.main.models.inventory import InventorySource from awx.main.migrations import _rbac as rbac from django.apps import apps from django.contrib.auth.models import User @@ -50,9 +51,6 @@ def test_credential_migration_team_admin(credential, team, user, permissions): credential.team = team credential.save() - # No permissions pre-migration - team.admin_role.children.remove(credential.owner_role) - team.member_role.children.remove(credential.usage_role) assert not credential.accessible_by(u, permissions['usage']) # Usage permissions post migration @@ -77,17 +75,15 @@ def test_credential_access_admin(user, team, credential): access = CredentialAccess(u) assert access.can_add({'user': u.pk}) - assert access.can_add({'team': team.pk}) - assert not access.can_change(credential, {'user': u.pk}) - # unowned credential can be deleted - assert access.can_delete(credential) + # unowned credential is superuser only + assert not access.can_delete(credential) # credential is now part of a team # that is part of an organization # that I am an admin for - credential.team = team + credential.owner_role.parents.add(team.admin_role) credential.save() credential.owner_role.rebuild_role_ancestor_list() @@ -164,17 +160,47 @@ def test_single_cred_multi_job_template_multi_org(user, organizations, credentia assert not access.can_change(jts[0].credential, {'organization': org.pk}) @pytest.mark.django_db -def test_cred_single_org(): - pass +def test_cred_inventory_source(user, inventory, credential): + u = user('member', False) + inventory.organization.member_role.members.add(u) + + InventorySource.objects.create( + name="test-inv-src", + credential=credential, + inventory=inventory, + ) + + assert not credential.accessible_by(u, {'use':True}) + + rbac.migrate_credential(apps, None) + assert credential.accessible_by(u, {'use':True}) @pytest.mark.django_db -def test_cred_created_by_multi_org(): - pass +def test_cred_project(user, credential, project): + u = user('member', False) + project.organization.member_role.members.add(u) + project.credential = credential + project.save() + + assert not credential.accessible_by(u, {'use':True}) + + rbac.migrate_credential(apps, None) + assert credential.accessible_by(u, {'use':True}) @pytest.mark.django_db -def test_cred_no_org(): - pass +def test_cred_no_org(user, credential): + su = user('su', True) + access = CredentialAccess(su) + assert access.can_change(credential, {'user': su.pk}) @pytest.mark.django_db -def test_cred_team(): - pass +def test_cred_team(user, team, credential): + u = user('a', False) + team.member_role.members.add(u) + credential.team = team + credential.save() + + assert not credential.accessible_by(u, {'use':True}) + + rbac.migrate_credential(apps, None) + assert credential.accessible_by(u, {'use':True}) From 766190fb80b202ced0bf4b68cde6342425a25105 Mon Sep 17 00:00:00 2001 From: Wayne Witzel III Date: Thu, 17 Mar 2016 16:27:47 -0400 Subject: [PATCH 4/4] Use getattrd instead of reduce --- awx/main/migrations/_rbac.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/awx/main/migrations/_rbac.py b/awx/main/migrations/_rbac.py index 2dc4c287b1..a333ff0233 100644 --- a/awx/main/migrations/_rbac.py +++ b/awx/main/migrations/_rbac.py @@ -2,6 +2,7 @@ from django.contrib.contenttypes.models import ContentType from django.db.models import Q from collections import defaultdict +from awx.main.utils import getattrd import _old_access as old_access def migrate_users(apps, schema_editor): @@ -63,7 +64,7 @@ def attrfunc(attr_path): org = get_org(JobTemplateInstance) ''' def attr(inst): - return reduce(getattr, [inst] + attr_path.split('.')) + return getattrd(inst, attr_path) return attr def _update_credential_parents(org, cred): @@ -96,10 +97,15 @@ def _discover_credentials(instances, cred, orgfunc): if pos == 0: _update_credential_parents(org, cred) else: - cred.pk, cred.user, cred.team = None, None, None + # Create a new credential + cred.pk = None cred.save() + + # Unlink the old information from the new credential + cred.user, cred.team = None, None cred.owner_role, cred.usage_role = None, None cred.save() + for i in orgs[org]: i.credential = cred i.save() @@ -135,14 +141,12 @@ def migrate_credential(apps, schema_editor): 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 = None - cred.team = None + cred.user, cred.team = None, None cred.save() elif cred.user is not None: cred.user.admin_role.children.add(cred.owner_role) - cred.user = None - cred.team = None + cred.user, cred.team = None, None cred.save() # no match found, log