diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticationProvider.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticationProvider.java index a0b12a0fcf..6e7b8c91b7 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticationProvider.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticationProvider.java @@ -21,6 +21,9 @@ import org.springframework.security.core.AuthenticationException; import org.springframework.security.oauth2.client.token.InMemoryAccessTokenRepository; import org.springframework.security.oauth2.client.token.SecurityTokenRepository; import org.springframework.security.oauth2.core.AccessToken; +import org.springframework.security.oauth2.core.OAuth2Error; +import org.springframework.security.oauth2.core.endpoint.AuthorizationRequest; +import org.springframework.security.oauth2.core.endpoint.AuthorizationResponse; import org.springframework.security.oauth2.oidc.client.authentication.OidcClientAuthenticationToken; import org.springframework.util.Assert; @@ -32,7 +35,7 @@ import org.springframework.util.Assert; * *

* The {@link AuthorizationCodeAuthenticationProvider} uses an {@link AuthorizationGrantAuthenticator} - * to authenticate the {@link AuthorizationCodeAuthenticationToken#getAuthorizationCode()} and ultimately + * to authenticate the authorization code credential and ultimately * return an "Authorized Client" as an {@link OAuth2ClientAuthenticationToken}. * * @author Joe Grandja @@ -49,6 +52,8 @@ import org.springframework.util.Assert; * @see Section 3.1.3.3 OpenID Connect Token Response */ public class AuthorizationCodeAuthenticationProvider implements AuthenticationProvider { + private static final String INVALID_STATE_PARAMETER_ERROR_CODE = "invalid_state_parameter"; + private static final String INVALID_REDIRECT_URI_PARAMETER_ERROR_CODE = "invalid_redirect_uri_parameter"; private final AuthorizationGrantAuthenticator authorizationCodeAuthenticator; private SecurityTokenRepository accessTokenRepository = new InMemoryAccessTokenRepository(); @@ -64,6 +69,24 @@ public class AuthorizationCodeAuthenticationProvider implements AuthenticationPr AuthorizationCodeAuthenticationToken authorizationCodeAuthentication = (AuthorizationCodeAuthenticationToken) authentication; + AuthorizationRequest authorizationRequest = authorizationCodeAuthentication.getAuthorizationRequest(); + AuthorizationResponse authorizationResponse = authorizationCodeAuthentication.getAuthorizationResponse(); + + if (authorizationResponse.statusError()) { + throw new OAuth2AuthenticationException( + authorizationResponse.getError(), authorizationResponse.getError().toString()); + } + + if (!authorizationResponse.getState().equals(authorizationRequest.getState())) { + OAuth2Error oauth2Error = new OAuth2Error(INVALID_STATE_PARAMETER_ERROR_CODE); + throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString()); + } + + if (!authorizationResponse.getRedirectUri().equals(authorizationRequest.getRedirectUri())) { + OAuth2Error oauth2Error = new OAuth2Error(INVALID_REDIRECT_URI_PARAMETER_ERROR_CODE); + throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString()); + } + OAuth2ClientAuthenticationToken oauth2ClientAuthentication = this.authorizationCodeAuthenticator.authenticate(authorizationCodeAuthentication); diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticationToken.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticationToken.java index 096c4d4a6c..e3adcfecfa 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticationToken.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticationToken.java @@ -18,6 +18,7 @@ package org.springframework.security.oauth2.client.authentication; import org.springframework.security.oauth2.client.registration.ClientRegistration; import org.springframework.security.oauth2.core.AuthorizationGrantType; import org.springframework.security.oauth2.core.endpoint.AuthorizationRequest; +import org.springframework.security.oauth2.core.endpoint.AuthorizationResponse; import org.springframework.util.Assert; /** @@ -28,38 +29,37 @@ import org.springframework.util.Assert; * @since 5.0 * @see AuthorizationGrantAuthenticationToken * @see ClientRegistration + * @see AuthorizationRequest + * @see AuthorizationResponse * @see Section 1.3.1 Authorization Code Grant */ public class AuthorizationCodeAuthenticationToken extends AuthorizationGrantAuthenticationToken { - private final String authorizationCode; private final ClientRegistration clientRegistration; private final AuthorizationRequest authorizationRequest; + private final AuthorizationResponse authorizationResponse; + + public AuthorizationCodeAuthenticationToken(ClientRegistration clientRegistration, + AuthorizationRequest authorizationRequest, + AuthorizationResponse authorizationResponse) { - public AuthorizationCodeAuthenticationToken(String authorizationCode, - ClientRegistration clientRegistration, - AuthorizationRequest authorizationRequest) { super(AuthorizationGrantType.AUTHORIZATION_CODE); - Assert.hasText(authorizationCode, "authorizationCode cannot be empty"); Assert.notNull(clientRegistration, "clientRegistration cannot be null"); Assert.notNull(authorizationRequest, "authorizationRequest cannot be null"); - this.authorizationCode = authorizationCode; + Assert.notNull(authorizationResponse, "authorizationResponse cannot be null"); this.clientRegistration = clientRegistration; this.authorizationRequest = authorizationRequest; + this.authorizationResponse = authorizationResponse; this.setAuthenticated(false); } @Override public Object getPrincipal() { - return this.getClientRegistration().getClientId(); + return ""; } @Override public Object getCredentials() { - return this.getAuthorizationCode(); - } - - public String getAuthorizationCode() { - return this.authorizationCode; + return ""; } public ClientRegistration getClientRegistration() { @@ -69,4 +69,8 @@ public class AuthorizationCodeAuthenticationToken extends AuthorizationGrantAuth public AuthorizationRequest getAuthorizationRequest() { return this.authorizationRequest; } + + public AuthorizationResponse getAuthorizationResponse() { + return this.authorizationResponse; + } } diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilter.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilter.java index 128befb59c..62f6059a07 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilter.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilter.java @@ -82,8 +82,6 @@ import java.io.IOException; public class AuthorizationCodeAuthenticationFilter extends AbstractAuthenticationProcessingFilter { public static final String DEFAULT_AUTHORIZATION_RESPONSE_BASE_URI = "/oauth2/authorize/code"; private static final String AUTHORIZATION_REQUEST_NOT_FOUND_ERROR_CODE = "authorization_request_not_found"; - private static final String INVALID_STATE_PARAMETER_ERROR_CODE = "invalid_state_parameter"; - private static final String INVALID_REDIRECT_URI_PARAMETER_ERROR_CODE = "invalid_redirect_uri_parameter"; private final AuthorizationResponseConverter authorizationResponseConverter = new AuthorizationResponseConverter(); private ClientRegistrationRepository clientRegistrationRepository; private RequestMatcher authorizationResponseMatcher = new AuthorizationResponseMatcher(); @@ -98,16 +96,16 @@ public class AuthorizationCodeAuthenticationFilter extends AbstractAuthenticatio public Authentication attemptAuthentication(HttpServletRequest request, HttpServletResponse response) throws AuthenticationException, IOException, ServletException { + AuthorizationRequest authorizationRequest = this.getAuthorizationRequestRepository().loadAuthorizationRequest(request); + if (authorizationRequest == null) { + OAuth2Error oauth2Error = new OAuth2Error(AUTHORIZATION_REQUEST_NOT_FOUND_ERROR_CODE); + throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString()); + } + this.getAuthorizationRequestRepository().removeAuthorizationRequest(request); + AuthorizationResponse authorizationResponse = this.authorizationResponseConverter.apply(request); - if (authorizationResponse.statusError()) { - this.getAuthorizationRequestRepository().removeAuthorizationRequest(request); - throw new OAuth2AuthenticationException( - authorizationResponse.getError(), authorizationResponse.getError().toString()); - } - - AuthorizationRequest matchingAuthorizationRequest = this.resolveAuthorizationRequest(request); - String registrationId = (String)matchingAuthorizationRequest.getAdditionalParameters().get(OAuth2Parameter.REGISTRATION_ID); + String registrationId = (String)authorizationRequest.getAdditionalParameters().get(OAuth2Parameter.REGISTRATION_ID); ClientRegistration clientRegistration = this.getClientRegistrationRepository().findByRegistrationId(registrationId); // The clientRegistration.redirectUri may contain Uri template variables, whether it's configured by @@ -116,13 +114,13 @@ public class AuthorizationCodeAuthenticationFilter extends AbstractAuthenticatio // The resulting redirectUri used for the authorization request and saved within the AuthorizationRequestRepository // MUST BE the same one used to complete the authorization code flow. // Therefore, we'll create a copy of the clientRegistration and override the redirectUri - // with the one contained in matchingAuthorizationRequest. + // with the one contained in authorizationRequest. clientRegistration = new ClientRegistration.Builder(clientRegistration) - .redirectUri(matchingAuthorizationRequest.getRedirectUri()) + .redirectUri(authorizationRequest.getRedirectUri()) .build(); AuthorizationCodeAuthenticationToken authorizationCodeAuthentication = new AuthorizationCodeAuthenticationToken( - authorizationResponse.getCode(), clientRegistration, matchingAuthorizationRequest); + clientRegistration, authorizationRequest, authorizationResponse); authorizationCodeAuthentication.setDetails(this.authenticationDetailsSource.buildDetails(request)); OAuth2ClientAuthenticationToken oauth2ClientAuthentication = @@ -172,31 +170,6 @@ public class AuthorizationCodeAuthenticationFilter extends AbstractAuthenticatio this.authorizationRequestRepository = authorizationRequestRepository; } - private AuthorizationRequest resolveAuthorizationRequest(HttpServletRequest request) { - AuthorizationRequest authorizationRequest = - this.getAuthorizationRequestRepository().loadAuthorizationRequest(request); - if (authorizationRequest == null) { - OAuth2Error oauth2Error = new OAuth2Error(AUTHORIZATION_REQUEST_NOT_FOUND_ERROR_CODE); - throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString()); - } - this.getAuthorizationRequestRepository().removeAuthorizationRequest(request); - this.assertMatchingAuthorizationRequest(request, authorizationRequest); - return authorizationRequest; - } - - private void assertMatchingAuthorizationRequest(HttpServletRequest request, AuthorizationRequest authorizationRequest) { - String state = request.getParameter(OAuth2Parameter.STATE); - if (!authorizationRequest.getState().equals(state)) { - OAuth2Error oauth2Error = new OAuth2Error(INVALID_STATE_PARAMETER_ERROR_CODE); - throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString()); - } - - if (!request.getRequestURL().toString().equals(authorizationRequest.getRedirectUri())) { - OAuth2Error oauth2Error = new OAuth2Error(INVALID_REDIRECT_URI_PARAMETER_ERROR_CODE); - throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString()); - } - } - private boolean authenticated() { Authentication currentAuthentication = SecurityContextHolder.getContext().getAuthentication(); return currentAuthentication != null && diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/converter/AuthorizationResponseConverter.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/converter/AuthorizationResponseConverter.java index 68ebac7c92..8205c72107 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/converter/AuthorizationResponseConverter.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/converter/AuthorizationResponseConverter.java @@ -38,15 +38,18 @@ public final class AuthorizationResponseConverter implements FunctionSection 4.1.2 Authorization Response */ public final class AuthorizationResponse { - private String code; + private String redirectUri; private String state; + private String code; private OAuth2Error error; private AuthorizationResponse() { } - public String getCode() { - return this.code; + public String getRedirectUri() { + return this.redirectUri; } public String getState() { return this.state; } + public String getCode() { + return this.code; + } + public OAuth2Error getError() { return this.error; } @@ -65,8 +70,9 @@ public final class AuthorizationResponse { } public static class Builder { - private String code; + private String redirectUri; private String state; + private String code; private String errorCode; private String errorDescription; private String errorUri; @@ -74,8 +80,8 @@ public final class AuthorizationResponse { private Builder() { } - public Builder code(String code) { - this.code = code; + public Builder redirectUri(String redirectUri) { + this.redirectUri = redirectUri; return this; } @@ -84,6 +90,11 @@ public final class AuthorizationResponse { return this; } + public Builder code(String code) { + this.code = code; + return this; + } + public Builder errorCode(String errorCode) { this.errorCode = errorCode; return this; @@ -103,14 +114,17 @@ public final class AuthorizationResponse { if (StringUtils.hasText(this.code) && StringUtils.hasText(this.errorCode)) { throw new IllegalArgumentException("code and errorCode cannot both be set"); } + Assert.hasText(this.redirectUri, "redirectUri cannot be empty"); + AuthorizationResponse authorizationResponse = new AuthorizationResponse(); + authorizationResponse.redirectUri = this.redirectUri; + authorizationResponse.state = this.state; if (StringUtils.hasText(this.code)) { authorizationResponse.code = this.code; } else { authorizationResponse.error = new OAuth2Error( this.errorCode, this.errorDescription, this.errorUri); } - authorizationResponse.state = this.state; return authorizationResponse; } } diff --git a/samples/boot/oauth2login/src/integration-test/java/org/springframework/security/samples/OAuth2LoginApplicationTests.java b/samples/boot/oauth2login/src/integration-test/java/org/springframework/security/samples/OAuth2LoginApplicationTests.java index 8bbc5428f2..05e4f66140 100644 --- a/samples/boot/oauth2login/src/integration-test/java/org/springframework/security/samples/OAuth2LoginApplicationTests.java +++ b/samples/boot/oauth2login/src/integration-test/java/org/springframework/security/samples/OAuth2LoginApplicationTests.java @@ -44,7 +44,6 @@ import org.springframework.security.oauth2.client.web.AuthorizationCodeAuthentic import org.springframework.security.oauth2.client.web.AuthorizationCodeRequestRedirectFilter; import org.springframework.security.oauth2.client.web.AuthorizationGrantTokenExchanger; import org.springframework.security.oauth2.core.AccessToken; -import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.endpoint.OAuth2Parameter; import org.springframework.security.oauth2.core.endpoint.ResponseType; import org.springframework.security.oauth2.core.endpoint.TokenResponse; @@ -282,30 +281,6 @@ public class OAuth2LoginApplicationTests { assertThat(errorElement.asText()).contains("invalid_redirect_uri_parameter"); } - @Test - public void requestAuthorizationCodeGrantWhenStandardErrorCodeResponseThenDisplayLoginPageWithError() throws Exception { - HtmlPage page = this.webClient.getPage("/"); - URL loginPageUrl = page.getBaseURL(); - URL loginErrorPageUrl = new URL(loginPageUrl.toString() + "?error"); - - String error = OAuth2Error.INVALID_CLIENT_ERROR_CODE; - String state = "state"; - String redirectUri = AUTHORIZE_BASE_URL + "/" + this.githubClientRegistration.getRegistrationId(); - - String authorizationResponseUri = - UriComponentsBuilder.fromHttpUrl(redirectUri) - .queryParam(OAuth2Parameter.ERROR, error) - .queryParam(OAuth2Parameter.STATE, state) - .build().encode().toUriString(); - - page = this.webClient.getPage(new URL(authorizationResponseUri)); - assertThat(page.getBaseURL()).isEqualTo(loginErrorPageUrl); - - HtmlElement errorElement = page.getBody().getFirstByXPath("p"); - assertThat(errorElement).isNotNull(); - assertThat(errorElement.asText()).contains(error); - } - private void assertLoginPage(HtmlPage page) throws Exception { assertThat(page.getTitleText()).isEqualTo("Login Page");