Make the config tests spawn the homeserver only when needed

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
+        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)

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.server import get_clock, setup_test_homeserver
-from tests.unittest import HomeserverTestCase, override_config, skip_unless
+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.config_dict: JsonDict = {
-        self._hs_args = {"clock": self.clock, "reactor": self.reactor}
+            **default_config("test"),
-
-    def default_config(self) -> Dict[str, Any]:
-        default_extra_config = {
             "public_baseurl": BASE_URL,
+            "enable_registration": False,
             "experimental_features": {
                 "msc3861": {
                     "enabled": True,
@@ -100,56 +71,83 @@ 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(
+    def parse_config(self) -> HomeServerConfig:
-        {
+        config = HomeServerConfig()
-            "enable_registration": False,
+        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",
-    @override_config(
+            client_secret=CLIENT_SECRET,
-        {
-            "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()
 
-    @override_config(
+        self.parse_config()
-        {
+
-            "enable_registration": False,
+    def test_client_secret_post_requires_client_secret(self) -> None:
-            "experimental_features": {
+        self.config_dict["experimental_features"]["msc3861"].update(
-                "msc3861": {
+            client_auth_method="client_secret_post",
-                    "client_auth_method": "private_key_jwt",
+            client_secret=None,
-                }
-            },
-        }
         )
-    def test_invalid_private_key_jwt(self) -> None:
+
         with self.assertRaises(ConfigError):
-            self.setup_test_homeserver()
+            self.parse_config()
 
-    @override_config(
+    def test_client_secret_basic_works(self) -> None:
-        {
+        self.config_dict["experimental_features"]["msc3861"].update(
-            "enable_registration": False,
+            client_auth_method="client_secret_basic",
-            "experimental_features": {
+            client_secret=CLIENT_SECRET,
-                "msc3861": {
+        )
-                    "client_auth_method": "private_key_jwt",
+
-                    "jwk": {
+        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()
+
+    def test_private_key_jwt_works(self) -> None:
+        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",
@@ -161,33 +159,21 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
                 "dq": "S4ooU1xNYYcjl9FcuJEEMqKsRrAXzzSKq6laPTwIp5dDwt2vXeAm1a4eDHXC-6rUSZGt5PbqVqzV4s-cjnJMI8YYkIdjNg4NSE1Ac_YpeDl3M3Colb5CQlU7yUB7xY2bt0NOOFp9UJZYJrOo09mFMGjy5eorsbitoZEbVqS3SuE",
                 "n": "nJbYKqFwnURKimaviyDFrNLD3gaKR1JW343Qem25VeZxoMq1665RHVoO8n1oBm4ClZdjIiZiVdpyqzD5-Ow12YQgQEf1ZHP3CCcOQQhU57Rh5XvScTe5IxYVkEW32IW2mp_CJ6WfjYpfeL4azarVk8H3Vr59d1rSrKTVVinVdZer9YLQyC_rWAQNtHafPBMrf6RYiNGV9EiYn72wFIXlLlBYQ9Fx7bfe1PaL6qrQSsZP3_rSpuvVdLh1lqGeCLR0pyclA9uo5m2tMyCXuuGQLbA_QJm5xEc7zd-WFdux2eXF045oxnSZ_kgQt-pdN7AxGWOVvwoTf9am6mSkEdv6iw",
             },
-                }
-            },
-        }
         )
-    def test_private_key_jwt_works(self) -> None:
+        self.parse_config()
-        self.setup_test_homeserver()
 
     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(
+    def test_oidc_sso_cannot_be_enabled(self) -> None:
-        {
+        self.config_dict["oidc_providers"] = [
-            "enable_registration": False,
-            "oidc_providers": [
             {
                 "idp_id": "microsoft",
                 "idp_name": "Microsoft",
@@ -199,99 +185,73 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
                 "token_endpoint": "https://login.microsoftonline.com/<tenant id>/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()
 
-    @override_config(
+        with self.assertRaises(ConfigError):
-        {
+            self.parse_config()
-            "enable_registration": False,
+
-            "cas_config": {
+    def test_cas_sso_cannot_be_enabled(self) -> None:
+        self.config_dict["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(
+        with self.assertRaises(ConfigError):
-        {
+            self.parse_config()
-            "enable_registration": False,
+
-            "modules": [
+    def test_auth_providers_cannot_be_enabled(self) -> None:
+        self.config_dict["modules"] = [
             {
                 "module": f"{__name__}.{CustomAuthModule.__qualname__}",
                 "config": {},
             }
-            ],
+        ]
-        }
-    )
-    def test_auth_providers_cannot_be_enabled(self) -> None:
-        with self.assertRaises(ConfigError):
-            self.setup_test_homeserver()
 
-    @override_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
-            "enable_registration": False,
+        config = self.parse_config()
-            "jwt_config": {
+        reactor, clock = get_clock()
+        with self.assertRaises(ConfigError):
+            setup_test_homeserver(
+                self.addCleanup, reactor=reactor, clock=clock, config=config
+            )
+
+    def test_jwt_auth_cannot_be_enabled(self) -> None:
+        self.config_dict["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(
+        with self.assertRaises(ConfigError):
-        {
+            self.parse_config()
-            "enable_registration": False,
+
-            "experimental_features": {
-                "msc3882_enabled": True,
-            },
-        }
-    )
     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:
-        with self.assertRaises(ConfigError):
+        self.config_dict.update(
-            self.setup_test_homeserver()
+            enable_registration_captcha=True,
-
+            recaptcha_public_key="test",
-    @override_config(
+            recaptcha_private_key="test",
-        {
-            "enable_registration": False,
-            "refresh_token_lifetime": "24h",
-            "refreshable_access_token_lifetime": "10m",
-            "nonrefreshable_access_token_lifetime": "24h",
-        }
         )
+        with self.assertRaises(ConfigError):
+            self.parse_config()
+
     def test_refreshable_tokens_cannot_be_enabled(self) -> None:
-        with self.assertRaises(ConfigError):
+        self.config_dict.update(
-            self.setup_test_homeserver()
+            refresh_token_lifetime="24h",
-
+            refreshable_access_token_lifetime="10m",
-    @override_config(
+            nonrefreshable_access_token_lifetime="24h",
-        {
-            "enable_registration": False,
-            "session_lifetime": "24h",
-        }
         )
-    def test_session_lifetime_cannot_be_set(self) -> None:
         with self.assertRaises(ConfigError):
-            self.setup_test_homeserver()
+            self.parse_config()
+
+    def test_session_lifetime_cannot_be_set(self) -> None:
+        self.config_dict["session_lifetime"] = "24h"
+        with self.assertRaises(ConfigError):
+            self.parse_config()