Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Saml SSO configuration yaml file referenced in the docs not working properly with newer versions of spring boot #12810

Open
Anubhav-2000 opened this issue Mar 1, 2023 · 8 comments
Assignees
Labels
in: docs An issue in Documentation or samples type: bug A general bug

Comments

@Anubhav-2000
Copy link
Contributor

Anubhav-2000 commented Mar 1, 2023

I was using the spring security docs and this link as a reference to implement SSO: https://medium.com/digital-software-architecture/spring-boot-spring-security-with-saml-2-83d87df5b470

This seems to work if the spring boot starter parent dependency is version 2.4.2. But fails with version 2.7.2. It gives an error Invalid signature for object [id…]

I messed around with the code a bit and it only worked when i changed the identityprovider tag in the yaml file to assertingparty tag.

So the spring docs says to use this yaml file: https://docs.spring.io/spring-security/reference/servlet/saml2/login/overview.html#saml2-specifying-identity-provider-metadata

But the correct yaml file is:
spring:
security:
saml2:
relyingparty:
registration:
adfs:
assertingparty:
entity-id: https://idp.example.com/issuer
verification.credentials:
- certificate-location: "classpath:idp.crt"
singlesignon.url: https://idp.example.com/issuer/sso
singlesignon.sign-request: false

@Anubhav-2000 Anubhav-2000 added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Mar 1, 2023
@jvalkeal
Copy link
Contributor

jvalkeal commented Mar 8, 2023

You're right that it's expected to work with assertingparty as Spring Boot 2.7.x deprecated identityprovider in favour of assertingparty. We need to change docs to mention something about this deprecation.

Can you share a bit more about the error, do you possible have a sample we could run? Did it fail at server startup or at runtime?

@jvalkeal
Copy link
Contributor

jvalkeal commented Mar 8, 2023

Also looking spring-projects/spring-boot#30128, some new properties are now handled via boot's auto-config, but those added in that issue don't check deprecated properties.

I also saw a simple boot app(2.7.x) not to fail at startup when using identityprovider but it did with assertingparty. I expected behaviour to be same.

Also boot 3.0.x did remove deprecations so identityprovider is not processed anymore.

@Anubhav-2000
Copy link
Contributor Author

Hi @jvalkeal ,
When i have my yaml file like this: https://docs.spring.io/spring-security/reference/servlet/saml2/login/overview.html#saml2-specifying-identity-provider-metadata

Along with my security config file as:

@EnableWebSecurity
public class SecurityConfig {
    @Autowired
    private RelyingPartyRegistrationRepository relyingPartyRegistrationRepository;

    @Bean
    SecurityFilterChain app(HttpSecurity http) throws Exception {
        // @formatter:off
        RelyingPartyRegistrationResolver relyingPartyRegistrationResolver = new DefaultRelyingPartyRegistrationResolver(this.relyingPartyRegistrationRepository);
        Saml2MetadataFilter filter = new Saml2MetadataFilter(relyingPartyRegistrationResolver, new OpenSamlMetadataResolver());
        http
                .authorizeHttpRequests((authorize) -> authorize
                        .anyRequest().authenticated()
                )
                .saml2Login(withDefaults())
                .addFilterBefore(filter, Saml2WebSsoAuthenticationFilter.class);;
        // @formatter:on

        return http.build();
    }
}

I get this error:
Screenshot 2023-03-08 at 11 57 02 PM

In the logs i get:
Signature of Assertion 'id170959100448639611173959713' from Issuer: (url of issuer) not valid

My pom.xml:

<?xml version="1.0" encoding="UTF-8"?>
<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>2.7.9</version>
		<relativePath/> <!-- lookup parent from repository -->
	</parent>
	<groupId>com.example</groupId>
	<artifactId>demo</artifactId>
	<version>0.0.1-SNAPSHOT</version>
	<name>demo</name>
	<description>Demo project for Spring Boot</description>
	<properties>
		<java.version>17</java.version>
	</properties>
	<dependencies>
		<dependency>
			<groupId>org.springframework.boot</groupId>
			<artifactId>spring-boot-starter-web</artifactId>
		</dependency>
		<dependency>
			<groupId>org.springframework.security</groupId>
			<artifactId>spring-security-saml2-service-provider</artifactId>
		</dependency>
		<dependency>
			<groupId>org.springframework.boot</groupId>
			<artifactId>spring-boot-starter-thymeleaf</artifactId>
		</dependency>
		<dependency>
			<groupId>org.springframework.boot</groupId>
			<artifactId>spring-boot-starter-actuator</artifactId>
		</dependency>
		<dependency>
			<groupId>org.springframework.boot</groupId>
			<artifactId>spring-boot-starter-test</artifactId>
			<scope>test</scope>
		</dependency>
		<dependency>
			<groupId>org.springframework.boot</groupId>
			<artifactId>spring-boot-starter-security</artifactId>
		</dependency>
		<dependency>
			<groupId>org.springframework.security</groupId>
			<artifactId>spring-security-test</artifactId>
			<scope>test</scope>
		</dependency>
	</dependencies>

	<build>
		<plugins>
			<plugin>
				<groupId>org.springframework.boot</groupId>
				<artifactId>spring-boot-maven-plugin</artifactId>
			</plugin>
		</plugins>
	</build>

</project>

@jvalkeal
Copy link
Contributor

jvalkeal commented Mar 9, 2023

If you use:

spring:
  security:
    saml2:
      relyingparty:
        registration:
          adfs:
            identityprovider:
            # assertingparty:
              entity-id: https://idp.example.com/issuer
              verification.credentials:
                - certificate-location: "classpath:idp.crt"
              singlesignon.url: https://idp.example.com/issuer/sso
              singlesignon.sign-request: false

you should get exception (as it happens with assertingparty):

Caused by: java.lang.IllegalStateException: Certificate  location 'class path resource [idp.crt]' does not exist
        at org.springframework.util.Assert.state(Assert.java:97) ~[spring-core-5.3.25.jar:5.3.25]
        at org.springframework.boot.autoconfigure.security.saml2.Saml2RelyingPartyRegistrationConfiguration.readCertificate(Saml2RelyingPartyRegistrationConfiguration.java:176) ~[spring-boot-autoconfigure-2.7.9.jar:2.7.9]

I think these all goes back to boot's handling of that deprecation, for example in this check empty list is returned, not null so it tries to use one from assertingparty.

https://github.com/spring-projects/spring-boot/blob/4bd0f7511991ddd15879e9a317f90eff1fd97722/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyRegistrationConfiguration.java#L237-L240

Other issue is below checks where deprecation is not even checked:

https://github.com/spring-projects/spring-boot/blob/4bd0f7511991ddd15879e9a317f90eff1fd97722/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyRegistrationConfiguration.java#L223-L233

@wilkinsona
Copy link
Member

I've opened spring-projects/spring-boot#34525 to fix the problem with Boot ignoring the certificate location.

@jvalkeal
Copy link
Contributor

jvalkeal commented Mar 9, 2023

We'll use this issue to just update docs.

@jvalkeal jvalkeal self-assigned this Mar 9, 2023
@jvalkeal jvalkeal added in: docs An issue in Documentation or samples and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 9, 2023
@Anubhav-2000
Copy link
Contributor Author

Should i still update the docs to reflect assertingparty instead of identityprovider?

@jvalkeal
Copy link
Contributor

@Anubhav-2000 If you want to create a PR, then it should be done against branch 6.0.x. Thx for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: docs An issue in Documentation or samples type: bug A general bug
Projects
Status: No status
Development

No branches or pull requests

3 participants