fix: include full-complement paramters in JWT_PRIVATE_SIGNING_JWK
#925
Conversation
Addresses the breaking upstream change in this commit: openedx/edx-platform@92731be See Changelog entry for details.
| @@ -189,6 +189,9 @@ | |||
| "n": "{{ jwt_rsa_key.n|long_to_base64 }}", | |||
| "p": "{{ jwt_rsa_key.p|long_to_base64 }}", | |||
| "q": "{{ jwt_rsa_key.q|long_to_base64 }}", | |||
| "dq": "{{ jwt_rsa_key.dq|long_to_base64 }}", | |||
| "dp": "{{ jwt_rsa_key.dp|long_to_base64 }}", | |||
| "qi": "{{ jwt_rsa_key.invq|long_to_base64 }}", | |||
There was a problem hiding this comment.
Some notes on confirming that this is correct:
RSA private numbers are represented with slightly different names in different libraries. n, e, d, p, and q always seem to have the same names. dp and dq aren't documented in pycryptodome (which is what Tutor is using here) but apparently they're present and have a matching definition (d mod (p - 1) and likewise for q.) For qi we can compare with PyJWT's translation from the cryptography library and see that cryptography's iqmp has the same definition as PyCryptodome's invq.
(The JWK spec doesn't actually define what qi is, but instead refers the reader to several print books.)
|
Let's merge this change and test it in quince. |
|
Wait... My understanding was that this change was required in Quince. Did I get that correctly @kdmccormick? |
|
@regisb We need the change on Nightly. I don't think we need it in Quince since the breaking edx-platform changed merged after the cut, but it's totally OK to have it in Quince regardless. |
Addresses the breaking upstream change in this commit: openedx/edx-platform@92731be
Relevant discussions:
See Changelog entry for details.
@timmc-edx , could you sanity check this?
HEADS UP REVIEWERS: I don't exactly grok how JWTs and JWKs work and whether this fix is safe. I hope one of you do. If not, we should find someone who does.