Save permissions via additional_data

Author: AlanCoding

ansible_base/rbac/models/role.py

@@ -22,6 +22,7 @@
 from ansible_base.rbac.permission_registry import permission_registry
 from ansible_base.rbac.prefetch import TypesPrefetch
 from ansible_base.rbac.validators import validate_assignment, validate_permissions_for_model
+from ansible_base.resource_registry.fields import AnsibleResourceField
 
 from ..remote import RemoteObject
 from .content_type import DABContentType
@@ -164,6 +165,8 @@ class Meta:
         default=None,
         on_delete=models.CASCADE,
     )
+    # This is a synchronized field, so add reverse relation for resource
+    resource = AnsibleResourceField(primary_key_field="id")
 
     objects = RoleDefinitionManager()
     router_basename = 'roledefinition'
@@ -323,6 +326,15 @@ def user_global_permissions(cls, user, permission_qs=None):
             perm_set.update(perm_qs)
         return perm_set
 
+    def save_remote_permissions(self, additional_data: list[str]):
+        """Save permissions from an external system.
+
+        This does very little more than just making the related permissions exactly equal to the set.
+        If a provided permission is not present in the local system, we pretend we did not get it.
+        However, we must still allow removing permissions from the role.
+        """
+        self.permissions.set(DABPermission.objects.filter(api_slug__in=additional_data))
+
     def summary_fields(self):
         return {'id': self.id, 'name': self.name, 'description': self.description, 'managed': self.managed}
 

ansible_base/resource_registry/rest_client.py

@@ -10,8 +10,8 @@
 
 ResourceRequestBody = namedtuple(
     "ResourceRequestBody",
-    ["ansible_id", "service_id", "is_partially_migrated", "resource_type", "resource_data"],
-    defaults=(None, None, None, None, None),
+    ["ansible_id", "service_id", "is_partially_migrated", "resource_type", "resource_data", "additional_data"],
+    defaults=(None, None, None, None, None, None),
 )
 
 

ansible_base/resource_registry/serializers.py

@@ -96,7 +96,7 @@ def create(self, validated_data):
 
 
 class ResourceSerializer(ResourceListSerializer):
-    additional_data = serializers.SerializerMethodField()
+    additional_data = serializers.JSONField(write_only=True, required=False)
     resource_data = ResourceDataField(source="*")
 
     class Meta:
@@ -106,12 +106,37 @@ class Meta:
             "additional_data",
         ]
 
-    def get_additional_data(self, obj):
-        if serializer := obj.content_type.resource_type.serializer_class:
+    def to_representation(self, instance):
+        ret = super().to_representation(instance)
+
+        additional_data = None
+        if serializer := instance.content_type.resource_type.serializer_class:
             if serializer.ADDITIONAL_DATA_SERIALIZER is not None:
-                return serializer.ADDITIONAL_DATA_SERIALIZER(obj.content_object).data
+                additional_data = serializer.ADDITIONAL_DATA_SERIALIZER(instance.content_object).data
+
+        ret['additional_data'] = additional_data
+        return ret
+
+    def _maybe_process_additional_data(self, instance, additional_data):
+        if not additional_data:
+            return
 
-        return None
+        if serializer := instance.content_type.resource_type.serializer_class:
+            if serializer and hasattr(serializer, 'process_additional_data'):
+                logger.debug(f'Processing additional data for resource {str(instance.ansible_id)}')
+                serializer(instance.content_object).process_additional_data(instance.content_object, additional_data)
+
+    def create(self, validated_data):
+        additional_data = validated_data.pop('additional_data', None)
+        instance = super().create(validated_data)
+        self._maybe_process_additional_data(instance, additional_data)
+        return instance
+
+    def update(self, instance, validated_data):
+        additional_data = validated_data.pop('additional_data', None)
+        instance = super().update(instance, validated_data)
+        self._maybe_process_additional_data(instance, additional_data)
+        return instance
 
 
 class ResourceTypeSerializer(serializers.ModelSerializer):

ansible_base/resource_registry/shared_types.py

@@ -100,3 +100,6 @@ class RoleDefinitionType(SharedResourceTypeSerializer):
         allow_null=True,
         default=None,
     )
+
+    def process_additional_data(self, instance, additional_data):
+        instance.save_remote_permissions(additional_data.get('permissions'))

test_app/tests/rbac/models/test_role_definition.py

@@ -105,3 +105,12 @@ def test_change_role_definition_member_permission(organization, inventory, org_t
     # Adding it back restores them
     member_rd.permissions.add(member_perm)
     assert [u.has_obj_perm(inventory, 'change') for u in (team_user, org_team_user)] == [True, True]
+
+
+@pytest.mark.django_db
+def test_save_remote_permissions(inv_rd):
+    "This is tested as part of resource registry in test_additional_data_write"
+    assert {perm.api_slug for perm in inv_rd.permissions.all()} == {'aap.view_inventory', 'aap.change_inventory'}
+    assert len(DABPermission.objects.filter(api_slug__in={'aap.view_inventory', 'fooland.not_here'})) == 1
+    inv_rd.permissions.set([DABPermission.objects.get(api_slug='aap.view_inventory')])
+    assert {perm.api_slug for perm in inv_rd.permissions.all()} == {'aap.view_inventory'}

test_app/tests/rbac/test_management_rbac_checks.py renamed to test_app/tests/rbac/test_management.py

@@ -1,9 +1,11 @@
 from io import StringIO
 
 import pytest
+from django.apps import apps
 
+from ansible_base.rbac.management import create_dab_permissions
 from ansible_base.rbac.management.commands.RBAC_checks import Command
-from ansible_base.rbac.models import DABContentType, ObjectRole, RoleDefinition
+from ansible_base.rbac.models import DABContentType, DABPermission, ObjectRole, RoleDefinition
 from test_app.models import Inventory
 
 
@@ -25,3 +27,14 @@ def test_role_definition_wrong_model(organization):
     rd, _ = RoleDefinition.objects.get_or_create(name='foo-def', permissions=['view_organization'])
     orole = ObjectRole.objects.create(object_id=inventory.id, content_type=DABContentType.objects.get_for_model(inventory), role_definition=rd)
     assert f"Object role {orole} has permission view_organization for an unlike content type" in run_and_get_output()
+
+
+@pytest.mark.django_db
+def test_recreate_model_permissions():
+    del_ct = 0
+    for permission in DABPermission.objects.filter(content_type__model='inventory'):
+        permission.delete()
+        del_ct += 1
+    assert del_ct == 5
+    create_dab_permissions(app_config=apps.get_app_config('dab_rbac'))
+    assert DABPermission.objects.filter(content_type__model='inventory').count() == 5

test_app/tests/resource_registry/test_resources_api_rest_client.py

@@ -5,9 +5,12 @@
 from requests.exceptions import HTTPError
 
 from ansible_base.authentication.models import AuthenticatorUser
+from ansible_base.rbac import permission_registry
+from ansible_base.rbac.models import RoleDefinition
 from ansible_base.resource_registry.models import Resource, service_id
 from ansible_base.resource_registry.resource_server import get_resource_server_config
 from ansible_base.resource_registry.rest_client import ResourceAPIClient, ResourceRequestBody
+from test_app.models import Inventory
 
 
 @pytest.fixture
@@ -182,7 +185,7 @@ def test_get_resource_404(resource_client):
 
 
 @pytest.mark.django_db
-def test_additional_data(resource_client, django_user_model, github_authenticator):
+def test_additional_data_read(resource_client, django_user_model, github_authenticator):
     user = django_user_model.objects.create(username="lisan_al_gaib")
 
     AuthenticatorUser.objects.create(provider=github_authenticator, user=user, uid="different_uid")
@@ -200,6 +203,30 @@ def test_additional_data(resource_client, django_user_model, github_authenticato
     assert additional["social_auth"][0]["sso_server"] == "https://github.com/login/oauth/authorize"
 
 
+@pytest.mark.django_db
+@pytest.mark.parametrize('partial', [True, False])
+def test_additional_data_write(resource_client, partial):
+    "Will remove a permission from a role definition using the additional_data field."
+    rd = RoleDefinition.objects.create_from_permissions(
+        permissions=['aap.change_inventory', 'aap.view_inventory'],
+        name='change-inv-for-now',
+        content_type=permission_registry.content_type_model.objects.get_for_model(Inventory),
+    )
+    ansible_id = str(rd.resource.ansible_id)
+
+    # Need this to make a coherent PUT
+    resp = resource_client.get_resource(ansible_id)
+    assert resp.status_code == 200
+    ref = resp.json()
+
+    data = ResourceRequestBody(additional_data={'permissions': ['aap.view_inventory', 'fooland.action_unicorns']}, resource_data=ref['resource_data'])
+    resp = resource_client.update_resource(ansible_id, data, partial=partial)
+    assert resp.status_code == 200, resp.__dict__
+
+    # Removed the change permission
+    assert {perm.api_slug for perm in rd.permissions.all()} == {'aap.view_inventory'}
+
+
 @pytest.mark.django_db
 def test_validate_local_user(resource_client, admin_user, member_rd):
     resp = resource_client.validate_local_user(username=admin_user.username, password="password")