Skip to content

Commit

Permalink
feat!: remove BasicAuthentication default
Browse files Browse the repository at this point in the history
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
  • Loading branch information
robrap committed Aug 21, 2023
1 parent c9734e3 commit 7113624
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 37 deletions.
6 changes: 2 additions & 4 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -3326,12 +3326,10 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring
######################### Django Rest Framework ########################

REST_FRAMEWORK = {
# This matches the original DRF default of Session and Basic Authentication, but
# adds observability to help us potentially adjust the defaults. We would like to
# add JwtAuthentication and drop BasicAuthentication, based on our findings.
# These default classes add observability around endpoints using defaults, and should
# not be used anywhere else.
'DEFAULT_AUTHENTICATION_CLASSES': [
'openedx.core.djangolib.default_auth_classes.DefaultSessionAuthentication',
'openedx.core.djangolib.default_auth_classes.DefaultBasicAuthentication'
],
'DEFAULT_PAGINATION_CLASS': 'edx_rest_framework_extensions.paginators.DefaultPagination',
'DEFAULT_RENDERER_CLASSES': (
Expand Down
34 changes: 1 addition & 33 deletions openedx/core/djangolib/default_auth_classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"""
from edx_django_utils.monitoring import set_custom_attribute
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
from rest_framework.authentication import BasicAuthentication, SessionAuthentication
from rest_framework.authentication import SessionAuthentication


class DefaultSessionAuthentication(SessionAuthentication):
Expand Down Expand Up @@ -33,38 +33,6 @@ def authenticate(self, request):
raise


class DefaultBasicAuthentication(BasicAuthentication):
"""
Default BasicAuthentication with observability
Note that BasicAuthentication was a default because it was a DRF default.
Observability will be used to determine if BasicAuthentication could
instead be dropped as a default.
"""

def authenticate(self, request):
# .. custom_attribute_name: using_default_auth_classes
# .. custom_attribute_description: This custom attribute will always be
# True (if not NULL), and signifies that a default authentication
# class was used. This can be used to find endpoints using the
# default authentication classes.
set_custom_attribute('using_default_auth_classes', True)

try:
user_and_auth = super().authenticate(request)
if user_and_auth:
# .. custom_attribute_name: basic_auth_result
# .. custom_attribute_description: The result of basic auth, represented
# by: 'success', 'failure', or 'skipped'.
set_custom_attribute('basic_auth_result', 'success')
else:
set_custom_attribute('basic_auth_result', 'skipped')
return user_and_auth
except Exception as exception:
set_custom_attribute('basic_auth_result', 'failure')
raise


class DefaultJwtAuthentication(JwtAuthentication):
"""
Default JwtAuthentication with observability
Expand Down

0 comments on commit 7113624

Please sign in to comment.