-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
chore: Remove edx-token-utils dependency #36077
Open
rijuma
wants to merge
8
commits into
master
Choose a base branch
from
rijuma/remove-edx-token-utils-dep
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+398
−128
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
e192f07
chore: Removed edx-token-utils dep and moved necessary logic to the repo
rijuma d49ec74
chore: Removed unused dependency
rijuma 7391ed7
chore: Run make compile-requirements to update dependencies
rijuma 4c09f40
chore: fix tests
rijuma 1aef9c7
chore: Moved jwt file to openedx.core.lib
rijuma 10c09c1
chore: Upgraded dependencies
rijuma 4c63afe
chore: Updated TOKEN_SIGNING on cms
rijuma c372f0c
chore: Updated defaults for token handling on CMS
rijuma File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -657,3 +657,35 @@ | |
# case of new django version these values will override. | ||
if django.VERSION[0] >= 4: # for greater than django 3.2 use with schemes. | ||
CSRF_TRUSTED_ORIGINS = CSRF_TRUSTED_ORIGINS_WITH_SCHEME | ||
|
||
|
||
############## Settings for JWT token handling ############## | ||
TOKEN_SIGNING = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there docs for how to generate a valid |
||
'JWT_ISSUER': 'token-test-issuer', | ||
'JWT_SIGNING_ALGORITHM': 'RS512', | ||
'JWT_SUPPORTED_VERSION': '1.2.0', | ||
'JWT_PRIVATE_SIGNING_JWK': '''{ | ||
"e": "AQAB", | ||
"d": "HIiV7KNjcdhVbpn3KT-I9n3JPf5YbGXsCIedmPqDH1d4QhBofuAqZ9zebQuxkRUpmqtYMv0Zi6ECSUqH387GYQF_XvFUFcjQRPycISd8TH0DAKaDpGr-AYNshnKiEtQpINhcP44I1AYNPCwyoxXA1fGTtmkKChsuWea7o8kytwU5xSejvh5-jiqu2SF4GEl0BEXIAPZsgbzoPIWNxgO4_RzNnWs6nJZeszcaDD0CyezVSuH9QcI6g5QFzAC_YuykSsaaFJhZ05DocBsLczShJ9Omf6PnK9xlm26I84xrEh_7x4fVmNBg3xWTLh8qOnHqGko93A1diLRCrKHOvnpvgQ", | ||
"n": "o5cn3ljSRi6FaDEKTn0PS-oL9EFyv1pI7dRgffQLD1qf5D6sprmYfWWokSsrWig8u2y0HChSygR6Jn5KXBqQn6FpM0dDJLnWQDRXHLl3Ey1iPYgDSmOIsIGrV9ZyNCQwk03wAgWbfdBTig3QSDYD-sTNOs3pc4UD_PqAvU2nz_1SS2ZiOwOn5F6gulE1L0iE3KEUEvOIagfHNVhz0oxa_VRZILkzV-zr6R_TW1m97h4H8jXl_VJyQGyhMGGypuDrQ9_vaY_RLEulLCyY0INglHWQ7pckxBtI5q55-Vio2wgewe2_qYcGsnBGaDNbySAsvYcWRrqDiFyzrJYivodqTQ", | ||
"q": "3T3DEtBUka7hLGdIsDlC96Uadx_q_E4Vb1cxx_4Ss_wGp1Loz3N3ZngGyInsKlmbBgLo1Ykd6T9TRvRNEWEtFSOcm2INIBoVoXk7W5RuPa8Cgq2tjQj9ziGQ08JMejrPlj3Q1wmALJr5VTfvSYBu0WkljhKNCy1KB6fCby0C9WE", | ||
"p": "vUqzWPZnDG4IXyo-k5F0bHV0BNL_pVhQoLW7eyFHnw74IOEfSbdsMspNcPSFIrtgPsn7981qv3lN_staZ6JflKfHayjB_lvltHyZxfl0dvruShZOx1N6ykEo7YrAskC_qxUyrIvqmJ64zPW3jkuOYrFs7Ykj3zFx3Zq1H5568G0", | ||
"kid": "token-test-sign", "kty": "RSA" | ||
}''', | ||
'JWT_PUBLIC_SIGNING_JWK_SET': '''{ | ||
"keys": [ | ||
{ | ||
"kid":"token-test-wrong-key", | ||
"e": "AQAB", | ||
"kty": "RSA", | ||
"n": "o5cn3ljSRi6FaDEKTn0PS-oL9EFyv1pI7dffgRQLD1qf5D6sprmYfWVokSsrWig8u2y0HChSygR6Jn5KXBqQn6FpM0dDJLnWQDRXHLl3Ey1iPYgDSmOIsIGrV9ZyNCQwk03wAgWbfdBTig3QSDYD-sTNOs3pc4UD_PqAvU2nz_1SS2ZiOwOn5F6gulE1L0iE3KEUEvOIagfHNVhz0oxa_VRZILkzV-zr6R_TW1m97h4H8jXl_VJyQGyhMGGypuDrQ9_vaY_RLEulLCyY0INglHWQ7pckxBtI5q55-Vio2wgewe2_qYcGsnBGaDNbySAsvYcWRrqDiFyzrJYivodqTQ" | ||
}, | ||
{ | ||
"kid":"token-test-sign", | ||
"e": "AQAB", | ||
"kty": "RSA", | ||
"n": "o5cn3ljSRi6FaDEKTn0PS-oL9EFyv1pI7dRgffQLD1qf5D6sprmYfWWokSsrWig8u2y0HChSygR6Jn5KXBqQn6FpM0dDJLnWQDRXHLl3Ey1iPYgDSmOIsIGrV9ZyNCQwk03wAgWbfdBTig3QSDYD-sTNOs3pc4UD_PqAvU2nz_1SS2ZiOwOn5F6gulE1L0iE3KEUEvOIagfHNVhz0oxa_VRZILkzV-zr6R_TW1m97h4H8jXl_VJyQGyhMGGypuDrQ9_vaY_RLEulLCyY0INglHWQ7pckxBtI5q55-Vio2wgewe2_qYcGsnBGaDNbySAsvYcWRrqDiFyzrJYivodqTQ" | ||
} | ||
] | ||
}''', | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
""" | ||
JWT Token handling and signing functions. | ||
""" | ||
|
||
import json | ||
from time import time | ||
|
||
from django.conf import settings | ||
from jwkest import Expired, Invalid, MissingKey, jwk | ||
from jwkest.jws import JWS | ||
|
||
|
||
def create_jwt(lms_user_id, expires_in_seconds, additional_token_claims, now=None): | ||
""" | ||
Produce an encoded JWT (string) indicating some temporary permission for the indicated user. | ||
|
||
What permission that is must be encoded in additional_claims. | ||
Arguments: | ||
lms_user_id (int): LMS user ID this token is being generated for | ||
expires_in_seconds (int): Time to token expiry, specified in seconds. | ||
additional_token_claims (dict): Additional claims to include in the token. | ||
now(int): optional now value for testing | ||
""" | ||
now = now or int(time()) | ||
|
||
payload = { | ||
'lms_user_id': lms_user_id, | ||
'exp': now + expires_in_seconds, | ||
'iat': now, | ||
'iss': settings.TOKEN_SIGNING['JWT_ISSUER'], | ||
'version': settings.TOKEN_SIGNING['JWT_SUPPORTED_VERSION'], | ||
} | ||
payload.update(additional_token_claims) | ||
return _encode_and_sign(payload) | ||
|
||
|
||
def _encode_and_sign(payload): | ||
""" | ||
Encode and sign the provided payload. | ||
|
||
The signing key and algorithm are pulled from settings. | ||
""" | ||
keys = jwk.KEYS() | ||
|
||
serialized_keypair = json.loads(settings.TOKEN_SIGNING['JWT_PRIVATE_SIGNING_JWK']) | ||
keys.add(serialized_keypair) | ||
algorithm = settings.TOKEN_SIGNING['JWT_SIGNING_ALGORITHM'] | ||
|
||
data = json.dumps(payload) | ||
jws = JWS(data, alg=algorithm) | ||
return jws.sign_compact(keys=keys) | ||
|
||
|
||
def unpack_jwt(token, lms_user_id, now=None): | ||
""" | ||
Unpack and verify an encoded JWT. | ||
|
||
Validate the user and expiration. | ||
|
||
Arguments: | ||
token (string): The token to be unpacked and verified. | ||
lms_user_id (int): LMS user ID this token should match with. | ||
now (int): Optional now value for testing. | ||
|
||
Returns a valid, decoded json payload (string). | ||
""" | ||
now = now or int(time()) | ||
payload = _unpack_and_verify(token) | ||
|
||
if "lms_user_id" not in payload: | ||
raise MissingKey("LMS user id is missing") | ||
if "exp" not in payload: | ||
raise MissingKey("Expiration is missing") | ||
if payload["lms_user_id"] != lms_user_id: | ||
raise Invalid("User does not match") | ||
if payload["exp"] < now: | ||
raise Expired("Token is expired") | ||
|
||
return payload | ||
|
||
|
||
def _unpack_and_verify(token): | ||
""" | ||
Unpack and verify the provided token. | ||
|
||
The signing key and algorithm are pulled from settings. | ||
""" | ||
keys = jwk.KEYS() | ||
keys.load_jwks(settings.TOKEN_SIGNING['JWT_PUBLIC_SIGNING_JWK_SET']) | ||
decoded = JWS().verify_compact(token.encode('utf-8'), keys) | ||
return decoded |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you add this here because the new tests you added are running in both the LMS and CMS environments? If so, I think that's not necessary and we can just run them in one of the two shards. So we could remove this and the block in
cms/envs/test.py
and then use the@skip_unless_lms
decorator to skip these tests in the CMS context.