Add `macaroon_secret_key_path` config option (#17983)

Another config option on my quest to a `*_path` variant for every
secret. This time it’s `macaroon_secret_key_path`.

Reading secrets from files has the security advantage of separating the secrets from the config. It also simplifies secrets management in Kubernetes. Also useful to NixOS users.

1 files changed, 15 insertions, 8 deletions

diff --git a/tests/config/test_load.py b/tests/config/test_load.py

index c5dee06af5..f8f7b72e40 100644
--- a/tests/config/test_load.py
+++ b/tests/config/test_load.py
@@ -39,7 +39,7 @@ except ImportError:
 
 class ConfigLoadingFileTestCase(ConfigFileTestCase):
     def test_load_fails_if_server_name_missing(self) -> None:
-        self.generate_config_and_remove_lines_containing("server_name")
+        self.generate_config_and_remove_lines_containing(["server_name"])
         with self.assertRaises(ConfigError):
             HomeServerConfig.load_config("", ["-c", self.config_file])
         with self.assertRaises(ConfigError):
@@ -76,7 +76,7 @@ class ConfigLoadingFileTestCase(ConfigFileTestCase):
             )
 
     def test_load_succeeds_if_macaroon_secret_key_missing(self) -> None:
-        self.generate_config_and_remove_lines_containing("macaroon")
+        self.generate_config_and_remove_lines_containing(["macaroon"])
         config1 = HomeServerConfig.load_config("", ["-c", self.config_file])
         config2 = HomeServerConfig.load_config("", ["-c", self.config_file])
         config3 = HomeServerConfig.load_or_generate_config("", ["-c", self.config_file])
@@ -111,7 +111,7 @@ class ConfigLoadingFileTestCase(ConfigFileTestCase):
         self.assertTrue(config3.registration.enable_registration)
 
     def test_stats_enabled(self) -> None:
-        self.generate_config_and_remove_lines_containing("enable_metrics")
+        self.generate_config_and_remove_lines_containing(["enable_metrics"])
         self.add_lines_to_config(["enable_metrics: true"])
 
         # The default Metrics Flags are off by default.
@@ -131,6 +131,7 @@ class ConfigLoadingFileTestCase(ConfigFileTestCase):
         [
             "turn_shared_secret_path: /does/not/exist",
             "registration_shared_secret_path: /does/not/exist",
+            "macaroon_secret_key_path: /does/not/exist",
             *["redis:\n  enabled: true\n  password_path: /does/not/exist"]
             * (hiredis is not None),
         ]
@@ -146,16 +147,20 @@ class ConfigLoadingFileTestCase(ConfigFileTestCase):
         [
             (
                 "turn_shared_secret_path: {}",
-                lambda c: c.voip.turn_shared_secret,
+                lambda c: c.voip.turn_shared_secret.encode("utf-8"),
             ),
             (
                 "registration_shared_secret_path: {}",
-                lambda c: c.registration.registration_shared_secret,
+                lambda c: c.registration.registration_shared_secret.encode("utf-8"),
+            ),
+            (
+                "macaroon_secret_key_path: {}",
+                lambda c: c.key.macaroon_secret_key,
             ),
             *[
                 (
                     "redis:\n  enabled: true\n  password_path: {}",
-                    lambda c: c.redis.redis_password,
+                    lambda c: c.redis.redis_password.encode("utf-8"),
                 )
             ]
             * (hiredis is not None),
@@ -164,11 +169,13 @@ class ConfigLoadingFileTestCase(ConfigFileTestCase):
     def test_secret_files_existing(
         self, config_line: str, get_secret: Callable[[RootConfig], str]
     ) -> None:
-        self.generate_config_and_remove_lines_containing("registration_shared_secret")
+        self.generate_config_and_remove_lines_containing(
+            ["registration_shared_secret", "macaroon_secret_key"]
+        )
         with tempfile.NamedTemporaryFile(buffering=0) as secret_file:
             secret_file.write(b"53C237")
 
             self.add_lines_to_config(["", config_line.format(secret_file.name)])
             config = HomeServerConfig.load_config("", ["-c", self.config_file])
 
-            self.assertEqual(get_secret(config), "53C237")
+            self.assertEqual(get_secret(config), b"53C237")