docs: ADR Persist Custom Roles in the Authorization Definition Tables by rodmgwgu · Pull Request #603 · openedx/openedx-authz

rodmgwgu · 2026-11-05T17:33:22Z

Closes #467

Problem

Custom roles need to be stored so administrators can create, copy, edit, and remove them, and so those changes keep the Casbin policy in sync. ADR 0016 requires storing them and ADR 0018 sets naming and preservation rules, but no ADR says where they live. The ADR 0025 definition tables can hold them, but a schema deployment currently prunes every role and grant the compiled schema does not declare, so it would delete custom roles.

Approach

Added ADR 0035, which defines:
- Custom roles and their grants stored in the existing role-definition and role-permission tables, with a definition_kind of user_defined, matching ADR 0028.
- Global role IDs with the reserved custom_ prefix, generated from the display name and never changed. Schema validation rejects static role IDs with that prefix.
- Definition rows and p rows updated in one transaction, using the same lock as schema apply.
- Copying a role copies its permissions only.
- Removing a role with assignments requires choosing to remove them or reassign them to another role.
- Schema deployments skip custom roles. When a deployment removes a permission a custom role grants, it drops the grant and warns.
Added an addendum to ADR 0025 pointing to ADR 0035.

Manual testing instructions

Docs only. Both files parse with docutils.

Rollback plan

Revert this PR. Docs only. No code or migration.

Retro compatibility

No behavior change. This PR only records the decision. The implementation will follow in separate PRs.

AI Usage

Kiro assisted with research and ADR authoring, with human guidance on the decisions and validation against the existing ADRs.

Merge checklist:

Changelog record added — not needed for docs
Documentation updated (not only docstrings)
Fixup commits are squashed away
Unit tests added/updated — not applicable
Manual testing instructions provided — not applicable
Noted any: Concerns, dependencies, migration issues, deadlines, tickets — references ADR 0034 from docs: add ADR for permission dependencies in the authorization schema #601

openedx-webhooks · 2026-11-05T17:33:28Z

Thanks for the pull request, @rodmgwgu!

This repository is currently maintained by @openedx/committers-openedx-authz.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

mariajgrimaldi · 2026-11-06T13:46:27Z

+deployments. These writes use the same lock as schema apply. Changes apply right away to every user
+who holds the role.


What kind of changes will we allow on a custom role? I thought custom roles would be immutable, which helps auditability and security. For example, if someone adds an editing permission to a role, every user who holds it can edit courses right away, and when someone asks why a user who couldn't edit yesterday can edit today, nothing explains it. Allowing changes to its permissions, or even its name, could lead to misleading behavior or security gaps, especially since ADR 0012 only audits assignments, so a change to a role definition wouldn't show up anywhere. Would it be better to keep custom roles immutable? What do you think of this?

I think making them immutable would surely simplify things and makes sense on a security standpoint. What I'm not sure is if product needs them to be mutable. I'll ask Anthony about this. FYI @gviedma-aulasneo

I think making the name immutable (in addition to the role id) makes sense, to avoid confusions and also to keep the name and id in sync.

However, editing the permissions should be allowed:

On the security side, this becomes responsibility of the site operator - We only allow superusers to do any changes over custom roles.

One important use case for this is for when there is a platform update and we add new permissions to the platform, a site operator may want to enable some of these new permissions on their existing custom roles.

mariajgrimaldi · 2026-11-06T13:58:51Z

+When a custom role has assignments, the request to remove it must say what happens to them, or it
+is rejected:
+
+* **Remove:** the assignments are deleted with the role.


We already audit each deleted assignment (ADR 0012), but the event only has operation, subject, role, scope and actor_id, so a deletion caused by removing the role looks the same as a manual removal. Should we record that the assignment was deleted because the role was removed?

Role addition and removals (and changes) will be tracked on a separate audit table (I'm working on defining that ADR).

Perhaps we could link these assignment audits to the role audits? what do you think?

#614 specifies how audit will work, including how assignment audits are related to role audits.

mariajgrimaldi · 2026-11-06T14:07:07Z

+* **Reassign:** each assignment moves to another role in the same scope. The target role must
+  support the scope's resource type. A user who already holds the target role in that scope keeps
+  a single assignment.


I understand the product value of this, but I don't think reassigning assignments is a good practice from a security perspective. For example, a role that only allows reading a course is assigned to a group of 20 users, and then those assignments are moved to a role that allows editing, so all 20 users can start editing even though nobody decided that for each of them. There are two things I'd like to understand:

Auditability today doesn't cover this kind of change, so someone trying to figure out why a user has this access won't find much.

Why is this better than creating a new role, assigning it explicitly, and then cleaning up roles with no assignments?

Some references I found for this:

The OWASP Authorization Cheat Sheet on least privilege.

GitHub moves users of a deleted custom role to the organization's base permissions, so access can only shrink.

In Google Cloud IAM, bindings to a deleted custom role stay but have no effect, and the role can be undeleted within 7 days.

I think this was a specific request from Product.
About auditability, it is considered and the ADR will be posted next week, I'll consider this so it clearly tracks this.
But I do see the security concern here.

Anthony Anderson, @gviedma-aulasneo what do you think?

I removed the reassign feature. It was a nice-to-have that can be removed for now.

BryanttV

Aside from Majo's comments, which should be reviewed by Product, I only have one additional comment. Thanks for your work on this!

BryanttV · 2026-11-09T22:48:53Z

ADR 0018 §4 (role identifier conflicts) needs to be reconciled with this ADR.

ADR 0018 explicitly leaves dynamic role naming out of scope (0018-authorization-schema-lifecycle.rst:65). It handles collisions at runtime: the dynamic role API rejects names used by static roles, and a deployment stops when a new static role conflicts with an existing dynamic role (:63-67).

This ADR sets the naming rule instead (reserved custom_ prefix, validated in the schema), which means that deployment-time conflicts can no longer occur. That changes the behavior 0018 describes, so I think we should make it explicit.

done, thanks!

carlos-marquez-wgu

Great work, thanks @rodmgwgu !

I have a couple of comments

carlos-marquez-wgu · 2026-11-13T17:27:17Z

+* The schema loader's prune step must skip custom roles and keep their grants, except grants for
+  permissions the schema removed.


This seems to contradict section 5

prune step will remove grants for permissions the schema removed only if the operators opts in (with the explicit option), otherwise the deployment stops

Reworded to not contradict section 5

carlos-marquez-wgu · 2026-11-13T17:38:08Z

+the display name converted to the role ID format (``[a-z][a-z0-9_]*``). The ID is set at creation and
+never changes, so renaming a role does not affect its assignments or policy rows. For example:
+
+* "Course Designer" becomes ``custom_course_designer``.


What happens when the role "Course Designer" is created, deleted, and then created again, does the second time will get the same id or is the old id reserved?

Asking specially for auditing purposes, if the new role gains the old id, we have two "different" custom roles being tracked with the same id

I think this also raises the question of how custom roles deletion happens, is that a hard a delete (rows gone for good), or will it be a soft delete (they get deactivated but will keep existing in their tables)?

Updated to cover these cases and specify soft delete and what happens when a new role tries to use the same name.

carlos-marquez-wgu · 2026-11-13T17:43:55Z

+
+* "Course Designer" becomes ``custom_course_designer``.
+* If that ID is taken, the next one is ``custom_course_designer_2``.
+* A name with no ASCII letters becomes ``custom_role``.


What if the name contains a few non-ASCII characters? Like "Diseñador de cursos", are the non-ASCII chars replaced by underscores or transliterated (disenador_de_cursos)?

Do we have to worry about max length for ID strings?

Specified these cases and max length. Thanks!

carlos-marquez-wgu · 2026-11-13T17:47:51Z

+* Static and custom role name conflicts are prevented by the ``custom_`` prefix rule instead of
+  detected at deployment, which supersedes the runtime conflict handling in `ADR 0018`_ section 4.
+
+This ADR does not decide who can manage custom roles or how to prevent privilege escalation, the


Only superusers will be able to manage custom roles, right? In that case I think we can drop the part This ADR does not decide who can manage custom roles, unless this has further discussion

Yes, that's is specified in the API ADR (#607), so it's out of scope for this ADR.

carlos-marquez-wgu · 2026-11-13T17:54:44Z

+deployments. These writes use the same lock as schema apply. Changes apply right away to every user
+who holds the role.
+
+Copying a role creates a custom role with the same resource type and grants. Assignments are not


Could we specify the copy of a role happens after such base role has its role extensions applied?

Something like this:

Copying a role creates a custom role with the same resource type and grants after extensions are applied.

Extensions are a schema-specific concept, which only applies on schema load. This copy would be a runtime, at this point the roles are already loaded with all their permissions assigned.

Updated to make this clear

carlos-marquez-wgu · 2026-11-13T17:56:15Z

+deployments. These writes use the same lock as schema apply. Changes apply right away to every user
+who holds the role.
+
+Copying a role creates a custom role with the same resource type and grants. Assignments are not


Could we copy a hidden role?

Also, can a custom role be marked as hidden in the future?

I guess the answer to both might be no, but if so, I am wondering if we should explicitly state the relationship with hidden roles and custom roles

In the API ADR (https://github.com/openedx/openedx-authz/pull/607/changes), we specify that the copy process is actually just a create call done by the frontend copying an existing role.

This means that only non-hidden roles can be copied.

Custom roles should not be hidden.

Updated to clarify this. Thanks!

docs: ADR Persist Custom Roles in the Authorization Definition Tables

68657e7

openedx-webhooks added open-source-contribution PR author is not from Axim or 2U core contributor PR author is a Core Contributor (who may or may not have write access to this repo). labels Nov 5, 2026

openedx-webhooks added this to Contributions Nov 5, 2026

github-project-automation Bot moved this to Needs Triage in Contributions Nov 5, 2026

rodmgwgu requested review from BryanttV, carlos-marquez-wgu, mariajgrimaldi and wgu-taylor-payne November 5, 2026 17:37

rodmgwgu commented Nov 5, 2026

View reviewed changes

Comment thread docs/decisions/0035-custom-role-persistence.rst Resolved

rodmgwgu marked this pull request as ready for review November 5, 2026 17:40

rodmgwgu mentioned this pull request Nov 5, 2026

docs: ADR - API Contract for Custom Roles #607

Open

3 of 6 tasks

mariajgrimaldi reviewed Nov 6, 2026

View reviewed changes

squash!: Attend comments

525acf7

BryanttV reviewed Nov 9, 2026

View reviewed changes

rodmgwgu mentioned this pull request Nov 9, 2026

docs: ADR - Audit Trail for Custom Role Changes #614

Open

3 of 6 tasks

rodmgwgu added 2 commits November 12, 2026 16:12

squash!: Make explicit role naming prefix

90280b9

squash!: Removed reassignment feature

8d1f454

rodmgwgu requested review from BryanttV and mariajgrimaldi November 12, 2026 22:23

carlos-marquez-wgu reviewed Nov 13, 2026

View reviewed changes

squash!: Attend comments

60a3de3

rodmgwgu requested a review from carlos-marquez-wgu November 13, 2026 23:08

		deployments. These writes use the same lock as schema apply. Changes apply right away to every user
		who holds the role.

		* The schema loader's prune step must skip custom roles and keep their grants, except grants for
		permissions the schema removed.

Conversation

rodmgwgu commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Approach

Manual testing instructions

Rollback plan

Retro compatibility

AI Usage

Uh oh!

openedx-webhooks commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rodmgwgu Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

BryanttV left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carlos-marquez-wgu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carlos-marquez-wgu Nov 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carlos-marquez-wgu Nov 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

rodmgwgu commented Nov 5, 2026 •

edited

Loading

openedx-webhooks commented Nov 5, 2026 •

edited

Loading

rodmgwgu Nov 6, 2026 •

edited

Loading

carlos-marquez-wgu Nov 13, 2026 •

edited

Loading

carlos-marquez-wgu Nov 13, 2026 •

edited

Loading