spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.73k stars 5.86k forks source link

Add RelayState-based Authentication Request Respository #14793

Open shardasud16 opened 6 months ago

shardasud16 commented 6 months ago

Describe the bug We have a business use case where an application opens multiple tabs of our application, and they share the same session ID. When an HTTP session times out and a new request is sent, all tabs send parallel requests. Based on my understanding so far, HttpSessionSaml2AuthenticationRequestRepository saves the Saml2AuthenticationRequest in the HttpSession using a static key per session. In our case, if six requests are sent in parallel, it overwrites the request. When the SSO response is received, say for the first request, the InResponseTo validation fails as the session has a different Saml2AuthenticationRequest with a different ID. I have very limited experience with Spring Security before this, thus my understanding might be limited.

To Reproduce

Open multiple tabs of an app which has SSO SAML2 authentication (open-saml-4.2.0). Set HTTP Session to 1 minute. Once all tabs' sessions expire, click on "your session has expired," click all one after the other. The same session gets shared between multiple tabs, and the request is stored in the session. When a response comes from the first request, the session might have request 2 stored, and in this case, InResponseTo validation fails. Expected behavior HttpSessionSaml2AuthenticationRequestRepository should store the Saml2AuthenticationRequest within the session with a unique ID added to the key like "RelayState":

private static final String DEFAULT_SAML2_AUTHN_REQUEST_ATTR_NAME = HttpSessionSaml2AuthenticationRequestRepository.class.getName().concat(".SAML2_AUTHN_REQUEST");

My knowledge is limited here and might be missing some key understanding. If you can redirect me to the correct usage, it would be helpful, as I can't find any thread for this similar issue anywhere. We can use some existing solutions like storing requests in our Hazelcast cluster and loading it for authentication, but before this, I want to understand if there is any issue with the current implementation.implementation

Spring Version: 6.2.1 open-saml- 4.2 SAML2.0 JAVA 17 Tomcat 10.1.8

jzheaux commented 5 months ago

Thanks for the detailed description, @shardasud16. Your idea to look up by relay state does have some merit, though there are important security implications to consider. In addition to that, the AuthnRequest isn't the only thing stored in the session (e.g. the RequestCache), and so it's not necessarily a panacea to store the AuthnRequest outside of the session.

That said, this has been requested more than once now, and I think that it's worth reconsidering so long as these tradeoffs are outlined in the documentation. While this won't become the Spring Security default, I think it's reasonable to have it as an option.

Will you please try this out in your application and tell me if it addresses your issue? If so, we can reconsider adding it to Spring Security:

@Component
public class SpringCacheSaml2AuthenticationRequestRepository implements Saml2AuthenticationRequestRepository<AbstractSaml2AuthenticationRequest> {
    private final Cache cache = new ConcurrentMapCache("authentication-requests");

    @Override
    public AbstractSaml2AuthenticationRequest loadAuthenticationRequest(HttpServletRequest request) {
        String relayState = request.getParameter(Saml2ParameterNames.RELAY_STATE);
        return this.cache.get(relayState, AbstractSaml2AuthenticationRequest.class);
    }

    @Override
    public void saveAuthenticationRequest(AbstractSaml2AuthenticationRequest authenticationRequest, HttpServletRequest request, HttpServletResponse response) {
        String relayState = request.getParameter(Saml2ParameterNames.RELAY_STATE);
        this.cache.put(relayState, authenticationRequest);
    }

    @Override
    public AbstractSaml2AuthenticationRequest removeAuthenticationRequest(HttpServletRequest request, HttpServletResponse response) {
        String relayState = request.getParameter(Saml2ParameterNames.RELAY_STATE);
        AbstractSaml2AuthenticationRequest authenticationRequest = this.cache.get(relayState, AbstractSaml2AuthenticationRequest.class);
        if (authenticationRequest == null) {
            return null;
        }
        this.cache.evict(relayState);
        return authenticationRequest;
    }
}
spring-projects-issues commented 5 months ago

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

shardasud16 commented 5 months ago

Apologies for the delay. We tested the suggestions provided earlier, and they resolved the issue. Thank you.

However, we encountered another problem where we still receive random errors such as:

"Did not find SecurityContext in HttpSession ###### using the SPRING_SECURITY_CONTEXT session attribute."

From what I gathered, if one request is successfully authenticated, the context is cleared. If another request's response arrives at the same time, we encounter this error. We also observed the same behavior in version 5.6.x. Since this occurrence is rare and similar to what was experienced in 5.6.x, we are proceeding with this solution.

I have a basic question: Does Spring support multiple SAML Auth requests for the same session ID? I believe this is a fundamental use case, and thus, I suspect we might be doing something wrong. Please provide any insights or suggestions if you have them.

Scenario: Multiple SAML2 authentication requests for the same session ID (multiple tabs on a browser).

jzheaux commented 5 months ago

"Did not find SecurityContext in HttpSession ###### using the SPRING_SECURITY_CONTEXT session attribute."

Such a message is not necessarily a problem. This message can come up quite often under normal pre-authenticated circumstances. Usually, it only means that this request is not yet authenticated, which is certainly the case when the authnrequest first arrives.

That said, it could indicate that one of the two scenarios I describe below is happening.

Does Spring support multiple SAML Auth requests for the same session ID?

Good question. To be clear, I think the question is more general than that: What happens if I try and log the user in on multiple tabs at the same time?

It's typically one of two things:

  1. If the first request hasn't yet written the session cookie to the browser, the second request may not be using the same session identifier. In the end, this may recreate the session multiple times with the browser only using the one that finished last.
  2. If both requests have the same session id, it may be that the first request hasn't finished writing to the session before the second request reads it. In the end, this may authenticate the same session multiple times.

These two scenarios are what happens any time two side-effect requests operate on a shared resource. The processing of each request isn't atomic and so there is bound to be interleaving. That said, it often is not an issue since the request material is the same and the underlying effect of logging in is idempotent. But, if it is an issue, you'd likely need to reconsider this part of your design to see if you can manage to perform only one login.

shardasud16 commented 5 months ago

Thank you very much and appreciate your help.