Skip to content

InResponseTo validation for saml2 executed even if saved request is not found #12203

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

Closed
inabumst opened this issue Nov 14, 2022 · 5 comments
Closed
Assignees
Labels
for: stackoverflow A question that's better suited to stackoverflow.com

Comments

@inabumst
Copy link

Summary
With 5.7.x mandatory validation of InResponseTo was introduced if it is provided in the authentication response. Validation logic expects to find saved Saml2AuthenticationRequest in HttpSession. However that is only possible if SameSite attribute is not set.
According security requirements of current project I'm working on it is set to Lax or Strict. This configuration is done on Apache through which developed application is accessible.
Looks like it is the same issue as closed #10828 with no solution provided.

Would highly appreciate any proposals how to deal with the issue as it currently stops us from upgrading to 5.7.x version.

To Reproduce
Use Spring Security Saml2 provider with combination of SameSite Lax or Strict set.

Expected behavior
Maybe possibility to disable InResponseTo validation or alternative way to save authentication request data?

@inabumst inabumst added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Nov 14, 2022
@jzheaux jzheaux self-assigned this Nov 14, 2022
@jzheaux jzheaux added for: stackoverflow A question that's better suited to stackoverflow.com and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Nov 14, 2022
@jzheaux
Copy link
Contributor

jzheaux commented Nov 14, 2022

Thanks for getting in touch! It feels like this is a question that would be better suited to Stack Overflow. We prefer to use GitHub issues only for bugs and enhancements. Feel free to update this issue with a link to the re-posted question (and I'll be happy to address your question there) or add more detail if you feel this is a genuine bug.

@jzheaux jzheaux closed this as completed Nov 14, 2022
@inabumst
Copy link
Author

inabumst commented Nov 15, 2022

Added question to Stack Overflow (https://stackoverflow.com/questions/74435518/spring-security-saml2-provider-inresponseto-validation-for-saml2-executed-even) as requested. Hope I'll get an answer there.
On the other hand as this validation at the moment is breaking change I would say it would be good mentioning that in "what is new for saml in 5.7 release" or provide information in documentation if it is not a bug, but feature.

@inabumst
Copy link
Author

@jzheaux haven't received any response regarding the issue on Stack Overflow.
I did more checking and in #9174 you stated that application can choose not to store AuthnRequest and that would lead that no validation be executed, but from the code analysis I have done it is only important if response contains InResponseTo attribute or not. Plus validation logic is in 2 places - responseValidator and assertionValidator.
Would really be interested to know more about the design and recommendations for those who can't avoid having SameSite set to Lax or Strict.

@akashgupta2703
Copy link

I am commenting on closed issue. Just want to make sure anyone landing here with same problem (seems to be very common) are able to view and comment on the solution suggested.

@inabumst I ran into the same issue and created an alternative implementation of Saml2AuthenticationRequestRepository which stores SAML2 request in database. I have posted the answer in the same stackoverflow question. Following is the code which uses RelayState as a key for saving/loading saml2 request. With this, everything seems to work fine so far -

import org.springframework.data.mongodb.core.MongoTemplate;
import org.springframework.data.mongodb.core.query.Criteria;
import org.springframework.data.mongodb.core.query.Query;
import org.springframework.security.saml2.core.Saml2ParameterNames;
import org.springframework.security.saml2.provider.service.authentication.AbstractSaml2AuthenticationRequest;
import org.springframework.security.saml2.provider.service.web.Saml2AuthenticationRequestRepository;
import org.springframework.stereotype.Repository;

import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;

@Repository
@RequiredArgsConstructor
@Slf4j
public class MongoSaml2AuthenticationRequestRepository implements Saml2AuthenticationRequestRepository<AbstractSaml2AuthenticationRequest> {

    public static final String SAML2_REQUEST_COLLECTION = "saml2RequestsRepository";
    private final @NonNull MongoTemplate mongoTemplate;
    
    @Override
    public AbstractSaml2AuthenticationRequest loadAuthenticationRequest(HttpServletRequest request) {
        
        String relayState = request.getParameter(Saml2ParameterNames.RELAY_STATE);
        if (relayState == null) {
            return null;
        }
        
        log.debug("Fetching SAML2 Authentication Request by relay state : {}", relayState.get());
        Query query = Query.query(Criteria.where("relayState").is(relayState.get()));
        AbstractSaml2AuthenticationRequest authenticationRequest = mongoTemplate.findOne(query, AbstractSaml2AuthenticationRequest.class, SAML2_REQUEST_COLLECTION);
        
        if (!authenticationRequest.getRelayState().equals(relayState.get())) {
            log.error("Relay State received from request '{}' is different from saved request '{}'.", relayState.get(), authenticationRequest.getRelayState());
            return null;
        }
        
        log.debug("SAML2 Request retrieved : {}", authenticationRequest);
        return authenticationRequest;
    }

    @Override
    public void saveAuthenticationRequest(AbstractSaml2AuthenticationRequest authenticationRequest,
            HttpServletRequest request, HttpServletResponse response) {
        
        //As per OpenSamlAuthenticationRequestResolver, it will always have value. However, one validation can be added to check for null and regenerate.
        String relayState = authenticationRequest.getRelayState();
        log.debug("Relay State Received: {}", relayState);
        mongoTemplate.save(authenticationRequest, SAML2_REQUEST_COLLECTION);
    }

    @Override
    public AbstractSaml2AuthenticationRequest removeAuthenticationRequest(HttpServletRequest request,
            HttpServletResponse response) {
        
        AbstractSaml2AuthenticationRequest authenticationRequest = loadAuthenticationRequest(request);
        if (authenticationRequest == null) {
            return null;
        }
        
        mongoTemplate.remove(authenticationRequest, SAML2_REQUEST_COLLECTION);
        return authenticationRequest;
    }

}

@jzheaux
Copy link
Contributor

jzheaux commented Jan 18, 2025

Related to #14793 and #14297

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: stackoverflow A question that's better suited to stackoverflow.com
Projects
None yet
Development

No branches or pull requests

3 participants