From ee251812b54c7217da3212f3b22efc72d909b1fb Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Wed, 3 Jul 2024 15:40:55 -0400 Subject: [PATCH] Add complete test that we have analogs to old versions of roles, fix some mismatches (#15321) * Add test that we got all permissions right for every role * Fix missing Org execute role and missing adhoc role permission * Add in missing Organization Approval Role as well * Remove Role from role names --- awx/main/migrations/_dab_rbac.py | 54 ++++++++++++++----- .../dab_rbac/test_translation_layer.py | 37 ++++++++++++- 2 files changed, 77 insertions(+), 14 deletions(-) diff --git a/awx/main/migrations/_dab_rbac.py b/awx/main/migrations/_dab_rbac.py index 00e3ec62d9..4f6e7b24a6 100644 --- a/awx/main/migrations/_dab_rbac.py +++ b/awx/main/migrations/_dab_rbac.py @@ -277,7 +277,6 @@ def setup_managed_role_definitions(apps, schema_editor): to_create = { 'object_admin': '{cls.__name__} Admin', 'org_admin': 'Organization Admin', - 'org_audit': 'Organization Audit', 'org_children': 'Organization {cls.__name__} Admin', 'special': '{cls.__name__} {action}', } @@ -334,12 +333,19 @@ def setup_managed_role_definitions(apps, schema_editor): for perm in special_perms: action = perm.codename.split('_')[0] view_perm = Permission.objects.get(content_type=ct, codename__startswith='view_') + perm_list = [perm, view_perm] + # Handle special-case where adhoc role also listed use permission + if action == 'adhoc': + for other_perm in object_perms: + if other_perm.codename == 'use_inventory': + perm_list.append(other_perm) + break managed_role_definitions.append( get_or_create_managed( to_create['special'].format(cls=cls, action=action.title()), f'Has {action} permissions to a single {cls._meta.verbose_name}', ct, - [perm, view_perm], + perm_list, RoleDefinition, ) ) @@ -355,18 +361,40 @@ def setup_managed_role_definitions(apps, schema_editor): ) ) - if 'org_audit' in to_create: - audit_permissions = [perm for perm in org_perms if perm.codename.startswith('view_')] - audit_permissions.append(Permission.objects.get(codename='audit_organization')) - managed_role_definitions.append( - get_or_create_managed( - to_create['org_audit'].format(cls=Organization), - 'Has permission to view all objects inside of a single organization', - org_ct, - audit_permissions, - RoleDefinition, - ) + # Special "organization action" roles + audit_permissions = [perm for perm in org_perms if perm.codename.startswith('view_')] + audit_permissions.append(Permission.objects.get(codename='audit_organization')) + managed_role_definitions.append( + get_or_create_managed( + 'Organization Audit', + 'Has permission to view all objects inside of a single organization', + org_ct, + audit_permissions, + RoleDefinition, ) + ) + + org_execute_permissions = {'view_jobtemplate', 'execute_jobtemplate', 'view_workflowjobtemplate', 'execute_workflowjobtemplate', 'view_organization'} + managed_role_definitions.append( + get_or_create_managed( + 'Organization Execute', + 'Has permission to execute all runnable objects in the organization', + org_ct, + [perm for perm in org_perms if perm.codename in org_execute_permissions], + RoleDefinition, + ) + ) + + org_approval_permissions = {'view_organization', 'view_workflowjobtemplate', 'approve_workflowjobtemplate'} + managed_role_definitions.append( + get_or_create_managed( + 'Organization Approval', + 'Has permission to approve any workflow steps within a single organization', + org_ct, + [perm for perm in org_perms if perm.codename in org_approval_permissions], + RoleDefinition, + ) + ) unexpected_role_definitions = RoleDefinition.objects.filter(managed=True).exclude(pk__in=[rd.pk for rd in managed_role_definitions]) for role_definition in unexpected_role_definitions: diff --git a/awx/main/tests/functional/dab_rbac/test_translation_layer.py b/awx/main/tests/functional/dab_rbac/test_translation_layer.py index 6032820497..dfa019767a 100644 --- a/awx/main/tests/functional/dab_rbac/test_translation_layer.py +++ b/awx/main/tests/functional/dab_rbac/test_translation_layer.py @@ -1,4 +1,5 @@ from unittest import mock +import json import pytest @@ -6,11 +7,13 @@ from django.contrib.contenttypes.models import ContentType from crum import impersonate -from awx.main.models.rbac import get_role_from_object_role, give_creator_permissions +from awx.main.fields import ImplicitRoleField +from awx.main.models.rbac import get_role_from_object_role, give_creator_permissions, get_role_codenames, get_role_definition from awx.main.models import User, Organization, WorkflowJobTemplate, WorkflowJobTemplateNode, Team from awx.api.versioning import reverse from ansible_base.rbac.models import RoleUserAssignment, RoleDefinition +from ansible_base.rbac import permission_registry @pytest.mark.django_db @@ -24,6 +27,7 @@ from ansible_base.rbac.models import RoleUserAssignment, RoleDefinition 'auditor_role', 'read_role', 'execute_role', + 'approval_role', 'notification_admin_role', ], ) @@ -39,6 +43,37 @@ def test_round_trip_roles(organization, rando, role_name, setup_managed_roles): assert old_role.id == getattr(organization, role_name).id +@pytest.mark.django_db +@pytest.mark.parametrize('model', sorted(permission_registry.all_registered_models, key=lambda cls: cls._meta.model_name)) +def test_role_migration_matches(request, model, setup_managed_roles): + fixture_name = model._meta.verbose_name.replace(' ', '_') + obj = request.getfixturevalue(fixture_name) + role_ct = 0 + for field in obj._meta.get_fields(): + if isinstance(field, ImplicitRoleField): + if field.name == 'read_role': + continue # intentionally left as "Compat" roles + role_ct += 1 + old_role = getattr(obj, field.name) + old_codenames = set(get_role_codenames(old_role)) + rd = get_role_definition(old_role) + new_codenames = set(rd.permissions.values_list('codename', flat=True)) + # all the old roles should map to a non-Compat role definition + if 'Compat' not in rd.name: + model_rds = RoleDefinition.objects.filter(content_type=ContentType.objects.get_for_model(obj)) + rd_data = {} + for rd in model_rds: + rd_data[rd.name] = list(rd.permissions.values_list('codename', flat=True)) + assert ( + 'Compat' not in rd.name + ), f'Permissions for old vs new roles did not match.\nold {field.name}: {old_codenames}\nnew:\n{json.dumps(rd_data, indent=2)}' + assert new_codenames == set(old_codenames) + + # In the old system these models did not have object-level roles, all others expect some model roles + if model._meta.model_name not in ('notificationtemplate', 'executionenvironment'): + assert role_ct > 0 + + @pytest.mark.django_db def test_role_naming(setup_managed_roles): qs = RoleDefinition.objects.filter(content_type=ContentType.objects.get(model='jobtemplate'), name__endswith='dmin')