Add PKCE support

[Kehrlann]

This is a first stab at PKCE implementation - see #45 . The idea is to validate the structure of how to achieve PKCE validation, and then start iterating from there.

Notes:

• The SHA256 code verification was stolen directly from spring-security, see org.springframework.security.oauth2.client.web.DefaultOAuth2AuthorizationRequestResolver#createHash . We might want to make that public in spring-security at some point ?
• We consider a client is public client when the principal is null but it has client ID.

TODOs:

Authorization endpoint

• RFC 7636 - 4.4 Server Returns the Code ; validate code_challenge & code_challenge_method
• CODE_CHALLENGE is required ; CODE_CHALLENGE_METHOD can be null (will default to plain)
• Only trigger PKCE checks for public clients (registeredClient.clientSecret is blank)
• Consider what is a "public" client - maybe we could mark some "redirectUris" to require PKCE vali
• Add tests for OAuth2AuthorizationEndpointFilter

Token endpoint

• Validate code_verifier for both "plain" and "S256" methods (for public clients)
• If code_challenge_method is null, default to plain
• Polish changes to OAuth2AuthorizationCodeAuthenticationProvider (null checks, try-catches)
• In OAuth2AuthorizationCodeAuthenticationProvider.authenticate, authenticate public client - See RFC 6749 - 3.2.1. Token Endpoint > Client Authentication [1]

Integration tests

• Add integration test for PKCE

[1] In the "authorization_code" "grant_type" request to the token endpoint, an unauthenticated client MUST send its "client_id" to prevent itself from inadvertently accepting a code intended for a client with a different "client_id". This protects the client from substitution of the authentication code. (It provides no additional security for the protected resource.)

[Kehrlann]

I started adding tests for OAuth2AuthorizationCodeAuthenticationProvider. They are messy for now, but mostly allow me to validate the implementation.
Two of them are failing, they show the need for OAuth2AccessTokenAuthenticationToken to support public clients - today, only private clients are supported.

The first stab at this stops at d6209df, the last commit is the failing tests, quite a bit messy.

[jgrandja]

Thanks for the PR @Kehrlann! Please see initial review comments.

[Kehrlann]

The PR is ready for review. Still to be decided: how do we identify public clients in OAuth2AuthorizationEndpointFilter ?

1. When the registeredClient has no clientSecret, its' a 100% public client, that's currently covered.
2. However, the registered client could have a secret and still be used as a public client. Something to explore could be: some redirect uris of a client could be tagged with "require PKCE".

[uhhhh2]

Thoughts on letting all clients (public and confidential) that use authorization code also use PKCE (confidential will have both secret & PKCE), but only require PKCE for public clients. (those without client secrets) like in spring-projects/spring-security#6548 ?

[Kehrlann]

With the current proposed implementation, confidential clients may send PKCE parameters. However, those parameters will not be validated. Not sure if we should put this in v1 ?

It makes sense to validate whatever PKCE params confidential clients send, especially if we're allowing confidential clients to do it in spring-security.

[Kehrlann]

Rebased on top of master. For info, I have only checked the build & ran the tests on conflicting commits.

[jgrandja]

Thanks for the updates @Kehrlann! I left a few comments inline.
Also, can you please squash the commits on next update.
Thanks.

[Kehrlann]

Included all corrections, squashed. There's still one open question, and we should be almost good to go.

[jgrandja]

Thanks for the great work @Kehrlann ! This is now in master!

FYI, I added a polish commit with some minor updates.

Also, I discovered an issue when reviewing the integration test OAuth2PkceTests. The test revealed an issue with the implementation so it needed to be re-worked. Ultimately, the client needs to be authenticated before it hits the token endpoint. Therefore, I needed to move the PKCE authentication logic from OAuth2AuthorizationCodeAuthenticationProvider to OAuth2ClientAuthenticationProvider.

Apologies, as I missed this during the review process. I decided to go ahead an apply the change. Take a look at the commit 5c31fb1 and let me know if you have any questions.

Thanks again for all the great work!