Skip to content

docs: add ADR 0028 API contract for the role and permission catalog - #508

Open
BryanttV wants to merge 1 commit into
mainfrom
bav/roles-catalog-api-adr
Open

BryanttV wants to merge 1 commit into
mainfrom
bav/roles-catalog-api-adr

Conversation

@BryanttV

@BryanttV BryanttV commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Description

Adds ADR 0028, which defines the response contract of GET /api/authz/v1/roles/ when queried by scope_type. ADR 0021 decided that this endpoint returns the definitions stored in the authz model, but left the exact response shape open. This ADR defines it.

The Roles and Permissions tab of the Admin Console renders a matrix (roles × permissions grouped by category) for one scope type at a time. Today the frontend hardcodes all of this (course/constants.ts, library/constants.ts). The matrix needs every permission of the scope type, including the ones a role does not grant, so a list of roles that only carries the permissions each one grants is not enough.

Key decisions

  • Query by scope_type (course | library, required) instead of scope, like /scopes/. The scope parameter is removed.
  • Authorization per scope type: courses.view_course_team for course and content_libraries.view_library_team for library, in any scope. This needs a new permission class, since DynamicScopePermission and AnyScopePermission cannot express it.
  • Normalized response: the categories and permissions catalogs are sent once, and each role references permissions by id. The client can build each matrix cell directly.
  • Pagination applies to roles only: every page carries the complete catalogs.
  • user_count is now calculated by scope type, not by a single scope.
  • Data source: the authz schema models, not raw Casbin rows. hidden roles (ADR 0023) are excluded.

Breaking change

This is a breaking change to GET /api/authz/v1/roles/ (scope becomes scope_type, user_count changes meaning, and the catalogs are added). The risk is low because the Admin Console only uses /roles/users/. The deviation from the compatibility promise of ADR 0021 is intentional and documented in the ADR.

Manual testing

Documentation only. To review, read docs/decisions/0028-api-contract-for-role-catalog.rst or check the docs.

Merge checklist

  • Documentation updated (not only docstrings)
  • Fixup commits are squashed away
  • Unit tests added/updated (n/a, docs only)
  • Manual testing instructions provided
  • Noted any: Concerns, dependencies, migration issues, deadlines, documented above)

Define the response of GET /api/authz/v1/roles/ queried by scope_type,
returning paginated roles with the categories and permissions catalogs
needed by the Admin Console roles and permissions matrix.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@openedx-webhooks 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 Sep 30, 2026
@openedx-webhooks

openedx-webhooks commented Sep 30, 2026 •

Copy link
Copy Markdown

Thanks for the pull request, @BryanttV!

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.

translated. Since the body depends on the request language, the response must vary on
``Accept-Language``.

Icons are Paragon icon names (validated by the schema, see `ADR 0026`_). The client maps

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to keep in mind: There is already a ADR 0026 that is not related to paragon icons, I'll change the number on my PR, this will need updating once my PR is merged.

@rodmgwgu rodmgwgu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looking good, Added some comments for consideration. Thanks!

``library`` (``ScopesTypeField``). The catalog describes what a scope type offers, not a
particular course or library, so a concrete scope is not needed.

* ``scope_type`` is required. Unlike ``/scopes/``, a request without it is invalid (400)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking that perhaps we should support a "scope_types" param instead, that accepts a list of scope types.

This for supporting future requirements, like listing the roles dynamically for assignments, when we enable support for roles with multiple scope types.


* ``scope_type`` is required. Unlike ``/scopes/``, a request without it is invalid (400)
because one matrix cannot mix course and library roles.
* The scope type is mapped to its scope namespace (``course`` to ``course-v1``, ``library``

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily for now, but to keep in mind: How would we handle this mapping if we introduce new scopes in the future? specially when the scope is not defined by authz itself?

Authorization
=============

The permission needed depends on the requested ``scope_type``, so a user cannot read the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change this to "scope_types", then should we check for permissions on all the requested scope types inclusive? or just filter the output to only what the user can view?

* A new permission class is needed for the per-scope-type authorization.
* ``definition_kind`` is reserved for user-defined roles. Their storage, the translation of
their names and any source detail remain out of scope.
* A role present in the Casbin policy but with no stored definition cannot be described. It

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if a role is present in Casbin but not stored, then we should not show it. Otherwise the filtering and pagination will become much more complex as we will need to query Casbin in addition to the DB.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core contributor PR author is a Core Contributor (who may or may not have write access to this repo). open-source-contribution PR author is not from Axim or 2U

Projects

Status: Needs Triage

Development

Successfully merging this pull request may close these issues.

Role and permission metadata (Catalog API)

3 participants