fix: update ADRs and add test cases with ENABLE_USER_ID_SCOPE flag off

openedx/core/djangoapps/oauth_dispatch/docs/decisions/0015-add-scope-user-id-for-jwt-password-grant.rst

@@ -1,5 +1,5 @@
-15. Add scope user_id for JWT token
-###################################
+15. Add scope user_id for JWT token for grant_type password
+###########################################################
 
 Status
 ------

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

@@ -1,5 +1,5 @@
-15. Add scope user_id for JWT token
-###################################
+16. Add scope user_id for JWT token and client credentials grant type
+#####################################################################
 
 Status
 ------
@@ -13,6 +13,13 @@ This ADR builds upon ADR 0015: Add Scope user_id for JWT Token. It inherits the
 
 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.
 
+Based on 0005-restricted-application-for-SSO_ and 0006-enforce-scopes-in-LMS-APIs_ ADRs, it may be that the original purpose of the user_id scope was to protect against leakage of the user_id in the case of some combination of Authorization Grant and Restricted Applications. More investigation would be required to refactor based on this context, but it may be useful for future iterations.
+
+.. _0005-restricted-application-for-SSO: 0005-restricted-application-for-SSO.rst
+.. _0006-enforce-scopes-in-LMS-APIs: 0006-enforce-scopes-in-LMS-APIs.rst
+
+The current implementation requires clients to remember to request using the scope, which is not a trustworthy solution.
+
 Decisions
 ---------
 
@@ -28,5 +35,6 @@ Consequences
 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.
+- Adding all allowable scopes as default: Including all allowable scopes by default, which would include the user_id scope, would not follow the principle of least privilege and would make every default token have more access than required. Instead, we will minimize the additional access.
 - 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.
+- See the context section for additional thoughts around Authorization Grant type and Restricted Applications.

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

@@ -100,7 +100,7 @@ class EdxOAuth2Validator(OAuth2Validator):
         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 settings.FEATURES.get('ENABLE_USER_ID_SCOPE', True):
+        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):
                     # copy the default scopes and add user_id to it to avoid modifying the original list

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

@@ -57,6 +57,7 @@ class CustomValidationTestCase(TestCase):
 
     In particular, inactive users should be able to validate.
     """
+
     def setUp(self):
         super().setUp()
         self.TEST_PASSWORD = 'Password1234'
@@ -92,6 +93,19 @@ class CustomValidationTestCase(TestCase):
 
         self.assertEqual(overriden_default_scopes, self.default_scopes + ['user_id'])
 
+    @mock.patch.dict(settings.FEATURES, ENABLE_USER_ID_SCOPE=False)
+    def test_get_default_scopes_without_user_id(self):
+        """
+        Test that if `ENABLE_USER_ID_SCOPE` flag is turned off, the get_default_scopes returns 
+        the default scopes without `user_id` even if it's allowed.
+        """
+        application_access = ApplicationAccessFactory(scopes=['user_id'])
+
+        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, self.default_scopes)
+
     @mock.patch.dict(settings.FEATURES, ENABLE_USER_ID_SCOPE=True)
     def test_get_default_scopes(self):
         """
@@ -112,6 +126,7 @@ class CustomAuthorizationViewTestCase(TestCase):
     an application even if the access token is expired.
     (This is a temporary override until Auth Scopes is implemented.)
     """
+
     def setUp(self):
         super().setUp()
         self.TEST_PASSWORD = 'Password1234'