saml: Support IdP-initiated SSO.

docs/production/authentication-methods.md

@@ -135,6 +135,24 @@ found at `https://yourzulipdomain.example.com/saml/metadata.xml`. You
 can use this for verifying your configuration or provide it to your
 IdP.
 
+### IdP-initiated SSO
+
+The above configuration is sufficient for Service Provider initialized
+SSO, i.e. you can visit the Zulip webapp and click "Sign in with
+{IdP}" and it'll correctly start the authentication flow.  If you are
+not hosting multiple organizations, with Zulip 2.2+, the above
+configuration is also sufficient for Identity Provider initiated SSO,
+i.e. clicking a "Sign in to Zulip" button on the IdP's website can
+correctly authenticate the user to Zulip.
+
+If you're hosting multiple organizations and thus using the
+`SOCIAL_AUTH_SUBDOMAIN` setting, you'll need to configure a custom
+`RelayState` in your IdP of the form `{"subdomain":
+"yourzuliporganization"}` to let Zulip know which organization to
+authenticate the user to when they visit your SSO URL from the IdP.
+(If the organization is on the root domain, use the empty string:
+`{"subdomain": ""}`.).
+
 ```eval_rst
 .. _ldap:
 ```

zerver/tests/test_auth_backends.py

@@ -1424,7 +1424,7 @@ class SAMLAuthBackendTest(SocialAuthBase):
         parsed_url = urllib.parse.urlparse(result.url)
         relay_state = urllib.parse.parse_qs(parsed_url.query)['RelayState'][0]
         # Make sure params are getting encoded into RelayState:
-        data = SAMLAuthBackend.get_data_from_redis(relay_state)
+        data = SAMLAuthBackend.get_data_from_redis(ujson.loads(relay_state)['state_token'])
         if next:
             self.assertEqual(data['next'], next)
         if is_signup:
@@ -1512,7 +1512,9 @@ class SAMLAuthBackendTest(SocialAuthBase):
                     mock.patch('zproject.backends.logging.info') as m:
                 # This mock causes AuthFailed to be raised.
                 saml_response = self.generate_saml_response(self.email, self.name)
-                relay_state = SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
+                relay_state = ujson.dumps(dict(
+                    state_token=SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
+                ))
                 post_params = {"SAMLResponse": saml_response, "RelayState": relay_state}
                 result = self.client_post('/complete/saml/',  post_params)
                 self.assertEqual(result.status_code, 302)
@@ -1525,7 +1527,9 @@ class SAMLAuthBackendTest(SocialAuthBase):
                             side_effect=AuthStateForbidden('State forbidden')), \
                     mock.patch('zproject.backends.logging.warning') as m:
                 saml_response = self.generate_saml_response(self.email, self.name)
-                relay_state = SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
+                relay_state = ujson.dumps(dict(
+                    state_token=SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
+                ))
                 post_params = {"SAMLResponse": saml_response, "RelayState": relay_state}
                 result = self.client_post('/complete/saml/',  post_params)
                 self.assertEqual(result.status_code, 302)
@@ -1540,12 +1544,14 @@ class SAMLAuthBackendTest(SocialAuthBase):
             result = self.client_get('/complete/saml/')
             self.assertEqual(result.status_code, 302)
             self.assertIn('login', result.url)
-            m.assert_called_with("SAML authentication failed: missing RelayState.")
+            m.assert_called_with("/complete/saml/: No SAMLResponse in request.")
 
         # Check that POSTing the RelayState, but with missing SAMLResponse,
         # doesn't cause errors either:
         with mock.patch('zproject.backends.logging.info') as m:
-            relay_state = SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
+            relay_state = ujson.dumps(dict(
+                state_token=SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
+            ))
             post_params = {"RelayState": relay_state}
             result = self.client_post('/complete/saml/',  post_params)
             self.assertEqual(result.status_code, 302)
@@ -1554,7 +1560,9 @@ class SAMLAuthBackendTest(SocialAuthBase):
 
         # Now test bad SAMLResponses.
         with mock.patch('zproject.backends.logging.info') as m:
-            relay_state = SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
+            relay_state = ujson.dumps(dict(
+                state_token=SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
+            ))
             post_params = {"RelayState": relay_state, 'SAMLResponse': ''}
             result = self.client_post('/complete/saml/',  post_params)
             self.assertEqual(result.status_code, 302)
@@ -1562,7 +1570,9 @@ class SAMLAuthBackendTest(SocialAuthBase):
             m.assert_called()
 
         with mock.patch('zproject.backends.logging.info') as m:
-            relay_state = SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
+            relay_state = ujson.dumps(dict(
+                state_token=SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
+            ))
             post_params = {"RelayState": relay_state, 'SAMLResponse': 'b'}
             result = self.client_post('/complete/saml/',  post_params)
             self.assertEqual(result.status_code, 302)
@@ -1570,36 +1580,31 @@ class SAMLAuthBackendTest(SocialAuthBase):
             m.assert_called()
 
         with mock.patch('zproject.backends.logging.info') as m:
-            relay_state = SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
+            relay_state = ujson.dumps(dict(
+                state_token=SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
+            ))
             post_params = {"RelayState": relay_state, 'SAMLResponse': 'dGVzdA=='}  # base64 encoded 'test'
             result = self.client_post('/complete/saml/',  post_params)
             self.assertEqual(result.status_code, 302)
             self.assertIn('login', result.url)
             m.assert_called()
 
-        with mock.patch('zproject.backends.logging.info') as m:
-            relay_state = SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
-            relay_state = relay_state[:-1]  # Break the token by removing the last character
-            post_params = {"RelayState": relay_state}
-            result = self.client_post('/complete/saml/',  post_params)
-            self.assertEqual(result.status_code, 302)
-            self.assertIn('login', result.url)
-            m.assert_called_with("SAML authentication failed: bad RelayState token.")
-
     def test_social_auth_complete_no_subdomain(self) -> None:
         with mock.patch('zproject.backends.logging.info') as m:
-            relay_state = SAMLAuthBackend.put_data_in_redis({})
+            post_params = {"RelayState": '',
-            post_params = {"RelayState": relay_state,
                            'SAMLResponse': self.generate_saml_response(email=self.example_email("hamlet"),
                                                                        name="King Hamlet")}
+            with mock.patch.object(SAMLAuthBackend, 'choose_subdomain', return_value=None):
                 result = self.client_post('/complete/saml/',  post_params)
             self.assertEqual(result.status_code, 302)
             self.assertEqual('/login/', result.url)
-            self.assertIn("/complete/saml/: Missing subdomain value in relayed_params.",
+            self.assertIn("/complete/saml/: Can't figure out subdomain for this authentication request",
                           m.call_args.args[0])
 
     def test_social_auth_complete_wrong_issuing_idp(self) -> None:
-        relay_state = SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
+        relay_state = ujson.dumps(dict(
+            state_token=SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
+        ))
         saml_response = self.generate_saml_response(email=self.example_email("hamlet"),
                                                     name="King Hamlet")
 
@@ -1626,7 +1631,9 @@ class SAMLAuthBackendTest(SocialAuthBase):
 
         with mock.patch('zproject.backends.logging.info') as m, \
                 mock.patch.object(SAMLAuthBackend, 'get_issuing_idp', return_value='test_idp'):
-            relay_state = SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
+            relay_state = ujson.dumps(dict(
+                state_token=SAMLAuthBackend.put_data_in_redis({"subdomain": "zulip"})
+            ))
             post_params = {"RelayState": relay_state, 'SAMLResponse': 'dGVzdA=='}
             result = self.client_post('/complete/saml/',  post_params)
             self.assertEqual(result.status_code, 302)
@@ -1720,7 +1727,8 @@ class SAMLAuthBackendTest(SocialAuthBase):
         self.assertEqual(result.status_code, 302)
         self.assertEqual('/login/', result.url)
         m.assert_called_with(
-            "User authenticated with IdP %s but this provider is not enabled for this realm %s.",
+            '/complete/saml/: Authentication request with IdP %s but this provider is not enabled ' +
+            'for this subdomain %s.',
             "test_idp", "zephyr",
         )
 
@@ -1761,6 +1769,104 @@ class SAMLAuthBackendTest(SocialAuthBase):
                 "IdPs don't have limit_to_subdomains specified and will be ignored: " +
                 "['test_idp']")
 
+    def test_idp_initiated_signin_subdomain_specified(self) -> None:
+        post_params = {
+            "RelayState": '{"subdomain": "zulip"}',
+            "SAMLResponse": self.generate_saml_response(email=self.email, name=self.name)
+        }
+
+        with mock.patch.object(OneLogin_Saml2_Response, 'is_valid', return_value=True):
+            # We're not able to generate valid signatures in tests, so we need the mock.
+            result = self.client_post('/complete/saml/',  post_params)
+
+        data = load_subdomain_token(result)
+        self.assertEqual(data['email'], self.example_email("hamlet"))
+        self.assertEqual(data['full_name'], self.name)
+        self.assertEqual(data['subdomain'], 'zulip')
+        self.assertEqual(result.status_code, 302)
+        parsed_url = urllib.parse.urlparse(result.url)
+        uri = "{}://{}{}".format(parsed_url.scheme, parsed_url.netloc,
+                                 parsed_url.path)
+        self.assertTrue(uri.startswith('http://zulip.testserver/accounts/login/subdomain/'))
+
+        self.client_get(uri)
+        self.assert_logged_in_user_id(self.example_user("hamlet").id)
+
+    def test_choose_subdomain_invalid_subdomain_specified(self) -> None:
+        post_params = {
+            "RelayState": '{"subdomain": "invalid"}',
+            "SAMLResponse": self.generate_saml_response(email=self.email, name=self.name)
+        }
+
+        with mock.patch.object(OneLogin_Saml2_Response, 'is_valid', return_value=True):
+            # We're not able to generate valid signatures in tests, so we need the mock.
+            result = self.client_post('/complete/saml/',  post_params)
+
+        self.assertEqual(result.status_code, 302)
+        self.assertEqual(result.url, "/accounts/find/")
+
+    def test_idp_initiated_signin_subdomain_implicit(self) -> None:
+        post_params = {
+            "RelayState": '',
+            "SAMLResponse": self.generate_saml_response(email=self.email, name=self.name)
+        }
+
+        with mock.patch.object(OneLogin_Saml2_Response, 'is_valid', return_value=True):
+            # We're not able to generate valid signatures in tests, so we need the mock.
+            result = self.client_post('http://zulip.testserver/complete/saml/',  post_params)
+
+        data = load_subdomain_token(result)
+        self.assertEqual(data['email'], self.example_email("hamlet"))
+        self.assertEqual(data['full_name'], self.name)
+        self.assertEqual(data['subdomain'], 'zulip')
+        self.assertEqual(result.status_code, 302)
+        parsed_url = urllib.parse.urlparse(result.url)
+        uri = "{}://{}{}".format(parsed_url.scheme, parsed_url.netloc,
+                                 parsed_url.path)
+        self.assertTrue(uri.startswith('http://zulip.testserver/accounts/login/subdomain/'))
+
+        self.client_get(uri)
+        self.assert_logged_in_user_id(self.example_user("hamlet").id)
+
+    def test_idp_initiated_signin_subdomain_implicit_no_relaystate_param(self) -> None:
+        post_params = {
+            "SAMLResponse": self.generate_saml_response(email=self.email, name=self.name)
+        }
+
+        with mock.patch.object(OneLogin_Saml2_Response, 'is_valid', return_value=True):
+            # We're not able to generate valid signatures in tests, so we need the mock.
+            result = self.client_post('http://zulip.testserver/complete/saml/',  post_params)
+
+        data = load_subdomain_token(result)
+        self.assertEqual(data['email'], self.example_email("hamlet"))
+        self.assertEqual(data['full_name'], self.name)
+        self.assertEqual(data['subdomain'], 'zulip')
+        self.assertEqual(result.status_code, 302)
+        parsed_url = urllib.parse.urlparse(result.url)
+        uri = "{}://{}{}".format(parsed_url.scheme, parsed_url.netloc,
+                                 parsed_url.path)
+        self.assertTrue(uri.startswith('http://zulip.testserver/accounts/login/subdomain/'))
+
+        self.client_get(uri)
+        self.assert_logged_in_user_id(self.example_user("hamlet").id)
+
+    def test_idp_initiated_signin_subdomain_implicit_invalid(self) -> None:
+        post_params = {
+            "RelayState": '',
+            "SAMLResponse": self.generate_saml_response(email=self.email, name=self.name)
+        }
+
+        with mock.patch("logging.info") as m:
+            with mock.patch('zproject.backends.get_subdomain', return_value='invalid'):
+                # Due to the quirks of our test setup, get_subdomain on all these `some_subdomain.testserver`
+                # requests returns 'zulip', so we need to mock it here.
+                result = self.client_post('http://invalid.testserver/complete/saml/',  post_params)
+
+            self.assertEqual(result.status_code, 302)
+            self.assertEqual('/login/', result.url)
+            self.assertIn("/complete/saml/: Can't figure out subdomain for this authentication request",
+                          m.call_args.args[0])
+
 class GitHubAuthBackendTest(SocialAuthBase):
     __unittest_skip__ = False
 

zproject/backends.py

@@ -16,6 +16,7 @@ import binascii
 import copy
 import logging
 import magic
+import ujson
 from abc import ABC, abstractmethod
 from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Type, TypeVar, Union, \
     cast
@@ -64,6 +65,7 @@ from zerver.lib.rate_limiter import RateLimitedObject
 from zerver.lib.request import JsonableError
 from zerver.lib.users import check_full_name, validate_user_custom_profile_field
 from zerver.lib.redis_utils import get_redis_client, get_dict_from_redis, put_dict_in_redis
+from zerver.lib.subdomains import get_subdomain
 from zerver.models import CustomProfileField, DisposableEmailError, DomainNotAllowedForRealmError, \
     EmailContainsPlusError, PreregistrationUser, UserProfile, Realm, custom_profile_fields_for_realm, \
     get_user_profile_by_id, remote_user_to_email, \
@@ -1549,7 +1551,7 @@ class SAMLAuthBackend(SocialAuthMixin, SAMLAuth):
         data_to_relay = {
             key: request_data[key] for key in params_to_relay if key in request_data
         }
-        relay_state = self.put_data_in_redis(data_to_relay)
+        relay_state = ujson.dumps({"state_token": self.put_data_in_redis(data_to_relay)})
 
         return auth.login(return_to=relay_state)
 
@@ -1565,10 +1567,6 @@ class SAMLAuthBackend(SocialAuthMixin, SAMLAuth):
         if key.startswith('saml_token_'):
             # Safety if statement, to not allow someone to poke around arbitrary redis keys here.
             data = get_dict_from_redis(redis_client, "saml_token_{token}", key)
-        if data is None:
-            # TODO: We will need some sort of user-facing message
-            # about the authentication session having expired here.
-            logging.info("SAML authentication failed: bad RelayState token.")
 
         return data
 
@@ -1596,6 +1594,46 @@ class SAMLAuthBackend(SocialAuthMixin, SAMLAuth):
 
         return None
 
+    def get_relayed_params(self) -> Dict[str, Any]:
+        request_data = self.strategy.request_data()
+        if 'RelayState' not in request_data:
+            return {}
+
+        relay_state = request_data['RelayState']
+        try:
+            data = ujson.loads(relay_state)
+            if 'state_token' in data:
+                # SP-initiated sign in. We stored relevant information in the first
+                # step of the flow
+                return self.get_data_from_redis(data['state_token']) or {}
+            else:
+                # IdP-initiated sign in. Right now we only support transporting subdomain through json in
+                # RelayState, but this format is nice in that it allows easy extensibility here.
+                return {'subdomain': data.get('subdomain')}
+        except (ValueError, TypeError):
+            return {}
+
+    def choose_subdomain(self, relayed_params: Dict[str, Any]) -> Optional[str]:
+        subdomain = relayed_params.get("subdomain")
+        if subdomain is not None:
+            return subdomain
+
+        # If not specified otherwise, the intended subdomain for this
+        # authentication attempt is the subdomain of the request.
+        request_subdomain = get_subdomain(self.strategy.request)
+        try:
+            # We only want to do a basic sanity-check here for whether
+            # this subdomain has a realm one could try to authenticate
+            # to.  True validation of whether the realm is active, the
+            # IdP is appropriate for the subdomain, etc. happens
+            # elsewhere in the flow and we shouldn't duplicate such
+            # logic here.
+            get_realm(request_subdomain)
+        except Realm.DoesNotExist:
+            return None
+        else:
+            return request_subdomain
+
     def auth_complete(self, *args: Any, **kwargs: Any) -> Optional[HttpResponse]:
         """
         Additional ugly wrapping on top of auth_complete in SocialAuthMixin.
@@ -1610,41 +1648,37 @@ class SAMLAuthBackend(SocialAuthMixin, SAMLAuth):
             and then change the RelayState param to the idp_name, because that's what
             SAMLAuth.auth_complete() expects.
         """
-        if 'RelayState' not in self.strategy.request_data():
-            logging.info("SAML authentication failed: missing RelayState.")
-            return None
-
-        # Set the relevant params that we transported in the RelayState:
-        redis_key = self.strategy.request_data()['RelayState']
-        relayed_params = self.get_data_from_redis(redis_key)
-        if relayed_params is None:
-            return None
-
         SAMLResponse = self.strategy.request_data().get('SAMLResponse')
         if SAMLResponse is None:
             logging.info("/complete/saml/: No SAMLResponse in request.")
             return None
 
+        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"
+            logging.info(error_msg, relayed_params)
+            return None
+
         idp_name = self.get_issuing_idp(SAMLResponse)
         if idp_name is None:
             logging.info("/complete/saml/: No valid IdP as issuer of the SAMLResponse.")
             return None
 
-        subdomain = relayed_params.get("subdomain")
-        if subdomain is None:
-            logging.info("/complete/saml/: Missing subdomain value in relayed_params.")
-            return None
-
         idp_valid = self.validate_idp_for_subdomain(idp_name, subdomain)
         if not idp_valid:
-            error_msg = "User authenticated with IdP %s but this provider is not " + \
+            error_msg = "/complete/saml/: Authentication request with IdP %s but this provider is not " + \
-                        "enabled for this realm %s."
+                        "enabled for this subdomain %s."
             logging.info(error_msg, idp_name, subdomain)
             return None
 
         result = None
         try:
-            for param, value in relayed_params.items():
+            params = relayed_params.copy()
+            params['subdomain'] = subdomain
+            for param, value in params.items():
                 if param in self.standard_relay_params:
                     self.strategy.session_set(param, value)