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

Expose RestOperations in DefaultOAuth2UserService and CustomUserTypesOAuth2UserService #5641

Merged
merged 3 commits into from
Sep 5, 2018

Conversation

jgrandja
Copy link
Contributor

@jgrandja jgrandja commented Aug 4, 2018

Fixes gh-5600, Fixes gh-5602

Copy link
Member

@rwinch rwinch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My initial round of feedback is the same as #5656 (comment)


// A Bearer Token Error may be in the WWW-Authenticate response header
// See https://tools.ietf.org/html/rfc6750#section-3
OAuth2Error oauth2Error = this.readErrorFromWwwAuthenticate(response.getHeaders());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub doesn't show it, but there is a tab between OAuth2Error oauth2Error. Please change this to a space to be consistent formatting.


@Before
public void setup() {
ClientRegistration clientRegistration = ClientRegistration.withRegistrationId("registration-1")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use leverage TestClientRegistrations here?

this.userRequest.getAccessToken().getTokenValue());
}

private ClientRegistration.Builder from(ClientRegistration registration) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could remove this method by creating a ClientRegistration.Builder as a member variable and then you can modify it in the tests before building. Alternatively, you could also directly use TestClientRegistrations and modify it. I'd probably prefer the layer of indirection with the member variable so refactoring would be easier.

@jgrandja
Copy link
Contributor Author

jgrandja commented Sep 4, 2018

@rwinch I applied the updates based on your feedback.

@rwinch rwinch self-assigned this Sep 5, 2018
@rwinch rwinch merged commit dfd572a into spring-projects:master Sep 5, 2018
@rwinch rwinch added type: enhancement A general enhancement in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: duplicate A duplicate of another issue labels Sep 5, 2018
@rwinch rwinch added this to the 5.1.0.RC2 milestone Sep 5, 2018
@jgrandja jgrandja deleted the gh-5601-rest-ops branch September 5, 2018 13:03
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: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants