spring-projects / spring-security

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

Support sending SAML 2.0 LogoutRequest to the IdP (Single Logout) #8731

Closed jeanblanchard closed 3 years ago

jeanblanchard commented 4 years ago

Expected Behavior

It would be nice to be able to send a samlp:LogoutRequest to the SAML Identity Provider, to trigger a Single Logout.

Current Behavior

Currently you can only do a local logout (=invalidate the session), but you stay authenticated to the IdP. In an authentication-required app, this means that, as soon as you log out locally, you get immediately redirected to the IdP, which logs you right back in. In effect, this means there is no way to logout at all, except by invalidating the session directly on the IdP (for those that allow it)

fpagliar commented 4 years ago

Is there any alternative currently available for this?

Right now it seems like given spring-security-saml is out of support, the upgrade path would be to implement your own copy of SingleLogoutProfileImpl, SAMLLogoutProcessingFilter, SAMLLogoutFilter from the old project, and a brand new initialization of OpenSAML in the new setup given that OpenSamlImplementation is package private.

jzheaux commented 4 years ago

@fpagliar, I don't believe there are any existing alternatives. But, since it sounds like you might be producing a solution anyway, I wonder if you'd be interested in contributing it back?

I'm thinking that it would make sense to create an implementation of LogoutSuccessHandler similar in nature to OidcClientInitiatedLogoutSuccessHandler but that would generate a <LogoutRequest> instead. I think it also makes sense to introduce a filter that knows how to process a <LogoutResponse>.

fpagliar commented 4 years ago

I'm looking at this and will be glad to contribute when possible. Might not get a full-fledged generic solution but it will at least be a good first approach to iterate on.

@jzheaux I'm wondering a bit on refactoring from the current structure. Looking at LogoutRequest and LogoutResponse, the basic structure of the bindings and request processing are basically the same. So extracting the common patterns from Saml2WebSsoAuthenticationRequestFilter and Saml2WebSsoAuthenticationFilter seems like a good approach. The handling of what to do on failed cases for a SLO flow is a gray area that will need some work. How does that sound?

jzheaux commented 4 years ago

@fpagliar, I agree that there are some common patterns across these filters. Would you care to share some of these thoughts over on https://github.com/spring-projects/spring-security/issues/8887 which seems to be thinking along the same lines as your last comment?

julisch94 commented 3 years ago

Hey there! We are waiting for this issue to be implemented. Just wanted to check in real quick.. Is there any progress yet? Thanks a lot.

scho commented 3 years ago

@fpagliar Would you mind sharing what you have so far?

ThomasHeil commented 3 years ago

Waiting for this, too. I think in times when browsers tend to restore sessions even beyond close+reopen (who made this up?), a proper logout is definitely needed and should be prioritized accordingly.

aabuniaj commented 3 years ago

Waiting on this as well. It would be nice to have this please. When would it be available? Would it be part of the spring.security.saml2.relyingparty.registration property under the identityprovider? something like: spring.security.saml2.relyingparty.registration.sp.identityprovider.singlelogout.url?

aabuniaj commented 3 years ago

Any walk around for the time being? I can't find any examples out there to logout.

ambra886 commented 3 years ago

Also us we waiting for this.

aabuniaj commented 3 years ago

Thank you @jzheaux for working on this. When should we expect this to be released? When can we start using it?

jzheaux commented 3 years ago

Based on this comment, there's just a bit more work to be done on this before releasing. Since our 5.5 release candidate is going out today, we'll target the 5.6 release in the fall for SLO.

aabuniaj commented 3 years ago

Hello,

Any updates on this ticket?

jzheaux commented 3 years ago

Hi, @aabuniaj. Coincidentally, yes, I was in the middle of updating when you commented. :) You can see the PR for the latest.

aabuniaj commented 3 years ago

Hi, @aabuniaj. Coincidentally, yes, I was in the middle of updating when you commented. :) You can see the PR for the latest.

I had a feeling something was getting done! Thanks for the update!

junytse commented 3 years ago

Hi @jzheaux I'm glad to see SLO implementation added to spring-security. Really appreciate your effort!

I was trying to integrate the spring-security development code to implement sp-initialted SLO on my project and here are the problem I meet and my workaround:

  1. HttpSessionLogoutRequestRepository.saveLogoutRequest(...) assert that "RelayState" is not empty, which seems to be used to identify request/response pair. However, OpenSamlLogoutRequestResolver.resolve(...) only add "RelayState" to REDIRECT binding but not POST binding. I'm not sure if this is intentional and I added .relayState(relayState) to the POST request builder part in my local code.
  2. OpenSamlLogoutRequestResolver.getRegistrationId(...) accept only Saml2Authentication authentications, while in my project I used a customized Authentication subclass. I was not able to change my authentication to extend Saml2Authentication and to make my code piece work, I had a workaround to reconstruct a Saml2Authentication object based on my custom Authentication object before passing it to Saml2LogoutRequestSuccessHandler.onLogoutSuccess(..). For me it would be more convenient if getRegistrationId(...) accept an interface that have an registration id getter, then I could implement it in my authentication subclass.
  3. I used Saml2LogoutResponseProcessingFilter to handle "SAMLResponse" back from IDP, but it doesn't have request matcher and logout handler/successhandler, which exist in SAMLLogoutProcessingFilter from old spring-security-extension. I need the matcher to match the logout response endpoint and handlers to do cleanup jobs such as invalidate session. I did a workaround by subclassing CompositeFilter with an extra RequestMatcher member, and adding both Saml2LogoutResponseProcessingFilter (for response validation) and a LogoutFilter (to call handlers) to the filter list.

The codes seems still changing and I'm not sure the above issue I meet above will be covered by new codes.

I'm looking forward to the release of this feature!

jzheaux commented 3 years ago

Thanks for the early feedback, @junytse!

  1. Yes, this appears to be an oversight. I'll take a closer look and apply any needed changes.

  2. Agreed that it would be nice if this were simpler, though I'll need a bit more time to think about this. One possibility is to place the relying party registration id on the Saml2AuthenticatedPrincipal interface instead. How would that play out in your situation?

  3. These two concerns are separated at this point. The processing filter validates the request and the LogoutFilter invalidates the session, clears the security context, etc.

The DSL takes care of this concern by adding the processing and logout filters for you:

http
    .saml2Logout(withDefaults());

Have you already tried using that?

junytse commented 3 years ago

Hi @jzheaux , glad to see your reply!

For your comments:

  1. I think storing/readint registration id in "principal" is better in my situation. It is easier to change implementation of Authentication.getPrincipal() to return a Saml2AuthenticatedPrincipal than making the logout Authentication class to be a Saml2Authentication subclass.
  2. I'm using xml-base configuration and not using those DSLs, but they differ only on formats. I think what you mean is that I could create a separated WebSecurityConfigurerAdapter (or <security:http> tag in xml configuration) and config the filter chain with the processing filter and LogoutFilter , and assign the path matcher using http directly (or request-matcher-ref attribute in xml configuration). However, my logout filters are placed on an existing filter chain and I have to set the request matcher on the filter level.

By the way, in my implementation I construct "LogoutRequest" with the following customizations:

I referenced those changes from previous Spring saml extension: https://github.com/spring-projects/spring-security-saml/blob/main/core/src/main/java/org/springframework/security/saml/websso/SingleLogoutProfileImpl.java#L110

VanillaChi commented 3 years ago

Hi @jzheaux glad to see this is ongoing and really appreciate your work! We're really looking forward to adopting it when the official 5.6 comes out, could we by any chance know the schedule for this feature as well as the 5.6 release so that we could arrange our product line nicely?

ooshirokm commented 3 years ago

Hi @jzheaux We're really looking forward to the official release of SLO. I believe the current version is released as version "M", but can we please ask when you plan to release "GA" and "RC" versions??

jzheaux commented 3 years ago

@VanillaChi, @ooshirokm, glad to hear you are looking forward to it. I've just added the 5.6.0 milestone to address your question.

jzheaux commented 3 years ago

@junytse Regarding SessionIndex, will you please add a ticket? There have been a number of requests so far on different threads, and it would be nice to keep track of it in a single place.

Regarding #3, would you be able to share a minimal sample (e.g. GitHub) so I can get a picture of what you mean? If you are using XML configuration, it seems to me that you will be able to wire the processing filter and an accompanying logout filter in the same chain.

jzheaux commented 3 years ago

@junytse I've updated the PR based on your feedback. Please look for the following changes:

  1. The RelayState should now be included on POSTs as well
  2. The registrationId has been moved to Saml2AuthenticatedPrincipal
  3. The logout filter has been added as a member of each processing filter. While not directly related to your comment about them being separate, I feel that keeping the two operations together is better from a security standpoint so that the intent of the request is not misinterpreted.
Olbix commented 2 years ago

@jzheaux regarding Missing "SessionIndex" https://github.com/spring-projects/spring-security/issues/10613

Akash-Anand0744 commented 6 months ago

@jzheaux

I know the ticket is closed but I am new to all this and have some doubt.

I am using

<artifactId>spring-security-saml2-core</artifactId>
                <version>1.0.10.RELEASE</version>

<spring-security.version>5.5.8</spring-security.version>

I see the same issue i.e.,

(SAMLLogoutFilter.java:159) DEBUG - Error processing metadata
org.opensaml.saml2.metadata.provider.MetadataProviderException: IDP doesn't contain any SingleLogout endpoints

Will upgrade to 5.6.0-M3 is more suitable or should I just add the pr changes to my release ? Where is the PR ?