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

Additions to handle SOAP based Logout Responses #454

Closed
thcase opened this issue Sep 11, 2020 · 6 comments
Closed

Additions to handle SOAP based Logout Responses #454

thcase opened this issue Sep 11, 2020 · 6 comments

Comments

@thcase
Copy link

thcase commented Sep 11, 2020

In working with a SAML IDP provider that uses SOAP based SLO, most of the library works fine with a few tweaks that I have identified in other methods. Most of the library works fine with the SOAP, what I had to do, however is the following changes, which I recommend additional methods or revising existing methods to handle SOAP Logout Responses possibly be considered.

  1. To extract the LogoutRequest from the SOAP envelope prior to calling the validatePostRequestMethod, I used a Regular expression to extract the SOAP body (e.g., const regex = /([\w\W]*)(<ns0\:LogoutRequest[\w\W]+<\/ns0:LogoutRequest)([\w\W]*)/gm, const subst = '$2', and const logoutRequest = req.body.replace(regex,subst)). I am not sure which method would be best to add this to. I could possible be added to the validatePostRequestMethod if adding an overall option for Logout callback method (currently in Pull Request Added option to set singleLogoutServiceBinding #440).
  2. When generating the logoutResponse (SAML.prototype.generateLogoutResponse, line 272), add a check to see if Logout callback method protocol method (Pull Request Added option to set singleLogoutServiceBinding #440) is SOAP to add wrap the Logout Response with a SOAP Envelope and Body.
@markstos
Copy link
Contributor

Please reference what the SAML spec says regarding these cases.

@thcase
Copy link
Author

thcase commented Sep 16, 2020

The SAML Specification (I believe it is this covers SOAP bindings in section 3.2. The ns0: prefix appears to be specific to the vendor I am dealing with. The main point is within the SOAP body is the standard SAML LogoutRequest (in plain text).

@srd90
Copy link

srd90 commented Sep 16, 2020

First of all title of the issue speaks about handling LogoutResponses but text part is all about processing LogoutRequest (sent by IdP to SP).

I assume that you are speaking handling of incoming (from SP point of view) LogoutRequests over SOAP and sending LogoutResponses over SOAP to request initiator.

Furthermore it seems that you are suggesting that implementation of validatePostRequest method should be changed so that it would extract LogoutRequest from SOAP message and proceed processing of that message as if it would be LogoutRequest via POST binding up until the moment when content of LogoutResponse is constructed (and in this case it would be wrapped into SOAP message).

AFAIK SOAP binding is used mostly for synchronous direct communication between IdP and SPs (so called back channel communication meaning that end user's browser is not involved in any way).

passport-saml module (already) fails to process LogoutRequests correctly over existing SLO bindings when HTTP request does not contain session cookie and to make things worse it returns always "LogoutRequest processed successfully" (for further information see #419 ).

IMHO if SOAP binding support for LogoutRequests would be implement as you are suggesting it would mean that passport-saml module would contain yet another binding which returns "Success" as LogoutResponse status but would fail to actually terminate application session (most probably express-session's session) which was created for "namedid & sessionindex" combination indicated by LogoutRequest during single sign on process.

@thcase
Copy link
Author

thcase commented Sep 16, 2020

I think you have captured what I am requesting. The current implementation doesn't handle SOAP for SLO, yet SOAP is in the specification. As far as not handling either HTTP-POST or SOAP based POSTS logout requests initiated by the IDP, I don't know as if that is really a function of the strategy (forgive me if I am new to Passport in general). The issue is the Strategy doesn't have any direct tie to the Session Storage being used by Passport.

In my case, in my logout route, after validating LogOutRequest is valid (i.e., calling validatePostRequest), if the isLoggedOut return variable is true, I then call my own method to clear out the session store. I am using Redis store and when a session is initiated, I regenerate the Session ID in the format of (prefix:NameId:UniqueID). The method to clear out sessions, clears out all Redis keys with (prefix:NameId:*) on an approved LogoutRequest. After clearing out the keys, I then respond to the IDP with a SOAP logout Response with application/xml as content type.

Again, the clearing out of Passport Sessions is specific to Session Store and I feel is outside the scope of the strategy. The only thing may be for the logout callback process to pass a closure to the strategy either in validatePostRequest method or as an option when setting up the strategy and that being called to handle the closing of Passport Sessions on an IDP initiated logout.

As you say, this doesn't work for HTTP-POST SLO, so, I am not sure why not adding the handling of SOAP SLO matters. If that was the case, you shouldn't handle HTTP-POST SLO either.

@markstos
Copy link
Contributor

markstos commented Oct 7, 2020

@thcase I'm open to PRs here, but don't have specific suggestions for the design. I recommend submitting a PR for further discussion. Help looking at #419 is welcome as well.

@srd90
Copy link

srd90 commented Jan 18, 2024

IMHO this issue along with issues:

all talk about workarounds/kludges to support SOAP binding. Instead of having these issues one could create new issue about adding support for that particular binding (and back channel communication between IdP and passport-saml). I.e. I am suggesting closing this issue.

As a side note: nowadays core SAML functionality is separated from passport-saml thus author of this issue might be able to introduce own thin passport layer module for SOAP binding (if he is not willing to contribute such support to passport-saml) by using core SAML functionality from https://github.com/node-saml/node-saml or from some other SAML library like https://github.com/tngan/samlify to validate SAML messages extracted from SOAP messages (see issue description).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants