SAML Session cookie not being deleted

[Amertz08]

I think this is related to #146. I am seeing both the Django session cookies for the SP and IDP removed (named via SESSION_COOKIE_NAME) from my browser on an SP initiated logout. However the "saml_session" cookie is not being removed. Using djangosaml2==1.1.1

[peppelinux]

Hi @Amertz08
the code is here:
https://github.com/peppelinux/djangosaml2/blob/aa3a6ab284bc7a752775edc527d0860e7ade9766/djangosaml2/middleware.py#L32

feel free to do your considerations, discuss them here and then a PR.
I think that this bug could be handled easily with your help

[Amertz08]

So first apparently did not have the middleware in use 😓 . Second it appears the session is not "empty". There is both a session key as well as values in the session cache. I don't have a great grasp of when a SAML session should be "empty". Obviously there is values in there but who should be emptying the session? Is this the IDPs responsibility? and then the SP can turn around and delete it.

[peppelinux]

@Amertz08 is there something we can do for this issue, have you found a new bug or is this just a question?

[Amertz08]

I am not sure if I have my SP/IDP misconfigured or this is a bug or a misunderstanding. Both django session cookies set in the IDP and SP get deleted but the SAML session cookie does not. The middleware shows it as being nonempty and thus does not delete it. What is supposed to empty the cookie?

[peppelinux]

@Amertz08 you have at least two cookies, one from SP and another from IDP

Now we have to face of the cookie management that belongs to this project needs some test and debug to see if It's a bug

[Amertz08]

So what I am seeing is a Django cookie for both the IDP and SP defined via the SESSION_COOKIE_NAME setting. On top of that I am seeing a third cookie defined by the SAML_SESSION_COOKIE_NAME setting. Going through the debugger it looks like the middleware isn't deleting the SAML_SESSION_COOKIE_NAME. It appears that it is not "empty." I do not know why it would not be cleaned up. I am logging out of the IDP and the SP behaves as such.

[peppelinux]

Hi @Amertz08
I didn't have time to dig this, just want to tell you that actually I don't now when I'll be able to start this investigation but if you could go ahead I'll follow you. Just make a PR to dev branch and post any additional information in this thread, I'll keep an eye on this

[peppelinux]

Let's move "definitely" to this:
#275

bad times are coming for cookies, it's useless and too weak to keep using them for our needs

[peppelinux]

For your question I suggest you to take a look here:

djangosaml2/djangosaml2/tests/__init__.py

Line 697 in 57ad2ba

def test_middleware_cookie_expireatbrowserclose(self):

as we can see saml_session cookie will be pruned after a month but you can even deal with this paramenter using Django global settings related to cookie behaviour

<Morsel: saml_session=k2hqzsea93ptawjefi5yfxbkf87pse2v; expires=Sat, 05 Jun 2021 22:45:17 GMT; HttpOnly; Max-Age=1209600; Path=/; SameSite=None>

[peppelinux]

Hi @Amertz08
are you interested in keeping this issue open and do you need more time to manage it or are you satisfied with what has been discussed and we can believe we can close it?

I hope your tests have confirmed what you expected it to be

[Amertz08]

How do you set the SAML cookie expire time?

We're not seeing any adverse affects to how it's working yet but we also don't have multiple SPs deployed. If the cookie expires after a while I guess that is fine.

Is it recommended to expire on close? I will try this if this is the recommended implementation.

[peppelinux]

Yes, is always recommended to expire on browser close in SSO systems BUT that would be your policy.
We just have to check this:

djangosaml2/djangosaml2/middleware.py

Line 72 in 9949c72

expires=expires, domain=settings.SESSION_COOKIE_DOMAIN,

the argument "expires" MUST be configurable, in otherwords we MUST check that it takes SESSION_COOKIE_AGE and the django poroject global policy about cookie.

that's why we use request.saml_session.get_expiry_age()

put a breakpoint() in it and check what's going on, you'll fined the answer. Let me know if there's the need to have a PR for this.

thank you, really appreciate your help

[peppelinux]

I made some step for you here

env/lib/python3.8/site-packages/djangosaml2/middleware.py(70)process_response()
-> response.set_cookie(
(Pdb) expires
'Wed, 09 Jun 2021 18:12:54 GMT'

so, saml_session cookie always expires, you just have to decide when it MUST do that using django general config.
Feel free to put your comments and close this issue when you'll have achived the solution
thank you for be here

[Amertz08]

@peppelinux thanks for your help on this.