Convert ConfigParser object to dict and prevent #287

.gitignore

@@ -14,3 +14,4 @@ _trial_temp.lock
 /sydent.db
 /sydent.pid
 /matrix_is_test/sydent.stderr
+/matrix_is_test/sydent.log

changelog.d/402.misc

@@ -0,0 +1 @@
+Convert ConfigParser object to dict and prevent sydent from creating DEFAULT section in config file (#287).

matrix_is_test/launcher.py

@@ -33,6 +33,7 @@
 terms.path = {terms_path}
 templates.path = {testsubject_path}/res
 brand.default = is-test
+log.path = {testsubject_path}/sydent.log
 
 
 ip.whitelist = 127.0.0.1
@@ -89,18 +90,15 @@ def launch(self):
             }
         )
 
-        stderr_fp = open(os.path.join(testsubject_path, "sydent.stderr"), "w")
-
         pybin = os.getenv("SYDENT_PYTHON", "python")
 
         self.process = Popen(
             args=[pybin, "-m", "sydent.sydent"],
             cwd=self.tmpdir,
             env=newEnv,
-            stderr=stderr_fp,
         )
         # XXX: wait for startup in a sensible way
-        time.sleep(2)
+        time.sleep(10)
 
         self._baseUrl = "http://localhost:%d" % (port,)
 

sydent/config/__init__.py

@@ -21,6 +21,8 @@
 
 from twisted.python import log
 
+from sydent.config._base import CONFIG_PARSER_DICT
+from sydent.config._configparser import SydentConfigParser
 from sydent.config.crypto import CryptoConfig
 from sydent.config.database import DatabaseConfig
 from sydent.config.email import EmailConfig
@@ -182,7 +184,7 @@ def __init__(self):
             self.http,
         ]
 
-    def _parse_config(self, cfg: ConfigParser) -> bool:
+    def _parse_config(self, cfg: CONFIG_PARSER_DICT) -> bool:
         """
         Run the parse_config method on each of the objects in
         self.config_sections
@@ -201,9 +203,9 @@ def _parse_config(self, cfg: ConfigParser) -> bool:
 
         return needs_saving
 
-    def _parse_from_config_parser(self, cfg: ConfigParser) -> bool:
+    def _parse_from_dict(self, cfg: CONFIG_PARSER_DICT) -> bool:
         """
-        Parse the configuration from a ConfigParser object
+        Parse the configuration from a dict
 
         :param cfg: the configuration to be parsed
 
@@ -214,6 +216,30 @@ def _parse_from_config_parser(self, cfg: ConfigParser) -> bool:
         """
         return self._parse_config(cfg)
 
+    def _parse_from_sydent_config_parser(self, cfg: SydentConfigParser) -> bool:
+        """
+        Parse the configuration from a SydentConfigParser object
+
+        :param cfg: the configuration to be parsed
+
+        :return: whether or not the config file needs updating. This method CAN
+            return True, but it *shouldn't*. Instead a ConfigError exception
+            should be raised. This is left in for the soon to be deprecated way
+            of generating config files.
+        """
+        config_dict: CONFIG_PARSER_DICT = {}
+        for section in cfg.sections():
+            config_dict[section] = {}
+            # Copy in any values that are in the DEFAULT section
+            # This must be done first as they might be overwritten
+            for key, val in cfg.items(DEFAULTSECT):
+                config_dict[section][key] = val
+            # Copy in the values set in this section
+            for key, val in cfg.items(section):
+                config_dict[section][key] = val
+
+        return self._parse_from_dict(config_dict)
+
     def parse_config_file(
         self, config_file: str, skip_logging_setup: bool = False
     ) -> None:
@@ -224,23 +250,22 @@ def parse_config_file(
         :param config_file: the file to be parsed
         """
         # If the config file doesn't exist, prepopulate the config object
-        # with the defaults, in the DEFAULT section.
+        # with the defaults.
         new_config_file = not os.path.exists(config_file)
 
-        cfg = ConfigParser()
+        cfg = SydentConfigParser()
         for sect, entries in CONFIG_DEFAULTS.items():
             cfg.add_section(sect)
             for k, v in entries.items():
-                cfg.set(DEFAULTSECT if new_config_file else sect, k, v)
-
+                cfg.set(sect, k, v)
         cfg.read(config_file)
 
         # Logging is configured in cfg, but these options must be parsed first
         # so that we can log while parsing the rest
         if not skip_logging_setup:
             setup_logging(cfg)
 
-        needs_updating = self._parse_from_config_parser(cfg)
+        needs_updating = self._parse_from_sydent_config_parser(cfg)
 
         # Don't edit config file when starting Sydent unless it's the first run
         if new_config_file:
@@ -268,17 +293,10 @@ def parse_config_dict(self, config_dict: Dict) -> None:
             for option in section_dict.keys():
                 config[section][option] = config_dict[section][option]
 
-        # Build a ConfigParser from the merged dictionary
-        cfg = ConfigParser()
-        for section, section_dict in config.items():
-            cfg.add_section(section)
-            for option, value in section_dict.items():
-                cfg.set(section, option, value)
-
         # This is only ever called by tests so don't configure logging
         # as tests do this themselves
 
-        self._parse_from_config_parser(cfg)
+        self._parse_from_dict(config)
 
 
 def setup_logging(cfg: ConfigParser) -> None:

sydent/config/_base.py

@@ -13,12 +13,15 @@
 # limitations under the License.
 
 from abc import ABC, abstractmethod
-from configparser import ConfigParser
+from typing import Dict
+
+# The type of dict that the SydentConfigParser object get's converted into
+CONFIG_PARSER_DICT = Dict[str, Dict[str, str]]
 
 
 class BaseConfig(ABC):
     @abstractmethod
-    def parse_config(self, cfg: ConfigParser) -> bool:
+    def parse_config(self, cfg: CONFIG_PARSER_DICT) -> bool:
         """
         Parse the a section of the config
 
@@ -29,3 +32,13 @@ def parse_config(self, cfg: ConfigParser) -> bool:
             config file.
         """
         pass
+
+
+def parse_cfg_bool(value: str):
+    """
+    Parse a string config option into a boolean
+    This method ignores capitalisation
+
+    :param value: the string to be parsed
+    """
+    return value.lower() == "true"

sydent/config/_configparser.py

@@ -0,0 +1,58 @@
+# Copyright 2021 The Matrix.org Foundation C.I.C.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from configparser import BasicInterpolation, ConfigParser, Interpolation
+
+
+class SydentInterpolation(Interpolation):
+    """Interpolation that uses BasicInterpolation with a blacklist"""
+
+    # Options to never interpolate for backwards compatablility
+    BLACLIST = ["email.invite.subject", "email.invite.subject_space"]
+
+    # The BasicInterpolation object to use
+    _basic_interpolation = BasicInterpolation()
+
+    def before_get(self, parser, section, option, value, defaults):
+        if option in self.BLACLIST:
+            return value
+        else:
+            return self._basic_interpolation.before_get(
+                parser, section, option, value, defaults
+            )
+
+    def before_set(self, parser, section, option, value):
+        if option in self.BLACLIST:
+            return value
+        else:
+            return self._basic_interpolation.before_set(parser, section, option, value)
+
+    def before_read(self, parser, section, option, value):
+        if option in self.BLACLIST:
+            return value
+        else:
+            return self._basic_interpolation.before_read(parser, section, option, value)
+
+    def before_write(self, parser, section, option, value):
+        if option in self.BLACLIST:
+            return value
+        else:
+            return self._basic_interpolation.before_write(
+                parser, section, option, value
+            )
+
+
+class SydentConfigParser(ConfigParser):
+
+    _DEFAULT_INTERPOLATION = SydentInterpolation()

sydent/config/crypto.py

@@ -13,27 +13,26 @@
 # limitations under the License.
 
 import logging
-from configparser import ConfigParser
 
 import nacl
 import signedjson.key
 
-from sydent.config._base import BaseConfig
+from sydent.config._base import CONFIG_PARSER_DICT, BaseConfig
 
 logger = logging.getLogger(__name__)
 
 
 class CryptoConfig(BaseConfig):
-    def parse_config(self, cfg: "ConfigParser") -> bool:
+    def parse_config(self, cfg: CONFIG_PARSER_DICT) -> bool:
         """
         Parse the crypto section of the config
         :param cfg: the configuration to be parsed
         """
+        config = cfg.get("crypto")
 
-        signing_key_str = cfg.get("crypto", "ed25519.signingkey")
-        signing_key_parts = signing_key_str.split(" ")
+        signing_key_str = config.get("ed25519.signingkey") or None
 
-        if signing_key_str == "":
+        if signing_key_str is None:
             logger.warning(
                 "'ed25519.signingkey' cannot be blank. Please generate a new"
                 " signing key with the 'generate-key' script."
@@ -42,7 +41,10 @@ def parse_config(self, cfg: "ConfigParser") -> bool:
             self.signing_key = signedjson.key.generate_signing_key("0")
 
             return True
-        elif len(signing_key_parts) == 1:
+
+        signing_key_parts = signing_key_str.split(" ")
+
+        if len(signing_key_parts) == 1:
             # old format key
             logger.warning(
                 "Updating signing key format for this run. Please run the"

sydent/config/database.py

@@ -12,18 +12,18 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from configparser import ConfigParser
-
-from sydent.config._base import BaseConfig
+from sydent.config._base import CONFIG_PARSER_DICT, BaseConfig
 
 
 class DatabaseConfig(BaseConfig):
-    def parse_config(self, cfg: "ConfigParser") -> bool:
+    def parse_config(self, cfg: CONFIG_PARSER_DICT) -> bool:
         """
         Parse the database section of the config
 
         :param cfg: the configuration to be parsed
         """
-        self.database_path = cfg.get("db", "db.file")
+        config = cfg.get("db")
+
+        self.database_path = config.get("db.file")
 
         return False

sydent/config/email.py

@@ -13,56 +13,54 @@
 # limitations under the License.
 
 import socket
-from configparser import ConfigParser
 from typing import Optional
 
-from sydent.config._base import BaseConfig
+from sydent.config._base import CONFIG_PARSER_DICT, BaseConfig
 
 
 class EmailConfig(BaseConfig):
-    def parse_config(self, cfg: "ConfigParser") -> bool:
+    def parse_config(self, cfg: CONFIG_PARSER_DICT) -> bool:
         """
         Parse the email section of the config
 
         :param cfg: the configuration to be parsed
         """
+        config = cfg.get("email")
 
         # These two options are deprecated
-        self.template: Optional[str] = cfg.get("email", "email.template", fallback=None)
+        self.template: Optional[str] = config.get("email.template", None)
 
-        self.invite_template = cfg.get("email", "email.invite_template", fallback=None)
+        self.invite_template = config.get("email.invite_template", None)
 
         # This isn't used anywhere...
-        self.validation_subject = cfg.get("email", "email.subject")
+        self.validation_subject = config.get("email.subject")
 
-        self.invite_subject = cfg.get("email", "email.invite.subject", raw=True)
-        self.invite_subject_space = cfg.get(
-            "email", "email.invite.subject_space", raw=True
-        )
+        # Interpolation is turned off for these two options
+        # This allows them to use %(variable)s substitution without raising errors
+        self.invite_subject = config.get("email.invite.subject")
+        self.invite_subject_space = config.get("email.invite.subject_space")
 
-        self.smtp_server = cfg.get("email", "email.smtphost")
-        self.smtp_port = cfg.get("email", "email.smtpport")
-        self.smtp_username = cfg.get("email", "email.smtpusername")
-        self.smtp_password = cfg.get("email", "email.smtppassword")
-        self.tls_mode = cfg.get("email", "email.tlsmode")
+        self.smtp_server = config.get("email.smtphost")
+        self.smtp_port = config.get("email.smtpport")
+        self.smtp_username = config.get("email.smtpusername")
+        self.smtp_password = config.get("email.smtppassword")
+        self.tls_mode = config.get("email.tlsmode")
 
         # This is the fully qualified domain name for SMTP HELO/EHLO
-        self.host_name = cfg.get("email", "email.hostname")
-        if self.host_name == "":
-            self.host_name = socket.getfqdn()
+        self.host_name = config.get("email.hostname") or socket.getfqdn()
 
-        self.sender = cfg.get("email", "email.from")
+        self.sender = config.get("email.from")
 
-        self.default_web_client_location = cfg.get(
-            "email", "email.default_web_client_location"
+        self.default_web_client_location = config.get(
+            "email.default_web_client_location"
         )
 
-        self.username_obfuscate_characters = cfg.getint(
-            "email", "email.third_party_invite_username_obfuscate_characters"
+        self.username_obfuscate_characters = int(
+            config.get("email.third_party_invite_username_obfuscate_characters")
         )
 
-        self.domain_obfuscate_characters = cfg.getint(
-            "email", "email.third_party_invite_domain_obfuscate_characters"
+        self.domain_obfuscate_characters = int(
+            config.get("email.third_party_invite_domain_obfuscate_characters")
         )
 
         return False