diff --git a/awx/api/generics.py b/awx/api/generics.py index 5b68bb4195..a0c609db26 100644 --- a/awx/api/generics.py +++ b/awx/api/generics.py @@ -971,7 +971,6 @@ class CopyAPIView(GenericAPIView): None, None, self.model, obj, request.user, create_kwargs=create_kwargs, copy_name=serializer.validated_data.get('name', '') ) if hasattr(new_obj, 'admin_role') and request.user not in new_obj.admin_role.members.all(): - new_obj.admin_role.members.add(request.user) give_creator_permissions(request.user, new_obj) if sub_objs: permission_check_func = None diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 2423862d24..c0577324c2 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -3142,7 +3142,6 @@ class CredentialSerializerCreate(CredentialSerializer): credential = super(CredentialSerializerCreate, self).create(validated_data) if user: - credential.admin_role.members.add(user) give_creator_permissions(user, credential) if team: if not credential.organization or team.organization.id != credential.organization.id: diff --git a/awx/api/views/__init__.py b/awx/api/views/__init__.py index fba3260e4c..944f982a4d 100644 --- a/awx/api/views/__init__.py +++ b/awx/api/views/__init__.py @@ -90,7 +90,7 @@ from awx.api.generics import ( from awx.api.views.labels import LabelSubListCreateAttachDetachView from awx.api.versioning import reverse from awx.main import models -from awx.main.models.rbac import give_creator_permissions, get_role_definition +from awx.main.models.rbac import get_role_definition from awx.main.utils import ( camelcase_to_underscore, extract_ansible_vars, @@ -2314,14 +2314,6 @@ class JobTemplateList(ListCreateAPIView): serializer_class = serializers.JobTemplateSerializer always_allow_superuser = False - def post(self, request, *args, **kwargs): - ret = super(JobTemplateList, self).post(request, *args, **kwargs) - if ret.status_code == 201: - job_template = models.JobTemplate.objects.get(id=ret.data['id']) - job_template.admin_role.members.add(request.user) - give_creator_permissions(request.user, job_template) - return ret - class JobTemplateDetail(RelatedJobsPreventDeleteMixin, RetrieveUpdateDestroyAPIView): model = models.JobTemplate diff --git a/awx/main/access.py b/awx/main/access.py index f12bd824a5..505c1de918 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -652,7 +652,6 @@ class UserAccess(BaseAccess): User.objects.filter(pk__in=Organization.accessible_objects(self.user, 'read_role').values('member_role__members')) | User.objects.filter(pk=self.user.id) | User.objects.filter(is_superuser=True) - | User.objects.filter(profile__is_system_auditor=True) ).distinct() return qs diff --git a/awx/main/migrations/0190_add_django_permissions.py b/awx/main/migrations/0190_add_django_permissions.py index 39202cdd2e..540c1c6b40 100644 --- a/awx/main/migrations/0190_add_django_permissions.py +++ b/awx/main/migrations/0190_add_django_permissions.py @@ -7,6 +7,7 @@ import django.db.models.deletion class Migration(migrations.Migration): dependencies = [ ('main', '0189_inbound_hop_nodes'), + ('dab_rbac', '__first__'), ] operations = [ diff --git a/awx/main/migrations/0192_custom_roles.py b/awx/main/migrations/0191_custom_roles.py similarity index 94% rename from awx/main/migrations/0192_custom_roles.py rename to awx/main/migrations/0191_custom_roles.py index ce6b7ba9e1..8a203bb75c 100644 --- a/awx/main/migrations/0192_custom_roles.py +++ b/awx/main/migrations/0191_custom_roles.py @@ -9,7 +9,7 @@ from ansible_base.rbac.migrations._managed_definitions import setup_managed_role class Migration(migrations.Migration): dependencies = [ - ('main', '0191_profile_is_system_auditor'), + ('main', '0190_add_django_permissions'), ('dab_rbac', '__first__'), ] diff --git a/awx/main/migrations/0191_profile_is_system_auditor.py b/awx/main/migrations/0191_profile_is_system_auditor.py deleted file mode 100644 index a7f7c18813..0000000000 --- a/awx/main/migrations/0191_profile_is_system_auditor.py +++ /dev/null @@ -1,20 +0,0 @@ -# Generated by Django 4.2.6 on 2023-11-20 16:30 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - dependencies = [ - ('main', '0190_add_django_permissions'), - ] - run_before = [ - ('dab_rbac', '__first__'), - ] - - operations = [ - migrations.AddField( - model_name='profile', - name='is_system_auditor', - field=models.BooleanField(default=False, help_text='Can view everying in the system, proxies to User model'), - ), - ] diff --git a/awx/main/migrations/_dab_rbac.py b/awx/main/migrations/_dab_rbac.py index 18c87697a9..1920653186 100644 --- a/awx/main/migrations/_dab_rbac.py +++ b/awx/main/migrations/_dab_rbac.py @@ -144,6 +144,7 @@ def migrate_to_new_rbac(apps, schema_editor): """ Role = apps.get_model('main', 'Role') RoleDefinition = apps.get_model('dab_rbac', 'RoleDefinition') + RoleUserAssignment = apps.get_model('dab_rbac', 'RoleUserAssignment') Permission = apps.get_model('auth', 'Permission') migration_time = now() @@ -224,14 +225,25 @@ def migrate_to_new_rbac(apps, schema_editor): content_type_id=role.content_type_id, ) + # Create new replacement system auditor role + new_system_auditor, created = RoleDefinition.objects.get_or_create( + name='System Auditor', + defaults={ + 'description': 'Migrated singleton role giving read permission to everything', + 'managed': True, + 'created_on': migration_time, + 'modified_on': migration_time, + }, + ) + new_system_auditor.permissions.add(*list(Permission.objects.filter(codename__startswith='view'))) + # migrate is_system_auditor flag, because it is no longer handled by a system role - role = Role.objects.filter(singleton_name='system_auditor').first() - if role: + old_system_auditor = Role.objects.filter(singleton_name='system_auditor').first() + if old_system_auditor: # if the system auditor role is not present, this is a new install and no users should exist ct = 0 for user in role.members.all(): - user.profile.is_system_auditor = True - user.profile.save(update_fields=['is_system_auditor']) + RoleUserAssignment.objects.create(user=user, role_definition=new_system_auditor) ct += 1 if ct: logger.info(f'Migrated {ct} users to new system auditor flag') diff --git a/awx/main/models/__init__.py b/awx/main/models/__init__.py index cf16b9f258..dce3f371d0 100644 --- a/awx/main/models/__init__.py +++ b/awx/main/models/__init__.py @@ -11,6 +11,7 @@ from django.db.models.signals import pre_delete # noqa # django-ansible-base from ansible_base.resource_registry.fields import AnsibleResourceField from ansible_base.rbac import permission_registry +from ansible_base.rbac.models import RoleDefinition, RoleUserAssignment from ansible_base.lib.utils.models import prevent_search from ansible_base.lib.utils.models import user_summary_fields @@ -199,11 +200,21 @@ User.add_to_class('auditor_of_organizations', user_get_auditor_of_organizations) User.add_to_class('created', created) +def get_system_auditor_role(): + rd, created = RoleDefinition.objects.get_or_create( + name='System Auditor', defaults={'description': 'Migrated singleton role giving read permission to everything'} + ) + if created: + rd.permissions.add(*list(permission_registry.permission_qs.filter(codename__startswith='view'))) + return rd + + @property def user_is_system_auditor(user): if not hasattr(user, '_is_system_auditor'): if user.pk: - user._is_system_auditor = user.profile.is_system_auditor + rd = get_system_auditor_role() + user._is_system_auditor = RoleUserAssignment.objects.filter(user=user, role_definition=rd).exists() else: # Odd case where user is unsaved, this should never be relied on return False @@ -217,11 +228,15 @@ def user_is_system_auditor(user, tf): # time they've logged in, and we've just created the new User in this # request), we need one to set up the system auditor role user.save() - if user.profile.is_system_auditor != bool(tf): - prior_value = user.profile.is_system_auditor - user.profile.is_system_auditor = bool(tf) - user.profile.save(update_fields=['is_system_auditor']) - user._is_system_auditor = user.profile.is_system_auditor + rd = get_system_auditor_role() + assignment = RoleUserAssignment.objects.filter(user=user, role_definition=rd).first() + prior_value = bool(assignment) + if prior_value != bool(tf): + if assignment: + assignment.delete() + else: + rd.give_global_permission(user) + user._is_system_auditor = bool(tf) entry = ActivityStream.objects.create(changes=json.dumps({"is_system_auditor": [prior_value, bool(tf)]}), object1='user', operation='update') entry.user.add(user) diff --git a/awx/main/models/organization.py b/awx/main/models/organization.py index c34dacdde5..e543a91f80 100644 --- a/awx/main/models/organization.py +++ b/awx/main/models/organization.py @@ -182,7 +182,6 @@ class Profile(CreatedModifiedModel): max_length=1024, default='', ) - is_system_auditor = models.BooleanField(default=False, help_text=_('Can view everying in the system, proxies to User model')) class UserSessionMembership(BaseModel): diff --git a/awx/main/models/rbac.py b/awx/main/models/rbac.py index 92aac71cd2..d99f52f4dd 100644 --- a/awx/main/models/rbac.py +++ b/awx/main/models/rbac.py @@ -7,6 +7,9 @@ import threading import contextlib import re +# django-rest-framework +from rest_framework.serializers import ValidationError + # Django from django.db import models, transaction, connection from django.db.models.signals import m2m_changed @@ -552,7 +555,15 @@ def get_role_definition(role): action_name = f.name.rsplit("_", 1)[0] rd_name = f'{obj._meta.model_name}-{action_name}-compat' perm_list = get_role_codenames(role) - rd, created = RoleDefinition.objects.get_or_create(name=rd_name, permissions=perm_list, defaults={'content_type_id': role.content_type_id}) + 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) return rd diff --git a/awx/main/tests/functional/dab_rbac/test_dab_rbac_api.py b/awx/main/tests/functional/dab_rbac/test_dab_rbac_api.py index 00976f2d2a..7db61ae04e 100644 --- a/awx/main/tests/functional/dab_rbac/test_dab_rbac_api.py +++ b/awx/main/tests/functional/dab_rbac/test_dab_rbac_api.py @@ -32,6 +32,13 @@ def test_custom_read_role(admin_user, post): assert rd.content_type == ContentType.objects.get_for_model(Inventory) +@pytest.mark.django_db +def test_custom_system_roles_prohibited(admin_user, post): + rd_url = django_reverse('roledefinition-list') + resp = post(url=rd_url, data={"name": "read role made for test", "content_type": None, "permissions": ['view_inventory']}, user=admin_user, expect=400) + assert 'System-wide roles are not enabled' in str(resp.data) + + @pytest.mark.django_db def test_assign_managed_role(admin_user, alice, rando, inventory, post): rd = RoleDefinition.objects.get(name='inventory-admin') diff --git a/awx/main/tests/functional/test_rbac_job_templates.py b/awx/main/tests/functional/test_rbac_job_templates.py index 5af5c9707b..17e7ff3524 100644 --- a/awx/main/tests/functional/test_rbac_job_templates.py +++ b/awx/main/tests/functional/test_rbac_job_templates.py @@ -177,7 +177,7 @@ def test_job_template_creator_access(project, organization, rando, post): jt_pk = response.data['id'] jt_obj = JobTemplate.objects.get(pk=jt_pk) # Creating a JT should place the creator in the admin role - assert rando in jt_obj.admin_role.members.all() + assert rando in jt_obj.admin_role @pytest.mark.django_db diff --git a/awx/main/tests/functional/test_rbac_user.py b/awx/main/tests/functional/test_rbac_user.py index 4b9a2b78ef..10ca851bbe 100644 --- a/awx/main/tests/functional/test_rbac_user.py +++ b/awx/main/tests/functional/test_rbac_user.py @@ -4,7 +4,7 @@ from unittest import mock from django.test import TransactionTestCase from awx.main.access import UserAccess, RoleAccess, TeamAccess -from awx.main.models import User, Organization, Inventory +from awx.main.models import User, Organization, Inventory, get_system_auditor_role class TestSysAuditorTransactional(TransactionTestCase): @@ -18,6 +18,7 @@ class TestSysAuditorTransactional(TransactionTestCase): def test_auditor_caching(self): rando = self.rando() + get_system_auditor_role() # pre-create role, normally done by migrations with self.assertNumQueries(2): v = rando.is_system_auditor assert not v diff --git a/awx/settings/defaults.py b/awx/settings/defaults.py index 556d899e96..98a5d69212 100644 --- a/awx/settings/defaults.py +++ b/awx/settings/defaults.py @@ -1164,5 +1164,13 @@ ANSIBLE_BASE_SERVICE_PREFIX = "awx" # Temporary, for old roles API compatibility, save child permissions at organization level ANSIBLE_BASE_CACHE_PARENT_PERMISSIONS = True +# Currently features are enabled to keep compatibility with old system, except custom roles +ANSIBLE_BASE_ALLOW_TEAM_ORG_ADMIN = False +# ANSIBLE_BASE_ALLOW_CUSTOM_ROLES = True +ANSIBLE_BASE_ALLOW_CUSTOM_TEAM_ROLES = False +ANSIBLE_BASE_ALLOW_SINGLETON_USER_ROLES = True +ANSIBLE_BASE_ALLOW_SINGLETON_TEAM_ROLES = False # System auditor has always been restricted to users +ANSIBLE_BASE_ALLOW_SINGLETON_ROLES_API = False # Do not allow creating user-defined system-wide roles + # system username for django-ansible-base SYSTEM_USERNAME = None