saml: Implement IdP-initated logout for Keycloak.

Fixes #13948.

docs/production/authentication-methods.md

@@ -598,6 +598,58 @@ to the root and `engineering` subdomains:
          importing, only the certificate will be displayed (not the private
          key).
 
+### IdP-initiated SAML Logout
+
+Zulip 5.0 introduces beta support for IdP-initiated SAML Logout. The
+implementation has primarily been tested with Keycloak and these
+instructions are for that provider; please [contact
+us](https://zulip.com/help/contact-support) for help using this with
+another IdP.
+
+1. In the KeyCloak configuration for Zulip, enable `Force Name ID Format`
+   and set `Name ID Format` to `email`. Zulip needs to receive
+   the user's email address in the NameID to know which user's
+   sessions to terminate.
+1. Make sure `Front Channel Logout` is enabled, which it should be by default.
+1. In `Fine Grain SAML Endpoint Configuration`, set `Logout Service Redirect Binding URL`
+   to the same value you provided for `SSO URL` above.
+1. Add the IdP's `Redirect Binding URL`for `SingleLogoutService` to
+   your IdP configuration dict in `SOCIAL_AUTH_SAML_ENABLED_IDPS` in
+   `/etc/zulip/settings.py` as `slo_url`. For example it may look like
+   this:
+
+   ```
+   "your_keycloak_idp_name": {
+       "entity_id": "https://keycloak.example.com/auth/realms/yourrealm",
+       "url": "https://keycloak.example.com/auth/realms/yourrealm/protocol/saml",
+       "slo_url": "https://keycloak.example.com/auth/realms/yourrealm/protocol/saml",
+       ...
+   ```
+
+   You can find these details in your `SAML 2.0 Identity Provider Metadata` (available
+   in your `Realm Settings`).
+
+1. Because Keycloak uses the old `Name ID Format` format for
+   pre-existing sessions, each user needs to be logged out before SAML
+   Logout will work for them. Test SAML logout with your account by
+   logging out from Zulip, logging back in using SAML, and then using
+   the SAML logout feature from KeyCloak. Check
+   `/var/log/zulip/errors.log` for error output if it doesn't work.
+1. Once SAML logout is working for you, you can use the `manage.py logout_all_users` management command to logout all users so that
+   SAML logout works for everyone.
+
+   ```bash
+   /home/zulip/deployments/current/manage.py logout_all_users
+   ```
+
+#### Caveats
+
+- This beta doesn't support using `SessionIndex` to limit which
+  sessions are affected; it always terminates all logged-in sessions
+  for the user identified in the `NameID`.
+- SAML Logout in a configuration where your IdP handles authentication
+  for multiple organizations is not yet supported.
+
 ## Apache-based SSO with `REMOTE_USER`
 
 If you have any existing SSO solution where a preferred way to deploy

zerver/lib/types.py

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

zerver/tests/fixtures/saml/logoutrequest.txt

@@ -0,0 +1 @@
+<samlp:LogoutRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" Destination="http://zulip.testserver/complete/saml/" ID="ID_fed666cd-2f56-48a8-89f6-cc34723c011a" IssueInstant="2021-10-21T11:15:18.548Z" Version="2.0"><saml:Issuer>https://idp.testshib.org/idp/shibboleth</saml:Issuer><saml:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">{email}</saml:NameID><samlp:SessionIndex>843cfae0-52d6-4ace-871d-78c61a81d4fb::2b1b41e9-f8c8-4114-ab1d-13a01889fb23</samlp:SessionIndex></samlp:LogoutRequest>

zerver/tests/test_auth_backends.py

@@ -29,7 +29,9 @@ from django.utils.timezone import now as timezone_now
 from django_auth_ldap.backend import LDAPSearch, _LDAPUser
 from jwt.exceptions import PyJWTError
 from onelogin.saml2.auth import OneLogin_Saml2_Auth
+from onelogin.saml2.logout_request import OneLogin_Saml2_Logout_Request
 from onelogin.saml2.response import OneLogin_Saml2_Response
+from onelogin.saml2.utils import OneLogin_Saml2_Utils
 from social_core.exceptions import AuthFailed, AuthStateForbidden
 from social_django.storage import BaseDjangoStorage
 from social_django.strategy import DjangoStrategy
@@ -1931,9 +1933,157 @@ class SAMLAuthBackendTest(SocialAuthBase):
 
         return saml_response
 
+    def generate_saml_logout_request_from_idp(self, email: str) -> str:
+        """
+        The logoutrequest.txt fixture has a pre-generated LogoutRequest,
+        with {email} placeholder, that can
+        be filled out with the data we want.
+        """
+        unencoded_logout_request = self.fixture_data("logoutrequest.txt", type="saml").format(
+            email=email,
+        )
+        logout_request: str = base64.b64encode(unencoded_logout_request.encode()).decode()
+
+        return logout_request
+
+    def make_idp_initiated_logout_request(
+        self, email: str, make_validity_checks_pass: bool = True
+    ) -> HttpResponse:
+        samlrequest = self.generate_saml_logout_request_from_idp(email)
+        parameters = {"SAMLRequest": samlrequest}
+
+        if make_validity_checks_pass:
+            # It's hard to create fully-correct LogoutRequests with signatures in tests,
+            # so we rely on mocking the validating functions instead.
+            with mock.patch.object(
+                OneLogin_Saml2_Logout_Request, "is_valid", return_value=True
+            ), mock.patch.object(
+                OneLogin_Saml2_Auth,
+                "validate_request_signature",
+                return_value=True,
+            ):
+                result = self.client_get("http://zulip.testserver/complete/saml/", parameters)
+        else:
+            result = self.client_get("http://zulip.testserver/complete/saml/", parameters)
+        return result
+
     def get_account_data_dict(self, email: str, name: str) -> Dict[str, Any]:
         return dict(email=email, name=name)
 
+    def test_saml_idp_initiated_logout_success(self) -> None:
+        hamlet = self.example_user("hamlet")
+        old_api_key = hamlet.api_key
+        self.login("hamlet")
+
+        self.assert_logged_in_user_id(hamlet.id)
+        result = self.make_idp_initiated_logout_request(hamlet.delivery_email)
+        self.assert_logged_in_user_id(None)
+
+        # The expected response is a redirect to the IdP's slo_url endpoint
+        # with a SAMLResponse announcing success.
+        self.assertEqual(result.status_code, 302)
+        redirect_to = result["Location"]
+        self.assertIn(settings.SOCIAL_AUTH_SAML_ENABLED_IDPS["test_idp"]["slo_url"], redirect_to)
+
+        parsed = urllib.parse.urlparse(redirect_to)
+        query_dict = urllib.parse.parse_qs(parsed.query)
+
+        self.assertIn("SAMLResponse", query_dict)
+        # Do some very basic parsing of the SAMLResponse to verify it's a success response.
+        saml_response_encoded = query_dict["SAMLResponse"][0]
+        saml_response = OneLogin_Saml2_Utils.decode_base64_and_inflate(
+            saml_response_encoded
+        ).decode()
+        self.assertIn(
+            '<samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success" />', saml_response
+        )
+
+        hamlet.refresh_from_db()
+        # Ensure that the user's api_key was rotated:
+        self.assertNotEqual(hamlet.api_key, old_api_key)
+
+    def test_saml_idp_initiated_logout_request_for_different_user(self) -> None:
+        """
+        This test verifies that sessions are revoked based on the NameID
+        in the LogoutRequest rather than just the logged in session cookie.
+        """
+        hamlet = self.example_user("hamlet")
+        cordelia = self.example_user("cordelia")
+        cordelia_old_api_key = cordelia.api_key
+        self.login("hamlet")
+
+        self.assert_logged_in_user_id(hamlet.id)
+        # We're logged in as hamlet, but deliver a LogoutRequest for cordelia.
+        # This means our session should not be affected.
+        self.make_idp_initiated_logout_request(cordelia.delivery_email)
+        self.assert_logged_in_user_id(hamlet.id)
+
+        cordelia.refresh_from_db()
+        # Cordelia's api_key should have been rotated:
+        self.assertNotEqual(cordelia.api_key, cordelia_old_api_key)
+
+    def test_saml_idp_initiated_logout_invalid_nameid_format(self) -> None:
+        hamlet = self.example_user("hamlet")
+        self.login("hamlet")
+
+        self.assert_logged_in_user_id(hamlet.id)
+        with self.assertLogs("zulip.auth.saml") as mock_logger:
+            # LogoutRequests need to have the email address in NameID
+            # so putting "hamlet" there is invalid.
+            result = self.make_idp_initiated_logout_request("hamlet")
+        self.assert_logged_in_user_id(hamlet.id)
+
+        self.assertEqual(
+            mock_logger.output,
+            [
+                "INFO:zulip.auth.saml:/complete/saml/: LogoutRequest failed: NameID is not a valid email address: hamlet"
+            ],
+        )
+        self.assertEqual(result.status_code, 302)
+        self.assertEqual(result["Location"], "/")
+
+    def test_saml_idp_initiated_logout_user_not_in_realm(self) -> None:
+        hamlet = self.example_user("hamlet")
+        self.login("hamlet")
+
+        self.assert_logged_in_user_id(hamlet.id)
+        with self.assertLogs("zulip.auth.saml") as mock_logger:
+            result = self.make_idp_initiated_logout_request("nonexistant@zulip.com")
+        self.assert_logged_in_user_id(hamlet.id)
+
+        self.assertEqual(
+            mock_logger.output,
+            [
+                "INFO:zulip.auth.saml:/complete/saml/: LogoutRequest failed: No user with email specified in NameID found in realm 2. return_data={}"
+            ],
+        )
+        self.assertEqual(result.status_code, 302)
+        self.assertEqual(result["Location"], "/")
+
+    def test_saml_idp_initiated_logout_invalid_signature(self) -> None:
+        hamlet = self.example_user("hamlet")
+        self.login("hamlet")
+
+        self.assert_logged_in_user_id(hamlet.id)
+        with self.assertLogs("zulip.auth.saml") as mock_logger:
+            # LogoutRequests  we generate in tests don't have signatures. We can use
+            # the make_validity_checks_pass argument to disable mocking of python3-saml
+            # internal validation functions to make validation of our LogoutRequest fail
+            # and test our error-handling of that.
+            result = self.make_idp_initiated_logout_request(
+                hamlet.delivery_email, make_validity_checks_pass=False
+            )
+        self.assert_logged_in_user_id(hamlet.id)
+
+        self.assertEqual(
+            mock_logger.output,
+            [
+                "INFO:zulip.auth.saml:/complete/saml/: LogoutRequest failed: ['invalid_logout_request_signature', 'Signature validation failed. Logout Request rejected']"
+            ],
+        )
+        self.assertEqual(result.status_code, 302)
+        self.assertEqual(result["Location"], "/")
+
     def test_auth_registration_with_no_name_provided(self) -> None:
         """
         The SAMLResponse may not actually provide name values, which is considered
@@ -2085,7 +2235,12 @@ class SAMLAuthBackendTest(SocialAuthBase):
             self.assertEqual(result.status_code, 302)
             self.assertIn("login", result.url)
         self.assertEqual(
-            m.output, [self.logger_output("/complete/saml/: No SAMLResponse in request.", "info")]
+            m.output,
+            [
+                self.logger_output(
+                    "/complete/saml/: No SAMLResponse or SAMLRequest in request.", "info"
+                )
+            ],
         )
 
         # Check that POSTing the RelayState, but with missing SAMLResponse,
@@ -2101,7 +2256,12 @@ class SAMLAuthBackendTest(SocialAuthBase):
             self.assertEqual(result.status_code, 302)
             self.assertIn("login", result.url)
         self.assertEqual(
-            m.output, [self.logger_output("/complete/saml/: No SAMLResponse in request.", "info")]
+            m.output,
+            [
+                self.logger_output(
+                    "/complete/saml/: No SAMLResponse or SAMLRequest in request.", "info"
+                )
+            ],
         )
 
         # Now test bad SAMLResponses.
@@ -2160,7 +2320,7 @@ class SAMLAuthBackendTest(SocialAuthBase):
             m.output,
             [
                 self.logger_output(
-                    "/complete/saml/: Can't figure out subdomain for this authentication request. relayed_params: {}".format(
+                    "/complete/saml/: Can't figure out subdomain for this SAMLResponse. relayed_params: {}".format(
                         "{}"
                     ),
                     "info",
@@ -2493,7 +2653,7 @@ class SAMLAuthBackendTest(SocialAuthBase):
             m.output,
             [
                 self.logger_output(
-                    "/complete/saml/: Can't figure out subdomain for this authentication request. relayed_params: {}",
+                    "/complete/saml/: Can't figure out subdomain for this SAMLResponse. relayed_params: {}",
                     "info",
                 )
             ],

zproject/backends.py

@@ -35,6 +35,7 @@ from django.utils.translation import gettext as _
 from django_auth_ldap.backend import LDAPBackend, _LDAPUser, ldap_error
 from lxml.etree import XMLSyntaxError
 from onelogin.saml2.errors import OneLogin_Saml2_Error
+from onelogin.saml2.logout_request import OneLogin_Saml2_Logout_Request
 from onelogin.saml2.response import OneLogin_Saml2_Response
 from onelogin.saml2.settings import OneLogin_Saml2_Settings
 from requests import HTTPError
@@ -62,6 +63,7 @@ from zerver.lib.actions import (
     do_create_user,
     do_deactivate_user,
     do_reactivate_user,
+    do_regenerate_api_key,
     do_update_user_custom_profile_data_if_changed,
 )
 from zerver.lib.avatar import avatar_url, is_avatar_new
@@ -73,6 +75,7 @@ from zerver.lib.mobile_auth_otp import is_valid_otp
 from zerver.lib.rate_limiter import RateLimitedObject
 from zerver.lib.redis_utils import get_dict_from_redis, get_redis_client, put_dict_in_redis
 from zerver.lib.request import RequestNotes
+from zerver.lib.sessions import delete_user_sessions
 from zerver.lib.subdomains import get_subdomain
 from zerver.lib.types import ProfileDataElementValue
 from zerver.lib.url_encoding import append_url_query_string
@@ -2252,21 +2255,33 @@ class SAMLAuthBackend(SocialAuthMixin, SAMLAuth):
 
         return data
 
-    def get_issuing_idp(self, SAMLResponse: str) -> Optional[str]:
+    def get_issuing_idp(self, saml_response_or_request: Tuple[str, str]) -> Optional[str]:
         """
-        Given a SAMLResponse, returns which of the configured IdPs is declared as the issuer.
+        Given a SAMLResponse or SAMLRequest, returns which of the configured IdPs
+        is declared as the issuer.
         This value MUST NOT be trusted as the true issuer!
         The signatures are not validated, so it can be tampered with by the user.
         That's not a problem for this function,
         and true validation happens later in the underlying libraries, but it's important
         to note this detail. The purpose of this function is merely as a helper to figure out which
-        of the configured IdPs' information to use for parsing and validating the response.
+        of the configured IdPs' information to use for parsing and validating the request.
         """
         try:
             config = self.generate_saml_config()
             saml_settings = OneLogin_Saml2_Settings(config, sp_validation_only=True)
-            resp = OneLogin_Saml2_Response(settings=saml_settings, response=SAMLResponse)
+            if saml_response_or_request[1] == "SAMLResponse":
+                resp = OneLogin_Saml2_Response(
+                    settings=saml_settings, response=saml_response_or_request[0]
+                )
                 issuers = resp.get_issuers()
+            else:
+                assert saml_response_or_request[1] == "SAMLRequest"
+
+                # The only valid SAMLRequest we can receive is a LogoutRequest.
+                logout_request_xml = OneLogin_Saml2_Logout_Request(
+                    config, saml_response_or_request[0]
+                ).get_xml()
+                issuers = [OneLogin_Saml2_Logout_Request.get_issuer(logout_request_xml)]
         except self.SAMLRESPONSE_PARSING_EXCEPTIONS:
             self.logger.info("Error while parsing SAMLResponse:", exc_info=True)
             return None
@@ -2357,10 +2372,76 @@ class SAMLAuthBackend(SocialAuthMixin, SAMLAuth):
         )
         raise AuthFailed(self, error_msg)
 
+    def process_logout(self, subdomain: str, idp_name: str) -> Optional[HttpResponse]:
+        """
+        We override process_logout, because we need to customize
+        the way of revoking sessions and introduce NameID validation.
+
+        The python-social-auth and python3-saml implementations expect a simple
+        callback function without arguments, to delete the session. We're not
+        happy with that for two reasons:
+        1. These implementations don't look at the NameID in the LogoutRequest, which
+           is not quite correct, as a LogoutRequest to logout user X can be delivered
+           through any means, and doesn't need a session to be valid.
+           E.g. a backchannel logout request sent by the IdP wouldn't have a session cookie.
+           Also, hypothetically, a LogoutRequest to logout user Y shouldn't logout user X, even if the
+           request is made with a session cookie belonging to user X.
+        2. We want to revoke all sessions for the user, not just the current session
+           of the request, so after validating the LogoutRequest, we need to identify
+           the user by the NameID, do some validation and then revoke all sessions.
+
+        TODO: This does not return a LogoutResponse in case of failure, like the spec requires.
+        https://github.com/zulip/zulip/issues/20076 is the related issue with more detail
+        on how to implement the desired behavior.
+        """
+        idp = self.get_idp(idp_name)
+        auth = self._create_saml_auth(idp)
+        # This validates the LogoutRequest and prepares the response
+        # (the URL to which to redirect the client to convey the response to the IdP)
+        # but is a no-op otherwise because keep_local_session=True keeps it from
+        # doing anything else. We want to take care of revoking session on our own.
+        url = auth.process_slo(keep_local_session=True)
+        errors = auth.get_errors()
+        if errors:
+            self.logger.info("/complete/saml/: LogoutRequest failed: %s", errors)
+            return None
+
+        logout_request_xml = auth.get_last_request_xml()
+        name_id = OneLogin_Saml2_Logout_Request.get_nameid(logout_request_xml)
+        try:
+            validate_email(name_id)
+        except ValidationError:
+            self.logger.info(
+                "/complete/saml/: LogoutRequest failed: NameID is not a valid email address: %s",
+                name_id,
+            )
+            return None
+
+        return_data: Dict[str, Any] = {}
+
+        realm = get_realm(subdomain)
+        user_profile = common_get_active_user(name_id, realm, return_data)
+        if user_profile is None:
+            self.logger.info(
+                "/complete/saml/: LogoutRequest failed: No user with email specified in NameID found in realm %s. return_data=%s",
+                realm.id,
+                return_data,
+            )
+            return None
+
+        self.logger.info(
+            "/complete/saml/: LogoutRequest triggered deletion of all session for user %s",
+            user_profile.id,
+        )
+        delete_user_sessions(user_profile)
+        do_regenerate_api_key(user_profile, user_profile)
+
+        return HttpResponseRedirect(url)
+
     def auth_complete(self, *args: Any, **kwargs: Any) -> Optional[HttpResponse]:
         """
         Additional ugly wrapping on top of auth_complete in SocialAuthMixin.
-        We handle two things here:
+        We handle two things for processing SAMLResponses here:
             1. Working around bad RelayState or SAMLResponse parameters in the request.
             Both parameters should be present if the user came to /complete/saml/ through
             the IdP as intended. The errors can happen if someone simply types the endpoint into
@@ -2370,26 +2451,35 @@ class SAMLAuthBackend(SocialAuthMixin, SAMLAuth):
             into the RelayState. We need to read them and set those values in the session,
             and then change the RelayState param to the idp_name, because that's what
             SAMLAuth.auth_complete() expects.
+
+        Additionally, this handles incoming LogoutRequests for IdP-initated logout.
         """
+
+        SAMLRequest = self.strategy.request_data().get("SAMLRequest")
         SAMLResponse = self.strategy.request_data().get("SAMLResponse")
-        if SAMLResponse is None:
-            self.logger.info("/complete/saml/: No SAMLResponse in request.")
+        if SAMLResponse is None and SAMLRequest is None:
+            self.logger.info("/complete/saml/: No SAMLResponse or SAMLRequest in request.")
             return None
+        elif SAMLRequest is not None:
+            saml_response_or_request = (SAMLRequest, "SAMLRequest")
+        elif SAMLResponse is not None:
+            saml_response_or_request = (SAMLResponse, "SAMLResponse")
 
         relayed_params = self.get_relayed_params()
 
         subdomain = self.choose_subdomain(relayed_params)
         if subdomain is None:
             error_msg = (
-                "/complete/saml/: Can't figure out subdomain for this authentication request. "
-                + "relayed_params: %s"
+                "/complete/saml/: Can't figure out subdomain for this %s. " + "relayed_params: %s"
             )
-            self.logger.info(error_msg, relayed_params)
+            self.logger.info(error_msg, saml_response_or_request[1], relayed_params)
             return None
 
-        idp_name = self.get_issuing_idp(SAMLResponse)
+        idp_name = self.get_issuing_idp(saml_response_or_request)
         if idp_name is None:
-            self.logger.info("/complete/saml/: No valid IdP as issuer of the SAMLResponse.")
+            self.logger.info(
+                "/complete/saml/: No valid IdP as issuer of the %s.", saml_response_or_request[1]
+            )
             return None
 
         idp_valid = self.validate_idp_for_subdomain(idp_name, subdomain)
@@ -2401,6 +2491,9 @@ class SAMLAuthBackend(SocialAuthMixin, SAMLAuth):
             self.logger.info(error_msg, idp_name, subdomain)
             return None
 
+        if saml_response_or_request[1] == "SAMLRequest":
+            return self.process_logout(subdomain, idp_name)
+
         result = None
         try:
             params = relayed_params.copy()

zproject/computed_settings.py

@@ -1129,6 +1129,13 @@ if "signatureAlgorithm" not in SOCIAL_AUTH_SAML_SECURITY_CONFIG:
     default_signature_alg = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"
     SOCIAL_AUTH_SAML_SECURITY_CONFIG["signatureAlgorithm"] = default_signature_alg
 
+if "wantMessagesSigned" not in SOCIAL_AUTH_SAML_SECURITY_CONFIG:
+    # This setting controls whether LogoutRequests delivered to us
+    # need to be signed. The default of False is not acceptable,
+    # because we don't want anyone to be able to submit a request
+    # to get other users logged out.
+    SOCIAL_AUTH_SAML_SECURITY_CONFIG["wantMessagesSigned"] = True
+
 for idp_name, idp_dict in SOCIAL_AUTH_SAML_ENABLED_IDPS.items():
     if DEVELOPMENT:
         idp_dict["entity_id"] = get_secret("saml_entity_id", "")

zproject/test_extra_settings.py

@@ -250,6 +250,7 @@ SOCIAL_AUTH_SAML_ENABLED_IDPS: Dict[str, SAMLIdPConfigDict] = {
     "test_idp": {
         "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",
         "x509cert": get_from_file_if_exists("zerver/tests/fixtures/saml/idp.crt"),
         "attr_user_permanent_id": "email",
         "attr_first_name": "first_name",