Merge tag 'v1.27.0rc2' into develop

clokep · clokep · commit 2c9b4a5f16b7 · 2021-02-11T11:56:03.000-05:00
Synapse 1.27.0rc2 (2021-02-11) ============================== Features -------- - Further improvements to the user experience of registration via single sign-on. ([\#9297](#9297)) Bugfixes -------- - Fix ratelimiting introduced in v1.27.0rc1 for invites to respect the `ratelimit` flag on application services. ([\#9302](#9302)) - Do not automatically calculate `public_baseurl` since it can be wrong in some situations. Reverts behaviour introduced in v1.26.0. ([\#9313](#9313)) Improved Documentation ---------------------- - Clarify the sample configuration for changes made to the template loading code. ([\#9310](#9310))
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,7 +1,29 @@
+Synapse 1.27.0rc2 (2021-02-11)
+==============================
+
+Features
+--------
+
+- Further improvements to the user experience of registration via single sign-on. ([\#9297](https://github.com/matrix-org/synapse/issues/9297))
+
+
+Bugfixes
+--------
+
+- Fix ratelimiting introduced in v1.27.0rc1 for invites to respect the `ratelimit` flag on application services. ([\#9302](https://github.com/matrix-org/synapse/issues/9302))
+- Do not automatically calculate `public_baseurl` since it can be wrong in some situations. Reverts behaviour introduced in v1.26.0. ([\#9313](https://github.com/matrix-org/synapse/issues/9313))
+
+
+Improved Documentation
+----------------------
+
+- Clarify the sample configuration for changes made to the template loading code. ([\#9310](https://github.com/matrix-org/synapse/issues/9310))
+
+
 Synapse 1.27.0rc1 (2021-02-02)
 ==============================
 
-Note that this release includes a change in Synapse to use Redis as a cache ─ as well as a pub/sub mechanism ─ if Redis support is enabled. No action is needed by server administrators, and we do not expect resource usage of the Redis instance to change dramatically.
+Note that this release includes a change in Synapse to use Redis as a cache ─ as well as a pub/sub mechanism ─ if Redis support is enabled for workers. No action is needed by server administrators, and we do not expect resource usage of the Redis instance to change dramatically.
 
 This release also changes the callback URI for OpenID Connect (OIDC) identity providers. If your server is configured to use single sign-on via an OIDC/OAuth2 IdP, you may need to make configuration changes. Please review [UPGRADE.rst](UPGRADE.rst) for more details on these changes.
 
diff --git a/changelog.d/9297.feature b/changelog.d/9297.feature
diff --git a/changelog.d/9302.bugfix b/changelog.d/9302.bugfix
diff --git a/changelog.d/9310.doc b/changelog.d/9310.doc
diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml
@@ -74,10 +74,6 @@ pid_file: DATADIR/homeserver.pid
 # Otherwise, it should be the URL to reach Synapse's client HTTP listener (see
 # 'listeners' below).
 #
-# If this is left unset, it defaults to 'https://<server_name>/'. (Note that
-# that will not work unless you configure Synapse or a reverse-proxy to listen
-# on port 443.)
-#
 #public_baseurl: https://example.com/
 
 # Set the soft limit on the number of file descriptors synapse can use
@@ -1177,9 +1173,8 @@ account_validity:
   # send an email to the account's email address with a renewal link. By
   # default, no such emails are sent.
   #
-  # If you enable this setting, you will also need to fill out the 'email'
-  # configuration section. You should also check that 'public_baseurl' is set
-  # correctly.
+  # If you enable this setting, you will also need to fill out the 'email' and
+  # 'public_baseurl' configuration sections.
   #
   #renew_at: 1w
 
@@ -1270,7 +1265,8 @@ account_validity:
 # The identity server which we suggest that clients should use when users log
 # in on this server.
 #
-# (By default, no suggestion is made, so it is left up to the client.)
+# (By default, no suggestion is made, so it is left up to the client.
+# This setting is ignored unless public_baseurl is also set.)
 #
 #default_identity_server: https://matrix.org
 
@@ -1295,6 +1291,8 @@ account_validity:
 # by the Matrix Identity Service API specification:
 # https://matrix.org/docs/spec/identity_service/latest
 #
+# If a delegate is specified, the config option public_baseurl must also be filled out.
+#
 account_threepid_delegates:
     #email: https://example.com     # Delegate email sending to example.com
     #msisdn: http://localhost:8090  # Delegate SMS sending to this local process
@@ -1948,9 +1946,9 @@ sso:
     # phishing attacks from evil.site. To avoid this, include a slash after the
     # hostname: "https://my.client/".
     #
-    # The login fallback page (used by clients that don't natively support the
-    # required login flows) is automatically whitelisted in addition to any URLs
-    # in this list.
+    # 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.
     #
diff --git a/synapse/__init__.py b/synapse/__init__.py
@@ -48,7 +48,7 @@
 except ImportError:
     pass
 
-__version__ = "1.27.0rc1"
+__version__ = "1.27.0rc2"
 
 if bool(os.environ.get("SYNAPSE_TEST_PATCH_LOG_CONTEXTS", False)):
     # We import here so that we don't have to install a bunch of deps when
diff --git a/synapse/api/urls.py b/synapse/api/urls.py
@@ -42,6 +42,8 @@ def __init__(self, hs_config):
         """
         if hs_config.form_secret is None:
             raise ConfigError("form_secret not set in config")
+        if hs_config.public_baseurl is None:
+            raise ConfigError("public_baseurl not set in config")
 
         self._hmac_secret = hs_config.form_secret.encode("utf-8")
         self._public_baseurl = hs_config.public_baseurl
diff --git a/synapse/config/cas.py b/synapse/config/cas.py
@@ -17,7 +17,7 @@
 
 from synapse.config.sso import SsoAttributeRequirement
 
-from ._base import Config
+from ._base import Config, ConfigError
 from ._util import validate_config
 
 
@@ -35,13 +35,15 @@ def read_config(self, config, **kwargs):
 
         if self.cas_enabled:
             self.cas_server_url = cas_config["server_url"]
-            public_base_url = cas_config.get("service_url") or self.public_baseurl
-            if public_base_url[-1] != "/":
-                public_base_url += "/"
+
+            # The public baseurl is required because it is used by the redirect
+            # template.
+            public_baseurl = self.public_baseurl
+            if not public_baseurl:
+                raise ConfigError("cas_config requires a public_baseurl to be set")
+
             # TODO Update this to a _synapse URL.
-            self.cas_service_url = (
-                public_base_url + "_matrix/client/r0/login/cas/ticket"
-            )
+            self.cas_service_url = public_baseurl + "_matrix/client/r0/login/cas/ticket"
             self.cas_displayname_attribute = cas_config.get("displayname_attribute")
             required_attributes = cas_config.get("required_attributes") or {}
             self.cas_required_attributes = _parsed_required_attributes_def(
diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py
@@ -166,6 +166,11 @@ def read_config(self, config, **kwargs):
             if not self.email_notif_from:
                 missing.append("email.notif_from")
 
+            # public_baseurl is required to build password reset and validation links that
+            # will be emailed to users
+            if config.get("public_baseurl") is None:
+                missing.append("public_baseurl")
+
             if missing:
                 raise ConfigError(
                     MISSING_PASSWORD_RESET_CONFIG_ERROR % (", ".join(missing),)
@@ -264,6 +269,9 @@ def read_config(self, config, **kwargs):
             if not self.email_notif_from:
                 missing.append("email.notif_from")
 
+            if config.get("public_baseurl") is None:
+                missing.append("public_baseurl")
+
             if missing:
                 raise ConfigError(
                     "email.enable_notifs is True but required keys are missing: %s"
diff --git a/synapse/config/oidc_config.py b/synapse/config/oidc_config.py
@@ -53,7 +53,10 @@ def read_config(self, config, **kwargs):
                     "Multiple OIDC providers have the idp_id %r." % idp_id
                 )
 
-        self.oidc_callback_url = self.public_baseurl + "_synapse/client/oidc/callback"
+        public_baseurl = self.public_baseurl
+        if public_baseurl is None:
+            raise ConfigError("oidc_config requires a public_baseurl to be set")
+        self.oidc_callback_url = public_baseurl + "_synapse/client/oidc/callback"
 
     @property
     def oidc_enabled(self) -> bool:
diff --git a/synapse/config/registration.py b/synapse/config/registration.py
@@ -49,6 +49,10 @@ def __init__(self, config, synapse_config):
 
             self.startup_job_max_delta = self.period * 10.0 / 100.0
 
+        if self.renew_by_email_enabled:
+            if "public_baseurl" not in synapse_config:
+                raise ConfigError("Can't send renewal emails without 'public_baseurl'")
+
         template_dir = config.get("template_dir")
 
         if not template_dir:
@@ -105,6 +109,13 @@ def read_config(self, config, **kwargs):
         account_threepid_delegates = config.get("account_threepid_delegates") or {}
         self.account_threepid_delegate_email = account_threepid_delegates.get("email")
         self.account_threepid_delegate_msisdn = account_threepid_delegates.get("msisdn")
+        if self.account_threepid_delegate_msisdn and not self.public_baseurl:
+            raise ConfigError(
+                "The configuration option `public_baseurl` is required if "
+                "`account_threepid_delegate.msisdn` is set, such that "
+                "clients know where to submit validation tokens to. Please "
+                "configure `public_baseurl`."
+            )
 
         self.default_identity_server = config.get("default_identity_server")
         self.allow_guest_access = config.get("allow_guest_access", False)
@@ -227,9 +238,8 @@ def generate_config_section(self, generate_secrets=False, **kwargs):
           # send an email to the account's email address with a renewal link. By
           # default, no such emails are sent.
           #
-          # If you enable this setting, you will also need to fill out the 'email'
-          # configuration section. You should also check that 'public_baseurl' is set
-          # correctly.
+          # If you enable this setting, you will also need to fill out the 'email' and
+          # 'public_baseurl' configuration sections.
           #
           #renew_at: 1w
 
@@ -320,7 +330,8 @@ def generate_config_section(self, generate_secrets=False, **kwargs):
         # The identity server which we suggest that clients should use when users log
         # in on this server.
         #
-        # (By default, no suggestion is made, so it is left up to the client.)
+        # (By default, no suggestion is made, so it is left up to the client.
+        # This setting is ignored unless public_baseurl is also set.)
         #
         #default_identity_server: https://matrix.org
 
@@ -345,6 +356,8 @@ def generate_config_section(self, generate_secrets=False, **kwargs):
         # by the Matrix Identity Service API specification:
         # https://matrix.org/docs/spec/identity_service/latest
         #
+        # If a delegate is specified, the config option public_baseurl must also be filled out.
+        #
         account_threepid_delegates:
             #email: https://example.com     # Delegate email sending to example.com
             #msisdn: http://localhost:8090  # Delegate SMS sending to this local process
diff --git a/synapse/config/saml2_config.py b/synapse/config/saml2_config.py
@@ -188,6 +188,8 @@ def _default_saml_config_dict(
         import saml2
 
         public_baseurl = self.public_baseurl
+        if public_baseurl is None:
+            raise ConfigError("saml2_config requires a public_baseurl to be set")
 
         if self.saml2_grandfathered_mxid_source_attribute:
             optional_attributes.add(self.saml2_grandfathered_mxid_source_attribute)
diff --git a/synapse/config/server.py b/synapse/config/server.py
@@ -230,11 +230,7 @@ def read_config(self, config, **kwargs):
         self.print_pidfile = config.get("print_pidfile")
         self.user_agent_suffix = config.get("user_agent_suffix")
         self.use_frozen_dicts = config.get("use_frozen_dicts", False)
-        self.public_baseurl = config.get("public_baseurl") or "https://%s/" % (
-            self.server_name,
-        )
-        if self.public_baseurl[-1] != "/":
-            self.public_baseurl += "/"
+        self.public_baseurl = config.get("public_baseurl")
 
         # Whether to enable user presence.
         self.use_presence = config.get("use_presence", True)
@@ -386,6 +382,9 @@ def read_config(self, config, **kwargs):
             config_path=("federation_ip_range_blacklist",),
         )
 
+        if self.public_baseurl is not None:
+            if self.public_baseurl[-1] != "/":
+                self.public_baseurl += "/"
         self.start_pushers = config.get("start_pushers", True)
 
         # (undocumented) option for torturing the worker-mode replication a bit,
@@ -813,10 +812,6 @@ def generate_config_section(
         # Otherwise, it should be the URL to reach Synapse's client HTTP listener (see
         # 'listeners' below).
         #
-        # If this is left unset, it defaults to 'https://<server_name>/'. (Note that
-        # that will not work unless you configure Synapse or a reverse-proxy to listen
-        # on port 443.)
-        #
         #public_baseurl: https://example.com/
 
         # Set the soft limit on the number of file descriptors synapse can use
diff --git a/synapse/config/sso.py b/synapse/config/sso.py
@@ -81,8 +81,11 @@ def read_config(self, config, **kwargs):
         # 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.
-        login_fallback_url = self.public_baseurl + "_matrix/static/client/login"
-        self.sso_client_whitelist.append(login_fallback_url)
+        # 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 """\
@@ -100,9 +103,9 @@ def generate_config_section(self, **kwargs):
             # phishing attacks from evil.site. To avoid this, include a slash after the
             # hostname: "https://my.client/".
             #
-            # The login fallback page (used by clients that don't natively support the
-            # required login flows) is automatically whitelisted in addition to any URLs
-            # in this list.
+            # 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.
             #
diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
@@ -504,6 +504,10 @@ async def requestMsisdnToken(
         except RequestTimedOutError:
             raise SynapseError(500, "Timed out contacting identity server")
 
+        # It is already checked that public_baseurl is configured since this code
+        # should only be used if account_threepid_delegate_msisdn is true.
+        assert self.hs.config.public_baseurl
+
         # we need to tell the client to send the token back to us, since it doesn't
         # otherwise know where to send it, so add submit_url response parameter
         # (see also MSC2078)
diff --git a/synapse/rest/well_known.py b/synapse/rest/well_known.py
@@ -34,6 +34,10 @@ def __init__(self, hs):
         self._config = hs.config
 
     def get_well_known(self):
+        # if we don't have a public_baseurl, we can't help much here.
+        if self._config.public_baseurl is None:
+            return None
+
         result = {"m.homeserver": {"base_url": self._config.public_baseurl}}
 
         if self._config.default_identity_server:
diff --git a/synapse/util/templates.py b/synapse/util/templates.py
@@ -17,7 +17,7 @@
 
 import time
 import urllib.parse
-from typing import TYPE_CHECKING, Callable, Iterable, Union
+from typing import TYPE_CHECKING, Callable, Iterable, Optional, Union
 
 import jinja2
 
@@ -74,14 +74,23 @@ def build_jinja_env(
     return env
 
 
-def _create_mxc_to_http_filter(public_baseurl: str) -> Callable:
+def _create_mxc_to_http_filter(
+    public_baseurl: Optional[str],
+) -> Callable[[str, int, int, str], str]:
     """Create and return a jinja2 filter that converts MXC urls to HTTP
 
     Args:
         public_baseurl: The public, accessible base URL of the homeserver
     """
 
-    def mxc_to_http_filter(value, width, height, resize_method="crop"):
+    def mxc_to_http_filter(
+        value: str, width: int, height: int, resize_method: str = "crop"
+    ) -> str:
+        if not public_baseurl:
+            raise RuntimeError(
+                "public_baseurl must be set in the homeserver config to convert MXC URLs to HTTP URLs."
+            )
+
         if value[0:6] != "mxc://":
             return ""
 
diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/v1/test_login.py
@@ -680,10 +680,12 @@ def make_homeserver(self, reactor, clock):
         self.redirect_path = "_synapse/client/login/sso/redirect/confirm"
 
         config = self.default_config()
+        config["public_baseurl"] = (
+            config.get("public_baseurl") or "https://matrix.goodserver.com:8448"
+        )
         config["cas_config"] = {
             "enabled": True,
             "server_url": CAS_SERVER,
-            "service_url": "https://matrix.goodserver.com:8448",
         }
 
         cas_user_id = "username"
diff --git a/tests/rest/test_well_known.py b/tests/rest/test_well_known.py
@@ -40,3 +40,12 @@ def test_well_known(self):
                 "m.identity_server": {"base_url": "https://testis"},
             },
         )
+
+    def test_well_known_no_public_baseurl(self):
+        self.hs.config.public_baseurl = None
+
+        channel = self.make_request(
+            "GET", "/.well-known/matrix/client", shorthand=False
+        )
+
+        self.assertEqual(channel.code, 404)
diff --git a/tests/utils.py b/tests/utils.py
@@ -159,6 +159,7 @@ def default_config(name, parse=False):
         },
         "rc_3pid_validation": {"per_second": 10000, "burst_count": 10000},
         "saml2_enabled": False,
+        "public_baseurl": None,
         "default_identity_server": None,
         "key_refresh_interval": 24 * 60 * 60 * 1000,
         "old_signing_keys": {},

Original file line number	Diff line number	Diff line change
`@@ -74,10 +74,6 @@ pid_file: DATADIR/homeserver.pid`
`74`	`74`	`# Otherwise, it should be the URL to reach Synapse's client HTTP listener (see`
`75`	`75`	`# 'listeners' below).`
`76`	`76`	`#`
`77`		`-# If this is left unset, it defaults to 'https://<server_name>/'. (Note that`
`78`		`-# that will not work unless you configure Synapse or a reverse-proxy to listen`
`79`		`-# on port 443.)`
`80`		`-#`
`81`	`77`	`#public_baseurl: https://example.com/`
`82`	`78`
`83`	`79`	`# Set the soft limit on the number of file descriptors synapse can use`
`@@ -1177,9 +1173,8 @@ account_validity:`
`1177`	`1173`	`# send an email to the account's email address with a renewal link. By`
`1178`	`1174`	`# default, no such emails are sent.`
`1179`	`1175`	`#`
`1180`		`- # If you enable this setting, you will also need to fill out the 'email'`
`1181`		`- # configuration section. You should also check that 'public_baseurl' is set`
`1182`		`- # correctly.`
	`1176`	`+ # If you enable this setting, you will also need to fill out the 'email' and`
	`1177`	`+ # 'public_baseurl' configuration sections.`
`1183`	`1178`	`#`
`1184`	`1179`	`#renew_at: 1w`
`1185`	`1180`
`@@ -1270,7 +1265,8 @@ account_validity:`
`1270`	`1265`	`# The identity server which we suggest that clients should use when users log`
`1271`	`1266`	`# in on this server.`
`1272`	`1267`	`#`
`1273`		`-# (By default, no suggestion is made, so it is left up to the client.)`
	`1268`	`+# (By default, no suggestion is made, so it is left up to the client.`
	`1269`	`+# This setting is ignored unless public_baseurl is also set.)`
`1274`	`1270`	`#`
`1275`	`1271`	`#default_identity_server: https://matrix.org`
`1276`	`1272`
`@@ -1295,6 +1291,8 @@ account_validity:`
`1295`	`1291`	`# by the Matrix Identity Service API specification:`
`1296`	`1292`	`# https://matrix.org/docs/spec/identity_service/latest`
`1297`	`1293`	`#`
	`1294`	`+# If a delegate is specified, the config option public_baseurl must also be filled out.`
	`1295`	`+#`
`1298`	`1296`	`account_threepid_delegates:`
`1299`	`1297`	`#email: https://example.com # Delegate email sending to example.com`
`1300`	`1298`	`#msisdn: http://localhost:8090 # Delegate SMS sending to this local process`
`@@ -1948,9 +1946,9 @@ sso:`
`1948`	`1946`	`# phishing attacks from evil.site. To avoid this, include a slash after the`
`1949`	`1947`	`# hostname: "https://my.client/".`
`1950`	`1948`	`#`
`1951`		`- # The login fallback page (used by clients that don't natively support the`
`1952`		`- # required login flows) is automatically whitelisted in addition to any URLs`
`1953`		`- # in this list.`
	`1949`	`+ # If public_baseurl is set, then the login fallback page (used by clients`
	`1950`	`+ # that don't natively support the required login flows) is whitelisted in`
	`1951`	`+ # addition to any URLs in this list.`
`1954`	`1952`	`#`
`1955`	`1953`	`# By default, this list is empty.`
`1956`	`1954`	`#`