chore: use feature-flag, remove mocks for tests and add ADR in decisions

openedx/core/djangoapps/oauth_dispatch/docs/decisions/0016-add-scope-user-id-for-jwt-client-credentials.rst

@@ -0,0 +1,32 @@
+15. Add scope user_id for JWT token
+###################################
+
+Status
+------
+
+Accepted
+
+Context
+-------
+
+This ADR builds upon ADR 0015: Add Scope user_id for JWT Token. It inherits the context from that ADR, including the initial addition of the user_id claim for analytics and the rationale for using a scope to control its inclusion.
+
+However, this ADR focuses specifically on the enterprise context highlighted in the consequences of ADR 0015. External organizations often utilize the LMS API with the grant_type: client_credentials to request tokens on behalf of their users. These tokens are crucial for enterprise integrations and require access to the user's user_id for proper functionality.
+
+Decisions
+---------
+
+- The scope ``user_id`` will be added to all requests having grant_type ``client_credentials`` in the API `/oauth2/access_token/`, if it is an allowed scope in the DOT Application and the payload request does not have `scopes` attribute in it.
+
+Consequences
+------------
+
+- Enterprises will have the flexibility to request the user_id claim in JWT tokens issued through the client_credentials grant. This enables them to integrate seamlessly with LMS functionalities requiring user identification.
+- The existing behavior for other grant types (like password) defined in ADR 0015 remains unchanged.
+- This pattern will be used to clean-up the manually added ``user_id`` scope for oauth clients using the enterprise public APIs.
+
+Deferred/Rejected Alternatives
+------------------------------
+
+- Adding All Default Scopes: Including all scopes by default could potentially expose unnecessary user information. This approach prioritizes security over granular control.
+- Default Inclusion with Opt-Out: This alternative explores automatically adding user_id for all cases and introducing an opt-out mechanism for specific scenarios where it's not required. Deferring this option allows for further analysis of potential security implications and user experience trade-offs.

openedx/core/djangoapps/oauth_dispatch/dot_overrides/validators.py

@@ -5,6 +5,7 @@ Classes that override default django-oauth-toolkit behavior
 
 from datetime import datetime, timedelta
 
+from django.conf import settings
 from django.contrib.auth import authenticate, get_user_model
 from django.db.models.signals import pre_save
 from django.dispatch import receiver
@@ -93,13 +94,16 @@ class EdxOAuth2Validator(OAuth2Validator):
 
     def get_default_scopes(self, client_id, request, *args, **kwargs):
         """
+        Returns the default scopes.
+
         If the request payload does not have `scopes` attribute for a grant_type of
-        client credentials, it should add `user_id` in the default scopes.
+        client credentials, add `user_id` as a default scope if it is an allowed scope.
         """
         default_scopes = super().get_default_scopes(client_id, request, *args, **kwargs)
-        if request.grant_type == 'client_credentials' and not request.scopes:
-            if get_scopes_backend().has_user_id_in_application_scopes(application=request.client):
-                default_scopes.append('user_id')
+        if settings.FEATURES.get('ENABLE_USER_ID_SCOPE', False):
+            if request.grant_type == 'client_credentials' and not request.scopes:
+                if get_scopes_backend().has_user_id_in_application_scopes(application=request.client):
+                    default_scopes.append('user_id')
         return default_scopes
 
     def validate_scopes(self, client_id, scopes, client, request, *args, **kwargs):

openedx/core/djangoapps/oauth_dispatch/tests/test_dot_overrides.py

@@ -13,6 +13,7 @@ from django.utils import timezone
 
 from common.djangoapps.student.tests.factories import UserFactory
 from openedx.core.djangolib.testing.utils import skip_unless_lms
+from openedx.core.djangoapps.oauth_dispatch.tests.factories import ApplicationAccessFactory
 
 # oauth_dispatch is not in CMS' INSTALLED_APPS so these imports will error during test collection
 if settings.ROOT_URLCONF == 'lms.urls':
@@ -66,6 +67,7 @@ class CustomValidationTestCase(TestCase):
         )
         self.validator = EdxOAuth2Validator()
         self.request_factory = RequestFactory()
+        self.default_scopes = list(settings.OAUTH2_DEFAULT_SCOPES.keys())
 
     def test_active_user_validates(self):
         assert self.user.is_active
@@ -78,39 +80,27 @@ class CustomValidationTestCase(TestCase):
         request = self.request_factory.get('/')
         assert self.validator.validate_user('darkhelmet', self.TEST_PASSWORD, client=None, request=request)
 
-    @mock.patch(
-        'openedx.core.djangoapps.oauth_dispatch.scopes.ApplicationModelScopes.has_user_id_in_application_scopes'
-    )
-    @mock.patch('oauth2_provider.oauth2_validators.OAuth2Validator.get_default_scopes')
-    def test_get_updated_default_scopes(self, mock_get_default_scopes, mock_has_user_id_in_application_scopes):
+    @mock.patch.dict(settings.FEATURES, ENABLE_USER_ID_SCOPE=True)
+    def test_get_default_scopes_with_user_id(self):
         """
         Test that get_default_scopes returns the default scopes plus the user_id scope if it's available.
         """
-        default_scopes = ['profile', 'email']
-        mock_get_default_scopes.return_value = default_scopes.copy()
-        mock_has_user_id_in_application_scopes.return_value = True
+        application_access = ApplicationAccessFactory(scopes=['user_id'])
 
-        request = mock.Mock(grant_type='client_credentials', client=None, scopes=None)
+        request = mock.Mock(grant_type='client_credentials', client=application_access.application, scopes=None)
         overriden_default_scopes = self.validator.get_default_scopes(request=request, client_id='client_id')
 
-        self.assertEqual(overriden_default_scopes, default_scopes + ['user_id'])
+        self.assertEqual(overriden_default_scopes, self.default_scopes + ['user_id'])
 
-    @mock.patch(
-        'openedx.core.djangoapps.oauth_dispatch.scopes.ApplicationModelScopes.has_user_id_in_application_scopes'
-    )
-    @mock.patch('oauth2_provider.oauth2_validators.OAuth2Validator.get_default_scopes')
-    def test_get_default_scopes(self, mock_get_default_scopes, mock_has_user_id_in_application_scopes):
+    @mock.patch.dict(settings.FEATURES, ENABLE_USER_ID_SCOPE=True)
+    def test_get_default_scopes(self):
         """
         Test that get_default_scopes returns the default scopes if user_id scope is not available.
         """
-        default_scopes = ['profile', 'email']
-        mock_get_default_scopes.return_value = default_scopes.copy()
-        mock_has_user_id_in_application_scopes.return_value = False
-
         request = mock.Mock(grant_type='client_credentials', client=None, scopes=None)
         overriden_default_scopes = self.validator.get_default_scopes(request=request, client_id='client_id')
 
-        self.assertEqual(overriden_default_scopes, default_scopes)
+        self.assertEqual(overriden_default_scopes, self.default_scopes)
 
 
 @skip_unless_lms