AAP-82996 Record DAB RBAC role assignment events in the activity stream (#16584)

* Record DAB RBAC role assignment events in the activity stream

* The steam message was showing incorrectly, it was inverted based on the legacy setup

* object_relationship_type was missing, this is set n the legacy handler and emit_activity_stream_change emits it.

* Skip cascade deletes from non-assignment origins, same reasoning as sync_assignments_to_old_rbac_delete

awx/main/models/__init__.py

@@ -201,6 +201,8 @@ def user_is_system_auditor(user):
 
 @user_is_system_auditor.setter
 def user_is_system_auditor(user, tf):
+    from awx.main.signals import disable_activity_stream
+
     if not user.id:
         # If the user doesn't have a primary key yet (i.e., this is the *first*
         # time they've logged in, and we've just created the new User in this
@@ -210,10 +212,14 @@ def user_is_system_auditor(user, tf):
     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)
+        # The role assignment change below is already recorded by this function's own
+        # 'update' entry, which captures the prior/new boolean value. Suppress activity
+        # stream for the assignment create/delete so it isn't recorded a second time.
+        with disable_activity_stream():
+            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/rbac.py

@@ -651,16 +651,24 @@ def disable_rbac_sync():
 
 
 def give_creator_permissions(user, obj):
+    from awx.main.signals import disable_activity_stream
+
     assignment = RoleDefinition.objects.give_creator_permissions(user, obj)
     if assignment:
         with disable_rbac_sync():
             old_role = get_role_from_object_role(assignment.object_role)
             if old_role is None:
                 return
-            old_role.members.add(user)
+            # The new-side assignment above is already recorded by
+            # record_role_assignment_activity_stream. Suppress activity stream for this
+            # mirrored write so it isn't recorded a second time.
+            with disable_activity_stream():
+                old_role.members.add(user)
 
 
 def sync_members_to_new_rbac(instance, action, model, pk_set, reverse, **kwargs):
+    from awx.main.signals import disable_activity_stream
+
     if action.startswith('pre_'):
         return
     if not rbac_sync_enabled.enabled:
@@ -684,7 +692,11 @@ def sync_members_to_new_rbac(instance, action, model, pk_set, reverse, **kwargs)
         else:
             user = get_user_model().objects.get(pk=user_or_role_id)
         rd = get_role_definition(role)
-        assignment = give_or_remove_permission(role, user, giving=is_giving, rd=rd)
+        # This mirrors an already-recorded legacy Role.members change into the new RBAC
+        # system. Suppress activity stream here so the mirrored RoleUserAssignment/
+        # RoleTeamAssignment write isn't recorded a second time (see record_role_assignment_activity_stream).
+        with disable_activity_stream():
+            assignment = give_or_remove_permission(role, user, giving=is_giving, rd=rd)
 
         # sync to resource server
         if rbac_sync_enabled.enabled:
@@ -695,6 +707,8 @@ def sync_members_to_new_rbac(instance, action, model, pk_set, reverse, **kwargs)
 
 
 def sync_parents_to_new_rbac(instance, action, model, pk_set, reverse, **kwargs):
+    from awx.main.signals import disable_activity_stream
+
     if action.startswith('pre_'):
         return
 
@@ -735,7 +749,10 @@ def sync_parents_to_new_rbac(instance, action, model, pk_set, reverse, **kwargs)
 
             team = Team.objects.get(pk=parent_role.object_id)
             rd = get_role_definition(child_role)
-            assignment = give_or_remove_permission(child_role, team, giving=is_giving, rd=rd)
+            # See comment in sync_members_to_new_rbac: suppress activity stream for this
+            # mirrored write to avoid double-recording an already-recorded legacy change.
+            with disable_activity_stream():
+                assignment = give_or_remove_permission(child_role, team, giving=is_giving, rd=rd)
 
             # sync to resource server
             if rbac_sync_enabled.enabled:

awx/main/signals.py

@@ -9,7 +9,7 @@ import json
 import sys
 
 # Django
-from django.db import connection
+from django.db import connection, models
 from django.conf import settings
 from django.db.models.signals import (
     pre_save,
@@ -27,6 +27,9 @@ from django.utils import timezone
 from crum import get_current_request, get_current_user
 from crum.signals import current_user_getter
 
+# Ansible_base app
+from ansible_base.rbac.models import RoleUserAssignment, RoleTeamAssignment
+
 # AWX
 from awx.main.models import (
     ActivityStream,
@@ -180,6 +183,87 @@ def rbac_activity_stream(instance, sender, **kwargs):
         activity_stream_associate(sender, instance, role=role, **kwargs)
 
 
+def _record_role_assignment_activity_stream(instance, operation):
+    if not activity_stream_enabled:
+        return
+    # Avoid flooding the activity stream when assignments are cascade-deleted as part
+    # of setup_managed_role_definitions() pruning stale managed RoleDefinitions on migrate.
+    if 'migrate' in sys.argv:
+        return
+
+    if isinstance(instance, RoleUserAssignment):
+        actor_field = 'user'
+        actor_obj = instance.user
+    else:
+        actor_field = 'team'
+        actor_obj = instance.team
+
+    content_object = None
+    if instance.object_id:
+        content_object = instance.content_object
+        if content_object is None and operation == 'disassociate':
+            # The target object is unresolvable, e.g. cascade-deleted along with its
+            # parent object, or a federated/remote content type. Skip recording a
+            # contentless entry rather than guessing at what changed.
+            # Note: FederatedForeignKey swallows ObjectDoesNotExist and returns None
+            # rather than raising, so there's no exception to catch here.
+            return
+
+    changes = {'role_definition': instance.role_definition.name, actor_field: getattr(actor_obj, 'username', None) or str(actor_obj)}
+
+    # object1 is the role's content object and object2 the associated user/team, matching
+    # the convention used for legacy Role.members association entries (rbac_activity_stream
+    # below), so this reads e.g. "disassociated organization X from user Y".
+    object1 = ''
+    activity_stream_cls = get_activity_stream_class()
+    if content_object is not None:
+        object1 = camelcase_to_underscore(content_object.__class__.__name__)
+        changes['object_type'] = object1
+        changes['object_id'] = content_object.pk
+        changes['object_name'] = str(content_object)
+        obj_rel = f'{content_object.__class__.__module__}.{content_object.__class__.__name__}.{instance.role_definition_id}'
+        if not hasattr(activity_stream_cls, object1):
+            logger.warning('ActivityStream has no relation field for object type %r; role assignment entry will not be linked to it', object1)
+            object1 = ''
+    else:
+        obj_rel = str(instance.role_definition_id)
+
+    activity_entry = activity_stream_cls(
+        operation=operation,
+        object1=object1,
+        object2=actor_field,
+        object_relationship_type=obj_rel,
+        changes=json.dumps(changes),
+        actor=get_current_user_or_none(),
+    )
+    activity_entry.save()
+    getattr(activity_entry, actor_field).add(actor_obj.pk)
+    if object1:
+        getattr(activity_entry, object1).add(content_object.pk)
+    connection.on_commit(lambda: emit_activity_stream_change(activity_entry))
+
+
+@receiver(post_save, sender=RoleUserAssignment)
+@receiver(post_save, sender=RoleTeamAssignment)
+def record_role_assignment_activity_stream(instance, created, **kwargs):
+    if created:
+        _record_role_assignment_activity_stream(instance, 'associate')
+
+
+@receiver(post_delete, sender=RoleUserAssignment)
+@receiver(post_delete, sender=RoleTeamAssignment)
+def record_role_unassignment_activity_stream(instance, origin=None, **kwargs):
+    # Skip cascade deletes from non-assignment origins, same reasoning as
+    # sync_assignments_to_old_rbac_delete: the actor or content object is itself
+    # being deleted in the same cascade, so linking a new activity stream entry
+    # to it would leave a dangling foreign key once the cascade completes.
+    if isinstance(origin, models.Model) and origin._meta.app_label != 'dab_rbac':
+        return
+    if isinstance(origin, models.QuerySet) and origin.model is not type(instance):
+        return
+    _record_role_assignment_activity_stream(instance, 'disassociate')
+
+
 def cleanup_detached_labels_on_deleted_parent(sender, instance, **kwargs):
     for label in instance.labels.all():
         if label.is_candidate_for_detach():

awx/main/tests/functional/dab_rbac/test_dab_rbac_activity_stream.py

@@ -0,0 +1,165 @@
+import json
+
+import pytest
+
+from awx.api.versioning import reverse
+
+from ansible_base.rbac.models import RoleDefinition, RoleUserAssignment
+from ansible_base.rbac import permission_registry
+
+from awx.main.models import ActivityStream, Inventory, Organization
+
+
+@pytest.mark.django_db
+class TestCreatorPermissionActivityStream:
+    '''
+    give_creator_permissions grants the new-side RoleUserAssignment and mirrors it into
+    the legacy Role.members m2m. Only the new-side grant should be recorded - the mirrored
+    write must not produce a second entry.
+    '''
+
+    def test_object_creation_records_single_entry(self, post, rando, organization, setup_managed_roles):
+        rd, _ = RoleDefinition.objects.get_or_create(
+            name='inventory-add',
+            permissions=['add_inventory', 'view_organization'],
+            content_type=permission_registry.content_type_model.objects.get_for_model(Organization),
+        )
+        rd.give_permission(rando, organization)
+
+        url = reverse('api:inventory_list')
+        response = post(url=url, data={'name': 'rando-created-inventory', 'organization': organization.id}, user=rando, expect=201)
+        inventory = Inventory.objects.get(pk=response.data['id'])
+
+        assert rando in inventory.admin_role.members.all()
+        entries = ActivityStream.objects.filter(operation='associate', user=rando, changes__icontains='Inventory Admin')
+        assert entries.count() == 1
+        entry = entries.get()
+        assert entry.object1 == 'inventory'
+        assert entry.object2 == 'user'
+
+
+@pytest.mark.django_db
+class TestRoleAssignmentActivityStream:
+    '''
+    Tests that assigning/removing a DAB RBAC role records an ActivityStream entry,
+    independent of whether the role has a legacy Role equivalent.
+    '''
+
+    def test_custom_role_assignment_recorded(self, rando, inventory, setup_managed_roles):
+        rd, _ = RoleDefinition.objects.get_or_create(
+            name='inventory-custom-role',
+            permissions=['view_inventory', 'change_inventory'],
+            content_type=permission_registry.content_type_model.objects.get_for_model(Inventory),
+        )
+        rd.give_permission(rando, inventory)
+
+        entries = ActivityStream.objects.filter(operation='associate', user=rando, changes__icontains=rd.name)
+        assert entries.count() == 1
+        entry = entries.get()
+        assert json.loads(entry.changes) == {
+            'role_definition': rd.name,
+            'user': rando.username,
+            'object_type': 'inventory',
+            'object_id': inventory.id,
+            'object_name': str(inventory),
+        }
+        # object1 is the role's content object and object2 the associated actor, matching
+        # the convention used for legacy Role.members association entries, so this reads
+        # "disassociated inventory X from user Y" rather than the other way around.
+        assert entry.object1 == 'inventory'
+        assert entry.object2 == 'user'
+        assert entry.object_relationship_type == f'{Inventory.__module__}.Inventory.{rd.id}'
+
+    def test_custom_role_removal_recorded(self, rando, inventory, setup_managed_roles):
+        rd, _ = RoleDefinition.objects.get_or_create(
+            name='inventory-custom-role',
+            permissions=['view_inventory', 'change_inventory'],
+            content_type=permission_registry.content_type_model.objects.get_for_model(Inventory),
+        )
+        rd.give_permission(rando, inventory)
+        rd.remove_permission(rando, inventory)
+
+        entries = ActivityStream.objects.filter(operation='disassociate', user=rando, changes__icontains=rd.name)
+        assert entries.count() == 1
+        entry = entries.get()
+        assert entry.object1 == 'inventory'
+        assert entry.object2 == 'user'
+
+    def test_custom_role_team_assignment_recorded(self, team, inventory, setup_managed_roles):
+        rd, _ = RoleDefinition.objects.get_or_create(
+            name='inventory-custom-role',
+            permissions=['view_inventory', 'change_inventory'],
+            content_type=permission_registry.content_type_model.objects.get_for_model(Inventory),
+        )
+        rd.give_permission(team, inventory)
+
+        entries = ActivityStream.objects.filter(operation='associate', team=team, changes__icontains=rd.name)
+        assert entries.count() == 1
+        entry = entries.get()
+        assert json.loads(entry.changes) == {
+            'role_definition': rd.name,
+            'team': str(team),
+            'object_type': 'inventory',
+            'object_id': inventory.id,
+            'object_name': str(inventory),
+        }
+        assert entry.object1 == 'inventory'
+        assert entry.object2 == 'team'
+
+    def test_custom_role_team_removal_recorded(self, team, inventory, setup_managed_roles):
+        rd, _ = RoleDefinition.objects.get_or_create(
+            name='inventory-custom-role',
+            permissions=['view_inventory', 'change_inventory'],
+            content_type=permission_registry.content_type_model.objects.get_for_model(Inventory),
+        )
+        rd.give_permission(team, inventory)
+        rd.remove_permission(team, inventory)
+
+        entries = ActivityStream.objects.filter(operation='disassociate', team=team, changes__icontains=rd.name)
+        assert entries.count() == 1
+        entry = entries.get()
+        assert entry.object1 == 'inventory'
+        assert entry.object2 == 'team'
+
+    def test_global_role_assignment_recorded(self, rando, setup_managed_roles):
+        rd, _ = RoleDefinition.objects.get_or_create(name='global-view-role', content_type=None)
+        RoleUserAssignment.objects.create(user=rando, role_definition=rd)
+
+        entries = ActivityStream.objects.filter(operation='associate', user=rando, changes__icontains=rd.name)
+        assert entries.count() == 1
+        entry = entries.get()
+        assert json.loads(entry.changes) == {'role_definition': rd.name, 'user': rando.username}
+        # No content object for a global role assignment, so object1 is left blank.
+        assert entry.object1 == ''
+        assert entry.object2 == 'user'
+        assert entry.object_relationship_type == str(rd.id)
+
+    def test_cascade_delete_does_not_record_contentless_entry(self, rando, setup_managed_roles):
+        '''
+        Deleting an object cascade-deletes its RoleUserAssignments. By the time the
+        post_delete handler runs, the object is already gone, so there is nothing
+        useful to record - it should be skipped rather than logged with no context.
+        '''
+        org = Organization.objects.create(name='cascade-delete-org')
+        rd = RoleDefinition.objects.get(name='Organization Admin')
+        rd.give_permission(rando, org)
+        assert ActivityStream.objects.filter(operation='associate', user=rando, changes__icontains=rd.name).count() == 1
+
+        org.delete()
+
+        assert ActivityStream.objects.filter(operation='disassociate', user=rando, changes__icontains=rd.name).count() == 0
+
+    def test_cascade_delete_of_team_does_not_leave_dangling_link(self, team, inventory, setup_managed_roles):
+        '''
+        Deleting a team's organization cascades to delete the team, which cascades to
+        delete its RoleTeamAssignments. Recording a fresh ActivityStream entry linked to
+        the team in response to that cascade would leave a dangling foreign key once the
+        team row is actually removed later in the same cascade - it must be skipped.
+        '''
+        rd = RoleDefinition.objects.get(name='Inventory Admin')
+        rd.give_permission(team, inventory)
+        assert ActivityStream.objects.filter(operation='associate', team=team, changes__icontains=rd.name).count() == 1
+
+        team.organization.delete()
+
+        assert ActivityStream.objects.filter(operation='disassociate', team=team, changes__icontains=rd.name).count() == 0

awx/main/tests/functional/dab_rbac/test_translation_layer.py

@@ -7,7 +7,7 @@ from crum import impersonate
 
 from awx.main.fields import ImplicitRoleField
 from awx.main.models.rbac import get_role_from_object_role, give_creator_permissions, get_role_codenames, get_role_definition
-from awx.main.models import User, Organization, WorkflowJobTemplate, WorkflowJobTemplateNode, Team
+from awx.main.models import ActivityStream, User, Organization, WorkflowJobTemplate, WorkflowJobTemplateNode, Team
 from awx.api.versioning import reverse
 
 from ansible_base.rbac.models import RoleUserAssignment, RoleDefinition
@@ -41,6 +41,22 @@ def test_round_trip_roles(organization, rando, role_name, setup_managed_roles):
     assert old_role.id == getattr(organization, role_name).id
 
 
+@pytest.mark.django_db
+def test_bulk_member_addition_records_one_entry_per_user(organization, rando, alice, setup_managed_roles):
+    """
+    Adding multiple users to a legacy role in one m2m .add() call syncs each user to the
+    new RBAC system individually (sync_members_to_new_rbac). The legacy-side add already
+    records one ActivityStream entry per user via rbac_activity_stream; the new-side mirror
+    must stay suppressed so this doesn't turn into two entries per user.
+    """
+    organization.admin_role.members.add(rando, alice)
+
+    assert RoleUserAssignment.objects.filter(user=rando, role_definition__name='Organization Admin').exists()
+    assert RoleUserAssignment.objects.filter(user=alice, role_definition__name='Organization Admin').exists()
+    assert ActivityStream.objects.filter(operation='associate', user=rando).count() == 1
+    assert ActivityStream.objects.filter(operation='associate', user=alice).count() == 1
+
+
 @pytest.mark.django_db
 @pytest.mark.parametrize('model', sorted(permission_registry.all_registered_models, key=lambda cls: cls._meta.model_name))
 def test_role_migration_matches(request, model, setup_managed_roles):
@@ -57,11 +73,11 @@ def test_role_migration_matches(request, model, setup_managed_roles):
             rd = get_role_definition(old_role)
             new_codenames = set(rd.permissions.values_list('codename', flat=True))
             # all the old roles should map to a non-Compat role definition
+            rd_data = {}
             if 'Compat' not in rd.name:
                 model_rds = RoleDefinition.objects.filter(content_type=permission_registry.content_type_model.objects.get_for_model(obj))
-                rd_data = {}
-                for rd in model_rds:
-                    rd_data[rd.name] = list(rd.permissions.values_list('codename', flat=True))
+                for other_rd in model_rds:
+                    rd_data[other_rd.name] = list(other_rd.permissions.values_list('codename', flat=True))
             assert 'Compat' not in rd.name, (
                 f'Permissions for old vs new roles did not match.\nold {field.name}: {old_codenames}\nnew:\n{json.dumps(rd_data, indent=2)}'
             )

awx/main/tests/functional/dab_rbac/test_translation_layer_new_to_old.py

@@ -1,9 +1,12 @@
+import json
 from unittest import mock
 
 from ansible_base.rbac.models import ObjectRole, RoleDefinition, RoleUserAssignment, RoleTeamAssignment
 from ansible_base.lib.utils.response import get_relative_url
 import pytest
 
+from awx.main.models import ActivityStream
+
 
 @pytest.mark.django_db
 class TestNewToOld:
@@ -21,6 +24,19 @@ class TestNewToOld:
         url = get_relative_url('roleuserassignment-list')
         post(url, user=admin, data={'role_definition': rd.id, 'user': bob.id, 'object_id': inventory.id}, expect=201)
         assert bob in inventory.admin_role.members.all()
+        # the new-side assignment should be recorded exactly once, not double-recorded via the old-side mirror
+        entries = ActivityStream.objects.filter(operation='associate', user=bob)
+        assert entries.count() == 1
+        entry = entries.get()
+        assert json.loads(entry.changes) == {
+            'role_definition': rd.name,
+            'user': bob.username,
+            'object_type': 'inventory',
+            'object_id': inventory.id,
+            'object_name': str(inventory),
+        }
+        assert entry.object1 == 'inventory'
+        assert entry.object2 == 'user'
 
     def test_new_to_old_rbac_removal(self, admin, delete, inventory, bob, setup_managed_roles):
         '''
@@ -34,6 +50,18 @@ class TestNewToOld:
         url = get_relative_url('roleuserassignment-detail', kwargs={'pk': user_assignment.id})
         delete(url, user=admin, expect=204)
         assert bob not in inventory.admin_role.members.all()
+        entries = ActivityStream.objects.filter(operation='disassociate', user=bob)
+        assert entries.count() == 1
+        entry = entries.get()
+        assert json.loads(entry.changes) == {
+            'role_definition': rd.name,
+            'user': bob.username,
+            'object_type': 'inventory',
+            'object_id': inventory.id,
+            'object_name': str(inventory),
+        }
+        assert entry.object1 == 'inventory'
+        assert entry.object2 == 'user'
 
     def test_new_to_old_rbac_team_member_addition(self, admin, post, team, bob, setup_managed_roles):
         '''
@@ -44,6 +72,18 @@ class TestNewToOld:
         url = get_relative_url('roleuserassignment-list')
         post(url, user=admin, data={'role_definition': rd.id, 'user': bob.id, 'object_id': team.id}, expect=201)
         assert bob in team.member_role.members.all()
+        entries = ActivityStream.objects.filter(operation='associate', user=bob)
+        assert entries.count() == 1
+        entry = entries.get()
+        assert json.loads(entry.changes) == {
+            'role_definition': rd.name,
+            'user': bob.username,
+            'object_type': 'team',
+            'object_id': team.id,
+            'object_name': str(team),
+        }
+        assert entry.object1 == 'team'
+        assert entry.object2 == 'user'
 
     def test_new_to_old_rbac_team_member_removal(self, admin, delete, team, bob, setup_managed_roles):
         '''
@@ -57,6 +97,18 @@ class TestNewToOld:
         url = get_relative_url('roleuserassignment-detail', kwargs={'pk': user_assignment.id})
         delete(url, user=admin, expect=204)
         assert bob not in team.member_role.members.all()
+        entries = ActivityStream.objects.filter(operation='disassociate', user=bob)
+        assert entries.count() == 1
+        entry = entries.get()
+        assert json.loads(entry.changes) == {
+            'role_definition': rd.name,
+            'user': bob.username,
+            'object_type': 'team',
+            'object_id': team.id,
+            'object_name': str(team),
+        }
+        assert entry.object1 == 'team'
+        assert entry.object2 == 'user'
 
     def test_new_to_old_rbac_team_addition(self, admin, post, team, inventory, setup_managed_roles):
         '''
@@ -67,6 +119,18 @@ class TestNewToOld:
         url = get_relative_url('roleteamassignment-list')
         post(url, user=admin, data={'role_definition': rd.id, 'team': team.id, 'object_id': inventory.id}, expect=201)
         assert team.member_role in inventory.admin_role.parents.all()
+        entries = ActivityStream.objects.filter(operation='associate', team=team)
+        assert entries.count() == 1
+        entry = entries.get()
+        assert json.loads(entry.changes) == {
+            'role_definition': rd.name,
+            'team': str(team),
+            'object_type': 'inventory',
+            'object_id': inventory.id,
+            'object_name': str(inventory),
+        }
+        assert entry.object1 == 'inventory'
+        assert entry.object2 == 'team'
 
     def test_new_to_old_rbac_team_removal(self, admin, delete, team, inventory, setup_managed_roles):
         '''
@@ -80,6 +144,18 @@ class TestNewToOld:
         url = get_relative_url('roleteamassignment-detail', kwargs={'pk': team_assignment.id})
         delete(url, user=admin, expect=204)
         assert team.member_role not in inventory.admin_role.parents.all()
+        entries = ActivityStream.objects.filter(operation='disassociate', team=team)
+        assert entries.count() == 1
+        entry = entries.get()
+        assert json.loads(entry.changes) == {
+            'role_definition': rd.name,
+            'team': str(team),
+            'object_type': 'inventory',
+            'object_id': inventory.id,
+            'object_name': str(inventory),
+        }
+        assert entry.object1 == 'inventory'
+        assert entry.object2 == 'team'
 
     def test_flush_rbac_cleanup_skips_sync(self, inventory, bob, setup_managed_roles):
         """Simulate what defer_rbac_computations._flush_rbac does on exit: