From 5228576d07453003d54049ff4c94b5efb0a4d22a Mon Sep 17 00:00:00 2001 From: wheleph Date: Thu, 31 Jul 2025 21:16:51 +0300 Subject: [PATCH 1/2] Pass client_secret_expires_at Closes gh-2111 Signed-off-by: wheleph --- ...ClientOidcClientRegistrationConverter.java | 4 + .../OidcClientRegistrationTests.java | 116 +++++++++++++++++- 2 files changed, 116 insertions(+), 4 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/converter/RegisteredClientOidcClientRegistrationConverter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/converter/RegisteredClientOidcClientRegistrationConverter.java index 84575aa80..1c1c447b8 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/converter/RegisteredClientOidcClientRegistrationConverter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/converter/RegisteredClientOidcClientRegistrationConverter.java @@ -49,6 +49,10 @@ public OidcClientRegistration convert(RegisteredClient registeredClient) { builder.clientSecret(registeredClient.getClientSecret()); } + if (registeredClient.getClientSecretExpiresAt() != null) { + builder.clientSecretExpiresAt(registeredClient.getClientSecretExpiresAt()); + } + builder.redirectUris((redirectUris) -> redirectUris.addAll(registeredClient.getRedirectUris())); diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java index a2fc98fa7..a2a329c97 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java @@ -15,6 +15,7 @@ */ package org.springframework.security.oauth2.server.authorization.config.annotation.web.configurers; +import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Collections; @@ -31,6 +32,7 @@ import jakarta.servlet.http.HttpServletResponse; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; +import org.assertj.core.data.TemporalUnitWithinOffset; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; @@ -508,6 +510,46 @@ public void requestWhenClientRegistersWithCustomMetadataThenSavedToRegisteredCli assertThat(registeredClient.getClientSettings().getSetting("non-registered-custom-metadata")).isNull(); } + /** + * Scenario to validate that if there's a customization that sets client secret expiration date, then the date + * is persisted and returned in the registration response + */ + @Test + public void requestWhenClientRegistersWithSecretExpirationThenClientRegistrationResponse() throws Exception { + this.spring.register(ClientSecretExpirationConfiguration.class).autowire(); + + // @formatter:off + OidcClientRegistration clientRegistration = OidcClientRegistration.builder() + .clientName("client-name") + .redirectUri("https://client.example.com") + .grantType(AuthorizationGrantType.AUTHORIZATION_CODE.getValue()) + .grantType(AuthorizationGrantType.CLIENT_CREDENTIALS.getValue()) + .scope("scope1") + .scope("scope2") + .build(); + // @formatter:on + + OidcClientRegistration clientRegistrationResponse = registerClient(clientRegistration); + + var expectedSecretExpiryDate = Instant.now().plus(Duration.ofHours(24)); + var allowedDelta = new TemporalUnitWithinOffset(1, ChronoUnit.MINUTES); + + // Returned response contains expiration date + assertThat(clientRegistrationResponse.getClientSecretExpiresAt()) + .isNotNull() + .isCloseTo(expectedSecretExpiryDate, allowedDelta); + + RegisteredClient registeredClient = this.registeredClientRepository + .findByClientId(clientRegistrationResponse.getClientId()); + + // Persisted RegisteredClient contains expiration date + assertThat(registeredClient) + .isNotNull(); + assertThat(registeredClient.getClientSecretExpiresAt()) + .isNotNull() + .isCloseTo(expectedSecretExpiryDate, allowedDelta); + } + private OidcClientRegistration registerClient(OidcClientRegistration clientRegistration) throws Exception { // ***** (1) Obtain the "initial" access token used for registering the client @@ -643,8 +685,40 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h @EnableWebSecurity @Configuration(proxyBeanMethods = false) - static class CustomClientMetadataConfiguration extends AuthorizationServerConfiguration { + static class CustomClientMetadataConfiguration extends ClientRegistrationConvertersConfiguration { + + private static final List supportedCustomClientMetadata = List.of("custom-metadata-name-1", "custom-metadata-name-2"); + + @Override + protected Converter registeredClientConverter() { + return new CustomRegisteredClientConverter(supportedCustomClientMetadata); + } + + @Override + protected Converter oidcClientRegistrationConverter() { + return new CustomClientRegistrationConverter(supportedCustomClientMetadata); + } + + } + + @EnableWebSecurity + @Configuration(proxyBeanMethods = false) + static class ClientSecretExpirationConfiguration extends ClientRegistrationConvertersConfiguration { + + @Override + protected Converter registeredClientConverter() { + return new ClientSecretExpirationRegisteredClientConverter(); + } + + } + /** + * This test configuration allows to override {@code RegisteredClient} -> {@code OidcClientRegistration} and + * {@code OidcClientRegistration} -> {@code RegisteredClient} converters + */ + @EnableWebSecurity + @Configuration(proxyBeanMethods = false) + static class ClientRegistrationConvertersConfiguration extends AuthorizationServerConfiguration { // @formatter:off @Bean @Override @@ -674,15 +748,27 @@ private Consumer> configureClientRegistrationConver // @formatter:off return (authenticationProviders) -> authenticationProviders.forEach((authenticationProvider) -> { - List supportedCustomClientMetadata = List.of("custom-metadata-name-1", "custom-metadata-name-2"); if (authenticationProvider instanceof OidcClientRegistrationAuthenticationProvider provider) { - provider.setRegisteredClientConverter(new CustomRegisteredClientConverter(supportedCustomClientMetadata)); - provider.setClientRegistrationConverter(new CustomClientRegistrationConverter(supportedCustomClientMetadata)); + var registeredClientConverter = registeredClientConverter(); + if (registeredClientConverter != null) { + provider.setRegisteredClientConverter(registeredClientConverter); + } + var oidcClientRegistrationConverter = oidcClientRegistrationConverter(); + if (oidcClientRegistrationConverter != null) { + provider.setClientRegistrationConverter(oidcClientRegistrationConverter); + } } }); // @formatter:on } + protected Converter registeredClientConverter() { + return null; + } + + protected Converter oidcClientRegistrationConverter() { + return null; + } } @EnableWebSecurity @@ -814,4 +900,26 @@ public OidcClientRegistration convert(RegisteredClient registeredClient) { } + /** + * This customization adds client secret expiration time by setting {@code RegisteredClient.clientSecretExpiresAt} + * during {@code OidcClientRegistration} -> {@code RegisteredClient} conversion + */ + private static final class ClientSecretExpirationRegisteredClientConverter + implements Converter { + + private static final OidcClientRegistrationRegisteredClientConverter delegate = + new OidcClientRegistrationRegisteredClientConverter(); + + @Override + public RegisteredClient convert(OidcClientRegistration clientRegistration) { + RegisteredClient registeredClient = delegate.convert(clientRegistration); + var registeredClientBuilder = RegisteredClient.from(registeredClient); + + var clientSecretExpiresAt = Instant.now().plus(Duration.ofHours(24)); + registeredClientBuilder.clientSecretExpiresAt(clientSecretExpiresAt); + + return registeredClientBuilder.build(); + } + } + } From a8d490522c1329eaeb0eee63826f2f26a3a084f0 Mon Sep 17 00:00:00 2001 From: wheleph Date: Sat, 16 Aug 2025 12:54:46 +0300 Subject: [PATCH 2/2] Post-review fixes Signed-off-by: wheleph --- .../OidcClientRegistrationTests.java | 79 +++++++++---------- 1 file changed, 38 insertions(+), 41 deletions(-) diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java index a2a329c97..78dbcced8 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java @@ -531,8 +531,8 @@ public void requestWhenClientRegistersWithSecretExpirationThenClientRegistration OidcClientRegistration clientRegistrationResponse = registerClient(clientRegistration); - var expectedSecretExpiryDate = Instant.now().plus(Duration.ofHours(24)); - var allowedDelta = new TemporalUnitWithinOffset(1, ChronoUnit.MINUTES); + Instant expectedSecretExpiryDate = Instant.now().plus(Duration.ofHours(24)); + TemporalUnitWithinOffset allowedDelta = new TemporalUnitWithinOffset(1, ChronoUnit.MINUTES); // Returned response contains expiration date assertThat(clientRegistrationResponse.getClientSecretExpiresAt()) @@ -685,40 +685,52 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity h @EnableWebSecurity @Configuration(proxyBeanMethods = false) - static class CustomClientMetadataConfiguration extends ClientRegistrationConvertersConfiguration { - - private static final List supportedCustomClientMetadata = List.of("custom-metadata-name-1", "custom-metadata-name-2"); + static class CustomClientMetadataConfiguration extends AuthorizationServerConfiguration { + // @formatter:off + @Bean @Override - protected Converter registeredClientConverter() { - return new CustomRegisteredClientConverter(supportedCustomClientMetadata); + public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity http) throws Exception { + OAuth2AuthorizationServerConfigurer authorizationServerConfigurer = + OAuth2AuthorizationServerConfigurer.authorizationServer(); + http + .securityMatcher(authorizationServerConfigurer.getEndpointsMatcher()) + .with(authorizationServerConfigurer, (authorizationServer) -> + authorizationServer + .oidc((oidc) -> + oidc + .clientRegistrationEndpoint((clientRegistration) -> + clientRegistration + .authenticationProviders(configureClientRegistrationConverters()) + ) + ) + ) + .authorizeHttpRequests((authorize) -> + authorize.anyRequest().authenticated() + ); + return http.build(); } + // @formatter:on - @Override - protected Converter oidcClientRegistrationConverter() { - return new CustomClientRegistrationConverter(supportedCustomClientMetadata); + private Consumer> configureClientRegistrationConverters() { + // @formatter:off + return (authenticationProviders) -> + authenticationProviders.forEach((authenticationProvider) -> { + List supportedCustomClientMetadata = List.of("custom-metadata-name-1", "custom-metadata-name-2"); + if (authenticationProvider instanceof OidcClientRegistrationAuthenticationProvider provider) { + provider.setRegisteredClientConverter(new CustomRegisteredClientConverter(supportedCustomClientMetadata)); + provider.setClientRegistrationConverter(new CustomClientRegistrationConverter(supportedCustomClientMetadata)); + } + }); + // @formatter:on } } @EnableWebSecurity @Configuration(proxyBeanMethods = false) - static class ClientSecretExpirationConfiguration extends ClientRegistrationConvertersConfiguration { - - @Override - protected Converter registeredClientConverter() { - return new ClientSecretExpirationRegisteredClientConverter(); - } + static class ClientSecretExpirationConfiguration extends AuthorizationServerConfiguration { - } - - /** - * This test configuration allows to override {@code RegisteredClient} -> {@code OidcClientRegistration} and - * {@code OidcClientRegistration} -> {@code RegisteredClient} converters - */ - @EnableWebSecurity - @Configuration(proxyBeanMethods = false) - static class ClientRegistrationConvertersConfiguration extends AuthorizationServerConfiguration { // @formatter:off @Bean @Override @@ -749,26 +761,12 @@ private Consumer> configureClientRegistrationConver return (authenticationProviders) -> authenticationProviders.forEach((authenticationProvider) -> { if (authenticationProvider instanceof OidcClientRegistrationAuthenticationProvider provider) { - var registeredClientConverter = registeredClientConverter(); - if (registeredClientConverter != null) { - provider.setRegisteredClientConverter(registeredClientConverter); - } - var oidcClientRegistrationConverter = oidcClientRegistrationConverter(); - if (oidcClientRegistrationConverter != null) { - provider.setClientRegistrationConverter(oidcClientRegistrationConverter); - } + provider.setRegisteredClientConverter(new ClientSecretExpirationRegisteredClientConverter()); } }); // @formatter:on } - protected Converter registeredClientConverter() { - return null; - } - - protected Converter oidcClientRegistrationConverter() { - return null; - } } @EnableWebSecurity @@ -921,5 +919,4 @@ public RegisteredClient convert(OidcClientRegistration clientRegistration) { return registeredClientBuilder.build(); } } - }