Client global logout doesn't work

[Lorenzo Gil Sanchez]

After succesfully adapt the dajngosaml2 code to do a SAML authentication request and log in into my application using a simpleSAMLphp IdP I tried to do the same with the logout process. But it doesn't quite work so well.

The problems are:

 * Bug in client.py:412 what is assigned to destination is a dictionary, not an actual url. This is because the config format changed. You should do self.config["service"]["sp"]["idp"][entity_id]['logout_service'] instead of just self.config["service"]["sp"]["idp"][entity_id].
 * Bug in client.py:416 self.client.users should be self.users
 * The global_logout method does not accept some basic parameters as the log and wheter we want to sign the request or not. On the opposite side, the local_logout method does accept too much parameters. Only the subject_id should be needed.
 * Returning the request ids of the logout requests in the global_logout method is not very useful for the caller of such method. Instead, we should return a list of urls that the caller can redirect to perform the actual logout. This is equivalent to what the authenticate method does.

All of these problems are fixed in this revision: http://bazaar.launchpad.net/~lgs/pysaml2/main/revision/268 I've tested that branch and it works.

There is also another thing: I don't understand why the global_logout returns a list of logout requests instead of just one. What's the rationale of this? In my code I just read the first element of this list ignoring the rest.

[Roland Hedberg (roland-hedberg)]

On 8/5/10 22:08, Lorenzo Gil Sanchez wrote:
> There is also another thing: I don't understand why the global_logout
> returns a list of logout requests instead of just one. What's the
> rationale of this? In my code I just read the first element of this list
> ignoring the rest.
>
You're probably correct I was thinking about the case where you use one
Idp and several AAs for collecting de identity of a subject.
But then you will not log in to the AA hence logout from an AA is sort
of strange.

-- Roland

[Lorenzo Gil Sanchez (lgs)]

You introduced a small subtle bug at http://bazaar.launchpad.net/~roland-hedberg/pysaml2/main/revision/429 when creating the logout request:

            request = samlp.LogoutRequest(
                id=sid(),
                version=VERSION,
                issue_instant=instant(),
                destination=destination,
                issuer=self.issuer(),
                session_index=sid(),
                name_id = name_id,
            )

This gives me the following exception when serializing this object into a string:

Exception Type: AttributeError
Exception Value: 'str' object has no attribute 'become_child_element_of'
Exception Location: /home/lgs/proyectos/yaco/pysaml2/src/saml2/__init__.py in _add_members_to_element_tree, line 500

I saw that the 'session_index' attribute is not neccesary and removing it from the request is what works for me:

            request = samlp.LogoutRequest(
                id=sid(),
                version=VERSION,
                issue_instant=instant(),
                destination=destination,
                issuer=self.issuer(),
                name_id = name_id,
            )

See my change at: http://bazaar.launchpad.net/~lgs/pysaml2/main/revision/271

[Lorenzo Gil Sanchez (lgs)]

Another different issue related to the global logout is the handling of the IdP logout response. We need to process that SAML response and do the local logout if everything is fine.

See my changes in changeset http://bazaar.launchpad.net/~lgs/pysaml2/main/revision/273

[Roland Hedberg (roland-hedberg)]

On 8/7/10 10:59, Lorenzo Gil Sanchez wrote:
> You introduced a small subtle bug at http://bazaar.launchpad.net
> /~roland-hedberg/pysaml2/main/revision/429 when creating the logout
> request:
>
> request = samlp.LogoutRequest(
> id=sid(),
> version=VERSION,
> issue_instant=instant(),
> destination=destination,
> issuer=self.issuer(),
> session_index=sid(),
> name_id = name_id,
> )
>
> This gives me the following exception when serializing this object into
> a string:
>
True, if anything session_index is an instance of samlp.SessionIndex.
Actually if my reading is correct it should be the session index that
the IdP uses for the login.
If you leave it out, I guess the IdP just closes the login session/-s
that this subject has due to a request from this SP.

-- Roland

[Roland Hedberg (roland-hedberg)]

On 8/7/10 11:05, Lorenzo Gil Sanchez wrote:
> Another different issue related to the global logout is the handling of
> the IdP logout response. We need to process that SAML response and do
> the local logout if everything is fine.
>
> See my changes in changeset
> http://bazaar.launchpad.net/~lgs/pysaml2/main/revision/273
>
>
Right!

I guess there is a special reason for returning None when there is no
SAMLResponse ?
You want to distinguish this case from when there is a SAMLResponse but
when this can be either OK or not OK.
So in fact you have three possible outcomes.

-- Roland