From 4a4da83e5d88ea31c7ce1a479526d838f7398f19 Mon Sep 17 00:00:00 2001 From: Manfred Hantschel Date: Mon, 29 Jul 2024 19:04:47 +0200 Subject: [PATCH] Upgrade to Spring Boot 3.3.2 Updated to Spring Boot 3.3.2 in order to reproduce a bug. The problem is caused by directly instantiating the DefaultSaml2CredentialsManager that contains annotated methods and post processing the instantiated manager. Currently, it is not possible to instantiate the manager in this fashion and the postProcess call was removed. As a result, it is now necessary to pass a bean as Saml2CredentialsManager. It seems to be caused by the same change, that led to https://github.com/spring-projects/spring-framework/issues/33286 and was fixed just hours ago. It's a breaking change! The two credential methods in the PartnerNetSaml2Configurer are package-private now. Replace the call by simply passing a DefaultSaml2CredentialsManager bean initialized with the configSupplier instead of using the credentials methods with the configSupplier. The methods will be restored with the Upgrade to 3.3.3, if the change in Spring Boot is fixing for our error, too. --- .../idp/saml2/PartnerNetSaml2Configurer.java | 48 ++++++++++++++----- .../ClientShowcaseSecurityConfig.java | 20 ++++++-- pom.xml | 7 +-- 3 files changed, 56 insertions(+), 19 deletions(-) diff --git a/pnet-idp-client-saml2/src/main/java/at/porscheinformatik/idp/saml2/PartnerNetSaml2Configurer.java b/pnet-idp-client-saml2/src/main/java/at/porscheinformatik/idp/saml2/PartnerNetSaml2Configurer.java index ece1e00..45b3877 100644 --- a/pnet-idp-client-saml2/src/main/java/at/porscheinformatik/idp/saml2/PartnerNetSaml2Configurer.java +++ b/pnet-idp-client-saml2/src/main/java/at/porscheinformatik/idp/saml2/PartnerNetSaml2Configurer.java @@ -74,9 +74,7 @@ public static PartnerNetSaml2Configurer apply(HttpSecurity http, String entityId throws Exception { http // - .authorizeHttpRequests() - .requestMatchers(HttpMethod.GET, DEFAULT_ENTITY_ID_PATH) - .permitAll(); + .authorizeHttpRequests().requestMatchers(HttpMethod.GET, DEFAULT_ENTITY_ID_PATH).permitAll(); return http.apply(new PartnerNetSaml2Configurer(entityId, metadataUrl)); } @@ -128,23 +126,48 @@ public PartnerNetSaml2Configurer failOnStartup() /** * @param credentialConfigs static list of credentials to use for authentication * @return the builder for a fluent api + * @deprecated Do not use this method, as it is buggy! It instantiates the DefaultSaml2CredentialsManager, that + * contains annotated methods which only work inside a Spring bean! This is not fixable. As a result, this is an + * intended breaking change. Use the {@link #credentials(Saml2CredentialsManager)} method instead and pass a + * DefaultSaml2CredentialsManager bean as manager! */ - public PartnerNetSaml2Configurer credentials(Saml2CredentialsConfig... credentialConfigs) + @Deprecated(forRemoval = true) + PartnerNetSaml2Configurer credentials(Saml2CredentialsConfig... credentialConfigs) { - return credentials(() -> Arrays.asList(credentialConfigs)); + credentialsManager = new DefaultSaml2CredentialsManager(() -> Arrays.asList(credentialConfigs)); + + return this; } /** * @param supplier the supplier that will be called periodically to load the most up to date set of credentials * @return the builder for a fluent api + * @deprecated Do not use this method, as it is buggy! It instantiates the DefaultSaml2CredentialsManager, that + * contains annotated methods which only work inside a Spring bean! This is not fixable. As a result, this is an + * intended breaking change. Use the {@link #credentials(Saml2CredentialsManager)} method instead and pass a + * DefaultSaml2CredentialsManager bean as manager! */ - public PartnerNetSaml2Configurer credentials(Supplier> supplier) + @Deprecated(forRemoval = true) + PartnerNetSaml2Configurer credentials(Supplier> supplier) { credentialsManager = new DefaultSaml2CredentialsManager(supplier); return this; } + /** + * Set the credentials manager to use for loading the credentials. + * + * @param credentialsManager the credentials manager to use + * @return the builder for a fluent api + */ + public PartnerNetSaml2Configurer credentials(Saml2CredentialsManager credentialsManager) + { + this.credentialsManager = credentialsManager; + + return this; + } + /** * Override the default client factory to be used for loading SAML metadata * @@ -335,7 +358,7 @@ private Saml2ResponseProcessor getResponseProcessor() private Saml2CredentialsManager getCredentialsManager() { - return postProcess(requireNonNull(credentialsManager, "No credentials configured")); + return requireNonNull(credentialsManager, "No credentials configured"); } private RelyingPartyRegistrationRepository getRelyingPartyRegistrationRepository( @@ -361,11 +384,12 @@ private Saml2AuthenticationRequestResolver buildRequestResolver( new OpenSaml4AuthenticationRequestResolver(relyingPartyRegistrationResolver); resolver.setAuthnRequestCustomizer(getAuthnRequestCustomizer()); - resolver - .setRelayStateResolver(request -> Saml2Utils // - .getRelayState(request) - .map(relayState -> String.format(AUTO_GENERATED_RELAY_STATE_FORMAT, UUID.randomUUID(), relayState)) // pre-append a random string - .orElseGet(() -> String.format(AUTO_GENERATED_RELAY_STATE_FORMAT, UUID.randomUUID(), ""))); // default to the auto generated UUID; + resolver.setRelayStateResolver(request -> Saml2Utils // + .getRelayState(request) + .map(relayState -> String.format(AUTO_GENERATED_RELAY_STATE_FORMAT, UUID.randomUUID(), + relayState)) // pre-append a random string + .orElseGet(() -> String.format(AUTO_GENERATED_RELAY_STATE_FORMAT, UUID.randomUUID(), + ""))); // default to the auto generated UUID; return resolver; } diff --git a/pnet-idp-client-showcase/src/main/java/at/porscheinformatik/pnet/idp/clientshowcase/security/ClientShowcaseSecurityConfig.java b/pnet-idp-client-showcase/src/main/java/at/porscheinformatik/pnet/idp/clientshowcase/security/ClientShowcaseSecurityConfig.java index b110e5d..cc1c915 100644 --- a/pnet-idp-client-showcase/src/main/java/at/porscheinformatik/pnet/idp/clientshowcase/security/ClientShowcaseSecurityConfig.java +++ b/pnet-idp-client-showcase/src/main/java/at/porscheinformatik/pnet/idp/clientshowcase/security/ClientShowcaseSecurityConfig.java @@ -9,6 +9,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.core.env.Environment; import org.springframework.core.env.Profiles; +import org.springframework.scheduling.annotation.EnableScheduling; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.authentication.ProviderManager; @@ -21,9 +22,11 @@ import at.porscheinformatik.idp.openidconnect.EnablePartnerNetOpenIdConnect; import at.porscheinformatik.idp.openidconnect.PartnerNetOpenIdConnectConfigurer; import at.porscheinformatik.idp.openidconnect.PartnerNetOpenIdConnectProvider; +import at.porscheinformatik.idp.saml2.DefaultSaml2CredentialsManager; import at.porscheinformatik.idp.saml2.EnablePartnerNetSaml2; import at.porscheinformatik.idp.saml2.PartnerNetSaml2Configurer; import at.porscheinformatik.idp.saml2.PartnerNetSaml2Provider; +import at.porscheinformatik.idp.saml2.Saml2CredentialsManager; import at.porscheinformatik.idp.saml2.Saml2CredentialsProperties; /** @@ -33,6 +36,7 @@ @EnableWebSecurity @EnablePartnerNetOpenIdConnect @EnablePartnerNetSaml2 +@EnableScheduling public class ClientShowcaseSecurityConfig { private static final Profiles PROD = Profiles.of("prod"); @@ -44,11 +48,13 @@ public class ClientShowcaseSecurityConfig public AuthenticationManager authenticationManager(List providers) { /* - * To get rid of the default AuthenticationManager registered by spring boot, that uses a auto generated password + * To get rid of the default AuthenticationManager registered by spring boot, that uses a auto generated + * password * visible in the logs, we register our own AuthenticationManager. * * If no providers are registered, we register a dummy provider that does nothing. - * If custom authentication mechanisms are registered, they have to register a authentication provider, or handle the authentication + * If custom authentication mechanisms are registered, they have to register a authentication provider, or + * handle the authentication * on their own. */ if (providers.isEmpty()) @@ -59,9 +65,15 @@ public AuthenticationManager authenticationManager(List return new ProviderManager(providers); } + @Bean + public Saml2CredentialsManager saml2CredentialsManager(Saml2CredentialsProperties samlCredentialsConfig) + { + return new DefaultSaml2CredentialsManager(samlCredentialsConfig); + } + @Bean public SecurityFilterChain securityFilterChain(HttpSecurity http, Environment environment, - Saml2CredentialsProperties samlCredentialsConfig, ForceAuthenticationFilter forceAuthenticationFilter, + Saml2CredentialsManager saml2CredentialsManager, ForceAuthenticationFilter forceAuthenticationFilter, ForceTenantFilter forceTenantFilter) throws Exception { if (environment.acceptsProfiles(LOCAL)) @@ -82,7 +94,7 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http, Environment en PartnerNetSaml2Configurer .apply(http, getPartnerNetSaml2Provider(environment)) - .credentials(samlCredentialsConfig) + .credentials(saml2CredentialsManager) .customizer(saml2 -> saml2.failureUrl("/loginerror")); http.logout(logout -> { diff --git a/pom.xml b/pom.xml index d6cbb53..17f55c7 100644 --- a/pom.xml +++ b/pom.xml @@ -1,9 +1,10 @@ - + 4.0.0 org.springframework.boot spring-boot-starter-parent - 3.0.2 + 3.3.2 @@ -106,4 +107,4 @@ - \ No newline at end of file +