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

Enable refresh_token usage for public clients #2138

Closed
jwdomes opened this issue Dec 14, 2022 · 10 comments · Fixed by #2402
Closed

Enable refresh_token usage for public clients #2138

jwdomes opened this issue Dec 14, 2022 · 10 comments · Fixed by #2402
Assignees
Labels
accepted Accepted the issue
Milestone

Comments

@jwdomes
Copy link

jwdomes commented Dec 14, 2022

Does CF UAA have a configuration that allows for public clients who used PKCE in the authorization code grant flow (and thus don't have a client_secret) to use a refresh_token without requiring a client_secret in the refresh request?

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/184036829

The labels on this github issue will be updated when the story is started.

@Tallicia
Copy link
Contributor

Are you referring using the code_challenge without a client_secret? https://docs.cloudfoundry.org/api/uaa/version/76.5.0/index.html#overview

@Tallicia
Copy link
Contributor

Is an empty string solution sufficient for your need, or is there something more?

It appears that the CF CLI uses the refresh endpoint using a public client as well: https://github.com/cloudfoundry/cli/blob/main/api/uaa/refresh_token.go#L48
They do pass client credentials in the Authentication header with value of:
"UAAOAuthClient": "cf",
"UAAOAuthClientSecret": ""
refresh_token.go
request.SetBasicAuth(client.config.UAAOAuthClient(), client.config.UAAOAuthClientSecret())

@strehle
Copy link
Member

strehle commented Jan 19, 2023

@jwdomes I think you mean option from #1888 where you can omit client_secret in authz code flow. Because in refresh_token grant you are right, this is not prepared for public usage.
We need to have the information about client_auth in the token, because I dont want to allow a refresh flow without a secret in cases where the token was requested with a secret

@jwdomes
Copy link
Author

jwdomes commented Jan 24, 2023

@strehle - would there be any option to implement the refresh_token grant flow for public clients that makes use of the code_challenge and/or code_verification in lieu of the client_secret used for non-public clients?

As it stands, the public client needs to make a call to the refresh endpoint within the lifetime of the auth token (for us that's 10 mins) or we're forced to make the user reauthenticate.

@Tallicia -

Is an empty string solution sufficient for your need, or is there something more?

It appears that the CF CLI uses the refresh endpoint using a public client as well: https://github.com/cloudfoundry/cli/blob/main/api/uaa/refresh_token.go#L48 They do pass client credentials in the Authentication header with value of: "UAAOAuthClient": "cf", "UAAOAuthClientSecret": "" refresh_token.go request.SetBasicAuth(client.config.UAAOAuthClient(), client.config.UAAOAuthClientSecret())

Does this imply that the refresh_token can be obtained with an empty client_secret in a public client scenario?

@strehle
Copy link
Member

strehle commented Jan 25, 2023

@jwdomes yes I can do this, however the refresh would work for tokens which are received only via authorization_code.

The cf client - typically used for "cf login" is if no changes are done a oauth2 client with an empty secret "" and "authorized_grant_types: password refresh_token".
This means, you should use here the known empty secret for a refresh flow. This client has normally no authorization_code in the list.

The term public and PKCE is bound to grant type "authorization_code". This combination should replace the implit flow.
In such scenarios you completly omit the parameter "client_secret" and/or Authorization header.

So my fix would allow a refresh flow only for such tokens. Tokens via password grant , e.g. "cf login " are bound to password grant and for this grant type the standard has no public usage allowed.

btw: https://oauth.net/2.1/ is not official.
If you use spring security in your client, then you can see how it works if you set in your application.yml

  • client-secret is omitted
    or
  • client-authentication-method is set to none (ClientAuthenticationMethod.NONE)

see:
https://docs.spring.io/spring-security/reference/servlet/oauth2/client/authorization-grants.html

@strehle strehle moved this from Inbox to Pending Review | Discussion in Foundational Infrastructure Working Group Jan 25, 2023
@cf-foundation-community-automation cf-foundation-community-automation bot moved this from Pending Review | Discussion to Inbox in Foundational Infrastructure Working Group Feb 1, 2023
@sandrooco
Copy link

What is the currently recommended approach/library to authenticating frontends when using UAA?

I have a frontend app and would like to use MSAL (official Microsoft auth package) for auth but it's not compatible since UAA doesn't support refresh_token for public clients.
Previously we've been using oidc-client-js with implicit grant which is deprecated now...

@strehle
Copy link
Member

strehle commented May 5, 2023

Hi, will work on it and provide this since problem understood.

@strehle strehle self-assigned this May 5, 2023
@strehle
Copy link
Member

strehle commented Jun 22, 2023

First PR ready: #2385
-> store the client authentication in UAA context.

Next we then use this and check for refresh flow that public tokens can be refreshed

@strehle
Copy link
Member

strehle commented Jul 12, 2023

created a PR #2402 and to be on safe side , also with standard
https://oauth.net/2.1/ , this will be allowed only if

  1. token before was generated without a secret
  2. refresh rotation is enabled

@cf-gitbot cf-gitbot added accepted Accepted the issue and removed delivered labels Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Accepted the issue
Projects
Development

Successfully merging a pull request may close this issue.

5 participants