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

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

Closed
jeanblanchard opened this issue Jun 19, 2020 · 25 comments
Closed
Assignees
Labels
in: saml2 An issue in SAML2 modules type: enhancement A general enhancement
Milestone

Comments

@jeanblanchard
Copy link

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)

@jeanblanchard jeanblanchard added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Jun 19, 2020
@jzheaux jzheaux added in: saml2 An issue in SAML2 modules and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 19, 2020
@fpagliar
Copy link

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
Copy link
Contributor

jzheaux commented Jul 22, 2020

@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
Copy link

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
Copy link
Contributor

jzheaux commented Jul 31, 2020

@fpagliar, I agree that there are some common patterns across these filters. Would you care to share some of these thoughts over on #8887 which seems to be thinking along the same lines as your last comment?

@julisch94
Copy link

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
Copy link

scho commented Jan 26, 2021

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

@ThomasHeil
Copy link

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.

@jzheaux jzheaux added this to the 5.5.x milestone Jan 28, 2021
@jzheaux jzheaux self-assigned this Jan 28, 2021
@aabuniaj
Copy link

aabuniaj commented Feb 7, 2021

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
Copy link

aabuniaj commented Feb 8, 2021

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

@ambra886
Copy link

ambra886 commented Mar 1, 2021

Also us we waiting for this.

jzheaux added a commit to jzheaux/spring-security that referenced this issue Mar 5, 2021
@jzheaux jzheaux modified the milestones: 5.5.x, 5.5.0-RC1 Apr 10, 2021
@eleftherias eleftherias modified the milestones: 5.6.0-M1, 5.6.0-M2 Jul 19, 2021
@jzheaux
Copy link
Contributor

jzheaux commented Jul 19, 2021

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

@aabuniaj
Copy link

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!

@rwinch rwinch modified the milestones: 5.6.0-M2, 5.6.0-M3 Aug 16, 2021
@junytse
Copy link
Contributor

junytse commented Aug 18, 2021

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
Copy link
Contributor

jzheaux commented Aug 20, 2021

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?

akohli96 pushed a commit to akohli96/spring-security that referenced this issue Aug 25, 2021
@junytse
Copy link
Contributor

junytse commented Aug 26, 2021

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:

  • Missing <SessionIndex>: I added "SessionIndex" based on a saved Assertion from login with codes like below:

    SessionIndexBuilder sessionIndexBuilder = new SessionIndexBuilder();
    for (AuthnStatement statement : assertion.getAuthnStatements()) {
        statement.getSessionIndex();
        SessionIndex index = sessionIndexBuilder.buildObject();
        index.setValue(statement.getSessionIndex());
        logoutRequest.getSessionIndexes().add(index);
    }
  • Missing attributes from <NameID>: I also copied "NameID" attributes from the one in the login Assertion:

    assertion.getSubject().getNameID()
    

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
Copy link

VanillaChi commented Sep 3, 2021

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
Copy link

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
Copy link
Contributor

jzheaux commented Sep 3, 2021

@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
Copy link
Contributor

jzheaux commented Sep 3, 2021

@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
Copy link
Contributor

jzheaux commented Sep 7, 2021

@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.

jzheaux added a commit to jzheaux/spring-security that referenced this issue Sep 13, 2021
@Olbix
Copy link

Olbix commented Dec 17, 2021

@jzheaux regarding Missing "SessionIndex"
#10613

@Akash-Anand0744
Copy link

@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 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: saml2 An issue in SAML2 modules type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests