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/docs/decisions/0012-auditability.rst b/docs/decisions/0012-auditability.rst new file mode 100644 index 00000000..f2d6cbe5 --- /dev/null +++ b/docs/decisions/0012-auditability.rst @@ -0,0 +1,213 @@ +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. + +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`_ 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. 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 +******** + +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) + +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 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. + +Event payload +------------- + +.. code:: python + + { + "operation": "created" | "deleted", + "subject": "", + "role": "", + "scope": "", + "actor_id": , + } + +The actor is resolved from ``django_crum.get_current_user()`` at API call time. No callers +need to pass ``actor_id=`` explicitly. + +Audit table +----------- + +``RoleAssignmentAudit`` mirrors the event payload. Registered in Django admin, filterable by +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``, +``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 +----------------------- + +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. + 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. + +``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 +************ + +#. **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_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 + 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 +*********************** + +``django-simple-history`` on ``ExtendedCasbinRule`` as the attribution audit trail +=================================================================================== + +Rejected for three reasons: + +- ``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. + +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/ +.. _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 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__)) diff --git a/openedx_authz/admin.py b/openedx_authz/admin.py index da343691..ee23e1af 100644 --- a/openedx_authz/admin.py +++ b/openedx_authz/admin.py @@ -7,7 +7,9 @@ from django.contrib import admin from django.utils.html import format_html +from openedx_authz.api.data import ContentLibraryData, CourseOverviewData from openedx_authz.models import AuthzCourseAuthoringMigrationRun, ExtendedCasbinRule +from openedx_authz.models.core import RoleAssignmentAudit def pretty_json(value) -> str: @@ -87,3 +89,68 @@ 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.for_scope_namespace(self.value()) + 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_id", "timestamp") + list_filter = ("operation", ScopeTypeFilter) + search_fields = ("subject", "role", "scope") + date_hierarchy = "timestamp" + readonly_fields = ("operation", "subject", "role", "scope", "actor_id", "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 bc7ca1e4..1c11b25f 100644 --- a/openedx_authz/api/roles.py +++ b/openedx_authz/api/roles.py @@ -10,7 +10,10 @@ from collections import defaultdict +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, @@ -24,6 +27,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_authz.models.core import RoleAssignmentAudit __all__ = [ "assign_role_to_subject_in_scope", @@ -195,7 +199,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 +233,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=RoleAssignmentAudit.OPERATIONS.created, + subject=subject.namespaced_key, + role=role.namespaced_key, + scope=scope.namespaced_key, + actor_id=getattr(get_current_user(), "id", None), + ) + ) + ) + # Invalidate policy cache to ensure changes are picked up AuthzEnforcer.invalidate_policy_cache() return True @@ -242,7 +264,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 +282,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=RoleAssignmentAudit.OPERATIONS.deleted, + subject=subject.namespaced_key, + role=role.namespaced_key, + scope=scope.namespaced_key, + actor_id=getattr(get_current_user(), "id", None), + ) + ) + ) + # Invalidate policy cache to ensure changes are picked up AuthzEnforcer.invalidate_policy_cache() return success 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/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/handlers.py b/openedx_authz/handlers.py index 755d485e..be6629c0 100644 --- a/openedx_authz/handlers.py +++ b/openedx_authz/handlers.py @@ -13,12 +13,13 @@ from django.conf import settings from django.db.models.signals import post_delete, post_save from django.dispatch import receiver +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.core import ExtendedCasbinRule, RoleAssignmentAudit from openedx_authz.models.subjects import UserSubject try: @@ -305,3 +306,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_id=role_assignment.actor_id, + 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/0009_roleassignmentaudit.py b/openedx_authz/migrations/0009_roleassignmentaudit.py new file mode 100644 index 00000000..6680128d --- /dev/null +++ b/openedx_authz/migrations/0009_roleassignmentaudit.py @@ -0,0 +1,77 @@ +# Generated by Django 5.2.13 on 2026-04-22 17:58 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("openedx_authz", "0008_authzcourseauthoringmigrationrun"), + ] + + 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( + 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_id", + models.IntegerField( + blank=True, + help_text=( + "Database ID of the user who performed the operation. Null for system-initiated actions." + ), + null=True, + ), + ), + ("timestamp", models.DateTimeField()), + ], + options={ + "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 a3208464..7a2584ef 100644 --- a/openedx_authz/models/core.py +++ b/openedx_authz/models/core.py @@ -5,10 +5,11 @@ schema that focuses on the core authorization logic. """ -from typing import ClassVar +from typing import ClassVar, NamedTuple from django.db import models, transaction +from openedx_authz.constants import AUTHZ_POLICY_ATTRIBUTES_SEPARATOR from openedx_authz.engine.filter import Filter @@ -231,3 +232,86 @@ 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. + + .. 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. + """ + + objects = RoleAssignmentAuditQuerySet.as_manager() + + 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_id = models.IntegerField( + null=True, + blank=True, + help_text="Database ID of the user who performed the operation. Null for system-initiated actions.", + ) + timestamp = models.DateTimeField() + + 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: + """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..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): diff --git a/openedx_authz/tests/test_handlers.py b/openedx_authz/tests/test_handlers.py index 81879f64..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, @@ -689,3 +692,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_id (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_id) + 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) diff --git a/requirements/base.in b/requirements/base.in index 11b452c6..87a5de8a 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 +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 1be06872..4b0051f4 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -7,12 +7,14 @@ 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 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 @@ -20,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 @@ -40,8 +42,11 @@ django==5.2.13 # edx-django-utils # edx-drf-extensions # 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 @@ -68,29 +73,37 @@ 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 + # via + # -r requirements/base.in + # 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]==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 +fastavro==1.12.1 + # via openedx-events +idna==3.13 # via requests inflection==0.5.1 # via drf-yasg openedx-atlas==0.7.0 # via -r requirements/base.in +openedx-events==11.2.0 + # via -r requirements/base.in packaging==26.1 # via drf-yasg pillow==12.2.0 diff --git a/requirements/dev.txt b/requirements/dev.txt index 19a8d456..48777cee 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 @@ -29,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 @@ -44,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 @@ -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,21 +161,26 @@ 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 +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 @@ -206,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 @@ -399,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 6b91f191..de6c5528 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 @@ -28,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 @@ -41,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 @@ -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,26 +127,34 @@ 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 +idna==3.13 # via # -r requirements/test.txt # requests @@ -190,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 91afebf2..35e3790a 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -13,14 +13,16 @@ 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 # 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 @@ -33,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 @@ -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,26 +117,34 @@ 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 +fastavro==1.12.1 + # via + # -r requirements/test.txt + # openedx-events +idna==3.13 # via # -r requirements/test.txt # requests @@ -159,6 +170,8 @@ mccabe==0.7.0 # via pylint 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/test.txt b/requirements/test.txt index f48ebd0c..e882b130 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -9,14 +9,16 @@ 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 # 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 @@ -29,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 @@ -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,24 +102,32 @@ 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 +fastavro==1.12.1 + # via + # -r requirements/base.txt + # openedx-events +idna==3.13 # via # -r requirements/base.txt # requests @@ -132,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