From 84a64ec23d95ac5ba4617e107a9be0123b18fa19 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Tue, 14 Apr 2026 10:52:10 +0200 Subject: [PATCH 01/16] feat: send role assignment lifecycle events Emits ROLE_ASSIGNMENT_CREATED and ROLE_ASSIGNMENT_DELETED events after role assignment operations commit, so consumers can stay up to date on the authorization state of the system. Each event includes the subject, role, scope, and the actor performing the operation (via get_current_user()). Also adds the auditability ADR (0012) documenting the design decisions behind this feature. --- docs/decisions/0012-auditability.rst | 238 +++++++++++++++++++++++++++ openedx_authz/api/roles.py | 45 ++++- 2 files changed, 281 insertions(+), 2 deletions(-) create mode 100644 docs/decisions/0012-auditability.rst diff --git a/docs/decisions/0012-auditability.rst b/docs/decisions/0012-auditability.rst new file mode 100644 index 00000000..62054964 --- /dev/null +++ b/docs/decisions/0012-auditability.rst @@ -0,0 +1,238 @@ +0012: Auditability for Authorization Changes +############################################ + +Status +****** + +**Draft** + +Context +******* + +The existing architecture (see `ADR 0005`_) introduced ``ExtendedCasbinRule``, which adds +``created_at``, ``updated_at``, and a ``metadata`` JSON field to the ``CasbinRule`` table. +This is not an audit trail: there is no actor, no operation type, and no mechanism for +downstream consumers to react to changes. + +As the framework is adopted across more Open edX services, operators and developers need +answers the current system cannot provide: + +- Who assigned this role, and when? +- Who removed a user's access, and was it intentional? +- Why was a permission check denied? + +A spike (OEPM-Spike: RBAC AuthZ Auditability) examined how peer systems approach this. +Auditability decomposes into three dimensions: + +1. **Attribution**: who changed access? (role assignments, removals) +2. **Explainability**: why was access granted or denied? (policy evaluation at check time) +3. **Usage**: who used access? (resource access events, business operations) + +SpiceDB and OpenFGA version the entire authorization graph, enabling historical +reconstruction. Keycloak uses event listeners on administrative actions. openedx-authz sits +between these: a mutable policy store with no built-in audit layer. + +The pycasbin ecosystem has no audit plugin and no mechanism in the +``casbin-django-orm-adapter`` for change tracking. ``WatcherEx`` provides rule-level hooks +but carries no actor context and does not cover update operations. + +Two transitive dependencies already cover what is needed: + +- **django-crum** (``0.7.9``, via ``edx-django-utils``): ``get_current_user()`` from + thread-local. Returns ``None`` in non-request contexts, treated as a system actor. +- **django-simple-history** (``3.11.0``, via ``edx-organizations``): model-level change + tracking with actor, timestamp, and before/after state. Not applied to any openedx-authz + model yet. + +The Auth0 FGA Logging API (October 2025) defines three acceptance criteria for this feature: + +- Who made a permission change? (attribution) +- What did a user access or attempt? (explainability + usage) +- Can logs be exported to external systems? (SIEM, Aspects) + +Decision +******** + +Three independent mechanisms, each answering a different question: + +- ``OpenedxPublicSignal``: something happened, react now +- ``RoleAssignmentAudit``: what happened, in what order, performed by whom +- ``django-simple-history`` on ``ExtendedCasbinRule``: what was the full state at time T + (future work) + +Attribution: Role Lifecycle Events and Audit Table +================================================== + +Emit an ``OpenedxPublicSignal`` from ``openedx_authz.api.roles`` after every successful role +assignment or removal, via ``transaction.on_commit``. A Celery handler writes the event to +``RoleAssignmentAudit``. + +The handler is enabled by default. Operators with Aspects or a SIEM can disable it via a +Django setting to avoid the redundant write. If the handler fails, the Casbin write and the +event are unaffected. + +.. note:: + + Whether to write to the audit table in the same process (no Celery) or via a separate + task is an open question. Needs latency benchmarking before implementation. + +Event payload +------------- + +.. code:: python + + { + "operation": "ASSIGN" | "REMOVE", + "user": "", + "role": "", + "scope": "", + "actor": "", + "timestamp": "", + } + +The actor is resolved from ``django_crum.get_current_user()`` at API call time. No callers +need to pass ``actor=`` explicitly. + +Audit table +----------- + +``RoleAssignmentAudit`` mirrors the event payload. Registered in Django admin, filterable by +user, role, scope, actor, and timestamp. + +Developer extensibility +----------------------- + +Plugin authors register handlers on the ``OpenedxPublicSignal`` to react to role lifecycle +events (notifications, cache updates, analytics). Developers without an event bus can consume +the underlying Django signal directly. If an event bus is configured, events are forwarded to +Aspects or external systems automatically. + +Explainability: Real-Time Decision Context +========================================== + +Expose ``enforce_ex()`` through the public Python API. It returns ``(result, explain_rule)``: +the boolean decision and the matched policy rule. Callers get the exact rule that allowed or +denied the request. + +Enforcement events are opt-in via ``AUTHZ_ENFORCEMENT_EVENTS_ENABLED``. When enabled, each +check fires an ``OpenedxPublicSignal`` forwarded to plugin consumers or an event bus. No audit +table is written: the volume makes per-check storage impractical. + +Historical explainability ("why did this user have access last Tuesday?") is deferred. Two +options are available, both requiring a breaking change to ``is_user_allowed`` to accept +``as_of``: + +- **Option A (event replay):** Replay ``ASSIGN``/``REMOVE`` events from ``RoleAssignmentAudit`` + up to T. No extra infrastructure; the data is already there once attribution is implemented. +- **Option B (snapshots):** Add ``HistoricalRecords()`` to ``ExtendedCasbinRule`` and use + ``as_of(T)`` for the full rule state, including policy definitions. History collection must + start before the target timestamp. + +``authz.policy`` is loaded into the DB and covered by Option B. ``model.conf`` is not +persisted. A ``model_hash`` field on ``ExtendedCasbinRule`` would let historical queries +detect whether the model changed. + +Consequences +************ + +Attribution +=========== + +- Operators get a filterable role assignment history in Django admin. No external tooling + required. +- Developers get a stable ``OpenedxPublicSignal`` extension point. First formally defined + event in openedx-authz. +- Events are best-effort: if the audit write fails, the Casbin policy is still durable. + Consumers requiring guaranteed delivery must implement their own retry logic. +- ``actor`` is nullable. Non-request contexts (management commands, background tasks) record + ``None``, logged as a system operation. +- No new dependencies introduced. +- Callers of ``openedx_authz.api.roles`` need no signature changes. + +Explainability +============== + +- Developers can retrieve the matched policy rule at check time for "why was this denied?" + debugging. +- The explanation is point-in-time only. Historical explainability is deferred. +- Enforcement events are opt-in by design. Enabling them without an external consumer + produces events that are emitted and discarded. +- No new dependencies introduced. + +Both flows +========== + +- ``RoleAssignmentAudit`` introduces a new migration. No existing table is modified. +- The ``OpenedxPublicSignal`` schema is a public API surface. Field additions are + backward-compatible; removals and renames are breaking changes. +- Usage auditing belongs at the application layer (Open edX tracking events, Aspects), not + in the authorization library. +- ``RoleAssignmentAudit`` is not tamper-proof. Compliance-grade immutability is a + later-phase concern. + +Alternatives Considered +*********************** + +``django-simple-history`` on ``ExtendedCasbinRule`` as the attribution audit trail +=================================================================================== + +Rejected for three reasons: + +- ``save_policy`` does bulk delete + bulk create and bypasses model signals. Any policy + reload creates a new snapshot. The ``history_date`` reflects when the table was written, + not when a role was assigned. Snapshot diffs cannot tell apart "Alice was assigned + instructor" from "policy reloaded, Alice already had the role." +- Model signals are not fired for bulk operations, so writes through ``save_policy`` are not + captured at all. +- ``ExtendedCasbinRule`` fields (``ptype``, ``v0``--``v5``) are semi-opaque and require an + interpretation layer. ``RoleAssignmentAudit`` translates at write time. + +``django-simple-history`` remains the right tool for Option B (point-in-time state +reconstruction), where it is a snapshot mechanism, not an operation log. + +Use Cases Addressed +******************* + ++------------------------------------------------------------+---------------+ +| Description | Flow | ++============================================================+===============+ +| Operator: who assigned a role to a user, and when? | Attribution | ++------------------------------------------------------------+---------------+ +| Operator: who removed a role from a user, and when? | Attribution | ++------------------------------------------------------------+---------------+ +| Operator: full role history for a given user | Attribution | ++------------------------------------------------------------+---------------+ +| Operator: access control history for a given resource | Attribution | ++------------------------------------------------------------+---------------+ +| Developer: hook into role lifecycle events from a plugin | Attribution | ++------------------------------------------------------------+---------------+ +| Operator/Developer: query role assignment history via API | Attribution | ++------------------------------------------------------------+---------------+ +| Developer: understand why a permission check was denied | Explainability| ++------------------------------------------------------------+---------------+ +| Operator/Developer: inspect a user's current permissions | Explainability| ++------------------------------------------------------------+---------------+ + +Deferred: resource access history / usage auditing; export to SIEM / Aspects (available as +a side effect of the event signal once an event bus is configured, not a first-class +deliverable of this ADR). + +References +********** + +- `ADR 0002`_ +- `ADR 0004`_ +- `ADR 0005`_ +- `Auth0 FGA Logging API`_ +- `openedx-events documentation`_ +- `django-simple-history documentation`_ +- `django-crum documentation`_ +- OEPM-Spike: RBAC AuthZ Auditability + +.. _ADR 0002: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0002-authorization-model-foundation.rst +.. _ADR 0004: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0004-technology-selection.rst +.. _ADR 0005: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0005-architecture-and-data-modeling.rst +.. _Auth0 FGA Logging API: https://auth0.com/blog/auth0-fga-logging-api-a-complete-audit-trail-for-authorization/ +.. _openedx-events documentation: https://docs.openedx.org/projects/openedx-events/en/latest/ +.. _django-simple-history documentation: https://django-simple-history.readthedocs.io/ +.. _django-crum documentation: https://pypi.org/project/django-crum/ diff --git a/openedx_authz/api/roles.py b/openedx_authz/api/roles.py index bc7ca1e4..6bbe6852 100644 --- a/openedx_authz/api/roles.py +++ b/openedx_authz/api/roles.py @@ -10,6 +10,7 @@ from collections import defaultdict +from crum import get_current_user from django.db import transaction from openedx_authz.api.data import ( @@ -24,6 +25,8 @@ from openedx_authz.api.permissions import get_permission_from_policy from openedx_authz.engine.enforcer import AuthzEnforcer from openedx_authz.models import ExtendedCasbinRule +from openedx_events.authz.signals import ROLE_ASSIGNMENT_CREATED, ROLE_ASSIGNMENT_DELETED +from openedx_events.authz.data import RoleAssignmentData as RoleAssignmentEventData __all__ = [ "assign_role_to_subject_in_scope", @@ -195,7 +198,11 @@ def get_all_roles_in_scope(scope: ScopeData) -> list[list[str]]: def assign_role_to_subject_in_scope(subject: SubjectData, role: RoleData, scope: ScopeData) -> bool: - """Assign a role to a subject. + """Assign a role to a subject within a specific scope. + + This function creates a role assignment by adding a grouping policy to the enforcer and + creating an ExtendedCasbinRule for auditing purposes. It also sends a ROLE_ASSIGNMENT_CREATED event + after the transaction commits. Args: subject: The ID of the subject. @@ -225,6 +232,20 @@ def assign_role_to_subject_in_scope(subject: SubjectData, role: RoleData, scope: if not extended_rule: raise Exception("Failed to create ExtendedCasbinRule for the assignment") + # .. event_implemented_name: ROLE_ASSIGNMENT_CREATED + # .. event_type: org.openedx.authz.role_assignment.created + transaction.on_commit( + lambda: ROLE_ASSIGNMENT_CREATED.send_event( + role_assignment=RoleAssignmentEventData( + operation=RoleAssignmentEventData.OPERATIONS.created, + subject=subject, + role=role, + scope=scope, + actor=get_current_user() + ) + ) + ) + # Invalidate policy cache to ensure changes are picked up AuthzEnforcer.invalidate_policy_cache() return True @@ -242,7 +263,11 @@ def batch_assign_role_to_subjects_in_scope(subjects: list[SubjectData], role: Ro def unassign_role_from_subject_in_scope(subject: SubjectData, role: RoleData, scope: ScopeData) -> bool: - """Unassign a role from a subject. + """Unassign a role from a subject within a specific scope. + + This function removes a role assignment by deleting the corresponding grouping policy from the enforcer and + deleting the associated ExtendedCasbinRule for auditing purposes. It also sends a ROLE_ASSIGNMENT_DELETED event + after the transaction commits. Args: subject: The ID of the subject. @@ -256,6 +281,22 @@ def unassign_role_from_subject_in_scope(subject: SubjectData, role: RoleData, sc success = enforcer.delete_roles_for_user_in_domain( subject.namespaced_key, role.namespaced_key, scope.namespaced_key ) + + # .. event_implemented_name: ROLE_ASSIGNMENT_DELETED + # .. event_type: org.openedx.authz.role_assignment.deleted + if success: + transaction.on_commit( + lambda: ROLE_ASSIGNMENT_DELETED.send_event( + role_assignment=RoleAssignmentEventData( + operation=RoleAssignmentEventData.OPERATIONS.deleted, + subject=subject, + role=role, + scope=scope, + actor=get_current_user() + ) + ) + ) + # Invalidate policy cache to ensure changes are picked up AuthzEnforcer.invalidate_policy_cache() return success From f0429c41b5683ece4cd52a283c4f1ec9084ffecb Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Tue, 14 Apr 2026 14:36:50 +0200 Subject: [PATCH 02/16] feat: add role assignment audit trail Introduces RoleAssignmentAudit, a read-only log of role assignment and removal events. Records store namespaced key strings (subject, role, scope) rather than FK references to live objects, so the audit history survives deletions independently of the authorization state. - Model and migration (0008) - Handler wired to ROLE_ASSIGNMENT_CREATED/DELETED signals - Read-only Django admin with scope-type filter and namespace-stripped display - Display properties (subject_display, role_display, scope_display) on the model - AUTHZ_POLICY_ATTRIBUTES_SEPARATOR moved to constants/ to avoid circular import - ADR 0012 updated with independence guarantee, actor exception, and filter rationale --- docs/decisions/0012-auditability.rst | 19 ++++ openedx_authz/admin.py | 69 ++++++++++++++ openedx_authz/api/roles.py | 22 +++-- openedx_authz/constants/__init__.py | 7 ++ openedx_authz/handlers.py | 34 +++++++ .../migrations/0008_roleassignmentaudit.py | 56 +++++++++++ openedx_authz/models/core.py | 68 +++++++++++++- openedx_authz/tests/api/test_roles.py | 4 +- openedx_authz/tests/test_handlers.py | 93 +++++++++++++++++++ 9 files changed, 360 insertions(+), 12 deletions(-) create mode 100644 openedx_authz/migrations/0008_roleassignmentaudit.py diff --git a/docs/decisions/0012-auditability.rst b/docs/decisions/0012-auditability.rst index 62054964..ad73be49 100644 --- a/docs/decisions/0012-auditability.rst +++ b/docs/decisions/0012-auditability.rst @@ -99,6 +99,16 @@ Audit table ``RoleAssignmentAudit`` mirrors the event payload. Registered in Django admin, filterable by user, role, scope, actor, and timestamp. +Subject, role, and scope are stored as plain namespaced key strings (e.g. ``user^alice``, +``role^instructor``, ``lib^lib:Org1:lib1``). There are no FK references to live ``Subject``, +``Scope``, or Casbin tables. Audit records survive the deletion of the underlying objects by +design: the value of an audit log depends on its unconditional durability. + +Because there are no FK references, the namespace prefix embedded in each string is the only +available signal for categorizing records by type. Admin filters (e.g. "content library", +"course") rely on ``scope__startswith`` lookups against that prefix rather than relational +joins. + Developer extensibility ----------------------- @@ -169,6 +179,15 @@ Both flows in the authorization library. - ``RoleAssignmentAudit`` is not tamper-proof. Compliance-grade immutability is a later-phase concern. +- Audit records are independent from live authorization state. Deleting a subject, scope, or + role does not remove its audit history. Records may reference identifiers that no longer + exist in the system. +- ``actor`` is the exception: it is stored as a FK to the ``User`` model with ``SET_NULL``. + Deleting a user sets ``actor`` to ``None``, losing attribution for any audit records they + produced. This is an accepted trade-off: user deletion is rare in Open edX (the standard + path is retirement, which anonymizes rather than hard-deletes), and the FK enables direct + admin filtering by actor. If unconditional attribution durability is needed, ``actor`` + should be changed to a plain string field. Alternatives Considered *********************** diff --git a/openedx_authz/admin.py b/openedx_authz/admin.py index da343691..4cfe83a2 100644 --- a/openedx_authz/admin.py +++ b/openedx_authz/admin.py @@ -7,7 +7,10 @@ from django.contrib import admin from django.utils.html import format_html +from openedx_authz.api.data import ContentLibraryData, CourseOverviewData +from openedx_authz.constants import AUTHZ_POLICY_ATTRIBUTES_SEPARATOR from openedx_authz.models import AuthzCourseAuthoringMigrationRun, ExtendedCasbinRule +from openedx_authz.models.core import RoleAssignmentAudit def pretty_json(value) -> str: @@ -87,3 +90,69 @@ class AuthzCourseAuthoringMigrationRunAdmin(admin.ModelAdmin): def pretty_metadata(self, obj): """Return formatted JSON for the metadata field.""" return pretty_json(obj.metadata) + + +class ScopeTypeFilter(admin.SimpleListFilter): + """Filter audit records by scope type (content library, course, etc.).""" + + title = "scope type" + parameter_name = "scope_type" + + def lookups(self, request, model_admin): + """Return the available scope type choices. + + Audit records are independent from live Casbin tables and scope objects: + there are no FK references to filter on. The namespace prefix in the + stored ``scope`` string (e.g. ``lib^``, ``course-v1^``) is the only + available signal for categorizing records by scope type. + """ + return [ + (ContentLibraryData.NAMESPACE, "Content Library"), + (CourseOverviewData.NAMESPACE, "Course"), + ] + + def queryset(self, request, queryset): + """Filter the queryset by scope namespace prefix.""" + if self.value(): + return queryset.filter( + scope__startswith=f"{self.value()}{AUTHZ_POLICY_ATTRIBUTES_SEPARATOR}" + ) + return queryset + + +@admin.register(RoleAssignmentAudit) +class RoleAssignmentAuditAdmin(admin.ModelAdmin): + """Read-only admin for the role assignment audit log.""" + + list_display = ("operation", "display_subject", "display_role", "display_scope", "actor", "timestamp") + list_filter = ("operation", ScopeTypeFilter) + search_fields = ("subject", "role", "scope") + date_hierarchy = "timestamp" + readonly_fields = ("operation", "subject", "role", "scope", "actor", "timestamp") + + @admin.display(description="subject") + def display_subject(self, obj): + """Subject key without the namespace prefix.""" + return obj.subject_display + + @admin.display(description="role") + def display_role(self, obj): + """Role name without the namespace prefix.""" + return obj.role_display + + @admin.display(description="scope") + def display_scope(self, obj): + """Scope key without the namespace prefix.""" + return obj.scope_display + + def has_add_permission(self, request): + """Audit records are created by the system only.""" + return False + + def has_change_permission(self, request, obj=None): + """Audit records must not be modified after creation.""" + return False + + def has_delete_permission(self, request, obj=None): + """Audit records must not be deleted through the admin.""" + return False diff --git a/openedx_authz/api/roles.py b/openedx_authz/api/roles.py index 6bbe6852..1f9b8650 100644 --- a/openedx_authz/api/roles.py +++ b/openedx_authz/api/roles.py @@ -13,6 +13,9 @@ from crum import get_current_user from django.db import transaction +from openedx_events.authz.data import RoleAssignmentData as RoleAssignmentEventData +from openedx_events.authz.signals import ROLE_ASSIGNMENT_CREATED, ROLE_ASSIGNMENT_DELETED + from openedx_authz.api.data import ( GroupingPolicyIndex, PermissionData, @@ -25,8 +28,7 @@ from openedx_authz.api.permissions import get_permission_from_policy from openedx_authz.engine.enforcer import AuthzEnforcer from openedx_authz.models import ExtendedCasbinRule -from openedx_events.authz.signals import ROLE_ASSIGNMENT_CREATED, ROLE_ASSIGNMENT_DELETED -from openedx_events.authz.data import RoleAssignmentData as RoleAssignmentEventData +from openedx_authz.models.core import RoleAssignmentAudit __all__ = [ "assign_role_to_subject_in_scope", @@ -237,10 +239,10 @@ def assign_role_to_subject_in_scope(subject: SubjectData, role: RoleData, scope: transaction.on_commit( lambda: ROLE_ASSIGNMENT_CREATED.send_event( role_assignment=RoleAssignmentEventData( - operation=RoleAssignmentEventData.OPERATIONS.created, - subject=subject, - role=role, - scope=scope, + operation=RoleAssignmentAudit.OPERATIONS.created, + subject=subject.namespaced_key, + role=role.namespaced_key, + scope=scope.namespaced_key, actor=get_current_user() ) ) @@ -288,10 +290,10 @@ def unassign_role_from_subject_in_scope(subject: SubjectData, role: RoleData, sc transaction.on_commit( lambda: ROLE_ASSIGNMENT_DELETED.send_event( role_assignment=RoleAssignmentEventData( - operation=RoleAssignmentEventData.OPERATIONS.deleted, - subject=subject, - role=role, - scope=scope, + operation=RoleAssignmentAudit.OPERATIONS.deleted, + subject=subject.namespaced_key, + role=role.namespaced_key, + scope=scope.namespaced_key, actor=get_current_user() ) ) diff --git a/openedx_authz/constants/__init__.py b/openedx_authz/constants/__init__.py index e69de29b..a8cf42ad 100644 --- a/openedx_authz/constants/__init__.py +++ b/openedx_authz/constants/__init__.py @@ -0,0 +1,7 @@ +"""Shared low-level constants for openedx_authz. + +Defined here rather than in api.data so that models and other modules at the +bottom of the import chain can use them without creating circular imports. +""" + +AUTHZ_POLICY_ATTRIBUTES_SEPARATOR = "^" diff --git a/openedx_authz/handlers.py b/openedx_authz/handlers.py index 755d485e..3fff3dfb 100644 --- a/openedx_authz/handlers.py +++ b/openedx_authz/handlers.py @@ -15,11 +15,15 @@ from django.dispatch import receiver from waffle.models import Flag +from openedx_events.authz.signals import ROLE_ASSIGNMENT_CREATED, ROLE_ASSIGNMENT_DELETED + from openedx_authz.api.users import unassign_all_roles_from_user from openedx_authz.engine.utils import run_course_authoring_migration from openedx_authz.models.authz_migration import MigrationType, ScopeType from openedx_authz.models.core import ExtendedCasbinRule from openedx_authz.models.subjects import UserSubject +from openedx_authz.api.users import unassign_all_roles_from_user +from openedx_authz.models.core import ExtendedCasbinRule, RoleAssignmentAudit try: from common.djangoapps.student.models import CourseAccessRole @@ -305,3 +309,33 @@ def trigger_course_authoring_migration( excluded_course_ids=excluded_course_ids, delete_after_migration=True, ) + + +@receiver(ROLE_ASSIGNMENT_CREATED) +@receiver(ROLE_ASSIGNMENT_DELETED) +def create_audit_record_on_role_assignment_change(sender, role_assignment, **kwargs): # pylint: disable=unused-argument + """ + Create an audit record when a role assignment is created or deleted. + + This handler listens for both creation and deletion of role assignments and logs the changes + for auditing purposes. + + Args: + sender: The signal class (ROLE_ASSIGNMENT_CREATED or ROLE_ASSIGNMENT_DELETED). + role_assignment: RoleAssignmentEventData carrying the operation, subject, role, scope, and actor. + **kwargs: Additional keyword arguments from the signal. + """ + try: + RoleAssignmentAudit.objects.create( + operation=role_assignment.operation, + subject=role_assignment.subject, + role=role_assignment.role, + scope=role_assignment.scope, + actor=role_assignment.actor, + timestamp=kwargs["metadata"].time, + ) + except Exception as exc: # pylint: disable=broad-exception-caught + logger.exception( + "Error creating audit record for role assignment change: %s", + exc, + ) diff --git a/openedx_authz/migrations/0008_roleassignmentaudit.py b/openedx_authz/migrations/0008_roleassignmentaudit.py new file mode 100644 index 00000000..05341ca3 --- /dev/null +++ b/openedx_authz/migrations/0008_roleassignmentaudit.py @@ -0,0 +1,56 @@ +# Generated by Django 4.2.24 on 2026-04-14 09:51 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("openedx_authz", "0007_coursescope"), + ] + + operations = [ + migrations.CreateModel( + name="RoleAssignmentAudit", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "operation", + models.CharField( + choices=[("created", "created"), ("deleted", "deleted")], + max_length=20, + ), + ), + ("subject", models.CharField(max_length=255)), + ("role", models.CharField(max_length=255)), + ("scope", models.CharField(max_length=255)), + ("timestamp", models.DateTimeField()), + ( + "actor", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="performed_role_assignment_audits", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "verbose_name": "Role Assignment Audit", + "verbose_name_plural": "Role Assignment Audits", + "ordering": ["-timestamp"], + }, + ), + ] diff --git a/openedx_authz/models/core.py b/openedx_authz/models/core.py index a3208464..76c26634 100644 --- a/openedx_authz/models/core.py +++ b/openedx_authz/models/core.py @@ -5,13 +5,17 @@ schema that focuses on the core authorization logic. """ -from typing import ClassVar +from typing import ClassVar, NamedTuple +from django.contrib.auth import get_user_model from django.db import models, transaction +from openedx_authz.constants import AUTHZ_POLICY_ATTRIBUTES_SEPARATOR from openedx_authz.engine.filter import Filter +User = get_user_model() + class BaseRegistryModel(models.Model): """Base model that supports automatic subclass registration. @@ -231,3 +235,65 @@ def create_based_on_policy( ) return extended_rule + + +class RoleAssignmentAudit(models.Model): + """Model to audit role assignments and unassignments. + + .. no_pii: + + This model captures the history of role assignments and unassignments for subjects + within specific scopes. It can be used for auditing purposes to track changes in + role assignments over time. + """ + + class Operations(NamedTuple): + created: str = "created" + deleted: str = "deleted" + + OPERATIONS = Operations() + + operation = models.CharField( + max_length=20, + choices=[(op, op) for op in OPERATIONS], + ) + subject = models.CharField( + max_length=255, + help_text="Namespaced key of the subject (e.g. 'user^john_doe').", + ) + role = models.CharField( + max_length=255, + help_text="Namespaced key of the role (e.g. 'role^library_admin').", + ) + scope = models.CharField( + max_length=255, + help_text="Namespaced key of the scope (e.g. 'course-v1^course-v1:org+course+run') or glob pattern.", + ) + actor = models.ForeignKey( + User, + on_delete=models.SET_NULL, + null=True, + blank=True, + related_name="performed_role_assignment_audits", + ) + timestamp = models.DateTimeField() + + class Meta: + verbose_name = "Role Assignment Audit" + verbose_name_plural = "Role Assignment Audits" + ordering = ["-timestamp"] + + @property + def subject_display(self) -> str: + """Subject key without the namespace prefix (e.g. ``john_doe`` for ``user^john_doe``).""" + return self.subject.split(AUTHZ_POLICY_ATTRIBUTES_SEPARATOR, 1)[-1] + + @property + def role_display(self) -> str: + """Role name without the namespace prefix (e.g. ``library_admin`` for ``role^library_admin``).""" + return self.role.split(AUTHZ_POLICY_ATTRIBUTES_SEPARATOR, 1)[-1] + + @property + def scope_display(self) -> str: + """Scope key without the namespace prefix (e.g. ``lib:Org1:lib1`` for ``lib^lib:Org1:lib1``).""" + return self.scope.split(AUTHZ_POLICY_ATTRIBUTES_SEPARATOR, 1)[-1] diff --git a/openedx_authz/tests/api/test_roles.py b/openedx_authz/tests/api/test_roles.py index ecdb80e2..1a21854d 100644 --- a/openedx_authz/tests/api/test_roles.py +++ b/openedx_authz/tests/api/test_roles.py @@ -6,12 +6,13 @@ """ from importlib.resources import files -from unittest.mock import patch +from unittest.mock import MagicMock, patch import casbin from ddt import data as ddt_data from ddt import ddt, unpack from django.test import TestCase +from openedx_events.authz.signals import ROLE_ASSIGNMENT_CREATED, ROLE_ASSIGNMENT_DELETED from openedx_authz.api.data import ( ActionData, @@ -51,6 +52,7 @@ from openedx_authz.engine.enforcer import AuthzEnforcer from openedx_authz.engine.utils import migrate_policy_between_enforcers from openedx_authz.models import ExtendedCasbinRule, Scope, Subject +from openedx_authz.models.core import RoleAssignmentAudit def _mock_get_or_create_scope(scope_data): diff --git a/openedx_authz/tests/test_handlers.py b/openedx_authz/tests/test_handlers.py index 81879f64..0f986f57 100644 --- a/openedx_authz/tests/test_handlers.py +++ b/openedx_authz/tests/test_handlers.py @@ -29,6 +29,16 @@ WaffleFlagOrgOverrideModel, ) +from datetime import datetime, timezone +from unittest.mock import MagicMock, patch + +from casbin_adapter.models import CasbinRule +from django.test import TestCase +from openedx_events.authz.data import RoleAssignmentData + +from openedx_authz.handlers import create_audit_record_on_role_assignment_change +from openedx_authz.models.core import ExtendedCasbinRule, RoleAssignmentAudit, Scope, Subject + AUTHZ_COURSE_AUTHORING_FLAG_NAME = "authz.enable_course_authoring" OTHER_WAFFLE_FLAG_NAME = "some.other.flag" @@ -689,3 +699,86 @@ def test_unknown_override_choice_follows_global(self): self.assertIsNone(get_migration_type(curr, prev, global_flag_enabled=False)) self.assertIsNone(get_migration_type(curr, prev, global_flag_enabled=True)) + + +class TestCreateAuditRecordHandler(TestCase): + """Confirm the audit record handler creates RoleAssignmentAudit entries correctly.""" + + TIMESTAMP = datetime(2026, 4, 14, 12, 0, 0, tzinfo=timezone.utc) + + def _call_handler(self, role_assignment, timestamp=None): + """Invoke the handler directly with a mock metadata object.""" + metadata = MagicMock() + metadata.time = timestamp or self.TIMESTAMP + create_audit_record_on_role_assignment_change( + sender=None, + role_assignment=role_assignment, + metadata=metadata, + ) + + def test_creates_audit_record_for_created_operation(self): + """Handler creates a RoleAssignmentAudit row with all fields set correctly. + + Expected result: + - One RoleAssignmentAudit record exists with operation, subject, role, scope, + actor (None), and the event timestamp. + """ + role_assignment = RoleAssignmentData( + operation=RoleAssignmentAudit.OPERATIONS.created, + subject="user^john_doe", + role="role^library_admin", + scope="lib^org1:lib1", + ) + + self._call_handler(role_assignment) + + audit = RoleAssignmentAudit.objects.get() + self.assertEqual(audit.operation, RoleAssignmentAudit.OPERATIONS.created) + self.assertEqual(audit.subject, "user^john_doe") + self.assertEqual(audit.role, "role^library_admin") + self.assertEqual(audit.scope, "lib^org1:lib1") + self.assertIsNone(audit.actor) + self.assertEqual(audit.timestamp, self.TIMESTAMP) + + def test_creates_audit_record_for_deleted_operation(self): + """Handler creates a RoleAssignmentAudit row with operation 'deleted'. + + Expected result: + - One RoleAssignmentAudit record exists with operation 'deleted'. + """ + role_assignment = RoleAssignmentData( + operation=RoleAssignmentAudit.OPERATIONS.deleted, + subject="user^john_doe", + role="role^library_admin", + scope="lib^org1:lib1", + ) + + self._call_handler(role_assignment) + + audit = RoleAssignmentAudit.objects.get() + self.assertEqual(audit.operation, RoleAssignmentAudit.OPERATIONS.deleted) + + def test_logs_exception_and_does_not_raise_when_creation_fails(self): + """Handler logs the error without re-raising when RoleAssignmentAudit.create fails. + + Expected result: + - logger.exception is called once with a message containing 'Error creating audit record'. + - No RoleAssignmentAudit record is created. + """ + role_assignment = RoleAssignmentData( + operation=RoleAssignmentAudit.OPERATIONS.created, + subject="user^john_doe", + role="role^library_admin", + scope="lib^org1:lib1", + ) + + with ( + patch("openedx_authz.handlers.RoleAssignmentAudit.objects.create") as mock_create, + patch("openedx_authz.handlers.logger") as mock_logger, + ): + mock_create.side_effect = Exception("DB error") + self._call_handler(role_assignment) + + mock_logger.exception.assert_called_once() + self.assertIn("Error creating audit record", mock_logger.exception.call_args[0][0]) + self.assertEqual(RoleAssignmentAudit.objects.count(), 0) From f91f2cde8c15fbc160f9d4cbb94f17926c5a2e2c Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Wed, 15 Apr 2026 12:29:13 +0200 Subject: [PATCH 03/16] refactor: move scope-type filtering to RoleAssignmentAuditQuerySet Add a `for_scope_namespace` queryset method so scope-type filtering is available both in the admin and in API-level querysets, without duplicating the namespace-prefix logic. The admin `ScopeTypeFilter` now delegates to this method instead of building the `scope__startswith` lookup directly. --- openedx_authz/admin.py | 5 +---- openedx_authz/models/core.py | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/openedx_authz/admin.py b/openedx_authz/admin.py index 4cfe83a2..12bed4d3 100644 --- a/openedx_authz/admin.py +++ b/openedx_authz/admin.py @@ -8,7 +8,6 @@ from django.utils.html import format_html from openedx_authz.api.data import ContentLibraryData, CourseOverviewData -from openedx_authz.constants import AUTHZ_POLICY_ATTRIBUTES_SEPARATOR from openedx_authz.models import AuthzCourseAuthoringMigrationRun, ExtendedCasbinRule from openedx_authz.models.core import RoleAssignmentAudit @@ -114,9 +113,7 @@ def lookups(self, request, model_admin): def queryset(self, request, queryset): """Filter the queryset by scope namespace prefix.""" if self.value(): - return queryset.filter( - scope__startswith=f"{self.value()}{AUTHZ_POLICY_ATTRIBUTES_SEPARATOR}" - ) + return queryset.for_scope_namespace(self.value()) return queryset diff --git a/openedx_authz/models/core.py b/openedx_authz/models/core.py index 76c26634..8849d813 100644 --- a/openedx_authz/models/core.py +++ b/openedx_authz/models/core.py @@ -237,6 +237,23 @@ def create_based_on_policy( return extended_rule +class RoleAssignmentAuditQuerySet(models.QuerySet): + """QuerySet for RoleAssignmentAudit with scope-based filtering.""" + + def for_scope_namespace(self, namespace: str) -> "RoleAssignmentAuditQuerySet": + """Return records whose scope starts with the given namespace prefix. + + Args: + namespace: The namespace to filter by (e.g. ``'lib'``, ``'course-v1'``). + Use the NAMESPACE class attribute from the corresponding ScopeData + subclass (e.g. ``ContentLibraryData.NAMESPACE``). + + Returns: + Queryset filtered to records matching the scope type. + """ + return self.filter(scope__startswith=f"{namespace}{AUTHZ_POLICY_ATTRIBUTES_SEPARATOR}") + + class RoleAssignmentAudit(models.Model): """Model to audit role assignments and unassignments. @@ -247,6 +264,8 @@ class RoleAssignmentAudit(models.Model): role assignments over time. """ + objects = RoleAssignmentAuditQuerySet.as_manager() + class Operations(NamedTuple): created: str = "created" deleted: str = "deleted" From 2028eaa730bc943bf3b186faf845c1dbebaf44e8 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Wed, 15 Apr 2026 12:29:30 +0200 Subject: [PATCH 04/16] chore: pin openedx-events to MJG/authz-audit branch The released version of openedx-events does not yet include the RoleAssignmentEventData event type needed by the audit trail feature. Pin to the feature branch until it is released. TODO: revert to a released version once MJG/authz-audit is merged and released to PyPI. --- requirements/base.in | 2 ++ requirements/base.txt | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/requirements/base.in b/requirements/base.in index 11b452c6..92a34492 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -14,3 +14,5 @@ edx-django-utils # Used for RequestCache edx-drf-extensions # Extensions for Django Rest Framework used by Open edX edx-organizations # Organizations library for Open edX django-waffle # Waffle library for feature flag management +# TODO: pin to released version once MJG/authz-audit is merged and released +git+https://github.com/openedx/openedx-events.git@MJG/authz-audit#egg=openedx-events diff --git a/requirements/base.txt b/requirements/base.txt index 1be06872..e0395d06 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -8,6 +8,7 @@ asgiref==3.11.1 # via django attrs==26.1.0 # via -r requirements/base.in + # openedx-events bracex==2.6 # via wcmatch casbin-django-orm-adapter==1.7.0 @@ -40,6 +41,7 @@ django==5.2.13 # edx-django-utils # edx-drf-extensions # edx-organizations + # openedx-events django-crum==0.7.9 # via edx-django-utils django-model-utils==5.0.0 @@ -69,15 +71,18 @@ edx-api-doc-tools==3.0.0 # via -r requirements/base.in edx-ccx-keys==2.0.2 # via -r requirements/base.in +edx-ccx-keys==2.0.2 + # via openedx-events edx-django-utils==8.0.1 # via # -r requirements/base.in # edx-drf-extensions + # openedx-events edx-drf-extensions==10.6.0 # via # -r requirements/base.in # edx-organizations -edx-opaque-keys==4.0.0 +edx-opaque-keys[django]==3.0.0 # via # -r requirements/base.in # edx-ccx-keys @@ -86,12 +91,20 @@ edx-opaque-keys==4.0.0 edx-organizations==8.0.0 # via -r requirements/base.in idna==3.12 +idna==3.11 + # openedx-events +fastavro==1.12.1 + # via openedx-events +idna==3.10 # via requests inflection==0.5.1 # via drf-yasg openedx-atlas==0.7.0 # via -r requirements/base.in packaging==26.1 +openedx-events @ git+https://github.com/openedx/openedx-events.git@MJG/authz-audit + # via -r requirements/base.in +packaging==26.0 # via drf-yasg pillow==12.2.0 # via edx-organizations From ed7884f85c289fc3a569f6ca8be73ec2372b3c4f Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Wed, 15 Apr 2026 16:28:55 +0200 Subject: [PATCH 05/16] docs: update ADR to make it more straightforward --- docs/decisions/0012-auditability.rst | 185 ++++++++++----------------- 1 file changed, 71 insertions(+), 114 deletions(-) diff --git a/docs/decisions/0012-auditability.rst b/docs/decisions/0012-auditability.rst index ad73be49..1812674a 100644 --- a/docs/decisions/0012-auditability.rst +++ b/docs/decisions/0012-auditability.rst @@ -14,8 +14,7 @@ The existing architecture (see `ADR 0005`_) introduced ``ExtendedCasbinRule``, w This is not an audit trail: there is no actor, no operation type, and no mechanism for downstream consumers to react to changes. -As the framework is adopted across more Open edX services, operators and developers need -answers the current system cannot provide: +Operators and developers need answers the current system cannot provide: - Who assigned this role, and when? - Who removed a user's access, and was it intentional? @@ -28,27 +27,14 @@ Auditability decomposes into three dimensions: 2. **Explainability**: why was access granted or denied? (policy evaluation at check time) 3. **Usage**: who used access? (resource access events, business operations) -SpiceDB and OpenFGA version the entire authorization graph, enabling historical -reconstruction. Keycloak uses event listeners on administrative actions. openedx-authz sits -between these: a mutable policy store with no built-in audit layer. +`SpiceDB`_ and `OpenFGA`_ track the full authorization graph as a versioned changelog, +enabling historical reconstruction. Keycloak uses event listeners on administrative actions. +openedx-authz sits between these: a mutable policy store with no built-in audit layer. +(See `OEPM-Spike\: RBAC AuthZ Auditability`_ for the peer system analysis.) -The pycasbin ecosystem has no audit plugin and no mechanism in the -``casbin-django-orm-adapter`` for change tracking. ``WatcherEx`` provides rule-level hooks -but carries no actor context and does not cover update operations. - -Two transitive dependencies already cover what is needed: - -- **django-crum** (``0.7.9``, via ``edx-django-utils``): ``get_current_user()`` from - thread-local. Returns ``None`` in non-request contexts, treated as a system actor. -- **django-simple-history** (``3.11.0``, via ``edx-organizations``): model-level change - tracking with actor, timestamp, and before/after state. Not applied to any openedx-authz - model yet. - -The Auth0 FGA Logging API (October 2025) defines three acceptance criteria for this feature: - -- Who made a permission change? (attribution) -- What did a user access or attempt? (explainability + usage) -- Can logs be exported to external systems? (SIEM, Aspects) +The pycasbin ecosystem has no audit plugin. Two transitive dependencies cover what is needed: +``django-crum`` (via ``edx-django-utils``) for actor capture, and ``django-simple-history`` +(via ``edx-organizations``) for point-in-time state reconstruction. Decision ******** @@ -60,34 +46,31 @@ Three independent mechanisms, each answering a different question: - ``django-simple-history`` on ``ExtendedCasbinRule``: what was the full state at time T (future work) -Attribution: Role Lifecycle Events and Audit Table -================================================== +See the `OEPM-Spike\: RBAC AuthZ Auditability`_ for the architecture diagram of the three +flows. + +#. Attribution: Role Lifecycle Events and Audit Table +===================================================== Emit an ``OpenedxPublicSignal`` from ``openedx_authz.api.roles`` after every successful role -assignment or removal, via ``transaction.on_commit``. A Celery handler writes the event to -``RoleAssignmentAudit``. +assignment or removal, via ``transaction.on_commit``. A synchronous Django signal receiver +writes the event to ``RoleAssignmentAudit`` in the same process. The handler is enabled by default. Operators with Aspects or a SIEM can disable it via a Django setting to avoid the redundant write. If the handler fails, the Casbin write and the event are unaffected. -.. note:: - - Whether to write to the audit table in the same process (no Celery) or via a separate - task is an open question. Needs latency benchmarking before implementation. - Event payload ------------- .. code:: python { - "operation": "ASSIGN" | "REMOVE", - "user": "", + "operation": "created" | "deleted", + "subject": "", "role": "", "scope": "", - "actor": "", - "timestamp": "", + "actor": "", } The actor is resolved from ``django_crum.get_current_user()`` at API call time. No callers @@ -117,8 +100,8 @@ events (notifications, cache updates, analytics). Developers without an event bu the underlying Django signal directly. If an event bus is configured, events are forwarded to Aspects or external systems automatically. -Explainability: Real-Time Decision Context -========================================== +#. Explainability: Real-Time Decision Context +============================================= Expose ``enforce_ex()`` through the public Python API. It returns ``(result, explain_rule)``: the boolean decision and the matched policy rule. Callers get the exact rule that allowed or @@ -134,6 +117,8 @@ options are available, both requiring a breaking change to ``is_user_allowed`` t - **Option A (event replay):** Replay ``ASSIGN``/``REMOVE`` events from ``RoleAssignmentAudit`` up to T. No extra infrastructure; the data is already there once attribution is implemented. + The `Auth0 FGA Logging API`_ uses this same pattern: their logging API is an event store + that you replay to answer historical questions. - **Option B (snapshots):** Add ``HistoricalRecords()`` to ``ExtendedCasbinRule`` and use ``as_of(T)`` for the full rule state, including policy definitions. History collection must start before the target timestamp. @@ -145,49 +130,47 @@ detect whether the model changed. Consequences ************ -Attribution -=========== - -- Operators get a filterable role assignment history in Django admin. No external tooling - required. -- Developers get a stable ``OpenedxPublicSignal`` extension point. First formally defined - event in openedx-authz. -- Events are best-effort: if the audit write fails, the Casbin policy is still durable. - Consumers requiring guaranteed delivery must implement their own retry logic. -- ``actor`` is nullable. Non-request contexts (management commands, background tasks) record - ``None``, logged as a system operation. -- No new dependencies introduced. -- Callers of ``openedx_authz.api.roles`` need no signature changes. - -Explainability -============== - -- Developers can retrieve the matched policy rule at check time for "why was this denied?" - debugging. -- The explanation is point-in-time only. Historical explainability is deferred. -- Enforcement events are opt-in by design. Enabling them without an external consumer - produces events that are emitted and discarded. -- No new dependencies introduced. - -Both flows -========== - -- ``RoleAssignmentAudit`` introduces a new migration. No existing table is modified. -- The ``OpenedxPublicSignal`` schema is a public API surface. Field additions are - backward-compatible; removals and renames are breaking changes. -- Usage auditing belongs at the application layer (Open edX tracking events, Aspects), not - in the authorization library. -- ``RoleAssignmentAudit`` is not tamper-proof. Compliance-grade immutability is a - later-phase concern. -- Audit records are independent from live authorization state. Deleting a subject, scope, or - role does not remove its audit history. Records may reference identifiers that no longer - exist in the system. -- ``actor`` is the exception: it is stored as a FK to the ``User`` model with ``SET_NULL``. - Deleting a user sets ``actor`` to ``None``, losing attribution for any audit records they - produced. This is an accepted trade-off: user deletion is rare in Open edX (the standard - path is retirement, which anonymizes rather than hard-deletes), and the FK enables direct - admin filtering by actor. If unconditional attribution durability is needed, ``actor`` - should be changed to a plain string field. +#. **Operators get a filterable role assignment history in Django admin.** No external + tooling required. + +#. **Developers get a stable** ``OpenedxPublicSignal`` **extension point.** First formally + defined event in openedx-authz. Callers of ``openedx_authz.api.roles`` need no signature + changes. + +#. **Events are best-effort.** If the audit write fails, the Casbin policy is still durable. + Consumers requiring guaranteed delivery must implement their own retry logic. + +#. **``actor`` is nullable.** Non-request contexts (management commands, background tasks) + record ``None``, logged as a system operation. ``actor`` is stored as a FK to ``User`` + with ``SET_NULL``: deleting a user loses attribution for their audit records. This is + accepted because user deletion is rare in Open edX (retirement anonymizes rather than + hard-deletes), and the FK enables admin filtering by actor. If unconditional attribution + durability is needed, ``actor`` should be a plain string field instead. + +#. **Audit records are independent from live authorization state.** Deleting a subject, + scope, or role does not remove its audit history. Records may reference identifiers that + no longer exist. + +#. **``RoleAssignmentAudit`` introduces a new migration.** No existing table is modified. + +#. **The** ``OpenedxPublicSignal`` **schema is a public API surface.** Field additions are + backward-compatible; removals and renames are breaking changes. + +#. **``RoleAssignmentAudit`` is not tamper-proof.** Compliance-grade immutability is a + later-phase concern. + +#. **No new dependencies introduced.** ``django-crum`` and ``django-simple-history`` are + already transitive dependencies. + +#. **Usage auditing belongs at the application layer** (Open edX tracking events, Aspects), + not in the authorization library. + +#. **Developers can retrieve the matched policy rule at check time** for "why was this + denied?" debugging. The explanation is point-in-time only; historical explainability is + deferred. + +#. **Enforcement events are opt-in by design.** Enabling them without an external consumer + produces events that are emitted and discarded. Alternatives Considered *********************** @@ -197,45 +180,15 @@ Alternatives Considered Rejected for three reasons: -- ``save_policy`` does bulk delete + bulk create and bypasses model signals. Any policy - reload creates a new snapshot. The ``history_date`` reflects when the table was written, - not when a role was assigned. Snapshot diffs cannot tell apart "Alice was assigned - instructor" from "policy reloaded, Alice already had the role." -- Model signals are not fired for bulk operations, so writes through ``save_policy`` are not - captured at all. +- ``save_policy`` (`casbin-django-orm-adapter adapter.py`_) uses ``QuerySet.delete()`` and + ``bulk_create``, both of which bypass model signals. History snapshots reflect when the + table was written, not when a role was assigned. - ``ExtendedCasbinRule`` fields (``ptype``, ``v0``--``v5``) are semi-opaque and require an interpretation layer. ``RoleAssignmentAudit`` translates at write time. ``django-simple-history`` remains the right tool for Option B (point-in-time state reconstruction), where it is a snapshot mechanism, not an operation log. -Use Cases Addressed -******************* - -+------------------------------------------------------------+---------------+ -| Description | Flow | -+============================================================+===============+ -| Operator: who assigned a role to a user, and when? | Attribution | -+------------------------------------------------------------+---------------+ -| Operator: who removed a role from a user, and when? | Attribution | -+------------------------------------------------------------+---------------+ -| Operator: full role history for a given user | Attribution | -+------------------------------------------------------------+---------------+ -| Operator: access control history for a given resource | Attribution | -+------------------------------------------------------------+---------------+ -| Developer: hook into role lifecycle events from a plugin | Attribution | -+------------------------------------------------------------+---------------+ -| Operator/Developer: query role assignment history via API | Attribution | -+------------------------------------------------------------+---------------+ -| Developer: understand why a permission check was denied | Explainability| -+------------------------------------------------------------+---------------+ -| Operator/Developer: inspect a user's current permissions | Explainability| -+------------------------------------------------------------+---------------+ - -Deferred: resource access history / usage auditing; export to SIEM / Aspects (available as -a side effect of the event signal once an event bus is configured, not a first-class -deliverable of this ADR). - References ********** @@ -246,12 +199,16 @@ References - `openedx-events documentation`_ - `django-simple-history documentation`_ - `django-crum documentation`_ -- OEPM-Spike: RBAC AuthZ Auditability +- `OEPM-Spike: RBAC AuthZ Auditability`_ .. _ADR 0002: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0002-authorization-model-foundation.rst .. _ADR 0004: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0004-technology-selection.rst .. _ADR 0005: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0005-architecture-and-data-modeling.rst .. _Auth0 FGA Logging API: https://auth0.com/blog/auth0-fga-logging-api-a-complete-audit-trail-for-authorization/ +.. _SpiceDB: https://github.com/authzed/spicedb +.. _OpenFGA: https://openfga.dev/ .. _openedx-events documentation: https://docs.openedx.org/projects/openedx-events/en/latest/ .. _django-simple-history documentation: https://django-simple-history.readthedocs.io/ .. _django-crum documentation: https://pypi.org/project/django-crum/ +.. _casbin-django-orm-adapter adapter.py: https://github.com/officialpycasbin/django-orm-adapter/blob/main/casbin_adapter/adapter.py +.. _OEPM-Spike\: RBAC AuthZ Auditability: https://openedx.atlassian.net/wiki/spaces/OEPM/pages/6045859842/Spike+-+RBAC+AuthZ+-+Auditability From 128155157228280b32bf69e2af1b793d5cf04e09 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Wed, 15 Apr 2026 17:46:01 +0200 Subject: [PATCH 06/16] chore: run make upgrade after including openedx-events --- requirements/base.txt | 17 +++++++++-------- requirements/dev.txt | 18 +++++++++++++++--- requirements/doc.txt | 17 ++++++++++++++--- requirements/quality.txt | 18 +++++++++++++++--- requirements/test.txt | 13 ++++++++++--- 5 files changed, 63 insertions(+), 20 deletions(-) diff --git a/requirements/base.txt b/requirements/base.txt index e0395d06..af13baaa 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -7,7 +7,8 @@ asgiref==3.11.1 # via django attrs==26.1.0 - # via -r requirements/base.in + # via + # -r requirements/base.in # openedx-events bracex==2.6 # via wcmatch @@ -70,9 +71,9 @@ drf-yasg==1.21.15 edx-api-doc-tools==3.0.0 # via -r requirements/base.in edx-ccx-keys==2.0.2 - # via -r requirements/base.in -edx-ccx-keys==2.0.2 - # via openedx-events + # via + # -r requirements/base.in + # openedx-events edx-django-utils==8.0.1 # via # -r requirements/base.in @@ -82,20 +83,20 @@ edx-drf-extensions==10.6.0 # via # -r requirements/base.in # edx-organizations -edx-opaque-keys[django]==3.0.0 +edx-opaque-keys[django]==4.0.0 # via # -r requirements/base.in # edx-ccx-keys # edx-drf-extensions # edx-organizations + # openedx-events edx-organizations==8.0.0 # via -r requirements/base.in idna==3.12 -idna==3.11 # openedx-events fastavro==1.12.1 # via openedx-events -idna==3.10 +idna==3.11 # via requests inflection==0.5.1 # via drf-yasg @@ -104,7 +105,7 @@ openedx-atlas==0.7.0 packaging==26.1 openedx-events @ git+https://github.com/openedx/openedx-events.git@MJG/authz-audit # via -r requirements/base.in -packaging==26.0 +packaging==26.1 # via drf-yasg pillow==12.2.0 # via edx-organizations diff --git a/requirements/dev.txt b/requirements/dev.txt index 19a8d456..54a88fe5 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -14,7 +14,9 @@ astroid==4.0.4 # pylint # pylint-celery attrs==26.1.0 - # via -r requirements/quality.txt + # via + # -r requirements/quality.txt + # openedx-events bracex==2.6 # via # -r requirements/quality.txt @@ -102,6 +104,7 @@ django==5.2.13 # edx-drf-extensions # edx-i18n-tools # edx-organizations + # openedx-events django-crum==0.7.9 # via # -r requirements/quality.txt @@ -142,11 +145,14 @@ drf-yasg==1.21.15 edx-api-doc-tools==3.0.0 # via -r requirements/quality.txt edx-ccx-keys==2.0.2 - # via -r requirements/quality.txt + # via + # -r requirements/quality.txt + # openedx-events edx-django-utils==8.0.1 # via # -r requirements/quality.txt # edx-drf-extensions + # openedx-events edx-drf-extensions==10.6.0 # via # -r requirements/quality.txt @@ -155,12 +161,13 @@ edx-i18n-tools==2.0.0 # via -r requirements/dev.in edx-lint==6.0.0 # via -r requirements/quality.txt -edx-opaque-keys==4.0.0 +edx-opaque-keys[django]==4.0.0 # via # -r requirements/quality.txt # edx-ccx-keys # edx-drf-extensions # edx-organizations + # openedx-events edx-organizations==8.0.0 # via -r requirements/quality.txt filelock==3.29.0 @@ -206,6 +213,11 @@ mccabe==0.7.0 # pylint openedx-atlas==0.7.0 # via -r requirements/quality.txt +<<<<<<< HEAD +======= +openedx-events @ git+https://github.com/openedx/openedx-events.git@MJG/authz-audit + # via -r requirements/quality.txt +>>>>>>> f9b6043 (chore: run make upgrade after including openedx-events) packaging==26.1 # via # -r requirements/ci.txt diff --git a/requirements/doc.txt b/requirements/doc.txt index 6b91f191..42be98c0 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -13,7 +13,9 @@ asgiref==3.11.1 # -r requirements/test.txt # django attrs==26.1.0 - # via -r requirements/test.txt + # via + # -r requirements/test.txt + # openedx-events babel==2.18.0 # via # pydata-sphinx-theme @@ -75,6 +77,7 @@ django==5.2.13 # edx-django-utils # edx-drf-extensions # edx-organizations + # openedx-events django-crum==0.7.9 # via # -r requirements/test.txt @@ -124,23 +127,31 @@ drf-yasg==1.21.15 edx-api-doc-tools==3.0.0 # via -r requirements/test.txt edx-ccx-keys==2.0.2 - # via -r requirements/test.txt + # via + # -r requirements/test.txt + # openedx-events edx-django-utils==8.0.1 # via # -r requirements/test.txt # edx-drf-extensions + # openedx-events edx-drf-extensions==10.6.0 # via # -r requirements/test.txt # edx-organizations -edx-opaque-keys==4.0.0 +edx-opaque-keys[django]==4.0.0 # via # -r requirements/test.txt # edx-ccx-keys # edx-drf-extensions # edx-organizations + # openedx-events edx-organizations==8.0.0 # via -r requirements/test.txt +fastavro==1.12.1 + # via + # -r requirements/test.txt + # openedx-events id==1.6.1 # via twine idna==3.12 diff --git a/requirements/quality.txt b/requirements/quality.txt index 91afebf2..5ac32a20 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -13,7 +13,9 @@ astroid==4.0.4 # pylint # pylint-celery attrs==26.1.0 - # via -r requirements/test.txt + # via + # -r requirements/test.txt + # openedx-events bracex==2.6 # via # -r requirements/test.txt @@ -74,6 +76,7 @@ django==5.2.13 # edx-django-utils # edx-drf-extensions # edx-organizations + # openedx-events django-crum==0.7.9 # via # -r requirements/test.txt @@ -114,23 +117,27 @@ drf-yasg==1.21.15 edx-api-doc-tools==3.0.0 # via -r requirements/test.txt edx-ccx-keys==2.0.2 - # via -r requirements/test.txt + # via + # -r requirements/test.txt + # openedx-events edx-django-utils==8.0.1 # via # -r requirements/test.txt # edx-drf-extensions + # openedx-events edx-drf-extensions==10.6.0 # via # -r requirements/test.txt # edx-organizations edx-lint==6.0.0 # via -r requirements/quality.in -edx-opaque-keys==4.0.0 +edx-opaque-keys[django]==4.0.0 # via # -r requirements/test.txt # edx-ccx-keys # edx-drf-extensions # edx-organizations + # openedx-events edx-organizations==8.0.0 # via -r requirements/test.txt idna==3.12 @@ -159,6 +166,11 @@ mccabe==0.7.0 # via pylint openedx-atlas==0.7.0 # via -r requirements/test.txt +<<<<<<< HEAD +======= +openedx-events @ git+https://github.com/openedx/openedx-events.git@MJG/authz-audit + # via -r requirements/test.txt +>>>>>>> f9b6043 (chore: run make upgrade after including openedx-events) packaging==26.1 # via # -r requirements/test.txt diff --git a/requirements/test.txt b/requirements/test.txt index f48ebd0c..1cf3a3c7 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -9,7 +9,9 @@ asgiref==3.11.1 # -r requirements/base.txt # django attrs==26.1.0 - # via -r requirements/base.txt + # via + # -r requirements/base.txt + # openedx-events bracex==2.6 # via # -r requirements/base.txt @@ -59,6 +61,7 @@ ddt==1.7.2 # edx-django-utils # edx-drf-extensions # edx-organizations + # openedx-events django-crum==0.7.9 # via # -r requirements/base.txt @@ -99,21 +102,25 @@ drf-yasg==1.21.15 edx-api-doc-tools==3.0.0 # via -r requirements/base.txt edx-ccx-keys==2.0.2 - # via -r requirements/base.txt + # via + # -r requirements/base.txt + # openedx-events edx-django-utils==8.0.1 # via # -r requirements/base.txt # edx-drf-extensions + # openedx-events edx-drf-extensions==10.6.0 # via # -r requirements/base.txt # edx-organizations -edx-opaque-keys==4.0.0 +edx-opaque-keys[django]==4.0.0 # via # -r requirements/base.txt # edx-ccx-keys # edx-drf-extensions # edx-organizations + # openedx-events edx-organizations==8.0.0 # via -r requirements/base.txt idna==3.12 From 5f46d56efc2c6d71c1a44903a9060562ec034fbe Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Wed, 15 Apr 2026 18:51:21 +0200 Subject: [PATCH 07/16] feat: store actor as username string in RoleAssignmentAudit Change RoleAssignmentAudit.actor from ForeignKey(User) to a plain CharField storing the username. Attribution of the operation is preserved unconditionally: deleting or retiring a user does not affect existing audit records, and the audit log has no dependency on the User table. The event payload carries actor as a username string resolved from get_current_user() at API call time. Update ADR-0012 to document the decision and its rationale. --- docs/decisions/0012-auditability.rst | 15 ++++---- openedx_authz/api/roles.py | 4 +- .../migrations/0008_roleassignmentaudit.py | 38 +++++++++++++------ openedx_authz/models/core.py | 7 ++-- openedx_authz/tests/api/test_roles.py | 6 +-- 5 files changed, 42 insertions(+), 28 deletions(-) diff --git a/docs/decisions/0012-auditability.rst b/docs/decisions/0012-auditability.rst index 1812674a..886b5199 100644 --- a/docs/decisions/0012-auditability.rst +++ b/docs/decisions/0012-auditability.rst @@ -70,11 +70,12 @@ Event payload "subject": "", "role": "", "scope": "", - "actor": "", + "actor": "", } The actor is resolved from ``django_crum.get_current_user()`` at API call time. No callers -need to pass ``actor=`` explicitly. +need to pass ``actor=`` explicitly. The username is stored as a plain string rather than a +reference to the ``User`` record, so attribution is preserved even if the user is deleted. Audit table ----------- @@ -141,11 +142,11 @@ Consequences Consumers requiring guaranteed delivery must implement their own retry logic. #. **``actor`` is nullable.** Non-request contexts (management commands, background tasks) - record ``None``, logged as a system operation. ``actor`` is stored as a FK to ``User`` - with ``SET_NULL``: deleting a user loses attribution for their audit records. This is - accepted because user deletion is rare in Open edX (retirement anonymizes rather than - hard-deletes), and the FK enables admin filtering by actor. If unconditional attribution - durability is needed, ``actor`` should be a plain string field instead. + record ``None``, logged as a system operation. ``actor`` is stored as a plain username + string rather than a FK to ``User``. Attribution is preserved unconditionally: deleting + or retiring a user does not affect existing audit records. This also avoids a dependency + on the ``User`` table from the audit log, keeping audit records fully independent from + live data. #. **Audit records are independent from live authorization state.** Deleting a subject, scope, or role does not remove its audit history. Records may reference identifiers that diff --git a/openedx_authz/api/roles.py b/openedx_authz/api/roles.py index 1f9b8650..ac472cb0 100644 --- a/openedx_authz/api/roles.py +++ b/openedx_authz/api/roles.py @@ -243,7 +243,7 @@ def assign_role_to_subject_in_scope(subject: SubjectData, role: RoleData, scope: subject=subject.namespaced_key, role=role.namespaced_key, scope=scope.namespaced_key, - actor=get_current_user() + actor=getattr(get_current_user(), "username", None) ) ) ) @@ -294,7 +294,7 @@ def unassign_role_from_subject_in_scope(subject: SubjectData, role: RoleData, sc subject=subject.namespaced_key, role=role.namespaced_key, scope=scope.namespaced_key, - actor=get_current_user() + actor=getattr(get_current_user(), "username", None) ) ) ) diff --git a/openedx_authz/migrations/0008_roleassignmentaudit.py b/openedx_authz/migrations/0008_roleassignmentaudit.py index 05341ca3..fef879fb 100644 --- a/openedx_authz/migrations/0008_roleassignmentaudit.py +++ b/openedx_authz/migrations/0008_roleassignmentaudit.py @@ -1,14 +1,11 @@ -# Generated by Django 4.2.24 on 2026-04-14 09:51 +# Generated by Django 4.2.24 on 2026-04-15 16:48 -from django.conf import settings from django.db import migrations, models -import django.db.models.deletion class Migration(migrations.Migration): dependencies = [ - migrations.swappable_dependency(settings.AUTH_USER_MODEL), ("openedx_authz", "0007_coursescope"), ] @@ -32,20 +29,37 @@ class Migration(migrations.Migration): max_length=20, ), ), - ("subject", models.CharField(max_length=255)), - ("role", models.CharField(max_length=255)), - ("scope", models.CharField(max_length=255)), - ("timestamp", models.DateTimeField()), + ( + "subject", + models.CharField( + help_text="Namespaced key of the subject (e.g. 'user^john_doe').", + max_length=255, + ), + ), + ( + "role", + models.CharField( + help_text="Namespaced key of the role (e.g. 'role^library_admin').", + max_length=255, + ), + ), + ( + "scope", + models.CharField( + help_text="Namespaced key of the scope (e.g. 'course-v1^course-v1:org+course+run') or glob pattern.", + max_length=255, + ), + ), ( "actor", - models.ForeignKey( + models.CharField( blank=True, + help_text="Username of the user who performed the operation, or None for system-initiated actions.", + max_length=150, null=True, - on_delete=django.db.models.deletion.SET_NULL, - related_name="performed_role_assignment_audits", - to=settings.AUTH_USER_MODEL, ), ), + ("timestamp", models.DateTimeField()), ], options={ "verbose_name": "Role Assignment Audit", diff --git a/openedx_authz/models/core.py b/openedx_authz/models/core.py index 8849d813..2e67953d 100644 --- a/openedx_authz/models/core.py +++ b/openedx_authz/models/core.py @@ -288,12 +288,11 @@ class Operations(NamedTuple): max_length=255, help_text="Namespaced key of the scope (e.g. 'course-v1^course-v1:org+course+run') or glob pattern.", ) - actor = models.ForeignKey( - User, - on_delete=models.SET_NULL, + actor = models.CharField( + max_length=150, null=True, blank=True, - related_name="performed_role_assignment_audits", + help_text="Username of the user who performed the operation, or None for system-initiated actions.", ) timestamp = models.DateTimeField() diff --git a/openedx_authz/tests/api/test_roles.py b/openedx_authz/tests/api/test_roles.py index 1a21854d..c68ffd41 100644 --- a/openedx_authz/tests/api/test_roles.py +++ b/openedx_authz/tests/api/test_roles.py @@ -6,13 +6,13 @@ """ from importlib.resources import files -from unittest.mock import MagicMock, patch +from unittest.mock import patch import casbin from ddt import data as ddt_data from ddt import ddt, unpack from django.test import TestCase -from openedx_events.authz.signals import ROLE_ASSIGNMENT_CREATED, ROLE_ASSIGNMENT_DELETED + from openedx_authz.api.data import ( ActionData, @@ -52,7 +52,7 @@ from openedx_authz.engine.enforcer import AuthzEnforcer from openedx_authz.engine.utils import migrate_policy_between_enforcers from openedx_authz.models import ExtendedCasbinRule, Scope, Subject -from openedx_authz.models.core import RoleAssignmentAudit + def _mock_get_or_create_scope(scope_data): From fdd85e0af6af9f387aaae78d3283e6623cef0aea Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Wed, 15 Apr 2026 20:56:49 +0200 Subject: [PATCH 08/16] refactor: address quality issues --- openedx_authz/admin.py | 3 ++- openedx_authz/api/roles.py | 1 - openedx_authz/migrations/0008_roleassignmentaudit.py | 10 ++++++++-- openedx_authz/models/core.py | 1 - openedx_authz/tests/api/test_roles.py | 2 -- 5 files changed, 10 insertions(+), 7 deletions(-) diff --git a/openedx_authz/admin.py b/openedx_authz/admin.py index 12bed4d3..56d578a9 100644 --- a/openedx_authz/admin.py +++ b/openedx_authz/admin.py @@ -98,7 +98,8 @@ class ScopeTypeFilter(admin.SimpleListFilter): parameter_name = "scope_type" def lookups(self, request, model_admin): - """Return the available scope type choices. + """ + Return the available scope type choices. Audit records are independent from live Casbin tables and scope objects: there are no FK references to filter on. The namespace prefix in the diff --git a/openedx_authz/api/roles.py b/openedx_authz/api/roles.py index ac472cb0..e204c928 100644 --- a/openedx_authz/api/roles.py +++ b/openedx_authz/api/roles.py @@ -12,7 +12,6 @@ from crum import get_current_user from django.db import transaction - from openedx_events.authz.data import RoleAssignmentData as RoleAssignmentEventData from openedx_events.authz.signals import ROLE_ASSIGNMENT_CREATED, ROLE_ASSIGNMENT_DELETED diff --git a/openedx_authz/migrations/0008_roleassignmentaudit.py b/openedx_authz/migrations/0008_roleassignmentaudit.py index fef879fb..a7b59382 100644 --- a/openedx_authz/migrations/0008_roleassignmentaudit.py +++ b/openedx_authz/migrations/0008_roleassignmentaudit.py @@ -46,7 +46,10 @@ class Migration(migrations.Migration): ( "scope", models.CharField( - help_text="Namespaced key of the scope (e.g. 'course-v1^course-v1:org+course+run') or glob pattern.", + help_text=( + "Namespaced key of the scope" + " (e.g. 'course-v1^course-v1:org+course+run') or glob pattern." + ), max_length=255, ), ), @@ -54,7 +57,10 @@ class Migration(migrations.Migration): "actor", models.CharField( blank=True, - help_text="Username of the user who performed the operation, or None for system-initiated actions.", + help_text=( + "Username of the user who performed the operation," + " or None for system-initiated actions." + ), max_length=150, null=True, ), diff --git a/openedx_authz/models/core.py b/openedx_authz/models/core.py index 2e67953d..352649f3 100644 --- a/openedx_authz/models/core.py +++ b/openedx_authz/models/core.py @@ -13,7 +13,6 @@ from openedx_authz.constants import AUTHZ_POLICY_ATTRIBUTES_SEPARATOR from openedx_authz.engine.filter import Filter - User = get_user_model() class BaseRegistryModel(models.Model): diff --git a/openedx_authz/tests/api/test_roles.py b/openedx_authz/tests/api/test_roles.py index c68ffd41..ecdb80e2 100644 --- a/openedx_authz/tests/api/test_roles.py +++ b/openedx_authz/tests/api/test_roles.py @@ -13,7 +13,6 @@ from ddt import ddt, unpack from django.test import TestCase - from openedx_authz.api.data import ( ActionData, ContentLibraryData, @@ -54,7 +53,6 @@ from openedx_authz.models import ExtendedCasbinRule, Scope, Subject - def _mock_get_or_create_scope(scope_data): """Mock implementation that creates actual Scope instances.""" scope, _ = Scope.objects.get_or_create(id=hash(scope_data.external_key) % 10000) From a46866aea11219c339d3e1c1950850a9a2dcd049 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Mon, 20 Apr 2026 16:07:47 +0200 Subject: [PATCH 09/16] test: add test cases for when assignment is false --- openedx_authz/models/core.py | 3 --- openedx_authz/tests/api/test_roles.py | 34 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/openedx_authz/models/core.py b/openedx_authz/models/core.py index 352649f3..b9c3cdcc 100644 --- a/openedx_authz/models/core.py +++ b/openedx_authz/models/core.py @@ -7,14 +7,11 @@ from typing import ClassVar, NamedTuple -from django.contrib.auth import get_user_model from django.db import models, transaction from openedx_authz.constants import AUTHZ_POLICY_ATTRIBUTES_SEPARATOR from openedx_authz.engine.filter import Filter -User = get_user_model() - class BaseRegistryModel(models.Model): """Base model that supports automatic subclass registration. diff --git a/openedx_authz/tests/api/test_roles.py b/openedx_authz/tests/api/test_roles.py index ecdb80e2..52dc8775 100644 --- a/openedx_authz/tests/api/test_roles.py +++ b/openedx_authz/tests/api/test_roles.py @@ -1327,6 +1327,40 @@ def test_unassign_and_reassign_subject(self): new_roles = {r.external_key for assignment in new_assignments for r in assignment.roles} self.assertIn("library_admin", new_roles) + def test_no_event_sent_when_role_already_assigned(self): + """No lifecycle event is emitted when the role assignment already exists. + + Expected result: + - ``assign_role_to_subject_in_scope`` returns False. + - ``transaction.on_commit`` is not called. + """ + subject = SubjectData(external_key="alice") + role = RoleData(external_key=roles.LIBRARY_ADMIN.external_key) + scope = ScopeData(external_key="lib:Org1:math_101") + + with patch("openedx_authz.api.roles.transaction.on_commit") as mock_on_commit: + result = assign_role_to_subject_in_scope(subject, role, scope) + + self.assertFalse(result) + mock_on_commit.assert_not_called() + + def test_no_event_sent_when_role_not_present_for_removal(self): + """No lifecycle event is emitted when the role to remove does not exist. + + Expected result: + - ``unassign_role_from_subject_in_scope`` returns False. + - ``transaction.on_commit`` is not called. + """ + subject = SubjectData(external_key="alice") + role = RoleData(external_key=roles.LIBRARY_USER.external_key) + scope = ScopeData(external_key="lib:Org1:math_101") + + with patch("openedx_authz.api.roles.transaction.on_commit") as mock_on_commit: + result = unassign_role_from_subject_in_scope(subject, role, scope) + + self.assertFalse(result) + mock_on_commit.assert_not_called() + @ddt class TestFieldIndexAndValues(TestCase): From 37c09f39fcae8b0679579a2b2daa2510f09d7bee Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Mon, 20 Apr 2026 16:21:00 +0200 Subject: [PATCH 10/16] refactor: address quality issues --- openedx_authz/api/roles.py | 4 ++-- openedx_authz/engine/utils.py | 5 ++++- openedx_authz/migrations/0008_roleassignmentaudit.py | 7 ++----- openedx_authz/models/core.py | 1 + 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/openedx_authz/api/roles.py b/openedx_authz/api/roles.py index e204c928..835ffe18 100644 --- a/openedx_authz/api/roles.py +++ b/openedx_authz/api/roles.py @@ -242,7 +242,7 @@ def assign_role_to_subject_in_scope(subject: SubjectData, role: RoleData, scope: subject=subject.namespaced_key, role=role.namespaced_key, scope=scope.namespaced_key, - actor=getattr(get_current_user(), "username", None) + actor=getattr(get_current_user(), "username", None), ) ) ) @@ -293,7 +293,7 @@ def unassign_role_from_subject_in_scope(subject: SubjectData, role: RoleData, sc subject=subject.namespaced_key, role=role.namespaced_key, scope=scope.namespaced_key, - actor=getattr(get_current_user(), "username", None) + actor=getattr(get_current_user(), "username", None), ) ) ) diff --git a/openedx_authz/engine/utils.py b/openedx_authz/engine/utils.py index 93f27e1c..344ab4e8 100644 --- a/openedx_authz/engine/utils.py +++ b/openedx_authz/engine/utils.py @@ -405,7 +405,10 @@ def migrate_authz_to_legacy_course_roles( _validate_migration_input(course_id_list, org_id) role_assignments = get_all_role_assignments_per_scope_type( - scope_types=(CourseOverviewData, OrgCourseOverviewGlobData) + scope_types=( + CourseOverviewData, + OrgCourseOverviewGlobData, + ) ) # Two cases here: diff --git a/openedx_authz/migrations/0008_roleassignmentaudit.py b/openedx_authz/migrations/0008_roleassignmentaudit.py index a7b59382..70f672a5 100644 --- a/openedx_authz/migrations/0008_roleassignmentaudit.py +++ b/openedx_authz/migrations/0008_roleassignmentaudit.py @@ -4,7 +4,6 @@ class Migration(migrations.Migration): - dependencies = [ ("openedx_authz", "0007_coursescope"), ] @@ -47,8 +46,7 @@ class Migration(migrations.Migration): "scope", models.CharField( help_text=( - "Namespaced key of the scope" - " (e.g. 'course-v1^course-v1:org+course+run') or glob pattern." + "Namespaced key of the scope (e.g. 'course-v1^course-v1:org+course+run') or glob pattern." ), max_length=255, ), @@ -58,8 +56,7 @@ class Migration(migrations.Migration): models.CharField( blank=True, help_text=( - "Username of the user who performed the operation," - " or None for system-initiated actions." + "Username of the user who performed the operation, or None for system-initiated actions." ), max_length=150, null=True, diff --git a/openedx_authz/models/core.py b/openedx_authz/models/core.py index b9c3cdcc..640cc63c 100644 --- a/openedx_authz/models/core.py +++ b/openedx_authz/models/core.py @@ -12,6 +12,7 @@ from openedx_authz.constants import AUTHZ_POLICY_ATTRIBUTES_SEPARATOR from openedx_authz.engine.filter import Filter + class BaseRegistryModel(models.Model): """Base model that supports automatic subclass registration. From f5473dabdaf16a05c5408c9999b65a251f59b08b Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Mon, 20 Apr 2026 16:34:25 +0200 Subject: [PATCH 11/16] feat: add indices to speed up filtering and search --- openedx_authz/migrations/0008_roleassignmentaudit.py | 4 ++++ openedx_authz/models/core.py | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/openedx_authz/migrations/0008_roleassignmentaudit.py b/openedx_authz/migrations/0008_roleassignmentaudit.py index 70f672a5..4991f2d5 100644 --- a/openedx_authz/migrations/0008_roleassignmentaudit.py +++ b/openedx_authz/migrations/0008_roleassignmentaudit.py @@ -68,6 +68,10 @@ class Migration(migrations.Migration): "verbose_name": "Role Assignment Audit", "verbose_name_plural": "Role Assignment Audits", "ordering": ["-timestamp"], + "indexes": [ + models.Index(fields=["subject"], name="role_audit_subject_idx"), + models.Index(fields=["scope"], name="role_audit_scope_idx"), + ], }, ), ] diff --git a/openedx_authz/models/core.py b/openedx_authz/models/core.py index 640cc63c..68d01dc1 100644 --- a/openedx_authz/models/core.py +++ b/openedx_authz/models/core.py @@ -297,6 +297,10 @@ class Meta: verbose_name = "Role Assignment Audit" verbose_name_plural = "Role Assignment Audits" ordering = ["-timestamp"] + indexes = [ + models.Index(fields=["subject"], name="role_audit_subject_idx"), + models.Index(fields=["scope"], name="role_audit_scope_idx"), + ] @property def subject_display(self) -> str: From c1dacc41ad5004d7305a77497cc671cc1ce52a4d Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Mon, 20 Apr 2026 18:03:09 +0200 Subject: [PATCH 12/16] refactor: use actor_id to avoid storing pii from the actor --- docs/decisions/0012-auditability.rst | 18 ++++++++---------- openedx_authz/admin.py | 4 ++-- openedx_authz/api/roles.py | 4 ++-- openedx_authz/handlers.py | 2 +- .../migrations/0008_roleassignmentaudit.py | 16 ++++++---------- openedx_authz/models/core.py | 5 ++--- openedx_authz/tests/test_handlers.py | 4 ++-- 7 files changed, 23 insertions(+), 30 deletions(-) diff --git a/docs/decisions/0012-auditability.rst b/docs/decisions/0012-auditability.rst index 886b5199..f2d6cbe5 100644 --- a/docs/decisions/0012-auditability.rst +++ b/docs/decisions/0012-auditability.rst @@ -70,18 +70,17 @@ Event payload "subject": "", "role": "", "scope": "", - "actor": "", + "actor_id": , } The actor is resolved from ``django_crum.get_current_user()`` at API call time. No callers -need to pass ``actor=`` explicitly. The username is stored as a plain string rather than a -reference to the ``User`` record, so attribution is preserved even if the user is deleted. +need to pass ``actor_id=`` explicitly. Audit table ----------- ``RoleAssignmentAudit`` mirrors the event payload. Registered in Django admin, filterable by -user, role, scope, actor, and timestamp. +user, role, scope, actor_id, and timestamp. Subject, role, and scope are stored as plain namespaced key strings (e.g. ``user^alice``, ``role^instructor``, ``lib^lib:Org1:lib1``). There are no FK references to live ``Subject``, @@ -141,12 +140,11 @@ Consequences #. **Events are best-effort.** If the audit write fails, the Casbin policy is still durable. Consumers requiring guaranteed delivery must implement their own retry logic. -#. **``actor`` is nullable.** Non-request contexts (management commands, background tasks) - record ``None``, logged as a system operation. ``actor`` is stored as a plain username - string rather than a FK to ``User``. Attribution is preserved unconditionally: deleting - or retiring a user does not affect existing audit records. This also avoids a dependency - on the ``User`` table from the audit log, keeping audit records fully independent from - live data. +#. **``actor_id`` is nullable.** Non-request contexts (management commands, background tasks) + record ``None``, logged as a system operation. ``actor_id`` is stored as a plain integer + (the database ID of the caller) rather than a FK to ``User``. This avoids a dependency + on the ``User`` table and keeps audit records fully independent from live data. Attribution + is preserved unconditionally: deleting or retiring a user does not affect existing records. #. **Audit records are independent from live authorization state.** Deleting a subject, scope, or role does not remove its audit history. Records may reference identifiers that diff --git a/openedx_authz/admin.py b/openedx_authz/admin.py index 56d578a9..ee23e1af 100644 --- a/openedx_authz/admin.py +++ b/openedx_authz/admin.py @@ -122,11 +122,11 @@ def queryset(self, request, queryset): class RoleAssignmentAuditAdmin(admin.ModelAdmin): """Read-only admin for the role assignment audit log.""" - list_display = ("operation", "display_subject", "display_role", "display_scope", "actor", "timestamp") + list_display = ("operation", "display_subject", "display_role", "display_scope", "actor_id", "timestamp") list_filter = ("operation", ScopeTypeFilter) search_fields = ("subject", "role", "scope") date_hierarchy = "timestamp" - readonly_fields = ("operation", "subject", "role", "scope", "actor", "timestamp") + readonly_fields = ("operation", "subject", "role", "scope", "actor_id", "timestamp") @admin.display(description="subject") def display_subject(self, obj): diff --git a/openedx_authz/api/roles.py b/openedx_authz/api/roles.py index 835ffe18..1c11b25f 100644 --- a/openedx_authz/api/roles.py +++ b/openedx_authz/api/roles.py @@ -242,7 +242,7 @@ def assign_role_to_subject_in_scope(subject: SubjectData, role: RoleData, scope: subject=subject.namespaced_key, role=role.namespaced_key, scope=scope.namespaced_key, - actor=getattr(get_current_user(), "username", None), + actor_id=getattr(get_current_user(), "id", None), ) ) ) @@ -293,7 +293,7 @@ def unassign_role_from_subject_in_scope(subject: SubjectData, role: RoleData, sc subject=subject.namespaced_key, role=role.namespaced_key, scope=scope.namespaced_key, - actor=getattr(get_current_user(), "username", None), + actor_id=getattr(get_current_user(), "id", None), ) ) ) diff --git a/openedx_authz/handlers.py b/openedx_authz/handlers.py index 3fff3dfb..af84ca73 100644 --- a/openedx_authz/handlers.py +++ b/openedx_authz/handlers.py @@ -331,7 +331,7 @@ def create_audit_record_on_role_assignment_change(sender, role_assignment, **kwa subject=role_assignment.subject, role=role_assignment.role, scope=role_assignment.scope, - actor=role_assignment.actor, + actor_id=role_assignment.actor_id, timestamp=kwargs["metadata"].time, ) except Exception as exc: # pylint: disable=broad-exception-caught diff --git a/openedx_authz/migrations/0008_roleassignmentaudit.py b/openedx_authz/migrations/0008_roleassignmentaudit.py index 4991f2d5..6466afbe 100644 --- a/openedx_authz/migrations/0008_roleassignmentaudit.py +++ b/openedx_authz/migrations/0008_roleassignmentaudit.py @@ -1,9 +1,10 @@ -# Generated by Django 4.2.24 on 2026-04-15 16:48 +# Generated by Django 4.2.24 on 2026-04-20 15:54 from django.db import migrations, models class Migration(migrations.Migration): + dependencies = [ ("openedx_authz", "0007_coursescope"), ] @@ -45,20 +46,15 @@ class Migration(migrations.Migration): ( "scope", models.CharField( - help_text=( - "Namespaced key of the scope (e.g. 'course-v1^course-v1:org+course+run') or glob pattern." - ), + help_text="Namespaced key of the scope (e.g. 'course-v1^course-v1:org+course+run') or glob pattern.", max_length=255, ), ), ( - "actor", - models.CharField( + "actor_id", + models.IntegerField( blank=True, - help_text=( - "Username of the user who performed the operation, or None for system-initiated actions." - ), - max_length=150, + help_text="Database ID of the user who performed the operation. Null for system-initiated actions.", null=True, ), ), diff --git a/openedx_authz/models/core.py b/openedx_authz/models/core.py index 68d01dc1..7a2584ef 100644 --- a/openedx_authz/models/core.py +++ b/openedx_authz/models/core.py @@ -285,11 +285,10 @@ class Operations(NamedTuple): max_length=255, help_text="Namespaced key of the scope (e.g. 'course-v1^course-v1:org+course+run') or glob pattern.", ) - actor = models.CharField( - max_length=150, + actor_id = models.IntegerField( null=True, blank=True, - help_text="Username of the user who performed the operation, or None for system-initiated actions.", + help_text="Database ID of the user who performed the operation. Null for system-initiated actions.", ) timestamp = models.DateTimeField() diff --git a/openedx_authz/tests/test_handlers.py b/openedx_authz/tests/test_handlers.py index 0f986f57..94c7d6d5 100644 --- a/openedx_authz/tests/test_handlers.py +++ b/openedx_authz/tests/test_handlers.py @@ -721,7 +721,7 @@ def test_creates_audit_record_for_created_operation(self): Expected result: - One RoleAssignmentAudit record exists with operation, subject, role, scope, - actor (None), and the event timestamp. + actor_id (None), and the event timestamp. """ role_assignment = RoleAssignmentData( operation=RoleAssignmentAudit.OPERATIONS.created, @@ -737,7 +737,7 @@ def test_creates_audit_record_for_created_operation(self): self.assertEqual(audit.subject, "user^john_doe") self.assertEqual(audit.role, "role^library_admin") self.assertEqual(audit.scope, "lib^org1:lib1") - self.assertIsNone(audit.actor) + self.assertIsNone(audit.actor_id) self.assertEqual(audit.timestamp, self.TIMESTAMP) def test_creates_audit_record_for_deleted_operation(self): From ad051b4c13ab056ce3880f694ff16bbcea8531de Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Tue, 21 Apr 2026 10:49:09 +0200 Subject: [PATCH 13/16] refactor: address quality issues --- openedx_authz/migrations/0008_roleassignmentaudit.py | 8 ++++++-- requirements/base.in | 1 + requirements/base.txt | 4 +++- requirements/dev.txt | 5 ----- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/openedx_authz/migrations/0008_roleassignmentaudit.py b/openedx_authz/migrations/0008_roleassignmentaudit.py index 6466afbe..add8f22e 100644 --- a/openedx_authz/migrations/0008_roleassignmentaudit.py +++ b/openedx_authz/migrations/0008_roleassignmentaudit.py @@ -46,7 +46,9 @@ class Migration(migrations.Migration): ( "scope", models.CharField( - help_text="Namespaced key of the scope (e.g. 'course-v1^course-v1:org+course+run') or glob pattern.", + help_text=( + "Namespaced key of the scope (e.g. 'course-v1^course-v1:org+course+run') or glob pattern." + ), max_length=255, ), ), @@ -54,7 +56,9 @@ class Migration(migrations.Migration): "actor_id", models.IntegerField( blank=True, - help_text="Database ID of the user who performed the operation. Null for system-initiated actions.", + help_text=( + "Database ID of the user who performed the operation. Null for system-initiated actions." + ), null=True, ), ), diff --git a/requirements/base.in b/requirements/base.in index 92a34492..0df86fc3 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -16,3 +16,4 @@ edx-organizations # Organizations library for Open edX django-waffle # Waffle library for feature flag management # TODO: pin to released version once MJG/authz-audit is merged and released git+https://github.com/openedx/openedx-events.git@MJG/authz-audit#egg=openedx-events +django-crum # Current Request User Middleware for Django diff --git a/requirements/base.txt b/requirements/base.txt index af13baaa..ec65dce5 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -44,7 +44,9 @@ django==5.2.13 # edx-organizations # openedx-events django-crum==0.7.9 - # via edx-django-utils + # via + # -r requirements/base.in + # edx-django-utils django-model-utils==5.0.0 # via edx-organizations django-simple-history==3.11.0 diff --git a/requirements/dev.txt b/requirements/dev.txt index 54a88fe5..11a3af96 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -213,11 +213,6 @@ mccabe==0.7.0 # pylint openedx-atlas==0.7.0 # via -r requirements/quality.txt -<<<<<<< HEAD -======= -openedx-events @ git+https://github.com/openedx/openedx-events.git@MJG/authz-audit - # via -r requirements/quality.txt ->>>>>>> f9b6043 (chore: run make upgrade after including openedx-events) packaging==26.1 # via # -r requirements/ci.txt From fa12139c7dbea2a8af36181b4748c8db11cd47ab Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Wed, 22 Apr 2026 19:43:07 +0200 Subject: [PATCH 14/16] chore: upgrade requirements to use openedx-events from pypi --- requirements/base.in | 3 +-- requirements/base.txt | 11 ++++------- requirements/dev.txt | 14 ++++++++++---- requirements/doc.txt | 8 +++++--- requirements/pip-tools.txt | 4 ++-- requirements/pip.txt | 2 +- requirements/quality.txt | 15 ++++++++------- requirements/test.txt | 12 +++++++++--- 8 files changed, 40 insertions(+), 29 deletions(-) diff --git a/requirements/base.in b/requirements/base.in index 0df86fc3..87a5de8a 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -14,6 +14,5 @@ edx-django-utils # Used for RequestCache edx-drf-extensions # Extensions for Django Rest Framework used by Open edX edx-organizations # Organizations library for Open edX django-waffle # Waffle library for feature flag management -# TODO: pin to released version once MJG/authz-audit is merged and released -git+https://github.com/openedx/openedx-events.git@MJG/authz-audit#egg=openedx-events +openedx-events # Open edX Events library for emitting events related to role assignment changes django-crum # Current Request User Middleware for Django diff --git a/requirements/base.txt b/requirements/base.txt index ec65dce5..4b0051f4 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -14,7 +14,7 @@ bracex==2.6 # via wcmatch casbin-django-orm-adapter==1.7.0 # via -r requirements/base.in -certifi==2026.2.25 +certifi==2026.4.22 # via requests cffi==2.0.0 # via @@ -22,7 +22,7 @@ cffi==2.0.0 # pynacl charset-normalizer==3.4.7 # via requests -click==8.3.2 +click==8.3.3 # via edx-django-utils cryptography==46.0.7 # via pyjwt @@ -94,18 +94,15 @@ edx-opaque-keys[django]==4.0.0 # openedx-events edx-organizations==8.0.0 # via -r requirements/base.in -idna==3.12 - # openedx-events fastavro==1.12.1 # via openedx-events -idna==3.11 +idna==3.13 # via requests inflection==0.5.1 # via drf-yasg openedx-atlas==0.7.0 # via -r requirements/base.in -packaging==26.1 -openedx-events @ git+https://github.com/openedx/openedx-events.git@MJG/authz-audit +openedx-events==11.2.0 # via -r requirements/base.in packaging==26.1 # via drf-yasg diff --git a/requirements/dev.txt b/requirements/dev.txt index 11a3af96..48777cee 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -31,7 +31,7 @@ cachetools==7.0.6 # tox casbin-django-orm-adapter==1.7.0 # via -r requirements/quality.txt -certifi==2026.2.25 +certifi==2026.4.22 # via # -r requirements/quality.txt # requests @@ -46,7 +46,7 @@ charset-normalizer==3.4.7 # via # -r requirements/quality.txt # requests -click==8.3.2 +click==8.3.3 # via # -r requirements/pip-tools.txt # -r requirements/quality.txt @@ -170,13 +170,17 @@ edx-opaque-keys[django]==4.0.0 # openedx-events edx-organizations==8.0.0 # via -r requirements/quality.txt +fastavro==1.12.1 + # via + # -r requirements/quality.txt + # openedx-events filelock==3.29.0 # via # -r requirements/ci.txt # python-discovery # tox # virtualenv -idna==3.12 +idna==3.13 # via # -r requirements/quality.txt # requests @@ -213,6 +217,8 @@ mccabe==0.7.0 # pylint openedx-atlas==0.7.0 # via -r requirements/quality.txt +openedx-events==11.2.0 + # via -r requirements/quality.txt packaging==26.1 # via # -r requirements/ci.txt @@ -406,7 +412,7 @@ wcmatch==10.1 # via # -r requirements/quality.txt # pycasbin -wheel==0.46.3 +wheel==0.47.0 # via # -r requirements/pip-tools.txt # pip-tools diff --git a/requirements/doc.txt b/requirements/doc.txt index 42be98c0..de6c5528 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -30,7 +30,7 @@ build==1.4.3 # via -r requirements/doc.in casbin-django-orm-adapter==1.7.0 # via -r requirements/test.txt -certifi==2026.2.25 +certifi==2026.4.22 # via # -r requirements/test.txt # requests @@ -43,7 +43,7 @@ charset-normalizer==3.4.7 # via # -r requirements/test.txt # requests -click==8.3.2 +click==8.3.3 # via # -r requirements/test.txt # code-annotations @@ -154,7 +154,7 @@ fastavro==1.12.1 # openedx-events id==1.6.1 # via twine -idna==3.12 +idna==3.13 # via # -r requirements/test.txt # requests @@ -201,6 +201,8 @@ nh3==0.3.4 # via readme-renderer openedx-atlas==0.7.0 # via -r requirements/test.txt +openedx-events==11.2.0 + # via -r requirements/test.txt packaging==26.1 # via # -r requirements/test.txt diff --git a/requirements/pip-tools.txt b/requirements/pip-tools.txt index ef46f514..d0bcefcd 100644 --- a/requirements/pip-tools.txt +++ b/requirements/pip-tools.txt @@ -6,7 +6,7 @@ # build==1.4.3 # via pip-tools -click==8.3.2 +click==8.3.3 # via pip-tools packaging==26.1 # via @@ -18,7 +18,7 @@ pyproject-hooks==1.2.0 # via # build # pip-tools -wheel==0.46.3 +wheel==0.47.0 # via pip-tools # The following packages are considered to be unsafe in a requirements file: diff --git a/requirements/pip.txt b/requirements/pip.txt index b76333d3..dfb1f6ab 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -6,7 +6,7 @@ # packaging==26.1 # via wheel -wheel==0.46.3 +wheel==0.47.0 # via -r requirements/pip.in # The following packages are considered to be unsafe in a requirements file: diff --git a/requirements/quality.txt b/requirements/quality.txt index 5ac32a20..35e3790a 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -22,7 +22,7 @@ bracex==2.6 # wcmatch casbin-django-orm-adapter==1.7.0 # via -r requirements/test.txt -certifi==2026.2.25 +certifi==2026.4.22 # via # -r requirements/test.txt # requests @@ -35,7 +35,7 @@ charset-normalizer==3.4.7 # via # -r requirements/test.txt # requests -click==8.3.2 +click==8.3.3 # via # -r requirements/test.txt # click-log @@ -140,7 +140,11 @@ edx-opaque-keys[django]==4.0.0 # openedx-events edx-organizations==8.0.0 # via -r requirements/test.txt -idna==3.12 +fastavro==1.12.1 + # via + # -r requirements/test.txt + # openedx-events +idna==3.13 # via # -r requirements/test.txt # requests @@ -166,11 +170,8 @@ mccabe==0.7.0 # via pylint openedx-atlas==0.7.0 # via -r requirements/test.txt -<<<<<<< HEAD -======= -openedx-events @ git+https://github.com/openedx/openedx-events.git@MJG/authz-audit +openedx-events==11.2.0 # via -r requirements/test.txt ->>>>>>> f9b6043 (chore: run make upgrade after including openedx-events) packaging==26.1 # via # -r requirements/test.txt diff --git a/requirements/test.txt b/requirements/test.txt index 1cf3a3c7..e882b130 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -18,7 +18,7 @@ bracex==2.6 # wcmatch casbin-django-orm-adapter==1.7.0 # via -r requirements/base.txt -certifi==2026.2.25 +certifi==2026.4.22 # via # -r requirements/base.txt # requests @@ -31,7 +31,7 @@ charset-normalizer==3.4.7 # via # -r requirements/base.txt # requests -click==8.3.2 +click==8.3.3 # via # -r requirements/base.txt # code-annotations @@ -123,7 +123,11 @@ edx-opaque-keys[django]==4.0.0 # openedx-events edx-organizations==8.0.0 # via -r requirements/base.txt -idna==3.12 +fastavro==1.12.1 + # via + # -r requirements/base.txt + # openedx-events +idna==3.13 # via # -r requirements/base.txt # requests @@ -139,6 +143,8 @@ markupsafe==3.0.3 # via jinja2 openedx-atlas==0.7.0 # via -r requirements/base.txt +openedx-events==11.2.0 + # via -r requirements/base.txt packaging==26.1 # via # -r requirements/base.txt From 1a79fa9c701f42f4111e7ac2d0e522e2ab32be35 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Wed, 22 Apr 2026 19:59:59 +0200 Subject: [PATCH 15/16] refactor: fix testing issues after rebase --- openedx_authz/handlers.py | 7 ++----- ...mentaudit.py => 0009_roleassignmentaudit.py} | 4 ++-- openedx_authz/tests/test_handlers.py | 17 +++++------------ 3 files changed, 9 insertions(+), 19 deletions(-) rename openedx_authz/migrations/{0008_roleassignmentaudit.py => 0009_roleassignmentaudit.py} (95%) diff --git a/openedx_authz/handlers.py b/openedx_authz/handlers.py index af84ca73..be6629c0 100644 --- a/openedx_authz/handlers.py +++ b/openedx_authz/handlers.py @@ -13,17 +13,14 @@ from django.conf import settings from django.db.models.signals import post_delete, post_save from django.dispatch import receiver -from waffle.models import Flag - from openedx_events.authz.signals import ROLE_ASSIGNMENT_CREATED, ROLE_ASSIGNMENT_DELETED +from waffle.models import Flag from openedx_authz.api.users import unassign_all_roles_from_user from openedx_authz.engine.utils import run_course_authoring_migration from openedx_authz.models.authz_migration import MigrationType, ScopeType -from openedx_authz.models.core import ExtendedCasbinRule -from openedx_authz.models.subjects import UserSubject -from openedx_authz.api.users import unassign_all_roles_from_user from openedx_authz.models.core import ExtendedCasbinRule, RoleAssignmentAudit +from openedx_authz.models.subjects import UserSubject try: from common.djangoapps.student.models import CourseAccessRole diff --git a/openedx_authz/migrations/0008_roleassignmentaudit.py b/openedx_authz/migrations/0009_roleassignmentaudit.py similarity index 95% rename from openedx_authz/migrations/0008_roleassignmentaudit.py rename to openedx_authz/migrations/0009_roleassignmentaudit.py index add8f22e..6680128d 100644 --- a/openedx_authz/migrations/0008_roleassignmentaudit.py +++ b/openedx_authz/migrations/0009_roleassignmentaudit.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.24 on 2026-04-20 15:54 +# Generated by Django 5.2.13 on 2026-04-22 17:58 from django.db import migrations, models @@ -6,7 +6,7 @@ class Migration(migrations.Migration): dependencies = [ - ("openedx_authz", "0007_coursescope"), + ("openedx_authz", "0008_authzcourseauthoringmigrationrun"), ] operations = [ diff --git a/openedx_authz/tests/test_handlers.py b/openedx_authz/tests/test_handlers.py index 94c7d6d5..88c5be99 100644 --- a/openedx_authz/tests/test_handlers.py +++ b/openedx_authz/tests/test_handlers.py @@ -7,21 +7,24 @@ waffle models are not imported in the test environment). """ +from datetime import datetime, timezone from types import SimpleNamespace -from unittest.mock import patch +from unittest.mock import MagicMock, patch from casbin_adapter.models import CasbinRule from ddt import data, ddt, unpack from django.test import TestCase, override_settings +from openedx_events.authz.data import RoleAssignmentData from openedx_authz.handlers import ( WAFFLE_OVERRIDE_FORCE_OFF, WAFFLE_OVERRIDE_FORCE_ON, + create_audit_record_on_role_assignment_change, get_migration_type, trigger_course_authoring_migration, ) from openedx_authz.models.authz_migration import MigrationType, ScopeType -from openedx_authz.models.core import ExtendedCasbinRule, Scope, Subject +from openedx_authz.models.core import ExtendedCasbinRule, RoleAssignmentAudit, Scope, Subject from openedx_authz.models.subjects import UserSubject from openedx_authz.tests.stubs.models import ( CourseAccessRole, @@ -29,16 +32,6 @@ WaffleFlagOrgOverrideModel, ) -from datetime import datetime, timezone -from unittest.mock import MagicMock, patch - -from casbin_adapter.models import CasbinRule -from django.test import TestCase -from openedx_events.authz.data import RoleAssignmentData - -from openedx_authz.handlers import create_audit_record_on_role_assignment_change -from openedx_authz.models.core import ExtendedCasbinRule, RoleAssignmentAudit, Scope, Subject - AUTHZ_COURSE_AUTHORING_FLAG_NAME = "authz.enable_course_authoring" OTHER_WAFFLE_FLAG_NAME = "some.other.flag" From 4dce6c12685c69ca64ce1302aa8153cfae36fa62 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Wed, 22 Apr 2026 20:10:16 +0200 Subject: [PATCH 16/16] chore: bump version --- CHANGELOG.rst | 10 ++++++++++ openedx_authz/__init__.py | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 297f6e93..5e71b4ca 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,16 @@ Change Log Unreleased ********** +1.13.0 - 2026-04-22 +******************* + +Added +===== + +* Add ``RoleAssignmentAudit`` model to record role assignment and removal events, including operation type, subject, role, scope, actor database ID, and timestamp. +* Emit ``ROLE_ASSIGNMENT_CREATED`` and ``ROLE_ASSIGNMENT_DELETED`` Open edX public signal events via ``transaction.on_commit`` after every successful role assignment or removal. +* Add Django admin for ``RoleAssignmentAudit`` with filters by operation type and scope type (course, content library), date hierarchy, and search by subject, role, and scope. + 1.12.0 - 2026-04-20 ******************* diff --git a/openedx_authz/__init__.py b/openedx_authz/__init__.py index e04c85ea..d8aafb1b 100644 --- a/openedx_authz/__init__.py +++ b/openedx_authz/__init__.py @@ -4,6 +4,6 @@ import os -__version__ = "1.12.0" +__version__ = "1.13.0" ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__))