An option to require PKCE parameters

[saschanaz]

It seems it's fully optional right now:

https://github.com/node-oauth/node-oauth2-server/blob/c993eb5a700f81fe204283b3428e0742015f9b8d/lib/grant-types/authorization-code-grant-type.js#L122-L144

Could be great if there's an option to force it. Of course one can block the request manually by checking the query, though.

[jankapunkt]

I have to read it up again, but I think to remember the standard does not define whether this is to be enforced, which is why it's optional in the first place and considered an implementation detail. However, I will check again and come back to you.

[saschanaz]

Note that there's a relevant attack described in OAuth 2.0 Security Best Current Practice:

4.8. PKCE Downgrade Attack

An authorization server that supports PKCE but does not make its use mandatory for all flows can be susceptible to a PKCE downgrade attack.

[jankapunkt]

@saschanaz I'm definitely with you on this topic, however I need to review the tests and code to see, what is already implemented. It mostly breaks down to this:

This fact can be used to mitigate this attack. [RFC7636] already mandates that

• an AS that supports PKCE MUST check whether a code challenge is contained in the authorization request and bind this information to the code that is issued; and

• when a code arrives at the token endpoint, and there was a code_challenge in the authorization request for which this code was issued, there must be a valid code_verifier in the token request.

Beyond this, to prevent PKCE downgrade attacks, the AS MUST ensure that if there was no code_challenge in the authorization request, a request to the token endpoint containing a code_verifier is rejected. Note: ASs that mandate the use of PKCE in general or for particular clients implicitly implement this security measure.

Would you support us with a review if we provide a PR? Can you test against a real-world setup?

[saschanaz]

Would you support us with a review if we provide a PR? Can you test against a real-world setup?

I can review to see if it fits what the spec says, but I don't have a real-world setup now since I still haven't figured out how to implement #180.

[jankapunkt]

FYI and FWIW;

While I'm actively working on a solution to force PKCE for all clients I wanted to underline, there is a current way to enforce PKCE for all clients via model implementation in model.saveAuthorizationCode.

The last two parameters of saveAuthorizationCode are codeChallenge and codeChallengeMethod. By throwing an error in absence of codeChallenge you can avoid issueing the access token in the first place, which baically implements the above described mitigation.

[jankapunkt]

self note: add this to the docs then we can close this issue