search

IdentityModel / oidc-client-js

OpenID Connect (OIDC) and OAuth2 protocol support for browser-based JavaScript applications
Apache License 2.0
2.43k stars 840 forks source link

Add usePkce flag to allow authorization_code flow without PKCE #1360

Open NickH-nz opened 3 years ago

NickH-nz commented 3 years ago

I ran into an issue recently with an Authorization Server that supports integrations with the OAuth2.0 authorization_code flow both with and without PKCE (with no upgrade path for enabling PKCE on old integrations).

You could argue that it's a bug with the Auth Server not being correctly backwards compaptible, (clients SHOULD include PKCE parameters regardless of server support, as per https://tools.ietf.org/html/rfc7636#section-5). But there is still a potential fix in this library by allowing to explicitly opt out of PKCE. The issue arises when an integration is set up without PKCE, but the client sends the code_challenge and code_challenge_method parameters along with the auth request. This is interpretted by the server as trying to connect incorrectly and returns with "invalid_grant".

The oidc-client-js library always includes the code_challenge and code_challenge_method parameters. This feature request is to add a flag to support opting out of the clause as the "SHOULD" in the spec allows.

NickH-nz commented 3 years ago

I have the bulk of the code working locally and can submit it in a PR for review, but wanted to get the input of the maintainers before progressing further.

brockallen commented 3 years ago

I believe a client would not know if the token server supported PKCE or not. IOW, it does not hurt to send the params.

Obothlale commented 3 years ago

I've experienced a 400 bad request in the past before, where an Identity Provider does not require PKCE. It's not a bad idea to make PKCE a bit more flexible.

brockallen commented 3 years ago

Yea, I've heard that too. But the specs clearly state that a token server is to ignore parameters it does not recognize. I don't want to code for non-spec compliant STSs.