IdentityPython / djangosaml2

Django SAML2 Service Provider based on pySAML2
Apache License 2.0
259 stars 143 forks source link

'NoneType' object has no attribute 'name_qualifier' (probable regression) #402

Open olivierdalang opened 6 months ago

olivierdalang commented 6 months ago

Hey ! Thanks for this great library !!

We're getting 'NoneType' object has no attribute 'name_qualifier' when using the saml logout view when the account is a local account (not logged in through saml but through regular django login).

Issue #72 was exactly describing this and was marked as fixed, and has a comment with someone experiencing this after closing, so it looks like a regression.

From the code, it seems like the case where the user has no associated saml session is thought of (it logs a warning), but then not handled (it still runs code to logout from that session).

See: https://github.com/IdentityPython/djangosaml2/blob/1be79465fbf3cc9e2d1685af4639a54ba4d22568/djangosaml2/views.py#L707-L715C43

Affected version: 1.9.2 (django 5.0.4)

Cheers

peppelinux commented 6 months ago

Yes, that issue should not have been closed. I think that's trivial handling this kind of exception by using also a logger.info that evidences that the user was succesfully logged out without any preexisting SAML2 session

there are some cases where the logout requires a preexistent user session, while the session cookie is not returned back to the SP, in these cases is important give awareness to the implemeters about this, otherwise it would be possibile for an IDP issues a SAML2 logout request and get SP's user be logged out

there are also several considerationbs to be made in cases where the user has multiple sessions and a saml2 logout request may require the logout of all those session.

Could I ask you to bring some proposal to be discussed in this thread and work on a PR to definitively resolve this issue? djangosaml2 is a community driven project that lives thanks to the community, your participation if the fuel of this project, thank you for this @olivierdalang