Skip to content

[FC-0118] docs: add ADR for standardizing authentication patterns#38301

Open
Faraz32123 wants to merge 10 commits intodocs/ADRs-axim_api_improvementsfrom
docs/ADR-standardize_authentication_patterns_and_security_scheme_usage
Open

[FC-0118] docs: add ADR for standardizing authentication patterns#38301
Faraz32123 wants to merge 10 commits intodocs/ADRs-axim_api_improvementsfrom
docs/ADR-standardize_authentication_patterns_and_security_scheme_usage

Conversation

@Faraz32123
Copy link
Copy Markdown
Contributor

@Faraz32123 Faraz32123 commented Apr 8, 2026

related issue: #38169

@Faraz32123
Copy link
Copy Markdown
Contributor Author

related doc worth reading: https://docs.openedx.org/projects/openedx-proposals/en/latest/best-practices/oep-0042-bp-authentication.html#consequences, we can make changes to the ADR based on the attached doc if needed.

@Faraz32123 Faraz32123 requested a review from feanil April 8, 2026 08:29
@Faraz32123 Faraz32123 changed the title docs: add ADR for standardizing authentication patterns [FC-0118] docs: add ADR for standardizing authentication patterns Apr 13, 2026
@Faraz32123 Faraz32123 force-pushed the docs/ADR-standardize_authentication_patterns_and_security_scheme_usage branch from f5cb0c1 to a6a25d3 Compare April 13, 2026 08:40
Decision
========

1. **OAuth2 via Django OAuth Toolkit (DOT) MUST be the standard authentication
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.

How is this different from JWT authentication? My understanding was that JWTs were what the OAUTH2 mechanism produced for use in the authorization header?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You are right! I think the wording needs to be changed a bit because showing Oauth2 & JWT as separate can cause confusion. In the codebase, we have two JWT issuance paths:

  • 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 calls), it is used for internal service-to-service communication.
    Both are validated by the same JwtAuthentication class.

The real distinction is between JwtAuthentication (supported) and BearerAuthentication (deprecated per OEP-0042) and the use of multiple authentication methods in single API endpoint. I am updating the ADR to reflect this and we will no longer treat "OAuth2" and "JWT" as separate mechanisms.

@Faraz32123 Faraz32123 requested a review from feanil April 20, 2026 10:15
authentication mechanism for all API access** (external and internal), per `OEP-0042`_
2. **``BearerAuthentication`` and ``BearerAuthenticationAllowInactiveUser`` are
deprecated and MUST NOT be used in new code**
3. **Session authentication MUST be used only for browser-based UI interactions**
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 not sure if this distinction is super important. What is the bad thing that happens if your API endpoints support session auth also? Right now DRF in openedx-platform supports both by default: https://github.com/openedx/openedx-platform/blob/master/openedx/envs/common.py#L822-L825 and I don't see a problem with that. I think it's more valuable that all endpoints support any valid auth scheme than having API calls not support the browser.

Copy link
Copy Markdown
Contributor Author

@Faraz32123 Faraz32123 Apr 22, 2026

Choose a reason for hiding this comment

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

@feanil I think you have valid point here that we should support any valid auth scheme instead of categorizing them on the basis of their usage.
I am adding a new commit that addresses above point with some additional remains(includes removal of JWT issuers & use of asymmetric keys) of OEP-0042, But it had some consequences(Link) that you might want to look in. That way we can decide the scope of this ADR. And we can remove the part that is not needed for this ADR accordingly.

1. **JWT authentication via** ``JwtAuthentication`` **MUST be the standard
authentication mechanism for all API access** (external and internal), per `OEP-0042`_
2. **Session authentication MAY be supported alongside** ``JwtAuthentication``
on any endpoint — this is the platform default and has no security implications
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 saying there are zero implications is inaccurate. maybe soften it to something like "this is the platform default and is acceptable for most endpoints", since session auth carries CSRF concerns, cookie-handling rules, and a different threat model from JWT

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

let me change the wording a bit.


class BrowserUIViewSet(viewsets.ViewSet):
"""Browser UI API - Session authentication only."""
authentication_classes = [SessionAuthenticationAllowInactiveUser]
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 the Browser-based API example contradicts Decision #1 here. Decision #1 says JwtAuthentication MUST be used for all API access, but this example only has SessionAuthenticationAllowInactiveUser. Should we either add JwtAuthentication alongside session here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we can miss it, decision no.1 says, JwtAuthentication MUST be the standard authentication mechanism for all API access i.e. for external and internal API types. May be I can change decision no.1 to all API(external and internal) access to resolve the confusion.

For browser based APIs, session authentication is kind of must with an option of JWTAuthentication to support any valid auth scheme.

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.

@feanil: Can you point to an example in edx-platform? I'm not clear on what this is, and thus not clear on the recommendation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@robrap can u please take a look at it again? I have changed the examples according to our decisions & also adjusted missing items from openedx/edx-drf-extensions#284.

@robrap
Copy link
Copy Markdown
Contributor

robrap commented Apr 30, 2026

@feanil: You should review openedx/edx-drf-extensions#284 and all its comments to see if you are missing anything, and you may want to reference it in this ADR.

@Faraz32123 Faraz32123 force-pushed the docs/ADR-standardize_authentication_patterns_and_security_scheme_usage branch from 6b097b7 to 7504911 Compare May 5, 2026 14:09
@Faraz32123 Faraz32123 force-pushed the docs/ADR-standardize_authentication_patterns_and_security_scheme_usage branch from 7504911 to 0ed80d7 Compare May 5, 2026 14:11
Copy link
Copy Markdown
Contributor

@robrap robrap left a comment

Choose a reason for hiding this comment

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

Additional thoughts. Thanks.

Comment thread docs/decisions/0034-unify-auth-oauth2-dot-v2.rst Outdated
Comment thread docs/decisions/0034-unify-auth-oauth2-dot-v2.rst Outdated
==========

* `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
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.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Questions we had on depr ticket were

  1. third-party clients still using Bearer tokens. (this should be done with proper notice/depr_ticket/community_post).
  2. how do we handle the 1-hour JWT expiry for long-running jobs. (This should be handled via refresh token).

May be I have missed some,
But I think that discussion should be done on deprecation ticket instead.

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.

  1. I added some additional clarity to the section Possible migration issues on the DEPR, including moving some notes from the comments to this section.
  2. I think using the DEPR ticket itself sounds great. I'd move most of "Rollout Plan" from this doc and update the DEPR as needed, and in this doc's Rollout Plan, I'd have a single line that points to the DEPR for BearerAuthentication deprecation. Otherwise it is unnecessarily split. If there are other rollout plan items independent of deprecation left for this doc, they can remain in this doc. Does that make sense?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@robrap I have moved the rollout plan for BearerAuthentication deprecation out of this ADR in this commit. You can make changes to the deprecation ticket.

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.

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.

Comment thread docs/decisions/0034-unify-auth-oauth2-dot-v2.rst Outdated
Comment thread docs/decisions/0034-unify-auth-oauth2-dot-v2.rst Outdated
@Faraz32123 Faraz32123 requested a review from robrap May 7, 2026 09:55
@@ -0,0 +1,196 @@
Standardize Authentication Patterns and Security Schemes
========================================================
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.

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.


1. **JWT authentication via** ``JwtAuthentication`` **MUST be the standard
authentication mechanism for all API(external and internal) access**, per `OEP-0042`_
2. **Session authentication MUST also be used when** the expected client for an API
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.

"For all API access" would include admin views, /oauth2/access_token/, and HMAC/webhook endpoints — none of which use JwtAuthentication. Narrow this to "DRF API endpoints that take user-authenticated requests" and list the exclusions for context.

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.

@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.

Move BearerAuthentication depr plan out of this doc So that it resides in single place i.e. to its deprecation ticket.
@Faraz32123 Faraz32123 requested a review from feanil May 8, 2026 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants