jerrcs / simplesamlphp

Automatically exported from code.google.com/p/simplesamlphp
Other
0 stars 0 forks source link

SP logout problem after IdP-first flow #397

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. configure SP1 with 'store.type' => 'phpsession' and saml:SP authsource
2. initiate IdP-first flow to SP1
3. initiate SLO on another SP (SP2)

- What is the expected output?

The expected result is that we are logged out from all SP's (SP1 and SP2).

- What do you see instead?

We are logged out of SP2 (where we initiated logout), but we are not logged out 
from SP1.

- What version of the product are you using?

SSP SVN trunk

- Please provide any additional information below.

Key parts of logout process are:

- modules/saml/www/sp/saml2-logout.php -> $source->handleLogout($idpEntityId)
- sspmod_saml_Auth_Source_SP::handleLogout($idpEntityId)
- SimpleSAML_Auth_Source::callLogoutCallback($assoc)

Because with IdP-first flow SimpleSAML_Auth_Source.LogoutCallbacks isn't 
defined, process stops, and SP1 session isn't destroyed.

With SP initiated SSO, SimpleSAML_Auth_Source.LogoutCallbacks is defined (in 
SimpleSAML_Auth_Default::initLogin is LogoutCallback defined and then after 
SimpleSAML_Auth_Source.LogoutCallbacks in SimpleSAML_Auth_Source:: 
addLogoutCallback), and session is destroyed by calling $session->doLogout() in 
SimpleSAML_Auth_Default::logoutCallback.

I'll write a patch, but I'm not sure what would be the preferred way to handle 
this problem?

Original issue reported on code.google.com by comel...@gmail.com on 20 Apr 2011 at 9:25

GoogleCodeExporter commented 8 years ago
Good catch! I am tempted to drop the entire addLogoutCallback / 
callLogoutCallback code, since it doesn't actually serve any purpose. (It made 
sense under my original plans for the SimpleSAML_Auth_Source class, but now it 
doesn't actually serve any purpose, and only complicates the code unnecessarily.

Can you confirm that replacing the contents of handleLogout with:

    $session = SimpleSAML_Session::getInstance();
    $session->doLogout($this->authId);

solves your problem?

Original comment by olavmrk@gmail.com on 27 Apr 2011 at 9:37

GoogleCodeExporter commented 8 years ago
Yes, it does.

Original comment by comel...@gmail.com on 3 May 2011 at 9:21

GoogleCodeExporter commented 8 years ago
Here is the proposed fix until this is properly fixed...

BTW Is SSP intended to work in the same installation as IdP and SP with 
'store.type' => 'phpsession'? Not that it doesn't work, it works.

Original comment by comel...@gmail.com on 17 May 2011 at 1:29

Attachments:

GoogleCodeExporter commented 8 years ago
Feel free to commit the fix. I created issue 406 for removing the 
addLogoutCallback and callLogoutCallback functions.

After version 1.7 of SSP, running one or more SPs and an IdP on the same host 
should work.

Original comment by olavmrk@gmail.com on 18 May 2011 at 7:44

GoogleCodeExporter commented 8 years ago
OK, committed as r2839.

Original comment by comel...@gmail.com on 18 May 2011 at 4:33

GoogleCodeExporter commented 8 years ago
Issue 407 has been merged into this issue.

Original comment by olavmrk@gmail.com on 27 May 2011 at 10:17

GoogleCodeExporter commented 8 years ago
Applied recommended patch and can confirm that this fixes the issue.

Original comment by marcus...@virtualthinking.com on 27 May 2011 at 10:35