From e3c6effeea6b9e2dbd0218ea69a0a7e390a2b867 Mon Sep 17 00:00:00 2001 From: Joe Grandja <10884212+jgrandja@users.noreply.github.com> Date: Wed, 12 Jun 2024 05:40:51 -0400 Subject: [PATCH] X509 client certificate authentication triggers when client id is provided Closes gh-1635 --- .../OAuth2ClientAuthenticationConfigurer.java | 2 +- .../web/OAuth2ClientAuthenticationFilter.java | 4 +-- ...entCertificateAuthenticationConverter.java | 6 ++++- .../OAuth2ClientCredentialsGrantTests.java | 26 +++++++++++++++++-- ...rtificateAuthenticationConverterTests.java | 8 +++--- 5 files changed, 35 insertions(+), 11 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2ClientAuthenticationConfigurer.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2ClientAuthenticationConfigurer.java index 65ffdfc6..e63fe8f6 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2ClientAuthenticationConfigurer.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2ClientAuthenticationConfigurer.java @@ -240,10 +240,10 @@ public final class OAuth2ClientAuthenticationConfigurer extends AbstractOAuth2Co List authenticationConverters = new ArrayList<>(); authenticationConverters.add(new JwtClientAssertionAuthenticationConverter()); - authenticationConverters.add(new X509ClientCertificateAuthenticationConverter()); authenticationConverters.add(new ClientSecretBasicAuthenticationConverter()); authenticationConverters.add(new ClientSecretPostAuthenticationConverter()); authenticationConverters.add(new PublicClientAuthenticationConverter()); + authenticationConverters.add(new X509ClientCertificateAuthenticationConverter()); return authenticationConverters; } 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 987914e3..e9ba1f32 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 @@ -114,10 +114,10 @@ public final class OAuth2ClientAuthenticationFilter extends OncePerRequestFilter this.authenticationConverter = new DelegatingAuthenticationConverter( Arrays.asList( new JwtClientAssertionAuthenticationConverter(), - new X509ClientCertificateAuthenticationConverter(), new ClientSecretBasicAuthenticationConverter(), new ClientSecretPostAuthenticationConverter(), - new PublicClientAuthenticationConverter())); + new PublicClientAuthenticationConverter(), + new X509ClientCertificateAuthenticationConverter())); // @formatter:on } diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/X509ClientCertificateAuthenticationConverter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/X509ClientCertificateAuthenticationConverter.java index dd68b7a4..0c71f961 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/X509ClientCertificateAuthenticationConverter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/authentication/X509ClientCertificateAuthenticationConverter.java @@ -59,7 +59,11 @@ public final class X509ClientCertificateAuthenticationConverter implements Authe // client_id (REQUIRED) String clientId = parameters.getFirst(OAuth2ParameterNames.CLIENT_ID); - if (!StringUtils.hasText(clientId) || parameters.get(OAuth2ParameterNames.CLIENT_ID).size() != 1) { + if (!StringUtils.hasText(clientId)) { + return null; + } + + if (parameters.get(OAuth2ParameterNames.CLIENT_ID).size() != 1) { throw new OAuth2AuthenticationException(OAuth2ErrorCodes.INVALID_REQUEST); } diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2ClientCredentialsGrantTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2ClientCredentialsGrantTests.java index 03a7e550..03422865 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2ClientCredentialsGrantTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2ClientCredentialsGrantTests.java @@ -309,6 +309,28 @@ public class OAuth2ClientCredentialsGrantTests { verify(jwtCustomizer).customize(any()); } + // gh-1635 + @Test + public void requestWhenTokenRequestIncludesBasicClientCredentialsAndX509ClientCertificateThenTokenResponse() + throws Exception { + this.spring.register(AuthorizationServerConfiguration.class).autowire(); + + RegisteredClient registeredClient = TestRegisteredClients.registeredClient2().build(); + this.registeredClientRepository.save(registeredClient); + + this.mvc + .perform(post(DEFAULT_TOKEN_ENDPOINT_URI).with(x509(TestX509Certificates.DEMO_CLIENT_PKI_CERTIFICATE)) + .param(OAuth2ParameterNames.GRANT_TYPE, AuthorizationGrantType.CLIENT_CREDENTIALS.getValue()) + .param(OAuth2ParameterNames.SCOPE, "scope1 scope2") + .header(HttpHeaders.AUTHORIZATION, + "Basic " + encodeBasicAuth(registeredClient.getClientId(), registeredClient.getClientSecret()))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.access_token").isNotEmpty()) + .andExpect(jsonPath("$.scope").value("scope1 scope2")); + + verify(jwtCustomizer).customize(any()); + } + @Test public void requestWhenTokenEndpointCustomizedThenUsed() throws Exception { this.spring.register(AuthorizationServerConfigurationCustomTokenEndpoint.class).autowire(); @@ -394,10 +416,10 @@ public class OAuth2ClientCredentialsGrantTests { List authenticationConverters = authenticationConvertersCaptor.getValue(); assertThat(authenticationConverters).allMatch((converter) -> converter == authenticationConverter || converter instanceof JwtClientAssertionAuthenticationConverter - || converter instanceof X509ClientCertificateAuthenticationConverter || converter instanceof ClientSecretBasicAuthenticationConverter || converter instanceof ClientSecretPostAuthenticationConverter - || converter instanceof PublicClientAuthenticationConverter); + || converter instanceof PublicClientAuthenticationConverter + || converter instanceof X509ClientCertificateAuthenticationConverter); verify(authenticationProvider).authenticate(eq(clientPrincipal)); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/X509ClientCertificateAuthenticationConverterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/X509ClientCertificateAuthenticationConverterTests.java index fcd44888..cc2600e5 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/X509ClientCertificateAuthenticationConverterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/authentication/X509ClientCertificateAuthenticationConverterTests.java @@ -58,14 +58,12 @@ public class X509ClientCertificateAuthenticationConverterTests { } @Test - public void convertWhenMissingClientIdThenInvalidRequestError() { + public void convertWhenMissingClientIdThenReturnNull() { MockHttpServletRequest request = new MockHttpServletRequest(); request.setAttribute("jakarta.servlet.request.X509Certificate", TestX509Certificates.DEMO_CLIENT_PKI_CERTIFICATE); - assertThatThrownBy(() -> this.converter.convert(request)).isInstanceOf(OAuth2AuthenticationException.class) - .extracting((ex) -> ((OAuth2AuthenticationException) ex).getError()) - .extracting("errorCode") - .isEqualTo(OAuth2ErrorCodes.INVALID_REQUEST); + Authentication authentication = this.converter.convert(request); + assertThat(authentication).isNull(); } @Test