-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Allow configurable refresh token strategy for authorization_code grant #1432
Allow configurable refresh token strategy for authorization_code grant #1432
Conversation
Merge via 71d9235 |
@jgrandja When this lands in 1.2.0, will we need to have a custom refresh token generator setup in our apps similar to https://github.com/spring-projects/spring-authorization-server/pull/1432/files#diff-ca4b9c945a7879f3295084d0dc482d46f195021de6f1c120e3bb6491cc25f387R732? Also, will this allow us to revoke the refresh token as well? |
@jgrandja Thank you so much for adding this feature, now I'm able to generate refresh tokens for my public client by defining the CustomRefreshTokenGenerator as seen in the tests. However, now there is a "contradiction." The authorization_code grant works without client authentication just with PKCE but refresh_token grant does not work with client authentication. This means that just for the sake of token refresh, I need to send a client secret to the auth server, which in the case of a public client makes no sense. I know that the recommendation (and the spring-authorization-server stance) is not using refresh tokens with public clients since they mean a potential point of attack stealing them from the browser but actually, server-side rendered webapps typically store session cookies and session hijacking is a similar potential point of attack. I also see that oauth2 is a bit different because session hijacking would affect the security of the webapp itself (=client) whereas stealing a refresh token would affect the resource server. However, in my case, I have a microservices application with oauth2-based SSO so in my case, it could be seen as one system. We used to have the old Spring Boot Oauth2 Authorization Server with password grant but now we are migrating to spring-authorization-server. So I'd prefer using refresh tokens without client authentication. Could you please help me if it is possible with configuration? Or would that require further changes regarding how refresh tokens are handled? Thank you very much in advance. |
Hi @jgrandja Is it possible to apply the custom client authentication configuration implemented in your test above when OpenID Connect 1.0 is enabled? Configuration details of the ClientAuthenticationMethod & AuthorizationGrantType values: Scope: Configuration details of the ClientSettings values: Thanks in advance. |
Does anyone have it working and using it productively? |
Yes, I got it working looking at the tests provided by jgrandja in these 2 commits test1 and test2 By default spring provides a refresh token generator bean which prevents refresh tokens from being issued when client authentication method is NONE (which is the case for public clients with PKCE). The test shows how you can provide a token generator bean for refresh tokens, thus allowing you to omit the condition in the default bean.
The above test (first commit I linked above) will allow you to generate a refresh token but you will still not be able to use the refresh token without actually providing the client secret, which defeats the purpose... the second test provides the complete solution enabling you to use the refresh token with no client authentication method
|
Hey @AfeefRazick, thanks for the code sample! When generating the refresh token, this snippet creates a new instance of private static final class CustomRefreshTokenGenerator implements OAuth2TokenGenerator<OAuth2RefreshToken> {
private final OAuth2RefreshTokenGenerator delegate = new OAuth2RefreshTokenGenerator();
@Nullable
@Override
public OAuth2RefreshToken generate(OAuth2TokenContext context) {
if (context.getAuthorizedScopes().contains(OidcScopes.OPENID) &&
!context.getAuthorizedScopes().contains("offline_access")) {
return null;
}
return this.delegate.generate(context);
}
} The check for the public client is in this Instead of delegating, shouldn't we generate a token and return a |
Hi @Suvink . In the code snippet provided
We are overriding the default refresh token generator. In our version of the refresh token generator we don't have the condition "if pkce return null" that is in the default refresh token generator you linked.
We are generating a token and returning it... via overriding the default generator bean. Spring follows the delegation mechanism for token generation. If I have mentioned anything wrong, please let me know. |
I assume you're referring to this condition in the default RefreshTokenGenerator? if (isPublicClientForAuthorizationCodeGrant(context)) {
// Do not issue refresh token to public client
return null;
}
I still don't understand this part. I added some debug points and followed through the classes. The delegation mechanism works well. It first hits the default RefreshTokenGenerator and it returns In the code snippet in my previous comment, we are creating a new instance of the default token generator and delegating the generation to it within the overridden generate method. private final OAuth2RefreshTokenGenerator delegate = new OAuth2RefreshTokenGenerator(); This default token generator has the check for the public client and will return Following is the implementation I did for the public class RefreshTokenGenerator implements OAuth2TokenGenerator<OAuth2RefreshToken> {
private final StringKeyGenerator REFRESH_TOKEN_GENERATOR = new Base64StringKeyGenerator(
Base64.getUrlEncoder().withoutPadding(), 96);
@Nullable
@Override
public OAuth2RefreshToken generate(OAuth2TokenContext context) {
if (context.getAuthorizedScopes().contains(OidcScopes.OPENID) &&
context.getAuthorizedScopes().contains("offline_access)) {
return null;
}
Instant issuedAt = Instant.now();
Instant expiresAt = issuedAt.plus(context.getRegisteredClient().getTokenSettings().getRefreshTokenTimeToLive());
return new OAuth2RefreshToken(this.REFRESH_TOKEN_GENERATOR.generateKey(), issuedAt, expiresAt);
}
} WDYT? |
Ah yes, good catch. What you just provided is identical to my implementation which follows this commit changes on line 951 - 986.
It seems I have made a mistake in my comment, I was unable to share my actual implementation so I resorted to the commit above but added the wrong snippet from that commit 😅. Apologies for the confusion. I hope you were able to get the usage of the refresh token part working as well. |
No worries. This is working fine now. |
@AfeefRazick I have been following this thread and the solution worked fine, but it seems to have broken confidential clients being able to use the refresh token to generate a new access token. Here is my implementation: package com.myapp.authorization.configuration
import jakarta.servlet.http.HttpServletRequest
import org.springframework.security.authentication.AuthenticationProvider
import org.springframework.security.core.Authentication
import org.springframework.security.core.Transient
import org.springframework.security.crypto.keygen.Base64StringKeyGenerator
import org.springframework.security.crypto.keygen.StringKeyGenerator
import org.springframework.security.oauth2.core.AuthorizationGrantType
import org.springframework.security.oauth2.core.ClientAuthenticationMethod
import org.springframework.security.oauth2.core.OAuth2AuthenticationException
import org.springframework.security.oauth2.core.OAuth2Error
import org.springframework.security.oauth2.core.OAuth2ErrorCodes
import org.springframework.security.oauth2.core.OAuth2RefreshToken
import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames
import org.springframework.security.oauth2.server.authorization.OAuth2TokenType
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2ClientAuthenticationToken
import org.springframework.security.oauth2.server.authorization.client.RegisteredClient
import org.springframework.security.oauth2.server.authorization.client.RegisteredClientRepository
import org.springframework.security.oauth2.server.authorization.token.OAuth2TokenContext
import org.springframework.security.oauth2.server.authorization.token.OAuth2TokenGenerator
import org.springframework.security.web.authentication.AuthenticationConverter
import java.time.Instant
import java.util.*
/***
* Source: https://github.com/spring-projects/spring-authorization-server/pull/1432
*
* Spring authorization server doesn't support issuing refresh tokens for public clients.
* To support this feature, we have to implement a custom authentication converter, authentication
* provider and refresh token generator [CustomRefreshTokenGenerator]
*/
class PublicClientRefreshTokenAuthenticationConverter : AuthenticationConverter {
override fun convert(request: HttpServletRequest): Authentication? {
// grant_type (REQUIRED)
val grantType = request.getParameter(OAuth2ParameterNames.GRANT_TYPE)
if (AuthorizationGrantType.REFRESH_TOKEN.value != grantType) {
return null
}
// client_id (REQUIRED)
val clientId = request.getParameter(OAuth2ParameterNames.CLIENT_ID)
if (clientId.isNullOrBlank()) {
return null
}
return PublicClientRefreshTokenAuthenticationToken(clientId)
}
}
@Transient
class PublicClientRefreshTokenAuthenticationToken : OAuth2ClientAuthenticationToken {
constructor(clientId: String) : super(clientId, ClientAuthenticationMethod.NONE, null, null)
constructor(registeredClient: RegisteredClient) : super(registeredClient, ClientAuthenticationMethod.NONE, null)
}
class PublicClientRefreshTokenAuthenticationProvider(private val registeredClientRepository: RegisteredClientRepository) :
AuthenticationProvider {
override fun authenticate(authentication: Authentication): Authentication? {
val publicClientAuthentication: PublicClientRefreshTokenAuthenticationToken =
authentication as PublicClientRefreshTokenAuthenticationToken
if (!ClientAuthenticationMethod.NONE.equals(publicClientAuthentication.clientAuthenticationMethod)) {
return null
}
val clientId: String = publicClientAuthentication.principal.toString()
val registeredClient = registeredClientRepository.findByClientId(clientId)
if (registeredClient == null) {
throwInvalidClient(OAuth2ParameterNames.CLIENT_ID)
}
if (!registeredClient!!.clientAuthenticationMethods.contains(
publicClientAuthentication.clientAuthenticationMethod,
)
) {
throwInvalidClient("authentication_method")
}
return PublicClientRefreshTokenAuthenticationToken(registeredClient)
}
override fun supports(authentication: Class<*>?): Boolean {
return PublicClientRefreshTokenAuthenticationToken::class.java.isAssignableFrom(authentication)
}
companion object {
private fun throwInvalidClient(parameterName: String) {
val error = OAuth2Error(
OAuth2ErrorCodes.INVALID_CLIENT,
"Public client authentication failed: $parameterName",
null,
)
throw OAuth2AuthenticationException(error)
}
}
}
/**
* Custom refresh token generator that overrides [org.springframework.security.oauth2.server.authorization.token.OAuth2RefreshTokenGenerator]
* to allow generating a refresh token for public clients
*/
class CustomRefreshTokenGenerator : OAuth2TokenGenerator<OAuth2RefreshToken?> {
private val refreshTokenGenerator: StringKeyGenerator =
Base64StringKeyGenerator(Base64.getUrlEncoder().withoutPadding(), 96)
override fun generate(context: OAuth2TokenContext): OAuth2RefreshToken? {
if (!OAuth2TokenType.REFRESH_TOKEN.equals(context.tokenType)) {
return null
}
val issuedAt = Instant.now()
val expiresAt = issuedAt.plus(context.registeredClient.tokenSettings.refreshTokenTimeToLive)
return OAuth2RefreshToken(this.refreshTokenGenerator.generateKey(), issuedAt, expiresAt)
}
}
** Security Configuration ** val authorizationServerConfigurer = OAuth2AuthorizationServerConfigurer()
authorizationServerConfigurer.oidc {
it.providerConfigurationEndpoint { providerConfiguration ->
providerConfiguration.providerConfigurationCustomizer { customizer ->
customizer.jwkSetUrl("https://" + authorizationServerConfigProperties.hostname + "/oauth2/jwks")
.authorizationEndpoint("https://" + authorizationServerConfigProperties.hostname + "/oauth2/authorize")
.tokenEndpoint("https://" + authorizationServerConfigProperties.hostname + "/oauth2/token")
.tokenRevocationEndpoint("https://" + authorizationServerConfigProperties.hostname + "/oauth2/revoke")
}
}
it.userInfoEndpoint { Customizer.withDefaults<OidcUserInfoEndpointConfigurer>() }
}
.clientAuthentication {
it
.authenticationConverter(PublicClientRefreshTokenAuthenticationConverter())
.authenticationProvider(PublicClientRefreshTokenAuthenticationProvider(registeredClientRepository))
} Token Generator @Bean
fun tokenGenerator(jwkSource: JWKSource<SecurityContext>, jwtCustomizer: OAuth2TokenCustomizer<JwtEncodingContext>): OAuth2TokenGenerator<*> {
val jwtGenerator = JwtGenerator(NimbusJwtEncoder(jwkSource))
jwtGenerator.setJwtCustomizer(jwtCustomizer)
val refreshTokenGenerator: OAuth2TokenGenerator<OAuth2RefreshToken?> = CustomRefreshTokenGenerator()
return DelegatingOAuth2TokenGenerator(jwtGenerator, refreshTokenGenerator)
} This seems to break the existing functionality where confidential clients could exchange a refresh token for an access token. This code from if (!registeredClient!!.clientAuthenticationMethods.contains(
publicClientAuthentication.clientAuthenticationMethod,
)
) {
throwInvalidClient("authentication_method")
} Any help with this? Isn't there another authentication converter that is supposed to handle this case before PublicClientRefreshTokenAuthenticationConverter kicks in? |
Hi, I am unable to confirm this right now, but you are may be right about having to add another converter before this converter. Either, springs default authentication converter (I assume ClientSecretPostAuthenticationConverter, or one of the other converters listed here Another issue is that the PublicClientAuthenticationConverter we have here is assuming any refresh token request is intended for it, when it should only look at refresh token requests without a client secret. So if you can implement a check for "if client secret is null". That may also work, allowing it to pass to the following converter implementations. This is an assumption, I can not confirm as of now. |
I updated my converter implementation to check if a client_secret is present in the request, we consider it a confidential client and return null, ensuring the other converts get a chance to authenticate, and it seems to work with a few of my test cases. Here is the code override fun convert(request: HttpServletRequest): Authentication? {
// grant_type (REQUIRED)
val grantType = request.getParameter(OAuth2ParameterNames.GRANT_TYPE)
if (AuthorizationGrantType.REFRESH_TOKEN.value != grantType) {
return null
}
// client_id (REQUIRED)
val clientId = request.getParameter(OAuth2ParameterNames.CLIENT_ID)
if (clientId.isNullOrBlank()) {
return null
}
// client_secret (must be excluded to be considered "public" client)
val clientSecret = request.getParameter(OAuth2ParameterNames.CLIENT_SECRET)
if (!clientSecret.isNullOrBlank()) {
return null
}
return PublicClientRefreshTokenAuthenticationToken(clientId)
} However, itt seems the class MyAuthentication: AbstractAuthenticationToken(AuthorityUtils.createAuthorityList(authority)) {
override fun getPrincipal(): Any {
return principal // this is a custom object which extends UserDetails
}
// omitted for brevity
} class MyAuthenticationHandler(): AuthenticationSuccessHandler() {
override fun onAuthenticationSuccess(
request: HttpServletRequest,
response: HttpServletResponse,
authentication: Authentication,
) {
val newAuth = MyAuthentication()
val securityContext: SecurityContext = SecurityContextHolder.createEmptyContext()
securityContext.authentication =
SecurityContextHolder.setContext(securityContext)
securityContextRepository.saveContext(securityContext, request, response)
// write response
}
} Here is where the error is thrown OAuth2TokenRevocationAuthenticationProvider -> OAuth2AuthenticationProviderUtils |
I'm glad that previous solution helped. About the revoking, I'm honestly not sure myself. However, this is where I can point you to. It seems that for the revocation, there is a different chain of revocation authentication converters as well as providers. And it can be configured here. OAuth2TokenRevocationEndpointConfigurer It seems that this is the (or one of many) default converter. OAuth2TokenRevocationAuthenticationConverter |
Closes gh-1430
Related gh-1422