Merge pull request #565 from matrix-org/erikj/macaroon_config

Derive macaroon_secret_key from signing key.

3 files changed, 34 insertions, 19 deletions

diff --git a/synapse/config/key.py b/synapse/config/key.py
index ac90cd3fc1..a072aec714 100644
--- a/synapse/config/key.py
+++ b/synapse/config/key.py
@@ -22,8 +22,14 @@ from signedjson.key import (
     read_signing_keys, write_signing_keys, NACL_ED25519
 )
 from unpaddedbase64 import decode_base64
+from synapse.util.stringutils import random_string_with_symbols
 
 import os
+import hashlib
+import logging
+
+
+logger = logging.getLogger(__name__)
 
 
 class KeyConfig(Config):
@@ -40,9 +46,29 @@ class KeyConfig(Config):
             config["perspectives"]
         )
 
-    def default_config(self, config_dir_path, server_name, **kwargs):
+        self.macaroon_secret_key = config.get(
+            "macaroon_secret_key", self.registration_shared_secret
+        )
+
+        if not self.macaroon_secret_key:
+            # Unfortunately, there are people out there that don't have this
+            # set. Lets just be "nice" and derive one from their secret key.
+            logger.warn("Config is missing missing macaroon_secret_key")
+            seed = self.signing_key[0].seed
+            self.macaroon_secret_key = hashlib.sha256(seed)
+
+    def default_config(self, config_dir_path, server_name, is_generating_file=False,
+                       **kwargs):
         base_key_name = os.path.join(config_dir_path, server_name)
+
+        if is_generating_file:
+            macaroon_secret_key = random_string_with_symbols(50)
+        else:
+            macaroon_secret_key = None
+
         return """\
+        macaroon_secret_key: "%(macaroon_secret_key)s"
+
         ## Signing Keys ##
 
         # Path to the signing key to sign messages with
diff --git a/synapse/config/registration.py b/synapse/config/registration.py
index 9b6dacc5b8..ab062d528c 100644
--- a/synapse/config/registration.py
+++ b/synapse/config/registration.py
@@ -32,26 +32,14 @@ class RegistrationConfig(Config):
             )
 
         self.registration_shared_secret = config.get("registration_shared_secret")
-        self.macaroon_secret_key = config.get("macaroon_secret_key")
-        if self.macaroon_secret_key is None:
-            raise Exception(
-                "Config is missing missing macaroon_secret_key - please set it"
-                " in your config file."
-            )
+
         self.bcrypt_rounds = config.get("bcrypt_rounds", 12)
         self.trusted_third_party_id_servers = config["trusted_third_party_id_servers"]
         self.allow_guest_access = config.get("allow_guest_access", False)
 
-    def default_config(self, is_generating_file=False, **kwargs):
+    def default_config(self, **kwargs):
         registration_shared_secret = random_string_with_symbols(50)
 
-        macaroon_line = ""
-        if is_generating_file:
-            macaroon_line += '\n        macaroon_secret_key: "%s"\n' % (
-                random_string_with_symbols(50),
-            )
-
-        macaroon_secret_key = random_string_with_symbols(50)
         return """\
         ## Registration ##
 
@@ -61,7 +49,7 @@ class RegistrationConfig(Config):
         # If set, allows registration by anyone who also has the shared
         # secret, even if registration is otherwise disabled.
         registration_shared_secret: "%(registration_shared_secret)s"
-%(macaroon_line)s
+
         # Set the number of bcrypt rounds used to generate password hash.
         # Larger numbers increase the work factor needed to generate the hash.
         # The default number of rounds is 12.
diff --git a/tests/config/test_load.py b/tests/config/test_load.py
index 7f41279715..fbbbf93fef 100644
--- a/tests/config/test_load.py
+++ b/tests/config/test_load.py
@@ -54,10 +54,11 @@ class ConfigLoadingTestCase(unittest.TestCase):
                 "was: %r" % (config.macaroon_secret_key,)
             )
 
-    def test_load_fails_if_macaroon_secret_key_missing(self):
+    def test_load_succeeds_if_macaroon_secret_key_missing(self):
         self.generate_config_and_remove_lines_containing("macaroon")
-        with self.assertRaises(Exception):
-            HomeServerConfig.load_config("", ["-c", self.file])
+        config1 = HomeServerConfig.load_config("", ["-c", self.file])
+        config2 = HomeServerConfig.load_config("", ["-c", self.file])
+        self.assertEqual(config1.macaroon_secret_key, config2.macaroon_secret_key)
 
     def generate_config(self):
         HomeServerConfig.load_config("", [