Skip to content

Commit

Permalink
feat: Add scope user_id to JWT payload (openedx#33455)
Browse files Browse the repository at this point in the history
  • Loading branch information
moeez96 authored Oct 30, 2023
1 parent e6e124b commit ba1f382
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 9 deletions.
1 change: 1 addition & 0 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -1246,6 +1246,7 @@
'certificates:read': _('Retrieve your course certificates'),
'grades:read': _('Retrieve your grades for your enrolled courses'),
'tpa:read': _('Retrieve your third-party authentication username mapping'),
# user_id is added in code as a default scope for JWT cookies and all password grant_type JWTs
'user_id': _('Know your user identifier'),
}),
'DEFAULT_SCOPES': OAUTH2_DEFAULT_SCOPES,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
15. Add scope user_id for JWT token
###################################

Status
------

Accepted

Context
-------

In Feb 2018, to enable analytics (Segment) from Microfrontends (MFEs), a ``user_id`` claim was added to the JWT token `in this PR<https://github.com/openedx/edx-platform/pull/19765>`__.

The LMS API `to create authentication tokens`_ is used by external organizations to request a token on behalf of their users, mostly using grant_type ``client_credentials`` in the request. Since ``user_id`` is considered sensitive information, especially when combined with email and username which were already available in the JWT, it was decided to only add the ``user_id`` claim when a ``user_id`` scope was supplied. All MFE JWT cookies, which are known to only be used directly by the user, automatically used the ``user_id`` scope in order to get the required ``user_id`` claim.

No ADR could be found for the Feb 2018 decision detailed above.

In June 2019, an `ADR was captured in ecommerce`_ around the requirements to have the LMS user_id available for requests to ecommerce.

In 2022, the mobile apps switched to using JWTs for authentication. However, these JWTs were missing the ``user_id`` scope and claim required by the ecommerce service.

.. _to create authentication tokens: https://github.com/openedx/edx-platform/blob/caf8e456e28f9b9a1f5fa7186d3d155112fb75be/openedx/core/djangoapps/oauth_dispatch/urls.py#L14
.. _ADR was captured in ecommerce: https://github.com/openedx/ecommerce/blob/master/docs/decisions/0004-unique-identifier-for-users.rst

Decisions
---------

- The original decision to add the ``user_id`` claim to the JWT token using the ``user_id`` scope has been captured in the context of this ADR, because no ADR could be found.
- The scope ``user_id`` will be added to all requests having grant_type ``password`` in the API `/oauth2/access_token/`.

Consequences
------------

- The claim ``user_id`` will be present in the JWT token for all requesters who already have access to the login credentials of the user account.
- The ``user_id`` scope will continue to protect other JWT requests that don't require this sensitive information.
- This pattern could potentially be used to clean-up the manually added ``user_id`` scope for oauth clients involved in the social auth flow in the future.
29 changes: 24 additions & 5 deletions openedx/core/djangoapps/oauth_dispatch/jwt.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from edx_toggles.toggles import SettingToggle
from jwt import PyJWK
from jwt.utils import base64url_encode
from oauth2_provider.models import Application

from common.djangoapps.student.models import UserProfile, anonymous_id_for_user

Expand Down Expand Up @@ -79,10 +80,11 @@ def create_jwt_token_dict(token_dict, oauth_adapter, use_asymmetric_key=None):
# .. custom_attribute_name: create_jwt_grant_type
# .. custom_attribute_description: The grant type of the newly created JWT.
set_custom_attribute('create_jwt_grant_type', grant_type)
scopes = _get_updated_scopes(token_dict['scope'].split(' '), grant_type)

jwt_access_token = _create_jwt(
access_token.user,
scopes=token_dict['scope'].split(' '),
scopes=scopes,
expires_in=jwt_expires_in,
use_asymmetric_key=use_asymmetric_key,
is_restricted=oauth_adapter.is_client_restricted(client),
Expand All @@ -91,11 +93,16 @@ def create_jwt_token_dict(token_dict, oauth_adapter, use_asymmetric_key=None):
)

jwt_token_dict = token_dict.copy()
# Note: only "scope" is not overwritten at this point.
# Note: only "refresh_token" is not overwritten at this point.
# At this time, the user_id scope added for grant type password is only added to the
# JWT, and is not added for the DOT access token or refresh token, so we must override
# here. If this inconsistency becomes an issue, then the user_id scope should be
# added earlier with the DOT tokens, and we would no longer need to override "scope".
jwt_token_dict.update({
"access_token": jwt_access_token,
"token_type": "JWT",
"expires_in": jwt_expires_in,
"scope": ' '.join(scopes),
})
return jwt_token_dict

Expand Down Expand Up @@ -167,9 +174,7 @@ def _create_jwt(
else:
increment('create_symmetric_jwt_count')

# Default scopes should only contain non-privileged data.
# Do not be misled by the fact that `email` and `profile` are default scopes. They
# were included for legacy compatibility, even though they contain privileged data.
# Scopes `email` and `profile` are included for legacy compatibility.
scopes = scopes or ['email', 'profile']
iat, exp = _compute_time_fields(expires_in)

Expand Down Expand Up @@ -285,3 +290,17 @@ def _encode_and_sign(payload, use_asymmetric_key, secret):

jwk = PyJWK(key, algorithm)
return jwt.encode(payload, jwk.key, algorithm=algorithm)


def _get_updated_scopes(scopes, grant_type):
"""
Default scopes should only contain non-privileged data.
Do not be misled by the fact that `email` and `profile` are default scopes.
They were included for legacy compatibility, even though they contain privileged
data. The scope `user_id` must be added for requests with grant_type password.
"""
scopes = scopes or ['email', 'profile']

if grant_type == Application.GRANT_PASSWORD and 'user_id' not in scopes:
scopes.append('user_id')
return scopes
3 changes: 2 additions & 1 deletion openedx/core/djangoapps/oauth_dispatch/tests/test_jwt.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ def setUp(self):
super().setUp()
self.user = UserFactory()
self.default_scopes = ['email', 'profile']
self.default_scopes_password_grant_type = ['email', 'profile', 'user_id']

def _create_client(self, oauth_adapter, client_restricted, grant_type=None):
"""
Expand Down Expand Up @@ -176,7 +177,7 @@ def test_password_grant_type(self):
jwt_token_dict = jwt_api.create_jwt_token_dict(token_dict, oauth_adapter, use_asymmetric_key=False)

self.assert_valid_jwt_access_token(
jwt_token_dict["access_token"], self.user, self.default_scopes,
jwt_token_dict["access_token"], self.user, self.default_scopes_password_grant_type,
grant_type='password',
)

Expand Down
10 changes: 7 additions & 3 deletions openedx/core/djangoapps/oauth_dispatch/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,17 +315,21 @@ def test_jwt_access_token_scopes_and_filters(self, grant_type):
scopes=['grades:read'],
filters=['test:filter'],
)
scopes = dot_app_access.scopes
requested_scopes = dot_app_access.scopes
filters = self.dot_adapter.get_authorization_filters(dot_app)
assert 'test:filter' in filters

response = self._post_request(self.user, dot_app, token_type='jwt', scope=scopes)
response = self._post_request(self.user, dot_app, token_type='jwt', scope=requested_scopes)
assert response.status_code == 200
data = json.loads(response.content.decode('utf-8'))
scopes_in_response = data['scope'].split(' ')
for requested_scope in requested_scopes:
assert requested_scope in scopes_in_response

self.assert_valid_jwt_access_token(
data['access_token'],
self.user,
scopes,
scopes_in_response,
filters=filters,
grant_type=grant_type,
)
Expand Down

0 comments on commit ba1f382

Please sign in to comment.