Skip to content

Commit c1dacc4

Browse files
refactor: use actor_id to avoid storing pii from the actor
1 parent f5473da commit c1dacc4

7 files changed

Lines changed: 23 additions & 30 deletions

File tree

docs/decisions/0012-auditability.rst

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -70,18 +70,17 @@ Event payload
7070
"subject": "<namespaced subject key, e.g. user^alice>",
7171
"role": "<namespaced role key, e.g. role^instructor>",
7272
"scope": "<namespaced scope key, e.g. course-v1^course-v1:Org+Course+Run>",
73-
"actor": "<username of the caller, or None for system actor>",
73+
"actor_id": <database ID of the caller (int), or None for system actor>,
7474
}
7575
7676
The actor is resolved from ``django_crum.get_current_user()`` at API call time. No callers
77-
need to pass ``actor=`` explicitly. The username is stored as a plain string rather than a
78-
reference to the ``User`` record, so attribution is preserved even if the user is deleted.
77+
need to pass ``actor_id=`` explicitly.
7978
8079
Audit table
8180
-----------
8281
8382
``RoleAssignmentAudit`` mirrors the event payload. Registered in Django admin, filterable by
84-
user, role, scope, actor, and timestamp.
83+
user, role, scope, actor_id, and timestamp.
8584
8685
Subject, role, and scope are stored as plain namespaced key strings (e.g. ``user^alice``,
8786
``role^instructor``, ``lib^lib:Org1:lib1``). There are no FK references to live ``Subject``,
@@ -141,12 +140,11 @@ Consequences
141140
#. **Events are best-effort.** If the audit write fails, the Casbin policy is still durable.
142141
Consumers requiring guaranteed delivery must implement their own retry logic.
143142
144-
#. **``actor`` is nullable.** Non-request contexts (management commands, background tasks)
145-
record ``None``, logged as a system operation. ``actor`` is stored as a plain username
146-
string rather than a FK to ``User``. Attribution is preserved unconditionally: deleting
147-
or retiring a user does not affect existing audit records. This also avoids a dependency
148-
on the ``User`` table from the audit log, keeping audit records fully independent from
149-
live data.
143+
#. **``actor_id`` is nullable.** Non-request contexts (management commands, background tasks)
144+
record ``None``, logged as a system operation. ``actor_id`` is stored as a plain integer
145+
(the database ID of the caller) rather than a FK to ``User``. This avoids a dependency
146+
on the ``User`` table and keeps audit records fully independent from live data. Attribution
147+
is preserved unconditionally: deleting or retiring a user does not affect existing records.
150148
151149
#. **Audit records are independent from live authorization state.** Deleting a subject,
152150
scope, or role does not remove its audit history. Records may reference identifiers that

openedx_authz/admin.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,11 @@ def queryset(self, request, queryset):
122122
class RoleAssignmentAuditAdmin(admin.ModelAdmin):
123123
"""Read-only admin for the role assignment audit log."""
124124

125-
list_display = ("operation", "display_subject", "display_role", "display_scope", "actor", "timestamp")
125+
list_display = ("operation", "display_subject", "display_role", "display_scope", "actor_id", "timestamp")
126126
list_filter = ("operation", ScopeTypeFilter)
127127
search_fields = ("subject", "role", "scope")
128128
date_hierarchy = "timestamp"
129-
readonly_fields = ("operation", "subject", "role", "scope", "actor", "timestamp")
129+
readonly_fields = ("operation", "subject", "role", "scope", "actor_id", "timestamp")
130130

131131
@admin.display(description="subject")
132132
def display_subject(self, obj):

openedx_authz/api/roles.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ def assign_role_to_subject_in_scope(subject: SubjectData, role: RoleData, scope:
242242
subject=subject.namespaced_key,
243243
role=role.namespaced_key,
244244
scope=scope.namespaced_key,
245-
actor=getattr(get_current_user(), "username", None),
245+
actor_id=getattr(get_current_user(), "id", None),
246246
)
247247
)
248248
)
@@ -293,7 +293,7 @@ def unassign_role_from_subject_in_scope(subject: SubjectData, role: RoleData, sc
293293
subject=subject.namespaced_key,
294294
role=role.namespaced_key,
295295
scope=scope.namespaced_key,
296-
actor=getattr(get_current_user(), "username", None),
296+
actor_id=getattr(get_current_user(), "id", None),
297297
)
298298
)
299299
)

openedx_authz/handlers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ def create_audit_record_on_role_assignment_change(sender, role_assignment, **kwa
331331
subject=role_assignment.subject,
332332
role=role_assignment.role,
333333
scope=role_assignment.scope,
334-
actor=role_assignment.actor,
334+
actor_id=role_assignment.actor_id,
335335
timestamp=kwargs["metadata"].time,
336336
)
337337
except Exception as exc: # pylint: disable=broad-exception-caught

openedx_authz/migrations/0008_roleassignmentaudit.py

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
# Generated by Django 4.2.24 on 2026-04-15 16:48
1+
# Generated by Django 4.2.24 on 2026-04-20 15:54
22

33
from django.db import migrations, models
44

55

66
class Migration(migrations.Migration):
7+
78
dependencies = [
89
("openedx_authz", "0007_coursescope"),
910
]
@@ -45,20 +46,15 @@ class Migration(migrations.Migration):
4546
(
4647
"scope",
4748
models.CharField(
48-
help_text=(
49-
"Namespaced key of the scope (e.g. 'course-v1^course-v1:org+course+run') or glob pattern."
50-
),
49+
help_text="Namespaced key of the scope (e.g. 'course-v1^course-v1:org+course+run') or glob pattern.",
5150
max_length=255,
5251
),
5352
),
5453
(
55-
"actor",
56-
models.CharField(
54+
"actor_id",
55+
models.IntegerField(
5756
blank=True,
58-
help_text=(
59-
"Username of the user who performed the operation, or None for system-initiated actions."
60-
),
61-
max_length=150,
57+
help_text="Database ID of the user who performed the operation. Null for system-initiated actions.",
6258
null=True,
6359
),
6460
),

openedx_authz/models/core.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -285,11 +285,10 @@ class Operations(NamedTuple):
285285
max_length=255,
286286
help_text="Namespaced key of the scope (e.g. 'course-v1^course-v1:org+course+run') or glob pattern.",
287287
)
288-
actor = models.CharField(
289-
max_length=150,
288+
actor_id = models.IntegerField(
290289
null=True,
291290
blank=True,
292-
help_text="Username of the user who performed the operation, or None for system-initiated actions.",
291+
help_text="Database ID of the user who performed the operation. Null for system-initiated actions.",
293292
)
294293
timestamp = models.DateTimeField()
295294

openedx_authz/tests/test_handlers.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -721,7 +721,7 @@ def test_creates_audit_record_for_created_operation(self):
721721
722722
Expected result:
723723
- One RoleAssignmentAudit record exists with operation, subject, role, scope,
724-
actor (None), and the event timestamp.
724+
actor_id (None), and the event timestamp.
725725
"""
726726
role_assignment = RoleAssignmentData(
727727
operation=RoleAssignmentAudit.OPERATIONS.created,
@@ -737,7 +737,7 @@ def test_creates_audit_record_for_created_operation(self):
737737
self.assertEqual(audit.subject, "user^john_doe")
738738
self.assertEqual(audit.role, "role^library_admin")
739739
self.assertEqual(audit.scope, "lib^org1:lib1")
740-
self.assertIsNone(audit.actor)
740+
self.assertIsNone(audit.actor_id)
741741
self.assertEqual(audit.timestamp, self.TIMESTAMP)
742742

743743
def test_creates_audit_record_for_deleted_operation(self):

0 commit comments

Comments
 (0)