From 32a2f050042531ad4673b42789e833e9cd307740 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Fri, 26 May 2023 14:50:19 +0200 Subject: [PATCH] Make the config tests spawn the homeserver only when needed --- synapse/config/experimental.py | 40 ++- tests/config/test_oauth_delegation.py | 358 ++++++++++++-------------- 2 files changed, 187 insertions(+), 211 deletions(-) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index d4dff22b0..1d189b2e2 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -69,7 +69,8 @@ class MSC3861: if value and not HAS_AUTHLIB: raise ConfigError( "MSC3861 is enabled but authlib is not installed. " - "Please install authlib to use MSC3861." + "Please install authlib to use MSC3861.", + ("experimental", "msc3861", "enabled"), ) issuer: str = attr.ib(default="", validator=attr.validators.instance_of(str)) @@ -114,7 +115,8 @@ class MSC3861: if value == ClientAuthMethod.PRIVATE_KEY_JWT and self.jwk is None: raise ConfigError( - "A JWKS must be provided when using the private_key_jwt client auth method" + "A JWKS must be provided when using the private_key_jwt client auth method", + ("experimental", "msc3861", "client_auth_method"), ) if ( @@ -127,7 +129,8 @@ class MSC3861: and self.client_secret is None ): raise ConfigError( - f"A client secret must be provided when using the {value} client auth method" + f"A client secret must be provided when using the {value} client auth method", + ("experimental", "msc3861", "client_auth_method"), ) account_management_url: Optional[str] = attr.ib( @@ -160,12 +163,14 @@ class MSC3861: or root.auth.password_enabled_for_login ): raise ConfigError( - "Password auth cannot be enabled when OAuth delegation is enabled" + "Password auth cannot be enabled when OAuth delegation is enabled", + ("password_config", "enabled"), ) if root.registration.enable_registration: raise ConfigError( - "Registration cannot be enabled when OAuth delegation is enabled" + "Registration cannot be enabled when OAuth delegation is enabled", + ("enable_registration",), ) if ( @@ -183,32 +188,38 @@ class MSC3861: if root.captcha.enable_registration_captcha: raise ConfigError( - "CAPTCHA cannot be enabled when OAuth delegation is enabled" + "CAPTCHA cannot be enabled when OAuth delegation is enabled", + ("captcha", "enable_registration_captcha"), ) if root.experimental.msc3882_enabled: raise ConfigError( - "MSC3882 cannot be enabled when OAuth delegation is enabled" + "MSC3882 cannot be enabled when OAuth delegation is enabled", + ("experimental_features", "msc3882_enabled"), ) if root.registration.refresh_token_lifetime: raise ConfigError( - "refresh_token_lifetime cannot be set when OAuth delegation is enabled" + "refresh_token_lifetime cannot be set when OAuth delegation is enabled", + ("refresh_token_lifetime",), ) if root.registration.nonrefreshable_access_token_lifetime: raise ConfigError( - "nonrefreshable_access_token_lifetime cannot be set when OAuth delegation is enabled" + "nonrefreshable_access_token_lifetime cannot be set when OAuth delegation is enabled", + ("nonrefreshable_access_token_lifetime",), ) if root.registration.session_lifetime: raise ConfigError( - "session_lifetime cannot be set when OAuth delegation is enabled" + "session_lifetime cannot be set when OAuth delegation is enabled", + ("session_lifetime",), ) if not root.experimental.msc3970_enabled: raise ConfigError( - "experimental_features.msc3970_enabled must be 'true' when OAuth delegation is enabled" + "experimental_features.msc3970_enabled must be 'true' when OAuth delegation is enabled", + ("experimental_features", "msc3970_enabled"), ) @@ -373,7 +384,12 @@ class ExperimentalConfig(Config): ) # MSC3861: Matrix architecture change to delegate authentication via OIDC - self.msc3861 = MSC3861(**experimental.get("msc3861", {})) + try: + self.msc3861 = MSC3861(**experimental.get("msc3861", {})) + except ValueError as exc: + raise ConfigError( + "Invalid MSC3861 configuration", ("experimental", "msc3861") + ) from exc # MSC3970: Scope transaction IDs to devices self.msc3970_enabled = experimental.get("msc3970_enabled", self.msc3861.enabled) diff --git a/tests/config/test_oauth_delegation.py b/tests/config/test_oauth_delegation.py index 6d294e014..2ead721b0 100644 --- a/tests/config/test_oauth_delegation.py +++ b/tests/config/test_oauth_delegation.py @@ -12,15 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import Any, Dict from unittest.mock import Mock from synapse.config import ConfigError +from synapse.config.homeserver import HomeServerConfig from synapse.module_api import ModuleApi from synapse.types import JsonDict -from tests.server import get_clock -from tests.unittest import HomeserverTestCase, override_config, skip_unless +from tests.server import get_clock, setup_test_homeserver +from tests.unittest import TestCase, skip_unless +from tests.utils import default_config try: import authlib # noqa: F401 @@ -51,45 +52,15 @@ class CustomAuthModule: ) -def _dict_merge(merge_dict: dict, into_dict: dict) -> None: - """Do a deep merge of two dicts - - Recursively merges `merge_dict` into `into_dict`: - * For keys where both `merge_dict` and `into_dict` have a dict value, the values - are recursively merged - * For all other keys, the values in `into_dict` (if any) are overwritten with - the value from `merge_dict`. - - Args: - merge_dict: dict to merge - into_dict: target dict to be modified - """ - for k, v in merge_dict.items(): - if k not in into_dict: - into_dict[k] = v - continue - - current_val = into_dict[k] - - if isinstance(v, dict) and isinstance(current_val, dict): - _dict_merge(v, current_val) - continue - - # otherwise we just overwrite - into_dict[k] = v - - @skip_unless(HAS_AUTHLIB, "requires authlib") -class MSC3861OAuthDelegation(HomeserverTestCase): +class MSC3861OAuthDelegation(TestCase): """Test that the Homeserver fails to initialize if the config is invalid.""" def setUp(self) -> None: - self.reactor, self.clock = get_clock() - self._hs_args = {"clock": self.clock, "reactor": self.reactor} - - def default_config(self) -> Dict[str, Any]: - default_extra_config = { + self.config_dict: JsonDict = { + **default_config("test"), "public_baseurl": BASE_URL, + "enable_registration": False, "experimental_features": { "msc3861": { "enabled": True, @@ -100,198 +71,187 @@ class MSC3861OAuthDelegation(HomeserverTestCase): } }, } - _dict_merge( - {} if self._extra_config is None else self._extra_config, - default_extra_config, - ) - self._extra_config = default_extra_config - return super().default_config() - @override_config( - { - "enable_registration": False, - } - ) + def parse_config(self) -> HomeServerConfig: + config = HomeServerConfig() + config.parse_config_dict(self.config_dict, "", "") + return config + def test_client_secret_post_works(self) -> None: - self.setup_test_homeserver() + self.config_dict["experimental_features"]["msc3861"].update( + client_auth_method="client_secret_post", + client_secret=CLIENT_SECRET, + ) - @override_config( - { - "enable_registration": False, - "experimental_features": { - "msc3861": { - "client_auth_method": "invalid", - } - }, - } - ) - def test_invalid_client_auth_method(self) -> None: - with self.assertRaises(ValueError): - self.setup_test_homeserver() + self.parse_config() + + def test_client_secret_post_requires_client_secret(self) -> None: + self.config_dict["experimental_features"]["msc3861"].update( + client_auth_method="client_secret_post", + client_secret=None, + ) - @override_config( - { - "enable_registration": False, - "experimental_features": { - "msc3861": { - "client_auth_method": "private_key_jwt", - } - }, - } - ) - def test_invalid_private_key_jwt(self) -> None: with self.assertRaises(ConfigError): - self.setup_test_homeserver() + self.parse_config() + + def test_client_secret_basic_works(self) -> None: + self.config_dict["experimental_features"]["msc3861"].update( + client_auth_method="client_secret_basic", + client_secret=CLIENT_SECRET, + ) + + self.parse_config() + + def test_client_secret_basic_requires_client_secret(self) -> None: + self.config_dict["experimental_features"]["msc3861"].update( + client_auth_method="client_secret_basic", + client_secret=None, + ) + + with self.assertRaises(ConfigError): + self.parse_config() + + def test_client_secret_jwt_works(self) -> None: + self.config_dict["experimental_features"]["msc3861"].update( + client_auth_method="client_secret_jwt", + client_secret=CLIENT_SECRET, + ) + + self.parse_config() + + def test_client_secret_jwt_requires_client_secret(self) -> None: + self.config_dict["experimental_features"]["msc3861"].update( + client_auth_method="client_secret_jwt", + client_secret=None, + ) + + with self.assertRaises(ConfigError): + self.parse_config() + + def test_invalid_client_auth_method(self) -> None: + self.config_dict["experimental_features"]["msc3861"].update( + client_auth_method="invalid", + ) + + with self.assertRaises(ConfigError): + self.parse_config() + + def test_private_key_jwt_requires_jwk(self) -> None: + self.config_dict["experimental_features"]["msc3861"].update( + client_auth_method="private_key_jwt", + ) + + with self.assertRaises(ConfigError): + self.parse_config() - @override_config( - { - "enable_registration": False, - "experimental_features": { - "msc3861": { - "client_auth_method": "private_key_jwt", - "jwk": { - "p": "-frVdP_tZ-J_nIR6HNMDq1N7aunwm51nAqNnhqIyuA8ikx7LlQED1tt2LD3YEvYyW8nxE2V95HlCRZXQPMiRJBFOsbmYkzl2t-MpavTaObB_fct_JqcRtdXddg4-_ihdjRDwUOreq_dpWh6MIKsC3UyekfkHmeEJg5YpOTL15j8", - "kty": "RSA", - "q": "oFw-Enr_YozQB1ab-kawn4jY3yHi8B1nSmYT0s8oTCflrmps5BFJfCkHL5ij3iY15z0o2m0N-jjB1oSJ98O4RayEEYNQlHnTNTl0kRIWzpoqblHUIxVcahIpP_xTovBJzwi8XXoLGqHOOMA-r40LSyVgP2Ut8D9qBwV6_UfT0LU", - "d": "WFkDPYo4b4LIS64D_QtQfGGuAObPvc3HFfp9VZXyq3SJR58XZRHE0jqtlEMNHhOTgbMYS3w8nxPQ_qVzY-5hs4fIanwvB64mAoOGl0qMHO65DTD_WsGFwzYClJPBVniavkLE2Hmpu8IGe6lGliN8vREC6_4t69liY-XcN_ECboVtC2behKkLOEASOIMuS7YcKAhTJFJwkl1dqDlliEn5A4u4xy7nuWQz3juB1OFdKlwGA5dfhDNglhoLIwNnkLsUPPFO-WB5ZNEW35xxHOToxj4bShvDuanVA6mJPtTKjz0XibjB36bj_nF_j7EtbE2PdGJ2KevAVgElR4lqS4ISgQ", - "e": "AQAB", - "kid": "test", - "qi": "cPfNk8l8W5exVNNea4d7QZZ8Qr8LgHghypYAxz8PQh1fNa8Ya1SNUDVzC2iHHhszxxA0vB9C7jGze8dBrvnzWYF1XvQcqNIVVgHhD57R1Nm3dj2NoHIKe0Cu4bCUtP8xnZQUN4KX7y4IIcgRcBWG1hT6DEYZ4BxqicnBXXNXAUI", - "dp": "dKlMHvslV1sMBQaKWpNb3gPq0B13TZhqr3-E2_8sPlvJ3fD8P4CmwwnOn50JDuhY3h9jY5L06sBwXjspYISVv8hX-ndMLkEeF3lrJeA5S70D8rgakfZcPIkffm3tlf1Ok3v5OzoxSv3-67Df4osMniyYwDUBCB5Oq1tTx77xpU8", - "dq": "S4ooU1xNYYcjl9FcuJEEMqKsRrAXzzSKq6laPTwIp5dDwt2vXeAm1a4eDHXC-6rUSZGt5PbqVqzV4s-cjnJMI8YYkIdjNg4NSE1Ac_YpeDl3M3Colb5CQlU7yUB7xY2bt0NOOFp9UJZYJrOo09mFMGjy5eorsbitoZEbVqS3SuE", - "n": "nJbYKqFwnURKimaviyDFrNLD3gaKR1JW343Qem25VeZxoMq1665RHVoO8n1oBm4ClZdjIiZiVdpyqzD5-Ow12YQgQEf1ZHP3CCcOQQhU57Rh5XvScTe5IxYVkEW32IW2mp_CJ6WfjYpfeL4azarVk8H3Vr59d1rSrKTVVinVdZer9YLQyC_rWAQNtHafPBMrf6RYiNGV9EiYn72wFIXlLlBYQ9Fx7bfe1PaL6qrQSsZP3_rSpuvVdLh1lqGeCLR0pyclA9uo5m2tMyCXuuGQLbA_QJm5xEc7zd-WFdux2eXF045oxnSZ_kgQt-pdN7AxGWOVvwoTf9am6mSkEdv6iw", - }, - } - }, - } - ) def test_private_key_jwt_works(self) -> None: - self.setup_test_homeserver() + self.config_dict["experimental_features"]["msc3861"].update( + client_auth_method="private_key_jwt", + jwk={ + "p": "-frVdP_tZ-J_nIR6HNMDq1N7aunwm51nAqNnhqIyuA8ikx7LlQED1tt2LD3YEvYyW8nxE2V95HlCRZXQPMiRJBFOsbmYkzl2t-MpavTaObB_fct_JqcRtdXddg4-_ihdjRDwUOreq_dpWh6MIKsC3UyekfkHmeEJg5YpOTL15j8", + "kty": "RSA", + "q": "oFw-Enr_YozQB1ab-kawn4jY3yHi8B1nSmYT0s8oTCflrmps5BFJfCkHL5ij3iY15z0o2m0N-jjB1oSJ98O4RayEEYNQlHnTNTl0kRIWzpoqblHUIxVcahIpP_xTovBJzwi8XXoLGqHOOMA-r40LSyVgP2Ut8D9qBwV6_UfT0LU", + "d": "WFkDPYo4b4LIS64D_QtQfGGuAObPvc3HFfp9VZXyq3SJR58XZRHE0jqtlEMNHhOTgbMYS3w8nxPQ_qVzY-5hs4fIanwvB64mAoOGl0qMHO65DTD_WsGFwzYClJPBVniavkLE2Hmpu8IGe6lGliN8vREC6_4t69liY-XcN_ECboVtC2behKkLOEASOIMuS7YcKAhTJFJwkl1dqDlliEn5A4u4xy7nuWQz3juB1OFdKlwGA5dfhDNglhoLIwNnkLsUPPFO-WB5ZNEW35xxHOToxj4bShvDuanVA6mJPtTKjz0XibjB36bj_nF_j7EtbE2PdGJ2KevAVgElR4lqS4ISgQ", + "e": "AQAB", + "kid": "test", + "qi": "cPfNk8l8W5exVNNea4d7QZZ8Qr8LgHghypYAxz8PQh1fNa8Ya1SNUDVzC2iHHhszxxA0vB9C7jGze8dBrvnzWYF1XvQcqNIVVgHhD57R1Nm3dj2NoHIKe0Cu4bCUtP8xnZQUN4KX7y4IIcgRcBWG1hT6DEYZ4BxqicnBXXNXAUI", + "dp": "dKlMHvslV1sMBQaKWpNb3gPq0B13TZhqr3-E2_8sPlvJ3fD8P4CmwwnOn50JDuhY3h9jY5L06sBwXjspYISVv8hX-ndMLkEeF3lrJeA5S70D8rgakfZcPIkffm3tlf1Ok3v5OzoxSv3-67Df4osMniyYwDUBCB5Oq1tTx77xpU8", + "dq": "S4ooU1xNYYcjl9FcuJEEMqKsRrAXzzSKq6laPTwIp5dDwt2vXeAm1a4eDHXC-6rUSZGt5PbqVqzV4s-cjnJMI8YYkIdjNg4NSE1Ac_YpeDl3M3Colb5CQlU7yUB7xY2bt0NOOFp9UJZYJrOo09mFMGjy5eorsbitoZEbVqS3SuE", + "n": "nJbYKqFwnURKimaviyDFrNLD3gaKR1JW343Qem25VeZxoMq1665RHVoO8n1oBm4ClZdjIiZiVdpyqzD5-Ow12YQgQEf1ZHP3CCcOQQhU57Rh5XvScTe5IxYVkEW32IW2mp_CJ6WfjYpfeL4azarVk8H3Vr59d1rSrKTVVinVdZer9YLQyC_rWAQNtHafPBMrf6RYiNGV9EiYn72wFIXlLlBYQ9Fx7bfe1PaL6qrQSsZP3_rSpuvVdLh1lqGeCLR0pyclA9uo5m2tMyCXuuGQLbA_QJm5xEc7zd-WFdux2eXF045oxnSZ_kgQt-pdN7AxGWOVvwoTf9am6mSkEdv6iw", + }, + ) + self.parse_config() def test_registration_cannot_be_enabled(self) -> None: + self.config_dict["enable_registration"] = True with self.assertRaises(ConfigError): - self.setup_test_homeserver() + self.parse_config() - @override_config( - { - "enable_registration": False, - "password_config": { - "enabled": True, - }, - } - ) def test_password_config_cannot_be_enabled(self) -> None: + self.config_dict["password_config"] = {"enabled": True} with self.assertRaises(ConfigError): - self.setup_test_homeserver() + self.parse_config() - @override_config( - { - "enable_registration": False, - "oidc_providers": [ - { - "idp_id": "microsoft", - "idp_name": "Microsoft", - "issuer": "https://login.microsoftonline.com//v2.0", - "client_id": "", - "client_secret": "", - "scopes": ["openid", "profile"], - "authorization_endpoint": "https://login.microsoftonline.com//oauth2/v2.0/authorize", - "token_endpoint": "https://login.microsoftonline.com//oauth2/v2.0/token", - "userinfo_endpoint": "https://graph.microsoft.com/oidc/userinfo", - } - ], - } - ) def test_oidc_sso_cannot_be_enabled(self) -> None: - with self.assertRaises(ConfigError): - self.setup_test_homeserver() + self.config_dict["oidc_providers"] = [ + { + "idp_id": "microsoft", + "idp_name": "Microsoft", + "issuer": "https://login.microsoftonline.com//v2.0", + "client_id": "", + "client_secret": "", + "scopes": ["openid", "profile"], + "authorization_endpoint": "https://login.microsoftonline.com//oauth2/v2.0/authorize", + "token_endpoint": "https://login.microsoftonline.com//oauth2/v2.0/token", + "userinfo_endpoint": "https://graph.microsoft.com/oidc/userinfo", + } + ] + + with self.assertRaises(ConfigError): + self.parse_config() - @override_config( - { - "enable_registration": False, - "cas_config": { - "enabled": True, - "server_url": "https://cas-server.com", - "displayname_attribute": "name", - "required_attributes": {"userGroup": "staff", "department": "None"}, - }, - } - ) def test_cas_sso_cannot_be_enabled(self) -> None: - with self.assertRaises(ConfigError): - self.setup_test_homeserver() - - @override_config( - { - "enable_registration": False, - "modules": [ - { - "module": f"{__name__}.{CustomAuthModule.__qualname__}", - "config": {}, - } - ], + self.config_dict["cas_config"] = { + "enabled": True, + "server_url": "https://cas-server.com", + "displayname_attribute": "name", + "required_attributes": {"userGroup": "staff", "department": "None"}, } - ) + + with self.assertRaises(ConfigError): + self.parse_config() + def test_auth_providers_cannot_be_enabled(self) -> None: - with self.assertRaises(ConfigError): - self.setup_test_homeserver() + self.config_dict["modules"] = [ + { + "module": f"{__name__}.{CustomAuthModule.__qualname__}", + "config": {}, + } + ] + + # This requires actually setting up an HS, as the module will be run on setup, + # which should raise as the module tries to register an auth provider + config = self.parse_config() + reactor, clock = get_clock() + with self.assertRaises(ConfigError): + setup_test_homeserver( + self.addCleanup, reactor=reactor, clock=clock, config=config + ) - @override_config( - { - "enable_registration": False, - "jwt_config": { - "enabled": True, - "secret": "my-secret-token", - "algorithm": "HS256", - }, - } - ) def test_jwt_auth_cannot_be_enabled(self) -> None: - with self.assertRaises(ConfigError): - self.setup_test_homeserver() - - @override_config( - { - "enable_registration": False, - "experimental_features": { - "msc3882_enabled": True, - }, + self.config_dict["jwt_config"] = { + "enabled": True, + "secret": "my-secret-token", + "algorithm": "HS256", } - ) + + with self.assertRaises(ConfigError): + self.parse_config() + def test_msc3882_auth_cannot_be_enabled(self) -> None: + self.config_dict["experimental_features"]["msc3882_enabled"] = True with self.assertRaises(ConfigError): - self.setup_test_homeserver() + self.parse_config() - @override_config( - { - "enable_registration": False, - "recaptcha_public_key": "test", - "recaptcha_private_key": "test", - "enable_registration_captcha": True, - } - ) def test_captcha_cannot_be_enabled(self) -> None: + self.config_dict.update( + enable_registration_captcha=True, + recaptcha_public_key="test", + recaptcha_private_key="test", + ) with self.assertRaises(ConfigError): - self.setup_test_homeserver() + self.parse_config() - @override_config( - { - "enable_registration": False, - "refresh_token_lifetime": "24h", - "refreshable_access_token_lifetime": "10m", - "nonrefreshable_access_token_lifetime": "24h", - } - ) def test_refreshable_tokens_cannot_be_enabled(self) -> None: + self.config_dict.update( + refresh_token_lifetime="24h", + refreshable_access_token_lifetime="10m", + nonrefreshable_access_token_lifetime="24h", + ) with self.assertRaises(ConfigError): - self.setup_test_homeserver() + self.parse_config() - @override_config( - { - "enable_registration": False, - "session_lifetime": "24h", - } - ) def test_session_lifetime_cannot_be_set(self) -> None: + self.config_dict["session_lifetime"] = "24h" with self.assertRaises(ConfigError): - self.setup_test_homeserver() + self.parse_config()