openedx / openedx/openedx-authz
Provide a typed exception surface (AuthzError hierarchy) at the public API boundary
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 0
- Forks
- 9
- Avg merge
- 13d 9h
- Merged PRs (30d)
- 9
Description
Summary
openedx_authz currently raises only Python/Django built-in exceptions from its public API. There is no library-owned exception module and no AuthzError base type. As a result, consumers cannot distinguish an authz-originated failure from an unrelated one without either coupling to the storage implementation (django.db.DatabaseError) or catching over-broad built-ins (ValueError, Exception). This forces every caller to make the same fail-open vs fail-closed decision by catching a storage-layer type the library never promised as part of its contract.
This proposes a small, backward-compatible exception taxonomy raised at the API boundary.
Current state (as of the installed package)
There is no exceptions.py/errors.py module and no class *Error(Exception) base anywhere in the package. What the public API raises today:
- Raw built-ins:
ValueError— pervasive for scope / external-key / namespaced-key parsing (api/data.py,engine/utils.py,api/permissions.py).NotImplementedError— abstract-method stubs on the*Database classes.IntegrityError—models/authz_migration.py.- A bare
raise Exception("Failed to create ExtendedCasbinRule for the assignment")—api/roles.py.
- Django ORM
DoesNotExistre-raises:User.DoesNotExist,ContentLibrary.DoesNotExist,CourseOverview.DoesNotExist,Organization.DoesNotExist. - Backend/storage failures are not wrapped at all. The read path
get_user_role_assignments_per_scope_type → get_user_role_assignments → get_subject_role_assignmentsbottoms out at the Casbin enforcer'sExtendedAdapter, which readsCasbinRule.objects(a Django ORM read). ADatabaseErrorthere propagates raw to the caller. - Two result enums exist but are not exceptions:
MigrationErrorReason(StrEnum)andRoleOperationError(BaseEnum)are return-value discriminators, not raisable types.
So the gap is library-wide, not confined to one function.
Why this matters (concrete consumer impact)
A downstream consumer that needs the user's authz course roles must today write:
from django.db import DatabaseError
try:
assignments = get_user_role_assignments_per_scope_type(
user_external_key=username,
scope_types=(CourseOverviewData,),
)
except DatabaseError:
... # degrade / deny
Problems with this:
- Leaky abstraction. The consumer couples to the fact that authz happens to store policy in a Django-ORM-backed Casbin adapter. If openedx_authz ever changes storage (different adapter, cache layer, remote policy service),
except DatabaseErrorsilently stops catching the real failure — and the consumer's fail-open/fail-closed logic breaks with no signal. - The right error-handling policy differs by caller, and only the caller knows it. A search-filter consumer wants to fail open (degrade to a narrower result set rather than 500). An enforcement consumer ("can this user edit this course?") must fail closed (deny) — for it, silently swallowing a backend error is a security hole. The library must not make this decision internally; it should surface a typed error and let each call site choose. But to choose, the call site needs a stable type to catch.
- Validation errors are indistinguishable. A malformed scope/external key raises a bare
ValueErrorthat can't be told apart from any otherValueErrorbubbling up the stack without string-matching the message.
Real example: openedx/openedx-platform#39073 (Meilisearch Studio-search access filter) catches django.db.DatabaseError from get_user_role_assignments_per_scope_type precisely because no authz-owned type exists. It's the correct pragmatic choice today, but it hard-couples the search module to authz's storage internals.
Proposal
Add a small exception module (e.g. openedx_authz/exceptions.py) and raise these types at the public API boundary:
AuthzError(Exception)— base for everything the library raises intentionally.AuthzBackendError(AuthzError)— storage/enforcer failures. Raisedfromthe underlyingDatabaseError(and the bareExceptioninapi/roles.py) at the API boundary.AuthzValidationError(AuthzError)— malformed scope / external-key / namespaced-key input, replacing the rawValueErrors inapi/data.py/engine/utils.py/api/permissions.py.
Callers then write except AuthzBackendError / except AuthzValidationError and never import django.db.
Backward compatibility
Make the new types subclass the built-ins they replace so no existing except clause breaks:
class AuthzError(Exception): ...
class AuthzBackendError(AuthzError, DatabaseError): ... # existing `except DatabaseError` still catches
class AuthzValidationError(AuthzError, ValueError): ... # existing `except ValueError` still catches
New code catches the specific authz type; old code keeps working unchanged. This makes the change additive and low-risk.
Scope of the change
- Wrap the enforcer/adapter read path (
get_subject_role_assignmentsand theget_user_role_assignments*functions built on it) so backend failures surface asAuthzBackendError. - Replace raw input-validation
ValueErrors in the*Dataparsing paths withAuthzValidationError. - Replace the bare
raise Exception(...)inapi/roles.pywithAuthzBackendError. - Leave the
DoesNotExistre-raises as-is (those are legitimately Django model semantics) unless the maintainers prefer to wrap them too.
Alternatives considered
- Consumers keep catching
DatabaseError. Works today but is the leaky coupling described above; breaks on any storage change and offers nothing to enforcement callers who must fail closed. - Library masks errors internally (returns
[]/Noneon backend failure). Rejected: the library cannot know whether a given caller's context makes fail-open safe. Masking is a policy decision that belongs at the call site, not in the library.
Notes / open questions for maintainers
- The library is mid-ADR-series on the authz model (ADR-0016/0017, static vs dynamic roles). This exception taxonomy may be better folded into that design discussion than taken as a standalone patch — flagging so it can be routed appropriately.
- Naming (
AuthzErrorvsOpenedxAuthzError, etc.) is open; matching the repo's existing convention is fine. - The
RoleOperationError/MigrationErrorReasonenums suggest the library already models failure as data in some layers; a raisable exception surface is the complementary piece for the query/validation APIs.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by tracing the public API paths through api/data.py, engine/utils.py, api/permissions.py, api/roles.py, and the get_subject_role_assignments/get_user_role_assignments functions. Review the proposed exception names and inheritance against existing Django and Python exceptions, then check the ADR-0016/0017 context before choosing scope. Done means intentional failures have a stable library-owned exception surface while the stated backward-compatibility behavior remains intact.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- django, python
- Domain
- authorization, backend-api-design, security
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100