From f34df89a33fb647e578407ffe41bbf033a3b78fe Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 28 Oct 2021 18:41:07 +0100 Subject: [PATCH 1/8] Set a default value for `public_baseurl`. We might as well use a default value for `public_baseurl` based on `server_name` - in many cases, it will be correct. --- docs/sample_config.yaml | 2 ++ synapse/api/urls.py | 3 --- synapse/config/account_validity.py | 4 ---- synapse/config/cas.py | 8 ++------ synapse/config/oidc.py | 2 -- synapse/config/registration.py | 11 ----------- synapse/config/saml2.py | 5 +---- synapse/config/server.py | 30 ++++++++++++++++++++++++++---- synapse/config/sso.py | 11 ++++------- synapse/handlers/identity.py | 4 ---- synapse/rest/well_known.py | 3 +-- 11 files changed, 36 insertions(+), 47 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index b90ed62d616e..1a18fd45783f 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -91,6 +91,8 @@ pid_file: DATADIR/homeserver.pid # Otherwise, it should be the URL to reach Synapse's client HTTP listener (see # 'listeners' below). # +# Defaults to 'https:///'. +# #public_baseurl: https://example.com/ # Set the soft limit on the number of file descriptors synapse can use diff --git a/synapse/api/urls.py b/synapse/api/urls.py index 6e84b1524faa..4486b3bc7dfb 100644 --- a/synapse/api/urls.py +++ b/synapse/api/urls.py @@ -38,9 +38,6 @@ class ConsentURIBuilder: def __init__(self, hs_config: HomeServerConfig): if hs_config.key.form_secret is None: raise ConfigError("form_secret not set in config") - if hs_config.server.public_baseurl is None: - raise ConfigError("public_baseurl not set in config") - self._hmac_secret = hs_config.key.form_secret.encode("utf-8") self._public_baseurl = hs_config.server.public_baseurl diff --git a/synapse/config/account_validity.py b/synapse/config/account_validity.py index b56c2a24dfc6..c533452cabb5 100644 --- a/synapse/config/account_validity.py +++ b/synapse/config/account_validity.py @@ -75,10 +75,6 @@ def read_config(self, config, **kwargs): self.account_validity_period * 10.0 / 100.0 ) - if self.account_validity_renew_by_email_enabled: - if not self.root.server.public_baseurl: - raise ConfigError("Can't send renewal emails without 'public_baseurl'") - # Load account validity templates. account_validity_template_dir = account_validity_config.get("template_dir") if account_validity_template_dir is not None: diff --git a/synapse/config/cas.py b/synapse/config/cas.py index 9b58ecf3d839..b72cd6098602 100644 --- a/synapse/config/cas.py +++ b/synapse/config/cas.py @@ -35,14 +35,10 @@ def read_config(self, config, **kwargs): if self.cas_enabled: self.cas_server_url = cas_config["server_url"] - # The public baseurl is required because it is used by the redirect - # template. - public_baseurl = self.root.server.public_baseurl - if not public_baseurl: - raise ConfigError("cas_config requires a public_baseurl to be set") - # TODO Update this to a _synapse URL. + public_baseurl = self.root.server.public_baseurl 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/oidc.py b/synapse/config/oidc.py index 10f579633016..42f113cd249d 100644 --- a/synapse/config/oidc.py +++ b/synapse/config/oidc.py @@ -59,8 +59,6 @@ def read_config(self, config, **kwargs): ) public_baseurl = self.root.server.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 diff --git a/synapse/config/registration.py b/synapse/config/registration.py index a3d2a38c4c17..e368c62e20b0 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -45,17 +45,6 @@ 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.root.server.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) diff --git a/synapse/config/saml2.py b/synapse/config/saml2.py index 9c51b6a25a22..ba2b0905ffe8 100644 --- a/synapse/config/saml2.py +++ b/synapse/config/saml2.py @@ -199,14 +199,11 @@ def _default_saml_config_dict( """ import saml2 - public_baseurl = self.root.server.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) optional_attributes -= required_attributes + public_baseurl = self.root.server.public_baseurl metadata_url = public_baseurl + "_synapse/client/saml2/metadata.xml" response_url = public_baseurl + "_synapse/client/saml2/authn_response" return { diff --git a/synapse/config/server.py b/synapse/config/server.py index ed094bdc442d..f7d0cd8549ba 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -263,10 +263,30 @@ def read_config(self, config, **kwargs): 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") - if self.public_baseurl is not None: - if self.public_baseurl[-1] != "/": - self.public_baseurl += "/" + # Whether we should serve a "client well-known": + # (a) at .well-known/matrix/client on our client HTTP listener + # (b) in the response to /login + # + # ... which together help ensure that clients use our public_baseurl instead of + # whatever they were told by the user. + # + # For the sake of backwards compatibility with existing installations, this is + # True if public_baseurl is specified explicitly, and otherwise False. (The + # reasoning here is that we have no way of knowing that the default + # public_baseurl is actually correct for existing installations - many things + # will not work correctly, but that's (probably?) better than sending clients + # to a completely broken URL. + self.serve_client_wellknown = False + + public_baseurl = config.get("public_baseurl") + if public_baseurl is None: + public_baseurl = f"https://{self.server_name}/" + logger.info("Using default public_baseurl %s", public_baseurl) + else: + self.serve_client_wellknown = True + if public_baseurl[-1] != "/": + public_baseurl += "/" + self.public_baseurl = public_baseurl # Whether to enable user presence. presence_config = config.get("presence") or {} @@ -772,6 +792,8 @@ def generate_config_section( # Otherwise, it should be the URL to reach Synapse's client HTTP listener (see # 'listeners' below). # + # Defaults to 'https:///'. + # #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 index 11a9b76aa033..e543126045ab 100644 --- a/synapse/config/sso.py +++ b/synapse/config/sso.py @@ -101,13 +101,10 @@ 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. - # 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.root.server.public_baseurl: - login_fallback_url = ( - self.root.server.public_baseurl + "_matrix/static/client/login" - ) - self.sso_client_whitelist.append(login_fallback_url) + login_fallback_url = ( + self.root.server.public_baseurl + "_matrix/static/client/login" + ) + self.sso_client_whitelist.append(login_fallback_url) def generate_config_section(self, **kwargs): return """\ diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 6a315117ba03..3dbe611f9563 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -537,10 +537,6 @@ 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.server.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 index 7ac01faab4fb..3c53749b9270 100644 --- a/synapse/rest/well_known.py +++ b/synapse/rest/well_known.py @@ -33,8 +33,7 @@ def __init__(self, hs: "HomeServer"): self._config = hs.config def get_well_known(self) -> Optional[JsonDict]: - # if we don't have a public_baseurl, we can't help much here. - if self._config.server.public_baseurl is None: + if not self._config.server.serve_client_wellknown: return None result = {"m.homeserver": {"base_url": self._config.server.public_baseurl}} From 3ed765a7592320129fd0c596b40848703589815b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 29 Oct 2021 14:12:21 +0100 Subject: [PATCH 2/8] Validation for public_baseurl --- synapse/config/server.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/synapse/config/server.py b/synapse/config/server.py index f7d0cd8549ba..4ac64c50c2d5 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -16,6 +16,7 @@ import logging import os.path import re +import urllib.parse from textwrap import indent from typing import Any, Dict, Iterable, List, Optional, Set, Tuple, Union @@ -288,6 +289,20 @@ def read_config(self, config, **kwargs): public_baseurl += "/" self.public_baseurl = public_baseurl + # check that public_baseurl is valid + try: + splits = urllib.parse.urlsplit(self.public_baseurl) + except Exception as e: + raise ConfigError(f"Unable to parse URL: {e}", ("public_baseurl",)) + if splits.scheme not in ("https", "http"): + raise ConfigError( + f"Invalid scheme '{splits.scheme}': only https and http are supported" + ) + if splits.query or splits.fragment: + raise ConfigError( + "public_baseurl cannot contain query parameters or a #-fragment" + ) + # Whether to enable user presence. presence_config = config.get("presence") or {} self.use_presence = presence_config.get("enabled") From dea0b2d62b9710060af4d2f3afb67693d5ccd4c3 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 29 Oct 2021 17:08:32 +0100 Subject: [PATCH 3/8] changelog --- changelog.d/11210.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11210.feature diff --git a/changelog.d/11210.feature b/changelog.d/11210.feature new file mode 100644 index 000000000000..8f8e3864151f --- /dev/null +++ b/changelog.d/11210.feature @@ -0,0 +1 @@ +Calculate a default value for `public_baseurl` based on `server_name`. From 83ce4dafea0a3890152829f33005fffdc9655e06 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 29 Oct 2021 17:59:04 +0100 Subject: [PATCH 4/8] fix lint --- synapse/config/cas.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/cas.py b/synapse/config/cas.py index b72cd6098602..3f818140432f 100644 --- a/synapse/config/cas.py +++ b/synapse/config/cas.py @@ -16,7 +16,7 @@ from synapse.config.sso import SsoAttributeRequirement -from ._base import Config, ConfigError +from ._base import Config from ._util import validate_config From 246394874ff6f89f331275da9ce7f534bd23c0c8 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 1 Nov 2021 12:28:51 +0000 Subject: [PATCH 5/8] fix broken unit test --- tests/push/test_email.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/push/test_email.py b/tests/push/test_email.py index fa8018e5a7bc..90f800e564b4 100644 --- a/tests/push/test_email.py +++ b/tests/push/test_email.py @@ -65,7 +65,7 @@ def make_homeserver(self, reactor, clock): "notif_from": "test@example.com", "riot_base_url": None, } - config["public_baseurl"] = "aaa" + config["public_baseurl"] = "http://aaa" config["start_pushers"] = True hs = self.setup_test_homeserver(config=config) From eab41f4a9279d095c0c9a75abb007a0ca210e843 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 1 Nov 2021 12:43:15 +0000 Subject: [PATCH 6/8] Remove redundant checks for `public_baseurl` public_baseurl will now be set automatically, so no need to check it is set. --- synapse/config/emailconfig.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py index 8ff59aa2f8db..afd65fecd30f 100644 --- a/synapse/config/emailconfig.py +++ b/synapse/config/emailconfig.py @@ -186,11 +186,6 @@ 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),) @@ -296,9 +291,6 @@ 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" From 5e4b6deab1d4190eae81ae9de189a618acbe9bb3 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 1 Nov 2021 12:44:04 +0000 Subject: [PATCH 7/8] more unit test fixes --- tests/rest/client/test_consent.py | 1 - tests/rest/client/test_register.py | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/rest/client/test_consent.py b/tests/rest/client/test_consent.py index 84d092ca8242..fcdc565814cf 100644 --- a/tests/rest/client/test_consent.py +++ b/tests/rest/client/test_consent.py @@ -35,7 +35,6 @@ class ConsentResourceTestCase(unittest.HomeserverTestCase): def make_homeserver(self, reactor, clock): config = self.default_config() - config["public_baseurl"] = "aaaa" config["form_secret"] = "123abc" # Make some temporary templates... diff --git a/tests/rest/client/test_register.py b/tests/rest/client/test_register.py index 66dcfc9f8897..6e7c0f11df3e 100644 --- a/tests/rest/client/test_register.py +++ b/tests/rest/client/test_register.py @@ -891,7 +891,6 @@ def make_homeserver(self, reactor, clock): "smtp_pass": None, "notif_from": "test@example.com", } - config["public_baseurl"] = "aaa" self.hs = self.setup_test_homeserver(config=config) From fff6f9f201f4c889af5636dd53d9b3ad165bd112 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 1 Nov 2021 12:51:40 +0000 Subject: [PATCH 8/8] Sample config: remove references to setting public_baseurl Since it has a default now, you don't need to set it. --- docs/sample_config.yaml | 11 ++++------- synapse/config/registration.py | 4 +--- synapse/config/sso.py | 7 +++---- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 1a18fd45783f..2753d9e519f5 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1249,7 +1249,7 @@ oembed: # in on this server. # # (By default, no suggestion is made, so it is left up to the client. -# This setting is ignored unless public_baseurl is also set.) +# This setting is ignored unless public_baseurl is also explicitly set.) # #default_identity_server: https://matrix.org @@ -1274,8 +1274,6 @@ oembed: # 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 @@ -1965,11 +1963,10 @@ 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. + # 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. + # By default, this list contains only the login fallback page. # #client_whitelist: # - https://riot.im/develop diff --git a/synapse/config/registration.py b/synapse/config/registration.py index e368c62e20b0..5379e80715b3 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -229,7 +229,7 @@ def generate_config_section(self, generate_secrets=False, **kwargs): # in on this server. # # (By default, no suggestion is made, so it is left up to the client. - # This setting is ignored unless public_baseurl is also set.) + # This setting is ignored unless public_baseurl is also explicitly set.) # #default_identity_server: https://matrix.org @@ -254,8 +254,6 @@ 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/sso.py b/synapse/config/sso.py index e543126045ab..60aacb13ea40 100644 --- a/synapse/config/sso.py +++ b/synapse/config/sso.py @@ -125,11 +125,10 @@ def generate_config_section(self, **kwargs): # 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. + # 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. + # By default, this list contains only the login fallback page. # #client_whitelist: # - https://riot.im/develop