sharpetronics/synapse
1
0
Fork 0
mirror of https://github.com/element-hq/synapse.git synced 2025-11-22 00:05:29 -05:00
Code Issues Projects Releases Wiki Activity

Introduce RootConfig.validate_config() which can be subclassed in HomeServerConfig to do cross-config class validation (#19027)

Browse Source 
This means we
can move the open registration config validation from `setup()` to
`HomeServerConfig.validate_config()` (much more sane).

Spawning from looking at this area of code in
https://github.com/element-hq/synapse/pull/19015
This commit is contained in:
Eric Eastwood 2025-10-09 14:56:22 -05:00 committed by GitHub
parent 715cc5ee37
commit 47fb4b43ca
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 155 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
changelog.d/19027.misc Normal file
View File

@ -0,0 +1 @@
Introduce `RootConfig.validate_config()` which can be subclassed in `HomeServerConfig` to do cross-config class validation.

18
synapse/app/homeserver.py
View File

@ -367,7 +367,7 @@ def create_homeserver(
if config.worker.worker_app:
raise ConfigError(
"You have specified `worker_app` in the config but are attempting to start a non-worker "
"You have specified `worker_app` in the config but are attempting to setup a non-worker "
"instance. Please use `python -m synapse.app.generic_worker` instead (or remove the option if this is the main process)."
)
@ -377,22 +377,6 @@ def create_homeserver(
if config.server.gc_seconds:
synapse.metrics.MIN_TIME_BETWEEN_GCS = config.server.gc_seconds
if (
config.registration.enable_registration
and not config.registration.enable_registration_without_verification
):
if (
not config.captcha.enable_registration_captcha
and not config.registration.registrations_require_3pid
and not config.registration.registration_requires_token
):
raise ConfigError(
"You have enabled open registration without any verification. This is a known vector for "
"spam and abuse. If you would like to allow public registration, please consider adding email, "
"captcha, or token-based verification. Otherwise this check can be removed by setting the "
"`enable_registration_without_verification` config option to `true`."
)
hs = SynapseHomeServer(
hostname=config.server.server_name,
config=config,

40
synapse/config/_base.py
View File

@ -545,18 +545,22 @@ class RootConfig:
@classmethod
def load_config(
cls: Type[TRootConfig], description: str, argv: List[str]
cls: Type[TRootConfig], description: str, argv_options: List[str]
) -> TRootConfig:
"""Parse the commandline and config files
Doesn't support config-file-generation: used by the worker apps.
Args:
description: TODO
argv_options: The options passed to Synapse. Usually `sys.argv[1:]`.
Returns:
Config object.
"""
config_parser = argparse.ArgumentParser(description=description)
cls.add_arguments_to_parser(config_parser)
obj, _ = cls.load_config_with_parser(config_parser, argv)
obj, _ = cls.load_config_with_parser(config_parser, argv_options)
return obj
@ -609,6 +613,10 @@ class RootConfig:
Used for workers where we want to add extra flags/subcommands.
Note: This is the common denominator for loading config and is also used by
`load_config` and `load_or_generate_config`. Which is why we call
`validate_config()` here.
Args:
parser
argv_options: The options passed to Synapse. Usually `sys.argv[1:]`.
@ -642,6 +650,10 @@ class RootConfig:
obj.invoke_all("read_arguments", config_args)
# Now that we finally have the full config sections parsed, allow subclasses to
# do some extra validation across the entire config.
obj.validate_config()
return obj, config_args
@classmethod
@ -842,15 +854,7 @@ class RootConfig:
):
return None
obj.parse_config_dict(
config_dict,
config_dir_path=config_dir_path,
data_dir_path=data_dir_path,
allow_secrets_in_config=config_args.secrets_in_config,
)
obj.invoke_all("read_arguments", config_args)
return obj
return cls.load_config(description, argv_options)
def parse_config_dict(
self,
@ -911,6 +915,20 @@ class RootConfig:
existing_config.root = None
return existing_config
def validate_config(self) -> None:
"""
Additional config validation across all config sections.
Override this in subclasses to add extra validation. This is called once all
config option values have been populated.
XXX: This should only validate, not modify the configuration, as the final
config state is required for proper validation across all config sections.
Raises:
ConfigError: if the config is invalid.
"""
def read_config_files(config_files: Iterable[str]) -> Dict[str, Any]:
"""Read the config files and shallowly merge them into a dict.

6
synapse/config/_base.pyi
View File

@ -158,11 +158,11 @@ class RootConfig:
) -> str: ...
@classmethod
def load_or_generate_config(
cls: Type[TRootConfig], description: str, argv: List[str]
cls: Type[TRootConfig], description: str, argv_options: List[str]
) -> Optional[TRootConfig]: ...
@classmethod
def load_config(
cls: Type[TRootConfig], description: str, argv: List[str]
cls: Type[TRootConfig], description: str, argv_options: List[str]
) -> TRootConfig: ...
@classmethod
def add_arguments_to_parser(
@ -170,7 +170,7 @@ class RootConfig:
) -> None: ...
@classmethod
def load_config_with_parser(
cls: Type[TRootConfig], parser: argparse.ArgumentParser, argv: List[str]
cls: Type[TRootConfig], parser: argparse.ArgumentParser, argv_options: List[str]
) -> Tuple[TRootConfig, argparse.Namespace]: ...
def generate_missing_files(
self, config_dict: dict, config_dir_path: str

26
synapse/config/homeserver.py
View File

@ -18,7 +18,8 @@
# [This file includes modifications made by New Vector Limited]
#
#
from ._base import RootConfig
from ._base import ConfigError, RootConfig
from .account_validity import AccountValidityConfig
from .api import ApiConfig
from .appservice import AppServiceConfig
@ -67,6 +68,10 @@ from .workers import WorkerConfig
class HomeServerConfig(RootConfig):
"""
Top-level config object for Synapse homeserver (main process and workers).
"""
config_classes = [
ModulesConfig,
ServerConfig,
@ -115,3 +120,22 @@ class HomeServerConfig(RootConfig):
# This must be last, as it checks for conflicts with other config options.
MasConfig,
]
def validate_config(
self,
) -> None:
if (
self.registration.enable_registration
and not self.registration.enable_registration_without_verification
):
if (
not self.captcha.enable_registration_captcha
and not self.registration.registrations_require_3pid
and not self.registration.registration_requires_token
):
raise ConfigError(
"You have enabled open registration without any verification. This is a known vector for "
"spam and abuse. If you would like to allow public registration, please consider adding email, "
"captcha, or token-based verification. Otherwise this check can be removed by setting the "
"`enable_registration_without_verification` config option to `true`."
)

9
tests/config/test_load.py
View File

@ -99,7 +99,14 @@ class ConfigLoadingFileTestCase(ConfigFileTestCase):
def test_disable_registration(self) -> None:
self.generate_config()
self.add_lines_to_config(
["enable_registration: true", "disable_registration: true"]
[
"enable_registration: true",
"disable_registration: true",
# We're not worried about open registration in this test. This test is
# focused on making sure that enable/disable_registration properly
# override each other.
"enable_registration_without_verification: true",
]
)
# Check that disable_registration clobbers enable_registration.
config = HomeServerConfig.load_config("", ["-c", self.config_file])

97
tests/config/test_registration_config.py
View File

@ -19,6 +19,8 @@
#
#
import argparse
import synapse.app.homeserver
from synapse.config import ConfigError
from synapse.config.homeserver import HomeServerConfig
@ -99,6 +101,39 @@ class RegistrationConfigTestCase(ConfigFileTestCase):
)
def test_refuse_to_start_if_open_registration_and_no_verification(self) -> None:
"""
Test that our utilities to start the main Synapse homeserver process refuses
to start if we detect open registration.
"""
self.generate_config()
self.add_lines_to_config(
[
" ",
"enable_registration: true",
"registrations_require_3pid: []",
"enable_registration_captcha: false",
"registration_requires_token: false",
]
)
# Test that allowing open registration without verification raises an error
with self.assertRaises(SystemExit):
# Do a normal homeserver creation and setup
homeserver_config = synapse.app.homeserver.load_or_generate_config(
["-c", self.config_file]
)
# XXX: The error will be raised at this point
hs = synapse.app.homeserver.create_homeserver(homeserver_config)
# Continue with the setup. We don't expect this to run because we raised
# earlier, but in the future, the code could be refactored to raise the
# error in a different place.
synapse.app.homeserver.setup(hs)
def test_load_config_error_if_open_registration_and_no_verification(self) -> None:
"""
Test that `HomeServerConfig.load_config(...)` raises an exception when we detect open
registration.
"""
self.generate_config()
self.add_lines_to_config(
[
@ -112,13 +147,57 @@ class RegistrationConfigTestCase(ConfigFileTestCase):
# Test that allowing open registration without verification raises an error
with self.assertRaises(ConfigError):
# Do a normal homeserver creation and setup
homeserver_config = synapse.app.homeserver.load_or_generate_config(
["-c", self.config_file]
_homeserver_config = HomeServerConfig.load_config(
description="test", argv_options=["-c", self.config_file]
)
def test_load_or_generate_config_error_if_open_registration_and_no_verification(
self,
) -> None:
"""
Test that `HomeServerConfig.load_or_generate_config(...)` raises an exception when we detect open
registration.
"""
self.generate_config()
self.add_lines_to_config(
[
" ",
"enable_registration: true",
"registrations_require_3pid: []",
"enable_registration_captcha: false",
"registration_requires_token: false",
]
)
# Test that allowing open registration without verification raises an error
with self.assertRaises(ConfigError):
_homeserver_config = HomeServerConfig.load_or_generate_config(
description="test", argv_options=["-c", self.config_file]
)
def test_load_config_with_parser_error_if_open_registration_and_no_verification(
self,
) -> None:
"""
Test that `HomeServerConfig.load_config_with_parser(...)` raises an exception when we detect open
registration.
"""
self.generate_config()
self.add_lines_to_config(
[
" ",
"enable_registration: true",
"registrations_require_3pid: []",
"enable_registration_captcha: false",
"registration_requires_token: false",
]
)
# Test that allowing open registration without verification raises an error
with self.assertRaises(ConfigError):
config_parser = argparse.ArgumentParser(description="test")
HomeServerConfig.add_arguments_to_parser(config_parser)
_homeserver_config = HomeServerConfig.load_config_with_parser(
parser=config_parser, argv_options=["-c", self.config_file]
)
# XXX: The error will be raised at this point
hs = synapse.app.homeserver.create_homeserver(homeserver_config)
# Continue with the setup. We don't expect this to run because we raised
# earlier, but in the future, the code could be refactored to raise the
# error in a different place.
synapse.app.homeserver.setup(hs)