Skip to content

Conversation

@aotitoola
Copy link

Fixes: gh-6477

@pivotal-issuemaster
Copy link

@d3jie Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@d3jie Thank you for signing the Contributor License Agreement!

@aotitoola aotitoola changed the title OAuth2LoginSpec discovers ReactiveOAuth2AccessTokenResponseClient @bean #6477 OAuth2LoginSpec discovers ReactiveOAuth2AccessTokenResponseClient @bean Mar 3, 2019
@jgrandja jgrandja self-assigned this Mar 4, 2019
@jgrandja jgrandja added in: config An issue in spring-security-config type: enhancement A general enhancement Reactive in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) labels Mar 4, 2019
@jgrandja jgrandja added this to the 5.2.0.M2 milestone Mar 4, 2019
@jgrandja
Copy link
Contributor

jgrandja commented Mar 4, 2019

Thanks for the PR @d3jie! I applied some minor polish to the commit and merged to master!
Thanks again and we look forward to your next contribution :)

@jgrandja jgrandja closed this Mar 4, 2019
@aotitoola
Copy link
Author

Thanks @jgrandja. Can you tell me about the minor polish you applied? Maybe I can learn a thing or two. :)

@jgrandja
Copy link
Contributor

jgrandja commented Mar 5, 2019

Sure @d3jie

In createDefault(), you had...

ReactiveAuthenticationManager result = new OAuth2LoginReactiveAuthenticationManager(getAccessTokenResponseClient(), getOauth2UserService());

...

OidcAuthorizationCodeReactiveAuthenticationManager oidc = new OidcAuthorizationCodeReactiveAuthenticationManager(getAccessTokenResponseClient(), getOidcUserService());

I changed it to...

ReactiveOAuth2AccessTokenResponseClient<OAuth2AuthorizationCodeGrantRequest> client = getAccessTokenResponseClient();

ReactiveAuthenticationManager result = new OAuth2LoginReactiveAuthenticationManager(client, getOauth2UserService());

...

OidcAuthorizationCodeReactiveAuthenticationManager oidc = new OidcAuthorizationCodeReactiveAuthenticationManager(client, getOidcUserService());

So we re-use the client instance in both managers if the @Bean was not provided.

And inOAuth2LoginTests, I renamed the @Bean method from oAuth2AccessTokenResponseClient() to accessTokenResponseClient(). We typically don't prefix member variables or method declarations with oauth2*.

Those were the only 2 changes.

@aotitoola
Copy link
Author

@jgrandja I can see what you did there. It didn't occur to me that I could do it this way. Many thanks. Looking forward to the next. 👍

@aotitoola aotitoola deleted the gh-6477 branch March 7, 2019 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in: config An issue in spring-security-config in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OAuth2LoginSpec discovers ReactiveOAuth2AccessTokenResponseClient @Bean

3 participants