Always attempt to use PCKE for OAuth 2 auth code flows

[bdemers]

The "OAuth 2.0 Security Best Current Practice" (draft)
https://tools.ietf.org/html/draft-ietf-oauth-security-topics-13#section-3.1.1

Clients utilizing the authorization grant type MUST use PKCE
[RFC7636] in order to (with the help of the authorization server)
detect and prevent attempts to inject (replay) authorization codes
into the authorization response.

NOTE: this change should be backward compatible with IdPs who do not yet support PKCE (as the related parameters should be ignored)

[bdemers]

(I'll update this issue if/when we get a time frame on this draft moving forward)
Currently looks like a couple of months out

[jgrandja]

@bdemers The issue with forcing PKCE for all Authorization Requests is that some providers do not support PKCE and therefore the request might fail.

[bdemers]

Hey @jgrandja!
That was one of my concerns too. but the OAuth 2.0 RFC states:

The authorization server MUST ignore unrecognized request parameters.

I'll be honest and say that I've only tried this with one IdP ;) but we could also take the position of defaulting to PKCE, and then set a flag to turn it off for non-conforming servers (if it is an issue).

Thoughts?

[jgrandja]

@bdemers I understand using PKCE for public and confidential clients (#6548 #7132) is best practice but I'm still concerned with breaking existing client apps by forcing PKCE for all authorization_code flows.

At minimum, I would like to test a few providers to see how they behave with PKCE parameters.

[bdemers]

@jgrandja, seems like a good idea ;)

I can do a little manual testing, how about, Google, FB, Google, Okta, and maybe Keycloak?

[jgrandja]

Sounds good @bdemers. Let me know how the testing goes with those providers.

[bdemers]

@jgrandja I tested out the following with success:

• GitHub
• Okta
• Facebook
• Google
• Keycloak

I also attempted to test Linkedin, but I couldn't get that to work with or without this patch. I tried to base my configuration off an older demo of yours: https://github.com/jgrandja/oauth2login-demo/blob/linkedin/src/main/resources/application.yml

Any chance you have an updated set of LinkedIn configuration?

[jgrandja]

Nice @bdemers!

Did you configure the Converter, which is required to work with LinkedIn since the Token Response does not return the required token_type.

[jgrandja]

@bdemers Just a heads up that #7748 has been in the works for a bit now. It should really simplify customizing the OAuth2AuthorizationRequest. And I have an idea on how to resolve #6548, which is related to this PR. Let me get #7748 merged first and then we can circle back to this PR and see what changes are required to get this in. I have a couple of other priority items I need to address first so I should get back to this within 2 weeks maybe less. Thanks for testing those scenarios!

[bdemers]

@jgrandja ahh, I had missed that. I also needed to tweak the properties, and I was able to get it to work with the current RELEASE.

spring:
  security:
    oauth2:
      client:
        provider:
          linkedin:
            authorization-uri: "https://www.linkedin.com/oauth/v2/authorization"
            token-uri: "https://www.linkedin.com/oauth/v2/accessToken"
            user-info-uri: "https://api.linkedin.com/v2/me"
            user-name-attribute: id
        registration:
          linked:
            client-id: id-here
            client-secret: secret-here
            client-authentication-method: post
            authorization-grant-type: authorization_code
            redirect-uri: "{baseUrl}/login/oauth2/code/{registrationId}"
            scope: r_basicprofile, r_emailaddress, r_liteprofile
            client-name: Linkedin

I guess the previous user-info-uri was deprecated? (and requires a different scope).

However with this patch: It does look like LinkedIn responses with a 500 when the code_verifier is sent to https://www.linkedin.com/oauth/v2/accessToken.

I'll try to ask around. I'm not 100% sure I have this set up correctly, but it's not looking good...

[wpcfan]

any update on this? which release is going to have this feature?

[bdemers]

@wpcfan I started chatting with some folks at LinkedIn about the problem. They were looking into it, I'll reach out again and check the status

[adiseshan]

Could you please let know if there is any update for this task.

[bdemers]

@adiseshan there is a little more talk on the topic on #6548

[ghost]

I've just learned that OAuth 2.1 will require PKCE for confidential clients. I would be very happy to see support for it soon: https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-02.txt#section-1.9

[drahkrub]

PKCE is also required by IdentityServer4 -> demo server.

[jgrandja]

@bdemers I'm going to close this since the changes are not backward compatible with all tested providers.

I'll leave gh-6548 open so we don't lose track of this as the plan is to add this enhancement.

Feel free to create a new PR that would ensure backwards compatibility.

We're going to prioritize this for the 5.7 release.

[shabloel]

Spring still does not deliver PKCE support out of the box for confidential clients. According to new best practice https://datatracker.ietf.org/doc/html/draft-ietf-oauth-browser-based-apps, PKCE always has to be enabled, also for a Confidential client. I am currently working on such an implementation where my client is a spring boot application and KEycloak is my authorisation server, so I am running into some problems

[jgrandja]

@shabloel As mentioned in previous comment

We're going to prioritize this for the 5.7 release.

Please stay tuned.

[shabloel]

thank you!:). I also found a work-around, it works half out of the box. properties file:
spring.security.oauth2.client.registration.mywebclient.client-authentication-method=none.
spring does all the work for you....except it leaves out the client-secret in its final access token request. You can easily add the client-secret in the access token request manually, then everything works fine.