diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/CodeVerifierAuthenticator.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/CodeVerifierAuthenticator.java index d3d9f3db3..09c3f253b 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/CodeVerifierAuthenticator.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/CodeVerifierAuthenticator.java @@ -1,18 +1,3 @@ -/* - * Copyright 2020-2024 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ package org.springframework.security.oauth2.server.authorization.authentication; import java.nio.charset.StandardCharsets; @@ -86,48 +71,53 @@ private boolean authenticate(OAuth2ClientAuthenticationToken clientAuthenticatio throwInvalidGrant(OAuth2ParameterNames.CODE); } - if (this.logger.isTraceEnabled()) { - this.logger.trace("Retrieved authorization with authorization code"); + if (logger.isTraceEnabled()) { + logger.trace("Retrieved authorization with authorization code"); } - OAuth2AuthorizationRequest authorizationRequest = authorization.getAttribute( + assert authorization != null; + OAuth2AuthorizationRequest authorizationRequest = authorization.getAttribute( OAuth2AuthorizationRequest.class.getName()); + if (authorizationRequest == null) { + throwInvalidGrant("Invalid authorization request"); + } - String codeChallenge = (String) authorizationRequest.getAdditionalParameters() + assert authorizationRequest != null; + String codeChallenge = (String) authorizationRequest.getAdditionalParameters() .get(PkceParameterNames.CODE_CHALLENGE); String codeVerifier = (String) parameters.get(PkceParameterNames.CODE_VERIFIER); if (!StringUtils.hasText(codeChallenge)) { if (registeredClient.getClientSettings().isRequireProofKey() || StringUtils.hasText(codeVerifier)) { - if (this.logger.isDebugEnabled()) { - this.logger.debug(LogMessage.format("Invalid request: code_challenge is required" + + if (logger.isDebugEnabled()) { + logger.debug(LogMessage.format("Invalid request: code_challenge is required" + " for registered client '%s'", registeredClient.getId())); } throwInvalidGrant(PkceParameterNames.CODE_CHALLENGE); } else { - if (this.logger.isTraceEnabled()) { - this.logger.trace("Did not authenticate code verifier since requireProofKey=false"); + if (logger.isTraceEnabled()) { + logger.trace("Did not authenticate code verifier since requireProofKey=false"); } return false; } } - if (this.logger.isTraceEnabled()) { - this.logger.trace("Validated code verifier parameters"); + if (logger.isTraceEnabled()) { + logger.trace("Validated code verifier parameters"); } String codeChallengeMethod = (String) authorizationRequest.getAdditionalParameters() .get(PkceParameterNames.CODE_CHALLENGE_METHOD); if (!codeVerifierValid(codeVerifier, codeChallenge, codeChallengeMethod)) { - if (this.logger.isDebugEnabled()) { - this.logger.debug(LogMessage.format("Invalid request: code_verifier is missing or invalid" + + if (logger.isDebugEnabled()) { + logger.debug(LogMessage.format("Invalid request: code_verifier is missing or invalid" + " for registered client '%s'", registeredClient.getId())); } throwInvalidGrant(PkceParameterNames.CODE_VERIFIER); } - if (this.logger.isTraceEnabled()) { - this.logger.trace("Authenticated code verifier"); + if (logger.isTraceEnabled()) { + logger.trace("Authenticated code verifier"); } return true; @@ -149,9 +139,9 @@ private boolean codeVerifierValid(String codeVerifier, String codeChallenge, Str String encodedVerifier = Base64.getUrlEncoder().withoutPadding().encodeToString(digest); return encodedVerifier.equals(codeChallenge); } catch (NoSuchAlgorithmException ex) { - // It is unlikely that SHA-256 is not available on the server. If it is not available, - // there will likely be bigger issues as well. We default to SERVER_ERROR. - throw new OAuth2AuthenticationException(OAuth2ErrorCodes.SERVER_ERROR); + throw new OAuth2AuthenticationException( + new OAuth2Error(OAuth2ErrorCodes.SERVER_ERROR, + "Failed to verify code verifier: SHA-256 algorithm not available", null)); } } return false; @@ -165,5 +155,4 @@ private static void throwInvalidGrant(String parameterName) { ); throw new OAuth2AuthenticationException(error); } - } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationToken.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationToken.java index 561d79018..e81fa098a 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationToken.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2ClientAuthenticationToken.java @@ -129,4 +129,21 @@ public Map getAdditionalParameters() { return this.additionalParameters; } + /** + * Indicates if the client secret is needed for client authentication. + * Checks if the authentication method is either 'client_secret_basic' or 'client_secret_post', + * both requiring the client secret for authentication. + * @return {@code true} if the client secret is required, {@code false} otherwise + */ + public boolean isClientSecretRequired() { + return getClientAuthenticationMethod() == ClientAuthenticationMethod.CLIENT_SECRET_BASIC || + getClientAuthenticationMethod() == ClientAuthenticationMethod.CLIENT_SECRET_POST;} + + /** + * Returns the client secret. + * + * @return the client secret + */ + public Object getClientSecret() { + return getCredentials();} } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationServerConfigurer.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationServerConfigurer.java index 1b04ac158..f9c66fd12 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationServerConfigurer.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationServerConfigurer.java @@ -16,6 +16,7 @@ package org.springframework.security.oauth2.server.authorization.config.annotation.web.configurers; import java.net.URI; +import java.net.URISyntaxException; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; @@ -250,7 +251,7 @@ public OAuth2AuthorizationServerConfigurer deviceVerificationEndpoint(Customizer public OAuth2AuthorizationServerConfigurer oidc(Customizer oidcCustomizer) { OidcConfigurer oidcConfigurer = getConfigurer(OidcConfigurer.class); if (oidcConfigurer == null) { - addConfigurer(OidcConfigurer.class, new OidcConfigurer(this::postProcess)); + addConfigurer(new OidcConfigurer(this::postProcess)); oidcConfigurer = getConfigurer(OidcConfigurer.class); } oidcCustomizer.customize(oidcConfigurer); @@ -369,8 +370,8 @@ private T getConfigurer(Class type) { return (T) this.configurers.get(type); } - private void addConfigurer(Class configurerType, T configurer) { - this.configurers.put(configurerType, configurer); + private void addConfigurer(T configurer) { + this.configurers.put(OidcConfigurer.class, configurer); } private RequestMatcher getRequestMatcher(Class configurerType) { @@ -378,19 +379,19 @@ private RequestMatcher getRequestMatcher(Cl return configurer != null ? configurer.getRequestMatcher() : null; } + private static void validateAuthorizationServerSettings(AuthorizationServerSettings authorizationServerSettings) { if (authorizationServerSettings.getIssuer() != null) { URI issuerUri; try { issuerUri = new URI(authorizationServerSettings.getIssuer()); - issuerUri.toURL(); - } catch (Exception ex) { + // rfc8414 https://datatracker.ietf.org/doc/html/rfc8414#section-2 + if (issuerUri.getRawQuery() != null || issuerUri.getRawFragment() != null) { + throw new IllegalArgumentException("issuer URL cannot contain query or fragment component"); + } + } catch (URISyntaxException ex) { throw new IllegalArgumentException("issuer must be a valid URL", ex); } - // rfc8414 https://datatracker.ietf.org/doc/html/rfc8414#section-2 - if (issuerUri.getQuery() != null || issuerUri.getFragment() != null) { - throw new IllegalArgumentException("issuer cannot contain query or fragment component"); - } } } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2ClientAuthenticationFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2ClientAuthenticationFilter.java index 6926314d8..b6e88fa02 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2ClientAuthenticationFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2ClientAuthenticationFilter.java @@ -119,6 +119,7 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse } if (authenticationRequest != null) { validateClientIdentifier(authenticationRequest); + validateClientAuthenticationMethod(authenticationRequest); Authentication authenticationResult = this.authenticationManager.authenticate(authenticationRequest); this.authenticationSuccessHandler.onAuthenticationSuccess(request, response, authenticationResult); } @@ -222,5 +223,13 @@ private static void validateClientIdentifier(Authentication authentication) { } } } + private static void validateClientAuthenticationMethod(Authentication authentication) { + if (!(authentication instanceof OAuth2ClientAuthenticationToken clientAuthentication)) { + return; + } + if (clientAuthentication.isClientSecretRequired() && clientAuthentication.getClientSecret() == null) { + throw new OAuth2AuthenticationException(new OAuth2Error(OAuth2ErrorCodes.INVALID_REQUEST)); + } + } }