Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Default value for public_baseurl #11210

Merged
merged 8 commits into from
Nov 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/11210.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Calculate a default value for `public_baseurl` based on `server_name`.
13 changes: 6 additions & 7 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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://<server_name>/'.
#
#public_baseurl: https://example.com/

# Set the soft limit on the number of file descriptors synapse can use
Expand Down Expand Up @@ -1247,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

Expand All @@ -1272,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
Expand Down Expand Up @@ -1963,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
Expand Down
3 changes: 0 additions & 3 deletions synapse/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 0 additions & 4 deletions synapse/config/account_validity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
10 changes: 3 additions & 7 deletions synapse/config/cas.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

from synapse.config.sso import SsoAttributeRequirement

from ._base import Config, ConfigError
from ._base import Config
from ._util import validate_config


Expand All @@ -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(
Expand Down
8 changes: 0 additions & 8 deletions synapse/config/emailconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),)
Expand Down Expand Up @@ -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"
Expand Down
2 changes: 0 additions & 2 deletions synapse/config/oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 1 addition & 14 deletions synapse/config/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -240,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

Expand All @@ -265,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
Expand Down
5 changes: 1 addition & 4 deletions synapse/config/saml2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
45 changes: 41 additions & 4 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -263,10 +264,44 @@ 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

# 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",))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this has minor potential to cause previously working configs to break (if somehow the generated public_baseurl from server_name creates an invalid URL). Looks like we already validate that it above though so should be OK.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be ok.

I feel like if server_name did generate a bad pubic_baseurl, that must be a pretty broken setup anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair! 👍

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 {}
Expand Down Expand Up @@ -772,6 +807,8 @@ def generate_config_section(
# Otherwise, it should be the URL to reach Synapse's client HTTP listener (see
# 'listeners' below).
#
# Defaults to 'https://<server_name>/'.
#
#public_baseurl: https://example.com/

# Set the soft limit on the number of file descriptors synapse can use
Expand Down
18 changes: 7 additions & 11 deletions synapse/config/sso.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 """\
Expand All @@ -128,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
Expand Down
4 changes: 0 additions & 4 deletions synapse/handlers/identity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions synapse/rest/well_known.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
2 changes: 1 addition & 1 deletion tests/push/test_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion tests/rest/client/test_consent.py
Original file line number Diff line number Diff line change
Expand Up @@ -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...
Expand Down
1 change: 0 additions & 1 deletion tests/rest/client/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down