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

Make autoconfig work better for web app with client credentials and @EnableOAuth2Client #5011

Closed
fkowal opened this issue Jan 22, 2016 · 5 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@fkowal
Copy link

fkowal commented Jan 22, 2016

I have a web application, configured like this

security:
  basic.enabled: false
  sessions: stateless
  oauth2:
    client:
      id: 'id'
      grant-type: client_credentials
      client-id: client_id
      client-secret: secret
      scope: myscope
      access-token-uri: http://someuri/oauth/token
    resource:
      jwt.key-uri: http://someuri/oauth/token_key

I @EnableOAuth2Client hoping that OAuthRestTemplate would use client_credentials on use.

sadly OAuth2RestOperationsConfiguration has the following code:

    protected abstract static class BaseConfiguration {

        @Bean
        @ConfigurationProperties("security.oauth2.client")
        @Primary
        public AuthorizationCodeResourceDetails oauth2RemoteResource() {
            AuthorizationCodeResourceDetails details = new AuthorizationCodeResourceDetails();
            return details;
        }

    }

    @Configuration
    @ConditionalOnNotWebApplication
    protected static class SingletonScopedConfiguration {

        @Bean
        @ConfigurationProperties("security.oauth2.client")
        @Primary
        public ClientCredentialsResourceDetails oauth2RemoteResource() {
            ClientCredentialsResourceDetails details = new ClientCredentialsResourceDetails();
            return details;
        }

        @Bean
        public DefaultOAuth2ClientContext oauth2ClientContext() {
            return new DefaultOAuth2ClientContext(new DefaultAccessTokenRequest());
        }

    }

SingletonScopedConfiguration is not used because of ConditionalOnNotWebApplication

AuthorizationCodeResourceDetails is created BUT the grant_type is overriden and set to client_credentials :/. This causes AuthorizationCodeAccessTokenProvider to fail on

public boolean supportsResource(OAuth2ProtectedResourceDetails resource) {
        return resource instanceof AuthorizationCodeResourceDetails
                && "authorization_code".equals(resource.getGrantType());
    }

Can i make a pull request changing @ConditionalOnNotWebApplication
to @ConditionalOnProperty(value="spring.oauth2.client.grant-type", havingValue="client-credentials") ?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 22, 2016
@dsyer
Copy link
Member

dsyer commented Jan 24, 2016

I don't think we'd want to change the default behaviour, but if you want to send a pull request with an explicit check for the grant type as well, that's probably fine.

@dsyer dsyer added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 24, 2016
@dsyer dsyer added this to the 1.4.2 milestone Sep 28, 2016
@dsyer
Copy link
Member

dsyer commented Sep 28, 2016

Actually, it is kind of screwy that an app that explicitly declares @EnableOAuth2Client and configures a security.oauth2.client.* for client credentials shouldn't be rewarded with a little more autoconfiguration love. It catches everyone out who tries to do it and it's not a big thing to fix. The OAuth2ClientContext is already created, but not the ClientCredentialsResourceDetails.

@dsyer dsyer self-assigned this Sep 28, 2016
@snicoll
Copy link
Member

snicoll commented Sep 28, 2016

Is there a reason to change this in 1.4.x. I agree with the general idea but that could come with side effects. 1.5 seems more sensible to me.

@dsyer
Copy link
Member

dsyer commented Sep 28, 2016

I guess I don't mind. Lots of people asking about it now, is all, and 1.5 is not ready yet.

@wilkinsona wilkinsona modified the milestones: 1.5.0 M1, 1.4.2 Sep 29, 2016
@philwebb philwebb modified the milestones: 1.5.0 RC1, 1.5.0 M1 Dec 20, 2016
@dsyer dsyer changed the title oauth2 rest operation configuration invalid with @EnableOAuth2Client Make autoconfig work better for web app with client credentials and @EnableOAuth2Client Dec 20, 2016
@dsyer
Copy link
Member

dsyer commented Dec 20, 2016

Fixed in 77a1a3b

@dsyer dsyer closed this as completed Dec 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants