-
Notifications
You must be signed in to change notification settings - Fork 4.3k
[FC-0118] docs: add ADR for standardizing authentication patterns #38301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: docs/ADRs-axim_api_improvements
Are you sure you want to change the base?
Changes from all commits
a6a25d3
cfcf06d
1c14012
f3abbe7
b54c1d0
0ed80d7
39d9d17
2bcb928
030119a
6ee7a7c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,190 @@ | ||
| Standardize Authentication Patterns and Security Schemes | ||
| ======================================================== | ||
|
|
||
| :Status: Proposed | ||
| :Date: 2026-04-07 | ||
| :Deciders: Open edX Platform / API Working Group | ||
| :Technical Story: Open edX REST API Standards - Consistent authentication patterns and security scheme usage | ||
|
|
||
| Context | ||
| ======= | ||
|
|
||
| Open edX APIs have inconsistent authentication patterns and security scheme implementations: | ||
|
|
||
| * Multiple authentication mechanisms are enabled globally but not consistently applied | ||
| * OAuth2 and JWT are not separate mechanisms DOT issues JWTs as OAuth2 tokens, | ||
| validated by ``JwtAuthentication``. The deprecated ``BearerAuthentication`` | ||
| handles old Bearer tokens and must not be confused with this. | ||
| * Security scheme declarations don't match actual authentication behavior | ||
| * External integrators cannot reliably predict which authentication method to use | ||
| * Internal APIs mix authentication mechanisms without clear patterns | ||
|
|
||
| This inconsistency creates confusion for: | ||
| - External developers determining which auth method to implement | ||
| - Internal teams maintaining consistent authentication patterns | ||
| - Security reviews and compliance assessments | ||
| - Automated tools expecting predictable authentication | ||
|
|
||
| The codebase has two JWT issuance paths, both validated by ``JwtAuthentication``: | ||
|
|
||
| * ``create_jwt_token_dict()`` — wraps a DOT OAuth2 access token into a JWT (DB-backed, revocable, for external clients) | ||
| * ``create_jwt_for_user()`` — issues a JWT directly with no OAuth2 flow and no DB row (non-revocable, for internal service communication) | ||
|
|
||
| Decision | ||
| ======== | ||
|
|
||
| 1. **JWT authentication via** ``JwtAuthentication`` **MUST be the standard | ||
| authentication mechanism for all DRF API endpoints that take user-authenticated | ||
| requests**, per `OEP-0042`_. This excludes admin views, ``/oauth2/access_token/``, | ||
| and HMAC/webhook endpoints, which have their own authentication mechanisms. | ||
| 2. **Session authentication MUST also be used when** the expected client for an API | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "For all API access" would include admin views,
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @feanil: Would another way to say user-authenticated requests be password-authenticated or user-password-authenticated? I wasn't clear at first. As an aside, for Mobile Authentication, there were some endpoints that will accept JwtAuthentication only if it has grant type password. |
||
| is a Browser/MFE. This would be added alongside ``JwtAuthentication`` on the | ||
| endpoint — which is the platform default. | ||
| 3. **``BearerAuthentication`` and ``BearerAuthenticationAllowInactiveUser`` are | ||
| deprecated and MUST NOT be used in new code** | ||
| 4. **``OAuth2Authentication`` and ``OAuth2AuthenticationAllowInactiveUser`` are | ||
| deprecated aliases for** ``BearerAuthentication`` **and MUST NOT be used in new code** | ||
| 5. **All new APIs MUST follow these authentication patterns based on use case** | ||
| 6. **Existing APIs MUST be audited and updated to remove** ``BearerAuthentication`` | ||
|
|
||
| Implementation requirements: | ||
|
|
||
| * All APIs: ``JwtAuthentication`` (+ ``SessionAuthentication`` where appropriate) | ||
| * ``BearerAuthentication`` / ``BearerAuthenticationAllowInactiveUser``: remove from all endpoints | ||
| * ``OAuth2Authentication`` / ``OAuth2AuthenticationAllowInactiveUser``: remove once external repos migrate | ||
|
|
||
| Consequences | ||
| ============ | ||
|
|
||
| * Pros | ||
|
|
||
| * Clear, predictable authentication patterns for different API use cases | ||
| * Improved security through proper separation of auth mechanisms | ||
| * Aligns with OEP-0042 — removes deprecated ``BearerAuthentication`` from active use | ||
| * Easier integration for external developers (single standard: JWT) | ||
| * Simplified internal service communication (same ``JwtAuthentication`` class) | ||
| * Better browser experience (session-based auth) | ||
|
|
||
| * Cons / Costs | ||
|
|
||
| * Existing APIs need audit and potential refactoring to match patterns | ||
| * Teams need to understand and implement proper authentication choices(where to use JWT or session) | ||
| * External clients still using Bearer tokens must migrate to JWT | ||
| * Migration effort for services currently using mixed authentication | ||
| * Depending on configs, Bearer tokens last ~2 weeks; JWTs expire in ~1 hour — long-running jobs that reuse | ||
| a token without checking expiry will start failing after migration | ||
|
|
||
| Relevance in edx-platform | ||
| ========================= | ||
|
|
||
| * **OAuth2/DOT**: LMS uses Django OAuth Toolkit at ``/oauth2/`` | ||
| (``lms/urls.py``, ``openedx/core/djangoapps/oauth_dispatch``). Settings include | ||
| ``OAUTH2_PROVIDER_APPLICATION_MODEL``, ``OAUTH2_VALIDATOR_CLASS`` (e.g. | ||
| ``EdxOAuth2Validator``). DOT issues JWTs as access tokens via ``create_jwt_token_dict()``. | ||
| * **Current API auth**: ``openedx/core/lib/api/view_utils.view_auth_classes`` | ||
| configures both **JWT** and **Bearer** (deprecated) and session across 49+ files: | ||
|
|
||
| .. code-block:: python | ||
|
|
||
| # openedx/core/lib/api/view_utils.py (current — violates OEP-0042) | ||
| func_or_class.authentication_classes = ( | ||
| JwtAuthentication, | ||
| BearerAuthenticationAllowInactiveUser, # deprecated per OEP-0042 | ||
| SessionAuthenticationAllowInactiveUser | ||
| ) | ||
|
|
||
| * **Bearer auth**: ``openedx/core/lib/api/authentication.py`` implements | ||
| ``BearerAuthentication`` / ``BearerAuthenticationAllowInactiveUser`` using | ||
| ``oauth2_provider`` (DOT) for access token validation. This is the deprecated path. | ||
|
|
||
| Code examples (authentication patterns by use case) | ||
| =================================================== | ||
|
|
||
| * **Standard API (JWT + Session):** | ||
|
|
||
| Example: ``lms/djangoapps/course_home_api/dates/views.py (DatesTabView)`` | ||
| (`permalink <https://github.com/openedx/openedx-platform/blob/be3fc121148587fb1da507519534063c89387091/lms/djangoapps/course_home_api/dates/views.py#L68-L72>`_) | ||
|
|
||
| .. code-block:: python | ||
|
|
||
| # Current state | ||
| authentication_classes = ( | ||
| JwtAuthentication, | ||
| BearerAuthenticationAllowInactiveUser, # to be removed per Decision #3 | ||
| SessionAuthenticationAllowInactiveUser, | ||
| ) | ||
|
|
||
| # Target state (after BearerAuth removal) | ||
| authentication_classes = ( | ||
| JwtAuthentication, | ||
| SessionAuthenticationAllowInactiveUser, | ||
| ) | ||
|
|
||
| Note: use ``SessionAuthenticationAllowInactiveUser`` or ``SessionAuthentication`` based on usecase of the API. | ||
|
|
||
| * **API intended for MFE/Browser clients:** | ||
|
|
||
| Example: ``lms/djangoapps/teams/views.py (TeamsDashboardView)`` | ||
| (`permalink <https://github.com/openedx/openedx-platform/blob/be3fc121148587fb1da507519534063c89387091/lms/djangoapps/teams/views.py#L114-L122>`_) | ||
|
|
||
| .. code-block:: python | ||
|
|
||
| # Current state | ||
| authentication_classes = ( | ||
| BearerAuthenticationAllowInactiveUser, # to be removed per Decision #3 | ||
| SessionAuthenticationAllowInactiveUser, | ||
| ) | ||
|
|
||
| # Target state (BearerAuth removed, JwtAuthentication added per Decision #1) | ||
| authentication_classes = ( | ||
| JwtAuthentication, | ||
| SessionAuthenticationAllowInactiveUser, | ||
| ) | ||
|
|
||
| Note: use ``SessionAuthenticationAllowInactiveUser`` or ``SessionAuthentication`` based on usecase of the API. | ||
|
|
||
| Implementation Notes | ||
| ==================== | ||
|
|
||
| * Supporting both ``JwtAuthentication`` and ``SessionAuthentication`` on the same | ||
| endpoint is acceptable — this is already the platform default in | ||
| ``openedx/envs/common.py`` (``DEFAULT_AUTHENTICATION_CLASSES``) | ||
| * ``SessionAuthenticationAllowInactiveUser`` is used in examples for consistency with | ||
| ``JwtAuthentication``, which also allows inactive users by default. Using standard | ||
| ``SessionAuthentication`` alongside ``JwtAuthentication`` would create inconsistent | ||
| behavior — active-user enforcement would depend on which auth method the client used. | ||
| Endpoints that need to enforce active-user status should do so via a permission class | ||
| rather than the authentication class. This ADR does not take a stance on inactive | ||
| user policy beyond noting this inconsistency. | ||
| * The primary migration target is the ``view_auth_classes`` decorator — one change | ||
| removes ``BearerAuthentication`` from 49+ endpoints | ||
| * Verify no active external clients are still sending Bearer tokens before | ||
| removing ``BearerAuthentication`` from any endpoint | ||
| * ``JWT_AUTH_ADD_KID_HEADER`` toggle in ``openedx/core/djangoapps/oauth_dispatch/jwt.py`` | ||
| is past its removal date (target: 2024-04-20) — KID header should be made always-on | ||
| and the toggle removed | ||
| * ``OAuth2Authentication`` / ``OAuth2AuthenticationAllowInactiveUser`` in | ||
| ``openedx/core/lib/api/authentication.py`` are deprecated aliases that exist only | ||
| to avoid breaking external repos — remove once those repos migrate to ``JwtAuthentication`` | ||
|
|
||
| Rollout Plan | ||
| ------------ | ||
|
|
||
| 1. Audit existing APIs and categorize — flag any using ``BearerAuthentication`` variants | ||
| 2. Remove overdue ``JWT_AUTH_ADD_KID_HEADER`` toggle — make KID header always-on | ||
|
|
||
| For all steps related to ``BearerAuthentication`` deprecation and removal (monitoring | ||
| active usage, marking deprecated in source, migrating external clients, token expiry | ||
| considerations, and third-party communication), see the | ||
| `DEPR: BearerAuthentication <https://github.com/openedx/edx-drf-extensions/issues/284>`_ ticket. | ||
|
|
||
| References | ||
| ========== | ||
|
|
||
| * `OEP-0042`_ — Open edX Authentication Best Practices (primary reference) | ||
| * `DEPR: BearerAuthentication <https://github.com/openedx/edx-drf-extensions/issues/284>`_ — Deprecation ticket for ``BearerAuthentication`` in edx-drf-extensions and edx-platform | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deprecation for BearerAuthentication is simpler than removal, and the DEPR ticket brings up issues and questions around removal that are not addressed by this doc. I'm wondering if you need/want this document to address all the details, or if that should be worked out on the DEPR, and if this doc should simply refer to the DEPR for those details (in the rollout plan)? You are welcome to update the DEPR as-needed, but please don't remove things that aren't clear to you. Thank you.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Questions we had on depr ticket were
May be I have missed some,
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yup, this makes sense. We can move whats related to the rollout plan for BearerAuthentication deprecation to the depr ticket to track it there. I'll only mention the depr ticket in this doc.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @Faraz32123. Note that the DEPR ticket is in Draft because there was no one to own seeing it through the DEPR process. I drafted it as a place to capture my notes in case anyone were to ever pick it up. From this ADR, it seems there might be some momentum. Feel free to pick-up the ADR (or find someone to do so) and announce it when and if someone is ready to move it forward. The DEPR should first be updated to match the current DEPR template. So, feel free to the update the DEPR as-needed. That being said - if you or anyone needs someone from the DEPR WG to support you on this process - I'm happy to do so. |
||
| * Django REST Framework - Authentication and permissions | ||
| * Django OAuth Toolkit documentation | ||
| * Open edX Authentication Patterns Guide | ||
|
|
||
| .. _OEP-0042: https://docs.openedx.org/projects/openedx-proposals/en/latest/best-practices/oep-0042-bp-authentication.html | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| 17. Standardize Authentication Patterns and Security Schemes | ||
| ------------------------------------------------------------ | ||
|
|
||
| This decision has been documented in the platform-level ADR: | ||
|
|
||
| :doc:`docs/decisions/0034-unify-auth-oauth2-dot-v2` | ||
|
|
||
| See that document for: | ||
|
|
||
| * The decision to standardize on ``JwtAuthentication`` for all DRF user-authenticated APIs | ||
| * Deprecation of ``BearerAuthentication`` and ``BearerAuthenticationAllowInactiveUser`` | ||
| * Code examples showing current and target states for existing views | ||
| * Rollout plan and reference to the `DEPR: BearerAuthentication <https://github.com/openedx/edx-drf-extensions/issues/284>`_ ticket |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other auth ADRs (0003, 0008, 0009, 0010, 0013, 0014) live under
openedx/core/djangoapps/oauth_dispatch/docs/decisions/. Add a pointer file there linking to this one.