diff --git a/openedx/core/djangoapps/oauth_dispatch/docs/decisions/0015-add-scope-user-id-for-jwt-password-grant.rst b/openedx/core/djangoapps/oauth_dispatch/docs/decisions/0015-add-scope-user-id-for-jwt-password-grant.rst index 4bd15b6517..90476e8f5b 100644 --- a/openedx/core/djangoapps/oauth_dispatch/docs/decisions/0015-add-scope-user-id-for-jwt-password-grant.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 index 234ce2a28b..7ec2e6d893 100644 --- 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 @@ -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. \ 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 59ee119a64..ff901d0e9b 100644 --- a/openedx/core/djangoapps/oauth_dispatch/dot_overrides/validators.py +++ b/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 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 18cfe4a0b6..0888372a62 100644 --- a/openedx/core/djangoapps/oauth_dispatch/tests/test_dot_overrides.py +++ b/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'