Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: upgrading django-auth-toolkit and oauthlib. #30634

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions common/djangoapps/third_party_auth/lti.py
Expand Up @@ -11,8 +11,8 @@
from oauthlib.common import Request
from oauthlib.oauth1.rfc5849.signature import (
collect_parameters,
construct_base_string,
normalize_base_string_uri,
signature_base_string,
base_string_uri,
normalize_parameters,
sign_hmac_sha1
)
Expand Down Expand Up @@ -154,10 +154,10 @@ def _get_validated_lti_params_from_values(cls, request, current_time,
# we proceed through the entire validation before rejecting any request for any reason.
# However, as noted there, the value of doing this is dubious.
try:
base_uri = normalize_base_string_uri(request.uri)
base_uri = base_string_uri(request.uri)
parameters = collect_parameters(uri_query=request.uri_query, body=request.body)
parameters_string = normalize_parameters(parameters)
base_string = construct_base_string(request.http_method, base_uri, parameters_string)
base_string = signature_base_string(request.http_method, base_uri, parameters_string)

computed_signature = sign_hmac_sha1(base_string, str(lti_consumer_secret), '')
submitted_signature = request.oauth_signature
Expand Down
5 changes: 4 additions & 1 deletion openedx/core/djangoapps/oauth_dispatch/api.py
Expand Up @@ -33,7 +33,10 @@ def create_dot_access_token(request, user, client, expires_in=None, scopes=None)
request_validator=dot_settings.OAUTH2_VALIDATOR_CLASS(),
)
_populate_create_access_token_request(request, user, client, scopes)
return token_generator.create_token(request, refresh_token=True)
token = token_generator.create_token(request, refresh_token=True)
token_generator.request_validator.save_token(token, request)
Copy link
Contributor Author

@awais786 awais786 Jun 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue about OIDC request_type "code token" does not save access token.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, the old create_token() method had a save_token optional parameter that defaulted to True. Since that param was dropped, we have replaced it with this explicit call to save_token. This looks fine.


return token


def _get_expires_in_value(expires_in):
Expand Down
11 changes: 5 additions & 6 deletions requirements/constraints.txt
Expand Up @@ -19,6 +19,11 @@ celery>=5.2.2,<6.0.0
# required for celery>=5.2.0;<5.3.0
click>=8.0,<9.0

# greater version has breaking changes.
# https://github.com/jazzband/django-oauth-toolkit/blob/master/CHANGELOG.md#200-2022-04-24
django-oauth-toolkit<2.0.0


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: single line break for consistency.

# django-storages version 1.9 drops support for boto storage backend.
django-storages<1.9

Expand All @@ -27,12 +32,6 @@ django-storages<1.9
# for them.
edx-enterprise==3.57.1

# oauthlib>3.0.1 causes test failures ( also remove the django-oauth-toolkit constraint when this is fixed )
oauthlib==3.0.1

# django-auth-toolkit==1.3.3 requires oauthlib>=3.1.0 which is pinned because of test failures
django-oauth-toolkit<=1.3.2

# Will be updated once we update python-dateutil package
matplotlib<3.4.0

Expand Down
5 changes: 2 additions & 3 deletions requirements/edx/base.txt
Expand Up @@ -328,7 +328,7 @@ django-multi-email-field==0.6.2
# via edx-enterprise
django-mysql==4.7.1
# via -r requirements/edx/base.in
django-oauth-toolkit==1.3.2
django-oauth-toolkit==1.7.1
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/base.in
Expand Down Expand Up @@ -751,9 +751,8 @@ numpy==1.22.4
# chem
# openedx-calc
# scipy
oauthlib==3.0.1
oauthlib==3.2.0
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/base.in
# django-oauth-toolkit
# lti-consumer-xblock
Expand Down
14 changes: 6 additions & 8 deletions requirements/edx/development.txt
Expand Up @@ -429,7 +429,7 @@ django-multi-email-field==0.6.2
# edx-enterprise
django-mysql==4.7.1
# via -r requirements/edx/testing.txt
django-oauth-toolkit==1.3.2
django-oauth-toolkit==1.7.1
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/testing.txt
Expand Down Expand Up @@ -701,7 +701,7 @@ execnet==1.9.0
# pytest-xdist
factory-boy==3.2.1
# via -r requirements/edx/testing.txt
faker==15.0.0

# via
# -r requirements/edx/testing.txt
# factory-boy
Expand Down Expand Up @@ -846,6 +846,7 @@ jsonschema==4.16.0
jwcrypto==1.4.2
# via
# -r requirements/edx/testing.txt
# django-oauth-toolkit
# pylti1p3
kombu==5.2.4
# via
Expand Down Expand Up @@ -980,9 +981,8 @@ numpy==1.22.4
# chem
# openedx-calc
# scipy
oauthlib==3.0.1
oauthlib==3.2.0
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/testing.txt
# django-oauth-toolkit
# lti-consumer-xblock
Expand All @@ -999,9 +999,7 @@ openedx-filters==0.8.0
# via
# -r requirements/edx/testing.txt
# lti-consumer-xblock
optimizely-sdk==4.1.0
# via -r requirements/edx/testing.txt
ora2==4.5.1

# via -r requirements/edx/testing.txt
oscrypto==1.3.0
# via
Expand Down Expand Up @@ -1601,7 +1599,7 @@ urllib3==1.26.12
# tableauserverclient
user-util==1.0.0
# via -r requirements/edx/testing.txt
uvicorn==0.18.3

# via
# -r requirements/edx/testing.txt
# pact-python
Expand Down
14 changes: 6 additions & 8 deletions requirements/edx/testing.txt
Expand Up @@ -411,7 +411,7 @@ django-multi-email-field==0.6.2
# edx-enterprise
django-mysql==4.7.1
# via -r requirements/edx/base.txt
django-oauth-toolkit==1.3.2
django-oauth-toolkit==1.7.1
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/base.txt
Expand Down Expand Up @@ -678,7 +678,7 @@ execnet==1.9.0
# via pytest-xdist
factory-boy==3.2.1
# via -r requirements/edx/testing.in
faker==15.0.0

# via factory-boy
fastapi==0.85.0
# via pact-python
Expand Down Expand Up @@ -811,6 +811,7 @@ jsonschema==4.16.0
jwcrypto==1.4.2
# via
# -r requirements/edx/base.txt
# django-oauth-toolkit
# pylti1p3
kombu==5.2.4
# via
Expand Down Expand Up @@ -932,9 +933,8 @@ numpy==1.22.4
# chem
# openedx-calc
# scipy
oauthlib==3.0.1
oauthlib==3.2.0
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/base.txt
# django-oauth-toolkit
# lti-consumer-xblock
Expand All @@ -951,9 +951,7 @@ openedx-filters==0.8.0
# via
# -r requirements/edx/base.txt
# lti-consumer-xblock
optimizely-sdk==4.1.0
# via -r requirements/edx/base.txt
ora2==4.5.1

# via -r requirements/edx/base.txt
oscrypto==1.3.0
# via
Expand Down Expand Up @@ -1491,7 +1489,7 @@ urllib3==1.26.12
# tableauserverclient
user-util==1.0.0
# via -r requirements/edx/base.txt
uvicorn==0.18.3

# via pact-python
vine==5.0.0
# via
Expand Down