diff --git a/openedx/core/djangoapps/oauth_dispatch/docs/decisions/0015-add-scope-user-id-for-jwt.rst b/openedx/core/djangoapps/oauth_dispatch/docs/decisions/0015-add-scope-user-id-for-jwt-password-grant.rst similarity index 94% rename from openedx/core/djangoapps/oauth_dispatch/docs/decisions/0015-add-scope-user-id-for-jwt.rst rename to openedx/core/djangoapps/oauth_dispatch/docs/decisions/0015-add-scope-user-id-for-jwt-password-grant.rst index 4bd15b651734..90476e8f5bd6 100644 --- a/openedx/core/djangoapps/oauth_dispatch/docs/decisions/0015-add-scope-user-id-for-jwt.rst +++ b/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 ------ diff --git a/openedx/core/djangoapps/oauth_dispatch/docs/decisions/0016-add-scope-user-id-for-jwt-client-credentials.rst b/openedx/core/djangoapps/oauth_dispatch/docs/decisions/0016-add-scope-user-id-for-jwt-client-credentials.rst new file mode 100644 index 000000000000..7ec2e6d893c9 --- /dev/null +++ b/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. \ No newline at end of file diff --git a/openedx/core/djangoapps/oauth_dispatch/dot_overrides/validators.py b/openedx/core/djangoapps/oauth_dispatch/dot_overrides/validators.py index 0b2ad35b2f42..83a17e16084b 100644 --- a/openedx/core/djangoapps/oauth_dispatch/dot_overrides/validators.py +++ b/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) diff --git a/openedx/core/djangoapps/oauth_dispatch/scopes.py b/openedx/core/djangoapps/oauth_dispatch/scopes.py index b9070ddc51ad..bcaf6df91f7e 100644 --- a/openedx/core/djangoapps/oauth_dispatch/scopes.py +++ b/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 diff --git a/openedx/core/djangoapps/oauth_dispatch/tests/test_dot_overrides.py b/openedx/core/djangoapps/oauth_dispatch/tests/test_dot_overrides.py index dc38b18c6eef..7c12965cf684 100644 --- a/openedx/core/djangoapps/oauth_dispatch/tests/test_dot_overrides.py +++ b/openedx/core/djangoapps/oauth_dispatch/tests/test_dot_overrides.py @@ -5,6 +5,7 @@ # pylint: disable=protected-access import datetime +from unittest import mock from django.conf import settings from django.test import RequestFactory, TestCase @@ -12,6 +13,7 @@ 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' diff --git a/openedx/core/djangoapps/oauth_dispatch/tests/test_scopes.py b/openedx/core/djangoapps/oauth_dispatch/tests/test_scopes.py index 5b68f6f14c53..d4545758c1b5 100644 --- a/openedx/core/djangoapps/oauth_dispatch/tests/test_scopes.py +++ b/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)