[DAB RBAC] Re-implement system auditor as a singleton role in new system (#14963)

* Add new enablement settings from DAB RBAC

* Initial implementation of system auditor as role without testing

* Fix system auditor role, remove duplicate assignments

* Make the system auditor role managed

* Flake8 fix

* Remove another thing from old solution

* Fix a few test failures

* Add extra setting to disable custom system roles via API

* Add test for custom role prohibition

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

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:

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

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
 

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 = [

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__'),
     ]
 

awx/main/migrations/0191_profile_is_system_auditor.py

@@ -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'),
-        ),
-    ]

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')

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)
 

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):

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
 
 

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')

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

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

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