tngan / samlify

Node.js library for SAML SSO
https://samlify.js.org
MIT License
609 stars 217 forks source link

Query parameter for SLO should be SAMLRequest/SAMLResponse instead of LogoutRequest/LogoutResponse #31

Closed tngan closed 7 years ago

tngan commented 7 years ago

as discussed in #26

AlexeySafronov commented 7 years ago

@tngan, you wrote:

@CatBakun @AlexeySafronov Yes, the string parameter should be only SAMLRequest /SAMLResponse instead of LogoutRequest/LogoutResponse, which are the message body but not the query parameter, thank you for pointing out, will update it soon.

Thanks for quick answer! I still think that query parameter must be SAMLRequest /SAMLResponse too.

I have created a new application in OneLogin (from template ''SAML Test Connector (IdP w/ attr w/ sign response) '') and set up as follows:

6a17f7712b

Then i wrote SP sample app based on your lib. When i try to logout from OneLogin (Idp-initiated logout ), I gets redirect with this query from OneLogin:

/sso/slo/callback?SAMLRequest=ndE9b4MwEIDhv8LmCWwwkOQEqJWyIKUd2qprZYyTIoGP%2Bg6pP7%2FkY4g6dOh4%0A0t3zDleRmcYZDnjChV%2Fc1%2BKIo3Zfi49N2neF1iq2RXeMVarz2O52ZazK3BzT%0AXrui34moJVpc64mN51pkKt2sq3FWvqkSdA55JqLvafQEl04tluABDQ0E3kyO%0AgC28Pj4dIEsUzAEZLY4ieneBBvQrmCjRVOdbuJTCnfY3Zohc4BURzSfzTCCl%0AmecEvRvxNPjE4iTPipwcm96wkWW63WpVybvaLf286u3%2BP%2Bl%2BCM4yhgd265np%0AkgFvgavZXKdfD2h%2BAA%3D%3D%0A

51ad130fe2

Then It was failed in original version of you lib.

I have changed this line to "parserType: 'SAMLRequest'," and method bigin work.

wiki HTTP Redirect Binding:

SAML requests or responses transmitted via HTTP Redirect have a SAMLRequest or SAMLResponse query string parameter, respectively.

My logout route:

app.get('/sso/slo/callback',
        function(req, res) {
                        sp.parseLogoutRequest(idp,
                            'redirect',
                            req,
                            function(requestInfo) {
                                logger.info(JSON.stringify(requestInfo));

                                //TODO: Logout from portal

                                sp.sendLogoutResponse(idp,
                                    requestInfo,
                                    'redirect',
                                    null,
                                    function cb(url) {
                                        res.redirect(url);
                                    });
                            });
        });
jakobsen9 commented 7 years ago

@tngan Are you able to help out in the Gitter chat?

tngan commented 7 years ago

@jakobsen9 Yes, feel free to leave description there.

tngan commented 7 years ago

@AlexeySafronov really appreciated that you provide a procedure to replicate that issue, you are right, the query parameter is different, once you url decoded -> base64 decoded -> inflate context (SAMLRequest), you will find out the raw xml is

<samlp:LogoutRequest ...
</samlp:LogoutRequest>

And the root of xml is <samlp:LogoutRequest>, in current version of express-saml2, it assumes the query parameter is same with the name of root xml node, that's what parserType in #26 is specified.

tngan commented 7 years ago

@AlexeySafronov a patch to this issue is made, feel free to test it

AlexeySafronov commented 7 years ago

@tngan, it's working now! Thanks alot!