Merge pull request #34300 from openedx/asaeed/add-application-scopes-…

…to-default-scopes

feat: add user_id in the default scopes if no scopes are requested from payload

openedx/core/djangoapps/oauth_dispatch/docs/decisions/0015-add-scope-user-id-for-jwt.rst → 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

@@ -0,0 +1,40 @@
+16. Add scope user_id for JWT token and client credentials grant type
+#####################################################################
+
+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.
+
+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
+---------
+
+- 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 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

@@ -5,6 +5,7 @@
 
 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
@@ -91,6 +92,32 @@ def save_bearer_token(self, token, request, *args, **kwargs):
         request.grant_type = grant_type
         request.user = user
 
+    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, add `user_id` as a default scope if it is an allowed scope.
+        """
+        default_scopes = super().get_default_scopes(client_id, request, *args, **kwargs)
+        # .. toggle_name: ENABLE_USER_ID_SCOPE
+        # .. toggle_implementation:DjangoSetting
+        # .. toggle_description: If enabled, the user_id scope will be added to the default scopes for client_credentials grant type.
+        # .. toggle_default: False
+        # .. toggle_use_cases: temporary
+        # .. toggle_creation_date: 2024-03-16
+        # .. toggle_target_removal_date: 2024-04-16
+        # .. toggle_warnings: This feature flag is temporary and will be removed once the feature is fully tested.
+        # .. toggle_tickets: https://github.com/openedx/edx-platform/issues/34381 (toggle removal ticket)
+        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
+                    extended_default_scopes = default_scopes.copy()
+                    extended_default_scopes.append('user_id')
+                    return extended_default_scopes
+        return default_scopes
+
     def validate_scopes(self, client_id, scopes, client, request, *args, **kwargs):
         """
         Ensure required scopes are permitted (as specified in the settings file)

openedx/core/djangoapps/oauth_dispatch/scopes.py

@@ -22,3 +22,15 @@ def get_available_scopes(self, application=None, request=None, *args, **kwargs):
         default_scopes = self.get_default_scopes()
         all_scopes = list(self.get_all_scopes().keys())
         return set(application_scopes + default_scopes).intersection(all_scopes)
+
+    def has_user_id_in_application_scopes(self, application):
+        """
+        Returns the user id associated with the given application.
+        """
+        try:
+            application_scopes = ApplicationAccess.get_scopes(application)
+            if 'user_id' in application_scopes:
+                return True
+        except ApplicationAccess.DoesNotExist:
+            return False
+        return False

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

@@ -5,13 +5,15 @@
 # pylint: disable=protected-access
 
 import datetime
+from unittest import mock
 
 from django.conf import settings
 from django.test import RequestFactory, TestCase
 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':
@@ -55,6 +57,7 @@ class CustomValidationTestCase(TestCase):
 
     In particular, inactive users should be able to validate.
     """
+
     def setUp(self):
         super().setUp()
         self.TEST_PASSWORD = 'Password1234'
@@ -65,6 +68,7 @@ def setUp(self):
         )
         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
@@ -77,6 +81,41 @@ def test_inactive_user_validates(self):
         request = self.request_factory.get('/')
         assert self.validator.validate_user('darkhelmet', self.TEST_PASSWORD, client=None, request=request)
 
+    @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.
+        """
+        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 + ['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):
+        """
+        Test that get_default_scopes returns the default scopes if user_id scope is not available.
+        """
+        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, self.default_scopes)
+
 
 @skip_unless_lms
 class CustomAuthorizationViewTestCase(TestCase):
@@ -87,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'

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

@@ -30,4 +30,13 @@ def test_get_available_scopes(self, application_scopes, expected_additional_scop
         application_access = ApplicationAccessFactory(scopes=application_scopes)
         scopes = ApplicationModelScopes()
         assert set(scopes.get_available_scopes(application_access.application)) == \
-               set(list(settings.OAUTH2_DEFAULT_SCOPES.keys()) + expected_additional_scopes)
+            set(list(settings.OAUTH2_DEFAULT_SCOPES.keys()) + expected_additional_scopes)
+
+    def test_has_user_id_in_application_scopes(self):
+        """ Verify the settings backend correctly identifies whether the user_id scope is available. """
+        application_access = ApplicationAccessFactory(scopes=['user_id'])
+        scopes = ApplicationModelScopes()
+        assert scopes.has_user_id_in_application_scopes(application_access.application)
+        application_access.scopes = []
+        application_access.save()
+        assert not scopes.has_user_id_in_application_scopes(application_access.application)