spring-projects / spring-projects/spring-security

Signature of Assertion from issuer was not valid and invalid destination for SAML response by multiple simultaneous login

Open
#14,885 4 comments 0 reactions 1 assignee View on GitHub

@jzheaux is already working on this.

Since Apr 26, 2024.

status: feedback-provided
Dominant language
Java
Stars
9.6k
Forks
6.3k
Avg merge
2d 11h
Merged PRs (30d)
52

Description

Describe the bug
I have encountered an issue with the Spring SAML library that leads to incorrect token validation during the authentication process. The problem occurs under concurrent authentication scenarios, wherein the response from the Identity Provider (IdP) intended for one user may be erroneously processed by another user's authentication thread, resulting in invalid error messages. After refreshing the page, a new authentication succeeds.
It seems that by requesting /authenticate the relyingPartyRegistration creates a registration with a registrationId in one thread and waits for response from external IdP to validate the request. But it can happen that a new request creates a new registrationId in the same thread, and it causes invalid destination and invalid signature.

To Reproduce
Initiate concurrent authentication requests for multiple users.
Observe the processing of authentication tokens and IdP responses.
Note instances where responses are incorrectly validated against tokens belonging to different users.
It can be reproduced with a script that sends /login and /authenticate requests for different idPs together in a loop.

Expected behavior
The Spring SAML library should ensure proper association of authentication tokens with their respective users, preventing cross-thread validation issues.

http.authorizeRequests()
                        .saml2Login()
                        .failureHandler(samlAuthenticationFailureHandler)
                        `.successHandler(samlAuthenticationSuccessHandler);`

RelyingPartyRegistrations
                .fromMetadataLocation(metadataUrl())
                .registrationId(idpId)
                .assertionConsumerServiceLocation( ".../login/" + idpId)
                .entityId( ".../login/" + idpId)
                .build();

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.