From 92a47097b7cd46e89d0b17f51e02a4bd069832c3 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Fri, 31 Oct 2025 15:02:51 -0400 Subject: [PATCH] feat: Drop the ENABLE_OAUTH2_PROVIDER flag. This setting was not actually not changing installation behavior, that is being set by whether oauth_dispatch is in INSTALLED_APPS or not. This flag was being used to: * Hide enable/disable certain URL paths. * We need these paths on all the time in the LMS because all other services and MFE rely on oauth to authenticate with the LMS so we just end up turning this on later in the settings stack. * We use it to only run certain oauth_dispatch tests in the LMS test environment because the oauth_dispatch app is not installed in the CMS. * We use the `skip_unless_lms` decorator now instead to do this or just run the tests in both suites because they are valid tests in both contexts. --- cms/envs/mock.yml | 1 - cms/envs/test.py | 4 +++ lms/envs/devstack.py | 1 - lms/envs/mock.yml | 1 - lms/envs/production.py | 11 +++--- lms/envs/test.py | 1 - lms/urls.py | 34 +++++++++---------- .../auth_exchange/tests/test_views.py | 4 +-- .../oauth_dispatch/tests/test_api.py | 12 ++----- .../tests/test_client_credentials.py | 5 ++- .../oauth_dispatch/tests/test_dot_adapter.py | 12 +++---- .../oauth_dispatch/tests/test_factories.py | 6 ---- .../oauth_dispatch/tests/test_views.py | 17 ++++------ openedx/core/lib/__init__.py | 2 +- .../core/lib/api/tests/test_authentication.py | 3 -- openedx/envs/common.py | 14 +------- 16 files changed, 46 insertions(+), 82 deletions(-) diff --git a/cms/envs/mock.yml b/cms/envs/mock.yml index dd2d9cf79d..3d6c92d288 100644 --- a/cms/envs/mock.yml +++ b/cms/envs/mock.yml @@ -430,7 +430,6 @@ FEATURES: ENABLE_MKTG_EMAIL_OPT_IN: true ENABLE_MKTG_SITE: true ENABLE_MOBILE_REST_API: true - ENABLE_OAUTH2_PROVIDER: true ENABLE_PASSWORD_RESET_FAILURE_EMAIL: true ENABLE_PROCTORED_EXAMS: true ENABLE_PUBLISHER: true diff --git a/cms/envs/test.py b/cms/envs/test.py index 5b6b4facbe..08ad3aa0d1 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -310,6 +310,10 @@ PROCTORING_SETTINGS = {} # (ref MST-637) PROCTORING_USER_OBFUSCATION_KEY = "85920908f28904ed733fe576320db18cabd7b6cd" +############### Settings for Django Rate limit ##################### + +RATELIMIT_RATE = '2/m' + ##### LOGISTRATION RATE LIMIT SETTINGS ##### LOGISTRATION_RATELIMIT_RATE = "5/5m" LOGISTRATION_PER_EMAIL_RATELIMIT_RATE = "6/5m" diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index b15533855c..aa015d452e 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -145,7 +145,6 @@ PIPELINE['SASS_ARGUMENTS'] = '--debug-info' AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING = True ########################### External REST APIs ################################# -ENABLE_OAUTH2_PROVIDER = True ENABLE_MOBILE_REST_API = True ENABLE_VIDEO_ABSTRACTION_LAYER_API = True diff --git a/lms/envs/mock.yml b/lms/envs/mock.yml index edc9f1aece..9c6b077dbe 100644 --- a/lms/envs/mock.yml +++ b/lms/envs/mock.yml @@ -586,7 +586,6 @@ FEATURES: ENABLE_MKTG_SITE: true ENABLE_MOBILE_REST_API: true ENABLE_NEW_BULK_EMAIL_EXPERIENCE: true - ENABLE_OAUTH2_PROVIDER: true ENABLE_ORA_MOBILE_SUPPORT: true ENABLE_ORA_USERNAMES_ON_DATA_EXPORT: true ENABLE_PAYMENT_FAKE: true diff --git a/lms/envs/production.py b/lms/envs/production.py index aeccaf0c0f..34899a126e 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -318,12 +318,11 @@ if ENABLE_THIRD_PARTY_AUTH: THIRD_PARTY_AUTH_CUSTOM_AUTH_FORMS = _YAML_TOKENS.get('THIRD_PARTY_AUTH_CUSTOM_AUTH_FORMS', {}) ##### OAUTH2 Provider ############## -if ENABLE_OAUTH2_PROVIDER: - OAUTH_ENFORCE_SECURE = True - OAUTH_ENFORCE_CLIENT_SECURE = True - # Defaults for the following are defined in lms.envs.common - OAUTH_EXPIRE_DELTA = datetime.timedelta(days=OAUTH_EXPIRE_CONFIDENTIAL_CLIENT_DAYS) - OAUTH_EXPIRE_DELTA_PUBLIC = datetime.timedelta(days=OAUTH_EXPIRE_PUBLIC_CLIENT_DAYS) +OAUTH_ENFORCE_SECURE = True +OAUTH_ENFORCE_CLIENT_SECURE = True +# Defaults for the following are defined in lms.envs.common +OAUTH_EXPIRE_DELTA = datetime.timedelta(days=OAUTH_EXPIRE_CONFIDENTIAL_CLIENT_DAYS) +OAUTH_EXPIRE_DELTA_PUBLIC = datetime.timedelta(days=OAUTH_EXPIRE_PUBLIC_CLIENT_DAYS) if ( ENABLE_COURSEWARE_SEARCH or diff --git a/lms/envs/test.py b/lms/envs/test.py index 218a7e8461..04980084d3 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -256,7 +256,6 @@ THIRD_PARTY_AUTH_CUSTOM_AUTH_FORMS = { } ############################## OAUTH2 Provider ################################ -ENABLE_OAUTH2_PROVIDER = True OAUTH_ENFORCE_SECURE = False ########################### External REST APIs ################################# diff --git a/lms/urls.py b/lms/urls.py index 490f4727e0..9668a394d6 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -828,16 +828,15 @@ urlpatterns += [ path('survey/', include('lms.djangoapps.survey.urls')), ] -if settings.FEATURES.get('ENABLE_OAUTH2_PROVIDER'): - urlpatterns += [ - # These URLs dispatch to django-oauth-toolkit or django-oauth2-provider as appropriate. - # Developers should use these routes, to maintain compatibility for existing client code - path('oauth2/', include('openedx.core.djangoapps.oauth_dispatch.urls')), - # The /_o/ prefix exists to provide a target for code in django-oauth-toolkit that - # uses reverse() with the 'oauth2_provider' namespace. Developers should not access these - # views directly, but should rather use the wrapped views at /oauth2/ - path('_o/', include('oauth2_provider.urls', namespace='oauth2_provider')), - ] +urlpatterns += [ + # These URLs dispatch to django-oauth-toolkit or django-oauth2-provider as appropriate. + # Developers should use these routes, to maintain compatibility for existing client code + path('oauth2/', include('openedx.core.djangoapps.oauth_dispatch.urls')), + # The /_o/ prefix exists to provide a target for code in django-oauth-toolkit that + # uses reverse() with the 'oauth2_provider' namespace. Developers should not access these + # views directly, but should rather use the wrapped views at /oauth2/ + path('_o/', include('oauth2_provider.urls', namespace='oauth2_provider')), +] if settings.FEATURES.get('ENABLE_SERVICE_STATUS'): urlpatterns += [ @@ -877,14 +876,13 @@ if enterprise_enabled(): ] # OAuth token exchange -if settings.FEATURES.get('ENABLE_OAUTH2_PROVIDER'): - urlpatterns += [ - path( - 'oauth2/login/', - LoginWithAccessTokenView.as_view(), - name='login_with_access_token' - ), - ] +urlpatterns += [ + path( + 'oauth2/login/', + LoginWithAccessTokenView.as_view(), + name='login_with_access_token' + ), +] # Certificates urlpatterns += [ diff --git a/openedx/core/djangoapps/auth_exchange/tests/test_views.py b/openedx/core/djangoapps/auth_exchange/tests/test_views.py index 0ab08ffe37..f560c5effb 100644 --- a/openedx/core/djangoapps/auth_exchange/tests/test_views.py +++ b/openedx/core/djangoapps/auth_exchange/tests/test_views.py @@ -11,7 +11,6 @@ from datetime import datetime import ddt import httpretty -from django.conf import settings from django.test import TestCase from django.test.utils import override_settings from django.urls import reverse @@ -28,6 +27,7 @@ from common.djangoapps.third_party_auth.tests.utils import ( ThirdPartyOAuthTestMixinFacebook, ThirdPartyOAuthTestMixinGoogle, ) +from openedx.core.djangolib.testing.utils import skip_unless_lms from .mixins import DOTAdapterMixin from .utils import TPA_FEATURE_ENABLED, TPA_FEATURES_KEY, AccessTokenExchangeTestMixin @@ -145,7 +145,7 @@ class DOTAccessTokenExchangeViewTestGoogle( pass # lint-amnesty, pylint: disable=unnecessary-pass -@unittest.skipUnless(settings.FEATURES.get("ENABLE_OAUTH2_PROVIDER"), "OAuth2 not enabled") +@skip_unless_lms class TestLoginWithAccessTokenView(TestCase): """ Tests for LoginWithAccessTokenView diff --git a/openedx/core/djangoapps/oauth_dispatch/tests/test_api.py b/openedx/core/djangoapps/oauth_dispatch/tests/test_api.py index 5bf1f524bc..dd1e3e2cf2 100644 --- a/openedx/core/djangoapps/oauth_dispatch/tests/test_api.py +++ b/openedx/core/djangoapps/oauth_dispatch/tests/test_api.py @@ -1,9 +1,6 @@ """ Tests for OAuth Dispatch python API module. """ -import unittest - -from django.conf import settings from django.http import HttpRequest from django.test import TestCase from oauth2_provider.models import AccessToken @@ -11,16 +8,13 @@ from oauth2_provider.models import AccessToken from common.djangoapps.student.tests.factories import UserFactory from common.test.utils import assert_dict_contains_subset -OAUTH_PROVIDER_ENABLED = settings.FEATURES.get('ENABLE_OAUTH2_PROVIDER') -if OAUTH_PROVIDER_ENABLED: - from openedx.core.djangoapps.oauth_dispatch import api - from openedx.core.djangoapps.oauth_dispatch.adapters import DOTAdapter - from openedx.core.djangoapps.oauth_dispatch.tests.constants import DUMMY_REDIRECT_URL +from openedx.core.djangoapps.oauth_dispatch import api +from openedx.core.djangoapps.oauth_dispatch.adapters import DOTAdapter +from openedx.core.djangoapps.oauth_dispatch.tests.constants import DUMMY_REDIRECT_URL EXPECTED_DEFAULT_EXPIRES_IN = 36000 -@unittest.skipUnless(OAUTH_PROVIDER_ENABLED, 'OAuth2 not enabled') class TestOAuthDispatchAPI(TestCase): """ Tests for oauth_dispatch's api.py module. """ def setUp(self): diff --git a/openedx/core/djangoapps/oauth_dispatch/tests/test_client_credentials.py b/openedx/core/djangoapps/oauth_dispatch/tests/test_client_credentials.py index 6f1c329ef2..846fd9d267 100644 --- a/openedx/core/djangoapps/oauth_dispatch/tests/test_client_credentials.py +++ b/openedx/core/djangoapps/oauth_dispatch/tests/test_client_credentials.py @@ -2,21 +2,20 @@ import json -import unittest -from django.conf import settings from django.test import TestCase from django.urls import reverse from oauth2_provider.models import Application from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangolib.testing.utils import skip_unless_lms from ..adapters import DOTAdapter from . import mixins from .constants import DUMMY_REDIRECT_URL -@unittest.skipUnless(settings.FEATURES.get("ENABLE_OAUTH2_PROVIDER"), "OAuth2 not enabled") +@skip_unless_lms class ClientCredentialsTest(mixins.AccessTokenMixin, TestCase): """ Tests validating the client credentials grant behavior. """ diff --git a/openedx/core/djangoapps/oauth_dispatch/tests/test_dot_adapter.py b/openedx/core/djangoapps/oauth_dispatch/tests/test_dot_adapter.py index ef30e6d341..556c089072 100644 --- a/openedx/core/djangoapps/oauth_dispatch/tests/test_dot_adapter.py +++ b/openedx/core/djangoapps/oauth_dispatch/tests/test_dot_adapter.py @@ -2,28 +2,26 @@ Tests for DOT Adapter """ -import unittest from datetime import timedelta import pytest import ddt -from django.conf import settings from django.test import TestCase from django.urls import reverse from django.utils.timezone import now from oauth2_provider import models from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangolib.testing.utils import skip_unless_lms # oauth_dispatch is not in CMS' INSTALLED_APPS so these imports will error during test collection -if settings.FEATURES.get("ENABLE_OAUTH2_PROVIDER"): - from ..adapters import DOTAdapter - from .constants import DUMMY_REDIRECT_URL, DUMMY_REDIRECT_URL2 - from ..models import RestrictedApplication +from ..adapters import DOTAdapter +from .constants import DUMMY_REDIRECT_URL, DUMMY_REDIRECT_URL2 +from ..models import RestrictedApplication @ddt.ddt -@unittest.skipUnless(settings.FEATURES.get("ENABLE_OAUTH2_PROVIDER"), "OAuth2 not enabled") +@skip_unless_lms class DOTAdapterTestCase(TestCase): """ Test class for DOTAdapter. diff --git a/openedx/core/djangoapps/oauth_dispatch/tests/test_factories.py b/openedx/core/djangoapps/oauth_dispatch/tests/test_factories.py index f784d3d178..bb4918c8ff 100644 --- a/openedx/core/djangoapps/oauth_dispatch/tests/test_factories.py +++ b/openedx/core/djangoapps/oauth_dispatch/tests/test_factories.py @@ -1,9 +1,6 @@ # pylint: disable=missing-docstring -import unittest - -from django.conf import settings from django.test import TestCase from oauth2_provider.models import AccessToken, Application, RefreshToken @@ -11,7 +8,6 @@ from openedx.core.djangoapps.oauth_dispatch.tests import factories from common.djangoapps.student.tests.factories import UserFactory -@unittest.skipUnless(settings.FEATURES.get("ENABLE_OAUTH2_PROVIDER"), "OAuth2 not enabled") class TestClientFactory(TestCase): def setUp(self): super().setUp() @@ -23,7 +19,6 @@ class TestClientFactory(TestCase): assert actual_application == expected_application -@unittest.skipUnless(settings.FEATURES.get("ENABLE_OAUTH2_PROVIDER"), "OAuth2 not enabled") class TestAccessTokenFactory(TestCase): def setUp(self): super().setUp() @@ -36,7 +31,6 @@ class TestAccessTokenFactory(TestCase): assert actual_access_token == expected_access_token -@unittest.skipUnless(settings.FEATURES.get("ENABLE_OAUTH2_PROVIDER"), "OAuth2 not enabled") class TestRefreshTokenFactory(TestCase): def setUp(self): super().setUp() diff --git a/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py b/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py index 06031c37a1..7769207817 100644 --- a/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py +++ b/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py @@ -4,7 +4,6 @@ Tests for Blocks Views import json -import unittest from unittest.mock import call, patch import ddt @@ -18,7 +17,7 @@ from oauth2_provider import models as dot_models from common.djangoapps.student.tests.factories import UserFactory from common.djangoapps.third_party_auth.tests.utils import ThirdPartyOAuthTestMixin, ThirdPartyOAuthTestMixinGoogle from openedx.core.djangoapps.oauth_dispatch.toggles import DISABLE_JWT_FOR_MOBILE - +from openedx.core.djangolib.testing.utils import skip_unless_lms from . import mixins # NOTE (CCB): We use this feature flag in a roundabout way to determine if the oauth_dispatch app is installed @@ -29,13 +28,11 @@ from . import mixins # NOTE (BJM): As of Django 1.9 we also can't import models for apps which aren't in INSTALLED_APPS, so making all of # these imports conditional except mixins, which doesn't currently import forbidden models, and is needed at test # discovery time. -OAUTH_PROVIDER_ENABLED = settings.FEATURES.get('ENABLE_OAUTH2_PROVIDER') -if OAUTH_PROVIDER_ENABLED: - from .constants import DUMMY_REDIRECT_URL - from .. import adapters - from .. import models - from .. import views +from .constants import DUMMY_REDIRECT_URL +from .. import adapters +from .. import models +from .. import views class AccessTokenLoginMixin: @@ -75,7 +72,7 @@ class AccessTokenLoginMixin: assert self.login_with_access_token(access_token=access_token).status_code == 401 -@unittest.skipUnless(OAUTH_PROVIDER_ENABLED, 'OAuth2 not enabled') +@skip_unless_lms class _DispatchingViewTestCase(TestCase): """ Base class for tests that exercise DispatchingViews. @@ -685,7 +682,7 @@ class TestAuthorizationView(_DispatchingViewTestCase): return response.redirect_chain[-1][0] -@unittest.skipUnless(OAUTH_PROVIDER_ENABLED, 'OAuth2 not enabled') +@skip_unless_lms class TestViewDispatch(TestCase): """ Test that the DispatchingView dispatches the right way. diff --git a/openedx/core/lib/__init__.py b/openedx/core/lib/__init__.py index 61565e7581..b394a735a6 100644 --- a/openedx/core/lib/__init__.py +++ b/openedx/core/lib/__init__.py @@ -34,7 +34,7 @@ def ensure_cms(message: str = "This code may only be called by CMS, but it was c Useful if you want to forbid authoring-oriented code from accidentally running in the LMS process. """ - if settings.ROOT_URLCONF != 'cms.urls': + if settings.ROOT_URLCONF != _CMS_URLCONF: raise ImproperlyConfigured( f"{message}. Expected ROOT_URLCONF to be '{_CMS_URLCONF}', got '{settings.ROOT_URLCONF}'" ) diff --git a/openedx/core/lib/api/tests/test_authentication.py b/openedx/core/lib/api/tests/test_authentication.py index 3888a015c4..881f850cfd 100644 --- a/openedx/core/lib/api/tests/test_authentication.py +++ b/openedx/core/lib/api/tests/test_authentication.py @@ -5,12 +5,10 @@ Tests for OAuth2. This module is copied from django-rest-framework-oauth import itertools import json -import unittest from collections import namedtuple from datetime import timedelta import ddt -from django.conf import settings from django.http import HttpResponse from django.test import TestCase from django.test.utils import override_settings @@ -54,7 +52,6 @@ urlpatterns = [ @ddt.ddt # pylint: disable=missing-docstring -@unittest.skipUnless(settings.FEATURES.get("ENABLE_OAUTH2_PROVIDER"), "OAuth2 not enabled") @override_settings(ROOT_URLCONF=__name__) class OAuth2AllowInActiveUsersTests(TestCase): # lint-amnesty, pylint: disable=missing-class-docstring OAUTH2_BASE_TESTING_URL = '/oauth2-inactive-test/' diff --git a/openedx/envs/common.py b/openedx/envs/common.py index c00a89cf4f..86f6d9e1dc 100644 --- a/openedx/envs/common.py +++ b/openedx/envs/common.py @@ -665,17 +665,6 @@ ENABLE_DISCUSSION_SERVICE = True # .. toggle_tickets: https://github.com/openedx/edx-platform/pull/3064 ENABLE_TEXTBOOK = True -# .. toggle_name: ENABLE_OAUTH2_PROVIDER -# .. toggle_implementation: DjangoSetting -# .. toggle_default: False -# .. toggle_description: Enable this feature to allow this Open edX platform to be an OAuth2 authentication -# provider. This is necessary to enable some other features, such as the REST API for the mobile application. -# .. toggle_use_cases: temporary -# .. toggle_creation_date: 2014-09-09 -# .. toggle_target_removal_date: None -# .. toggle_warning: This temporary feature toggle does not have a target removal date. -ENABLE_OAUTH2_PROVIDER = False - # Allows to configure the LMS to provide CORS headers to serve requests from other # domains ENABLE_CORS_HEADERS = False @@ -765,8 +754,7 @@ EMBARGO = False # toggle is uncertain. ENABLE_MKTG_SITE = False -# Expose Mobile REST API. Note that if you use this, you must also set -# ENABLE_OAUTH2_PROVIDER to True +# Expose Mobile REST API. ENABLE_MOBILE_REST_API = False # Let students save and manage their annotations