From 45e3626bd29f6c408b84f744a0cae48e70a9b1e7 Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Tue, 23 May 2023 14:40:53 +0200 Subject: [PATCH] saml: Clean up additional session vars if authentication fails. This doesn't have any obvious security implications right now, but nonetheless such information is not meant to stick around in the session if authentication didn't succeed and not cleaning up would be a bug. --- zproject/backends.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/zproject/backends.py b/zproject/backends.py index 722e3c1f20..e4b90ca38b 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -2742,8 +2742,10 @@ class SAMLAuthBackend(SocialAuthMixin, SAMLAuth): self.logger.info("/complete/saml/: error while parsing SAMLResponse:", exc_info=True) # Fall through to returning None. finally: + # We need a finally: block to ensure we don't keep around information in the session + # if the authentication failed. if result is None: - for param in self.standard_relay_params: + for param in [*self.standard_relay_params, "saml_idp_name", "saml_session_index"]: # If an attacker managed to eavesdrop on the RelayState token, # they may pass it here to the endpoint with an invalid SAMLResponse. # We remove these potentially sensitive parameters that we have set in the session