Skip to content

Can't use a custom authorization grant type in a ClientRegistration #7040

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

Closed
edouardhue opened this issue Jun 26, 2019 · 7 comments · Fixed by #7047
Closed

Can't use a custom authorization grant type in a ClientRegistration #7040

edouardhue opened this issue Jun 26, 2019 · 7 comments · Fixed by #7047
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

@edouardhue
Copy link
Contributor

edouardhue commented Jun 26, 2019

Summary

Documentation suggests that defining additional grant types should be supported. Though, the ClientRegistration builder won't validate a registration using a custom grant type.

Actual Behavior

  1. Define a custom org.springframework.security.oauth2.core.AuthorizationGrantType.
  2. Build a org.springframework.security.oauth2.client.registration.ClientRegistration with this custom type.

When calling build(), any unsupported grant type is validated as an authorization code grant type, and it fails.

Expected Behavior

Custom grant type should not be validated with the wrong validator. It could be nice to be able to provide a custom validator.

Version

spring-security-oauth2-client 5.1.5.RELEASE

Sample

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 26, 2019
@jgrandja
Copy link
Contributor

@edouardhue Specifying a custom grant using AuthorizationGrantType provides no value as there is functionality that needs to be implemented associated with the grant_type. The current authorization grants supported in Spring Security 5.1 are authorization_code and client_credentials and password targeted for the upcoming 5.2 release.

I'm going to close this ticket as invalid. If you're looking for a specific grant_type and it's associated functionality (flow) than please log another ticket with more specific details. Thanks.

@jgrandja jgrandja self-assigned this Jun 26, 2019
@jgrandja jgrandja added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 26, 2019
@edouardhue
Copy link
Contributor Author

Hi @jgrandja,

I am currently migrating from Spring Security 4 to 5 and I can't port my code. I had implemented a custom grant type, on both authorization and resource server sides.

The Javadoc in https://docs.spring.io/spring-security/site/docs/current/api/org/springframework/security/oauth2/core/AuthorizationGrantType.html clearly states that "The OAuth 2.0 Authorization Framework defines four standard grant types: authorization code, implicit, resource owner password credentials, and client credentials. It also provides an extensibility mechanism for defining additional grant types."

AuthorizationGrantType's constructor is public, custom types may be defined in Spring Security 5. The default validation I pointed out in the ClientRegistration builder is the only point that won't allow a custom grant type to be used by a client. Looks like a violation of the open-closed principle to me!

@jgrandja
Copy link
Contributor

@edouardhue

I am currently migrating from Spring Security 4 to 5 and I can't port my code.

The new OAuth support was introduced in Spring Security 5. So I'm confused as to what you are porting since it wasn't available in Spring Security 4?

I had implemented a custom grant type, on both authorization and resource server sides.

Authorization Grants are implemented between the Client and Authorization Server NOT Resource Server. The Resource Server just receives the access token.

It's still not clear to me what you are trying to accomplish. Can you please be more specific with details so I can better understand.

@edouardhue
Copy link
Contributor Author

@jgrandja

Authorization Grants are implemented between the Client and Authorization Server NOT Resource Server. The Resource Server just receives the access token.

Sorry for my too quick answer yesterday, allow me to correct myself. I indeed meant client instead of resource server.

The new OAuth support was introduced in Spring Security 5. So I'm confused as to what you are porting since it wasn't available in Spring Security 4?

I used the spring-security-oauth subproject with Spring Security 4 (all under Spring Boot 1.5).

It's still not clear to me what you are trying to accomplish. Can you please be more specific with details so I can better understand.

For very specific reasons, I need to define a custom grant type carrying some custom credentials that can't fit into the standard grant types. On the client side, I only had to implement a custom OAuth2ProtectedResourceDetails and the matching AccessTokenProvider.

When migrating to Spring Security 5 native support, I extended AbstractOAuth2AuthorizationGrantRequest and implemented the matching Converter to RequestEntity and OAuth2AccessTokenResponseClient. I only miss the ability to declare this custom grant type in the ClientRegistration, as the validation in the Builder assumes that any registration with a grant type that is neither client_credentials nor implicit must use the authorization_code type.

I ask for this method to be changed to allow for extension:

public ClientRegistration build() {
	Assert.notNull(this.authorizationGrantType, "authorizationGrantType cannot be null");
	if (AuthorizationGrantType.CLIENT_CREDENTIALS.equals(this.authorizationGrantType)) {
		this.validateClientCredentialsGrantType();
	} else if (AuthorizationGrantType.IMPLICIT.equals(this.authorizationGrantType)) {
		this.validateImplicitGrantType();
	} else if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(this.authorizationGrantType)) {
		this.validateAuthorizationCodeGrantType();
	}
	return this.create();
}

Maybe a validation function could be passed to build(), to allow for custom or additional validation… but the private scoping of the attributes of the Builder would be a problem.

Making the OAuth2AuthorizationGrantRequestEntityUtils public could be useful too.

@jgrandja
Copy link
Contributor

Thank you for the explanation @edouardhue. I now understand how you are porting your code and the issue you are having with the ClientRegistration validation. We would welcome a PR from you if you're interested? Otherwise, I'll address this for you shortly.

FYI, I would recommend tracking the work in #6811 as it will help with creating a custom grant for the client with the introduction of the OAuth2AuthorizedClientProvider. The work is still in progress but it is targeted for 5.2 in Aug.

@jgrandja jgrandja reopened this Jun 27, 2019
@jgrandja jgrandja added type: enhancement A general enhancement and removed status: invalid An issue that we don't feel is valid labels Jun 27, 2019
@jgrandja jgrandja added this to the 5.2.x milestone Jun 27, 2019
@edouardhue
Copy link
Contributor Author

Thanks for reopening the issue. I'll submit a PR soon.

@edouardhue
Copy link
Contributor Author

There it is: #7047

edouardhue added a commit to edouardhue/spring-security that referenced this issue Jul 1, 2019
ClientRegistration.Builder defaulted to validating as an
authorization_code registration, though a custom grant type could be in
use. The actual grant_type is now verified for every case.
 - Fixed validation in ClientRegistration.Builder
 - New test that fails unless the issue is fixed.

Also made OAuth2AuthorizationGrantRequestEntityUtils public to help
implementing custom token response clients.

Fixes spring-projectsgh-7040
rwinch pushed a commit that referenced this issue Jul 3, 2019
ClientRegistration.Builder defaulted to validating as an
authorization_code registration, though a custom grant type could be in
use. The actual grant_type is now verified for every case.
 - Fixed validation in ClientRegistration.Builder
 - New test that fails unless the issue is fixed.

Also made OAuth2AuthorizationGrantRequestEntityUtils public to help
implementing custom token response clients.

Fixes gh-7040
@rwinch rwinch modified the milestones: 5.2.x, 5.2.0.RC1 Jul 3, 2019
rwinch pushed a commit that referenced this issue Jul 3, 2019
ClientRegistration.Builder defaulted to validating as an
authorization_code registration, though a custom grant type could be in
use. The actual grant_type is now verified for every case.
 - Fixed validation in ClientRegistration.Builder
 - New test that fails unless the issue is fixed.

Also made OAuth2AuthorizationGrantRequestEntityUtils public to help
implementing custom token response clients.

Fixes gh-7040
@spring-projects-issues spring-projects-issues added the status: backported An issue that has been backported to maintenance branches label Jul 3, 2019
kostya05983 pushed a commit to kostya05983/spring-security that referenced this issue Aug 26, 2019
ClientRegistration.Builder defaulted to validating as an
authorization_code registration, though a custom grant type could be in
use. The actual grant_type is now verified for every case.
 - Fixed validation in ClientRegistration.Builder
 - New test that fails unless the issue is fixed.

Also made OAuth2AuthorizationGrantRequestEntityUtils public to help
implementing custom token response clients.

Fixes spring-projectsgh-7040
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

Successfully merging a pull request may close this issue.

4 participants