From 0122ef1037c8bfe826ea09d9fc7cd63fb9c59fd1 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 27 Mar 2020 20:21:09 +0000 Subject: [PATCH] Revert "Merge pull request #7153 from matrix-org/babolivier/sso_whitelist_login_fallback" This was incorrectly merged to master. This reverts commit 319c41f573eb14a966367b60b2e6e93bf6b028d9, reversing changes made to 229eb81498b0fe1da81e9b5b333a0285acde9446. --- changelog.d/7153.feature | 1 - docs/sample_config.yaml | 4 ---- synapse/config/sso.py | 15 --------------- tests/rest/client/v1/test_login.py | 9 +-------- 4 files changed, 1 insertion(+), 28 deletions(-) delete mode 100644 changelog.d/7153.feature diff --git a/changelog.d/7153.feature b/changelog.d/7153.feature deleted file mode 100644 index 414ebe1f6..000000000 --- a/changelog.d/7153.feature +++ /dev/null @@ -1 +0,0 @@ -Always whitelist the login fallback in the SSO configuration if `public_baseurl` is set. diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 07e922dc2..2ff0dd05a 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1392,10 +1392,6 @@ sso: # phishing attacks from evil.site. To avoid this, include a slash after the # hostname: "https://my.client/". # - # If public_baseurl is set, then the login fallback page (used by clients - # 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. # #client_whitelist: diff --git a/synapse/config/sso.py b/synapse/config/sso.py index ec3dca9ef..95762689b 100644 --- a/synapse/config/sso.py +++ b/synapse/config/sso.py @@ -39,17 +39,6 @@ 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. @@ -65,10 +54,6 @@ class SSOConfig(Config): # phishing attacks from evil.site. To avoid this, include a slash after the # hostname: "https://my.client/". # - # If public_baseurl is set, then the login fallback page (used by clients - # 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. # #client_whitelist: diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/v1/test_login.py index aed8853d6..da2c9bfa1 100644 --- a/tests/rest/client/v1/test_login.py +++ b/tests/rest/client/v1/test_login.py @@ -350,14 +350,7 @@ class CASRedirectConfirmTestCase(unittest.HomeserverTestCase): def test_cas_redirect_whitelisted(self): """Tests that the SSO login flow serves a redirect to a whitelisted url """ - 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.""" + redirect_url = "https://legit-site.com/" cas_ticket_url = ( "/_matrix/client/r0/login/cas/ticket?redirectUrl=%s&ticket=ticket" % (urllib.parse.quote(redirect_url))