From 737f674f802461fb41c7634a6c9e981606db9f7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Wed, 23 Sep 2026 10:05:07 -0600 Subject: [PATCH 1/3] feat: add authz schema applier --- src/openedx_authz/engine/renderer.py | 608 +++++++++++++++++- .../tests/integration/test_schema_apply.py | 446 +++++++++++++ src/openedx_authz/tests/schema/test_apply.py | 582 +++++++++++++++++ .../tests/schema/test_renderer.py | 118 +++- .../tests/schema/test_source_storage.py | 343 ++++++++++ 5 files changed, 2080 insertions(+), 17 deletions(-) create mode 100644 src/openedx_authz/tests/integration/test_schema_apply.py create mode 100644 src/openedx_authz/tests/schema/test_apply.py create mode 100644 src/openedx_authz/tests/schema/test_source_storage.py diff --git a/src/openedx_authz/engine/renderer.py b/src/openedx_authz/engine/renderer.py index 96248a89..ed71c49d 100644 --- a/src/openedx_authz/engine/renderer.py +++ b/src/openedx_authz/engine/renderer.py @@ -1,27 +1,48 @@ -"""Render compiled definitions to Casbin rows (ADR 0018 §1, §5). +"""Render compiled definitions to Casbin rows and apply them (ADR 0018 §1, §5). -This is the Casbin-aware edge of the schema pipeline. It implements the -``render`` lifecycle step: +This is the only Casbin/Django-aware part of the schema pipeline. It implements +the ``render`` and ``apply`` lifecycle steps: * ``render`` builds the Casbin ``p`` rows for a :class:`CompiledSchema` in memory, without touching the database. +* ``apply`` persists the rows in a single transaction, while preserving data + owned by other services (ADR 0018 §3): dynamic roles, user assignments, and + the legacy ``g2`` action-inheritance rows that still live in ``authz.policy``. Key semantics: - * Definition rows only: ``render`` emits ``p`` rows and never ``g`` - (assignments) or ``g2`` (legacy action inheritance), so data owned by - other services is out of its reach by construction (ADR 0018 §3). - * Namespacing happens here, at the boundary: schema objects carry bare - identifiers, and the internal Casbin form (``role^``, ``act^``, - ``^*``) is applied on the way out. - * Deterministic output: rows are emitted in sorted order, so the rendered - set can be compared against a stored policy without spurious diffs. - -``render`` is pure — it imports nothing from Casbin or Django and performs no -database access. + * Idempotent (ADR 0018 §2): re-applying identical definitions changes + nothing and creates no duplicates. After a successful apply the stored + policy equals the compiled definition — no stale rows remain. + * Change report before write (ADR 0018 §6): :meth:`SchemaApplier.plan` + reports the ``p`` rows that will be added or removed by comparing rendered + output against the currently stored policy. + * Adoption, not duplication (ADR 0025 §6): a rendered row that already + exists as a ``p`` row gains definition and source records without being + rewritten, while a stored row no schema declares is left in place and + enforceable but unattributed. + * Removal is force-gated (ADR 0018 §6): a role slated for removal that still + has user assignments requires an explicit force option. Without force the + apply aborts before any write; with force the role's ``p`` rows and its + ``g`` assignment rows are removed together. + +:meth:`SchemaApplier.apply` reconciles the stored policy to the rendered set: +it adds missing rows, removes stale rows it owns, and prunes definition/source +records that the compiled schema no longer contains, all in one transaction. + +The definition/source model (ADR 0018 §3, ADR 0025) is the ownership record +that makes precise pruning safe, and it is consulted rather than assumed: +removal candidates are intersected with the stored role-permission grants (see +:meth:`SchemaApplier._managed_rows`), so unmanaged ``p`` rows, dynamic roles, +user assignments, and legacy ``g2`` action inheritance are all preserved. + +``render`` is pure and imports nothing from Casbin/Django. ``plan``/``apply`` +import the enforcer lazily so this module stays importable without a configured +Django environment. """ from __future__ import annotations +import logging from dataclasses import dataclass, field from openedx_authz.data import ( @@ -33,8 +54,11 @@ PolicyIndex, ) from openedx_authz.data import AUTHZ_POLICY_ATTRIBUTES_SEPARATOR as SEP +from openedx_authz.engine.schema.exceptions import SchemaApplyError from openedx_authz.engine.schema.types import CompiledSchema, RoleDefinition +logger = logging.getLogger(__name__) + @dataclass(frozen=True) class PolicyRow: @@ -93,6 +117,76 @@ class RenderedPolicy: rows: list[PolicyRow] = field(default_factory=list) +@dataclass +class DefinitionDiff: + """What would change for one kind of definition (ADR 0018 §6). + + ``updated`` covers metadata-only edits — a new display name or icon — which + change no policy row at all and would otherwise be invisible in the report. + """ + + added: list[str] = field(default_factory=list) + updated: list[str] = field(default_factory=list) + removed: list[str] = field(default_factory=list) + + @property + def is_empty(self) -> bool: + """True when this kind of definition is untouched.""" + return not (self.added or self.updated or self.removed) + + def __len__(self) -> int: + return len(self.added) + len(self.updated) + len(self.removed) + + +@dataclass +class ChangePlan: + """Diff between rendered definitions and what is currently stored. + + Presented to the operator before any write (ADR 0018 §6). Covers both the + Casbin ``p`` rows and the definition tables, because the apply step syncs + definitions even when no policy row changes. + """ + + added_rows: list[PolicyRow] = field(default_factory=list) + removed_rows: list[PolicyRow] = field(default_factory=list) + unchanged: bool = False + # (role_subject, assignment_subject) pairs: roles being removed that still + # have user assignments; block removal unless force is set. + blocking_assignments: list[tuple[str, str]] = field(default_factory=list) + # Definition-level changes, keyed by kind for reporting. + categories: DefinitionDiff = field(default_factory=DefinitionDiff) + permissions: DefinitionDiff = field(default_factory=DefinitionDiff) + roles: DefinitionDiff = field(default_factory=DefinitionDiff) + grants: DefinitionDiff = field(default_factory=DefinitionDiff) + + @property + def definition_diffs(self) -> list[tuple[str, DefinitionDiff]]: + """The definition diffs paired with their display label.""" + return [ + ("category", self.categories), + ("permission", self.permissions), + ("role", self.roles), + ("role-permission", self.grants), + ] + + @property + def definitions_unchanged(self) -> bool: + """True when no definition of any kind would change.""" + return all(diff.is_empty for _, diff in self.definition_diffs) + + +@dataclass +class ApplyResult: + """Outcome of an apply operation, for reporting.""" + + added: int = 0 + removed: int = 0 + unchanged: bool = False + # The change plan that was applied, so callers can report the same detailed + # policy-row and definition breakdown they would print for a dry run. + plan: ChangePlan | None = None + + class PolicyRenderer: """Turns a :class:`CompiledSchema` into Casbin ``p`` rows in memory.""" @@ -110,3 +204,489 @@ def render(self, schema: CompiledSchema) -> RenderedPolicy: for permission in sorted(role.permissions): rows.append(PolicyRow.from_grant(role.id, permission, scope)) return RenderedPolicy(rows=rows) + + +class SchemaApplier: + """Compares, then transactionally applies rendered policy to the database.""" + + def __init__(self, enforcer=None): + """Args: + enforcer: Casbin enforcer; defaults to ``AuthzEnforcer.get_enforcer()``. + + The default is resolved lazily inside methods (not at import) to respect + the plugin/settings timing constraint. + """ + self._enforcer = enforcer + + def plan(self, rendered: RenderedPolicy, schema: CompiledSchema | None = None) -> ChangePlan: + """Compute the change report without writing (ADR 0018 §6). + + Compares ``rendered`` against the currently stored ``p`` rows. Flags + roles that would be removed (their subject no longer appears in the + rendered set) that still have user assignments as blocking. + + When ``schema`` is given, the report also covers the definition tables. + Apply syncs definitions even when no policy row changes, so a + metadata-only edit is a real change the operator has to see — without it + the report would say "unchanged" and then rewrite display metadata. + """ + enforcer = self._resolve_enforcer() + + rendered_set = set(rendered.rows) + stored_set = {PolicyRow.from_policy(row) for row in enforcer.get_policy()} + managed_set = self._managed_rows() + + added = sorted(rendered_set - stored_set, key=self._row_sort_key) + # Only rows the loader recorded as its own may be pruned (ADR 0025 §6): + # a stored row that no schema declares stays in place and enforceable, + # unattributed. Intersecting with the managed set is what keeps the + # ownership boundary of ADR 0018 §3 real rather than aspirational. + removed = sorted((stored_set & managed_set) - rendered_set, key=self._row_sort_key) + + rendered_subjects = {row.subject for row in rendered_set} + removed_subjects = {row.subject for row in removed} - rendered_subjects + + blocking = self._find_blocking_assignments(enforcer, removed_subjects) + + definitions = self._diff_definitions(schema) if schema is not None else {} + + plan = ChangePlan( + added_rows=added, + removed_rows=removed, + blocking_assignments=blocking, + **definitions, + ) + plan.unchanged = not added and not removed and plan.definitions_unchanged + return plan + + def apply( + self, + rendered: RenderedPolicy, + schema: CompiledSchema, + *, + force: bool = False, + ) -> ApplyResult: + """Reconcile the stored policy to the rendered set in one transaction. + + Adds rendered rows not already present, removes stale schema-owned rows + no longer rendered, prunes definition/source records the compiled schema + no longer contains, and invalidates the policy cache so the enforcer + reloads. Preserves dynamic roles, user assignments, and ``g2`` rows. + + A role slated for removal that still has user assignments is blocking: + without ``force`` the apply aborts before any write; with ``force`` the + role's stale ``p`` rows and its ``g`` assignment rows are removed + together (ADR 0018 §6). + + If the write fails, the transaction rolls back and the policy cache is + invalidated so the enforcer reloads the last committed state rather than + keeping the uncommitted in-memory rows (ADR 0018 §5). + + Raises: + SchemaApplyError: If the plan has blocking assignments and ``force`` + is False. + """ + from django.db import transaction # pylint: disable=import-outside-toplevel + + from openedx_authz.engine.enforcer import AuthzEnforcer # pylint: disable=import-outside-toplevel + + plan = self.plan(rendered, schema) + + if plan.blocking_assignments and not force: + details = ", ".join(f"{role} (assigned to {subject})" for role, subject in plan.blocking_assignments) + raise SchemaApplyError( + "Refusing to proceed: static roles with existing assignments would be removed: " + f"{details}. Re-run with force to remove them together with their assignments." + ) + + enforcer = self._resolve_enforcer() + + # Reconcile p rows and sync definition/source records atomically. + # Definitions are synced even when p rows are unchanged so metadata-only + # edits land and pre-existing p rows get adopted on first run. + # + # add_policy/remove_policy mutate the enforcer's in-memory model as well + # as the database, so a rollback would otherwise leave this process + # enforcing rows the database no longer has. Bumping the policy cache + # version on the failure path forces a reload from the committed state, + # keeping Casbin on the last working version (ADR 0018 §5). + try: + with transaction.atomic(): + for row in plan.added_rows: + enforcer.add_policy(*row.as_policy()) + for row in plan.removed_rows: + enforcer.remove_policy(*row.as_policy()) + removed_assignments: list[tuple[str, str, str]] = [] + if force and plan.blocking_assignments: + removed_assignments = self._remove_assignments(enforcer, plan.blocking_assignments) + self._store_sources(schema) + # Emit the audit events only if the transaction commits, mirroring + # unassign_role_from_subject_in_scope, so no audit row is written for + # an assignment removal that gets rolled back. + if removed_assignments: + transaction.on_commit(lambda: self._emit_assignment_deleted(removed_assignments)) + except Exception: + # Runs outside the rolled-back block, so the new version commits. + AuthzEnforcer.invalidate_policy_cache() + logger.exception("Authz schema apply failed; policy cache invalidated to force a reload.") + raise + + changed = bool(plan.added_rows or plan.removed_rows) + if changed: + AuthzEnforcer.invalidate_policy_cache() + logger.info( + "Authz schema apply: added %d p row(s), removed %d p row(s).", + len(plan.added_rows), + len(plan.removed_rows), + ) + else: + logger.info("Authz schema apply: policy rows unchanged; definitions synced.") + + return ApplyResult( + added=len(plan.added_rows), + removed=len(plan.removed_rows), + unchanged=plan.unchanged, + plan=plan, + ) + + # ---- helpers ---------------------------------------------------------- + + def _resolve_enforcer(self): + """Lazily resolve the enforcer to honor plugin/settings timing.""" + if self._enforcer is None: + from openedx_authz.engine.enforcer import AuthzEnforcer # pylint: disable=import-outside-toplevel + + self._enforcer = AuthzEnforcer.get_enforcer() + return self._enforcer + + @staticmethod + def _remove_assignments(enforcer, blocking_assignments: list[tuple[str, str]]) -> list[tuple[str, str, str]]: + """Remove the ``g`` assignment rows for force-removed roles (ADR 0018 §6). + + ``blocking_assignments`` are ``(role_subject, assignment_subject)`` pairs + produced by :meth:`plan`. Each corresponds to a grouping row of the shape + ``[assignment_subject, role_subject, scope]``; the scope segment is + preserved by matching against the live grouping policy so we remove the + exact stored row rather than a reconstructed one. + + Returns the ``(subject, role, scope)`` triples that were removed so the + caller can emit a ``ROLE_ASSIGNMENT_DELETED`` audit event per removal. + """ + targets = set(blocking_assignments) + removed: list[tuple[str, str, str]] = [] + for grouping in list(enforcer.get_grouping_policy()): + if len(grouping) >= 2 and (grouping[1], grouping[0]) in targets: + enforcer.remove_grouping_policy(*grouping) + subject, role = grouping[0], grouping[1] + scope = grouping[2] if len(grouping) >= 3 else "" + removed.append((subject, role, scope)) + return removed + + @staticmethod + def _emit_assignment_deleted(removed_assignments: list[tuple[str, str, str]]) -> None: + """Emit ``ROLE_ASSIGNMENT_DELETED`` for each force-removed assignment. + + Every assignment change must leave an audit trail: the + ``create_audit_record_on_role_assignment_change`` handler turns each event + into a :class:`RoleAssignmentAudit` row, matching the audit behavior of + ``unassign_role_from_subject_in_scope``. Imported lazily so the module + stays importable without Django/openedx-events configured. + """ + if not removed_assignments: + return + + # pylint: disable=import-outside-toplevel + from crum import get_current_user + from openedx_events.authz.data import RoleAssignmentData as RoleAssignmentEventData + from openedx_events.authz.signals import ROLE_ASSIGNMENT_DELETED + + from openedx_authz.models.core import RoleAssignmentAudit + + actor_id = getattr(get_current_user(), "id", None) + for subject, role, scope in removed_assignments: + ROLE_ASSIGNMENT_DELETED.send_event( + role_assignment=RoleAssignmentEventData( + operation=RoleAssignmentAudit.OPERATIONS.deleted, + subject=subject, + role=role, + scope=scope, + actor_id=actor_id, + ) + ) + + @staticmethod + def _find_blocking_assignments(enforcer, removed_subjects: set[str]) -> list[tuple[str, str]]: + """Return (role_subject, assignment_subject) for removed roles still assigned. + + Grouping (``g``) rows have the shape ``[subject, role, scope]``; a role + being removed is blocking if any ``g`` row references it at index 1. + """ + if not removed_subjects: + return [] + blocking: list[tuple[str, str]] = [] + for grouping in enforcer.get_grouping_policy(): + if len(grouping) >= 2 and grouping[1] in removed_subjects: + blocking.append((grouping[1], grouping[0])) + return sorted(set(blocking)) + + @staticmethod + def _row_sort_key(row: PolicyRow) -> tuple[str, str, str, str]: + return (row.subject, row.action, row.scope, row.effect) + + def _diff_definitions(self, schema: CompiledSchema) -> dict[str, DefinitionDiff]: + """Diff the compiled definitions against the stored ones (ADR 0018 §6). + + Returns one :class:`DefinitionDiff` per kind, keyed by the + :class:`ChangePlan` field name. Grants carry no updatable fields, so + they only ever appear as added or removed. + """ + from openedx_authz.models import schema as m # pylint: disable=import-outside-toplevel + + return { + "categories": self._diff_kind( + {cid: compiled.definition for cid, compiled in schema.categories.items()}, + {obj.category_id: obj for obj in m.AuthzPermissionCategory.objects.all()}, + lambda definition, obj: ( + definition.display_name == obj.display_name + and (definition.description or "") == obj.description + and definition.icon == obj.icon + ), + ), + "permissions": self._diff_kind( + {pid: compiled.definition for pid, compiled in schema.permissions.items()}, + { + f"{obj.namespace}.{obj.name}": obj + for obj in m.AuthzPermissionDefinition.objects.select_related("category") + }, + lambda definition, obj: ( + definition.display_name == obj.display_name + and (definition.description or "") == obj.description + and definition.icon == obj.icon + and list(definition.scopes) == list(obj.scopes or []) + and definition.category_id == (obj.category.category_id if obj.category_id else "") + ), + ), + "roles": self._diff_kind( + {rid: compiled.definition for rid, compiled in schema.roles.items()}, + {obj.role_id: obj for obj in m.AuthzRoleDefinition.objects.all()}, + lambda definition, obj: ( + definition.display_name == obj.display_name + and (definition.description or "") == obj.description + and definition.icon == obj.icon + and list(definition.scopes) == list(obj.scopes or []) + and definition.hidden == obj.hidden + ), + ), + "grants": self._diff_kind( + {self._grant_key(rid, perm, scope): None for rid, perm, scope in self._compiled_grants(schema)}, + { + self._grant_key( + grant.role.role_id, f"{grant.permission.namespace}.{grant.permission.name}", grant.scope + ): None + for grant in m.AuthzRolePermission.objects.select_related("role", "permission") + }, + lambda _compiled, _stored: True, + ), + } + + @staticmethod + def _compiled_grants(schema: CompiledSchema): + """Yield every ``(role_id, permission_id, scope)`` the schema grants.""" + for role_id, compiled in schema.roles.items(): + definition = compiled.definition + for scope in definition.scopes: + for permission_id in definition.permissions: + yield role_id, permission_id, scope + + @staticmethod + def _grant_key(role_id: str, permission_id: str, scope: str) -> str: + return f"{role_id} -> {permission_id} @ {scope}" + + @staticmethod + def _diff_kind(compiled: dict, stored: dict, matches) -> DefinitionDiff: + """Split keys into added/updated/removed using ``matches`` for equality.""" + compiled_keys, stored_keys = set(compiled), set(stored) + return DefinitionDiff( + added=sorted(compiled_keys - stored_keys), + updated=sorted(key for key in compiled_keys & stored_keys if not matches(compiled[key], stored[key])), + removed=sorted(stored_keys - compiled_keys), + ) + + @staticmethod + def _managed_rows() -> set[PolicyRow]: + """Return the ``p`` rows the loader previously recorded as schema-owned. + + Ownership lives in the definition tables (ADR 0025 §1): every stored + role-permission grant corresponds one-to-one with a rendered ``p`` row. + Anything outside this set was not produced by the loader — a row from + the legacy policy file, an administrative fix (ADR 0018 §7), or a role + owned by another service — and is therefore not ours to remove. + + Empty on a first deployment, which is what makes adoption safe: nothing + is pruned before the loader has recorded what it owns. + """ + from openedx_authz.models import schema as m # pylint: disable=import-outside-toplevel + + return { + PolicyRow.from_grant( + grant.role.role_id, + f"{grant.permission.namespace}.{grant.permission.name}", + grant.scope, + ) + for grant in m.AuthzRolePermission.objects.select_related("role", "permission") + } + + def _store_sources(self, schema: CompiledSchema) -> None: + """Persist compiled definitions and their sources (ADR 0025). + + Upserts categories, permissions, roles, and each ``(role, permission, + scope)`` grant, linking every definition and grant to its contributing + sources, then prunes any definition rows the compiled schema no longer + contains (see :meth:`_prune_definitions`). Idempotent: re-applying an + identical schema is a no-op. Pre-existing ``p`` rows are adopted because + grants are upserted for every rendered triple regardless of prior + ``p``-row existence. + + Called inside the ``apply`` transaction. + """ + from openedx_authz.models import schema as m # pylint: disable=import-outside-toplevel + + source_cache: dict[tuple[str, str], object] = {} + + def source_obj(record): + key = (record.distribution, record.module) + cached = source_cache.get(key) + if cached is not None: + return cached + obj, _ = m.AuthzSchemaSource.objects.update_or_create( + distribution=record.distribution, + module=record.module, + defaults={ + "distribution_version": record.distribution_version, + "resource_path": record.resource_path, + "content_digest": record.content_digest, + "schema_version": record.schema_version, + }, + ) + source_cache[key] = obj + return obj + + # Categories. + category_objs: dict[str, object] = {} + for cid, compiled in schema.categories.items(): + definition = compiled.definition + obj, _ = m.AuthzPermissionCategory.objects.update_or_create( + category_id=definition.id, + defaults={ + "display_name": definition.display_name, + "description": definition.description or "", + "icon": definition.icon, + }, + ) + category_objs[cid] = obj + for record in compiled.sources: + m.AuthzCategorySource.objects.update_or_create( + category=obj, source=source_obj(record), defaults={"origin_kind": m.OriginKind.BASE} + ) + + # Permissions. + permission_objs: dict[str, object] = {} + for pid, compiled in schema.permissions.items(): + definition = compiled.definition + obj, _ = m.AuthzPermissionDefinition.objects.update_or_create( + namespace=definition.namespace, + name=definition.name, + defaults={ + "display_name": definition.display_name, + "description": definition.description or "", + "category": category_objs.get(definition.category_id), + "scopes": list(definition.scopes), + "icon": definition.icon, + }, + ) + permission_objs[pid] = obj + for record in compiled.sources: + m.AuthzPermissionSource.objects.update_or_create( + permission=obj, source=source_obj(record), defaults={"origin_kind": m.OriginKind.BASE} + ) + + # Roles. + role_objs: dict[str, object] = {} + for rid, compiled in schema.roles.items(): + definition = compiled.definition + obj, _ = m.AuthzRoleDefinition.objects.update_or_create( + role_id=definition.id, + defaults={ + "display_name": definition.display_name, + "description": definition.description or "", + "scopes": list(definition.scopes), + "icon": definition.icon, + "hidden": definition.hidden, + }, + ) + role_objs[rid] = obj + for record in compiled.sources: + m.AuthzRoleSource.objects.update_or_create( + role=obj, source=source_obj(record), defaults={"origin_kind": m.OriginKind.BASE} + ) + + # Role-permission grants (one per rendered role/permission/scope triple). + # Track the grant keys the schema still contains so stale grants can be + # pruned below. + live_grant_ids: set[int] = set() + for rid, compiled in schema.roles.items(): + role_obj = role_objs[rid] + definition = compiled.definition + for scope in definition.scopes: + for perm_id in definition.permissions: + permission_obj = permission_objs.get(perm_id) + if permission_obj is None: + continue # validated away in practice; skip defensively + grant, _ = m.AuthzRolePermission.objects.update_or_create( + role=role_obj, permission=permission_obj, scope=scope + ) + live_grant_ids.add(grant.pk) + for rel in schema.role_permission_sources.get((rid, perm_id), []): + m.AuthzRolePermissionSource.objects.update_or_create( + role_permission=grant, + source=source_obj(rel.source), + defaults={"origin_kind": rel.origin_kind, "priority": rel.priority}, + ) + + self._prune_definitions(m, schema, live_grant_ids) + + logger.info( + "Authz schema apply: persisted %d role(s), %d permission(s), %d category(ies).", + len(schema.roles), + len(schema.permissions), + len(schema.categories), + ) + + @staticmethod + def _prune_definitions(m, schema: CompiledSchema, live_grant_ids: set[int]) -> None: + """Delete definition/source rows the compiled schema no longer contains. + + Removes stale role-permission grants, roles, permissions, and categories + so the definition tables match the compiled schema (ADR 0018 §2). Source + link rows and per-source records cascade via their foreign keys; the + shared :class:`AuthzSchemaSource` rows are left in place because they may + still back other definitions and carry no access on their own. + + Ordering matters: grants first (they reference roles and permissions), + then roles and permissions, then categories. + """ + # Stale role-permission grants: any grant not re-created this run. + m.AuthzRolePermission.objects.exclude(pk__in=live_grant_ids).delete() + + live_role_ids = {compiled.definition.id for compiled in schema.roles.values()} + m.AuthzRoleDefinition.objects.exclude(role_id__in=live_role_ids).delete() + + live_permission_keys = { + (compiled.definition.namespace, compiled.definition.name) for compiled in schema.permissions.values() + } + for permission_obj in m.AuthzPermissionDefinition.objects.all(): + if (permission_obj.namespace, permission_obj.name) not in live_permission_keys: + permission_obj.delete() + + live_category_ids = {compiled.definition.id for compiled in schema.categories.values()} + m.AuthzPermissionCategory.objects.exclude(category_id__in=live_category_ids).delete() diff --git a/src/openedx_authz/tests/integration/test_schema_apply.py b/src/openedx_authz/tests/integration/test_schema_apply.py new file mode 100644 index 00000000..b06c607e --- /dev/null +++ b/src/openedx_authz/tests/integration/test_schema_apply.py @@ -0,0 +1,446 @@ +"""End-to-end integration tests for schema apply pruning (ADR 0018 §2, §5, §6). + +Unlike the unit tests in ``tests/schema/test_apply.py`` (which use a fake +enforcer and stub persistence), these exercise the *real* stack: + +* the shared Casbin :class:`~openedx_authz.engine.enforcer.AuthzEnforcer` + (DB-backed adapter, production matcher), and +* the real definition/source ORM tables. + +They prove the reconciliation contract from end to end: after a schema removes a +permission or a role, re-applying prunes the stale Casbin ``p`` rows and the +definition rows so the stored schema follows the compiled definition, and +force-removal of an assigned role removes its ``g`` assignment rows too. + +The tests assert at the behavioral level with ``enforce()`` (a permission +removal flips a real allow into a deny; force-removal revokes access) and back +that up with the stored ``p``/``g`` rows and the definition tables. They still +require no populated platform data: the staff/superuser matcher returns ``False`` +for an unknown user (so no ``auth_user`` row is needed and access comes purely +from the role assignment), and scope matching is in-memory (no +``course_overviews_courseoverview`` lookup). Assignments are seeded with the +low-level grouping API to avoid the assignment audit/signal machinery. + +Database setup: the integration ``conftest`` makes ``django_db_setup`` a no-op so +its other tests reuse an externally provisioned database. This module instead +restores a real setup that builds tables **directly from the models +(``run_syncdb``) with migrations disabled**. Running edx-platform migrations on +the sqlite test DB fails (some platform migrations introspect tables at import +time, e.g. ``course_overviews.0009_readd_facebook_url``), which is why the +platform itself runs tests with ``--nomigrations``. Building from models +sidesteps that and still creates every table these tests touch. + +Run these in an edx-platform environment (e.g. tutor):: + + pytest -p no:randomly --create-db --ds=cms.envs.test \\ + /mnt/openedx-authz/openedx_authz/tests/integration/test_schema_apply.py +""" + +from __future__ import annotations + +from unittest import mock + +import pytest +from django.db import IntegrityError +from django.test import TestCase + +from openedx_authz.engine.enforcer import AuthzEnforcer +from openedx_authz.engine.renderer import PolicyRenderer, SchemaApplier +from openedx_authz.engine.schema.compilation import SchemaCompiler +from openedx_authz.engine.schema.exceptions import SchemaApplyError +from openedx_authz.engine.schema.types import ( + PermissionCategory, + PermissionDefinition, + RoleDefinition, + RoleExtension, + SchemaDocument, + SourceRecord, +) +from openedx_authz.models.core import RoleAssignmentAudit +from openedx_authz.models.schema import AuthzRoleDefinition, AuthzRolePermission + + +@pytest.fixture(scope="session") +def django_db_setup(request, django_test_environment, django_db_blocker): # pylint: disable=unused-argument + """Build the test database from models, with migrations disabled. + + Overrides both pytest-django's default (which would run migrations) and the + integration ``conftest`` no-op (which would build nothing). Migrations are + disabled because some edx-platform migrations fail on the sqlite test DB by + introspecting tables at import time; ``run_syncdb`` creates the tables from + the installed models instead, which is enough for these tests. + """ + from django.test.utils import setup_databases, teardown_databases # pylint: disable=import-outside-toplevel + from pytest_django.fixtures import _disable_migrations # pylint: disable=import-outside-toplevel + + _disable_migrations() + with django_db_blocker.unblock(): + db_cfg = setup_databases(verbosity=request.config.option.verbose, interactive=False) + yield + with django_db_blocker.unblock(): + teardown_databases(db_cfg, verbosity=request.config.option.verbose) + + +SCOPE_NAMESPACE = "course-v1" +COURSE_SCOPE = "course-v1^course-v1:OpenedX+DemoX+DemoCourse" +USER_SUBJECT = "user^schema_apply_alice" +ROLE_SUBJECT = "role^schema_apply_editor" +VIEW_ACTION = "act^courses.view_course" +TAGS_ACTION = "act^courses.manage_tags" + + +def _source(name: str) -> SourceRecord: + return SourceRecord( + distribution="openedx-authz", + distribution_version="0.0.0", + module=f"openedx_authz.tests.{name}", + resource_path=f"{name}.authz.yaml", + schema_version="1.0", + content_digest=f"digest-{name}", + ) + + +def _document(name="core", *, priority=100, roles=(), extensions=()): + return SchemaDocument( + source=_source(name), + priority=priority, + categories=[PermissionCategory(id="cat", display_name="Cat", description="d")], + permissions=[ + PermissionDefinition( + namespace="courses", + name="view_course", + display_name="View", + description="d", + category_id="cat", + scopes=(SCOPE_NAMESPACE,), + ), + PermissionDefinition( + namespace="courses", + name="manage_tags", + display_name="Tags", + description="d", + category_id="cat", + scopes=(SCOPE_NAMESPACE,), + ), + ], + roles=list(roles), + role_extensions=list(extensions), + ) + + +def _editor_role(permissions): + return RoleDefinition( + id="schema_apply_editor", + display_name="Editor", + description="d", + scopes=(SCOPE_NAMESPACE,), + permissions=tuple(permissions), + ) + + +class SchemaApplyIntegrationBase(TestCase): + """Shared setup and helpers for the real-stack apply tests. + + Holds no test methods; the concrete cases below inherit the clean-policy + setup and the ``p``/``g`` inspection helpers. + """ + + def setUp(self): + """Start each test from a clean policy and a known enforcer instance.""" + self.enforcer = AuthzEnforcer.get_enforcer() + self.enforcer.clear_policy() + self.applier = SchemaApplier() + + def tearDown(self): + """Leave no policy behind for other integration tests.""" + self.enforcer.clear_policy() + + # -- helpers ------------------------------------------------------------ + + def _apply(self, *documents, force=False): + """Compile, render, and apply ``documents``, then reload the enforcer.""" + schema = SchemaCompiler().compile(list(documents)) + rendered = PolicyRenderer().render(schema) + result = self.applier.apply(rendered, schema, force=force) + self.enforcer.load_policy() + return result + + def _p_rows_for_role(self): + """Return the stored ``p`` rows whose subject is the test role.""" + return [row for row in self.enforcer.get_policy() if row[0] == ROLE_SUBJECT] + + def _grouping_for_role(self): + """Return the stored ``g`` (assignment) rows referencing the test role.""" + return [g for g in self.enforcer.get_grouping_policy() if len(g) >= 2 and g[1] == ROLE_SUBJECT] + + def _assign_user_to_role(self): + """Add a raw ``g`` assignment row for the test role. + + Uses the low-level grouping API rather than the public role-assignment + API so the test doesn't depend on the assignment audit/signal machinery. + The staff/superuser matcher returns ``False`` for an unknown user (no + ``User`` row required), so enforcement decisions come purely from this + role assignment. + """ + self.enforcer.add_grouping_policy(USER_SUBJECT, ROLE_SUBJECT, COURSE_SCOPE) + self.enforcer.load_policy() + + +class TestSchemaApplyPruningIntegration(SchemaApplyIntegrationBase): + """Real enforcer + real DB reconciliation across successive applies.""" + + def test_first_apply_persists_rows_and_definitions(self): + """A first apply writes p rows and definition tables together.""" + result = self._apply(_document(roles=[_editor_role(("courses.view_course", "courses.manage_tags"))])) + + self.assertEqual(result.added, 2) + self.assertEqual(result.removed, 0) + self.assertEqual(len(self._p_rows_for_role()), 2) + + editor = AuthzRoleDefinition.objects.get(role_id="schema_apply_editor") + self.assertEqual(editor.role_permissions.count(), 2) + + def test_reapply_identical_schema_is_idempotent(self): + """Re-applying the same schema changes nothing (ADR 0018 §2).""" + doc = _document(roles=[_editor_role(("courses.view_course", "courses.manage_tags"))]) + self._apply(doc) + + result = self._apply(doc) + + self.assertEqual(result.added, 0) + self.assertEqual(result.removed, 0) + self.assertTrue(result.unchanged) + self.assertEqual(len(self._p_rows_for_role()), 2) + + def test_removed_permission_prunes_p_row_and_flips_enforcement(self): + """Dropping a permission via extension flips the live enforcement result. + + This is the core §2 guarantee, checked at the behavioral level: a user + assigned the role is *allowed* ``manage_tags`` before the removal and + *denied* it afterwards, while the untouched ``view_course`` stays + allowed. Stored ``p`` rows and the definition tables are checked too, so + a regression that left enforcement drifting on a stale row would fail + here. + """ + base = _document(roles=[_editor_role(("courses.view_course", "courses.manage_tags"))]) + self._apply(base) + self._assign_user_to_role() + + # Before removal: both actions enforce as allowed via the role. + self.assertTrue(self.enforcer.enforce(USER_SUBJECT, TAGS_ACTION, COURSE_SCOPE)) + self.assertTrue(self.enforcer.enforce(USER_SUBJECT, VIEW_ACTION, COURSE_SCOPE)) + + # Remove manage_tags from the role via a higher-priority extension. + extension_doc = _document( + "modx", + priority=200, + roles=[], + extensions=[RoleExtension(role_id="schema_apply_editor", remove_permissions=("courses.manage_tags",))], + ) + result = self._apply(base, extension_doc) + + self.assertEqual(result.removed, 1) + + # After removal: manage_tags is denied, view_course still allowed. + self.assertFalse(self.enforcer.enforce(USER_SUBJECT, TAGS_ACTION, COURSE_SCOPE)) + self.assertTrue(self.enforcer.enforce(USER_SUBJECT, VIEW_ACTION, COURSE_SCOPE)) + + # The stale p row is gone from the stored policy... + stored = self.enforcer.get_policy() + self.assertNotIn([ROLE_SUBJECT, TAGS_ACTION, "course-v1^*", "allow"], stored) + self.assertIn([ROLE_SUBJECT, VIEW_ACTION, "course-v1^*", "allow"], stored) + + # ...and from the definition tables. + editor = AuthzRoleDefinition.objects.get(role_id="schema_apply_editor") + self.assertEqual(editor.role_permissions.count(), 1) + self.assertFalse( + AuthzRolePermission.objects.filter( + role=editor, permission__namespace="courses", permission__name="manage_tags" + ).exists() + ) + + def test_removed_role_without_assignments_is_pruned(self): + """A role no longer in the schema is removed when nothing is assigned.""" + self._apply(_document(roles=[_editor_role(("courses.view_course",))])) + self.assertTrue(AuthzRoleDefinition.objects.filter(role_id="schema_apply_editor").exists()) + + # Apply a schema without the role at all. + result = self._apply(_document(roles=[])) + + self.assertEqual(result.removed, 1) + self.assertEqual(self._p_rows_for_role(), []) + self.assertFalse(AuthzRoleDefinition.objects.filter(role_id="schema_apply_editor").exists()) + + def test_removing_assigned_role_requires_force(self): + """Removing a role with a live assignment aborts without force (§6).""" + self._apply(_document(roles=[_editor_role(("courses.view_course",))])) + self._assign_user_to_role() + + with self.assertRaises(SchemaApplyError): + self._apply(_document(roles=[]), force=False) + + # Nothing was pruned: the p row, the assignment, and the definition are + # intact, and the user still enforces as allowed. + self.assertEqual(len(self._p_rows_for_role()), 1) + self.assertEqual(len(self._grouping_for_role()), 1) + self.assertTrue(AuthzRoleDefinition.objects.filter(role_id="schema_apply_editor").exists()) + self.assertTrue(self.enforcer.enforce(USER_SUBJECT, VIEW_ACTION, COURSE_SCOPE)) + + def test_force_removes_assigned_role_and_its_assignment(self): + """With force, a removed role loses its p rows, g assignment, and access (§6).""" + self._apply(_document(roles=[_editor_role(("courses.view_course",))])) + self._assign_user_to_role() + self.assertEqual(len(self._p_rows_for_role()), 1) + self.assertEqual(len(self._grouping_for_role()), 1) + self.assertTrue(self.enforcer.enforce(USER_SUBJECT, VIEW_ACTION, COURSE_SCOPE)) + + # Run on_commit hooks so the ROLE_ASSIGNMENT_DELETED audit event fires. + with self.captureOnCommitCallbacks(execute=True): + result = self._apply(_document(roles=[]), force=True) + + self.assertEqual(result.removed, 1) + # Access is revoked, and both the p rows and the g assignment are gone. + self.assertFalse(self.enforcer.enforce(USER_SUBJECT, VIEW_ACTION, COURSE_SCOPE)) + self.assertEqual(self._p_rows_for_role(), []) + self.assertEqual(self._grouping_for_role(), []) + self.assertFalse(AuthzRoleDefinition.objects.filter(role_id="schema_apply_editor").exists()) + + # Every assignment change leaves an audit trail: the force removal emits + # ROLE_ASSIGNMENT_DELETED, which is recorded as a 'deleted' audit row. + audit = RoleAssignmentAudit.objects.filter(subject=USER_SUBJECT, role=ROLE_SUBJECT, scope=COURSE_SCOPE) + self.assertEqual(audit.count(), 1) + self.assertEqual(audit.get().operation, RoleAssignmentAudit.OPERATIONS.deleted) + + +class TestSchemaApplyAdoptionIntegration(SchemaApplyIntegrationBase): + """Pre-existing policy rows are adopted, unmanaged rows are left alone. + + ADR 0025 §6: a rendered ``(role, permission, scope)`` that already exists as + a ``p`` row gains definition and source records instead of being rewritten, + while a stored row no schema declares stays in place and enforceable but + unattributed. This is the realistic first deployment, where ``load_policies`` + has already written the ``p`` rows and the definition tables are empty. + """ + + UNMANAGED_ROLE = "role^schema_apply_legacy" + + def _seed_rendered_rows(self, *documents): + """Write the rendered rows straight to the policy, bypassing apply.""" + schema = SchemaCompiler().compile(list(documents)) + for row in PolicyRenderer().render(schema).rows: + self.enforcer.add_policy(*row.as_policy()) + self.enforcer.load_policy() + + def test_preexisting_rows_are_adopted_not_duplicated(self): + """Pre-existing rendered rows are adopted (gain definitions), not duplicated.""" + document = _document(roles=[_editor_role(("courses.view_course", "courses.manage_tags"))]) + self._seed_rendered_rows(document) + self.assertEqual(len(self._p_rows_for_role()), 2) + self.assertFalse(AuthzRoleDefinition.objects.filter(role_id="schema_apply_editor").exists()) + + result = self._apply(document) + + # Nothing to write to the policy, yet the definitions now exist — so the + # run is reported as a change even though no p row moved. + self.assertEqual(result.added, 0) + self.assertEqual(result.removed, 0) + self.assertFalse(result.unchanged) + self.assertEqual(len(self._p_rows_for_role()), 2) + editor = AuthzRoleDefinition.objects.get(role_id="schema_apply_editor") + self.assertEqual(editor.role_permissions.count(), 2) + + def test_adopted_rows_keep_enforcing(self): + """Adoption must not interrupt access that already worked.""" + document = _document(roles=[_editor_role(("courses.view_course",))]) + self._seed_rendered_rows(document) + self._assign_user_to_role() + self.assertTrue(self.enforcer.enforce(USER_SUBJECT, VIEW_ACTION, COURSE_SCOPE)) + + self._apply(document) + + self.assertTrue(self.enforcer.enforce(USER_SUBJECT, VIEW_ACTION, COURSE_SCOPE)) + + def test_adopted_grant_records_the_contributing_source(self): + """An adopted grant records its contributing source module.""" + document = _document(roles=[_editor_role(("courses.view_course",))]) + self._seed_rendered_rows(document) + + self._apply(document) + + grant = AuthzRolePermission.objects.get(role__role_id="schema_apply_editor") + self.assertEqual([source.module for source in grant.sources.all()], ["openedx_authz.tests.core"]) + + def test_unmanaged_row_is_preserved_and_still_enforces(self): + """A row no schema declares is not the loader's to remove.""" + self.enforcer.add_policy(self.UNMANAGED_ROLE, VIEW_ACTION, "course-v1^*", "allow") + self.enforcer.add_grouping_policy(USER_SUBJECT, self.UNMANAGED_ROLE, COURSE_SCOPE) + self.enforcer.load_policy() + + result = self._apply(_document(roles=[_editor_role(("courses.view_course",))])) + + self.assertEqual(result.removed, 0) + self.assertIn([self.UNMANAGED_ROLE, VIEW_ACTION, "course-v1^*", "allow"], self.enforcer.get_policy()) + self.assertTrue(self.enforcer.enforce(USER_SUBJECT, VIEW_ACTION, COURSE_SCOPE)) + # ...and it is not attributed to any schema source. + self.assertFalse(AuthzRoleDefinition.objects.filter(role_id="schema_apply_legacy").exists()) + + def test_unmanaged_row_survives_an_empty_schema(self): + """An unmanaged row is preserved even when the applied schema is empty.""" + self.enforcer.add_policy(self.UNMANAGED_ROLE, VIEW_ACTION, "course-v1^*", "allow") + self.enforcer.load_policy() + + result = self._apply(_document(roles=[])) + + self.assertEqual(result.removed, 0) + self.assertIn([self.UNMANAGED_ROLE, VIEW_ACTION, "course-v1^*", "allow"], self.enforcer.get_policy()) + + +class TestSchemaApplyFailureIntegration(SchemaApplyIntegrationBase): + """A failed apply leaves Casbin on the last committed state (ADR 0018 §5). + + ``add_policy`` writes through to the adapter *and* mutates the enforcer's + in-memory model, so a rollback would otherwise leave this process enforcing + rows the database never committed. + """ + + def _fail_during_store(self): + """Make ``_store_sources`` write a row and then fail, like a DB error would.""" + + def _store_then_fail(_self, _schema): + AuthzRoleDefinition.objects.create( + role_id="schema_apply_half_written", + display_name="Half written", + description="", + scopes=[SCOPE_NAMESPACE], + hidden=False, + ) + raise IntegrityError("simulated write failure") + + patcher = mock.patch.object(SchemaApplier, "_store_sources", _store_then_fail) + patcher.start() + self.addCleanup(patcher.stop) + + def test_failed_apply_rolls_back_every_definition_write(self): + """A failed apply rolls back every definition row it had started writing.""" + self._fail_during_store() + + with self.assertRaises(IntegrityError): + self._apply(_document(roles=[_editor_role(("courses.view_course",))])) + + self.assertFalse(AuthzRoleDefinition.objects.filter(role_id="schema_apply_half_written").exists()) + self.assertFalse(AuthzRoleDefinition.objects.filter(role_id="schema_apply_editor").exists()) + + def test_failed_apply_leaves_enforcement_on_the_committed_state(self): + """The reload triggered by cache invalidation drops the uncommitted rows.""" + self._assign_user_to_role() + self.assertFalse(self.enforcer.enforce(USER_SUBJECT, VIEW_ACTION, COURSE_SCOPE)) + self._fail_during_store() + + with self.assertRaises(IntegrityError): + self._apply(_document(roles=[_editor_role(("courses.view_course",))])) + + # The apply invalidated the policy cache, so acquiring the enforcer + # reloads from the database rather than trusting the in-memory model. + reloaded = AuthzEnforcer.get_enforcer() + self.assertEqual([row for row in reloaded.get_policy() if row[0] == ROLE_SUBJECT], []) + self.assertFalse(reloaded.enforce(USER_SUBJECT, VIEW_ACTION, COURSE_SCOPE)) diff --git a/src/openedx_authz/tests/schema/test_apply.py b/src/openedx_authz/tests/schema/test_apply.py new file mode 100644 index 00000000..f9117536 --- /dev/null +++ b/src/openedx_authz/tests/schema/test_apply.py @@ -0,0 +1,582 @@ +"""Tests for the apply/plan reconciliation path (ADR 0018 §2, §5, §6). + +Three layers are exercised: + +* A fake in-memory enforcer drives the ``p``/``g`` row reconciliation logic + (add, remove, force-gated assignment removal, idempotency) against real + definition tables, because pruning is driven by the recorded ownership rather + than by a raw policy diff (ADR 0025 §6). +* Failure handling: a rolled-back apply must not leave the enforcer's in-memory + model ahead of the database (ADR 0018 §5). +* A Django ``TestCase`` covers definition/source pruning through + :meth:`SchemaApplier._store_sources`, confirming the definition tables track + the compiled schema across successive applies. +""" + +from __future__ import annotations + +from unittest import mock + +import pytest +from django.db import IntegrityError +from django.test import TestCase + +from openedx_authz.engine.renderer import PolicyRenderer, SchemaApplier +from openedx_authz.engine.schema.compilation import SchemaCompiler +from openedx_authz.engine.schema.exceptions import SchemaApplyError +from openedx_authz.models.schema import ( + AuthzPermissionCategory, + AuthzPermissionDefinition, + AuthzRoleDefinition, + AuthzRolePermission, +) + +from .factories import category, extension, make_document, permission, role + +PERMS = [ + permission(name="view_course", cat="cat"), + permission(name="manage_tags", cat="cat"), + permission(name="export_course", cat="cat"), +] + + +def _doc(*, name="core", priority=100, roles=None, permissions=None, categories=None, role_extensions=None): + return make_document( + name, + priority=priority, + categories=categories if categories is not None else [category("cat")], + permissions=permissions if permissions is not None else PERMS, + roles=roles if roles is not None else [], + role_extensions=role_extensions or [], + ) + + +class FakeEnforcer: + """Minimal in-memory stand-in for the Casbin enforcer used by apply/plan. + + Stores ``p`` rows and ``g`` (grouping) rows as lists of string lists, which + is the shape the real enforcer returns. + """ + + def __init__(self, policies=None, grouping=None): + self._policies = [list(row) for row in (policies or [])] + self._grouping = [list(row) for row in (grouping or [])] + + def get_policy(self): + """Return a copy of the stored ``p`` rows.""" + return [list(row) for row in self._policies] + + def get_grouping_policy(self): + """Return a copy of the stored ``g`` (grouping) rows.""" + return [list(row) for row in self._grouping] + + def add_policy(self, *args): + """Add a ``p`` row, ignoring exact duplicates. Returns True if added.""" + row = list(args) + if row not in self._policies: + self._policies.append(row) + return True + return False + + def remove_policy(self, *args): + """Remove a ``p`` row if present. Returns True if removed.""" + row = list(args) + if row in self._policies: + self._policies.remove(row) + return True + return False + + def add_grouping_policy(self, *args): + """Add a ``g`` row, ignoring exact duplicates. Returns True if added.""" + row = list(args) + if row not in self._grouping: + self._grouping.append(row) + return True + return False + + def remove_grouping_policy(self, *args): + """Remove a ``g`` row if present. Returns True if removed.""" + row = list(args) + if row in self._grouping: + self._grouping.remove(row) + return True + return False + + +def _compile(*documents): + return SchemaCompiler().compile(list(documents)) + + +def _render(*documents): + return PolicyRenderer().render(_compile(*documents)) + + +def _editor(perms): + return _doc(roles=[role(rid="course_editor", scopes=("course-v1",), permissions=perms)]) + + +def _without_manage_tags(): + """An extension that removes ``courses.manage_tags`` from ``course_editor``.""" + return _doc( + name="modx", + priority=200, + roles=[], + categories=[], + permissions=[], + role_extensions=[extension("course_editor", remove_permissions=("courses.manage_tags",))], + ) + + +@pytest.fixture(name="cache_invalidation") +def cache_invalidation_fixture(monkeypatch): + """Capture policy-cache invalidation rather than writing a version row. + + Returns the mock so tests can assert *whether* the cache was invalidated, + which is the observable contract on both the success and failure paths. + """ + invalidate = mock.Mock(name="invalidate_policy_cache") + monkeypatch.setattr( + "openedx_authz.engine.enforcer.AuthzEnforcer.invalidate_policy_cache", + staticmethod(invalidate), + raising=False, + ) + return invalidate + + +def _apply(enforcer, *documents, force=False): + """Compile, render and apply one coherent schema. + + Rendering and persistence must come from the *same* compiled schema: + pruning is driven by the ownership recorded in the definition tables, so a + render that disagrees with what was stored would leave rows unattributed + and unprunable. + """ + schema = _compile(*documents) + rendered = PolicyRenderer().render(schema) + return SchemaApplier(enforcer=enforcer).apply(rendered, schema, force=force) + + +@pytest.mark.django_db +@pytest.mark.usefixtures("cache_invalidation") +class TestApplyReconciliation: + """Enforcer-level add/remove/idempotency behavior.""" + + def test_first_apply_adds_all_rows(self): + """A first apply adds one policy row per rendered grant.""" + enforcer = FakeEnforcer() + + result = _apply(enforcer, _editor(("courses.view_course", "courses.manage_tags"))) + + assert result.added == 2 + assert result.removed == 0 + assert len(enforcer.get_policy()) == 2 + + def test_reapply_is_idempotent(self): + """Re-applying the same schema adds and removes nothing.""" + enforcer = FakeEnforcer() + document = _editor(("courses.view_course", "courses.manage_tags")) + + _apply(enforcer, document) + result = _apply(enforcer, document) + + assert result.added == 0 + assert result.removed == 0 + assert result.unchanged is True + assert len(enforcer.get_policy()) == 2 + + def test_removed_permission_prunes_stale_p_row(self): + """Removing a permission prunes its stale policy row, keeping the others.""" + # Start with two permissions on the role, then drop one via extension. + enforcer = FakeEnforcer() + base = _editor(("courses.view_course", "courses.manage_tags")) + _apply(enforcer, base) + assert len(enforcer.get_policy()) == 2 + + result = _apply(enforcer, base, _without_manage_tags()) + + assert result.removed == 1 + remaining = {tuple(row) for row in enforcer.get_policy()} + assert ("role^course_editor", "act^courses.manage_tags", "course-v1^*", "allow") not in remaining + assert ("role^course_editor", "act^courses.view_course", "course-v1^*", "allow") in remaining + + def test_removed_role_without_assignments_is_pruned(self): + """A role dropped from the schema is pruned when it has no assignments.""" + enforcer = FakeEnforcer() + _apply(enforcer, _editor(("courses.view_course",))) + + # Nothing rendered now -> the role's p row is stale and removed. + result = _apply(enforcer) + + assert result.removed == 1 + assert enforcer.get_policy() == [] + + +@pytest.mark.django_db +@pytest.mark.usefixtures("cache_invalidation") +class TestOwnershipBoundary: + """Only rows the loader recorded as its own may be pruned (ADR 0025 §6). + + A stored ``p`` row that no schema declares — a legacy policy-file row, an + administrative fix (ADR 0018 §7), or a row owned by another service — stays + in place and enforceable, and is never attributed to a schema source. + """ + + UNMANAGED = ("role^legacy_thing", "act^courses.view_course", "course-v1^*", "allow") + + def test_unmanaged_policy_row_is_preserved(self): + """A policy row no schema owns is left in place by apply.""" + enforcer = FakeEnforcer(policies=[self.UNMANAGED]) + + _apply(enforcer, _editor(("courses.view_course",))) + + assert list(self.UNMANAGED) in enforcer.get_policy() + + def test_unmanaged_row_is_not_reported_as_removed(self): + """An unmanaged row is never counted among the removed rows.""" + enforcer = FakeEnforcer(policies=[self.UNMANAGED]) + + result = _apply(enforcer, _editor(("courses.view_course",))) + + assert result.removed == 0 + + def test_unmanaged_row_survives_an_empty_schema(self): + """Even with nothing to render, an unowned row is not ours to delete.""" + enforcer = FakeEnforcer(policies=[self.UNMANAGED]) + + result = _apply(enforcer) + + assert result.removed == 0 + assert enforcer.get_policy() == [list(self.UNMANAGED)] + + def test_unmanaged_row_is_not_attributed(self): + """An unmanaged row gains no definition/source attribution.""" + enforcer = FakeEnforcer(policies=[self.UNMANAGED]) + + _apply(enforcer, _editor(("courses.view_course",))) + + assert not AuthzRoleDefinition.objects.filter(role_id="legacy_thing").exists() + + def test_adopts_preexisting_rows_without_definitions(self): + """ADR 0025 §6: an existing row gains definitions instead of being rewritten. + + This is the realistic first deployment: ``load_policies`` already wrote + the ``p`` rows and the definition tables are empty. No policy row moves, + but the definitions are new, so the run is *not* reported as unchanged. + """ + document = _editor(("courses.view_course",)) + preexisting = [row.as_policy() for row in _render(document).rows] + enforcer = FakeEnforcer(policies=preexisting) + + result = _apply(enforcer, document) + + assert result.added == 0 + assert result.removed == 0 + assert result.unchanged is False + assert enforcer.get_policy() == preexisting + grant = AuthzRolePermission.objects.get() + assert grant.role.role_id == "course_editor" + assert grant.sources.count() == 1 + + def test_pruning_follows_the_recorded_grant(self): + """The prune is driven by the grant row, not by the raw policy diff.""" + enforcer = FakeEnforcer() + base = _editor(("courses.view_course", "courses.manage_tags")) + _apply(enforcer, base) + assert AuthzRolePermission.objects.count() == 2 + + _apply(enforcer, base, _without_manage_tags()) + + assert AuthzRolePermission.objects.count() == 1 + assert len(enforcer.get_policy()) == 1 + + +@pytest.mark.django_db +@pytest.mark.usefixtures("cache_invalidation") +class TestForceGate: + """Removal of a role that still has user assignments is force-gated.""" + + def _assigned_enforcer(self): + """Build an enforcer holding a stored role plus one user assignment to it.""" + enforcer = FakeEnforcer() + _apply(enforcer, _editor(("courses.view_course",))) + # A user is assigned the role (g row: [subject, role, scope]). + enforcer.add_grouping_policy("user^alice", "role^course_editor", "course-v1:OpenedX+DemoX+Demo") + return enforcer + + def test_blocking_assignment_aborts_without_force(self): + """Removing a role with a live assignment aborts without ``force``.""" + enforcer = self._assigned_enforcer() + + with pytest.raises(SchemaApplyError): + _apply(enforcer, force=False) + + # No write happened: the p row is still there. + assert len(enforcer.get_policy()) == 1 + + def test_force_removes_role_rows_and_assignments(self): + """With ``force``, the removed role's policy and assignment rows are pruned.""" + enforcer = self._assigned_enforcer() + + result = _apply(enforcer, force=True) + + assert result.removed == 1 + assert enforcer.get_policy() == [] + assert enforcer.get_grouping_policy() == [] + + +@pytest.mark.django_db +class TestApplyFailure: + """A failed write must not leave Casbin ahead of the database (ADR 0018 §5). + + ``add_policy``/``remove_policy`` mutate the enforcer's in-memory model as + well as the database, so a rollback would otherwise leave the process + enforcing rows that were never committed. + """ + + @staticmethod + def _failing_store(monkeypatch): + """Write a definition row, then fail, so rollback is observable.""" + + def _store_then_fail(self, schema): # pylint: disable=unused-argument + AuthzRoleDefinition.objects.create( + role_id="half_written", + display_name="Half written", + description="", + scopes=["course-v1"], + hidden=False, + ) + raise IntegrityError("simulated write failure") + + monkeypatch.setattr(SchemaApplier, "_store_sources", _store_then_fail) + + def test_failure_propagates(self, monkeypatch, cache_invalidation): # pylint: disable=unused-argument + """A write failure during apply propagates to the caller.""" + self._failing_store(monkeypatch) + + with pytest.raises(IntegrityError): + _apply(FakeEnforcer(), _editor(("courses.view_course",))) + + def test_failure_rolls_back_definition_writes(self, monkeypatch, cache_invalidation): # pylint: disable=unused-argument + """A failed apply rolls back any definition rows it had written.""" + self._failing_store(monkeypatch) + + with pytest.raises(IntegrityError): + _apply(FakeEnforcer(), _editor(("courses.view_course",))) + + assert not AuthzRoleDefinition.objects.filter(role_id="half_written").exists() + + def test_failure_invalidates_the_policy_cache(self, monkeypatch, cache_invalidation): + """The in-memory model kept the rolled-back rows, so force a reload.""" + self._failing_store(monkeypatch) + enforcer = FakeEnforcer() + + with pytest.raises(IntegrityError): + _apply(enforcer, _editor(("courses.view_course",))) + + # The fake enforcer models the real divergence: it still holds the row + # the database rolled back. Invalidating the cache is what makes the + # next enforcer access reload the committed state. + assert len(enforcer.get_policy()) == 1 + cache_invalidation.assert_called_once_with() + + def test_successful_apply_invalidates_once_when_rows_change(self, cache_invalidation): + """A successful apply that changes rows invalidates the policy cache once.""" + _apply(FakeEnforcer(), _editor(("courses.view_course",))) + + cache_invalidation.assert_called_once_with() + + def test_successful_apply_skips_invalidation_when_unchanged(self, cache_invalidation): + """An apply that changes nothing does not invalidate the policy cache.""" + enforcer = FakeEnforcer() + document = _editor(("courses.view_course",)) + _apply(enforcer, document) + cache_invalidation.reset_mock() + + _apply(enforcer, document) + + cache_invalidation.assert_not_called() + + +class TestDefinitionPruning(TestCase): + """Definition/source tables track the compiled schema across applies.""" + + def test_removed_permission_prunes_grant_and_definition(self): + """Dropping a permission prunes both its grant and its definition row.""" + applier = SchemaApplier() + + first = SchemaCompiler().compile([_editor(("courses.view_course", "courses.manage_tags"))]) + applier._store_sources(first) # pylint: disable=protected-access + editor = AuthzRoleDefinition.objects.get(role_id="course_editor") + assert editor.role_permissions.count() == 2 + + # Drop manage_tags via an extension and remove the permission definition. + second = SchemaCompiler().compile( + [ + _doc( + roles=[role(rid="course_editor", permissions=("courses.view_course",))], + permissions=[permission(name="view_course", cat="cat")], + ) + ] + ) + applier._store_sources(second) # pylint: disable=protected-access + + editor.refresh_from_db() + assert editor.role_permissions.count() == 1 + assert not AuthzPermissionDefinition.objects.filter(name="manage_tags").exists() + assert not AuthzPermissionDefinition.objects.filter(name="export_course").exists() + + def test_removed_role_and_category_are_pruned(self): + """Applying an empty schema prunes every role, grant, permission, and category.""" + applier = SchemaApplier() + applier._store_sources( # pylint: disable=protected-access + SchemaCompiler().compile([_editor(("courses.view_course",))]) + ) + assert AuthzRoleDefinition.objects.filter(role_id="course_editor").exists() + + # Apply an empty schema: everything the previous schema owned is pruned. + applier._store_sources(SchemaCompiler().compile([])) # pylint: disable=protected-access + + assert AuthzRoleDefinition.objects.count() == 0 + assert AuthzRolePermission.objects.count() == 0 + assert AuthzPermissionDefinition.objects.count() == 0 + assert AuthzPermissionCategory.objects.count() == 0 + + +@pytest.mark.django_db +@pytest.mark.usefixtures("cache_invalidation") +class TestDefinitionChangeReport: + """The plan reports definition changes, not just policy rows (ADR 0018 §6). + + Apply syncs the definition tables unconditionally, so a metadata-only edit + changes stored state while leaving every ``p`` row identical. Reporting only + rows would tell the operator "unchanged" and then rewrite their metadata. + """ + + @staticmethod + def _plan(enforcer, *documents): + schema = _compile(*documents) + return SchemaApplier(enforcer=enforcer).plan(PolicyRenderer().render(schema), schema) + + def test_first_run_reports_every_definition_as_added(self): + """A first run reports every role, category, permission, and grant as added.""" + plan = self._plan(FakeEnforcer(), _editor(("courses.view_course",))) + + assert plan.roles.added == ["course_editor"] + assert plan.categories.added == ["cat"] + assert "courses.view_course" in plan.permissions.added + assert plan.grants.added == ["course_editor -> courses.view_course @ course-v1"] + assert plan.unchanged is False + + def test_identical_reapply_reports_no_definition_changes(self): + """Re-planning an identical schema reports no definition changes.""" + enforcer = FakeEnforcer() + document = _editor(("courses.view_course",)) + _apply(enforcer, document) + + plan = self._plan(enforcer, document) + + assert plan.definitions_unchanged is True + assert plan.unchanged is True + + def test_metadata_only_change_is_reported(self): + """No p row moves, yet the role's display name would be rewritten.""" + enforcer = FakeEnforcer() + before = _doc(roles=[role(rid="course_editor", permissions=("courses.view_course",))]) + _apply(enforcer, before) + + after = _doc( + roles=[ + role( + rid="course_editor", + permissions=("courses.view_course",), + display_name="Course author", + ) + ] + ) + plan = self._plan(enforcer, after) + + assert not plan.added_rows + assert not plan.removed_rows + assert plan.roles.updated == ["course_editor"] + assert plan.unchanged is False + + def test_hidden_flag_change_is_reported(self): + """Flipping a role's ``hidden`` flag is reported as an update.""" + enforcer = FakeEnforcer() + before = _doc(roles=[role(rid="course_editor", permissions=("courses.view_course",))]) + _apply(enforcer, before) + + after = _doc(roles=[role(rid="course_editor", permissions=("courses.view_course",), hidden=True)]) + plan = self._plan(enforcer, after) + + assert plan.roles.updated == ["course_editor"] + + def test_permission_metadata_change_is_reported(self): + """Editing a permission's display name is reported as an update.""" + enforcer = FakeEnforcer() + _apply(enforcer, _editor(("courses.view_course",))) + + renamed = _doc( + roles=[role(rid="course_editor", permissions=("courses.view_course",))], + permissions=[ + permission(name="view_course", cat="cat", display_name="See course"), + permission(name="manage_tags", cat="cat"), + permission(name="export_course", cat="cat"), + ], + ) + plan = self._plan(enforcer, renamed) + + assert plan.permissions.updated == ["courses.view_course"] + + def test_category_metadata_change_is_reported(self): + """Editing a category's display metadata is reported as an update.""" + enforcer = FakeEnforcer() + _apply(enforcer, _editor(("courses.view_course",))) + + recategorized = _doc( + roles=[role(rid="course_editor", permissions=("courses.view_course",))], + categories=[category("cat", display_name="Course content", icon="Article")], + ) + plan = self._plan(enforcer, recategorized) + + assert plan.categories.updated == ["cat"] + + def test_dropped_definitions_are_reported_as_removed(self): + """Definitions absent from the new schema are reported as removed.""" + enforcer = FakeEnforcer() + _apply(enforcer, _editor(("courses.view_course",))) + + plan = self._plan(enforcer) + + assert plan.roles.removed == ["course_editor"] + assert plan.categories.removed == ["cat"] + assert plan.grants.removed == ["course_editor -> courses.view_course @ course-v1"] + + def test_grant_change_is_reported_alongside_the_row(self): + """A removed grant is reported both as a grant change and a removed row.""" + enforcer = FakeEnforcer() + base = _editor(("courses.view_course", "courses.manage_tags")) + _apply(enforcer, base) + + plan = self._plan(enforcer, base, _without_manage_tags()) + + assert plan.grants.removed == ["course_editor -> courses.manage_tags @ course-v1"] + assert len(plan.removed_rows) == 1 + + def test_plan_without_a_schema_reports_rows_only(self): + """``plan`` stays usable for row-only comparisons (schema optional).""" + enforcer = FakeEnforcer() + + plan = SchemaApplier(enforcer=enforcer).plan(_render(_editor(("courses.view_course",)))) + + assert len(plan.added_rows) == 1 + assert plan.definitions_unchanged is True + + def test_plan_does_not_write(self): + """Planning is read-only: it writes no policy rows or definitions.""" + enforcer = FakeEnforcer() + + self._plan(enforcer, _editor(("courses.view_course",))) + + assert enforcer.get_policy() == [] + assert AuthzRoleDefinition.objects.count() == 0 diff --git a/src/openedx_authz/tests/schema/test_renderer.py b/src/openedx_authz/tests/schema/test_renderer.py index 29f806a2..f8b1f477 100644 --- a/src/openedx_authz/tests/schema/test_renderer.py +++ b/src/openedx_authz/tests/schema/test_renderer.py @@ -1,11 +1,16 @@ -"""Unit tests for the (pure) render step. +"""Unit tests for the (pure) render step and renderer helper methods. Covers turning a compiled schema into Casbin ``p`` rows: one row per role-permission-scope, the ``role^``/``act^``/``^*`` namespacing convention, -deterministic output, and scope fan-out. +deterministic output, and scope fan-out; plus the ``SchemaApplier`` helpers for +enforcer resolution and role-assignment-deleted event emission. """ -from openedx_authz.engine.renderer import PolicyRenderer, PolicyRow +import sys +import types +from unittest import mock + +from openedx_authz.engine.renderer import PolicyRenderer, PolicyRow, SchemaApplier from openedx_authz.engine.schema.compilation import SchemaCompiler from .factories import category, make_document, permission, role @@ -82,3 +87,110 @@ def test_multiple_scopes_multiply_rows(self): rendered = PolicyRenderer().render(SchemaCompiler().compile([doc])) scopes = {row.scope for row in rendered.rows} assert scopes == {"course-v1^*", "ccx-v1^*"} + + +class TestResolveEnforcer: + """``SchemaApplier._resolve_enforcer``: injected vs. lazily resolved enforcer.""" + + def test_returns_injected_enforcer_without_importing(self): + """An enforcer passed in is returned as-is (no lazy resolution).""" + sentinel = object() + applier = SchemaApplier(enforcer=sentinel) + + # Patch the lazy import target to prove it is never touched. + with mock.patch("openedx_authz.engine.enforcer.AuthzEnforcer") as authz_enforcer: + assert applier._resolve_enforcer() is sentinel # pylint: disable=protected-access + authz_enforcer.get_enforcer.assert_not_called() + + def test_lazily_resolves_when_enforcer_is_none(self): + """When no enforcer was injected, it is fetched via AuthzEnforcer and cached.""" + resolved = object() + applier = SchemaApplier() # enforcer defaults to None + + with mock.patch("openedx_authz.engine.enforcer.AuthzEnforcer") as authz_enforcer: + authz_enforcer.get_enforcer.return_value = resolved + + first = applier._resolve_enforcer() # pylint: disable=protected-access + second = applier._resolve_enforcer() # pylint: disable=protected-access + + assert first is resolved + # Cached after the first resolution: only one lookup despite two calls. + assert second is resolved + authz_enforcer.get_enforcer.assert_called_once_with() + + +class TestEmitAssignmentDeleted: + """``SchemaApplier._emit_assignment_deleted``: one event per removed assignment.""" + + def test_no_op_when_no_assignments(self): + """Empty input emits nothing and does not import event machinery.""" + with mock.patch.dict(sys.modules): + # If the method tried to import openedx_events, a missing stub would + # raise; the early return means it never gets there. + SchemaApplier._emit_assignment_deleted([]) # pylint: disable=protected-access + + def test_emits_one_event_per_removed_assignment(self): + """Each removed (subject, role, scope) triple sends a ROLE_ASSIGNMENT_DELETED.""" + removed = [ + ("user^alice", "role^course_editor", "course-v1^course-v1:Org+C+R"), + ("user^bob", "role^course_auditor", "course-v1^*"), + ] + + # Build lazy-import stubs for the modules the method imports internally. + crum_mod = types.ModuleType("crum") + crum_mod.get_current_user = lambda: types.SimpleNamespace(id=42) + + role_assignment_data = mock.MagicMock(name="RoleAssignmentEventData") + events_data = types.ModuleType("openedx_events.authz.data") + events_data.RoleAssignmentData = role_assignment_data + + signal = mock.MagicMock(name="ROLE_ASSIGNMENT_DELETED") + events_signals = types.ModuleType("openedx_events.authz.signals") + events_signals.ROLE_ASSIGNMENT_DELETED = signal + + with mock.patch.dict( + sys.modules, + { + "crum": crum_mod, + "openedx_events.authz.data": events_data, + "openedx_events.authz.signals": events_signals, + }, + ): + SchemaApplier._emit_assignment_deleted(removed) # pylint: disable=protected-access + + assert signal.send_event.call_count == 2 + + # Verify field mapping for the first emitted event. + first_event_data = role_assignment_data.call_args_list[0].kwargs + assert first_event_data["operation"] == "deleted" + assert first_event_data["subject"] == "user^alice" + assert first_event_data["role"] == "role^course_editor" + assert first_event_data["scope"] == "course-v1^course-v1:Org+C+R" + assert first_event_data["actor_id"] == 42 + + def test_actor_id_none_when_no_current_user(self): + """A missing current user yields actor_id=None on the event.""" + removed = [("user^alice", "role^course_editor", "course-v1^*")] + + crum_mod = types.ModuleType("crum") + crum_mod.get_current_user = lambda: None + + role_assignment_data = mock.MagicMock(name="RoleAssignmentEventData") + events_data = types.ModuleType("openedx_events.authz.data") + events_data.RoleAssignmentData = role_assignment_data + + signal = mock.MagicMock(name="ROLE_ASSIGNMENT_DELETED") + events_signals = types.ModuleType("openedx_events.authz.signals") + events_signals.ROLE_ASSIGNMENT_DELETED = signal + + with mock.patch.dict( + sys.modules, + { + "crum": crum_mod, + "openedx_events.authz.data": events_data, + "openedx_events.authz.signals": events_signals, + }, + ): + SchemaApplier._emit_assignment_deleted(removed) # pylint: disable=protected-access + + assert role_assignment_data.call_args_list[0].kwargs["actor_id"] is None diff --git a/src/openedx_authz/tests/schema/test_source_storage.py b/src/openedx_authz/tests/schema/test_source_storage.py new file mode 100644 index 00000000..f06763c3 --- /dev/null +++ b/src/openedx_authz/tests/schema/test_source_storage.py @@ -0,0 +1,343 @@ +"""Tests for persisting compiled definitions and their sources (ADR 0025). + +These exercise ``SchemaApplier._store_sources`` directly (it performs only ORM +upserts, no enforcer access) plus the origin query helpers. The full ``apply`` +path (enforcer + p rows) is covered by the engine tests. +""" + +from dataclasses import replace + +from django.test import TestCase + +from openedx_authz.engine.renderer import SchemaApplier +from openedx_authz.engine.schema.compilation import SchemaCompiler +from openedx_authz.models.schema import ( + AuthzPermissionCategory, + AuthzPermissionDefinition, + AuthzRoleDefinition, + AuthzRolePermission, + AuthzRolePermissionSource, + AuthzRoleSource, + AuthzSchemaSource, + OriginKind, + origin_for_role_permission, + origins_for_category, + origins_for_permission, + origins_for_role, +) + +from .factories import category, extension, make_document, permission, role + +CORE_PERMS = [ + permission(name="view_course", cat="cat"), + permission(name="manage_tags", cat="cat"), + permission(name="export_course", cat="cat"), +] + + +def _core_doc(): + return make_document( + "core", + priority=100, + categories=[category("cat")], + permissions=CORE_PERMS, + roles=[role(rid="course_admin", permissions=("courses.view_course", "courses.manage_tags"))], + ) + + +def _module_extension_doc(): + return make_document( + "modx", + priority=200, + role_extensions=[extension("course_admin", add_permissions=("courses.export_course",))], + ) + + +def _store(*documents): + schema = SchemaCompiler().compile(list(documents)) + SchemaApplier()._store_sources(schema) # pylint: disable=protected-access + return schema + + +class TestStoreSources(TestCase): + """Persistence of compiled definitions and their provenance.""" + + def test_definitions_are_persisted(self): + """Storing a document writes its roles, permissions, and grants to the DB.""" + _store(_core_doc()) + self.assertEqual(AuthzRoleDefinition.objects.count(), 1) + self.assertEqual(AuthzPermissionDefinition.objects.count(), 3) + role_obj = AuthzRoleDefinition.objects.get(role_id="course_admin") + # course_admin has 2 permissions x 1 scope = 2 grants. + self.assertEqual(role_obj.role_permissions.count(), 2) + + def test_source_identity_is_distribution_and_module(self): + """A source row is keyed by its distribution and module, not its file path.""" + _store(_core_doc()) + source = AuthzSchemaSource.objects.get() + self.assertEqual(source.distribution, "test-dist") + self.assertEqual(source.module, "pkg.core") + + def test_extension_grant_attributed_to_module_not_core(self): + """A grant added by an extension records its own origin_kind and priority. + + The base grant stays BASE while the extension-contributed grant is + marked EXTENSION with the extending module's priority. + """ + _store(_core_doc(), _module_extension_doc()) + + # Both grants live on course_admin, with distinct origins. + self.assertEqual(origin_for_role_permission("course_admin", "courses.view_course"), ["test-dist"]) + self.assertEqual(origin_for_role_permission("course_admin", "courses.export_course"), ["test-dist"]) + + export_grant = AuthzRolePermission.objects.get( + role__role_id="course_admin", permission__namespace="courses", permission__name="export_course" + ) + link = AuthzRolePermissionSource.objects.get(role_permission=export_grant) + self.assertEqual(link.origin_kind, OriginKind.EXTENSION) + self.assertEqual(link.priority, 200) + + view_grant = AuthzRolePermission.objects.get(role__role_id="course_admin", permission__name="view_course") + view_link = AuthzRolePermissionSource.objects.get(role_permission=view_grant) + self.assertEqual(view_link.origin_kind, OriginKind.BASE) + + def test_origin_query_helpers(self): + """The role- and permission-level origin helpers report the contributing distribution.""" + _store(_core_doc(), _module_extension_doc()) + self.assertEqual(origins_for_role("course_admin"), ["test-dist"]) + self.assertEqual(origins_for_permission("courses.export_course"), ["test-dist"]) + + def test_store_is_idempotent(self): + """Re-storing the same documents leaves all row counts unchanged.""" + _store(_core_doc(), _module_extension_doc()) + counts = ( + AuthzRoleDefinition.objects.count(), + AuthzPermissionDefinition.objects.count(), + AuthzRolePermission.objects.count(), + AuthzRolePermissionSource.objects.count(), + AuthzSchemaSource.objects.count(), + ) + _store(_core_doc(), _module_extension_doc()) + counts_again = ( + AuthzRoleDefinition.objects.count(), + AuthzPermissionDefinition.objects.count(), + AuthzRolePermission.objects.count(), + AuthzRolePermissionSource.objects.count(), + AuthzSchemaSource.objects.count(), + ) + self.assertEqual(counts, counts_again) + + def test_metadata_change_updates_in_place(self): + """Re-storing with changed metadata updates the existing row instead of adding one.""" + _store(_core_doc()) + changed = make_document( + "core", + priority=100, + categories=[category("cat")], + permissions=CORE_PERMS, + roles=[ + role( + rid="course_admin", + display_name="Course Administrator", + permissions=("courses.view_course", "courses.manage_tags"), + ) + ], + ) + _store(changed) + self.assertEqual(AuthzRoleDefinition.objects.count(), 1) + self.assertEqual(AuthzRoleDefinition.objects.get(role_id="course_admin").display_name, "Course Administrator") + + def test_moving_definition_between_files_keeps_single_source(self): + """Moving a definition to another file in the same module reuses its source row.""" + # Same module, different resource_path -> identity unchanged. + doc_a = _core_doc() + doc_b = make_document( + "core", # same module name -> same (distribution, module) + priority=100, + categories=[category("cat")], + permissions=CORE_PERMS, + roles=[role(rid="course_admin", permissions=("courses.view_course", "courses.manage_tags"))], + ) + doc_b.source = doc_b.source.__class__(**{**doc_b.source.__dict__, "resource_path": "moved.authz.yaml"}) + _store(doc_a) + _store(doc_b) + self.assertEqual(AuthzSchemaSource.objects.count(), 1) + + +class TestSourceGranularity(TestCase): + """Source identity is per module, not per file (ADR 0025 §2). + + ``resource_path`` and ``content_digest`` are explicitly non-identifying, so + several files in one module collapse into a single source row. This is what + lets a definition move between files without churn, and it means those two + advisory fields hold whichever file was processed last. + """ + + @staticmethod + def _same_module(name: str, resource_path: str, roles): + """Build a document in module ``pkg.`` with an explicit file path. + + Each file gets its own digest so the per-module collapse is observable. + """ + document = make_document(name, priority=100, categories=[category("cat")], permissions=CORE_PERMS, roles=roles) + document.source = replace( + document.source, resource_path=resource_path, content_digest=f"digest-{resource_path}" + ) + return document + + def test_multiple_files_in_one_module_share_one_source_row(self): + """Several files in one module collapse into a single source row.""" + roles_file = self._same_module("core", "roles.yaml", [role(rid="course_admin")]) + extra_file = self._same_module("core", "more_roles.yaml", [role(rid="course_auditor")]) + + _store(roles_file, extra_file) + + self.assertEqual(AuthzSchemaSource.objects.count(), 1) + self.assertEqual(AuthzRoleDefinition.objects.count(), 2) + + def test_advisory_fields_come_from_the_first_file_of_the_module(self): + """Why the digest is advisory, not a change-detection signal. + + One source row covers the whole module, and the per-apply cache fills it + from whichever of the module's files is processed first. So the stored + ``resource_path``/``content_digest`` describe one file out of several and + cannot represent the module's contents — change detection diffs compiled + definitions instead (ADR 0025 §2). + """ + roles_file = self._same_module("core", "roles.yaml", [role(rid="course_admin")]) + extra_file = self._same_module("core", "more_roles.yaml", [role(rid="course_auditor")]) + + _store(roles_file, extra_file) + + source = AuthzSchemaSource.objects.get() + self.assertEqual(source.resource_path, "roles.yaml") + self.assertNotEqual(source.content_digest, extra_file.source.content_digest) + + def test_distinct_modules_get_distinct_source_rows(self): + """Definitions from different modules produce separate source rows.""" + first = self._same_module("core", "roles.yaml", [role(rid="course_admin")]) + second = self._same_module("other", "roles.yaml", [role(rid="course_auditor")]) + + _store(first, second) + + self.assertEqual(AuthzSchemaSource.objects.count(), 2) + self.assertEqual(sorted(AuthzSchemaSource.objects.values_list("module", flat=True)), ["pkg.core", "pkg.other"]) + + def test_shared_definition_gains_a_link_per_contributing_module(self): + """ADR 0025 §2: the many-to-many exists to represent shared ownership.""" + first = self._same_module("core", "roles.yaml", [role(rid="course_admin")]) + second = self._same_module("other", "roles.yaml", [role(rid="course_admin")]) + + _store(first, second) + + role_obj = AuthzRoleDefinition.objects.get(role_id="course_admin") + self.assertEqual(AuthzRoleSource.objects.filter(role=role_obj).count(), 2) + + def test_shared_grant_gains_a_source_link_per_module(self): + """A grant defined by two modules gets one source link per contributing module.""" + admin = [role(rid="course_admin", permissions=("courses.view_course",))] + first = self._same_module("core", "roles.yaml", admin) + second = self._same_module("other", "roles.yaml", admin) + + _store(first, second) + + grant = AuthzRolePermission.objects.get(role__role_id="course_admin", permission__name="view_course") + self.assertEqual(AuthzRolePermissionSource.objects.filter(role_permission=grant).count(), 2) + self.assertEqual(sorted(origin_for_role_permission("course_admin", "courses.view_course")), ["test-dist"]) + + def test_category_origins_are_queryable(self): + """The category origin helper reports the contributing distribution.""" + _store(_core_doc()) + + self.assertEqual(origins_for_category("cat"), ["test-dist"]) + + def test_source_rows_survive_definition_pruning(self): + """Sources are shared and carry no access, so they are never pruned.""" + _store(_core_doc()) + self.assertEqual(AuthzSchemaSource.objects.count(), 1) + + _store() + + self.assertEqual(AuthzRoleDefinition.objects.count(), 0) + self.assertEqual(AuthzSchemaSource.objects.count(), 1) + + def test_hidden_flag_reaches_the_database(self): + """ADR 0023 §1: ``hidden`` is compiled state that has to be persisted.""" + _store(self._same_module("core", "roles.yaml", [role(rid="course_auditor", hidden=True)])) + + self.assertTrue(AuthzRoleDefinition.objects.get(role_id="course_auditor").hidden) + + def test_hidden_flag_can_be_cleared(self): + """Re-storing a role with hidden=False clears a previously persisted hidden flag.""" + _store(self._same_module("core", "roles.yaml", [role(rid="course_auditor", hidden=True)])) + + _store(self._same_module("core", "roles.yaml", [role(rid="course_auditor", hidden=False)])) + + self.assertFalse(AuthzRoleDefinition.objects.get(role_id="course_auditor").hidden) + + +class TestDefinitionDisplay(TestCase): + """Human-readable identifiers used by the Django admin fallback (ADR 0018 §7).""" + + def test_source_string_is_distribution_and_module_path(self): + """A source renders as ``distribution:module/path`` for its id and str().""" + _store(_core_doc()) + + source = AuthzSchemaSource.objects.get() + self.assertEqual(source.source_id, "test-dist:pkg/core") + self.assertEqual(str(source), "test-dist:pkg/core") + + def test_permission_string_is_its_complete_id(self): + """A permission renders as its full ``namespace.name`` identifier.""" + _store(_core_doc()) + + perm = AuthzPermissionDefinition.objects.get(namespace="courses", name="view_course") + self.assertEqual(perm.identifier, "courses.view_course") + self.assertEqual(str(perm), "courses.view_course") + + def test_role_and_category_strings_are_their_stable_ids(self): + """Roles and categories render as their stable string ids.""" + _store(_core_doc()) + + self.assertEqual(str(AuthzRoleDefinition.objects.get(role_id="course_admin")), "course_admin") + self.assertEqual(str(AuthzPermissionCategory.objects.get(category_id="cat")), "cat") + + def test_grant_string_names_role_permission_and_scope(self): + """Regression: this used to render the FK integers, not the identifiers.""" + _store(_core_doc()) + + grant = AuthzRolePermission.objects.get(role__role_id="course_admin", permission__name="view_course") + self.assertEqual(str(grant), "course_admin -> courses.view_course @ course-v1") + + +class TestDefensiveStorage(TestCase): + """Paths guarded against states validation is expected to have rejected.""" + + def test_grant_for_an_undefined_permission_is_skipped(self): + """A role listing a permission with no definition writes no grant.""" + document = make_document( + "core", + priority=100, + categories=[category("cat")], + permissions=[], + roles=[role(rid="course_admin", permissions=("courses.ghost",))], + ) + + _store(document) + + self.assertTrue(AuthzRoleDefinition.objects.filter(role_id="course_admin").exists()) + self.assertEqual(AuthzRolePermission.objects.count(), 0) + + def test_permission_with_an_unknown_category_is_stored_uncategorized(self): + """A permission referencing a missing category is stored with no category.""" + document = make_document( + "core", + priority=100, + categories=[], + permissions=[permission(name="view_course", cat="missing")], + roles=[], + ) + + _store(document) + + self.assertIsNone(AuthzPermissionDefinition.objects.get(name="view_course").category) From ad8db0bea05a8f8f754699b286bd394c7432cf01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Mon, 5 Oct 2026 16:00:41 -0600 Subject: [PATCH 2/3] squash!: Refactor imports --- src/openedx_authz/engine/renderer.py | 44 ++++++------- .../tests/schema/test_renderer.py | 62 +++++-------------- 2 files changed, 36 insertions(+), 70 deletions(-) diff --git a/src/openedx_authz/engine/renderer.py b/src/openedx_authz/engine/renderer.py index ed71c49d..463fb89b 100644 --- a/src/openedx_authz/engine/renderer.py +++ b/src/openedx_authz/engine/renderer.py @@ -35,9 +35,9 @@ :meth:`SchemaApplier._managed_rows`), so unmanaged ``p`` rows, dynamic roles, user assignments, and legacy ``g2`` action inheritance are all preserved. -``render`` is pure and imports nothing from Casbin/Django. ``plan``/``apply`` -import the enforcer lazily so this module stays importable without a configured -Django environment. +``render`` is pure in behavior — it touches no database and holds no Casbin +state. ``plan``/``apply`` resolve the enforcer only when called (not at import), +so enforcer initialization still happens after Django settings are configured. """ from __future__ import annotations @@ -45,6 +45,11 @@ import logging from dataclasses import dataclass, field +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_DELETED + from openedx_authz.data import ( ACTION_NAMESPACE, EFFECT_ALLOW, @@ -54,8 +59,11 @@ PolicyIndex, ) from openedx_authz.data import AUTHZ_POLICY_ATTRIBUTES_SEPARATOR as SEP +from openedx_authz.engine.enforcer import AuthzEnforcer from openedx_authz.engine.schema.exceptions import SchemaApplyError from openedx_authz.engine.schema.types import CompiledSchema, RoleDefinition +from openedx_authz.models import schema as m +from openedx_authz.models.core import RoleAssignmentAudit logger = logging.getLogger(__name__) @@ -286,10 +294,6 @@ def apply( SchemaApplyError: If the plan has blocking assignments and ``force`` is False. """ - from django.db import transaction # pylint: disable=import-outside-toplevel - - from openedx_authz.engine.enforcer import AuthzEnforcer # pylint: disable=import-outside-toplevel - plan = self.plan(rendered, schema) if plan.blocking_assignments and not force: @@ -352,10 +356,14 @@ def apply( # ---- helpers ---------------------------------------------------------- def _resolve_enforcer(self): - """Lazily resolve the enforcer to honor plugin/settings timing.""" - if self._enforcer is None: - from openedx_authz.engine.enforcer import AuthzEnforcer # pylint: disable=import-outside-toplevel + """Resolve the enforcer, deferring instantiation to honor plugin/settings timing. + ``AuthzEnforcer.get_enforcer()`` is called (not merely imported) lazily: + it reads ``CASBIN_MODEL``/``CASBIN_DB_ALIAS`` and initializes Casbin, so + it must run after Django settings are configured. Importing the class at + module top is inert — it instantiates nothing. + """ + if self._enforcer is None: self._enforcer = AuthzEnforcer.get_enforcer() return self._enforcer @@ -389,19 +397,11 @@ def _emit_assignment_deleted(removed_assignments: list[tuple[str, str, str]]) -> Every assignment change must leave an audit trail: the ``create_audit_record_on_role_assignment_change`` handler turns each event into a :class:`RoleAssignmentAudit` row, matching the audit behavior of - ``unassign_role_from_subject_in_scope``. Imported lazily so the module - stays importable without Django/openedx-events configured. + ``unassign_role_from_subject_in_scope``. """ if not removed_assignments: return - # pylint: disable=import-outside-toplevel - from crum import get_current_user - from openedx_events.authz.data import RoleAssignmentData as RoleAssignmentEventData - from openedx_events.authz.signals import ROLE_ASSIGNMENT_DELETED - - from openedx_authz.models.core import RoleAssignmentAudit - actor_id = getattr(get_current_user(), "id", None) for subject, role, scope in removed_assignments: ROLE_ASSIGNMENT_DELETED.send_event( @@ -440,8 +440,6 @@ def _diff_definitions(self, schema: CompiledSchema) -> dict[str, DefinitionDiff] :class:`ChangePlan` field name. Grants carry no updatable fields, so they only ever appear as added or removed. """ - from openedx_authz.models import schema as m # pylint: disable=import-outside-toplevel - return { "categories": self._diff_kind( {cid: compiled.definition for cid, compiled in schema.categories.items()}, @@ -525,8 +523,6 @@ def _managed_rows() -> set[PolicyRow]: Empty on a first deployment, which is what makes adoption safe: nothing is pruned before the loader has recorded what it owns. """ - from openedx_authz.models import schema as m # pylint: disable=import-outside-toplevel - return { PolicyRow.from_grant( grant.role.role_id, @@ -549,8 +545,6 @@ def _store_sources(self, schema: CompiledSchema) -> None: Called inside the ``apply`` transaction. """ - from openedx_authz.models import schema as m # pylint: disable=import-outside-toplevel - source_cache: dict[tuple[str, str], object] = {} def source_obj(record): diff --git a/src/openedx_authz/tests/schema/test_renderer.py b/src/openedx_authz/tests/schema/test_renderer.py index f8b1f477..f3cbb651 100644 --- a/src/openedx_authz/tests/schema/test_renderer.py +++ b/src/openedx_authz/tests/schema/test_renderer.py @@ -6,7 +6,6 @@ enforcer resolution and role-assignment-deleted event emission. """ -import sys import types from unittest import mock @@ -97,8 +96,8 @@ def test_returns_injected_enforcer_without_importing(self): sentinel = object() applier = SchemaApplier(enforcer=sentinel) - # Patch the lazy import target to prove it is never touched. - with mock.patch("openedx_authz.engine.enforcer.AuthzEnforcer") as authz_enforcer: + # Patch the enforcer accessor to prove it is never touched. + with mock.patch("openedx_authz.engine.renderer.AuthzEnforcer") as authz_enforcer: assert applier._resolve_enforcer() is sentinel # pylint: disable=protected-access authz_enforcer.get_enforcer.assert_not_called() @@ -107,7 +106,7 @@ def test_lazily_resolves_when_enforcer_is_none(self): resolved = object() applier = SchemaApplier() # enforcer defaults to None - with mock.patch("openedx_authz.engine.enforcer.AuthzEnforcer") as authz_enforcer: + with mock.patch("openedx_authz.engine.renderer.AuthzEnforcer") as authz_enforcer: authz_enforcer.get_enforcer.return_value = resolved first = applier._resolve_enforcer() # pylint: disable=protected-access @@ -123,11 +122,10 @@ class TestEmitAssignmentDeleted: """``SchemaApplier._emit_assignment_deleted``: one event per removed assignment.""" def test_no_op_when_no_assignments(self): - """Empty input emits nothing and does not import event machinery.""" - with mock.patch.dict(sys.modules): - # If the method tried to import openedx_events, a missing stub would - # raise; the early return means it never gets there. + """Empty input emits nothing: the signal is never sent.""" + with mock.patch("openedx_authz.engine.renderer.ROLE_ASSIGNMENT_DELETED") as signal: SchemaApplier._emit_assignment_deleted([]) # pylint: disable=protected-access + signal.send_event.assert_not_called() def test_emits_one_event_per_removed_assignment(self): """Each removed (subject, role, scope) triple sends a ROLE_ASSIGNMENT_DELETED.""" @@ -136,25 +134,13 @@ def test_emits_one_event_per_removed_assignment(self): ("user^bob", "role^course_auditor", "course-v1^*"), ] - # Build lazy-import stubs for the modules the method imports internally. - crum_mod = types.ModuleType("crum") - crum_mod.get_current_user = lambda: types.SimpleNamespace(id=42) - - role_assignment_data = mock.MagicMock(name="RoleAssignmentEventData") - events_data = types.ModuleType("openedx_events.authz.data") - events_data.RoleAssignmentData = role_assignment_data - - signal = mock.MagicMock(name="ROLE_ASSIGNMENT_DELETED") - events_signals = types.ModuleType("openedx_events.authz.signals") - events_signals.ROLE_ASSIGNMENT_DELETED = signal - - with mock.patch.dict( - sys.modules, - { - "crum": crum_mod, - "openedx_events.authz.data": events_data, - "openedx_events.authz.signals": events_signals, - }, + with ( + mock.patch( + "openedx_authz.engine.renderer.get_current_user", + return_value=types.SimpleNamespace(id=42), + ), + mock.patch("openedx_authz.engine.renderer.RoleAssignmentEventData") as role_assignment_data, + mock.patch("openedx_authz.engine.renderer.ROLE_ASSIGNMENT_DELETED") as signal, ): SchemaApplier._emit_assignment_deleted(removed) # pylint: disable=protected-access @@ -172,24 +158,10 @@ def test_actor_id_none_when_no_current_user(self): """A missing current user yields actor_id=None on the event.""" removed = [("user^alice", "role^course_editor", "course-v1^*")] - crum_mod = types.ModuleType("crum") - crum_mod.get_current_user = lambda: None - - role_assignment_data = mock.MagicMock(name="RoleAssignmentEventData") - events_data = types.ModuleType("openedx_events.authz.data") - events_data.RoleAssignmentData = role_assignment_data - - signal = mock.MagicMock(name="ROLE_ASSIGNMENT_DELETED") - events_signals = types.ModuleType("openedx_events.authz.signals") - events_signals.ROLE_ASSIGNMENT_DELETED = signal - - with mock.patch.dict( - sys.modules, - { - "crum": crum_mod, - "openedx_events.authz.data": events_data, - "openedx_events.authz.signals": events_signals, - }, + with ( + mock.patch("openedx_authz.engine.renderer.get_current_user", return_value=None), + mock.patch("openedx_authz.engine.renderer.RoleAssignmentEventData") as role_assignment_data, + mock.patch("openedx_authz.engine.renderer.ROLE_ASSIGNMENT_DELETED"), ): SchemaApplier._emit_assignment_deleted(removed) # pylint: disable=protected-access From 9cdeeae6cc9ba64a24645590a41f1bb1c0180118 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Mon, 5 Oct 2026 16:31:50 -0600 Subject: [PATCH 3/3] squash!: Improve redability --- src/openedx_authz/engine/renderer.py | 48 ++++++++++++++-------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/openedx_authz/engine/renderer.py b/src/openedx_authz/engine/renderer.py index 463fb89b..87b85dc0 100644 --- a/src/openedx_authz/engine/renderer.py +++ b/src/openedx_authz/engine/renderer.py @@ -62,7 +62,7 @@ from openedx_authz.engine.enforcer import AuthzEnforcer from openedx_authz.engine.schema.exceptions import SchemaApplyError from openedx_authz.engine.schema.types import CompiledSchema, RoleDefinition -from openedx_authz.models import schema as m +from openedx_authz.models import schema as schema_models from openedx_authz.models.core import RoleAssignmentAudit logger = logging.getLogger(__name__) @@ -443,7 +443,7 @@ def _diff_definitions(self, schema: CompiledSchema) -> dict[str, DefinitionDiff] return { "categories": self._diff_kind( {cid: compiled.definition for cid, compiled in schema.categories.items()}, - {obj.category_id: obj for obj in m.AuthzPermissionCategory.objects.all()}, + {obj.category_id: obj for obj in schema_models.AuthzPermissionCategory.objects.all()}, lambda definition, obj: ( definition.display_name == obj.display_name and (definition.description or "") == obj.description @@ -454,7 +454,7 @@ def _diff_definitions(self, schema: CompiledSchema) -> dict[str, DefinitionDiff] {pid: compiled.definition for pid, compiled in schema.permissions.items()}, { f"{obj.namespace}.{obj.name}": obj - for obj in m.AuthzPermissionDefinition.objects.select_related("category") + for obj in schema_models.AuthzPermissionDefinition.objects.select_related("category") }, lambda definition, obj: ( definition.display_name == obj.display_name @@ -466,7 +466,7 @@ def _diff_definitions(self, schema: CompiledSchema) -> dict[str, DefinitionDiff] ), "roles": self._diff_kind( {rid: compiled.definition for rid, compiled in schema.roles.items()}, - {obj.role_id: obj for obj in m.AuthzRoleDefinition.objects.all()}, + {obj.role_id: obj for obj in schema_models.AuthzRoleDefinition.objects.all()}, lambda definition, obj: ( definition.display_name == obj.display_name and (definition.description or "") == obj.description @@ -481,7 +481,7 @@ def _diff_definitions(self, schema: CompiledSchema) -> dict[str, DefinitionDiff] self._grant_key( grant.role.role_id, f"{grant.permission.namespace}.{grant.permission.name}", grant.scope ): None - for grant in m.AuthzRolePermission.objects.select_related("role", "permission") + for grant in schema_models.AuthzRolePermission.objects.select_related("role", "permission") }, lambda _compiled, _stored: True, ), @@ -529,7 +529,7 @@ def _managed_rows() -> set[PolicyRow]: f"{grant.permission.namespace}.{grant.permission.name}", grant.scope, ) - for grant in m.AuthzRolePermission.objects.select_related("role", "permission") + for grant in schema_models.AuthzRolePermission.objects.select_related("role", "permission") } def _store_sources(self, schema: CompiledSchema) -> None: @@ -552,7 +552,7 @@ def source_obj(record): cached = source_cache.get(key) if cached is not None: return cached - obj, _ = m.AuthzSchemaSource.objects.update_or_create( + obj, _ = schema_models.AuthzSchemaSource.objects.update_or_create( distribution=record.distribution, module=record.module, defaults={ @@ -569,7 +569,7 @@ def source_obj(record): category_objs: dict[str, object] = {} for cid, compiled in schema.categories.items(): definition = compiled.definition - obj, _ = m.AuthzPermissionCategory.objects.update_or_create( + obj, _ = schema_models.AuthzPermissionCategory.objects.update_or_create( category_id=definition.id, defaults={ "display_name": definition.display_name, @@ -579,15 +579,15 @@ def source_obj(record): ) category_objs[cid] = obj for record in compiled.sources: - m.AuthzCategorySource.objects.update_or_create( - category=obj, source=source_obj(record), defaults={"origin_kind": m.OriginKind.BASE} + schema_models.AuthzCategorySource.objects.update_or_create( + category=obj, source=source_obj(record), defaults={"origin_kind": schema_models.OriginKind.BASE} ) # Permissions. permission_objs: dict[str, object] = {} for pid, compiled in schema.permissions.items(): definition = compiled.definition - obj, _ = m.AuthzPermissionDefinition.objects.update_or_create( + obj, _ = schema_models.AuthzPermissionDefinition.objects.update_or_create( namespace=definition.namespace, name=definition.name, defaults={ @@ -600,15 +600,15 @@ def source_obj(record): ) permission_objs[pid] = obj for record in compiled.sources: - m.AuthzPermissionSource.objects.update_or_create( - permission=obj, source=source_obj(record), defaults={"origin_kind": m.OriginKind.BASE} + schema_models.AuthzPermissionSource.objects.update_or_create( + permission=obj, source=source_obj(record), defaults={"origin_kind": schema_models.OriginKind.BASE} ) # Roles. role_objs: dict[str, object] = {} for rid, compiled in schema.roles.items(): definition = compiled.definition - obj, _ = m.AuthzRoleDefinition.objects.update_or_create( + obj, _ = schema_models.AuthzRoleDefinition.objects.update_or_create( role_id=definition.id, defaults={ "display_name": definition.display_name, @@ -620,8 +620,8 @@ def source_obj(record): ) role_objs[rid] = obj for record in compiled.sources: - m.AuthzRoleSource.objects.update_or_create( - role=obj, source=source_obj(record), defaults={"origin_kind": m.OriginKind.BASE} + schema_models.AuthzRoleSource.objects.update_or_create( + role=obj, source=source_obj(record), defaults={"origin_kind": schema_models.OriginKind.BASE} ) # Role-permission grants (one per rendered role/permission/scope triple). @@ -636,18 +636,18 @@ def source_obj(record): permission_obj = permission_objs.get(perm_id) if permission_obj is None: continue # validated away in practice; skip defensively - grant, _ = m.AuthzRolePermission.objects.update_or_create( + grant, _ = schema_models.AuthzRolePermission.objects.update_or_create( role=role_obj, permission=permission_obj, scope=scope ) live_grant_ids.add(grant.pk) for rel in schema.role_permission_sources.get((rid, perm_id), []): - m.AuthzRolePermissionSource.objects.update_or_create( + schema_models.AuthzRolePermissionSource.objects.update_or_create( role_permission=grant, source=source_obj(rel.source), defaults={"origin_kind": rel.origin_kind, "priority": rel.priority}, ) - self._prune_definitions(m, schema, live_grant_ids) + self._prune_definitions(schema, live_grant_ids) logger.info( "Authz schema apply: persisted %d role(s), %d permission(s), %d category(ies).", @@ -657,7 +657,7 @@ def source_obj(record): ) @staticmethod - def _prune_definitions(m, schema: CompiledSchema, live_grant_ids: set[int]) -> None: + def _prune_definitions(schema: CompiledSchema, live_grant_ids: set[int]) -> None: """Delete definition/source rows the compiled schema no longer contains. Removes stale role-permission grants, roles, permissions, and categories @@ -670,17 +670,17 @@ def _prune_definitions(m, schema: CompiledSchema, live_grant_ids: set[int]) -> N then roles and permissions, then categories. """ # Stale role-permission grants: any grant not re-created this run. - m.AuthzRolePermission.objects.exclude(pk__in=live_grant_ids).delete() + schema_models.AuthzRolePermission.objects.exclude(pk__in=live_grant_ids).delete() live_role_ids = {compiled.definition.id for compiled in schema.roles.values()} - m.AuthzRoleDefinition.objects.exclude(role_id__in=live_role_ids).delete() + schema_models.AuthzRoleDefinition.objects.exclude(role_id__in=live_role_ids).delete() live_permission_keys = { (compiled.definition.namespace, compiled.definition.name) for compiled in schema.permissions.values() } - for permission_obj in m.AuthzPermissionDefinition.objects.all(): + for permission_obj in schema_models.AuthzPermissionDefinition.objects.all(): if (permission_obj.namespace, permission_obj.name) not in live_permission_keys: permission_obj.delete() live_category_ids = {compiled.definition.id for compiled in schema.categories.values()} - m.AuthzPermissionCategory.objects.exclude(category_id__in=live_category_ids).delete() + schema_models.AuthzPermissionCategory.objects.exclude(category_id__in=live_category_ids).delete()