Skip to content

Commit 401c3ba

Browse files
authored
[AAP-51985] Implements cross-service RBAC cleanup for object deletion (#834)
Implements automatic cleanup of orphaned role assignments when objects are deleted by adding /service-index/object-delete/ API endpoint for bulk role cleanup.
1 parent 4d1381a commit 401c3ba

File tree

8 files changed

+809
-4
lines changed

8 files changed

+809
-4
lines changed

ansible_base/rbac/service_api/router.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,4 @@
99
# Different basenames used to distinguish between the duplicate, public, endpoints
1010
service_router.register(r'role-user-assignments', views.ServiceRoleUserAssignmentViewSet, basename='serviceuserassignment')
1111
service_router.register(r'role-team-assignments', views.ServiceRoleTeamAssignmentViewSet, basename='serviceteamassignment')
12+
service_router.register(r'object-delete', views.ServiceObjectDeleteViewSet, basename='serviceobjectdelete')

ansible_base/rbac/service_api/views.py

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
from django.db import transaction
2-
from rest_framework import permissions, status
2+
from rest_framework import permissions, status, viewsets
33
from rest_framework.decorators import action
44
from rest_framework.response import Response
55
from rest_framework.viewsets import GenericViewSet, mixins
@@ -145,3 +145,65 @@ def assign(self, request):
145145
@action(detail=False, methods=['post'], url_path='unassign')
146146
def unassign(self, request):
147147
return self._unassign(request)
148+
149+
150+
class ServiceObjectDeleteViewSet(viewsets.ViewSet):
151+
"""
152+
Bulk deletion of role assignments for deleted objects.
153+
Uses standard create() method to bypass service token authentication restrictions.
154+
Handles both user and team assignments in a single API call.
155+
"""
156+
157+
permission_classes = [HasResourceRegistryPermissions]
158+
159+
def create(self, request):
160+
"""
161+
Delete all role assignments (user and team) for a specific resource.
162+
163+
Expected request data:
164+
{
165+
"resource_type": "main.inventory",
166+
"resource_pk": "4"
167+
}
168+
"""
169+
from ..models import DABContentType
170+
171+
# Validate request data
172+
serializer_data = {
173+
'resource_type': request.data.get('resource_type'),
174+
'resource_pk': request.data.get('resource_pk'),
175+
}
176+
177+
if not serializer_data['resource_type'] or not serializer_data['resource_pk']:
178+
return Response({'error': 'Both resource_type and resource_pk are required'}, status=status.HTTP_400_BAD_REQUEST)
179+
180+
try:
181+
# Parse resource_type (e.g., "main.inventory" -> app_label="main", model="inventory")
182+
app_label, model_name = serializer_data['resource_type'].split('.', 1)
183+
except ValueError:
184+
return Response({'error': 'Invalid resource_type format. Expected: app_label.model_name'}, status=status.HTTP_400_BAD_REQUEST)
185+
186+
try:
187+
# Get the content type
188+
content_type = DABContentType.objects.get(app_label=app_label, model=model_name)
189+
except DABContentType.DoesNotExist:
190+
return Response({'error': f'Content type not found: {serializer_data["resource_type"]}'}, status=status.HTTP_400_BAD_REQUEST)
191+
192+
# Perform bulk deletion in a transaction
193+
with transaction.atomic():
194+
# Delete user role assignments
195+
user_deleted_count = RoleUserAssignment.objects.filter(content_type=content_type, object_id=serializer_data['resource_pk']).delete()[0]
196+
197+
# Delete team role assignments
198+
team_deleted_count = RoleTeamAssignment.objects.filter(content_type=content_type, object_id=serializer_data['resource_pk']).delete()[0]
199+
200+
total_deleted = user_deleted_count + team_deleted_count
201+
202+
return Response(
203+
{
204+
'message': f'Deleted {total_deleted} role assignments for {serializer_data["resource_type"]} {serializer_data["resource_pk"]}',
205+
'deleted_count': total_deleted,
206+
'breakdown': {'user_assignments_deleted': user_deleted_count, 'team_assignments_deleted': team_deleted_count},
207+
},
208+
status=status.HTTP_200_OK,
209+
)

ansible_base/rbac/sync.py

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
- totally immutable model
99
- have very weird way of referencing related objects
1010
- must run various internal RBAC logic for rebuilding RoleEvaluation entries
11-
1211
2. RoleDefinition sync timing - which has timing issues:
1312
- post_save fires before many-to-many relations are saved
1413
- permissions need to be attached before syncing
14+
3. Object deletion cleanup - cross-service sync for orphaned assignments
1515
"""
1616

1717
import logging
@@ -62,6 +62,31 @@ def maybe_reverse_sync_unassignment(role_definition, actor, content_object):
6262
client.sync_unassignment(role_definition, actor, content_object)
6363

6464

65+
def maybe_reverse_sync_object_deletion(content_object):
66+
"""Trigger reverse-sync for object deletion to clean up orphaned role assignments.
67+
This is called when a resource with object-level role assignments is deleted.
68+
69+
Args:
70+
content_object: The deleted resource instance to clean up assignments for
71+
"""
72+
if not reverse_sync_enabled_all_conditions(content_object):
73+
logger.debug(f"Skipping reverse-sync object deletion for {content_object}")
74+
return
75+
76+
logger.debug(f"Performing reverse-sync object deletion for {content_object}")
77+
78+
try:
79+
from ansible_base.resource_registry.utils.sync_to_resource_server import get_current_user_resource_client
80+
81+
client = get_current_user_resource_client()
82+
client.sync_object_deletion(content_object)
83+
logger.debug(f"Successfully synced object deletion for {content_object}")
84+
except Exception as e:
85+
# Log the error but don't let sync failures break local deletion
86+
logger.warning(f"Failed to sync object deletion for {content_object}: {e}")
87+
return
88+
89+
6590
def maybe_reverse_sync_role_definition(instance, action="update"):
6691
"""Manually trigger reverse-sync for a RoleDefinition if appropriate.
6792

ansible_base/rbac/triggers.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,15 +255,33 @@ def rbac_post_delete_remove_object_roles(instance, *args, **kwargs):
255255

256256
# Similar to user deletion, clean up any orphaned object roles
257257
ObjectRole.objects.filter(users__isnull=True, teams__isnull=True).delete()
258+
deleted_count, _ = ObjectRole.objects.filter(users__isnull=True, teams__isnull=True).delete()
259+
if deleted_count:
260+
had_object_assignments = True
258261

259262
ct = permission_registry.content_type_model.objects.get_for_model(instance)
260-
ObjectRole.objects.filter(content_type=ct, object_id=instance.pk).delete()
263+
264+
# Use bulk delete return value to determine if object-level assignments existed
265+
# This avoids the inefficient .exists() query and works correctly for team deletion cases
266+
deleted_count, _ = ObjectRole.objects.filter(content_type=ct, object_id=instance.pk).delete()
267+
had_object_assignments = deleted_count > 0
261268

262269
parent_field_name = permission_registry.get_parent_fd_name(instance)
263270
if parent_field_name:
264271
# Delete all evaluations from inherited permissions
265272
get_evaluation_model(instance).objects.filter(content_type_id=ct.id, object_id=instance.pk).delete()
266273

274+
# Only sync when object-level assignments existed - this is the key performance optimization
275+
if had_object_assignments:
276+
try:
277+
from ansible_base.rbac.sync import maybe_reverse_sync_object_deletion
278+
279+
maybe_reverse_sync_object_deletion(instance)
280+
except Exception:
281+
# Continue with local deletion even if cross-service sync fails
282+
# This ensures we don't break local operations due to network/auth issues
283+
logger.exception(f"Failed to sync object deletion for {instance}")
284+
267285

268286
def rbac_post_user_delete(instance, *args, **kwargs):
269287
"""

ansible_base/resource_registry/rest_client.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,26 @@ def sync_unassignment(self, role_definition, actor, content_object):
212212

213213
return self._sync_assignment(data, giving=False)
214214

215+
def sync_object_deletion(self, content_object):
216+
"""Sync object deletion to Gateway for cleanup of all related role assignments"""
217+
from ansible_base.rbac.models import DABContentType
218+
219+
# Get the content type information
220+
content_type = DABContentType.objects.get_for_model(content_object)
221+
222+
data = {
223+
'resource_type': f'{content_type.app_label}.{content_type.model}',
224+
'resource_pk': str(content_object.pk), # Convert pk to string for JSON serialization
225+
}
226+
227+
# Make single API call to the new object-delete endpoint
228+
response = self._make_request("post", "object-delete/", data=data)
229+
230+
if response.status_code == 200:
231+
return response.json()
232+
else:
233+
return {'error': f'Failed with status {response.status_code}', 'status_code': response.status_code}
234+
215235
def _sync_assignment(self, data, giving=True):
216236
if giving:
217237
sub_url = 'assign'
@@ -224,4 +244,4 @@ def _sync_assignment(self, data, giving=True):
224244

225245
url = f'role-{actor_type}-assignments/{sub_url}/'
226246

227-
return self._make_request(method="post", path=url, data=data)
247+
return self._make_request("post", url, data=data)

0 commit comments

Comments
 (0)