Merge branch 'rav/sso-confirm-whitelist' into 'release-v1.11.1'

Add a whitelist for the SSO confirmation step.

See merge request new-vector/synapse!3

4 files changed, 84 insertions, 14 deletions

diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml
index bbb8a4d934..f719ec696f 100644
--- a/docs/sample_config.yaml
+++ b/docs/sample_config.yaml
@@ -1363,6 +1363,22 @@ saml2_config:
 # Additional settings to use with single-sign on systems such as SAML2 and CAS.
 #
 sso:
+    # A list of client URLs which are whitelisted so that the user does not
+    # have to confirm giving access to their account to the URL. Any client
+    # whose URL starts with an entry in the following list will not be subject
+    # to an additional confirmation step after the SSO login is completed.
+    #
+    # WARNING: An entry such as "https://my.client" is insecure, because it
+    # will also match "https://my.client.evil.site", exposing your users to
+    # phishing attacks from evil.site. To avoid this, include a slash after the
+    # hostname: "https://my.client/".
+    #
+    # By default, this list is empty.
+    #
+    #client_whitelist:
+    #  - https://riot.im/develop
+    #  - https://my.custom.client/
+
     # Directory in which Synapse will try to find the template files below.
     # If not set, default templates from within the Synapse package will be used.
     #
@@ -1372,8 +1388,8 @@ sso:
     #
     # Synapse will look for the following templates in this directory:
     #
-    # * HTML page for confirmation of redirect during authentication:
-    #   'sso_redirect_confirm.html'.
+    # * HTML page for a confirmation step before redirecting back to the client
+    #   with the login token: 'sso_redirect_confirm.html'.
     #
     #   When rendering, this template is given three variables:
     #     * redirect_url: the URL the user is about to be redirected to. Needs
@@ -1381,7 +1397,7 @@ sso:
     #                     https://jinja.palletsprojects.com/en/2.11.x/templates/#html-escaping).
     #
     #     * display_url: the same as `redirect_url`, but with the query
-    #                    parameters stripped. The intention is to have a 
+    #                    parameters stripped. The intention is to have a
     #                    human-readable URL to show to users, not to use it as
     #                    the final address to redirect to. Needs manual escaping
     #                    (see https://jinja.palletsprojects.com/en/2.11.x/templates/#html-escaping).
diff --git a/synapse/config/sso.py b/synapse/config/sso.py
index f426b65b4f..56299bd4e4 100644
--- a/synapse/config/sso.py
+++ b/synapse/config/sso.py
@@ -37,11 +37,29 @@ class SSOConfig(Config):
 
         self.sso_redirect_confirm_template_dir = template_dir
 
+        self.sso_client_whitelist = sso_config.get("client_whitelist") or []
+
     def generate_config_section(self, **kwargs):
         return """\
         # Additional settings to use with single-sign on systems such as SAML2 and CAS.
         #
         sso:
+            # A list of client URLs which are whitelisted so that the user does not
+            # have to confirm giving access to their account to the URL. Any client
+            # whose URL starts with an entry in the following list will not be subject
+            # to an additional confirmation step after the SSO login is completed.
+            #
+            # WARNING: An entry such as "https://my.client" is insecure, because it
+            # will also match "https://my.client.evil.site", exposing your users to
+            # phishing attacks from evil.site. To avoid this, include a slash after the
+            # hostname: "https://my.client/".
+            #
+            # By default, this list is empty.
+            #
+            #client_whitelist:
+            #  - https://riot.im/develop
+            #  - https://my.custom.client/
+
             # Directory in which Synapse will try to find the template files below.
             # If not set, default templates from within the Synapse package will be used.
             #
diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py
index 1acfd01d8e..b2bc7537db 100644
--- a/synapse/rest/client/v1/login.py
+++ b/synapse/rest/client/v1/login.py
@@ -556,6 +556,9 @@ class SSOAuthHandler(object):
 
         self._server_name = hs.config.server_name
 
+        # cast to tuple for use with str.startswith
+        self._whitelisted_sso_clients = tuple(hs.config.sso_client_whitelist)
+
     async def on_successful_auth(
         self, username, request, client_redirect_url, user_display_name=None
     ):
@@ -605,11 +608,6 @@ class SSOAuthHandler(object):
             registered_user_id
         )
 
-        # Remove the query parameters from the redirect URL to get a shorter version of
-        # it. This is only to display a human-readable URL in the template, but not the
-        # URL we redirect users to.
-        redirect_url_no_params = client_redirect_url.split("?")[0]
-
         # Append the login token to the original redirect URL (i.e. with its query
         # parameters kept intact) to build the URL to which the template needs to
         # redirect the users once they have clicked on the confirmation link.
@@ -617,17 +615,29 @@ class SSOAuthHandler(object):
             client_redirect_url, "loginToken", login_token
         )
 
-        # Serve the redirect confirmation page
+        # if the client is whitelisted, we can redirect straight to it
+        if client_redirect_url.startswith(self._whitelisted_sso_clients):
+            request.redirect(redirect_url)
+            finish_request(request)
+            return
+
+        # Otherwise, serve the redirect confirmation page.
+
+        # Remove the query parameters from the redirect URL to get a shorter version of
+        # it. This is only to display a human-readable URL in the template, but not the
+        # URL we redirect users to.
+        redirect_url_no_params = client_redirect_url.split("?")[0]
+
         html = self._template.render(
             display_url=redirect_url_no_params,
             redirect_url=redirect_url,
             server_name=self._server_name,
-        )
+        ).encode("utf-8")
 
         request.setResponseCode(200)
         request.setHeader(b"Content-Type", b"text/html; charset=utf-8")
         request.setHeader(b"Content-Length", b"%d" % (len(html),))
-        request.write(html.encode("utf8"))
+        request.write(html)
         finish_request(request)
 
     @staticmethod
diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/v1/test_login.py
index 2b8ad5c753..da2c9bfa1e 100644
--- a/tests/rest/client/v1/test_login.py
+++ b/tests/rest/client/v1/test_login.py
@@ -268,13 +268,11 @@ class CASRedirectConfirmTestCase(unittest.HomeserverTestCase):
         self.redirect_path = "_synapse/client/login/sso/redirect/confirm"
 
         config = self.default_config()
-        config["enable_registration"] = True
         config["cas_config"] = {
             "enabled": True,
             "server_url": "https://fake.test",
             "service_url": "https://matrix.goodserver.com:8448",
         }
-        config["public_baseurl"] = self.base_url
 
         async def get_raw(uri, args):
             """Return an example response payload from a call to the `/proxyValidate`
@@ -310,7 +308,7 @@ class CASRedirectConfirmTestCase(unittest.HomeserverTestCase):
         """Tests that the SSO login flow serves a confirmation page before redirecting a
         user to the redirect URL.
         """
-        base_url = "/login/cas/ticket?redirectUrl"
+        base_url = "/_matrix/client/r0/login/cas/ticket?redirectUrl"
         redirect_url = "https://dodgy-site.com/"
 
         url_parts = list(urllib.parse.urlparse(base_url))
@@ -325,6 +323,7 @@ class CASRedirectConfirmTestCase(unittest.HomeserverTestCase):
         self.render(request)
 
         # Test that the response is HTML.
+        self.assertEqual(channel.code, 200)
         content_type_header_value = ""
         for header in channel.result.get("headers", []):
             if header[0] == b"Content-Type":
@@ -337,3 +336,30 @@ class CASRedirectConfirmTestCase(unittest.HomeserverTestCase):
 
         # And that it contains our redirect link
         self.assertIn(redirect_url, channel.result["body"].decode("UTF-8"))
+
+    @override_config(
+        {
+            "sso": {
+                "client_whitelist": [
+                    "https://legit-site.com/",
+                    "https://other-site.com/",
+                ]
+            }
+        }
+    )
+    def test_cas_redirect_whitelisted(self):
+        """Tests that the SSO login flow serves a redirect to a whitelisted url
+        """
+        redirect_url = "https://legit-site.com/"
+        cas_ticket_url = (
+            "/_matrix/client/r0/login/cas/ticket?redirectUrl=%s&ticket=ticket"
+            % (urllib.parse.quote(redirect_url))
+        )
+
+        # Get Synapse to call the fake CAS and serve the template.
+        request, channel = self.make_request("GET", cas_ticket_url)
+        self.render(request)
+
+        self.assertEqual(channel.code, 302)
+        location_headers = channel.headers.getRawHeaders("Location")
+        self.assertEqual(location_headers[0][: len(redirect_url)], redirect_url)