Add client support for PKCE #6485

sdoxsee · 2019-01-28T19:20:59Z

This is a PR for #6446 that involves simple extensions to existing spring security that would probably be used by spring desktop apps (swing, javafx, etc.). It is a WIP but wanted to get some early feedback. I made some tweaks to the sample app oauth2webclient just to illustrate how this can be used with spring boot. I omitted the client-secret and added client-authentication-method: none. Thanks!

Took the discussion in #5652 into account.

Here's a brief overview of the use-cases for client-side PKCE as it pertains to spring security (ref #6446):

Native/Mobile

Of course, only JVM-based Mobile OS's (Android). Android uses a "Custom URI scheme" that is registered with the OS to capture the code. I'd imagine that you'd probably use a library like https://github.com/openid/AppAuth-Android rather than spring security? It appears there was an effort at one time to connect Spring and Android (https://github.com/spring-projects/spring-android) but the project now seems quite stale (dead?). In theory, you could probably have a webserver embedded in your native app that, if it used spring security, could handle the code callback but that seems heavy to me.

Bottom line: I don't think this is the target use-case for spring security client PKCE support but could be used in theory.
Desktop

JVM desktop apps with spring exist (we have one!). We'd need some sort of loopback webserver to capture the authorization code. This could be a very simple one that spins up, on demand, serves a certain port and then shuts down--but that requires custom code (and could be done at a later time). If I simply make our non-web app a web one (by commenting out main.web-application-type: none), we'd get a tomcat and spring security auto configuration just like other web applications. The difference here is that the app needs to be delivered to the users own machines--thereby making the client_secret unviable and PKCE a good fit. Turning non-web apps into web ones feels a bit heavy but simple.

I think using refresh tokens in spring security clients is safe because it's in memory rather than disk?

Bottom line: Simpler is better, desktops have the resources for a local webserver. Why not leverage spring-boot's embedded server? Any other lighter-weight suggestions?
Client-side browser-based JavaScript clients (SPA)

These authorization request are typically all javascript-initiated. The authorization code comes back to client and client makes the token call itself. I figure that spring security is necessary. Question: Why are implicit calls created in spring security (e.g. OAuth2AuthorizationRequest.implicit())?

Bottom line: Why is spring supporting this category at all? We probably don't need PKCE support.

rwinch

Thanks for the PR @sdoxsee! I've provided some comments inline. @jgrandja might have additional feedback (or even responses to my comments), but I thought I could get the ball rolling.

...main/java/org/springframework/security/crypto/keygen/PkceCodeVerifierStringKeyGenerator.java

samples/boot/oauth2webclient/src/main/resources/application.yml

.../main/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequest.java

...main/java/org/springframework/security/crypto/keygen/PkceCodeVerifierStringKeyGenerator.java

rwinch · 2019-01-29T22:08:30Z

...rg/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java

@@ -105,6 +109,9 @@ private OAuth2AuthorizationRequest resolve(HttpServletRequest request, String re
 		OAuth2AuthorizationRequest.Builder builder;
 		if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(clientRegistration.getAuthorizationGrantType())) {
 			builder = OAuth2AuthorizationRequest.authorizationCode();
+			if (isEmpty(clientRegistration.getClientSecret())) {


Is this the best way to determine if PKCE should be used? This seems implicit rather than explicit which can lead to problems later on (if there are other reasons the client secret can be empty). Additionally implicit options in security tends to lead to security vulnerabilities.

I think this makes sense. If AuthorizationGrantType.AUTHORIZATION_CODE and ClientAuthenticationMethod.NONE (public client) than PKCE parameters can be applied.

@jgrandja I like the AuthorizationGrantType.AUTHORIZATION_CODE and ClientAuthenticationMethod.NONE combination to determine PKCE usage. @rwinch I agree the absence of a client secret is not great. Are you ok with AuthorizationGrantType.AUTHORIZATION_CODE + ClientAuthenticationMethod.NONE?

Hi @jgrandja and @rwinch, I received an email from someone watching this PR that raises a good point. Attaching below:

Hi Stephen,

Recently I bumped into your pullrequest for PKCE.

For our project we even have the requirement of using PKCE for normal web applications as an extra hardening.

Okta has put a demo of the PKCE in their playground:

https://oauth.com/playground/authorization-code-with-pkce.html

You will see that in the final step, to get the token, they pass both the client_secret (client authentication client_secret_basic in this case) and code_verifier.

In the pull request I saw some discussion on how to determine PKCE:

#6485 (comment)

In our case or the above mentioned example on Okta this would not be fine. I believe we need a special parameter where we can force PKCE.

What do you think?

I think he makes a good point but it would mean some kind of further flag(?) on ClientRegistration like "withPkce" (true/false) that would only apply for authorization_code grant type. This way PKCE could be used with or without a client secret.

@sdoxsee I'm not sure I'm following the thread in the email you received.

PKCE is meant to be used for Public Clients leveraging the Authorization Code grant. Public Clients are not assigned full credentials (no client secret). So I guess I'm confused with what the user is saying.

Looking at the Okta developer guide for Authorization Code Flow with PKCE, you will see that the client authentication is not required (so no client_secret).

Important: Unlike the regular Authorization Code Flow, this call does not require the Authorization header with the client ID and secret. This is why this version of the Authorization Code flow is appropriate for native apps.

And for the standard Authorization Code Flow, you will see that the client authentication is required as this flow is targeted for confidential clients.

Important: The call to the /token endpoint requires authentication. In this case, it is a Basic Auth digest of the client ID and secret. You can find the client ID and secret in your application’s General tab. This requirement is why this call is only appropriate for applications that can guarantee the secrecy of the client secret. For more on Basic Auth, please see Token Authentication Methods.

Thanks for the explanation @sdoxsee. Now I understand the use case. However, this seems more of an edge case to me. I'd prefer that we leave this out of this PR so we can get this merged. Can you please add a new ticket so we can gather more information.

the person who brought this up is saying that they have this requirement to add PKCE to a confidential client

Can you also please have the user respond to the new ticket with more detail and specifically which Authorization Server supports PKCE for a confidential client (Okta?). A link to server docs would be even better.

Hi,
The current best practices for OAuth security specifies that PKCE should also be used for webapps:

Note: although PKCE so far was recommended as a mechanism to protect
native apps, this advice applies to all kinds of OAuth clients,
including web applications.

see: https://tools.ietf.org/html/draft-ietf-oauth-security-topics-11#section-2.1.1

Now that you are implementing PKCE you better foresee that PKCE can also be used for web applications.

@vanwobe Thanks for the reference. Just below the reference it continues with...

Authorization servers MUST bind authorization codes to a certain
client and authenticate it using an appropriate mechanism (e.g.
client credentials OR PKCE).

The statement below is a bit misleading from the previous as it states client credentials OR PKCE (not both).

Either way, I'm all for making things more secure. I'm very curious though which OAuth 2.0 Provider's have implemented this recommended best practice? Can you point me to a provider that supports PKCE for a confidential client (in a web app)? A link to the reference documentation would be helpful.

Our primary goal when building out new features is to be spec compliant first and foremost and to provide out-of-the-box support for extension features supported by popular OAuth 2.0 Providers. As well, to provide extension points for users to implement for edge-case scenarios.

We have it implemented in our custom provider but that aside, did you see the example from oauth.com: https://oauth.com/playground/authorization-code-with-pkce.html
This exactly demonstrates the use of PKCE in a web application.

@vanwobe @sdoxsee I have confirmed that Okta does indeed support PKCE for confidential clients. I have logged #6548 to track this feature enhancement. I'd rather get this PR merged based on the spec and than look at adding that support as a feature enhancement after.

...ework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolver.java

.../main/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequest.java

...re/src/main/java/org/springframework/security/oauth2/core/endpoint/OAuth2ParameterNames.java

sdoxsee · 2019-01-31T17:55:14Z

I've force-pushed a new commit that tries to take @rwinch's suggestions into consideration.

As for explicit detection of PKCE rather that detecting the absence of a client_secret, would ClientAuthenticationMethod.NONE be appropriate?

I've extended the OAuth2AuthorizationRequest class to handle PKCE specifics. The OAuth2AuthorizationRequest class needed tweaks to allow it to be subclassed as well as its Builder.

I didn't subclass OAuth2AuthorizationCodeGrantRequestEntityConverter so I used an "instanceof" to be sure I could access the codeVerifier on the OAuth2AuthorizationRequest :/

I plan on writing tests once the design is a little more settled. Thanks for the feedback!

Also want to acknowledge @kbolduc for pairing with me on this!

jgrandja

Thanks for the PR @sdoxsee. See comments below.

...main/java/org/springframework/security/crypto/keygen/PkceCodeVerifierStringKeyGenerator.java

...-core/src/main/java/org/springframework/security/oauth2/core/ClientAuthenticationMethod.java

.../main/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequest.java

...java/org/springframework/security/oauth2/core/pkce/endpoint/CodeChallengeTransformation.java

...a/org/springframework/security/oauth2/core/pkce/endpoint/PkceOAuth2AuthorizationRequest.java

...src/main/java/org/springframework/security/oauth2/core/pkce/endpoint/PkceParameterNames.java

...rg/springframework/security/oauth2/core/pkce/endpoint/Sha256CodeChallengeTransformation.java

...src/main/java/org/springframework/security/oauth2/core/pkce/endpoint/PkceParameterNames.java

sdoxsee · 2019-02-08T15:52:16Z

@jgrandja I rebased on #5940 so I had to force push, sorry! Working on tests now but if you or @rwinch can provide feedback then that would be great. Thanks!

sdoxsee · 2019-02-11T16:13:33Z

I've added some tests and fixed the bad checkstyle from the previous commit. When you get a chance, please let me know how to proceed. Thanks!

jgrandja

Thanks for the updates @sdoxsee! I've added a few more comments.

...rg/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolver.java

...security/oauth2/client/endpoint/OAuth2AuthorizationCodeGrantRequestEntityConverterTests.java

...c/main/java/org/springframework/security/oauth2/client/registration/ClientRegistrations.java

...st/java/org/springframework/security/oauth2/client/registration/TestClientRegistrations.java

...-core/src/main/java/org/springframework/security/oauth2/core/ClientAuthenticationMethod.java

...src/main/java/org/springframework/security/oauth2/core/pkce/endpoint/PkceParameterNames.java

sdoxsee · 2019-02-15T19:03:54Z

@jgrandja Just pushed another commit to address your latest suggestions. Tests pass locally--hopefully Travis will agree. Thanks!

jgrandja

Thanks for the updates @sdoxsee. We're almost ready to get this merged! See my comments for minor updates.

Also, we need to update WebClientReactiveAuthorizationCodeTokenResponseClient to support a PKCE token request.

Thanks!

.../security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java

...c/main/java/org/springframework/security/oauth2/client/registration/ClientRegistrations.java

...st/java/org/springframework/security/oauth2/client/registration/ClientRegistrationsTest.java

...-core/src/main/java/org/springframework/security/oauth2/core/ClientAuthenticationMethod.java

.../src/test/java/org/springframework/security/oauth2/core/ClientAuthenticationMethodTests.java

sdoxsee · 2019-02-22T02:31:18Z

Thanks for the updates @sdoxsee. We're almost ready to get this merged! See my comments for minor updates.

Also, we need to update WebClientReactiveAuthorizationCodeTokenResponseClient to support a PKCE token request.

Thanks!

Hi @jgrandja. I've pushed the changes you recommended. I added support for PKCE token request to WebClientReactiveAuthorizationCodeTokenResponseClient. As I did that, I found some differences in logic compared to OAuth2AuthorizationCodeGrantRequestEntityConverter that was adding REDIRECT_URI regardless of whether it was null or not (and it is optional for authorization_code). I now only add it if it has a non-null value. I didn't add any tests for WebClientReactiveAuthorizationCodeTokenResponseClient since the test class was only looking at responses so far. I can certainly add tests to inspect the request before it is sent but that was starting to balloon a bit. Please let me know and I'll add some tests (and for the things I mention in the next paragraph).

Also in WebClientReactiveAuthorizationCodeTokenResponseClient and OAuth2AuthorizationCodeGrantRequestEntityConverter, I add the client_id for any ClientAuthenticationMethod other than BASIC because the spec says:

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".
where I assume "unauthenticated client** must mean a public client (ie. NONE)...but client_id must also appear for POST because it isn't coming in the Authorization: Basic header. You could argue that we should always send it since we always have it and it IS optional or that I should explicitly list off NONE and POST as the condition where client_id should be included. Anyway, sorry to open another can of worms but glad you pushed off #6548 so we can get this one out soon--hopefully :)

dfcoffin · 2019-02-22T04:02:10Z

Hi @sdoxsee. If you look at the first paragraph of the spec <https://tools.ietf.org/html/rfc6749#section-3.2.1> you quoted, it says: Confidential clients or other clients issued client credentials (e.g., ClientID and secret) MUST Authenticate with the authorization server as described in Section 2.3 <https://tools.ietf.org/html/rfc6749#section-2.3> when making requests to the token endpoint. Section 2.3 Client Authentication states: If the client type is confidential, the client and authorization server establish a client authentication method suitable for the security requirements of the authorization server. The authorization server MAY accept any form of client authentication meeting its security requirements. Confidential clients are typically issued (or establish) a set of client credentials used for authenticating with the authorization server (e.g., password, public/private key pair). The authorization server MAY establish a client authentication method with public clients. However, the authorization server MUST NOT rely on public client authentication for the purpose of identifying the client. The client MUST NOT use more than one authentication method in each request. I believe, based on the last paragraph of Section 2.3, inclusion of the ClientID in both the Authorization Header and in the body is a violation of the standard. Just my 2 cents. Best regards, Don Donald F. Coffin Founder/CTO REMI Networks 2335 Dunwoody Xing #E Dunwoody, GA 30338-8221 Phone: (949) 636-8571 Email: donald.coffin@reminetworks.com <mailto:donald.coffin@reminetworks.com> From: Stephen Doxsee <notifications@github.com> Sent: February 21, 2019 09:31 PM To: spring-projects/spring-security <spring-security@noreply.github.com> Cc: Subscribed <subscribed@noreply.github.com> Subject: Re: [spring-projects/spring-security] Add client support for PKCE (#6485) Thanks for the updates @sdoxsee <https://github.com/sdoxsee> . We're almost ready to get this merged! See my comments for minor updates. Also, we need to update WebClientReactiveAuthorizationCodeTokenResponseClient to support a PKCE token request. Thanks! Hi @jgrandja <https://github.com/jgrandja> . I've pushed the changes you recommended. I added support for PKCE token request to WebClientReactiveAuthorizationCodeTokenResponseClient. As I did that, I found some differences in logic compared to OAuth2AuthorizationCodeGrantRequestEntityConverter that was adding REDIRECT_URI regardless of whether it was null or not (and it is optional for authorization_code). I now only add it if it has a non-null value. I didn't add any tests for WebClientReactiveAuthorizationCodeTokenResponseClient since the test class was only looking at responses so far. I can certainly add tests to inspect the request before it is sent but that was starting to balloon a bit. Please let me know and I'll add some tests (and for the things I mention in the next paragraph). Also in WebClientReactiveAuthorizationCodeTokenResponseClient and OAuth2AuthorizationCodeGrantRequestEntityConverter, I add the client_id for any ClientAuthenticationMethod other than BASIC because the spec <https://tools.ietf.org/html/rfc6749#section-3.2.1> says: 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". where I assume "unauthenticated client** must mean a public client (ie. NONE)...but client_id must also appear for POST because it isn't coming in the Authorization: Basic header. You could argue that we should always send it since we always have it and it IS optional or that I should explicitly list off NONE and POST as the condition where client_id should be included. Anyway, sorry to open another can of worms but glad you pushed off #6548 <#6548> so we can get this one out soon--hopefully :) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#6485 (comment)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvrQ-BPsc3cidYYuizT_BJDfy8nE_L5ks5vP1Z5gaJpZM4aWmPz> . <https://github.com/notifications/beacon/ABvrQ5I0pYZqyoJAdtPDI8q6P93vV_4oks5vP1Z5gaJpZM4aWmPz.gif>

jgrandja · 2019-02-22T04:28:07Z

@dfcoffin Thanks for pointing out the following statement in 2.3. Client Authentication

The client MUST NOT use more than one authentication method in each
request.

Okta does support client authentication via credentials as well as code_verifier to verify the code_challenge, so in essence, it is authenticating the client twice. Will keep this in mind while we consider #6548

sdoxsee · 2019-02-22T15:00:09Z

Thanks @jgrandja and @dfcoffin. Specs are fun eh? ...sigh. Haha. Thanks for hashing out the details!

@jgrandja I see your point regarding code_verifier potentially being considered as another authentication mechanism but perhaps it's not so much authenticating as it is making sure the token request is being made by the same party that made authorization request? The spec calls it a "mitigation" or "proof of possession of the 'code verifier'". I think there's at least sufficient "gray" in the spec to allow for it.

@dfcoffin. Regarding where the spec says

"The client MUST NOT use more than one authentication method in each request."

you say,

I believe, based on the last paragraph of Section 2.3, inclusion of the ClientID in both the Authorization Header and in the body is a violation of the standard.

I believe that including the client_id in the body is fine all the time (even though the oauth2 authorization_code example doesn't include it) because it is not for authentication unless it also includes the client_secret. We only include the client_secret in the body for ClientAuthenticationMethod.POST--in which case no Authorization Header is sent...therefore, no violation?

What do you think? Thanks!

dfcoffin · 2019-02-22T20:26:37Z

@sdoxsee The reason I pointed out that the last paragraph in Section 2.3 will be violated if the client_id is always provided, unless it is omitted for confidential clients or any client provided with a secret. The Section 3.2.1 Client Authentication opening paragraph clearly states any client issued a secret MUST supply a client_id and secret in the HTTP Authorization header element using Base64 encoding.

Therefore, if client_id is always included in the Authorization Code request body, it will violate Section 2.3's final paragraph.

sdoxsee · 2019-02-22T20:43:59Z

Thanks @dfcoffin

The Section 3.2.1 Client Authentication opening paragraph clearly states any client issued a secret MUST supply a client_id and secret in the HTTP Authorization header element using Base64 encoding.

I'm not able to find anywhere in either 3.2.1 or 2.3 where it says that "any client issued a secret MUST supply a client_id and secret in the HTTP Authorization header". In fact it says in 2.3.1

Clients in possession of a client password MAY use the HTTP Basic
authentication scheme as defined in [RFC2617] to authenticate with
the authorization server.

and then goes on to describe other methods of authenticating in 2.3.2. No?

Regards,
Stephen

dfcoffin · 2019-02-22T23:20:30Z

@sdoxsee @jgrandja I agree Section 2.3.1 Client Password's title is a bit misleading, but it describes how clients assigned a client password (a.k.a. secret) MAY use the HTTP Basic authentication scheme as defined in [RFC2617] to authenticate with the authorization server. It provides the following example:

   For example (with extra line breaks for display purposes only):

     Authorization: Basic czZCaGRSa3F0Mzo3RmpmcDBaQnIxS3REUmJuZlZkbUl3

   Alternatively, the authorization server MAY support including the
   client credentials in the request-body using the following
   parameters:

   client_id
         REQUIRED.  The client identifier issued to the client during
         the registration process described by Section 2.2.

   client_secret
         REQUIRED.  The client secret.  The client MAY omit the
         parameter if the client secret is an empty string.

It then states:

   Including the client credentials in the request-body using the two
   parameters is NOT RECOMMENDED and SHOULD be limited to clients unable
   to directly utilize the HTTP Basic authentication scheme (or other
   password-based HTTP authentication schemes).  The parameters can only
   be transmitted in the request-body and MUST NOT be included in the
   request URI.

I agree this section is rather vague, but given the example of using client_id and secret as userID:password as defined by [RFC2617], I believe was what the authors of the standard intended as the method an authorization server MUST use when authenticating confidential_clients, which the standard defines in Section 2.1 Client Types.

jgrandja · 2019-02-25T22:39:32Z

@sdoxsee Can you please add a test for WebClientReactiveAuthorizationCodeTokenResponseClientTests. Alternatively, you can update getTokenResponseWhenSuccessResponseThenReturnAccessTokenResponse to check the request similar to how DefaultAuthorizationCodeTokenResponseClientTests.getTokenResponseWhenSuccessResponseThenReturnAccessTokenResponse(), e.g. RecordedRequest recordedRequest = this.server.takeRequest();

Also, one more change I think makes sense. In ClientRegistration.create(), if authorization_code and client_secret empty than default to ClientAuthenticationMethod.NONE.

I appreciate your patience with all this back and forth. I think this is it and we can merge. Thanks again for your efforts here.

sdoxsee · 2019-02-26T18:50:09Z

Thanks @jgrandja. I'll probably add 1 test to WebClientReactiveAuthorizationCodeTokenResponseClientTests testing for PCKE parameters as well as enhance the existing getTokenResponseWhenSuccessResponseThenReturnAccessTokenResponse to assert on the request string (i.e. especially that it doesn't have PKCE params)

As for

Also, one more change I think makes sense. In ClientRegistration.create(), if authorization_code and client_secret empty than default to ClientAuthenticationMethod.NONE.

Do you think that contradicts @rwinch's earlier comment by making this detection "implicit" rather than "explicit"? However, I agree with what you say since #6548 will mean that PKCE can be used with a secret as well--only that the ClientAuthenticationMethod would be BASIC rather than NONE in that case.

Also, what should be done with the defaulted BASIC for clientAuthenticationMethod on ClientRegistration? Should I add BASIC as the fallback in create() if nothing else is set on the builder? I think defaulting it at create() is better than setting it at initialization time because otherwise I can't easily detect if someone set it to something explicit on the builder before calling build (i.e. we'd never get a null without it being explicitly set to null)

Thanks,
Stephen

jgrandja · 2019-02-26T19:12:24Z

@sdoxsee

what should be done with the defaulted BASIC for clientAuthenticationMethod on ClientRegistration?...

I think this is all the logic we need in create()

// this.clientAuthenticationMethod is initialized to ClientAuthenticationMethod.BASIC
clientRegistration.clientAuthenticationMethod = this.clientAuthenticationMethod;

// Override for this case
if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(this.authorizationGrantType) &&
		StringUtils.isEmpty(this.clientSecret)) {

	clientRegistration.clientAuthenticationMethod = ClientAuthenticationMethod.NONE;
}

Makes sense?

sdoxsee · 2019-02-26T19:19:42Z

Yes @jgrandja. Thanks, almost wrote the exact same but with !StringUtils.hasText(this.clientSecret) to be consistent with the logic a couple lines up from there (e.g. "<whitespace>" should be considered as "" for clientSecret). Going to go with !StringUtils.hasText(this.clientSecret) if that's ok and I'll add your comments in the code as well.

sdoxsee · 2019-02-26T20:26:30Z

@jgrandja pushed but not sure what's wrong with the build :/

jgrandja

Thanks @sdoxsee! A couple of minor comments. After you apply those updates please rebase and squash to 1 commit and ensure the commit message follows the format with ... Fixes gh-6446.
Than we'll be ready to merge!

jgrandja · 2019-02-27T15:42:51Z

...urity/oauth2/client/endpoint/WebClientReactiveAuthorizationCodeTokenResponseClientTests.java

@@ -288,4 +294,52 @@ public void setCustomWebClientThenCustomWebClientIsUsed() {

 		verify(customClient, atLeastOnce()).post();
 	}
+
+	private OAuth2AuthorizationCodeGrantRequest pkceAuthorizationCodeGrantRequest() {


Please move this method below the associated test.

jgrandja · 2019-02-27T15:43:27Z

...rc/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java

@@ -500,6 +500,11 @@ private ClientRegistration create() {
 			clientRegistration.clientId = this.clientId;


Update copyright header.

jgrandja · 2019-02-27T15:44:03Z

...st/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java

@@ -174,6 +174,41 @@ public void buildWhenAuthorizationCodeGrantClientAuthenticationMethodNotProvided
 		assertThat(clientRegistration.getClientAuthenticationMethod()).isEqualTo(ClientAuthenticationMethod.BASIC);


Update copyright header.

- Support has been added for "RFC7636: Proof Key for Code Exchange by OAuth Public Clients" (PKCE, pronounced "pixy") to mitigate against attacks targeting the interception of the authorization code - PkceParameterNames was added for the 3 additional parameters used by PKCE (i.e. code_verifier, code_challenge, and code_challenge_method) - Default code_verifier length has been set to 128 characters--the maximum allowed by RFC7636 - ClientAuthenticationMethod.NONE was added to allow clients to request tokens without providing a client secret Fixes spring-projectsgh-6446

sdoxsee · 2019-02-27T18:55:19Z

Rebased, squashed and passing. Happy to contribute to #6548 when the time comes.
Thanks to @kbolduc for pairing with me on this and to others who provided insights.
Thanks to both @jgrandja and @rwinch for all of your help and patience :)

jgrandja · 2019-02-28T16:53:42Z

Thank you @sdoxsee @kbolduc for your great work! This is now in master!

ExpDev07 · 2019-05-13T19:12:37Z

Any documentation on how to use this in a Spring app? How would I setup the configuration?

heyarny · 2019-08-14T16:23:05Z

I'd like to see an example on how to use this as well, especially with password grant.
I don't want to see the refresh token within the SPA.
Any documentation out there about this new feature?

rwinch · 2019-08-15T13:43:27Z

@ExpDev07 @heyarny There is no documentation for this feature yet. Would one of you create a ticket?

@sdoxsee Would you be interested in submitting a PR for adding documentation for PKCE?

jgrandja self-assigned this Jan 28, 2019

jgrandja added New Feature in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) labels Jan 28, 2019

rwinch requested changes Jan 29, 2019

View reviewed changes

sdoxsee force-pushed the master branch from 9cbd606 to 12c7aaf Compare January 31, 2019 17:34

jgrandja requested changes Feb 2, 2019

View reviewed changes

sdoxsee force-pushed the master branch from 12c7aaf to 5299619 Compare February 8, 2019 15:50

jgrandja requested changes Feb 14, 2019

View reviewed changes

jgrandja removed the wip label Feb 21, 2019

jgrandja requested changes Feb 21, 2019

View reviewed changes

jgrandja mentioned this pull request Feb 21, 2019

Consider enabling PKCE for confidential clients #6548

Closed

jgrandja requested changes Feb 27, 2019

View reviewed changes

sdoxsee force-pushed the master branch from c10fd16 to 8f8e932 Compare February 27, 2019 17:59

rwinch approved these changes Feb 27, 2019

View reviewed changes

jgrandja approved these changes Feb 28, 2019

View reviewed changes

jgrandja added this to the 5.2.0.M2 milestone Feb 28, 2019

jgrandja closed this Feb 28, 2019

rwinch added the type: enhancement A general enhancement label May 3, 2019

jgrandja mentioned this pull request May 13, 2019

Make #buildFormParameters in the oauth2 client auth converters protected #6854

Closed

This was referenced Aug 19, 2019

Add documentation for Public Client PKCE support #7279

Closed

Add documentation for public client PKCE support #7280

Closed

		@@ -500,6 +500,11 @@ private ClientRegistration create() {
		clientRegistration.clientId = this.clientId;

		@@ -174,6 +174,41 @@ public void buildWhenAuthorizationCodeGrantClientAuthenticationMethodNotProvided
		assertThat(clientRegistration.getClientAuthenticationMethod()).isEqualTo(ClientAuthenticationMethod.BASIC);

Add client support for PKCE #6485

Add client support for PKCE #6485

Conversation

sdoxsee commented Jan 28, 2019

rwinch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sdoxsee commented Jan 31, 2019 • edited Loading

jgrandja left a comment

Choose a reason for hiding this comment

sdoxsee commented Feb 8, 2019

sdoxsee commented Feb 11, 2019

jgrandja left a comment

Choose a reason for hiding this comment

sdoxsee commented Feb 15, 2019

jgrandja left a comment

Choose a reason for hiding this comment

sdoxsee commented Feb 22, 2019

dfcoffin commented Feb 22, 2019 via email

jgrandja commented Feb 22, 2019

sdoxsee commented Feb 22, 2019 • edited Loading

dfcoffin commented Feb 22, 2019

sdoxsee commented Feb 22, 2019

dfcoffin commented Feb 22, 2019 • edited Loading

jgrandja commented Feb 25, 2019 • edited Loading

sdoxsee commented Feb 26, 2019

jgrandja commented Feb 26, 2019

sdoxsee commented Feb 26, 2019

sdoxsee commented Feb 26, 2019

jgrandja left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sdoxsee commented Feb 27, 2019

jgrandja commented Feb 28, 2019

ExpDev07 commented May 13, 2019

heyarny commented Aug 14, 2019

rwinch commented Aug 15, 2019

sdoxsee commented Jan 31, 2019 •

edited

Loading

sdoxsee commented Feb 22, 2019 •

edited

Loading

dfcoffin commented Feb 22, 2019 •

edited

Loading

jgrandja commented Feb 25, 2019 •

edited

Loading

jgrandja left a comment •

edited

Loading