Use updated heuristic for identifying locally managed roles (ansible#562

)

See the prior criteria `if
permission_registry.get_resource_prefix(model) == 'shared':`, I always
knew this wasn't perfect and would, at some point, kind of not work.

This is hard to talk out - whether or not you can do permission
management is going to be kind of divorced from whether the permissions
that role gives are for shared resources.

I still don't think you should be able to create custom roles that give
- team membership or change permission --> covered by
`ANSIBLE_BASE_ALLOW_CUSTOM_TEAM_ROLES`
- organization "change" permission --> kind of tough, added criteria
back here to cover this

The PR ansible#486
essentially introduced a new distinction of RoleDefinitions, where some
are external, and some are local, determined by the setting. This
distinction is completely separate from whether shared resources are
managed by the roles.

ansible_base/lib/dynamic_config/settings_logic.py

@@ -161,6 +161,7 @@ def get_dab_settings(
             # Shadow local variable so subsequent conditionals works.
             installed_apps = dab_data['INSTALLED_APPS']
 
+    if ('ansible_base.jwt_consumer' in installed_apps) or ('ansible_base.rbac' in installed_apps):
         dab_data['ANSIBLE_BASE_JWT_MANAGED_ROLES'] = ["Platform Auditor", "Organization Admin", "Organization Member", "Team Admin", "Team Member"]
 
     if 'ansible_base.rbac' in installed_apps:
@@ -210,6 +211,12 @@ def get_dab_settings(
 
         # API clients can assign users and teams roles for shared resources
         dab_data['ALLOW_LOCAL_RESOURCE_MANAGEMENT'] = True
+        # API clients can assign roles provided by the JWT
+        # this should only be left as True for testing purposes
+        # TODO: change this default to False
+        dab_data['ALLOW_LOCAL_ASSIGNING_JWT_ROLES'] = True
+        # API clients can create custom roles that change shared resources
+        dab_data['ALLOW_SHARED_RESOURCE_CUSTOM_ROLES'] = False
 
         dab_data['MANAGE_ORGANIZATION_AUTH'] = True
 

ansible_base/rbac/api/serializers.py

@@ -148,7 +148,8 @@ def validate(self, validated_data):
         else:
             content_type = self.instance.content_type
         validate_permissions_for_model(permissions, content_type)
-        check_locally_managed(permissions, content_type)
+        if getattr(self, 'instance', None):
+            check_locally_managed(self.instance)
         return super().validate(validated_data)
 
 
@@ -258,7 +259,7 @@ def create(self, validated_data):
             obj.validate_role_assignment(actor, rd)
 
         # Return a 400 if the role is not managed locally
-        check_locally_managed(rd.permissions.prefetch_related('content_type'), rd.content_type)
+        check_locally_managed(rd)
 
         if rd.content_type:
             # Object role assignment

ansible_base/rbac/api/views.py

@@ -134,7 +134,7 @@ def perform_create(self, serializer):
 
     def perform_destroy(self, instance):
         check_can_remove_assignment(self.request.user, instance)
-        check_locally_managed(instance.role_definition.permissions.prefetch_related('content_type'), role_content_type=instance.content_type)
+        check_locally_managed(instance.role_definition)
 
         if instance.content_type_id:
             with transaction.atomic():

ansible_base/rbac/validators.py

@@ -1,6 +1,5 @@
 import re
 from collections import defaultdict
-from collections.abc import Iterable
 from typing import Optional, Type, Union
 
 from django.conf import settings
@@ -156,6 +155,17 @@ def validate_permissions_for_model(permissions, content_type: Optional[Model], m
     if settings.ANSIBLE_BASE_DELETE_REQUIRE_CHANGE and role_model is not None:
         check_has_change_with_delete(codename_set, permissions_by_model)
 
+    if (not managed) and (not settings.ALLOW_SHARED_RESOURCE_CUSTOM_ROLES):
+        for perm in permissions:
+            # View permission for shared objects is interpreted as permission to view
+            # the resource locally, which is needed to be able to view parent objects
+            # For system roles, this exception is not allowed
+            if content_type and perm.codename.startswith('view'):
+                continue
+            model = perm.content_type.model_class()
+            if permission_registry.get_resource_prefix(model) == 'shared':
+                raise ValidationError({'permissions', 'Local custom roles can only include view permission for shared models'})
+
 
 def validate_codename_for_model(codename: str, model: Union[Model, Type[Model]]) -> str:
     """Shortcut method and validation to allow action name, codename, or app_name.codename
@@ -234,22 +244,13 @@ def validate_assignment(rd, actor, obj) -> None:
         raise ValidationError(f'Role type {rd_model} does not match object {obj_ct.model}')
 
 
-def check_locally_managed(permissions_qs: Iterable[Model], role_content_type: Optional[Model]) -> None:
+def check_locally_managed(rd: Model) -> None:
     """Can the given role definition be managed here, or is it externally managed
 
-    If the role definition manages permissions on any shared resources, then
-    those should be locked down locally.
     This rule is a bridge solution until the RoleDefinition model declares
     explicitly whether it is managed locally or by a remote system.
     """
-    if settings.ALLOW_LOCAL_RESOURCE_MANAGEMENT is True:
+    if not (('ansible_base.jwt_consumer' in settings.INSTALLED_APPS) and (not settings.ALLOW_LOCAL_ASSIGNING_JWT_ROLES)):
         return
-    for perm in permissions_qs:
-        # View permission for shared objects is interpreted as permission to view
-        # the resource locally, which is needed to be able to view parent objects
-        # For system roles, this exception is not allowed
-        if role_content_type and perm.codename.startswith('view'):
-            continue
-        model = perm.content_type.model_class()
-        if permission_registry.get_resource_prefix(model) == 'shared':
-            raise ValidationError('Not managed locally, use the resource server instead')
+    if rd.name in settings.ANSIBLE_BASE_JWT_MANAGED_ROLES:
+        raise ValidationError('Not managed locally, use the resource server instead')

test_app/settings.py

@@ -182,6 +182,8 @@
 ANSIBLE_BASE_JWT_MANAGED_ROLES.append("System Auditor")  # noqa: F821 this is set by dynamic settings for jwt_consumer
 ANSIBLE_BASE_ALLOW_SINGLETON_USER_ROLES = True
 ANSIBLE_BASE_ALLOW_SINGLETON_TEAM_ROLES = True
+ALLOW_SHARED_RESOURCE_CUSTOM_ROLES = True  # Allow making custom roles with org change permission, for example
+ALLOW_LOCAL_ASSIGNING_JWT_ROLES = False
 
 ANSIBLE_BASE_USER_VIEWSET = 'test_app.views.UserViewSet'
 

test_app/tests/rbac/api/test_rbac_validation.py

@@ -5,8 +5,8 @@
 
 from ansible_base.lib.utils.auth import get_team_model
 from ansible_base.lib.utils.response import get_relative_url
-from ansible_base.rbac import permission_registry
 from ansible_base.rbac.models import RoleDefinition
+from ansible_base.rbac.permission_registry import permission_registry
 
 Team = get_team_model()
 User = get_user_model()
@@ -16,42 +16,60 @@
 class TestSharedAssignmentsDisabled:
     NON_LOCAL_MESSAGE = 'Not managed locally, use the resource server instead'
 
-    @override_settings(ALLOW_LOCAL_RESOURCE_MANAGEMENT=False)
     def test_team_member_role_not_assignable(self, member_rd, team, rando, admin_api_client):
         url = get_relative_url('roleuserassignment-list')
         response = admin_api_client.post(url, data={'object_id': team.id, 'role_definition': member_rd.id, 'user': rando.id})
         assert response.status_code == 400, response.data
         assert self.NON_LOCAL_MESSAGE in str(response.data)
+        assert not rando.has_obj_perm(team, 'member')
+
+        # Other non-shared team member roles can have their assignments modified
+        new_member_rd = RoleDefinition.objects.create_from_permissions(
+            name='another member role',
+            permissions=['member_team', 'view_team'],
+            content_type=permission_registry.content_type_model.objects.get_for_model(team),
+            managed=True,
+        )
+        response = admin_api_client.post(url, data={'object_id': team.id, 'role_definition': new_member_rd.id, 'user': rando.id})
+        assert response.status_code == 201, response.data
+        assert rando.has_obj_perm(team, 'member')
 
-    @override_settings(ALLOW_LOCAL_RESOURCE_MANAGEMENT=False)
-    def test_custom_roles_for_shared_stuff_not_allowed(self, admin_api_client):
+    @override_settings(ALLOW_LOCAL_ASSIGNING_JWT_ROLES=True)
+    def test_team_member_role_assignable_with_setting(self, member_rd, team, rando, admin_api_client):
+        url = get_relative_url('roleuserassignment-list')
+        response = admin_api_client.post(url, data={'object_id': team.id, 'role_definition': member_rd.id, 'user': rando.id})
+        assert response.status_code == 201, response.data
+
+    @pytest.mark.parametrize('allowed', [True, False])
+    def test_custom_roles_for_shared_stuff_not_allowed(self, admin_api_client, allowed):
         url = get_relative_url('roledefinition-list')
-        response = admin_api_client.post(
-            url,
-            data={
-                'name': 'Alternative Organization Admin Role in Local Server',
-                'content_type': 'aap.organization',
-                'permissions': ['aap.view_organization', 'local.change_organization'],
-            },
-        )
-        assert response.status_code == 400, response.data
-        assert self.NON_LOCAL_MESSAGE in str(response.data)
+        with override_settings(ALLOW_SHARED_RESOURCE_CUSTOM_ROLES=allowed):
+            response = admin_api_client.post(
+                url,
+                data={
+                    'name': 'Alternative Organization Admin Role in Local Server',
+                    'content_type': 'aap.organization',
+                    'permissions': ['aap.view_organization', 'local.change_organization'],
+                },
+            )
+        if allowed is False:
+            assert response.status_code == 400, response.data
+            assert 'Local custom roles can only include view permission for shared models' in str(response.data)
+        else:
+            assert response.status_code == 201, response.data
 
-    @override_settings(ALLOW_LOCAL_RESOURCE_MANAGEMENT=False)
     def test_auditor_for_external_models(self, admin_api_client, rando, external_auditor_constructor):
         rd, created = external_auditor_constructor.get_or_create(apps)
         url = get_relative_url('roleuserassignment-list')
         response = admin_api_client.post(url, data={'role_definition': rd.id, 'user': rando.id})
         assert response.status_code == 400, response.data
         assert self.NON_LOCAL_MESSAGE in str(response.data)
 
-    @override_settings(ALLOW_LOCAL_RESOURCE_MANAGEMENT=False)
     def test_resource_roles_still_assignable(self, org_inv_rd, organization, rando, admin_api_client):
         url = get_relative_url('roleuserassignment-list')
         response = admin_api_client.post(url, data={'object_id': organization.id, 'role_definition': org_inv_rd.id, 'user': rando.id})
         assert response.status_code == 201, response.data
 
-    @override_settings(ALLOW_LOCAL_RESOURCE_MANAGEMENT=False)
     def test_org_resource_roles_creatable(self, admin_api_client):
         url = get_relative_url('roledefinition-list')
         # This only contains shared view_organization, which is necessary to create custom org-level roles for child resources

test_app/tests/rbac/api/test_rbac_views.py

@@ -155,6 +155,7 @@ def test_remove_team_assignment(user_api_client, user, inv_rd, team, inventory):
 
 
 @pytest.mark.django_db
+@override_settings(ALLOW_LOCAL_ASSIGNING_JWT_ROLES=True)
 def test_team_assignment_validation_error(admin_api_client, team, organization, org_member_rd):
     url = get_relative_url('roleteamassignment-list')
     response = admin_api_client.post(url, data={'team': team.id, 'object_id': organization.id, 'role_definition': org_member_rd.id})

test_app/tests/rbac/api/test_user_permissions.py

@@ -164,6 +164,7 @@ def test_user_can_not_delete_themselves(self, user, user_api_client, admin_user,
 
 @pytest.mark.django_db
 class TestRoleBasedAssignment:
+    @override_settings(ALLOW_LOCAL_ASSIGNING_JWT_ROLES=True)
     def test_org_admins_can_add_members(self, user, user_api_client, organization, org_member_rd, org_admin_rd):
         rando = User.objects.create(username='rando')
         unrelated_org = Organization.objects.create(name='another-org')
@@ -184,6 +185,7 @@ def test_org_admins_can_add_members(self, user, user_api_client, organization, o
         assert response.status_code == 201, response.data
         assert rando.has_obj_perm(organization, 'member')
 
+    @override_settings(ALLOW_LOCAL_ASSIGNING_JWT_ROLES=True)
     def test_team_admins_can_add_children(self, user, user_api_client, organization, inventory, inv_rd, admin_rd, member_rd):
         url = get_relative_url('roleteamassignment-list')