Added pkce on client side for authorization grant flow. Test cases added#743
Added pkce on client side for authorization grant flow. Test cases added#743amans330 wants to merge 5 commits intooauthlib:masterfrom
Conversation
|
@auvipy I noticed a build fail. Though it doesn't seem to be caused by my code change. Anything I need to do about this? |
|
@thedrow will finish it by eod today. Just starting a new job today! |
|
Hi @JonathanHuot I made the changes based on the above comments. Please review. |
Merge branch 'master' into pkce
|
@JonathanHuot This PR is awaiting review from you. |
JonathanHuot
left a comment
There was a problem hiding this comment.
Great PR so far, I have added minor comments inline.
Also, I think it is worth to put additional python functions to compute code_challenge_value and code_verifier. The oauthlib framework still needs to be open to any custom values, but at the same time, by default, it should prepare the ground for the implementor. For example, see the RFC:
The client first creates a code verifier, "code_verifier", for each
OAuth 2.0 [RFC6749] Authorization Request, in the following manner:
code_verifier = high-entropy cryptographic random STRING using the
unreserved characters [A-Z] / [a-z] / [0-9] / "-" / "." / "_" / "~"
from Section 2.3 of [RFC3986], with a minimum length of 43 characters
and a maximum length of 128 characters.
ABNF for "code_verifier" is as follows.
code-verifier = 43*128unreserved
unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
ALPHA = %x41-5A / %x61-7A
DIGIT = %x30-39
NOTE: The code verifier SHOULD have enough entropy to make it
impractical to guess the value. It is RECOMMENDED that the output of
a suitable random number generator be used to create a 32-octet
sequence. The octet sequence is then base64url-encoded to produce a
43-octet URL safe string to use as the code verifier.
and
The client then creates a code challenge derived from the code
verifier by using one of the following transformations on the code
verifier:
plain
code_challenge = code_verifier
S256
code_challenge = BASE64URL-ENCODE(SHA256(ASCII(code_verifier)))
If the client is capable of using "S256", it MUST use "S256", as
"S256" is Mandatory To Implement (MTI) on the server. Clients are
permitted to use "plain" only if they cannot support "S256" for some
technical reason and know via out-of-band configuration that the
server supports "plain".
By doing this in oauthlib side, it will prevent implementor to do mistake during the implementation and will be very easy to implement it right.
Thoughts?
| to the client. The parameter SHOULD be used for preventing | ||
| cross-site request forgery as described in `Section 10.12`_. | ||
|
|
||
| :param code_challenge: OPTIONAL. A challenge derived from the code verifier that is sent in the |
There was a problem hiding this comment.
The "OPTIONAL" of code_challenge is a bit misleading compared to the same for code_challenge_method.
If we are handling a client with PKCE enforced, the code_challenge is REQUIRED, and method is always OPTIONAL, but also ignored in case of client is not using PKCE.
What do you think ?
| authorization request as described in | ||
| `Section 4.1.1`_, and their values MUST be identical. * | ||
|
|
||
| :param code_verifier: A cryptographically random string that is used to correlate the |
There was a problem hiding this comment.
same here, I think because these are options only for Authorization Server supporting PKCE and with clients with PKCE enabled (or enforced), we should somehow, find a way to specify it is a PKCE parameter only.
|
Hi, @JonathanHuot, I'd love to see this capability/enhancement in |
Hi, Please review my pull request for PKCE implementation on the client-side. Do let me know if any changes are required.