Skip to content

Commit

Permalink
feat: Added token grant_type in JWT payload
Browse files Browse the repository at this point in the history
To exchange jwt with session cookies we need to determine JWT grant type in
AccessTokenExchangeView. JWT only having password grant type will be allowed to exchange session.
Added ADR for mobile migration to JWT authentication.

LEARNER-8886
  • Loading branch information
jawad-khan authored and robrap committed Jun 24, 2022
1 parent 20de3c7 commit 81b44b3
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
13. Mobile migration to JWT
###########################

Status
------

Accepted

Context
-------

The Open edX mobile apps contain the final usage of the deprecated opaque Bearer access tokens, as documented in the `OAuth2 and Bearer Tokens section of OEP-42 on Authentication`_. Once the mobile apps are no longer using these opaque tokens, BearerAuthentication could be removed through the DEPR process.

Additionally, the Open edX mobile apps plan to add payment features which require calling some ecommerce apis. However, the opaque Bearer access tokens can only be used for authentication and api calls in the lms. As detailed in OEP-42, the JWT access token is the recommended method of authenticating across services.

Until this time, JWT access tokens were not supported for parts of the mobile authentication workflow. Once JWT authentication is enabled for mobile, most of the required apis for the new features already support JWT, and the ones that don't can be easily updated.

.. _OAuth2 and Bearer Tokens section of OEP-42 on Authentication: https://github.com/openedx/open-edx-proposals/blob/6accfc7d5440c9c02f0c17e6ce65c7141af9551f/oeps/best-practices/oep-0042-bp-authentication.rst#oauth2-and-bearer-tokens

Decisions
---------

The mobile apps will migrate its authentication worklflow from using opaque Bearer access tokens to JWT access tokens for authentication, enabling the use of JWTs for cross-service authenticated api calls.

The mobile app currently obtains an edX-issued access token in either of the following ways:

* ``AccessToken View: username/email + password combo with grant_type=password``
* ``AccessTokenExchangeView: 3rd party (social-auth) OAuth 2.0 access token -> 1st party (Open edX) OAuth 2.0 access token``

Additionally, the mobile app can exchange an access token for a session cookie that can be used in a WebView:

* ``LoginWithAccessTokenView: 1st party (Open edX) OAuth 2.0 access token -> session cookie``

The above three endpoints will be updated to also support JWTs. The mobile apps will then use ``JwtAuthentication`` on all the apis they call. Note: almost all the apis already support ``JwtAuthentication``.

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

For migrating the mobile authentication flow from opaque Bearer access tokens to JWTs, the following security risks were identified, and will be mitigated as detailed below:

* Reduce JWT access token expiration.

* One major difference between JWT and Bearer access tokens is that the JWT is non-revocable. Intentionally, there is no database lookup for a JWT and it is simply trusted if found to be valid.
* One consequence of this is that a JWT should have a short lifetime in order to limit security risks if the token is hijacked.
* JWT access token expiration time needs to be configurable separately from the opaque Bearer tokens to enable this change. (Completed as of the writing of this ADR.)

* User Account Status:

* For 1st-party token exchange for session login using JWTs, it is especially important to ensure that the user account has not been disabled, since the JWT is non-revocable.

* Password Grant Check

* For the currently proposed 1st-party token exchange for session login using JWTs, we would need an equivalent check to the existing `_is_grant_password` to not expand permissiveness of the endpoint.
* For resolution, see the `ADR to add grant type in JWT payload`_.

* Asymmetrically Signed JWT

* We need to check if the JWT was asymmetrically signed by the LMS. We want to ensure that a symmetrically signed JWT, created (signed) by another IDA, could not be compromised and used by an attacker to exchange for a session cookie, which would allow for full compromise of the user.
* Implementation will involve adding a method to ``edx-drf-extensions`` like ``get_decoded_jwt_from_auth``, but that will decode only asymmetric JWTs.

.. _ADR to add grant type in JWT payload: https://github.com/edx/edx-platform/blob/master/openedx/core/djangoapps/oauth_dispatch/docs/decisions/0014-add-grant-type-in-jwt-payload.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
14. Added grant type in JWT payload
###################################

Status
------

Accepted

Context
-------

Open edX mobile apps are `migrating to JWT token`_ authentication.
As part of the current mobile authentication flow, we exchange an opaque access token for a session.
This is only allowed for access tokens created with the password grant type.
However, as we migrate from using the opaque access token to a JWT, we are no longer able to determine the grant type of the JWT.

.. _migrating to JWT token: 0013-mobile-migration-to-jwt.rst

Decisions
---------

A new claim ``grant_type`` will be added to the payload while creating the JWT.
The claim ``grant_type`` will be determined using the grant type information stored for the django-oauth-toolkit (DOT) opaque access token that the JWT was based off of during the mobile flow.
JWTs created from an access token in mobile flow will have a grant type of ``password``, and can be used for the session exchange.

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

* JWT will have an additional claim "grant_type" in its payload, which we can use to take decisions regarding auth exchange.
18 changes: 18 additions & 0 deletions openedx/core/djangoapps/oauth_dispatch/jwt.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,19 @@


import json
import logging
from time import time

from django.conf import settings
from edx_django_utils.monitoring import set_custom_attribute
from edx_rbac.utils import create_role_auth_claim_for_user
from jwkest import jwk
from jwkest.jws import JWS

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

log = logging.getLogger(__name__)


def create_jwt_for_user(user, secret=None, aud=None, additional_claims=None, scopes=None):
"""
Expand Down Expand Up @@ -63,6 +67,16 @@ def create_jwt_token_dict(token_dict, oauth_adapter, use_asymmetric_key=None):
client = oauth_adapter.get_client_for_token(access_token)

jwt_expires_in = _get_jwt_access_token_expire_seconds()
try:
grant_type = access_token.application.authorization_grant_type
except Exception: # pylint: disable=broad-except
# TODO: Remove this broad except if proven this doesn't happen.
grant_type = 'unknown-error'
log.exception('Unable to get grant_type from access token.')

# .. 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)

jwt_access_token = _create_jwt(
access_token.user,
Expand All @@ -71,6 +85,7 @@ def create_jwt_token_dict(token_dict, oauth_adapter, use_asymmetric_key=None):
use_asymmetric_key=use_asymmetric_key,
is_restricted=oauth_adapter.is_client_restricted(client),
filters=oauth_adapter.get_authorization_filters(client),
grant_type=grant_type,
)

jwt_token_dict = token_dict.copy()
Expand Down Expand Up @@ -120,6 +135,7 @@ def _create_jwt(
additional_claims=None,
use_asymmetric_key=None,
secret=None,
grant_type=None,
):
"""
Returns an encoded JWT (string).
Expand All @@ -132,6 +148,7 @@ def _create_jwt(
expires_in (int): Optional. Overrides time to token expiry, specified in seconds.
filters (list): Optional. Filters to include in the JWT.
is_restricted (Boolean): Whether the client to whom the JWT is issued is restricted.
grant_type (str): grant type of the new JWT token.
Deprecated Arguments (to be removed):
aud (string): Optional. Overrides configured JWT audience claim.
Expand All @@ -152,6 +169,7 @@ def _create_jwt(
# TODO (ARCH-204) Consider getting rid of the 'aud' claim since we don't use it.
'aud': aud if aud else settings.JWT_AUTH['JWT_AUDIENCE'],
'exp': exp,
'grant_type': grant_type or '',
'iat': iat,
'iss': settings.JWT_AUTH['JWT_ISSUER'],
'preferred_username': user.username,
Expand Down
4 changes: 3 additions & 1 deletion openedx/core/djangoapps/oauth_dispatch/tests/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class AccessTokenMixin:

def assert_valid_jwt_access_token(self, access_token, user, scopes=None, should_be_expired=False, filters=None,
should_be_asymmetric_key=False, should_be_restricted=None, aud=None, secret=None,
expires_in=None):
expires_in=None, grant_type=None):
"""
Verify the specified JWT access token is valid, and belongs to the specified user.
Returns:
Expand Down Expand Up @@ -95,6 +95,8 @@ def _decode_jwt(verify_expiration):
if should_be_restricted is not None:
expected['is_restricted'] = should_be_restricted

expected['grant_type'] = grant_type or ''

self.assertDictContainsSubset(expected, payload)

if expires_in:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,4 @@ def test_jwt_access_token(self):
content = json.loads(response.content.decode('utf-8'))
access_token = content['access_token']
assert content['scope'] == data['scope']
self.assert_valid_jwt_access_token(access_token, self.user, scopes)
self.assert_valid_jwt_access_token(access_token, self.user, scopes, grant_type='client-credentials')
37 changes: 34 additions & 3 deletions openedx/core/djangoapps/oauth_dispatch/tests/test_jwt.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def setUp(self):
self.user = UserFactory()
self.default_scopes = ['email', 'profile']

def _create_client(self, oauth_adapter, client_restricted):
def _create_client(self, oauth_adapter, client_restricted, grant_type=None):
"""
Creates and returns an OAuth client using the given oauth_adapter.
Configures the client as a RestrictedApplication if client_restricted is
Expand All @@ -33,14 +33,15 @@ def _create_client(self, oauth_adapter, client_restricted):
user=self.user,
redirect_uri='',
client_id='public-client-id',
grant_type=grant_type or '',
)
if client_restricted:
RestrictedApplication.objects.create(application=client)
return client

def _get_token_dict(self, client_restricted, oauth_adapter):
def _get_token_dict(self, client_restricted, oauth_adapter, grant_type=None):
""" Creates and returns an (opaque) access token dict """
client = self._create_client(oauth_adapter, client_restricted)
client = self._create_client(oauth_adapter, client_restricted, grant_type=grant_type)
expires_in = 60 * 60
expires = now() + timedelta(seconds=expires_in)
token_dict = dict(
Expand Down Expand Up @@ -163,3 +164,33 @@ def test_scopes(self):
assert jwt_payload['scopes'] == self.default_scopes
assert jwt_scopes_payload['scopes'] == scopes
assert jwt_scopes_payload['user_id'] == self.user.id

def test_password_grant_type(self):
oauth_adapter = DOTAdapter()
token_dict = self._get_token_dict(client_restricted=False, oauth_adapter=oauth_adapter, grant_type='password')
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,
grant_type='password',
)

def test_None_grant_type(self):
oauth_adapter = DOTAdapter()
token_dict = self._get_token_dict(client_restricted=False, oauth_adapter=oauth_adapter, grant_type=None)
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,
grant_type='',
)

def test_random_str_grant_type(self):
oauth_adapter = DOTAdapter()
token_dict = self._get_token_dict(client_restricted=False, oauth_adapter=oauth_adapter, grant_type='test rand')
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,
grant_type='test rand',
)
12 changes: 8 additions & 4 deletions openedx/core/djangoapps/oauth_dispatch/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ def _generate_key_pair(self):

return serialized_public_keys_json, serialized_keypair_json

def _test_jwt_access_token(self, client_attr, token_type=None, headers=None):
def _test_jwt_access_token(self, client_attr, token_type=None, headers=None, grant_type=None):
"""
Test response for JWT token.
"""
Expand All @@ -186,6 +186,7 @@ def _test_jwt_access_token(self, client_attr, token_type=None, headers=None):
data['access_token'],
self.user,
data['scope'].split(' '),
grant_type=grant_type,
should_be_restricted=False,
expires_in=expected_default_expires_in,
)
Expand Down Expand Up @@ -217,15 +218,16 @@ def test_restricted_non_jwt_access_token_fields(self):

@ddt.data('dot_app')
def test_jwt_access_token_from_parameter(self, client_attr):
self._test_jwt_access_token(client_attr, token_type='jwt')
self._test_jwt_access_token(client_attr, token_type='jwt', grant_type='password')

@ddt.data('dot_app')
def test_jwt_access_token_from_header(self, client_attr):
self._test_jwt_access_token(client_attr, headers={'HTTP_X_TOKEN_TYPE': 'jwt'})
self._test_jwt_access_token(client_attr, headers={'HTTP_X_TOKEN_TYPE': 'jwt'}, grant_type='password')

@ddt.data('dot_app')
def test_jwt_access_token_from_parameter_not_header(self, client_attr):
self._test_jwt_access_token(client_attr, token_type='jwt', headers={'HTTP_X_TOKEN_TYPE': 'invalid'})
self._test_jwt_access_token(client_attr, token_type='jwt', grant_type='password',
headers={'HTTP_X_TOKEN_TYPE': 'invalid'})

@ddt.data(
('jwt', 'jwt'),
Expand Down Expand Up @@ -274,6 +276,7 @@ def test_restricted_jwt_access_token(self):
should_be_expired=False,
should_be_asymmetric_key=True,
should_be_restricted=True,
grant_type='password'
)

def test_restricted_access_token(self):
Expand Down Expand Up @@ -331,6 +334,7 @@ def test_jwt_access_token_scopes_and_filters(self, grant_type):
self.user,
scopes,
filters=filters,
grant_type=grant_type,
)


Expand Down

0 comments on commit 81b44b3

Please sign in to comment.