panva / openid-client

OAuth 2 / OpenID Connect Client API for JavaScript Runtimes
MIT License
1.83k stars 392 forks source link

Code_verifier check is implemented but not documented? #152

Closed tetchel closed 5 years ago

tetchel commented 5 years ago

Hey, the documentation at https://github.com/panva/node-openid-client#processing-callback states:

Aside from state and response_type, checks for nonce (implicit and hybrid responses) and max_age are implemented. 
id_token signature and claims validation does not need to be requested, it is done automatically.

Looking at the code, it looks like a code_verifier check is also implemented for use with PKCE. However, code_verifier is not mentioned anywhere in the doc, and PKCE is only mentioned in the "Usage with passport" section.

Maybe code_verifier could be documented as one of the available checks that can be passed to the authorizationCallback function.

panva commented 5 years ago

Hi @tetchel, can you propose a doc update and send a PR?

I’d like to see a complete section on using S256 pkce in auth code flow.

tetchel commented 5 years ago

The documentation at https://auth0.com/docs/api-auth/tutorials/authorization-code-grant-pkce is very good for explaining PKCE, combined of course with the relevant RFC https://tools.ietf.org/html/rfc7636. Does the first link match what you have in mind?

tetchel commented 5 years ago

https://openid.net/2015/05/26/enhancing-oauth-security-for-mobile-applications-with-pkse/ is a great reference too, explaining why PKCE is necessary to secure public clients.

panva commented 5 years ago

Does the first link match what you have in mind?

Nope, the documentation, if improved, should instruct how to use PKCE with this software, not what it is in general.

Makes sense?

BTW thanks for bringing this up

tetchel commented 5 years ago

OK, combined with https://github.com/panva/node-openid-client/issues/150#issuecomment-466070949 I could come up with an example auth code flow using PKCE when I get a chance.

A minimal 'fix' for this might be just adding a couple words to the README section I posted above (bold is new):

Aside from state and response_type, checks for nonce (implicit and hybrid responses), max_age, and code_verifier (for use with PKCE) are implemented. id_token signature and claims validation does not need to be requested, it is done automatically.

I can open a PR if you like that change.

panva commented 5 years ago

sure

panva commented 5 years ago

Thank you