From 853fe044b18a79b7fcf4e29df8bd4557b23a1b33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Fri, 16 Oct 2026 11:40:48 -0600 Subject: [PATCH 1/3] docs: Clarify that yaml priority only applies to role_extensions --- .../0017-static-authorization-schema.rst | 8 +- docs/decisions/0023-extend-static-roles.rst | 4 +- ...iority-applies-to-role-extensions-only.rst | 106 ++++++++++++++++++ docs/references/authorization-schema.rst | 4 +- src/openedx_authz/schema/authz-schema-v1.json | 2 +- 5 files changed, 117 insertions(+), 7 deletions(-) create mode 100644 docs/decisions/0031-priority-applies-to-role-extensions-only.rst diff --git a/docs/decisions/0017-static-authorization-schema.rst b/docs/decisions/0017-static-authorization-schema.rst index 9149b294..b3375d31 100644 --- a/docs/decisions/0017-static-authorization-schema.rst +++ b/docs/decisions/0017-static-authorization-schema.rst @@ -19,7 +19,7 @@ Decision 1. Schema format and boundary ============================= -The authz schema is a versioned YAML format for static permissions, permission categories, roles, and changes to existing roles. Every file declares ``schema_version`` and ``priority``, which apply to all definitions in that file. Priority is used when role extensions conflict. Open edX publishes a :download:`machine-readable JSON Schema <../../src/openedx_authz/schema/authz-schema-v1.json>` for this format so that editors, CI, and the compiler all apply the same field and validation rules. +The authz schema is a versioned YAML format for static permissions, permission categories, roles, and changes to existing roles. Every file declares ``schema_version`` and ``priority``, which apply to all definitions in that file. Priority is used only when role extensions conflict; it does not resolve conflicting base definitions (see `ADR 0031`_). Open edX publishes a :download:`machine-readable JSON Schema <../../src/openedx_authz/schema/authz-schema-v1.json>` for this format so that editors, CI, and the compiler all apply the same field and validation rules. The existing static role and permission definitions in Python modules and ``authz.policy`` will move into the schema. Once this migration is complete, the schema becomes the source for static definitions, so developers add a new role or permission there without duplicating it in Python constants or policy files. @@ -97,7 +97,7 @@ In these examples, ``courses.view_course`` appears under ``course_content`` in t p, role^course_observer, act^courses.view_course, course-v1^*, allow -The compiler also creates a row that links ``course_admin`` to ``courses.delete_course``. Priority resolves conflicts between role extensions; it does not apply to individual permissions or control how roles or permissions appear in the UI. +The compiler also creates a row that links ``course_admin`` to ``courses.delete_course``. Priority resolves conflicts between role extensions; it does not resolve conflicting base definitions (`ADR 0031`_), does not apply to individual permissions, and does not control how roles or permissions appear in the UI. The schema contains static definitions, while user assignments and user-defined roles stay in the application database. Casbin's ``model.conf`` and matcher also remain owned by ``openedx-authz``. @@ -119,7 +119,7 @@ After loading every file, validation checks the combined definitions. It rejects * references to permissions or categories that do not exist; * a role extension whose role does not exist in the combined definitions; * a role used in a scope where one of its permissions cannot apply; -* conflicting static definitions; +* conflicting static (base) definitions, regardless of their priority (`ADR 0031`_); * conflicting role extensions with the same priority; and * unsupported combinations of schema versions. @@ -160,12 +160,14 @@ References * `ADR 0016`_ * `ADR 0023`_ +* `ADR 0031`_ * `Casbin adapters`_ * `JSON Schema`_ * `Paragon icons`_ .. _ADR 0016: 0016-static-and-dynamic-roles.rst .. _ADR 0023: https://github.com/openedx/openedx-authz/pull/430 +.. _ADR 0031: 0031-priority-applies-to-role-extensions-only.rst .. _Casbin adapters: https://v3.casbin.org/docs/adapters .. _JSON Schema: https://json-schema.org/draft/2020-12/json-schema-core .. _Paragon icons: https://paragon-openedx.netlify.app/components/icon/ diff --git a/docs/decisions/0023-extend-static-roles.rst b/docs/decisions/0023-extend-static-roles.rst index 5df9a872..bb8f7bf8 100644 --- a/docs/decisions/0023-extend-static-roles.rst +++ b/docs/decisions/0023-extend-static-roles.rst @@ -56,7 +56,7 @@ Setting ``hidden: true`` removes the role from the normal API results used to di The compiler resolves extensions after it loads every static role and permission. It rejects an extension when the target role or one of the permissions does not exist. Adding a permission that the role already has or removing one it does not have produces a warning and leaves the result unchanged. -Several applications or deployment files may extend the same role. Changes to different fields are combined, while priority resolves changes to the same metadata field or permission. If two contributions with the same priority disagree, validation stops before the database changes. +Several applications or deployment files may extend the same role. Changes to different fields are combined, while priority resolves changes to the same metadata field or permission. If two contributions with the same priority disagree, validation stops before the database changes. Priority resolves conflicts between extensions only; it does not let an extension or a competing base definition replace a role's base definition, which stays single-source-of-truth (`ADR 0031`_). 4. Tutor patch ============== @@ -131,10 +131,12 @@ References * `ADR 0017`_ * `ADR 0018`_ * `ADR 0019`_ +* `ADR 0031`_ * :ref:`Authorization Schema Reference` * `Tutor plugin development`_ .. _ADR 0017: 0017-static-authorization-schema.rst .. _ADR 0018: 0018-authorization-schema-lifecycle.rst .. _ADR 0019: 0019-authorization-schema-discovery.rst +.. _ADR 0031: 0031-priority-applies-to-role-extensions-only.rst .. _Tutor plugin development: https://docs.tutor.edly.io/plugins/v0/gettingstarted.html diff --git a/docs/decisions/0031-priority-applies-to-role-extensions-only.rst b/docs/decisions/0031-priority-applies-to-role-extensions-only.rst new file mode 100644 index 00000000..f948b6f0 --- /dev/null +++ b/docs/decisions/0031-priority-applies-to-role-extensions-only.rst @@ -0,0 +1,106 @@ +0031: Priority Applies Only to Role Extensions, Not Base Definitions +################################################################### + +Status +****** + +**Draft** + +Context +******* + +The authorization schema is contributed as YAML files. Every file declares a +``priority`` number alongside its ``schema_version``, and that number applies to +all definitions in the file. `ADR 0017`_ and `ADR 0023`_ introduced ``priority`` +for one purpose: to resolve conflicts between ``role_extensions``. When several +files change the same metadata field or the same permission on the same existing +role, the highest priority wins; an equal-priority disagreement is rejected. + +Because ``priority`` sits at the top of every file and visually governs +everything below it, the YAML structure invites a reasonable but incorrect +assumption: that priority also decides a winner when two files declare the same +*base definition* differently. This ADR records that it does not, and explains +why, so the intent is visible to anyone reading the schema format later. + +Two kinds of contribution matter here, and the distinction is the whole point: + +* A **base definition** is a complete, self-contained declaration of a + permission category, a permission, or a role. It is the single source of truth + for that thing: a role's base definition *is* its full set of granted + permissions and its display metadata. +* A **role extension** is an explicit, partial change to an existing base role. + It adds or removes individual permissions, or replaces individual display + fields. Priority exists to decide which change wins when two extensions touch + the same field or the same permission. + +Decision +******** + +Priority applies only to ``role_extensions``. It does **not** resolve conflicts +between base definitions. + +Two files that declare the same permission category, permission, or role +identifier with differing content are a conflict, and validation rejects that +conflict as an error no matter what priorities the files declare. (Two files that +declare the identical base definition are not a conflict; that remains a +harmless duplicate.) Resolving such a disagreement is a deliberate authoring +decision, not something a priority number should settle silently. + +We reach this decision for three reasons: + +1. **Safety of access-control definitions.** A base definition is a complete + grant of access. If priority could pick a winner, one file could silently + replace another file's entire role — including the full set of permissions it + grants — and the replaced definition would simply have no effect. A change + that broadens or narrows what a role can do should be explicit and reviewable, + never the quiet side effect of one file declaring a larger number than + another. + +2. **No need for it.** Deployments and applications adapt existing roles through + ``role_extensions``, which exist precisely so a contributor can change a role + without owning and restating its full definition (`ADR 0023`_). There is no + requirement for two files to each fully redefine the same base definition and + have one of them win. + +3. **Base definitions stay a single source of truth.** A category, permission, + or role is defined once; extensions are the mechanism for modifying it. + Keeping that boundary sharp preserves clear provenance for every granted + permission (`ADR 0025`_) and keeps the extension mechanism meaningful rather + than redundant. + +There is also a conceptual mismatch that makes "apply the same priority rule to +base definitions" less tidy than it sounds. Extension resolution is granular: it +compares one field or one permission at a time, so two files can change different +parts of the same role and both take effect. A base definition has no such +granularity — it would have to be replaced as a whole. Priority over base +definitions would therefore be an all-or-nothing override, which is exactly the +behavior the first reason argues against. + +Wherever ``priority`` is documented, the text states that it governs role +extensions only, so a reader of the YAML format does not infer that it also +ranks competing base definitions. + +Consequences +************ + +* Base definitions remain single-source-of-truth. A disagreement between two + base definitions of the same category, permission, or role is surfaced as an + error for an author to resolve, rather than resolved implicitly by priority. +* To adapt an existing role, contributors use ``role_extensions`` rather than a + second, higher-priority base definition of the same role. +* Documentation of the ``priority`` field states plainly that it applies to role + extensions only. +* If a future need ever arises to replace a whole base definition from a separate + file, it will be introduced as its own explicitly designed and reviewable + mechanism, not as an implicit consequence of priority. + +References +********** + +* `ADR 0017`_ +* `ADR 0023`_ +* `ADR 0025`_ + +.. _ADR 0017: 0017-static-authorization-schema.rst +.. _ADR 0023: 0023-extend-static-roles.rst +.. _ADR 0025: 0025-authorization-schema-source-tracking.rst diff --git a/docs/references/authorization-schema.rst b/docs/references/authorization-schema.rst index 1312898c..136d59b6 100644 --- a/docs/references/authorization-schema.rst +++ b/docs/references/authorization-schema.rst @@ -84,9 +84,9 @@ The version of the YAML format used by the file. Write it as a quoted ``major.mi ``priority`` ============ -An integer used when several files extend the same role and change the same field or permission. A higher number takes precedence. Contributions with the same priority may be combined when they agree or affect different fields, but conflicting values at the same priority fail validation. +An integer used only when several files extend the same role through ``role_extensions`` and change the same field or permission. A higher number takes precedence. Contributions with the same priority may be combined when they agree or affect different fields, but conflicting values at the same priority fail validation. -Priority does not control the order shown in a user interface. Clients may sort roles, permissions, and categories for their own presentation. +Priority applies to role extensions only. It does not resolve conflicting base definitions: two files that declare the same permission category, permission, or role ID with different content are rejected regardless of their priorities. Priority also does not control the order shown in a user interface. Clients may sort roles, permissions, and categories for their own presentation. ``permission_categories`` ========================= diff --git a/src/openedx_authz/schema/authz-schema-v1.json b/src/openedx_authz/schema/authz-schema-v1.json index 5f50cfc1..111854bb 100644 --- a/src/openedx_authz/schema/authz-schema-v1.json +++ b/src/openedx_authz/schema/authz-schema-v1.json @@ -16,7 +16,7 @@ }, "priority": { "title": "priority", - "description": "The precedence used when contributions extend the same role. A higher number takes precedence when extensions conflict.", + "description": "The precedence used only when role_extensions contributions change the same role. A higher number takes precedence when extensions conflict. Priority does not resolve conflicting base definitions (permission categories, permissions, or roles); conflicting base definitions are rejected regardless of priority.", "type": "integer", "examples": [100] }, From d042d9135f9fb3479a6c361409d16edd2066dab9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Fri, 16 Oct 2026 12:06:40 -0600 Subject: [PATCH 2/3] fix: reject conflicting base definitions regardless of priority --- .../engine/schema/compilation.py | 67 ++++----- .../tests/schema/test_compilation.py | 133 +++++++++--------- 2 files changed, 92 insertions(+), 108 deletions(-) diff --git a/src/openedx_authz/engine/schema/compilation.py b/src/openedx_authz/engine/schema/compilation.py index 84b8a65e..f27d9a9b 100644 --- a/src/openedx_authz/engine/schema/compilation.py +++ b/src/openedx_authz/engine/schema/compilation.py @@ -112,10 +112,11 @@ def compile(self, documents: list[SchemaDocument]) -> CompiledSchema: ) def _collect(self, documents: list[SchemaDocument], kind: DefinitionKind, key) -> dict[str, _Tracked]: - """Gather base definitions keyed by identifier, resolving by priority. + """Gather base definitions keyed by identifier. - Higher priority wins on conflict; equal priority with differing content - raises; identical duplicates merge their sources. + Identical duplicates merge their sources; differing definitions for the + same identifier are a conflict and raise, regardless of priority + (ADR 0031). Priority applies only to role extensions. """ tracked: dict[str, _Tracked] = {} for document in documents: @@ -130,7 +131,7 @@ def _collect(self, documents: list[SchemaDocument], kind: DefinitionKind, key) - ) continue - tracked[identifier] = self._resolve_priority( + tracked[identifier] = self._merge_base_definition( kind.label, identifier, existing=existing, @@ -142,7 +143,7 @@ def _collect(self, documents: list[SchemaDocument], kind: DefinitionKind, key) - ) return tracked - def _resolve_priority( + def _merge_base_definition( self, kind: str, identifier: str, @@ -150,41 +151,26 @@ def _resolve_priority( existing: _Tracked, incoming: _Tracked, ) -> _Tracked: - """Resolve two competing definitions for one identifier by priority. - - Single responsibility: decide which of two :class:`_Tracked` entries for - the same identifier survives. - - * Identical definitions merge their sources (both files contributed the - same thing). - * Otherwise higher priority wins; the loser is warned about (ADR 0017 - §4) so a valid-but-ineffective file does not look like it took effect. - * Equal priority with disagreeing definitions is unresolvable and raises - :class:`SchemaCompileError` so deployment stops before the database - changes. - - Each ``_Tracked`` carries a single source; ``existing`` and ``incoming`` - are symmetric, so this also serves conflicts between extension - contributions where only priority (not load order) decides the winner. + """Combine two base definitions for the same identifier. + + A base definition is a single source of truth, so ``priority`` does not + pick a winner between two of them (ADR 0031). + + * Identical definitions merge their sources: both files declared the + same thing, so the result records both as contributors. + * Differing definitions are an unresolvable conflict and raise + :class:`SchemaCompileError`, regardless of priority, so deployment + stops before the database changes. Whole-set validation rejects this + case first; raising here keeps the compiler safe on a direct call. """ if existing.definition == incoming.definition: return replace(existing, sources=existing.sources + incoming.sources) - if incoming.priority == existing.priority: - raise SchemaCompileError( - f"Conflicting {kind} definition for {identifier!r} at equal priority " - f"{incoming.priority} " - f"({existing.sources[0].source_id} vs {incoming.sources[0].source_id})." - ) - winner, loser = (incoming, existing) if incoming.priority > existing.priority else (existing, incoming) - self._warn_discarded( - kind, - identifier, - loser=loser.sources[0], - loser_priority=loser.priority, - winner=winner.sources[0], - winner_priority=winner.priority, + raise SchemaCompileError( + f"Conflicting {kind} definition for {identifier!r} " + f"({existing.sources[0].source_id} vs {incoming.sources[0].source_id}). " + f"Priority does not resolve base definitions (ADR 0031); " + f"use role_extensions to adapt an existing role." ) - return winner @staticmethod def _warn_discarded( @@ -390,10 +376,11 @@ def _resolve_contributions_based_on_priority( ) -> tuple[object, int, list[SourceRecord]]: """Pick the winning value among competing extension contributions. - The batch counterpart to :meth:`_resolve_priority`: where that method - decides between two base definitions pairwise, this decides among any - number of ``(value, priority, source)`` contributions to the same - extension field or permission. + This resolves extension contributions, where priority does decide a + winner (ADR 0023). It picks among any number of + ``(value, priority, source)`` contributions to the same extension field + or permission. Base definitions are handled separately by + :meth:`_merge_base_definition`, which does not use priority (ADR 0031). The highest priority wins. Every contribution in the top-priority group must agree; if they do not, ``on_tie(top_values)`` builds the message diff --git a/src/openedx_authz/tests/schema/test_compilation.py b/src/openedx_authz/tests/schema/test_compilation.py index d23ee835..b7654e77 100644 --- a/src/openedx_authz/tests/schema/test_compilation.py +++ b/src/openedx_authz/tests/schema/test_compilation.py @@ -102,7 +102,11 @@ def test_extension_sources_are_retained(self): class TestPriorityResolution: - """How priority resolves conflicting contributions (higher wins; ties fail).""" + """How priority resolves conflicting role-extension contributions (higher wins; ties fail). + + Priority applies to role extensions only; base-definition conflicts are + covered by :class:`TestBaseDefinitionConflicts` (ADR 0031). + """ def test_equal_priority_metadata_conflict_raises(self): """Two extensions setting the same field at equal priority is an error.""" @@ -133,19 +137,57 @@ def test_equal_priority_add_remove_conflict_raises(self): with pytest.raises(SchemaCompileError): SchemaCompiler().compile([_base(), add, rem]) + +class TestBaseDefinitionConflicts: + """Priority does not resolve conflicting base definitions (ADR 0031). + + A base definition (category, permission, or role) is a single source of + truth. Two files that declare the same identifier differently are an + unresolvable conflict regardless of priority; only identical declarations + merge. Contrast with :class:`TestPriorityResolution`, where priority does + pick a winner among role extensions. + """ + def test_conflicting_base_definition_equal_priority_raises(self): - """Two base definitions of the same role at equal priority is an error.""" + """Two differing base definitions at equal priority is an error.""" a = make_document("a", priority=100, roles=[role(rid="dup", display_name="A", permissions=())]) b = make_document("b", priority=100, roles=[role(rid="dup", display_name="B", permissions=())]) - with pytest.raises(SchemaCompileError): + with pytest.raises(SchemaCompileError, match="Conflicting role definition"): SchemaCompiler().compile([a, b]) - def test_higher_priority_base_definition_wins(self): - """The higher-priority base definition replaces the lower one.""" + def test_higher_priority_does_not_win_for_base_definitions(self): + """A higher-priority base definition does not replace a lower-priority one. + + Unlike role extensions, the disagreement is an unresolvable conflict and + raises regardless of priority. + """ + lo = make_document("lo", priority=100, roles=[role(rid="dup", display_name="Lo", permissions=())]) + hi = make_document("hi", priority=200, roles=[role(rid="dup", display_name="Hi", permissions=())]) + with pytest.raises(SchemaCompileError): + SchemaCompiler().compile([lo, hi]) + + def test_base_definition_conflict_raises_regardless_of_order(self): + """Discovery order must not change the outcome; both orders raise.""" lo = make_document("lo", priority=100, roles=[role(rid="dup", display_name="Lo", permissions=())]) hi = make_document("hi", priority=200, roles=[role(rid="dup", display_name="Hi", permissions=())]) - compiled = SchemaCompiler().compile([lo, hi]) - assert compiled.roles["dup"].definition.display_name == "Hi" + with pytest.raises(SchemaCompileError): + SchemaCompiler().compile([lo, hi]) + with pytest.raises(SchemaCompileError): + SchemaCompiler().compile([hi, lo]) + + def test_error_mentions_priority_does_not_apply(self): + """The conflict error points the author to role_extensions (ADR 0031).""" + lo = make_document("lo", priority=100, categories=[category("cat", display_name="Lo")]) + hi = make_document("hi", priority=200, categories=[category("cat", display_name="Hi")]) + with pytest.raises(SchemaCompileError, match="Priority does not resolve base definitions"): + SchemaCompiler().compile([lo, hi]) + + def test_identical_base_definitions_merge_regardless_of_priority(self): + """Identical declarations are not a conflict; their sources merge.""" + first = make_document("first", priority=100, roles=[role(rid="dup", permissions=())]) + second = make_document("second", priority=200, roles=[role(rid="dup", permissions=())]) + compiled = SchemaCompiler().compile([first, second]) + assert len(compiled.roles["dup"].sources) == 2 class TestProvenance: @@ -186,56 +228,20 @@ def test_removed_permission_has_no_provenance(self): class TestDiscardedContributionWarnings: - """Priority silently picks a winner; the loser must be reported. - - ADR 0017 §4 requires warning about contributions that do not take effect - because another file has a higher priority. A losing file is valid and was - loaded, so without a warning it looks like it applied. + """Priority silently picks a winner among role extensions; the loser must be reported. + + ADR 0017 §4 requires warning about extension contributions that do not take + effect because another file has a higher priority. A losing file is valid + and was loaded, so without a warning it looks like it applied. Priority does + not resolve base definitions (ADR 0031), so a losing base definition is not + a case here; conflicting base definitions raise instead (see + :class:`TestBaseDefinitionConflicts`). """ @staticmethod def _compile(*documents): return SchemaCompiler().compile(list(documents)) - def test_lower_priority_base_definition_warns(self, caplog): - """A base definition that loses on priority is reported as having no effect.""" - low = make_document("low", priority=100, roles=[role(rid="course_editor", display_name="Editor")]) - high = make_document("high", priority=200, roles=[role(rid="course_editor", display_name="Author")]) - - with caplog.at_level(logging.WARNING): - compiled = self._compile(high, low) - - assert compiled.roles["course_editor"].definition.display_name == "Author" - assert "has no effect" in caplog.text - assert make_source("low").source_id in caplog.text - assert make_source("high").source_id in caplog.text - - def test_warning_names_both_priorities(self, caplog): - """The discard warning names both the losing and winning priorities.""" - low = make_document("low", priority=100, roles=[role(rid="course_editor", display_name="Editor")]) - high = make_document("high", priority=200, roles=[role(rid="course_editor", display_name="Author")]) - - with caplog.at_level(logging.WARNING): - self._compile(low, high) - - assert "priority 100" in caplog.text - assert "priority 200" in caplog.text - - def test_warns_regardless_of_document_order(self, caplog): - """Discovery order must not decide whether the operator is told.""" - low = make_document("low", priority=100, roles=[role(rid="course_editor", display_name="Editor")]) - high = make_document("high", priority=200, roles=[role(rid="course_editor", display_name="Author")]) - - with caplog.at_level(logging.WARNING): - self._compile(low, high) - ascending = caplog.text - caplog.clear() - with caplog.at_level(logging.WARNING): - self._compile(high, low) - - assert "has no effect" in ascending - assert "has no effect" in caplog.text - def test_identical_duplicate_does_not_warn(self): """An identical definition merges sources; nothing is discarded.""" first = make_document("first", priority=100, roles=[role(rid="course_editor")]) @@ -245,17 +251,6 @@ def test_identical_duplicate_does_not_warn(self): assert len(compiled.roles["course_editor"].sources) == 2 - def test_uses_singular_kind_label(self, caplog): - """Messages say 'category', not the truncated attribute name.""" - low = make_document("low", priority=100, categories=[category("cat", display_name="Low")]) - high = make_document("high", priority=200, categories=[category("cat", display_name="High")]) - - with caplog.at_level(logging.WARNING): - self._compile(low, high) - - assert "category 'cat'" in caplog.text - assert "categorie" not in caplog.text - def test_conflict_error_uses_singular_kind_label(self): """A conflict error uses the singular kind label ('category').""" left = make_document("left", priority=100, categories=[category("cat", display_name="Left")]) @@ -377,12 +372,14 @@ def test_identical_duplicate_permissions_merge_sources(self): assert len(compiled.permissions["courses.view_course"].sources) == 2 - def test_lower_priority_base_definition_is_kept_out(self): - """The 'keep existing' branch: a later, lower-priority file loses.""" + def test_differing_base_definition_raises_even_though_validation_guards_it(self): + """Validation errors on this first; compilation must still raise, not pick a winner. + + Priority does not resolve base definitions (ADR 0031), so a direct call + to the compiler with two differing base definitions is a conflict. + """ high = make_document("high", priority=200, roles=[role(rid="course_editor", display_name="Author")]) low = make_document("low", priority=100, roles=[role(rid="course_editor", display_name="Editor")]) - compiled = SchemaCompiler().compile([high, low]) - - assert compiled.roles["course_editor"].definition.display_name == "Author" - assert [s.source_id for s in compiled.roles["course_editor"].sources] == [make_source("high").source_id] + with pytest.raises(SchemaCompileError): + SchemaCompiler().compile([high, low]) From ea79a3f675869525ebb8536ef326ae3048ba5fd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Fri, 16 Oct 2026 12:14:23 -0600 Subject: [PATCH 3/3] squash!: Update CHANGELOG --- CHANGELOG.rst | 14 ++++++++++++-- ...31-priority-applies-to-role-extensions-only.rst | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index cc5c1324..d2f888b9 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -32,8 +32,9 @@ Added removing roles that still have assignments, and repeatable ``--dir`` for CI and local runs. * Added ``role_extensions`` support: an application or deployment can add or remove permissions and replace the display metadata or ``hidden`` flag of an existing static role without copying its - definition. ``priority`` resolves conflicts; an unresolvable equal-priority conflict stops the run - before any database change (ADR 0023). + definition. ``priority`` resolves conflicts between extensions; an unresolvable equal-priority + conflict stops the run before any database change (ADR 0023). Priority does not resolve conflicting + base definitions (ADR 0031). * Added first-class tables for compiled definitions and their provenance, in migration ``0011_authz_schema_definitions``: permission categories, permission definitions, role definitions, role-permission grants, schema sources, and one source-link table per definition kind @@ -51,6 +52,15 @@ Added ``openedx_authz/engine/schema/paragon_icons.py`` and regenerated with ``make paragon_icons`` (ADR 0029). +Fixed +===== + +* The schema compiler no longer resolves conflicting base definitions by ``priority``. Two files + that declare the same permission category, permission, or role with differing content are now a + conflict regardless of priority, matching schema validation; only ``role_extensions`` are resolved + by priority. This aligns the compiler with ADR 0031 and removes a path where a higher-priority file + could silently replace a role's base definition. + Notes ===== diff --git a/docs/decisions/0031-priority-applies-to-role-extensions-only.rst b/docs/decisions/0031-priority-applies-to-role-extensions-only.rst index f948b6f0..56563d90 100644 --- a/docs/decisions/0031-priority-applies-to-role-extensions-only.rst +++ b/docs/decisions/0031-priority-applies-to-role-extensions-only.rst @@ -1,5 +1,5 @@ 0031: Priority Applies Only to Role Extensions, Not Base Definitions -################################################################### +#################################################################### Status ******