saml: Rework SP-initiated logout config to support IdP-level config.

This gives more flexibility on a server with multiple organizations and
SAML IdPs. Such a server can have some organizations handled by IdPs
with SLO set up, and some without it set up. In such a scenario, having
a generic True/False server-wide setting is insufficient and instead
being able to specify the IdPs/orgs for SLO is needed.

docs/production/authentication-methods.md

@@ -703,7 +703,8 @@ this with another IdP.
 #### SP-initiated Single Logout
 
 After configuring IdP-initiated Logout, you only need to set
-`SAML_ENABLE_SP_INITIATED_SINGLE_LOGOUT = True` in
+`"sp_initiated_logout_enabled": True` in the appropriate IdP
+configuration dict in `SOCIAL_AUTH_SAML_ENABLED_IDPS` in
 `/etc/zulip/settings.py` to also enable SP-initiated Logout. When this
 is active, a user who logged in to Zulip via SAML, upon clicking
 "Logout" in the Zulip web app will be redirected to the IdP's Single

zerver/tests/test_auth_backends.py

@@ -2067,7 +2067,6 @@ class SAMLAuthBackendTest(SocialAuthBase):
     def get_account_data_dict(self, email: str, name: str) -> Dict[str, Any]:
         return dict(email=email, name=name)
 
-    @override_settings(SAML_ENABLE_SP_INITIATED_SINGLE_LOGOUT=True)
     def test_saml_sp_initiated_logout_success(self) -> None:
         hamlet = self.example_user("hamlet")
 
@@ -2126,7 +2125,6 @@ class SAMLAuthBackendTest(SocialAuthBase):
         self.client_get(result["Location"])
         self.assert_logged_in_user_id(None)
 
-    @override_settings(SAML_ENABLE_SP_INITIATED_SINGLE_LOGOUT=True)
     def test_saml_sp_initiated_logout_invalid_logoutresponse(self) -> None:
         hamlet = self.example_user("hamlet")
         self.login("hamlet")
@@ -2148,14 +2146,13 @@ class SAMLAuthBackendTest(SocialAuthBase):
         )
         self.assert_logged_in_user_id(hamlet.id)
 
-    @override_settings(SAML_ENABLE_SP_INITIATED_SINGLE_LOGOUT=True)
     def test_saml_sp_initiated_logout_endpoint_when_not_logged_in(self) -> None:
         self.assert_logged_in_user_id(None)
 
         result = self.client_post("/accounts/logout/")
-        self.assert_json_error(result, "Not logged in.")
+        self.assertEqual(result.status_code, 302)
+        self.assertEqual(result["Location"], "/accounts/login/")
 
-    @override_settings(SAML_ENABLE_SP_INITIATED_SINGLE_LOGOUT=True)
     def test_saml_sp_initiated_logout_logged_in_not_via_saml(self) -> None:
         """
         If the user is logged in, but not via SAML, the normal logout flow
@@ -2171,7 +2168,6 @@ class SAMLAuthBackendTest(SocialAuthBase):
         self.client_get(result["Location"])
         self.assert_logged_in_user_id(None)
 
-    @override_settings(SAML_ENABLE_SP_INITIATED_SINGLE_LOGOUT=True)
     def test_saml_sp_initiated_logout_when_saml_not_enabled(self) -> None:
         """
         If SAML is not enabled, the normal logout flow should be correctly executed.

zerver/views/auth.py

@@ -1118,24 +1118,35 @@ def json_fetch_api_key(
 logout_then_login = require_post(django_logout_then_login)
 
 
-@require_post
+def should_do_saml_sp_initiated_logout(request: HttpRequest) -> bool:
-def logout_view(request: HttpRequest, /, **kwargs: Any) -> HttpResponse:
     realm = RequestNotes.get_notes(request).realm
     assert realm is not None
 
-    if not settings.SAML_ENABLE_SP_INITIATED_SINGLE_LOGOUT or not saml_auth_enabled(realm):
-        return logout_then_login(request, **kwargs)
-
     if not request.user.is_authenticated:
-        raise JsonableError(_("Not logged in."))
+        return False
+
+    if not saml_auth_enabled(realm):
+        return False
+
+    idp_name = SAMLSPInitiatedLogout.get_logged_in_user_idp(request)
+    if idp_name is None:
+        # This session wasn't authenticated via SAML, so proceed with normal logout process.
+        return False
+
+    return settings.SOCIAL_AUTH_SAML_ENABLED_IDPS[idp_name].get(
+        "sp_initiated_logout_enabled", False
+    )
+
+
+@require_post
+def logout_view(request: HttpRequest, /, **kwargs: Any) -> HttpResponse:
+    if not should_do_saml_sp_initiated_logout(request):
+        return logout_then_login(request, **kwargs)
 
     # This will first redirect to the IdP with a LogoutRequest and if successful on the IdP side,
     # the user will be redirected to our SAMLResponse-handling endpoint with a success LogoutResponse,
     # where we will finally terminate their session.
     result = SAMLSPInitiatedLogout.slo_request_to_idp(request, return_to=None)
-    if result is None:
-        # This session wasn't authenticated via SAML, so proceed with normal logout process.
-        return logout_then_login(request, **kwargs)
 
     return result
 

zproject/backends.py

@@ -2882,11 +2882,11 @@ class SAMLSPInitiatedLogout:
     @classmethod
     def slo_request_to_idp(
         cls, request: HttpRequest, return_to: Optional[str] = None
-    ) -> Optional[HttpResponse]:
+    ) -> HttpResponse:
         """
         Generates the redirect to the IdP's SLO endpoint with
-        the appropriately generated LogoutRequest or None if the session
+        the appropriately generated LogoutRequest. This should only be called
-        wasn't authenticated via SAML.
+        on requests with a session that was indeed obtained via SAML.
         """
 
         user_profile = request.user
@@ -2900,7 +2900,7 @@ class SAMLSPInitiatedLogout:
 
         idp_name = cls.get_logged_in_user_idp(request)
         if idp_name is None:
-            return None
+            raise AssertionError("User not logged in via SAML")
 
         idp = saml_backend.get_idp(idp_name)
         auth = saml_backend._create_saml_auth(idp)

zproject/default_settings.py

@@ -92,8 +92,6 @@ SOCIAL_AUTH_SAML_SECURITY_CONFIG: Dict[str, Any] = {}
 # the limit_to_subdomains setting to be considered valid:
 SAML_REQUIRE_LIMIT_TO_SUBDOMAINS = False
 
-SAML_ENABLE_SP_INITIATED_SINGLE_LOGOUT = False
-
 # Historical name for SOCIAL_AUTH_GITHUB_KEY; still allowed in production.
 GOOGLE_OAUTH2_CLIENT_ID: Optional[str] = None
 

zproject/prod_settings_template.py

@@ -450,6 +450,9 @@ SOCIAL_AUTH_SAML_ENABLED_IDPS: Dict[str, Any] = {
         ## default, Zulip asks the user whether they want to create an
         ## account or try to log in again using another method.
         # "auto_signup": False,
+        ## Determines whether Service Provider initiated SAML Single Logout should be enabled.
+        ## Note that IdP-initiated Single Logout must be configured before enabling this.
+        # "sp_initiated_logout_enabled": False,
     },
 }
 
@@ -494,9 +497,6 @@ SOCIAL_AUTH_SAML_SUPPORT_CONTACT = {
 #    }
 # }
 
-# This setting allows enabling of SP-initiated logout with SAML.
-# SAML_ENABLE_SP_INITIATED_SINGLE_LOGOUT = True
-
 ########
 ## Apple authentication ("Sign in with Apple").
 ##

zproject/settings_types.py

@@ -12,6 +12,7 @@ class SAMLIdPConfigDict(TypedDict, total=False):
     entity_id: str
     url: str
     slo_url: str
+    sp_initiated_logout_enabled: bool
     attr_user_permanent_id: str
     attr_first_name: str
     attr_last_name: str

zproject/test_extra_settings.py

@@ -245,6 +245,7 @@ SOCIAL_AUTH_SAML_ENABLED_IDPS: Dict[str, SAMLIdPConfigDict] = {
         "entity_id": "https://idp.testshib.org/idp/shibboleth",
         "url": "https://idp.testshib.org/idp/profile/SAML2/Redirect/SSO",
         "slo_url": "https://idp.testshib.org/idp/profile/SAML2/Redirect/Logout",
+        "sp_initiated_logout_enabled": True,
         "x509cert": get_from_file_if_exists("zerver/tests/fixtures/saml/idp.crt"),
         "attr_user_permanent_id": "email",
         "attr_first_name": "first_name",