Skip to content

Commit 34f2f04

Browse files
committed
Synchronize the created and add object_created field
1 parent cb33897 commit 34f2f04

File tree

6 files changed

+184
-12
lines changed

6 files changed

+184
-12
lines changed

ansible_base/rbac/migrations/0004_remote_permissions_additions.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import ansible_base.rbac.models.content_type
44
import ansible_base.rbac.remote
55
from django.db import migrations, models
6+
import django.utils.timezone
67

78

89
class Migration(migrations.Migration):
@@ -159,6 +160,28 @@ class Migration(migrations.Migration):
159160
model_name='roleevaluationuuid',
160161
name='dab_rbac_ro_role_id_4fe905_idx',
161162
),
163+
# Added field for a checksum like purpose for remote models
164+
migrations.AddField(
165+
model_name='roleuserassignment',
166+
name='object_created',
167+
field=models.DateTimeField(help_text='The created timestamp of related object, if applicable.', null=True),
168+
),
169+
migrations.AddField(
170+
model_name='roleteamassignment',
171+
name='object_created',
172+
field=models.DateTimeField(help_text='The created timestamp of related object, if applicable.', null=True),
173+
),
174+
# Make assignment created timestamp backdateable
175+
migrations.AlterField(
176+
model_name='roleteamassignment',
177+
name='created',
178+
field=models.DateTimeField(default=django.utils.timezone.now, editable=False, help_text='The date/time this resource was created.'),
179+
),
180+
migrations.AlterField(
181+
model_name='roleuserassignment',
182+
name='created',
183+
field=models.DateTimeField(default=django.utils.timezone.now, editable=False, help_text='The date/time this resource was created.'),
184+
),
162185
# Fields unique to DAB RBAC and not generally shared with ContentType or Permission
163186
migrations.AddField(
164187
model_name='dabcontenttype',

ansible_base/rbac/models/role.py

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import logging
22
from collections.abc import Iterable
3+
from datetime import datetime
34
from typing import Optional, Type, Union
45
from uuid import UUID
56

@@ -9,6 +10,7 @@
910
from django.db.models.functions import Cast
1011
from django.db.models.query import QuerySet
1112
from django.db.utils import IntegrityError
13+
from django.utils import timezone
1214
from django.utils.translation import gettext_lazy as _
1315

1416
# Django-rest-framework
@@ -63,6 +65,18 @@ def __getattr__(self, attr):
6365
return rd
6466

6567

68+
def get_created_timestamp(obj: Union[models.Model, RemoteObject]) -> Optional[datetime]:
69+
"""Given some obj from the users app, try to infer the created timestamp"""
70+
if isinstance(obj, RemoteObject):
71+
return obj.created
72+
for field_name in ('created', 'created_at'):
73+
if hasattr(obj, field_name):
74+
val = getattr(obj, field_name)
75+
if isinstance(val, datetime):
76+
return val
77+
return None
78+
79+
6680
class RoleDefinitionManager(models.Manager):
6781
def contribute_to_class(self, cls: Type[models.Model], name: str) -> None:
6882
"""After Django populates the model for the manager, attach the manager role manager"""
@@ -178,13 +192,13 @@ def __str__(self):
178192
managed_str = ', managed=True'
179193
return f'RoleDefinition(pk={self.id}, name={self.name}{managed_str})'
180194

181-
def give_global_permission(self, actor):
182-
return self.give_or_remove_global_permission(actor, giving=True)
195+
def give_global_permission(self, actor, assignment_created=None):
196+
return self.give_or_remove_global_permission(actor, giving=True, assignment_created=assignment_created)
183197

184198
def remove_global_permission(self, actor):
185199
return self.give_or_remove_global_permission(actor, giving=False)
186200

187-
def give_or_remove_global_permission(self, actor, giving=True):
201+
def give_or_remove_global_permission(self, actor, giving=True, assignment_created=None):
188202
if giving and (self.content_type is not None):
189203
raise ValidationError('Role definition content type must be null to assign globally')
190204

@@ -202,6 +216,8 @@ def give_or_remove_global_permission(self, actor, giving=True):
202216
raise RuntimeError(f'Cannot {giving and "give" or "remove"} permission for {actor}, must be a user or team')
203217

204218
if giving:
219+
if assignment_created:
220+
kwargs['created'] = assignment_created
205221
assignment, _ = cls.objects.get_or_create(**kwargs)
206222
else:
207223
assignment = cls.objects.filter(**kwargs).first()
@@ -221,8 +237,8 @@ def give_or_remove_global_permission(self, actor, giving=True):
221237

222238
return assignment
223239

224-
def give_permission(self, actor, content_object):
225-
return self.give_or_remove_permission(actor, content_object, giving=True)
240+
def give_permission(self, actor, content_object, assignment_created=None):
241+
return self.give_or_remove_permission(actor, content_object, giving=True, assignment_created=assignment_created)
226242

227243
def remove_permission(self, actor, content_object):
228244
return self.give_or_remove_permission(actor, content_object, giving=False)
@@ -247,7 +263,7 @@ def get_or_create_object_role(self, kwargs, defaults):
247263
object_role = ObjectRole.objects.create(**kwargs, **defaults)
248264
return (object_role, True)
249265

250-
def give_or_remove_permission(self, actor, content_object, giving=True, sync_action=False):
266+
def give_or_remove_permission(self, actor, content_object, giving=True, sync_action=False, assignment_created=None):
251267
"Shortcut method to do whatever needed to give user or team these permissions"
252268
validate_assignment(self, actor, content_object)
253269

@@ -278,15 +294,28 @@ def give_or_remove_permission(self, actor, content_object, giving=True, sync_act
278294

279295
update_teams, to_update = needed_updates_on_assignment(self, actor, object_role, created=created, giving=True)
280296

297+
assignment_defaults = {}
298+
object_created = get_created_timestamp(content_object)
299+
if object_created:
300+
assignment_defaults['object_created'] = object_created
301+
if assignment_created:
302+
assignment_defaults['created'] = assignment_created
303+
281304
assignment = None
282305
if actor._meta.model_name == 'user':
283306
if giving:
284-
assignment, created = RoleUserAssignment.objects.get_or_create(user=actor, object_role=object_role)
307+
try:
308+
assignment = RoleUserAssignment.objects.get(user=actor, object_role=object_role)
309+
except RoleUserAssignment.DoesNotExist:
310+
assignment = RoleUserAssignment.objects.create(user=actor, object_role=object_role, **assignment_defaults)
285311
else:
286312
object_role.users.remove(actor)
287313
elif isinstance(actor, permission_registry.team_model):
288314
if giving:
289-
assignment, created = RoleTeamAssignment.objects.get_or_create(team=actor, object_role=object_role)
315+
try:
316+
assignment = RoleTeamAssignment.objects.get(team=actor, object_role=object_role)
317+
except RoleTeamAssignment.DoesNotExist:
318+
assignment = RoleTeamAssignment.objects.create(team=actor, object_role=object_role, **assignment_defaults)
290319
else:
291320
object_role.teams.remove(actor)
292321

@@ -410,6 +439,14 @@ class AssignmentBase(ImmutableCommonModel, ObjectRoleFields):
410439
null=True, blank=True, help_text=_('The primary key of the object this assignment applies to; null value indicates system-wide assignment.')
411440
)
412441
content_type = models.ForeignKey(DABContentType, on_delete=models.CASCADE, null=True, help_text=_("The content type this applies to."))
442+
# The object_created field can be used for a checksum-like purpose to verify nothing strange happened with the related object
443+
object_created = models.DateTimeField(help_text=_("The created timestamp of related object, if applicable."), null=True)
444+
# Define this with default to make it possible to backdate if necessary, for sync
445+
created = models.DateTimeField(
446+
default=timezone.now,
447+
editable=False,
448+
help_text=_("The date/time this resource was created."),
449+
)
413450

414451
# object_role is internal, and not shown in serializer
415452
# content_type does not have a link, and ResourceType will be used in lieu sometime

ansible_base/rbac/remote.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,11 @@ def __init__(self, ct: models.Model, abstract=False):
5858
class RemoteObject:
5959
"""Placeholder for objects that live in another project."""
6060

61-
def __init__(self, content_type: models.Model, object_id: Union[int, str], parent_reference=None):
61+
def __init__(self, content_type: models.Model, object_id: Union[int, str], parent_reference=None, created=None):
6262
self.content_type = content_type
6363
self.object_id = object_id
64+
# Allow tracking details of the object
65+
self.created = created
6466
# Since object is remote, we do not have its properties here, so a pointer to the parent can be specified here
6567
self.parent_reference = parent_reference
6668
if not hasattr(self, '_meta'):

ansible_base/rbac/service_api/serializers.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ class BaseAssignmentSerializer(serializers.ModelSerializer):
5757
object_ansible_id = ObjectIDAnsibleIDField(source='object_id', required=False, allow_null=True)
5858
object_id = serializers.CharField(allow_blank=True, required=False, allow_null=True)
5959
from_service = serializers.CharField(write_only=True)
60+
# Force created field to be writable
61+
created = serializers.DateTimeField(required=False)
6062

6163
def to_representation(self, instance):
6264
# hack to surface content_object for ObjectIDAnsibleIDField
@@ -131,10 +133,10 @@ def create(self, validated_data):
131133
raise serializers.ValidationError({'object_id': _('Object must be specified for this role assignment')})
132134

133135
with transaction.atomic():
134-
assignment = rd.give_permission(actor, obj)
136+
assignment = rd.give_permission(actor, obj, assignment_created=validated_data.get('created'))
135137
else:
136138
with transaction.atomic():
137-
assignment = rd.give_global_permission(actor)
139+
assignment = rd.give_global_permission(actor, assignment_created=validated_data.get('created'))
138140

139141
return assignment
140142

test_app/tests/rbac/remote/test_remote_assignment.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,61 @@ def test_org_roles_same_type_different_service(rando, organization):
143143
], f'User should have permission to exactly {service_name} resource'
144144

145145
rds[service_name].remove_permission(rando, organization)
146+
147+
148+
@pytest.mark.django_db
149+
def test_object_created_field_local_model(rando, org_inv_rd, organization):
150+
"""
151+
Test that the object_created field is set to the local object's created timestamp
152+
when creating an assignment for a local model like inventory.
153+
154+
This test should FAIL because the object_created field doesn't exist yet.
155+
"""
156+
# Create the assignment
157+
assignment = org_inv_rd.give_permission(rando, organization)
158+
159+
# Verify the assignment was created
160+
assert assignment.user == rando
161+
assert assignment.role_definition == org_inv_rd
162+
assert assignment.object_id == organization.pk
163+
164+
# This should FAIL - the object_created field should be set to inventory.created
165+
assert hasattr(assignment, 'object_created'), "Assignment should have an object_created field"
166+
167+
assert assignment.object_created == organization.created, (
168+
f"object_created should be set to the organization's created timestamp. "
169+
f"Expected: {organization.created}, but object_created field is missing or has wrong value"
170+
)
171+
172+
173+
@pytest.mark.django_db
174+
def test_object_created_field_remote_object(rando, foo_type, foo_rd):
175+
"""
176+
Test that the object_created field is properly handled when creating an assignment
177+
for a remote object (stand-in object pattern).
178+
179+
This test should FAIL because the object_created field doesn't exist yet.
180+
"""
181+
from datetime import datetime, timezone
182+
183+
# Create a remote object stand-in
184+
remote_foo = RemoteObject(content_type=foo_type, object_id=42)
185+
186+
# Create the assignment
187+
assignment = foo_rd.give_permission(rando, remote_foo)
188+
189+
# Verify the assignment was created
190+
assert assignment.user == rando
191+
assert assignment.role_definition == foo_rd
192+
assert assignment.object_id == 42
193+
assert isinstance(assignment.content_object, RemoteObject)
194+
195+
# This should FAIL - the object_created field should exist and be handled for remote objects
196+
assert hasattr(assignment, 'object_created'), "Assignment should have an object_created field even for remote objects"
197+
198+
# For remote objects, the object_created field might be None or set to a default value
199+
# since we don't have the actual remote object's creation timestamp
200+
# The exact behavior will depend on implementation, but the field should exist
201+
assert assignment.object_created is not None or assignment.object_created is None, (
202+
f"object_created field should exist for remote objects. " f"Current value: {getattr(assignment, 'object_created', 'FIELD_MISSING')}"
203+
)

test_app/tests/rbac/remote/test_service_api.py

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import pytest
44

55
from ansible_base.lib.utils.response import get_relative_url
6-
from ansible_base.rbac.models import DABContentType, DABPermission, RoleDefinition
6+
from ansible_base.rbac.models import DABContentType, DABPermission, RoleDefinition, RoleTeamAssignment, RoleUserAssignment
77
from test_app.models import Team, User
88

99

@@ -287,3 +287,53 @@ def test_service_api_permissions(reverse_name, normal_case, unauth_case, admin_a
287287

288288
unauth_response = unauthenticated_api_client.get(url)
289289
assert unauth_response.status_code == unauth_case, unauth_response.data
290+
291+
292+
@pytest.mark.django_db
293+
def test_service_assignment_created_timestamp_sync(admin_api_client, rando, inv_rd, inventory):
294+
"""
295+
Test that demonstrates the field sync issue: the 'created' timestamp field is displayed
296+
in responses but not applied when creating assignments via POST to /assign/.
297+
298+
This test should FAIL, showing that custom timestamps are ignored and auto-generated instead.
299+
"""
300+
from datetime import datetime, timezone
301+
302+
from django.utils.dateparse import parse_datetime
303+
304+
url = get_relative_url('serviceuserassignment-assign')
305+
306+
creator_user = User.objects.create(username='timestamp_creator')
307+
308+
# Set a specific timestamp that's different from "now"
309+
custom_timestamp = datetime(2023, 1, 15, 10, 30, 45, tzinfo=timezone.utc)
310+
custom_timestamp_str = custom_timestamp.isoformat()
311+
312+
post_data = {
313+
"role_definition": inv_rd.name,
314+
"user_ansible_id": str(rando.resource.ansible_id),
315+
"object_id": str(inventory.pk),
316+
"created_by_ansible_id": str(creator_user.resource.ansible_id),
317+
"created": custom_timestamp_str,
318+
"from_service": "test_service",
319+
}
320+
321+
response = admin_api_client.post(url, data=post_data)
322+
assert response.status_code == 201, response.data
323+
324+
assignment = RoleUserAssignment.objects.get(user=rando, role_definition=inv_rd, object_id=inventory.pk)
325+
326+
# Test if the custom timestamp was properly set
327+
expected_created = custom_timestamp
328+
actual_created = assignment.created
329+
330+
# This should FAIL, demonstrating the field sync issue
331+
assert actual_created == expected_created, (
332+
f"FIELD SYNC ISSUE: Expected created timestamp '{expected_created}' but got '{actual_created}'. "
333+
f"The 'created' field is displayed in responses but not applied from POST data."
334+
)
335+
336+
# Verify response contains the timestamp field (showing it's "displayed")
337+
response_created = parse_datetime(response.data['created'])
338+
# Note: This will show the auto-generated timestamp, not our custom one
339+
assert response_created == expected_created, f"Response created timestamp should match: expected '{expected_created}' but got '{response_created}'"

0 commit comments

Comments
 (0)