Skip to content

Commit

Permalink
Upgrade to Spring Boot 3.3.2
Browse files Browse the repository at this point in the history
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
spring-projects/spring-framework#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.
  • Loading branch information
thred committed Jul 29, 2024
1 parent ebd0076 commit 4a4da83
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down Expand Up @@ -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<List<Saml2CredentialsConfig>> supplier)
@Deprecated(forRemoval = true)
PartnerNetSaml2Configurer credentials(Supplier<List<Saml2CredentialsConfig>> 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
*
Expand Down Expand Up @@ -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(
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

/**
Expand All @@ -33,6 +36,7 @@
@EnableWebSecurity
@EnablePartnerNetOpenIdConnect
@EnablePartnerNetSaml2
@EnableScheduling
public class ClientShowcaseSecurityConfig
{
private static final Profiles PROD = Profiles.of("prod");
Expand All @@ -44,11 +48,13 @@ public class ClientShowcaseSecurityConfig
public AuthenticationManager authenticationManager(List<AuthenticationProvider> 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())
Expand All @@ -59,9 +65,15 @@ public AuthenticationManager authenticationManager(List<AuthenticationProvider>
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))
Expand All @@ -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 -> {
Expand Down
7 changes: 4 additions & 3 deletions pom.xml
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>3.0.2</version>
<version>3.3.2</version>
<!-- HINT: When upgrading to a newer version, check if the following bug is fixed: https://github.com/spring-projects/spring-security/issues/12665 -->
<!-- If fixed, Remove the workaround in ClientShowcaseSecurityConfig. -->
</parent>
Expand Down Expand Up @@ -106,4 +107,4 @@
</plugins>
</pluginManagement>
</build>
</project>
</project>

0 comments on commit 4a4da83

Please sign in to comment.