Skip to content

Implemented solution and tests for "OAuth2AuthorizedClientArgumentResolver should get new access token when expired and client_credentials #6609" #6839

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.

Sign up for GitHub

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
wants to merge 5 commits into from
Closed

Implemented solution and tests for "OAuth2AuthorizedClientArgumentResolver should get new access token when expired and client_credentials #6609" #6839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5a94d18
Implemented code+tests for both imperative & reactive codelines for i…
mkheck May 3, 2019
ff0c229
Cleanup of imports in test
mkheck May 3, 2019
a8e2b6b
Removed clock/expiration checking per Joe for #6609
mkheck May 7, 2019
01ab4e7
Removed extra comment lines
mkheck May 7, 2019
6b21b50
Removed extra comment lines
mkheck May 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .../security/oauth2/client/web/method/annotation/OAuth2AuthorizedClientArgumentResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ public Object resolveArgument(MethodParameter parameter,
"@RegisteredOAuth2AuthorizedClient(registrationId = \"client1\").");
}

ClientRegistration clientRegistration = this.clientRegistrationRepository.findByRegistrationId(clientRegistrationId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on my explanation in this comment, we don't need any changes here as the current implementation does exactly what is required. Please reset the changes here.

if (clientRegistration == null) {
return null;
}

Authentication principal = SecurityContextHolder.getContext().getAuthentication();
HttpServletRequest servletRequest = webRequest.getNativeRequest(HttpServletRequest.class);

Expand All @@ -114,11 +119,6 @@ public Object resolveArgument(MethodParameter parameter,
return authorizedClient;
}

ClientRegistration clientRegistration = this.clientRegistrationRepository.findByRegistrationId(clientRegistrationId);
if (clientRegistration == null) {
return null;
}

if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(clientRegistration.getAuthorizationGrantType())) {
throw new ClientAuthorizationRequiredException(clientRegistrationId);
}
Expand Down
20 changes: 10 additions & 10 deletions ...k/security/oauth2/client/web/reactive/function/client/OAuth2AuthorizedClientResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public void setClientCredentialsTokenResponseClient(
}

Mono<Request> createDefaultedRequest(String clientRegistrationId,
Authentication authentication, ServerWebExchange exchange) {
Authentication authentication, ServerWebExchange exchange) {
Mono<Authentication> defaultedAuthentication = Mono.justOrEmpty(authentication)
.switchIfEmpty(currentAuthentication());

Expand All @@ -124,14 +124,14 @@ Mono<OAuth2AuthorizedClient> loadAuthorizedClient(Request request) {

private Mono<OAuth2AuthorizedClient> authorizedClientNotLoaded(String clientRegistrationId, Authentication authentication, ServerWebExchange exchange) {
return this.clientRegistrationRepository.findByRegistrationId(clientRegistrationId)
.switchIfEmpty(Mono.error(() -> new IllegalArgumentException("Client Registration with id " + clientRegistrationId + " was not found")))
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any code changes here - just formatting changes. Am I missing something?

.flatMap(clientRegistration -> {
if (AuthorizationGrantType.CLIENT_CREDENTIALS.equals(clientRegistration.getAuthorizationGrantType())) {
return clientCredentials(clientRegistration, authentication, exchange);
}
return Mono.error(() -> new ClientAuthorizationRequiredException(clientRegistrationId));
});
}
.switchIfEmpty(Mono.error(() -> new IllegalArgumentException("Client Registration with id " + clientRegistrationId + " was not found")))
.flatMap(clientRegistration -> {
if (AuthorizationGrantType.CLIENT_CREDENTIALS.equals(clientRegistration.getAuthorizationGrantType())) {
return clientCredentials(clientRegistration, authentication, exchange);
}
return Mono.error(() -> new ClientAuthorizationRequiredException(clientRegistrationId));
});
}

Mono<OAuth2AuthorizedClient> clientCredentials(
ClientRegistration clientRegistration, Authentication authentication, ServerWebExchange exchange) {
Expand Down Expand Up @@ -176,7 +176,7 @@ static class Request {
private final ServerWebExchange exchange;

public Request(String clientRegistrationId, Authentication authentication,
ServerWebExchange exchange) {
ServerWebExchange exchange) {
this.clientRegistrationId = clientRegistrationId;
this.authentication = authentication;
this.exchange = exchange;
Expand Down
20 changes: 10 additions & 10 deletions ...y/oauth2/client/web/reactive/result/method/annotation/OAuth2AuthorizedClientResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public void setClientCredentialsTokenResponseClient(
}

Mono<Request> createDefaultedRequest(String clientRegistrationId,
Authentication authentication, ServerWebExchange exchange) {
Authentication authentication, ServerWebExchange exchange) {
Mono<Authentication> defaultedAuthentication = Mono.justOrEmpty(authentication)
.switchIfEmpty(currentAuthentication());

Expand All @@ -125,14 +125,14 @@ Mono<OAuth2AuthorizedClient> loadAuthorizedClient(Request request) {

private Mono<OAuth2AuthorizedClient> authorizedClientNotLoaded(String clientRegistrationId, Authentication authentication, ServerWebExchange exchange) {
return this.clientRegistrationRepository.findByRegistrationId(clientRegistrationId)
.switchIfEmpty(Mono.error(() -> new IllegalArgumentException("Client Registration with id " + clientRegistrationId + " was not found")))
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any code changes here - just formatting changes. Am I missing something?

.flatMap(clientRegistration -> {
if (AuthorizationGrantType.CLIENT_CREDENTIALS.equals(clientRegistration.getAuthorizationGrantType())) {
return clientCredentials(clientRegistration, authentication, exchange);
}
return Mono.error(() -> new ClientAuthorizationRequiredException(clientRegistrationId));
});
}
.switchIfEmpty(Mono.error(() -> new IllegalArgumentException("Client Registration with id " + clientRegistrationId + " was not found")))
.flatMap(clientRegistration -> {
if (AuthorizationGrantType.CLIENT_CREDENTIALS.equals(clientRegistration.getAuthorizationGrantType())) {
return clientCredentials(clientRegistration, authentication, exchange);
}
return Mono.error(() -> new ClientAuthorizationRequiredException(clientRegistrationId));
});
}

private Mono<? extends OAuth2AuthorizedClient> clientCredentials(
ClientRegistration clientRegistration, Authentication authentication, ServerWebExchange exchange) {
Expand Down Expand Up @@ -177,7 +177,7 @@ static class Request {
private final ServerWebExchange exchange;

public Request(String clientRegistrationId, Authentication authentication,
ServerWebExchange exchange) {
ServerWebExchange exchange) {
this.clientRegistrationId = clientRegistrationId;
this.authentication = authentication;
this.exchange = exchange;
Expand Down
4 changes: 1 addition & 3 deletions ...rity/oauth2/client/web/method/annotation/OAuth2AuthorizedClientArgumentResolverTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@

import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
import static org.mockito.ArgumentMatchers.any;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be included since there are no code changes.

import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;

/**
Expand Down