-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
[DEPR]: BasicAuthentication as default authentication class in edx-platform #33028
Labels
depr
Proposal for deprecation & removal per OEP-21
Comments
Merged
robrap
added a commit
that referenced
this issue
Aug 21, 2023
Removed BasicAuthentication as a default from DRF endpoints that have not overridden the authentication classes. It appears this is not in use, and was just implicitly a default because it came from DRF's defaults. See DEPR: #33028
mehedikhan72
pushed a commit
to mehedikhan72/edx-platform
that referenced
this issue
Aug 24, 2023
Removed BasicAuthentication as a default from DRF endpoints that have not overridden the authentication classes. It appears this is not in use, and was just implicitly a default because it came from DRF's defaults. See DEPR: openedx#33028
Agrendalath
pushed a commit
to open-craft/edx-platform
that referenced
this issue
Oct 22, 2024
Removed BasicAuthentication as a default from DRF endpoints that have not overridden the authentication classes. It appears this is not in use, and was just implicitly a default because it came from DRF's defaults. See DEPR: openedx#33028 (cherry picked from commit 7113624)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Proposal Date
2023-08-16
Target Ticket Acceptance Date
2023-08-18
Earliest Open edX Named Release Without This Functionality
Quince - 2023-10
Rationale
BasicAuthentication was unintentionally a default authentication class for DRF endpoints in edx-platform because the defaults were never made explicit in edx-platform, and thus the default of
[SessionAuthentication, BasicAuthentication]
from DRF itself was being used.BasicAuthentication is simply going to be removed from the default list.
This will simplify the authentication story for the platform, and is actively being pursued as part of a variety of other clean-up efforts in this area.
Note: Additionally, if a deployment wants to be hidden behind basic auth, this can lead to login errors if these credentials are passed through to the LMS service.
Removal
A recent change was made to make this default more explicit, and this line will be removed:
edx-platform/lms/envs/common.py
Line 3334 in 62718f2
We will also remove DefaultBasicAuthentication, which was simply added for monitoring purposes to double-check the assumption that this was not being used:
edx-platform/openedx/core/djangolib/default_auth_classes.py
Line 36 in 62718f2
Replacement
SessionAuthentication is already in place, and will serve as a replacement.
Deprecation
No response
Migration
No response
Additional Info
Separately, JwtAuthentication is being added as a new default. I'd like to fast track this removal, based on the fact that it was never explicitly added, and no endpoints that have overridden the defaults (for example to add JwtAuthentication) ever added BasicAuthentication.
Hopefully, this DEPR is just a formality in cleaning up something that does not need to exist.
The text was updated successfully, but these errors were encountered: