From c9734e3399d17b803943871d2e32b0aec13906ef Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Mon, 21 Aug 2023 14:42:30 -0400 Subject: [PATCH 1/2] fix: change session_auth_result value to n/a Changes one of the values of the custom attribute session_auth_result from 'skipped' to 'n/a'. --- openedx/core/djangolib/default_auth_classes.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangolib/default_auth_classes.py b/openedx/core/djangolib/default_auth_classes.py index 238993c7aa..67ecdc3941 100644 --- a/openedx/core/djangolib/default_auth_classes.py +++ b/openedx/core/djangolib/default_auth_classes.py @@ -23,10 +23,10 @@ class DefaultSessionAuthentication(SessionAuthentication): if user_and_auth: # .. custom_attribute_name: session_auth_result # .. custom_attribute_description: The result of session auth, represented - # by: 'success', 'failure', or 'skipped'. + # by: 'success', 'failure', or 'n/a'. set_custom_attribute('session_auth_result', 'success') else: - set_custom_attribute('session_auth_result', 'skipped') + set_custom_attribute('session_auth_result', 'n/a') return user_and_auth except Exception as exception: set_custom_attribute('session_auth_result', 'failure') From 71136240ad5b32d0768a31142f631696542f9b55 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Mon, 21 Aug 2023 14:45:34 -0400 Subject: [PATCH 2/2] feat!: remove BasicAuthentication default 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: https://github.com/openedx/edx-platform/issues/33028 --- lms/envs/common.py | 6 ++-- .../core/djangolib/default_auth_classes.py | 34 +------------------ 2 files changed, 3 insertions(+), 37 deletions(-) diff --git a/lms/envs/common.py b/lms/envs/common.py index 7b92e23594..e4274f4b3f 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3326,12 +3326,10 @@ CROSS_DOMAIN_CSRF_COOKIE_NAME = '' ######################### 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': ( diff --git a/openedx/core/djangolib/default_auth_classes.py b/openedx/core/djangolib/default_auth_classes.py index 67ecdc3941..651bd79238 100644 --- a/openedx/core/djangolib/default_auth_classes.py +++ b/openedx/core/djangolib/default_auth_classes.py @@ -4,7 +4,7 @@ DEFAULT_AUTHENTICATION_CLASSES for observability purposes. """ 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): @@ -33,38 +33,6 @@ class DefaultSessionAuthentication(SessionAuthentication): 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