From c2ab0b30664439d09436a39e5448728c67b0414c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 26 Mar 2020 18:58:58 +0100 Subject: [PATCH 1/5] Whitelist the login fallback by default for SSO --- synapse/config/sso.py | 17 ++++++++++++++++- tests/rest/client/v1/test_login.py | 13 ++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/synapse/config/sso.py b/synapse/config/sso.py index 95762689b..5ae9db83d 100644 --- a/synapse/config/sso.py +++ b/synapse/config/sso.py @@ -39,6 +39,17 @@ class SSOConfig(Config): self.sso_client_whitelist = sso_config.get("client_whitelist") or [] + # Attempt to also whitelist the server's login fallback, since that fallback sets + # the redirect URL to itself (so it can process the login token then return + # gracefully to the client). This would make it pointless to ask the user for + # confirmation, since the URL the confirmation page would be showing wouldn't be + # the client's. + # public_baseurl is an optional setting, so we only add the fallback's URL to the + # list if it's provided (because we can't figure out what that URL is otherwise). + if self.public_baseurl: + login_fallback_url = self.public_baseurl + "_matrix/static/client/login" + self.sso_client_whitelist.append(login_fallback_url) + def generate_config_section(self, **kwargs): return """\ # Additional settings to use with single-sign on systems such as SAML2 and CAS. @@ -54,7 +65,11 @@ class SSOConfig(Config): # phishing attacks from evil.site. To avoid this, include a slash after the # hostname: "https://my.client/". # - # By default, this list is empty. + # If public_baseurl is set, then the login fallback page (used by clients + # that don't have full support for SSO) is always included in this list. + # + # By default, this list is empty, except if public_baseurl is set (in which + # case the login fallback page is the only element in the list). # #client_whitelist: # - https://riot.im/develop diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/v1/test_login.py index da2c9bfa1..ed02ff1dc 100644 --- a/tests/rest/client/v1/test_login.py +++ b/tests/rest/client/v1/test_login.py @@ -350,7 +350,18 @@ class CASRedirectConfirmTestCase(unittest.HomeserverTestCase): 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/" + self._test_redirect("https://legit-site.com/") + + @override_config( + { + "public_baseurl": "https://example.com" + } + ) + def test_cas_redirect_login_fallback(self): + self._test_redirect("https://example.com/_matrix/static/client/login") + + def _test_redirect(self, redirect_url): + """Tests that the SSO login flow serves a redirect for the given redirect URL.""" cas_ticket_url = ( "/_matrix/client/r0/login/cas/ticket?redirectUrl=%s&ticket=ticket" % (urllib.parse.quote(redirect_url)) From 7083147961d7bc45ce49047c4878da2bf5202f79 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 26 Mar 2020 19:01:54 +0100 Subject: [PATCH 2/5] Regenerate sample config --- docs/sample_config.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 2ff0dd05a..556d4419f 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1392,7 +1392,11 @@ sso: # phishing attacks from evil.site. To avoid this, include a slash after the # hostname: "https://my.client/". # - # By default, this list is empty. + # If public_baseurl is set, then the login fallback page (used by clients + # that don't have full support for SSO) is always included in this list. + # + # By default, this list is empty, except if public_baseurl is set (in which + # case the login fallback page is the only element in the list). # #client_whitelist: # - https://riot.im/develop From 48b37f61ce17f5a667a9c8b7c6ad1d6abb7fdae8 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 26 Mar 2020 19:02:59 +0100 Subject: [PATCH 3/5] Changelog --- changelog.d/7153.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/7153.feature diff --git a/changelog.d/7153.feature b/changelog.d/7153.feature new file mode 100644 index 000000000..414ebe1f6 --- /dev/null +++ b/changelog.d/7153.feature @@ -0,0 +1 @@ +Always whitelist the login fallback in the SSO configuration if `public_baseurl` is set. From bdf3cdaec800966a493998c65665ee951b3d39f5 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 26 Mar 2020 19:06:44 +0100 Subject: [PATCH 4/5] Lint --- tests/rest/client/v1/test_login.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/v1/test_login.py index ed02ff1dc..aed8853d6 100644 --- a/tests/rest/client/v1/test_login.py +++ b/tests/rest/client/v1/test_login.py @@ -352,11 +352,7 @@ class CASRedirectConfirmTestCase(unittest.HomeserverTestCase): """ self._test_redirect("https://legit-site.com/") - @override_config( - { - "public_baseurl": "https://example.com" - } - ) + @override_config({"public_baseurl": "https://example.com"}) def test_cas_redirect_login_fallback(self): self._test_redirect("https://example.com/_matrix/static/client/login") From 63aea691a761a9b6a2058b54792fc5859e12cfba Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 27 Mar 2020 15:09:12 +0100 Subject: [PATCH 5/5] Update the wording of the config comment --- docs/sample_config.yaml | 6 +++--- synapse/config/sso.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 556d4419f..07e922dc2 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1393,10 +1393,10 @@ sso: # hostname: "https://my.client/". # # If public_baseurl is set, then the login fallback page (used by clients - # that don't have full support for SSO) is always included in this list. + # that don't natively support the required login flows) is whitelisted in + # addition to any URLs in this list. # - # By default, this list is empty, except if public_baseurl is set (in which - # case the login fallback page is the only element in the list). + # By default, this list is empty. # #client_whitelist: # - https://riot.im/develop diff --git a/synapse/config/sso.py b/synapse/config/sso.py index 5ae9db83d..ec3dca9ef 100644 --- a/synapse/config/sso.py +++ b/synapse/config/sso.py @@ -66,10 +66,10 @@ class SSOConfig(Config): # hostname: "https://my.client/". # # If public_baseurl is set, then the login fallback page (used by clients - # that don't have full support for SSO) is always included in this list. + # that don't natively support the required login flows) is whitelisted in + # addition to any URLs in this list. # - # By default, this list is empty, except if public_baseurl is set (in which - # case the login fallback page is the only element in the list). + # By default, this list is empty. # #client_whitelist: # - https://riot.im/develop