Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PKCE Refresh token #288

Open
kuselan84 opened this issue May 9, 2024 · 14 comments
Open

PKCE Refresh token #288

kuselan84 opened this issue May 9, 2024 · 14 comments
Labels
compliance 📜 OAuth 2.0 standard compliance discussion 🗨️ Discussion about a particular topic. documentation 📑 Improvements or additions to documentation enhancement ✨ New feature or request hacktoberfest investigating 🔍

Comments

@kuselan84
Copy link

Token handler on PCKE flow is not verifying code_verifier and expecting client_secret.

Providing client_secret will defeat PKCE flow.

Please assist.

@jankapunkt
Copy link
Member

Hello @kuselan84 thank you for getting in contact. What is exactly the nature of your issue?

  • Did you try to implement the PKCE workflow
  • or do you think we have a flaw in our implementation that conflicts with the RFC 7636

@jankapunkt jankapunkt added question ❓ Information is being requested investigating 🔍 labels May 13, 2024
@kuselan84
Copy link
Author

Hi @jankapunkt,

I'm trying implement PKCE flow using guide at PKCE Support.

I send auth request as follows:

POST http://localhost:3000/authorize
Content-Type: application/x-www-form-urlencoded

response_type=code&
client_id=myclient&
redirect_uri=http://localhost:3000/client/app&
state=mystate&
scope=offline_access&
code_challenge=p4eKNknklsnn5fQb9A20lfYv36WHZk_K9y8PRCPooNc&
code_challenge_method=S256

I received code to redirect uri successfully. Then I request token as follows:

POST http://localhost:3000/token
Content-Type: application/x-www-form-urlencoded

grant_type=authorization_code&
client_id=myclient&
code=8b51da4fade34a5ccf0d1dc40816812052e2b3ae&
code_verifier=e4fz4xjFKhlQUWXgsME5dp3CugzrYf1BLUwuywRu9gc

I received access token successfully. Then I request access token again using refresh token as follows:

POST http://localhost:3000/token
Content-Type: application/x-www-form-urlencoded

grant_type=refresh_token&
client_id=myclient&
code_verifier=e4fz4xjFKhlQUWXgsME5dp3CugzrYf1BLUwuywRu9gc&
refresh_token=f5360c80cd7fd774d3a165360796a1747d3de012f6608ce6bcb3997876f7c892

It failed with this error message:

{
  "error": "invalid_client",
  "error_description": "Invalid client: cannot retrieve client credentials"
}

The request works if I send client_secret parameter. However it ignore code_verifier verification.

@kuselan84
Copy link
Author

Perhaps it could be fixed so that neither the code_verifier nor the client_secret are required.
If the client securely manages the refresh_token, it alone could suffice for obtaining a new access_token.
eg.

POST http://localhost:3000/token
Content-Type: application/x-www-form-urlencoded

grant_type=refresh_token&
client_id=myclient&
refresh_token=f5360c80cd7fd774d3a165360796a1747d3de012f6608ce6bcb3997876f7c892

@jankapunkt
Copy link
Member

@kuselan84 I read up again the RFC and some resources and there is indeed no specification on the RFC about the refresh token in combination with PKCE. I also think this is why we did not 100% cover this in our implementation so far. This is very unexpected.

Assumptions

  • if a client has retrieved the refresh token (together with the access token) then it must have passed the PKCE
  • there is no need to pass PKCE challenge with refresh token
  • however, if the client was issued a client_secret in the authorization code grant workflow, then it must issue the client_secret even with refresh grant
  • The request works if I send client_secret parameter. However it ignore code_verifier verification. is therefore correct behaviour, because you will not need the code_verifier with refresh token.

Concerns

Perhaps it could be fixed so that neither the code_verifier nor the client_secret are required.
If the client securely manages the refresh_token, it alone could suffice for obtaining a new access_token.
eg.

I agree with this. However I'm still unsure how If the client securely manages the refresh_token is actually realized in a PKCE scenario.

Is this even possible in a PKCE scenario? From how I see it, the client is considered unsafe. A stack answer is also inconclusive on this.

Summary

I'm generally positive with you as the RFC and literature is inconclusive and if in doubt we should always decide for the flexibility and let developers decide. However we will have to warn over the concerns in the documentation.

I would still not consider this as a bug, though.

@jankapunkt jankapunkt added documentation 📑 Improvements or additions to documentation enhancement ✨ New feature or request discussion 🗨️ Discussion about a particular topic. compliance 📜 OAuth 2.0 standard compliance and removed question ❓ Information is being requested labels May 14, 2024
@kuselan84
Copy link
Author

Thank you for the reply, @jankapunkt.

I have opted to disable client_secret on token endpoint like this as per documentation:

router.post('/token', oauth.token({requireClientAuthentication:{refresh_token:false}}));

I'm going to implement Refresh token reuse Detection to protect refresh token.

@jankapunkt
Copy link
Member

If there is no veto from @jorenvandeweyer @dhensby @shrihari-prakash I'm going to implement this. The impl. also needs to update the tests accordingly.

@dhensby
Copy link
Contributor

dhensby commented May 21, 2024

disclaimer: I have not familiarised myself with the PKCE spec recently

My understanding PKCE is primarily aimed at clients that are deemed to be unable to securely store sensitive date (hence, there is no client_secret issued to them as part of their credentials). Therefore, I'd be very cautious about even issuing refresh tokens to clients that cannot store secrets securely...

@shrihari-prakash
Copy link
Contributor

Hello @jankapunkt ,

In my opinion, I think it is normal that the code_verifier is ignored in refresh token exchanges. Because if we take a step back and see the purpose of PKCE itself, for instance in a native app setup when the auth code flow is done and the redirect is sent to your app, a malicious application can register itself as a handler for the redirect URI and get the authorization code which can be abused. The primary thing to note is that there are two parties involved here.

But in case of refresh tokens, there is none of the redirect stuff and it is purely handled within the application. I don't see why a code_verifier is required in this case although as @dhensby mentioned, if the client cannot store refresh tokens securely, there is no point to even sending it to client :).

@jankapunkt
Copy link
Member

@dhensby @shrihari-prakash thanks for your input. The biggest problem of all this is, that the standard says NOTHING about all this. Real edge-case here.

@dhensby
Copy link
Contributor

dhensby commented May 21, 2024

So there are two issues at play here.

  1. Should PKCE flows return refresh tokens at all?
  2. If PKCE flow does issue a refresh token, how is that token exchanged properly for a renewed access token?

Should PKCE flows return refresh tokens?

This is actually less a question about PKCE and more about public vs confidential clients. tl;dr: Public clients can't store secrets, confidential clients can. PKCE code challenges can (and are) used by confidential clients as it's recommended in some of the more advanced standards such as FAPI 2.0.

The OAuth specification essentially says, it's up to the risk tolerance of the authorisation server as to whether refresh tokens are issued:

The client type designation is based on the authorization server's definition of secure authentication and its acceptable exposure levels of client credentials. The authorization server SHOULD NOT make assumptions about the client type.

Personally, I would not issue refresh tokens to any public clients, but of course native apps that have access to store refresh tokens in secure enclaves, certainly feels like an acceptable level of risk following a PKCE flow.

I'd note that in section 1.1 of the PKCE extension there is no mention of refresh tokens forming part of the protocol, whereas the OAuth protocol does mention them in the equivalent section section 1.5 of its spec. Certainly not conclusive, but a fairly clear omission.

My view on this is: PKCE flows issuing refresh tokens is up to the server, the server should be storing meta data about the client (ie: whether it is a public or confidential client) and acting on that information as it feels appropriate. NB: If the client has authentication credentials (ie: it has a client_secret) it must provide that even with the PKCE flow.

Answer: Yes, PKCE can (but probably shouldn't for public clients) release refresh tokens.

How is a refresh token obtained through PKCE exchanged?

The specification is annoyingly vague around this. If we start at refreshing an access token it says:

If the client type is confidential or the client was issued client credentials (or assigned other authentication requirements), the client MUST authenticate with the authorization server as described in Section 3.2.1.

[Section 3.2.1] says:

Confidential clients or other clients issued client credentials MUST authenticate with the authorization server as described in Section 2.3 when making requests to the token endpoint

Section 2.3 says:

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.

So all the authentication rhetoric is around confidential clients...

However it does also sneak in these statements that could apply to PKCE flows:

If the client type is confidential or the client was issued client credentials (or assigned other authentication requirements), the client MUST authenticate with the authorization server as described in Section 3.2.1.

and

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.

Is the PKCE flow establishing an authentication method with the client, or just a single-use challenge?

I think as with the question of issuing refresh tokens at all, this really comes down to the level of acceptable risk for the authorisation server. On the one hand, I agree with @shrihari-prakash - the code_challenge is used when exchanging the auth_code for an access token (and optional refresh token), as the auth_code is the thing that could be intercepted and the authorisation server wants some assurance that it is communicating with the same client instance that the end-user authorised. Once the tokens have been released to the verified client, the code_challenge has done its job. This is laid out fairly clearly in the PKCE protocol, as it is only talking about associating the challenge with the authorisation code and not any other tokens that may be issued - but the fact that the spec is entirely silent on refresh tokens does make this hard to rely on as definitive.

On the other hand, associating the challenge with the client and for subsequent refreshes does seem sensible if one is going to actually issue refresh tokens and they want some level of assurance that they are refreshing the token with the same client as before. It fits in with the implied spec that the request should include the same authentication that was used in the original token exchange.

Although this is a rather long and meandering post, I think I'm settling that the challenge does not need to be supplied when exchanging the refresh token when using a public client. It is completely academic for a public client to provide the code challenge with a refresh token, if the client is not capable of secure storage, then a refresh token should not be issued. If it is capable of secure storage of the refresh token, then it doesn't need the code challenge to prove itself. A public client without the capability of secure storage wouldn't be able to store the challenge any more securely than the refresh token, so if one can be extracted from the client, then so can the other - providing both provides no real security benefit over just the refresh token.

Lastly, it appears that that Okta (who run Auth0) don't require the challenge when exchanging refresh tokens.


Just to touch on this specific issue as raised by OP:

If the client has a client_secret issued, then it must be providing its secret with every request. It shouldn't be able to use PKCE to circumvent the authentication requirement - if you need a client to do both un-authenticated PKCE and authenticated flows, they should be two separate clients (that is clear in the spec):

A client may be implemented as a distributed set of components, each with a different client type and security context (e.g., a distributed client with both a confidential server-based component and a public browser-based component). If the authorization server does not provide support for such clients or does not provide guidance with regard to their registration, the client SHOULD register each component as a separate client.

@jankapunkt is quite right in his "assumption", that if the client has a client secret, that it must provide that during the refresh exchange.

In regards to resolving this issue, I think we need to add the concept of confidential/public clients to the library because it's not just a case of enable or disable authentication on refresh endpoints, it's a case of appropriately authenticating a client holistically with the entire request context.

@jorenvandeweyer
Copy link
Member

To summarise two mechanism are here confused as one.

  • Confidential & Public Clients
  • The PKCE extension for authorization_code

They should both be implemented independently.

OAuth 2.0 Authorization Code Grant

tools.ietf.org/html/rfc6749#section-1.3.1

The Authorization Code grant type is used by confidential and public clients to exchange an authorization code for an access token.
After the user returns to the client via the redirect URL, the application will get the authorization code from the URL and use it to request an access token.
It is recommended that all clients use the PKCE extension with this flow as well to provide better security.


PKCE extension

PKCE is not a form of client authentication, and PKCE is not a replacement for a client secret or other client authentication. PKCE is recommended even if a client is using a client secret or other form of client authentication like private_key_jwt.

When the PKCE extension is used in an authentication flow, the server must require the code verifier on the code exchange.

The code verifier should never be used outside this flow (so not in the refresh token flow).

Examples

Confidential Client without PKCE
grant_type=authorization_code&
client_id=myclient&
client_secret=mysecret&
code=8b51da4fade34a5ccf0d1dc40816812052e2b3ae
Confidential Client with PKCE
grant_type=authorization_code&
client_id=myclient&
client_secret=mysecret&
code=8b51da4fade34a5ccf0d1dc40816812052e2b3ae&
code_verifier=e4fz4xjFKhlQUWXgsME5dp3CugzrYf1BLUwuywRu9gc
Public Client without PKCE
grant_type=authorization_code&
client_id=myclient&
code=8b51da4fade34a5ccf0d1dc40816812052e2b3ae
Public Client with PKCE
grant_type=authorization_code&
client_id=myclient&
code=8b51da4fade34a5ccf0d1dc40816812052e2b3ae&
code_verifier=e4fz4xjFKhlQUWXgsME5dp3CugzrYf1BLUwuywRu9gc

Confidential & Public Client

This is currently not implemented in the library but can be implemented by the implementor.

We already have an open issue for this improvement. #81


@jankapunkt Which changes are you proposing?

@MartinKolarik
Copy link

MartinKolarik commented Aug 30, 2024

@jorenvandeweyer's summary is correct here.

With grant_type=authorization_code, PKCE may or may not be used, and client_secret may or may not be required. Both decisions are up to the server and any of the four possible combinations is valid.

With grant_type=refresh_token, PKCE does not apply. client_secret may or may not be required, again, based on the decision of the server. For public clients, it may very well make sense to allow the refresh flow without a secret (though right now, this seems difficult to achieve as there's only one server-wide setting that applies to all clients).

@jankapunkt
Copy link
Member

@jorenvandeweyer @dhensby @shrihari-prakash how do we proceed with this? I think we should make a decision very soon. I currently have a milestone crunch until mid-November. After that I'll be free- @MartinKolarik offered to provide a PR but I may not be available to review and test until mid-Nov. What about you?

@shrihari-prakash
Copy link
Contributor

@jorenvandeweyer @dhensby @shrihari-prakash how do we proceed with this? I think we should make a decision very soon. I currently have a milestone crunch until mid-November. After that I'll be free- @MartinKolarik offered to provide a PR but I may not be available to review and test until mid-Nov. What about you?

On my side, I am more inclined towards the current behavior of the library because verifying refresh of a token is not a job of the code and verifier because there is no redirection / app handler registration involved in this step, but as @dhensby mentioned, this can definitely enhance the level of reassurance a bit. Is it a wiser choice to accept an option to opt into this new behavior?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compliance 📜 OAuth 2.0 standard compliance discussion 🗨️ Discussion about a particular topic. documentation 📑 Improvements or additions to documentation enhancement ✨ New feature or request hacktoberfest investigating 🔍
Projects
None yet
Development

No branches or pull requests

6 participants