SAML-Toolkits / wordpress-saml

OneLogin SAML plugin for Wordpress
MIT License
65 stars 74 forks source link

When logging out the sessionIndex and nameID is not passed to the IdP #4

Closed jlmnrc closed 9 years ago

jlmnrc commented 9 years ago

Thank you for the great plugin, it allows me to login to my ADFS successfully. However, when it is logging out, it does not pass the nameID and session index to the Logout request. Am I missing any settings?

pitbulk commented 9 years ago

@johnliman, Thanks for report this bug with ADFS.

If you review the toolkit demo, I pass those parameters to the logout function: https://github.com/onelogin/php-saml/blob/master/demo1/index.php#L32

But if you review the Wordpress plugin I'm not providing the sessionIndex and the nameID required by ADFS: https://github.com/pitbulk/wordpress-onelogin/blob/master/onelogin-saml-sso/php/functions.php#L68

In order to be solved I may save those values on the saml_acs method, and recover them on the saml_slo.

I wrote a fast fix here, test it: https://gist.github.com/pitbulk/f848045a6c2f2a10710a

jlmnrc commented 9 years ago

thanks @pitbulk for your fast reply, there are missing bracket at line 69 and 72. I will test it and let you know.

pitbulk commented 9 years ago

You are right, fixed.

pitbulk commented 9 years ago

Also maybe you may review the settings.php file of the plugin: https://github.com/pitbulk/wordpress-onelogin/blob/master/onelogin-saml-sso/php/settings.php#L34

There the NameIDFormat is hardcoded to 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress'

and ADFS requires a different NameIDFormat.

jlmnrc commented 9 years ago

I have configured the ADFS to receive emailAddress so I am able to login. I am now figuring out the logout issue, I suspect it is the session id, but pending to test with my ADFS counterparts tomorrow.

jlmnrc commented 9 years ago

Thanks @pitbulk for your plugin, it is now passing the nameId, but it doesn't solve my ADFS slo.

I am checking with the ADFS team again tomorrow.

cheers

jlmnrc commented 9 years ago

Hi @pitbulk I have successfully logout from ADFS, however, the onelogin library throw an error:

Signature validation failed. Logout Response rejected

It fails when verifying the signature. Any idea?

pitbulk commented 9 years ago

Can you try to use SAML Tracer to record the ADFS Logout Response and use this tool to validate it: https://www.samltool.com/validate_logout_res.php

jlmnrc commented 9 years ago

It returns the same result:

Signature validation failed. Logout Response rejected

jlmnrc commented 9 years ago

Latest update: I could login and logout from ADFS successfully.

However, after logout, the plugin throw "Signature validation failed. Logout Response rejected" error. I also tried IdP initiated logout, and it gives the same error. So it must be the signature problem?

pitbulk commented 9 years ago

What SigAlg is used by ADFS? Can you provide to me the complete information so I can debug myself and try to figure out the problem?

jlmnrc commented 9 years ago

SigAlg: http://www.w3.org/2000/09/xmldsig#rsa-sha1

pitbulk commented 9 years ago

I tested myself and yes, the validation failed.

Can you review that ADFS is using the same public cert for the SAML Response on the SSO process and the SAML Response on the SLO process?

jlmnrc commented 9 years ago

that's what I suspected, but doesn't the login uses the same public cert too?

jlmnrc commented 9 years ago

Hi @pitbulk, the gist that you provided does work, and log me out from ADFS.

Are you planning to move the gist into the main branch anytime soon?

pitbulk commented 9 years ago

I will integrate this change in the next release. I hope to be able to do it next week.

jlmnrc commented 9 years ago

Great! thanks so much for the great plugin.

I still have error when doing SLO with ADFS, I am asking the ADFS counterparts and still waiting for their answer.

jlmnrc commented 9 years ago

Hi @pitbulk, I have found out the problem with ADFS logout. It is because the SAMLResponse is retrieved and encoded, i.e. if I changed the parameter "$retrieveParametersFromServer" to true, it will be validated successfully.

Original:

public function processSLO($keepLocalSession = false, $requestId = null, $retrieveParametersFromServer = false)

If I changed it to:

public function processSLO($keepLocalSession = false, $requestId = null, $retrieveParametersFromServer = true)

The above changes is only a quick test for me if it will work, and it does work. I am hoping you will find a way to fix it so it could work with ADFS as well.

Thanks again for the support so far.