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

WebClient support should get new access token when expired and client_credentials #5893

Closed
rwinch opened this issue Sep 21, 2018 · 17 comments
Closed
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Milestone

Comments

@rwinch
Copy link
Member

rwinch commented Sep 21, 2018

Summary

WebClient support should get new access token when an access token is expired and client_credentials is used

@rwinch rwinch added this to the 5.1.1 milestone Sep 21, 2018
@rwinch rwinch added Reactive in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) labels Sep 21, 2018
@rwinch rwinch modified the milestones: 5.1.1, 5.1.2 Oct 12, 2018
@vpavic
Copy link
Contributor

vpavic commented Oct 17, 2018

I don't think this behavior should be enabled by default since the OAuth 2.0 spec actually discourages the issuing/use of refresh tokens with client credentials grant - see https://tools.ietf.org/html/rfc6749#section-4.4.3.

This should probably be made as opt-in feature.

@dlesnef1
Copy link

@rwinch I think there was a misunderstanding of my original question.

My Authentication Server does not return a refresh_token, only the standard response outlined by https://tools.ietf.org/html/rfc6749#section-4.4.3.

What I am attempting to do is configure WebClient (make a Bean) such that it makes the access request to the Authentication Server under the hood and remembers the access_token. Then I can autowire in the Bean and make requests to a protected resource server without having to provide the access_token directly.

Here is the old configuration for a RestTemplate Bean.

@Bean
	@Qualifier("blah")
	public RestTemplate oAuthRestTemplate() {
		ClientCredentialsResourceDetails resourceDetails = new ClientCredentialsResourceDetails();
		resourceDetails.setClientId(clientId);
		resourceDetails.setClientSecret(clientSecret);
		resourceDetails.setAccessTokenUri(accessTokenUri);
		resourceDetails.setScope(scopes);
		resourceDetails.setGrantType("client_credentials");

		RestTemplate restTemplate = new OAuth2RestTemplate(resourceDetails, new DefaultOAuth2ClientContext());
		restTemplate.setErrorHandler(new DefaultResponseErrorHandler());
		return restTemplate;
	}

By refreshing the token, I mean it will be the configured WebClient's job to notice the access_token is expiring (the response includes the default "expires_in" field) and make a request to the Authentication Server to reset the token.

@rwinch rwinch changed the title WebClient support should refresh client_credentials WebClient support should get new access token when expired and client_credentials Oct 17, 2018
@rwinch
Copy link
Member Author

rwinch commented Oct 17, 2018

@dlesnef1 You have described what I view this feature as doing. I have updated the title to make it more clear that it isn't using a refresh token. Any interest in submitting a PR for this?

@rwinch rwinch modified the milestones: 5.1.2, 5.2.0.M1, 5.2.x Oct 17, 2018
@dlesnef1
Copy link

Hey @rwinch I will definitely give it a go. I read the contributing guide and peaked at some other PRs.

Any advice on getting started would be appreciated, but I will begin looking at the code soon.

@rwinch
Copy link
Member Author

rwinch commented Oct 18, 2018

You will want to update it in the following locations:

We probably want to make it clear in the naming and in the method it is invoked that this is technically not a refresh, so rather than placing it in a method talking about refresh, then we would want to place it in a method which gets a new access token when client credentials is expired.

If you need feedback you can create a PR and I can take a look from there. Feel free to reach out with questions too

@warrenbailey
Copy link

Hi all, I've just hit this issue too. I'm using an UnAuthenticatedServerOAuth2AuthorizedClientRepository as I'm doing server to server Oauth 2 using a client credentials grant. That repository stores the access token in a hash map and continually reuses it, which is great until the token expires as there's no code in place to get a new token.

I can work around it by creating a wrapper for the authorised client repository which checks the token expiry when it's loaded. However I agree this functionality should be in the exchange filter function.

@dlesnef1 Any progress on a PR? If not, I can take a look.

@rwinch
Copy link
Member Author

rwinch commented Nov 7, 2018

@warrenbailey Thanks for the offer to look at this. So I don't forget to respond...Let's give @dlesnef1 a week to respond. If you don't hear anything, I think it is safe for you to take the PR

@rwinch
Copy link
Member Author

rwinch commented Nov 14, 2018

@dlesnef1 Any updates on this? If not, can we let @warrenbailey take a look?

@dlesnef1
Copy link

Hey, sorry for the late delay. Please use @warrenbailey's work as I will not be able to finish this before they are able too.

@warrenbailey
Copy link

Thanks @rwinch @dlesnef1 I'll take a look at it

@warrenbailey
Copy link

I've had a go at fixing it. I've tested the fix against my problem and it solves it. However I'd value some feedback on PR as Reactive is pretty new to me.

@rwinch rwinch removed this from the 5.2.x milestone Nov 30, 2018
@rwinch rwinch added status: duplicate A duplicate of another issue and removed status: duplicate A duplicate of another issue labels Nov 30, 2018
@rwinch rwinch added this to the 5.2.x milestone Nov 30, 2018
@jgrandja jgrandja added the type: enhancement A general enhancement label Dec 13, 2018
@jgrandja jgrandja modified the milestones: 5.2.x, 5.2.0.M1 Dec 13, 2018
warrenbailey pushed a commit to warrenbailey/spring-security that referenced this issue Dec 22, 2018
…ials token.

Once client credentials access token has expired retrieve a new token from the OAuth2 authorization server.
These tokens can't be refreshed because they do not have a refresh token associated with. This is standard behaviour for Oauth 2 client credentails
@xyloman
Copy link
Contributor

xyloman commented Apr 2, 2019

@rwinch will this feature be back ported to spring security 5.1.x and included in spring boot 2.1.x?

@jgrandja
Copy link
Contributor

jgrandja commented Apr 8, 2019

@xyloman This is a new feature so it won't be back-ported to 5.1.x.

@xyloman
Copy link
Contributor

xyloman commented Nov 26, 2019

@jgrandja we are still dealing with client credentials expiring and the tokens not being refreshed when on spring boot 2.1.10.RELEASE is there a workaround or plans to support refreshing client credential tokens when they are expired in spring security 5.1.x?

@xyloman
Copy link
Contributor

xyloman commented Nov 27, 2019

@jgrandja I should also add that upgrading to spring boot 2.2.1 would likely fix this issue but we are needing spring cloud as well in all use cases.

@arundotin
Copy link

I upgraded to spring boot 2.2.1 version and i still face the same issue... WebClient couldn't make successful call to the resource-server once the token that it received for the first time (during boot-up) is expired

@jgrandja jgrandja added the status: backported An issue that has been backported to maintenance branches label Nov 29, 2019
jgrandja pushed a commit that referenced this issue Nov 29, 2019
Once client credentials access token has expired retrieve a new token from the OAuth2 authorization server.
These tokens can't be refreshed because they do not have a refresh token associated with. This is standard behaviour for Oauth 2 client credentails

Fixes gh-5893
@jgrandja
Copy link
Contributor

Apologies @xyloman, as this should have been back ported to 5.1.x. I reviewed the changes again and this is a patch fix so I went ahead and back-ported via #7685.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

8 participants