Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do some aesthetic adjustments to role presentation fields #15153

Merged
merged 5 commits into from
Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion awx/main/migrations/_dab_rbac.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,12 @@ def setup_managed_role_definitions(apps, schema_editor):
"""
Idepotent method to create or sync the managed role definitions
"""
to_create = settings.ANSIBLE_BASE_ROLE_PRECREATE
to_create = {
'object_admin': '{cls.__name__} Admin',
'org_admin': 'Organization Admin',
'org_children': 'Organization {cls.__name__} Admin',
'special': '{cls.__name__} {action}',
}

ContentType = apps.get_model('contenttypes', 'ContentType')
Permission = apps.get_model('dab_rbac', 'DABPermission')
Expand Down
28 changes: 18 additions & 10 deletions awx/main/models/rbac.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
# django-rest-framework
from rest_framework.serializers import ValidationError

# crum to impersonate users
from crum import impersonate

# Django
from django.db import models, transaction, connection
from django.db.models.signals import m2m_changed
Expand Down Expand Up @@ -553,17 +556,22 @@ def get_role_definition(role):
return
f = obj._meta.get_field(role.role_field)
action_name = f.name.rsplit("_", 1)[0]
rd_name = f'{type(obj).__name__} {action_name.title()} Compat'
model_print = type(obj).__name__
rd_name = f'{model_print} {action_name.title()} Compat'
perm_list = get_role_codenames(role)
defaults = {'content_type_id': role.content_type_id}
try:
rd, created = RoleDefinition.objects.get_or_create(name=rd_name, permissions=perm_list, defaults=defaults)
except ValidationError:
# This is a tricky case - practically speaking, users should not be allowed to create team roles
# or roles that include the team member permission.
# If we need to create this for compatibility purposes then we will create it as a managed non-editable role
defaults['managed'] = True
rd, created = RoleDefinition.objects.get_or_create(name=rd_name, permissions=perm_list, defaults=defaults)
defaults = {
'content_type_id': role.content_type_id,
'description': f'Has {action_name.title()} permission to {model_print} for backwards API compatibility',
}
with impersonate(None):
try:
rd, created = RoleDefinition.objects.get_or_create(name=rd_name, permissions=perm_list, defaults=defaults)
except ValidationError:
# This is a tricky case - practically speaking, users should not be allowed to create team roles
# or roles that include the team member permission.
# If we need to create this for compatibility purposes then we will create it as a managed non-editable role
defaults['managed'] = True
rd, created = RoleDefinition.objects.get_or_create(name=rd_name, permissions=perm_list, defaults=defaults)
return rd


Expand Down
4 changes: 2 additions & 2 deletions awx/main/tests/functional/api/test_credential.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def test_idempotent_credential_type_setup():


@pytest.mark.django_db
def test_create_user_credential_via_credentials_list(post, get, alice, credentialtype_ssh):
def test_create_user_credential_via_credentials_list(post, get, alice, credentialtype_ssh, setup_managed_roles):
params = {
'credential_type': 1,
'inputs': {'username': 'someusername'},
Expand Down Expand Up @@ -81,7 +81,7 @@ def test_credential_validation_error_with_multiple_owner_fields(post, admin, ali


@pytest.mark.django_db
def test_create_user_credential_via_user_credentials_list(post, get, alice, credentialtype_ssh):
def test_create_user_credential_via_user_credentials_list(post, get, alice, credentialtype_ssh, setup_managed_roles):
params = {
'credential_type': 1,
'inputs': {'username': 'someusername'},
Expand Down
8 changes: 8 additions & 0 deletions awx/main/tests/functional/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

from django.db.models.signals import post_migrate

from awx.main.migrations._dab_rbac import setup_managed_role_definitions

# AWX
from awx.main.models.projects import Project
from awx.main.models.ha import Instance
Expand Down Expand Up @@ -90,6 +92,12 @@ def deploy_jobtemplate(project, inventory, credential):
return jt


@pytest.fixture
def setup_managed_roles():
"Run the migration script to pre-create managed role definitions"
setup_managed_role_definitions(apps, None)


@pytest.fixture
def team(organization):
return organization.teams.create(name='test-team')
Expand Down
10 changes: 0 additions & 10 deletions awx/main/tests/functional/dab_rbac/conftest.py

This file was deleted.

45 changes: 0 additions & 45 deletions awx/main/tests/functional/dab_rbac/test_dab_migration.py

This file was deleted.

10 changes: 5 additions & 5 deletions awx/main/tests/functional/dab_rbac/test_dab_rbac_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@


@pytest.mark.django_db
def test_managed_roles_created(managed_roles):
def test_managed_roles_created(setup_managed_roles):
"Managed RoleDefinitions are created in post_migration signal, we expect to see them here"
for cls in (JobTemplate, Inventory):
ct = ContentType.objects.get_for_model(cls)
Expand All @@ -22,7 +22,7 @@ def test_managed_roles_created(managed_roles):


@pytest.mark.django_db
def test_custom_read_role(admin_user, post, managed_roles):
def test_custom_read_role(admin_user, post, setup_managed_roles):
rd_url = django_reverse('roledefinition-list')
resp = post(
url=rd_url, data={"name": "read role made for test", "content_type": "awx.inventory", "permissions": ['view_inventory']}, user=admin_user, expect=201
Expand All @@ -40,7 +40,7 @@ def test_custom_system_roles_prohibited(admin_user, post):


@pytest.mark.django_db
def test_assignment_to_invisible_user(admin_user, alice, rando, inventory, post, managed_roles):
def test_assignment_to_invisible_user(admin_user, alice, rando, inventory, post, setup_managed_roles):
"Alice can not see rando, and so can not give them a role assignment"
rd = RoleDefinition.objects.get(name='Inventory Admin')
rd.give_permission(alice, inventory)
Expand All @@ -51,7 +51,7 @@ def test_assignment_to_invisible_user(admin_user, alice, rando, inventory, post,


@pytest.mark.django_db
def test_assign_managed_role(admin_user, alice, rando, inventory, post, managed_roles, organization):
def test_assign_managed_role(admin_user, alice, rando, inventory, post, setup_managed_roles, organization):
rd = RoleDefinition.objects.get(name='Inventory Admin')
rd.give_permission(alice, inventory)
# When alice and rando are members of the same org, they can see each other
Expand All @@ -78,7 +78,7 @@ def test_assign_custom_delete_role(admin_user, rando, inventory, delete, patch):


@pytest.mark.django_db
def test_assign_custom_add_role(admin_user, rando, organization, post, managed_roles):
def test_assign_custom_add_role(admin_user, rando, organization, post, setup_managed_roles):
rd, _ = RoleDefinition.objects.get_or_create(
name='inventory-add', permissions=['add_inventory', 'view_organization'], content_type=ContentType.objects.get_for_model(Organization)
)
Expand Down
50 changes: 43 additions & 7 deletions awx/main/tests/functional/dab_rbac/test_translation_layer.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,23 @@

import pytest

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.models import User, Organization, WorkflowJobTemplate, WorkflowJobTemplateNode, Team
from awx.api.versioning import reverse

from ansible_base.rbac.models import RoleUserAssignment
from ansible_base.rbac.models import RoleUserAssignment, RoleDefinition


@pytest.mark.django_db
@pytest.mark.parametrize(
'role_name',
['execution_environment_admin_role', 'project_admin_role', 'admin_role', 'auditor_role', 'read_role', 'execute_role', 'notification_admin_role'],
)
def test_round_trip_roles(organization, rando, role_name, managed_roles):
def test_round_trip_roles(organization, rando, role_name, setup_managed_roles):
"""
Make an assignment with the old-style role,
get the equivelent new role
Expand All @@ -28,7 +32,39 @@ def test_round_trip_roles(organization, rando, role_name, managed_roles):


@pytest.mark.django_db
def test_organization_level_permissions(organization, inventory, managed_roles):
def test_role_naming(setup_managed_roles):
qs = RoleDefinition.objects.filter(content_type=ContentType.objects.get(model='jobtemplate'), name__endswith='dmin')
assert qs.count() == 1 # sanity
rd = qs.first()
assert rd.name == 'JobTemplate Admin'
assert rd.description
assert rd.created_by is None


@pytest.mark.django_db
def test_action_role_naming(setup_managed_roles):
qs = RoleDefinition.objects.filter(content_type=ContentType.objects.get(model='jobtemplate'), name__endswith='ecute')
assert qs.count() == 1 # sanity
rd = qs.first()
assert rd.name == 'JobTemplate Execute'
assert rd.description
assert rd.created_by is None


@pytest.mark.django_db
def test_compat_role_naming(setup_managed_roles, job_template, rando, alice):
with impersonate(alice):
job_template.read_role.members.add(rando)
qs = RoleDefinition.objects.filter(content_type=ContentType.objects.get(model='jobtemplate'), name__endswith='ompat')
assert qs.count() == 1 # sanity
rd = qs.first()
assert rd.name == 'JobTemplate Read Compat'
assert rd.description
assert rd.created_by is None


@pytest.mark.django_db
def test_organization_level_permissions(organization, inventory, setup_managed_roles):
u1 = User.objects.create(username='alice')
u2 = User.objects.create(username='bob')

Expand Down Expand Up @@ -58,14 +94,14 @@ def test_organization_level_permissions(organization, inventory, managed_roles):


@pytest.mark.django_db
def test_organization_execute_role(organization, rando, managed_roles):
def test_organization_execute_role(organization, rando, setup_managed_roles):
organization.execute_role.members.add(rando)
assert rando in organization.execute_role
assert set(Organization.accessible_objects(rando, 'execute_role')) == set([organization])


@pytest.mark.django_db
def test_workflow_approval_list(get, post, admin_user, managed_roles):
def test_workflow_approval_list(get, post, admin_user, setup_managed_roles):
workflow_job_template = WorkflowJobTemplate.objects.create()
approval_node = WorkflowJobTemplateNode.objects.create(workflow_job_template=workflow_job_template)
url = reverse('api:workflow_job_template_node_create_approval', kwargs={'pk': approval_node.pk, 'version': 'v2'})
Expand All @@ -79,14 +115,14 @@ def test_workflow_approval_list(get, post, admin_user, managed_roles):


@pytest.mark.django_db
def test_creator_permission(rando, admin_user, inventory, managed_roles):
def test_creator_permission(rando, admin_user, inventory, setup_managed_roles):
give_creator_permissions(rando, inventory)
assert rando in inventory.admin_role
assert rando in inventory.admin_role.members.all()


@pytest.mark.django_db
def test_team_team_read_role(rando, team, admin_user, post, managed_roles):
def test_team_team_read_role(rando, team, admin_user, post, setup_managed_roles):
orgs = [Organization.objects.create(name=f'foo-{i}') for i in range(2)]
teams = [Team.objects.create(name=f'foo-{i}', organization=orgs[i]) for i in range(2)]
teams[1].member_role.members.add(rando)
Expand Down
2 changes: 1 addition & 1 deletion awx/main/tests/functional/test_rbac_job_templates.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ def test_system_admin_orphan_capabilities(self, job_template, admin_user):

@pytest.mark.django_db
@pytest.mark.job_permissions
def test_job_template_creator_access(project, organization, rando, post):
def test_job_template_creator_access(project, organization, rando, post, setup_managed_roles):
project.use_role.members.add(rando)
response = post(
url=reverse('api:job_template_list'),
Expand Down
9 changes: 2 additions & 7 deletions awx/settings/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -1148,13 +1148,8 @@

# Settings for the ansible_base RBAC system

# Only used internally, names of the managed RoleDefinitions to create
ANSIBLE_BASE_ROLE_PRECREATE = {
'object_admin': '{cls.__name__} Admin',
'org_admin': 'Organization Admin',
'org_children': 'Organization {cls.__name__} Admin',
'special': '{cls.__name__} {action}',
}
# This has been moved to data migration code
ANSIBLE_BASE_ROLE_PRECREATE = {}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a big goof. Since I had put up ansible/django-ansible-base#296 I confused myself about the contract with DAB. This was unexpectedly creating the managed role definitions in the post_migrate hook, in addition to AWX logic I added that did the same thing. Had it been removed in DAB (i.e. 296 was merged) that might have worked, but the path forward isn't super clear for that and I have cold feet on it.

In other words, we had duplicate role definitions which was causing unexpected names to be found.


# Name for auto-created roles that give users permissions to what they create
ANSIBLE_BASE_ROLE_CREATOR_NAME = '{cls.__name__} Creator'
Expand Down