From cf178439ec17fea2e2dbbcbf599f71f3c2f0d6eb Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Tue, 7 Sep 2021 10:59:22 +0100 Subject: [PATCH 01/25] Add SydentConfig class and use when calling Sydent constructor Configuration handling will be slowly moved over to this SydentConfig class and away from the rest of the code. --- scripts/casefold_db.py | 5 ++++- sydent/config/__init__.py | 22 ++++++++++++++++++++++ sydent/sydent.py | 16 ++++++++++++---- tests/utils.py | 5 +++++ 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/scripts/casefold_db.py b/scripts/casefold_db.py index d8646d42..3108d7b5 100755 --- a/scripts/casefold_db.py +++ b/scripts/casefold_db.py @@ -22,6 +22,7 @@ import signedjson.sign +from sydent.config import SydentConfig from sydent.sydent import Sydent, parse_config_file from sydent.util import json_decoder from sydent.util.emailutils import sendEmail @@ -251,8 +252,10 @@ def update_global_associations( config = parse_config_file(args.config_path) + sydent_config = SydentConfig() + reactor = ResolvingMemoryReactorClock() - sydent = Sydent(config, reactor, False) + sydent = Sydent(config, sydent_config, reactor, False) update_global_associations(sydent, sydent.db, not args.no_email, args.dry_run) update_local_associations(sydent, sydent.db, not args.no_email, args.dry_run) diff --git a/sydent/config/__init__.py b/sydent/config/__init__.py index 29eefaf9..11cb4904 100644 --- a/sydent/config/__init__.py +++ b/sydent/config/__init__.py @@ -12,6 +12,28 @@ # See the License for the specific language governing permissions and # limitations under the License. +from configparser import ConfigParser + class ConfigError(Exception): pass + + +class SydentConfig: + """This is the class in charge of handling Sydent's configuration. + Handling of each individual section is delegated to other classes + stored in a `config_sections` list. + """ + + def __init__(self): + self.config_sections = [] + + def _parse_config(self, cfg: ConfigParser) -> None: + """ + Run the parse_config method on each of the objects in + self.config_sections + + :param cfg: the configuration to be parsed + """ + for section in self.config_sections: + section.parse_config(cfg) diff --git a/sydent/sydent.py b/sydent/sydent.py index bf7e44d8..d16e4c5c 100644 --- a/sydent/sydent.py +++ b/sydent/sydent.py @@ -27,6 +27,7 @@ from twisted.internet import address, task from twisted.python import log +from sydent.config import SydentConfig from sydent.db.hashing_metadata import HashingMetadataStore from sydent.db.sqlitedb import SqliteDatabase from sydent.db.valsession import ThreePidValSessionStore @@ -201,14 +202,19 @@ class Sydent: def __init__( - self, cfg, reactor=twisted.internet.reactor, use_tls_for_federation=True + self, + cfg, + sydent_config: SydentConfig, + reactor=twisted.internet.reactor, + use_tls_for_federation=True, ): + self.cfg = cfg + self.config = sydent_config + self.reactor = reactor self.config_file = get_config_file_path() self.use_tls_for_federation = use_tls_for_federation - self.cfg = cfg - logger.info("Starting Sydent server") self.pidfile = self.cfg.get("general", "pidfile.path") @@ -602,5 +608,7 @@ def run_gc(): if __name__ == "__main__": cfg = parse_config_file(get_config_file_path()) setup_logging(cfg) - syd = Sydent(cfg) + + sydent_config = SydentConfig() + syd = Sydent(cfg, sydent_config=sydent_config) syd.run() diff --git a/tests/utils.py b/tests/utils.py index da13b44e..f3f7c5f4 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -23,6 +23,7 @@ from twisted.web.server import Request, Site from zope.interface import implementer +from sydent.config import SydentConfig from sydent.sydent import Sydent, parse_config_dict # Expires on Jan 11 2030 at 17:53:40 GMT @@ -68,9 +69,13 @@ def make_sydent(test_config={}): test_config["db"].setdefault("db.file", ":memory:") reactor = ResolvingMemoryReactorClock() + + sydent_config = SydentConfig() + return Sydent( reactor=reactor, cfg=parse_config_dict(test_config), + sydent_config=sydent_config, use_tls_for_federation=False, ) From 2985b37e49a730124b58eb6640eac07eada30719 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Tue, 7 Sep 2021 11:12:51 +0100 Subject: [PATCH 02/25] Move database config handling over to SydentConfig --- scripts/casefold_db.py | 1 + sydent/config/__init__.py | 14 +++++++++++++- sydent/config/database.py | 28 ++++++++++++++++++++++++++++ sydent/db/sqlitedb.py | 2 +- sydent/sydent.py | 2 ++ tests/utils.py | 5 ++++- 6 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 sydent/config/database.py diff --git a/scripts/casefold_db.py b/scripts/casefold_db.py index 3108d7b5..a4dd0c2d 100755 --- a/scripts/casefold_db.py +++ b/scripts/casefold_db.py @@ -253,6 +253,7 @@ def update_global_associations( config = parse_config_file(args.config_path) sydent_config = SydentConfig() + sydent_config.parse_from_config_parser(config) reactor = ResolvingMemoryReactorClock() sydent = Sydent(config, sydent_config, reactor, False) diff --git a/sydent/config/__init__.py b/sydent/config/__init__.py index 11cb4904..de02c21d 100644 --- a/sydent/config/__init__.py +++ b/sydent/config/__init__.py @@ -14,6 +14,8 @@ from configparser import ConfigParser +from sydent.config.database import DatabaseConfig + class ConfigError(Exception): pass @@ -26,7 +28,9 @@ class SydentConfig: """ def __init__(self): - self.config_sections = [] + self.database = DatabaseConfig() + + self.config_sections = [self.database] def _parse_config(self, cfg: ConfigParser) -> None: """ @@ -37,3 +41,11 @@ def _parse_config(self, cfg: ConfigParser) -> None: """ for section in self.config_sections: section.parse_config(cfg) + + def parse_from_config_parser(self, cfg: ConfigParser) -> None: + """ + Parse the configuration from a ConfigParser object + + :param cfg: the configuration to be parsed + """ + self._parse_config(cfg) diff --git a/sydent/config/database.py b/sydent/config/database.py new file mode 100644 index 00000000..5be2d4e8 --- /dev/null +++ b/sydent/config/database.py @@ -0,0 +1,28 @@ +# 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 typing import TYPE_CHECKING + +if TYPE_CHECKING: + from configparser import ConfigParser + + +class DatabaseConfig: + def parse_config(self, cfg: "ConfigParser") -> None: + """ + Parse the database section of the config + + :param cfg: the configuration to be parsed + """ + self.database_path = cfg.get("db", "db.file") diff --git a/sydent/db/sqlitedb.py b/sydent/db/sqlitedb.py index bfdb7ad9..742a1507 100644 --- a/sydent/db/sqlitedb.py +++ b/sydent/db/sqlitedb.py @@ -28,7 +28,7 @@ class SqliteDatabase: def __init__(self, syd: "Sydent") -> None: self.sydent = syd - dbFilePath = self.sydent.cfg.get("db", "db.file") + dbFilePath = self.sydent.config.database.database_path logger.info("Using DB file %s", dbFilePath) self.db = sqlite3.connect(dbFilePath) diff --git a/sydent/sydent.py b/sydent/sydent.py index d16e4c5c..bac82921 100644 --- a/sydent/sydent.py +++ b/sydent/sydent.py @@ -610,5 +610,7 @@ def run_gc(): setup_logging(cfg) sydent_config = SydentConfig() + sydent_config.parse_from_config_parser(cfg) + syd = Sydent(cfg, sydent_config=sydent_config) syd.run() diff --git a/tests/utils.py b/tests/utils.py index f3f7c5f4..408269ba 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -70,11 +70,14 @@ def make_sydent(test_config={}): reactor = ResolvingMemoryReactorClock() + cfg = parse_config_dict(test_config) + sydent_config = SydentConfig() + sydent_config.parse_from_config_parser(cfg) return Sydent( reactor=reactor, - cfg=parse_config_dict(test_config), + cfg=cfg, sydent_config=sydent_config, use_tls_for_federation=False, ) From 3a1a400e73a613217acf85a393cf520e05657515 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Tue, 7 Sep 2021 11:36:28 +0100 Subject: [PATCH 03/25] Move crypto config handling over to SydentConfig The SydentEd25519 class is no longer needed as all it did was handle the signingkey config option before (which is now done in CryptoConfig) In the current setup, if no key is present a new one is generated and saved back to file. This is a special case for the crypto config and so the line `return self.crypto.save_key` isn't more general. In the future this might not be needed (e.g. by having sydent only do key generation when it first creates a config file for you, rather than everytime one is not found) and so this special casing can be removed. --- sydent/config/__init__.py | 16 +++++- sydent/{sign/ed25519.py => config/crypto.py} | 51 +++++++++++--------- sydent/sign/__init__.py | 0 sydent/sydent.py | 9 ++-- 4 files changed, 49 insertions(+), 27 deletions(-) rename sydent/{sign/ed25519.py => config/crypto.py} (57%) delete mode 100644 sydent/sign/__init__.py diff --git a/sydent/config/__init__.py b/sydent/config/__init__.py index de02c21d..512c7381 100644 --- a/sydent/config/__init__.py +++ b/sydent/config/__init__.py @@ -14,6 +14,7 @@ from configparser import ConfigParser +from sydent.config.crypto import CryptoConfig from sydent.config.database import DatabaseConfig @@ -29,8 +30,12 @@ class SydentConfig: def __init__(self): self.database = DatabaseConfig() + self.crypto = CryptoConfig() - self.config_sections = [self.database] + self.config_sections = [ + self.database, + self.crypto, + ] def _parse_config(self, cfg: ConfigParser) -> None: """ @@ -42,10 +47,17 @@ def _parse_config(self, cfg: ConfigParser) -> None: for section in self.config_sections: section.parse_config(cfg) - def parse_from_config_parser(self, cfg: ConfigParser) -> None: + def parse_from_config_parser(self, cfg: ConfigParser) -> bool: """ Parse the configuration from a ConfigParser object :param cfg: the configuration to be parsed + ... + :return: Whether or not cfg has been changed and needs saving """ self._parse_config(cfg) + + # TODO: Don't alter config file when starting Sydent unless + # user has asked for this specifially (e.g. on first + # run only, or when specify --generate-config) + return self.crypto.save_key diff --git a/sydent/sign/ed25519.py b/sydent/config/crypto.py similarity index 57% rename from sydent/sign/ed25519.py rename to sydent/config/crypto.py index cdb74cf7..363a111f 100644 --- a/sydent/sign/ed25519.py +++ b/sydent/config/crypto.py @@ -1,4 +1,4 @@ -# Copyright 2014 OpenMarket Ltd +# 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. @@ -12,53 +12,60 @@ # See the License for the specific language governing permissions and # limitations under the License. + import logging +from typing import TYPE_CHECKING -import nacl.encoding -import nacl.exceptions -import nacl.signing +import nacl import signedjson.key +if TYPE_CHECKING: + from configparser import ConfigParser + logger = logging.getLogger(__name__) -class SydentEd25519: - def __init__(self, syd): - self.sydent = syd +class CryptoConfig: + def parse_config(self, cfg: "ConfigParser") -> None: + """ + Parse the crypto section of the config + :param cfg: the configuration to be parsed + """ - save_key = False + signing_key_str = cfg.get("crypto", "ed25519.signingkey") + signing_key_parts = signing_key_str.split(" ") - sk_str = self.sydent.cfg.get("crypto", "ed25519.signingkey") - sk_parts = sk_str.split(" ") + self.save_key = False - if sk_str == "": + if signing_key_str == "": logger.info( "This server does not yet have an ed25519 signing key. " - + "Creating one and saving it in the config file." + "Creating one and saving it in the config file." ) + self.signing_key = signedjson.key.generate_signing_key("0") - save_key = True - elif len(sk_parts) == 1: + + self.save_key = True + elif len(signing_key_parts) == 1: # old format key logger.info("Updating signing key format: brace yourselves") + self.signing_key = nacl.signing.SigningKey( - sk_str, encoder=nacl.encoding.HexEncoder + signing_key_str, encoder=nacl.encoding.HexEncoder ) self.signing_key.version = "0" self.signing_key.alg = signedjson.key.NACL_ED25519 - save_key = True + self.save_key = True else: self.signing_key = signedjson.key.decode_signing_key_base64( - sk_parts[0], sk_parts[1], sk_parts[2] + signing_key_parts[0], signing_key_parts[1], signing_key_parts[2] ) - if save_key: - sk_str = "%s %s %s" % ( + if self.save_key: + signing_key_str = "%s %s %s" % ( self.signing_key.alg, self.signing_key.version, signedjson.key.encode_signing_key_base64(self.signing_key), ) - self.sydent.cfg.set("crypto", "ed25519.signingkey", sk_str) - self.sydent.save_config() - logger.info("Key saved") + cfg.set("crypto", "ed25519.signingkey", signing_key_str) diff --git a/sydent/sign/__init__.py b/sydent/sign/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/sydent/sydent.py b/sydent/sydent.py index bac82921..04a34638 100644 --- a/sydent/sydent.py +++ b/sydent/sydent.py @@ -69,7 +69,6 @@ from sydent.http.servlets.v1_servlet import V1Servlet from sydent.http.servlets.v2_servlet import V2Servlet from sydent.replication.pusher import Pusher -from sydent.sign.ed25519 import SydentEd25519 from sydent.threepid.bind import ThreepidBinder from sydent.util.hash import sha256_and_url_safe_base64 from sydent.util.ip_range import DEFAULT_IP_RANGE_BLACKLIST, generate_ip_set @@ -326,7 +325,7 @@ def __init__( self.validators.msisdn = MsisdnValidator(self) self.keyring = Keyring() - self.keyring.ed25519 = SydentEd25519(self).signing_key + self.keyring.ed25519 = self.config.crypto.signing_key self.keyring.ed25519.alg = "ed25519" self.sig_verifier = Verifier(self) @@ -610,7 +609,11 @@ def run_gc(): setup_logging(cfg) sydent_config = SydentConfig() - sydent_config.parse_from_config_parser(cfg) + cfg_needs_saving = sydent_config.parse_from_config_parser(cfg) syd = Sydent(cfg, sydent_config=sydent_config) + + if cfg_needs_saving: + syd.save_config() + syd.run() From 91fdc6b836b164c6ea4e094f8be02f3e835a209d Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Tue, 7 Sep 2021 11:59:58 +0100 Subject: [PATCH 04/25] Move sms config handling over to SydentConfig --- sydent/config/__init__.py | 3 ++ sydent/config/sms.py | 71 ++++++++++++++++++++++++++++ sydent/sms/openmarket.py | 6 +-- sydent/validators/msisdnvalidator.py | 42 ++-------------- 4 files changed, 80 insertions(+), 42 deletions(-) create mode 100644 sydent/config/sms.py diff --git a/sydent/config/__init__.py b/sydent/config/__init__.py index 512c7381..1bc735f3 100644 --- a/sydent/config/__init__.py +++ b/sydent/config/__init__.py @@ -16,6 +16,7 @@ from sydent.config.crypto import CryptoConfig from sydent.config.database import DatabaseConfig +from sydent.config.sms import SMSConfig class ConfigError(Exception): @@ -31,10 +32,12 @@ class SydentConfig: def __init__(self): self.database = DatabaseConfig() self.crypto = CryptoConfig() + self.sms = SMSConfig() self.config_sections = [ self.database, self.crypto, + self.sms, ] def _parse_config(self, cfg: ConfigParser) -> None: diff --git a/sydent/config/sms.py b/sydent/config/sms.py new file mode 100644 index 00000000..f7368240 --- /dev/null +++ b/sydent/config/sms.py @@ -0,0 +1,71 @@ +# 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 typing import TYPE_CHECKING, Dict, List + +if TYPE_CHECKING: + from configparser import ConfigParser + + +class SMSConfig: + def parse_config(self, cfg: "ConfigParser") -> None: + """ + Parse the sms section of the config + + :param cfg: the configuration to be parsed + """ + self.body_template = cfg.get("sms", "bodyTemplate") + + # Make sure username and password are bytes otherwise we can't use them with + # b64encode. + self.api_username = cfg.get("sms", "username").encode("UTF-8") + self.api_password = cfg.get("sms", "password").encode("UTF-8") + + self.originators: Dict[str, List[Dict[str, str]]] = {} + self.smsRules = {} + + for opt in cfg.options("sms"): + if opt.startswith("originators."): + country = opt.split(".")[1] + rawVal = cfg.get("sms", opt) + rawList = [i.strip() for i in rawVal.split(",")] + + self.originators[country] = [] + for origString in rawList: + parts = origString.split(":") + if len(parts) != 2: + raise Exception( + "Originators must be in form: long:, short: or alpha:, separated by commas" + ) + if parts[0] not in ["long", "short", "alpha"]: + raise Exception( + "Invalid originator type: valid types are long, short and alpha" + ) + self.originators[country].append( + { + "type": parts[0], + "text": parts[1], + } + ) + elif opt.startswith("smsrule."): + country = opt.split(".")[1] + action = cfg.get("sms", opt) + + if action not in ["allow", "reject"]: + raise Exception( + "Invalid SMS rule action: %s, expecting 'allow' or 'reject'" + % action + ) + + self.smsRules[country] = action diff --git a/sydent/sms/openmarket.py b/sydent/sms/openmarket.py index 55bad951..f2c93686 100644 --- a/sydent/sms/openmarket.py +++ b/sydent/sms/openmarket.py @@ -83,10 +83,8 @@ async def sendTextSMS( "address": source["text"], } - # Make sure username and password are bytes otherwise we can't use them with - # b64encode. - username = self.sydent.cfg.get("sms", "username").encode("UTF-8") - password = self.sydent.cfg.get("sms", "password").encode("UTF-8") + username = self.sydent.config.sms.api_username + password = self.sydent.config.sms.api_password b64creds = b64encode(b"%s:%s" % (username, password)) req_headers = Headers( diff --git a/sydent/validators/msisdnvalidator.py b/sydent/validators/msisdnvalidator.py index 2561d731..541d2c31 100644 --- a/sydent/validators/msisdnvalidator.py +++ b/sydent/validators/msisdnvalidator.py @@ -14,7 +14,7 @@ # limitations under the License. import logging -from typing import TYPE_CHECKING, Dict, List, Optional +from typing import TYPE_CHECKING, Dict, Optional import phonenumbers # type: ignore @@ -35,42 +35,8 @@ def __init__(self, sydent: "Sydent") -> None: self.omSms = OpenMarketSMS(sydent) # cache originators & sms rules from config file - self.originators: Dict[str, List[Dict[str, str]]] = {} - self.smsRules = {} - for opt in self.sydent.cfg.options("sms"): - if opt.startswith("originators."): - country = opt.split(".")[1] - rawVal = self.sydent.cfg.get("sms", opt) - rawList = [i.strip() for i in rawVal.split(",")] - - self.originators[country] = [] - for origString in rawList: - parts = origString.split(":") - if len(parts) != 2: - raise Exception( - "Originators must be in form: long:, short: or alpha:, separated by commas" - ) - if parts[0] not in ["long", "short", "alpha"]: - raise Exception( - "Invalid originator type: valid types are long, short and alpha" - ) - self.originators[country].append( - { - "type": parts[0], - "text": parts[1], - } - ) - elif opt.startswith("smsrule."): - country = opt.split(".")[1] - action = self.sydent.cfg.get("sms", opt) - - if action not in ["allow", "reject"]: - raise Exception( - "Invalid SMS rule action: %s, expecting 'allow' or 'reject'" - % action - ) - - self.smsRules[country] = action + self.originators = self.sydent.config.sms.originators + self.smsRules = self.sydent.config.sms.smsRules def requestToken( self, @@ -115,7 +81,7 @@ def requestToken( ) return valSession.id - smsBodyTemplate = self.sydent.cfg.get("sms", "bodyTemplate") + smsBodyTemplate = self.sydent.config.sms.body_template originator = self.getOriginator(phoneNumber) logger.info( From 4b0a900ee456da471fd4278d1de702ca61d22725 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Tue, 7 Sep 2021 13:20:52 +0100 Subject: [PATCH 05/25] Move deprecated email template config over to SydentConfig --- scripts/casefold_db.py | 15 ++++---- sydent/config/__init__.py | 3 ++ sydent/config/email.py | 36 ++++++++++++++++++++ sydent/http/servlets/store_invite_servlet.py | 14 +++++--- sydent/validators/emailvalidator.py | 13 ++++--- tests/test_casefold_migration.py | 14 +++++--- tests/test_jinja_templates.py | 26 ++++++++------ 7 files changed, 90 insertions(+), 31 deletions(-) create mode 100644 sydent/config/email.py diff --git a/scripts/casefold_db.py b/scripts/casefold_db.py index a4dd0c2d..3a59b5f1 100755 --- a/scripts/casefold_db.py +++ b/scripts/casefold_db.py @@ -40,7 +40,7 @@ def calculate_lookup_hash(sydent, address): def update_local_associations( - sydent, db: sqlite3.Connection, send_email: bool, dry_run: bool + sydent: Sydent, db: sqlite3.Connection, send_email: bool, dry_run: bool ): """Update the DB table local_threepid_associations so that all stored emails are casefolded, and any duplicate mxid's associated with the @@ -105,11 +105,14 @@ def update_local_associations( if mxid in processed_mxids: continue else: - templateFile = sydent.get_branded_template( - None, - "migration_template.eml", - ("email", "email.template"), - ) + if sydent.config.email.template is None: + templateFile = sydent.get_branded_template( + None, + "migration_template.eml", + ("email", "email.template"), + ) + else: + templateFile = sydent.config.email.template sendEmail( sydent, diff --git a/sydent/config/__init__.py b/sydent/config/__init__.py index 1bc735f3..3ce91c3c 100644 --- a/sydent/config/__init__.py +++ b/sydent/config/__init__.py @@ -16,6 +16,7 @@ from sydent.config.crypto import CryptoConfig from sydent.config.database import DatabaseConfig +from sydent.config.email import EmailConfig from sydent.config.sms import SMSConfig @@ -33,11 +34,13 @@ def __init__(self): self.database = DatabaseConfig() self.crypto = CryptoConfig() self.sms = SMSConfig() + self.email = EmailConfig() self.config_sections = [ self.database, self.crypto, self.sms, + self.email, ] def _parse_config(self, cfg: ConfigParser) -> None: diff --git a/sydent/config/email.py b/sydent/config/email.py new file mode 100644 index 00000000..ecdde989 --- /dev/null +++ b/sydent/config/email.py @@ -0,0 +1,36 @@ +# 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 typing import TYPE_CHECKING + +if TYPE_CHECKING: + from configparser import ConfigParser + + +class EmailConfig: + def parse_config(self, cfg: "ConfigParser") -> None: + """ + Parse the email section of the config + + :param cfg: the configuration to be parsed + """ + + # These two options are deprecated + self.template = None + if cfg.has_option("email", "email.template"): + self.template = cfg.get("email", "email.template") + + self.invite_template = None + if cfg.has_option("email", "email.invite_template"): + self.invite_template = cfg.get("email", "email.invite_template") diff --git a/sydent/http/servlets/store_invite_servlet.py b/sydent/http/servlets/store_invite_servlet.py index 3277db96..a1b6d02d 100644 --- a/sydent/http/servlets/store_invite_servlet.py +++ b/sydent/http/servlets/store_invite_servlet.py @@ -158,11 +158,15 @@ def render_POST(self, request: Request) -> JsonDict: substitutions["subject_header_value"] = subject_header.encode() brand = self.sydent.brand_from_request(request) - templateFile = self.sydent.get_branded_template( - brand, - "invite_template.eml", - ("email", "email.invite_template"), - ) + + if self.sydent.config.email.invite_template is None: + templateFile = self.sydent.get_branded_template( + brand, + "invite_template.eml", + ("email", "email.invite_template"), + ) + else: + templateFile = self.sydent.config.email.invite_template sendEmail(self.sydent, templateFile, normalised_address, substitutions) diff --git a/sydent/validators/emailvalidator.py b/sydent/validators/emailvalidator.py index a34b3099..839297d6 100644 --- a/sydent/validators/emailvalidator.py +++ b/sydent/validators/emailvalidator.py @@ -63,11 +63,14 @@ def requestToken( valSessionStore.setMtime(valSession.id, time_msec()) - templateFile = self.sydent.get_branded_template( - brand, - "verification_template.eml", - ("email", "email.template"), - ) + if self.sydent.config.email.template is None: + templateFile = self.sydent.get_branded_template( + brand, + "verification_template.eml", + ("email", "email.template"), + ) + else: + templateFile = self.sydent.config.email.template if int(valSession.sendAttemptNumber) >= int(sendAttempt): logger.info( diff --git a/tests/test_casefold_migration.py b/tests/test_casefold_migration.py index 6500232b..d0916615 100644 --- a/tests/test_casefold_migration.py +++ b/tests/test_casefold_migration.py @@ -171,11 +171,15 @@ def setUp(self): def test_migration_email(self): with patch("sydent.util.emailutils.smtplib") as smtplib: - templateFile = self.sydent.get_branded_template( - None, - "migration_template.eml", - ("email", "email.template"), - ) + if self.sydent.config.email.template is None: + templateFile = self.sydent.get_branded_template( + None, + "migration_template.eml", + ("email", "email.template"), + ) + else: + templateFile = self.sydent.config.email.template + sendEmail( self.sydent, templateFile, diff --git a/tests/test_jinja_templates.py b/tests/test_jinja_templates.py index 6736f913..78b71872 100644 --- a/tests/test_jinja_templates.py +++ b/tests/test_jinja_templates.py @@ -54,11 +54,14 @@ def test_jinja_vector_invite(self): "room_type": "", } - templateFile = self.sydent.get_branded_template( - "vector-im", - "invite_template.eml", - ("email", "email.invite_template"), - ) + if self.sydent.config.email.invite_template is None: + templateFile = self.sydent.get_branded_template( + "vector-im", + "invite_template.eml", + ("email", "email.invite_template"), + ) + else: + templateFile = self.sydent.config.email.invite_template with patch("sydent.util.emailutils.smtplib") as smtplib: sendEmail(self.sydent, templateFile, "test@test.com", substitutions) @@ -117,11 +120,14 @@ def test_jinja_matrix_invite(self): "room_type": "", } - templateFile = self.sydent.get_branded_template( - "matrix-org", - "invite_template.eml", - ("email", "email.invite_template"), - ) + if self.sydent.config.email.invite_template is None: + templateFile = self.sydent.get_branded_template( + "matrix-org", + "invite_template.eml", + ("email", "email.invite_template"), + ) + else: + templateFile = self.sydent.config.email.invite_template with patch("sydent.util.emailutils.smtplib") as smtplib: sendEmail(self.sydent, templateFile, "test@test.com", substitutions) From 21b030ad3a95b036cf34338ba4f76037805132f7 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Tue, 7 Sep 2021 13:29:33 +0100 Subject: [PATCH 06/25] Move email config handled by synapse.py over to SynapseConfig --- sydent/config/email.py | 12 ++++++++++++ sydent/http/servlets/store_invite_servlet.py | 10 +++++++--- sydent/sydent.py | 13 ------------- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/sydent/config/email.py b/sydent/config/email.py index ecdde989..7d234540 100644 --- a/sydent/config/email.py +++ b/sydent/config/email.py @@ -34,3 +34,15 @@ def parse_config(self, cfg: "ConfigParser") -> None: self.invite_template = None if cfg.has_option("email", "email.invite_template"): self.invite_template = cfg.get("email", "email.invite_template") + + self.default_web_client_location = cfg.get( + "email", "email.default_web_client_location" + ) + + self.username_obfuscate_characters = cfg.getint( + "email", "email.third_party_invite_username_obfuscate_characters" + ) + + self.domain_obfuscate_characters = cfg.getint( + "email", "email.third_party_invite_domain_obfuscate_characters" + ) diff --git a/sydent/http/servlets/store_invite_servlet.py b/sydent/http/servlets/store_invite_servlet.py index a1b6d02d..f0e630a3 100644 --- a/sydent/http/servlets/store_invite_servlet.py +++ b/sydent/http/servlets/store_invite_servlet.py @@ -138,7 +138,9 @@ def render_POST(self, request: Request) -> JsonDict: if substitutions["room_name"] != "": substitutions["bracketed_room_name"] = "(%s) " % substitutions["room_name"] - substitutions["web_client_location"] = self.sydent.default_web_client_location + substitutions[ + "web_client_location" + ] = self.sydent.config.email.default_web_client_location if "org.matrix.web_client_location" in substitutions: substitutions["web_client_location"] = substitutions.pop( "org.matrix.web_client_location" @@ -214,9 +216,11 @@ def redact_email_address(self, address: str) -> str: # Obfuscate strings redacted_username = self._redact( - username, self.sydent.username_obfuscate_characters + username, self.sydent.config.email.username_obfuscate_characters + ) + redacted_domain = self._redact( + domain, self.sydent.config.email.domain_obfuscate_characters ) - redacted_domain = self._redact(domain, self.sydent.domain_obfuscate_characters) return redacted_username + "@" + redacted_domain diff --git a/sydent/sydent.py b/sydent/sydent.py index 04a34638..6c6e1308 100644 --- a/sydent/sydent.py +++ b/sydent/sydent.py @@ -288,19 +288,6 @@ def __init__( self.ip_blacklist = generate_ip_set(ip_blacklist) self.ip_whitelist = generate_ip_set(ip_whitelist) - self.default_web_client_location = self.cfg.get( - "email", "email.default_web_client_location" - ) - self.username_obfuscate_characters = int( - self.cfg.get( - "email", "email.third_party_invite_username_obfuscate_characters" - ) - ) - self.domain_obfuscate_characters = int( - self.cfg.get( - "email", "email.third_party_invite_domain_obfuscate_characters" - ) - ) self.template_environment = Environment( loader=FileSystemLoader(self.cfg.get("general", "templates.path")), autoescape=True, From 11f871198013e679a3ea77cc65850304c3c3993b Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Tue, 7 Sep 2021 13:37:05 +0100 Subject: [PATCH 07/25] Move rest of email config over to SydenConfig --- sydent/config/email.py | 22 ++++++++++++++++++++ sydent/http/servlets/store_invite_servlet.py | 18 ++++++---------- sydent/util/emailutils.py | 18 +++++++--------- 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/sydent/config/email.py b/sydent/config/email.py index 7d234540..d54ba11f 100644 --- a/sydent/config/email.py +++ b/sydent/config/email.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import socket from typing import TYPE_CHECKING if TYPE_CHECKING: @@ -35,6 +36,27 @@ def parse_config(self, cfg: "ConfigParser") -> None: if cfg.has_option("email", "email.invite_template"): self.invite_template = cfg.get("email", "email.invite_template") + # This isn't used anywhere... + self.validation_subject = cfg.get("email", "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 + ) + + 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") + + # 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.sender = cfg.get("email", "email.from") + self.default_web_client_location = cfg.get( "email", "email.default_web_client_location" ) diff --git a/sydent/http/servlets/store_invite_servlet.py b/sydent/http/servlets/store_invite_servlet.py index f0e630a3..e0833d2c 100644 --- a/sydent/http/servlets/store_invite_servlet.py +++ b/sydent/http/servlets/store_invite_servlet.py @@ -146,18 +146,12 @@ def render_POST(self, request: Request) -> JsonDict: "org.matrix.web_client_location" ) - subject_header = Header( - self.sydent.cfg.get( - "email", - "email.invite.subject_space" - if substitutions["room_type"] == "m.space" - else "email.invite.subject", - raw=True, - ) - % substitutions, - "utf8", - ) - substitutions["subject_header_value"] = subject_header.encode() + if substitutions["room_type"] == "m.space": + subject = self.sydent.config.email.invite_subject_space % substitutions + else: + subject = self.sydent.config.email.invite_subject % substitutions + + substitutions["subject_header_value"] = Header(subject, "utf8").encode() brand = self.sydent.brand_from_request(request) diff --git a/sydent/util/emailutils.py b/sydent/util/emailutils.py index 6e532dc3..da54548e 100644 --- a/sydent/util/emailutils.py +++ b/sydent/util/emailutils.py @@ -16,7 +16,6 @@ import logging import random import smtplib -import socket import string import urllib from html import escape @@ -46,11 +45,9 @@ def sendEmail( :param mailTo: The email address to send the email to. :param substitutions: The substitutions to use with the template. """ - mailFrom = sydent.cfg.get("email", "email.from") + mailFrom = sydent.config.email.sender + myHostname = sydent.config.email.host_name - myHostname = sydent.cfg.get("email", "email.hostname") - if myHostname == "": - myHostname = socket.getfqdn() midRandom = "".join([random.choice(string.ascii_letters) for _ in range(16)]) messageid = "<%d%s@%s>" % (time_msec(), midRandom, myHostname) @@ -90,11 +87,12 @@ def sendEmail( logger.info("Parsed to address changed the address: %s -> %s", mailTo, parsedTo) raise EmailAddressException() - mailServer = sydent.cfg.get("email", "email.smtphost") - mailPort = sydent.cfg.get("email", "email.smtpport") - mailUsername = sydent.cfg.get("email", "email.smtpusername") - mailPassword = sydent.cfg.get("email", "email.smtppassword") - mailTLSMode = sydent.cfg.get("email", "email.tlsmode") + mailServer = sydent.config.email.smtp_server + mailPort = sydent.config.email.smtp_port + mailUsername = sydent.config.email.smtp_username + mailPassword = sydent.config.email.smtp_password + mailTLSMode = sydent.config.email.tls_mode + logger.info( "Sending mail to %s with mail server: %s" % ( From f09779b2d35d18770479dbfa85650b447e1b86a3 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Tue, 7 Sep 2021 13:46:26 +0100 Subject: [PATCH 08/25] Move deprecated http template config over to SydentConfig --- sydent/config/__init__.py | 3 +++ sydent/config/http.py | 31 +++++++++++++++++++++++++++ sydent/http/servlets/emailservlet.py | 13 ++++++----- sydent/http/servlets/msisdnservlet.py | 13 ++++++----- 4 files changed, 50 insertions(+), 10 deletions(-) create mode 100644 sydent/config/http.py diff --git a/sydent/config/__init__.py b/sydent/config/__init__.py index 3ce91c3c..4f973a70 100644 --- a/sydent/config/__init__.py +++ b/sydent/config/__init__.py @@ -17,6 +17,7 @@ from sydent.config.crypto import CryptoConfig from sydent.config.database import DatabaseConfig from sydent.config.email import EmailConfig +from sydent.config.http import HTTPConfig from sydent.config.sms import SMSConfig @@ -35,12 +36,14 @@ def __init__(self): self.crypto = CryptoConfig() self.sms = SMSConfig() self.email = EmailConfig() + self.http = HTTPConfig() self.config_sections = [ self.database, self.crypto, self.sms, self.email, + self.http, ] def _parse_config(self, cfg: ConfigParser) -> None: diff --git a/sydent/config/http.py b/sydent/config/http.py new file mode 100644 index 00000000..d8fe1bcd --- /dev/null +++ b/sydent/config/http.py @@ -0,0 +1,31 @@ +# 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 typing import TYPE_CHECKING + +if TYPE_CHECKING: + from configparser import ConfigParser + + +class HTTPConfig: + def parse_config(self, cfg: "ConfigParser") -> None: + """ + Parse the http section of the config + + :param cfg: the configuration to be parsed + """ + # This option is deprecated + self.verify_response_template = None + if cfg.has_option("http", "verify_response_template"): + self.verify_response_template = cfg.get("http", "verify_response_template") diff --git a/sydent/http/servlets/emailservlet.py b/sydent/http/servlets/emailservlet.py index 9b8abe8a..45c15219 100644 --- a/sydent/http/servlets/emailservlet.py +++ b/sydent/http/servlets/emailservlet.py @@ -121,11 +121,14 @@ def render_GET(self, request: Request) -> bytes: msg = "Verification failed: you may need to request another verification email" brand = self.sydent.brand_from_request(request) - templateFile = self.sydent.get_branded_template( - brand, - "verify_response_template.html", - ("http", "verify_response_template"), - ) + if self.sydent.config.http.verify_response_template is None: + templateFile = self.sydent.get_branded_template( + brand, + "verify_response_template.html", + ("http", "verify_response_template"), + ) + else: + templateFile = self.sydent.config.http.verify_response_template request.setHeader("Content-Type", "text/html") res = open(templateFile).read() % {"message": msg} diff --git a/sydent/http/servlets/msisdnservlet.py b/sydent/http/servlets/msisdnservlet.py index 3a21a1c9..c223c933 100644 --- a/sydent/http/servlets/msisdnservlet.py +++ b/sydent/http/servlets/msisdnservlet.py @@ -144,11 +144,14 @@ def render_GET(self, request: Request) -> str: msg = "Verification failed: you may need to request another verification text" brand = self.sydent.brand_from_request(request) - templateFile = self.sydent.get_branded_template( - brand, - "verify_response_template.html", - ("http", "verify_response_template"), - ) + if self.sydent.config.http.verify_response_template is None: + templateFile = self.sydent.get_branded_template( + brand, + "verify_response_template.html", + ("http", "verify_response_template"), + ) + else: + templateFile = self.sydent.config.http.verify_response_template request.setHeader("Content-Type", "text/html") return open(templateFile).read() % {"message": msg} From 25aac48b94e2b858b7d193750a61063f2aacbe60 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Tue, 7 Sep 2021 14:12:30 +0100 Subject: [PATCH 09/25] Move rest of http config handling over to SynapseConfig --- sydent/config/http.py | 42 ++++++++++++++++++++ sydent/http/federation_tls_options.py | 3 +- sydent/http/httpclient.py | 2 +- sydent/http/httpcommon.py | 8 ++-- sydent/http/httpserver.py | 9 +++-- sydent/http/servlets/store_invite_servlet.py | 2 +- sydent/replication/peer.py | 10 ++--- sydent/sydent.py | 18 ++++----- sydent/validators/emailvalidator.py | 2 +- 9 files changed, 66 insertions(+), 30 deletions(-) diff --git a/sydent/config/http.py b/sydent/config/http.py index d8fe1bcd..d145e75e 100644 --- a/sydent/config/http.py +++ b/sydent/config/http.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +from configparser import NoOptionError from typing import TYPE_CHECKING if TYPE_CHECKING: @@ -29,3 +30,44 @@ def parse_config(self, cfg: "ConfigParser") -> None: self.verify_response_template = None if cfg.has_option("http", "verify_response_template"): self.verify_response_template = cfg.get("http", "verify_response_template") + + self.client_bind_address = cfg.get("http", "clientapi.http.bind_address") + self.client_port = cfg.getint("http", "clientapi.http.port") + + # internal port is allowed to be set to an empty string in the config + self.internal_port = cfg.get("http", "internalapi.http.port") + if self.internal_port: + self.internal_api_enabled = True + self.internal_port = int(self.internal_port) + try: + self.internal_bind_address = cfg.get( + "http", "internalapi.http.bind_address" + ) + except NoOptionError: + self.internal_bind_address = "::1" + else: + self.internal_api_enabled = False + + self.cert_file = cfg.get("http", "replication.https.certfile") + self.ca_cert_File = cfg.get("http", "replication.https.cacert") + + self.replication_bind_address = cfg.get( + "http", "replication.https.bind_address" + ) + self.replication_port = cfg.getint("http", "replication.https.port") + + self.obey_x_forwarded_for = cfg.get("http", "obey_x_forwarded_for") + + self.verify_federation_certs = cfg.getboolean("http", "federation.verifycerts") + + self.server_http_url_base = cfg.get("http", "client_http_base") + + self.base_replication_urls = {} + + for section in cfg.sections(): + if section.startswith("peer."): + # peer name is all the characters after 'peer.' + peer = section[5:] + if cfg.has_option(section, "base_replication_url"): + base_url = cfg.get(section, "base_replication_url") + self.base_replication_urls[peer] = base_url diff --git a/sydent/http/federation_tls_options.py b/sydent/http/federation_tls_options.py index 54d9ce58..c7e55042 100644 --- a/sydent/http/federation_tls_options.py +++ b/sydent/http/federation_tls_options.py @@ -89,8 +89,7 @@ class ClientTLSOptionsFactory: """Factory for Twisted ClientTLSOptions that are used to make connections to remote servers for federation.""" - def __init__(self, config): - verify_requests = config.getboolean("http", "federation.verifycerts") + def __init__(self, verify_requests): if verify_requests: self._options = ssl.CertificateOptions(trustRoot=ssl.platformTrust()) else: diff --git a/sydent/http/httpclient.py b/sydent/http/httpclient.py index adb6a477..eea4b9bb 100644 --- a/sydent/http/httpclient.py +++ b/sydent/http/httpclient.py @@ -145,7 +145,7 @@ def __init__(self, sydent: "Sydent") -> None: ip_whitelist=sydent.ip_whitelist, ip_blacklist=sydent.ip_blacklist, ), - ClientTLSOptionsFactory(sydent.cfg) + ClientTLSOptionsFactory(sydent.config.http.verify_federation_certs) if sydent.use_tls_for_federation else None, ) diff --git a/sydent/http/httpcommon.py b/sydent/http/httpcommon.py index 868a4c64..c4cf3e88 100644 --- a/sydent/http/httpcommon.py +++ b/sydent/http/httpcommon.py @@ -42,9 +42,9 @@ def __init__(self, sydent: "Sydent") -> None: self.trustRoot = self.makeTrustRoot() def makeMyCertificate(self): - privKeyAndCertFilename = self.sydent.cfg.get( - "http", "replication.https.certfile" - ) + # TODO Move some of this loading into parse_config + privKeyAndCertFilename = self.sydent.config.http.cert_file + if privKeyAndCertFilename == "": logger.warning( "No HTTPS private key / cert found: not starting replication server " @@ -69,7 +69,7 @@ def makeMyCertificate(self): def makeTrustRoot(self): # If this option is specified, use a specific root CA cert. This is useful for testing when it's not # practical to get the client cert signed by a real root CA but should never be used on a production server. - caCertFilename = self.sydent.cfg.get("http", "replication.https.cacert") + caCertFilename = self.sydent.config.http.ca_cert_File if len(caCertFilename) > 0: try: fp = open(caCertFilename) diff --git a/sydent/http/httpserver.py b/sydent/http/httpserver.py index 9a9acba9..541986c3 100644 --- a/sydent/http/httpserver.py +++ b/sydent/http/httpserver.py @@ -139,8 +139,9 @@ def __init__(self, sydent: "Sydent") -> None: self.factory.displayTracebacks = False def setup(self): - httpPort = int(self.sydent.cfg.get("http", "clientapi.http.port")) - interface = self.sydent.cfg.get("http", "clientapi.http.bind_address") + httpPort = self.sydent.config.http.client_port + interface = self.sydent.config.http.client_bind_address + logger.info("Starting Client API HTTP server on %s:%d", interface, httpPort) self.sydent.reactor.listenTCP( httpPort, @@ -199,8 +200,8 @@ def __init__(self, sydent: "Sydent") -> None: self.factory.displayTracebacks = False def setup(self): - httpPort = int(self.sydent.cfg.get("http", "replication.https.port")) - interface = self.sydent.cfg.get("http", "replication.https.bind_address") + httpPort = self.sydent.config.http.replication_port + interface = self.sydent.config.http.replication_bind_address if self.sydent.sslComponents.myPrivateCertificate: # We will already have logged a warn if this is absent, so don't do it again diff --git a/sydent/http/servlets/store_invite_servlet.py b/sydent/http/servlets/store_invite_servlet.py index e0833d2c..c6ee1c42 100644 --- a/sydent/http/servlets/store_invite_servlet.py +++ b/sydent/http/servlets/store_invite_servlet.py @@ -170,7 +170,7 @@ def render_POST(self, request: Request) -> JsonDict: pubKeyBase64 = encode_base64(pubKey.encode()) baseUrl = "%s/_matrix/identity/api/v1" % ( - self.sydent.cfg.get("http", "client_http_base"), + self.sydent.config.http.server_http_url_base, ) keysToReturn = [] diff --git a/sydent/replication/peer.py b/sydent/replication/peer.py index 2dfce56c..8d20a93e 100644 --- a/sydent/replication/peer.py +++ b/sydent/replication/peer.py @@ -14,7 +14,6 @@ # limitations under the License. import binascii -import configparser import json import logging from typing import TYPE_CHECKING, Any, Dict @@ -138,12 +137,9 @@ def __init__( self.lastSentVersion = lastSentVersion # look up or build the replication URL - try: - replication_url = sydent.cfg.get( - "peer.%s" % server_name, - "base_replication_url", - ) - except (configparser.NoSectionError, configparser.NoOptionError): + replication_url = self.sydent.config.http.base_replication_urls.get(server_name) + + if replication_url is None: if not port: port = 1001 replication_url = "https://%s:%i" % (server_name, port) diff --git a/sydent/sydent.py b/sydent/sydent.py index 6c6e1308..11bce48e 100644 --- a/sydent/sydent.py +++ b/sydent/sydent.py @@ -395,14 +395,12 @@ def run(self): self.replicationHttpsServer.setup() self.pusher.setup() - internalport = self.cfg.get("http", "internalapi.http.port") - if internalport: - try: - interface = self.cfg.get("http", "internalapi.http.bind_address") - except configparser.NoOptionError: - interface = "::1" + if self.config.http.internal_api_enabled: + internalport = self.config.http.internal_port + interface = self.config.http.internal_bind_address + self.internalApiHttpServer = InternalApiHttpServer(self) - self.internalApiHttpServer.setup(interface, int(internalport)) + self.internalApiHttpServer.setup(interface, internalport) if self.pidfile: with open(self.pidfile, "w") as pidfile: @@ -411,9 +409,9 @@ def run(self): self.reactor.run() def ip_from_request(self, request): - if self.cfg.get( - "http", "obey_x_forwarded_for" - ) and request.requestHeaders.hasHeader("X-Forwarded-For"): + if self.config.http.obey_x_forwarded_for and request.requestHeaders.hasHeader( + "X-Forwarded-For" + ): return request.requestHeaders.getRawHeaders("X-Forwarded-For")[0] client = request.getClientAddress() if isinstance(client, (address.IPv4Address, address.IPv6Address)): diff --git a/sydent/validators/emailvalidator.py b/sydent/validators/emailvalidator.py index 839297d6..b2ef9664 100644 --- a/sydent/validators/emailvalidator.py +++ b/sydent/validators/emailvalidator.py @@ -112,7 +112,7 @@ def makeValidateLink( :return: The validation link. """ - base = self.sydent.cfg.get("http", "client_http_base") + base = self.sydent.config.http.server_http_url_base link = "%s/_matrix/identity/api/v1/validate/email/submitToken?token=%s&client_secret=%s&sid=%d" % ( base, urllib.parse.quote(valSession.token), From 1492c1c2fc79ecfefddd0ad2241ce03f7c0f5305 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Tue, 7 Sep 2021 14:32:37 +0100 Subject: [PATCH 10/25] Move server name config handling to SydentConfig --- scripts/casefold_db.py | 6 +-- sydent/config/__init__.py | 3 ++ sydent/config/general.py | 39 +++++++++++++++++++ sydent/hs_federation/verifier.py | 2 +- .../http/servlets/blindlysignstuffservlet.py | 2 +- sydent/http/servlets/lookupservlet.py | 6 ++- sydent/replication/peer.py | 4 +- sydent/sydent.py | 16 +------- sydent/threepid/bind.py | 4 +- sydent/threepid/signer.py | 2 +- tests/test_casefold_migration.py | 2 +- 11 files changed, 59 insertions(+), 27 deletions(-) create mode 100644 sydent/config/general.py diff --git a/scripts/casefold_db.py b/scripts/casefold_db.py index 3a59b5f1..daa5fa04 100755 --- a/scripts/casefold_db.py +++ b/scripts/casefold_db.py @@ -143,7 +143,7 @@ def update_local_associations( def update_global_associations( - sydent, db: sqlite3.Connection, send_email: bool, dry_run: bool + sydent: Sydent, db: sqlite3.Connection, send_email: bool, dry_run: bool ): """Update the DB table global_threepid_associations so that all stored emails are casefolded, the signed association is re-signed and any duplicate @@ -153,7 +153,7 @@ def update_global_associations( """ # get every row where the local server is origin server and medium is email - origin_server = sydent.server_name + origin_server = sydent.config.general.server_name medium = "email" cur = db.cursor() @@ -180,7 +180,7 @@ def update_global_associations( sg_assoc["address"] = address.casefold() sg_assoc = json.dumps( signedjson.sign.sign_json( - sg_assoc, sydent.server_name, sydent.keyring.ed25519 + sg_assoc, sydent.config.general.server_name, sydent.keyring.ed25519 ) ) diff --git a/sydent/config/__init__.py b/sydent/config/__init__.py index 4f973a70..babc28b4 100644 --- a/sydent/config/__init__.py +++ b/sydent/config/__init__.py @@ -17,6 +17,7 @@ from sydent.config.crypto import CryptoConfig from sydent.config.database import DatabaseConfig from sydent.config.email import EmailConfig +from sydent.config.general import GeneralConfig from sydent.config.http import HTTPConfig from sydent.config.sms import SMSConfig @@ -32,6 +33,7 @@ class SydentConfig: """ def __init__(self): + self.general = GeneralConfig() self.database = DatabaseConfig() self.crypto = CryptoConfig() self.sms = SMSConfig() @@ -39,6 +41,7 @@ def __init__(self): self.http = HTTPConfig() self.config_sections = [ + self.general, self.database, self.crypto, self.sms, diff --git a/sydent/config/general.py b/sydent/config/general.py new file mode 100644 index 00000000..c3de58ca --- /dev/null +++ b/sydent/config/general.py @@ -0,0 +1,39 @@ +# 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. + +import logging +import os +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from configparser import ConfigParser + +logger = logging.getLogger(__name__) + + +class GeneralConfig: + def parse_config(self, cfg: "ConfigParser") -> None: + """ + Parse the 'general' section of the config + + :param cfg: the configuration to be parsed + """ + self.server_name = cfg.get("general", "server.name") + if self.server_name == "": + self.server_name = os.uname()[1] + logger.warning( + "You have not specified a server name. I have guessed that this server is called '%s' ." + "If this is incorrect, you should edit 'general.server.name' in the config file." + % (self.server_name,) + ) diff --git a/sydent/hs_federation/verifier.py b/sydent/hs_federation/verifier.py index 27a74abd..c24fcc0e 100644 --- a/sydent/hs_federation/verifier.py +++ b/sydent/hs_federation/verifier.py @@ -174,7 +174,7 @@ async def authenticate_request( json_request = { "method": request.method, "uri": request.uri, - "destination_is": self.sydent.server_name, + "destination_is": self.sydent.config.general.server_name, "signatures": {}, } diff --git a/sydent/http/servlets/blindlysignstuffservlet.py b/sydent/http/servlets/blindlysignstuffservlet.py index 99247a64..72df5bff 100644 --- a/sydent/http/servlets/blindlysignstuffservlet.py +++ b/sydent/http/servlets/blindlysignstuffservlet.py @@ -36,7 +36,7 @@ class BlindlySignStuffServlet(Resource): def __init__(self, syd: "Sydent", require_auth: bool = False) -> None: self.sydent = syd - self.server_name = syd.server_name + self.server_name = syd.config.general.server_name self.tokenStore = JoinTokenStore(syd) self.require_auth = require_auth diff --git a/sydent/http/servlets/lookupservlet.py b/sydent/http/servlets/lookupservlet.py index 33ae74e7..56bb58e6 100644 --- a/sydent/http/servlets/lookupservlet.py +++ b/sydent/http/servlets/lookupservlet.py @@ -63,7 +63,7 @@ def render_GET(self, request: Request) -> JsonDict: return {} sgassoc = json_decoder.decode(sgassoc) - if self.sydent.server_name not in sgassoc["signatures"]: + if self.sydent.config.general.server_name not in sgassoc["signatures"]: # We have not yet worked out what the proper trust model should be. # # Maybe clients implicitly trust a server they talk to (and so we @@ -82,7 +82,9 @@ def render_GET(self, request: Request) -> JsonDict: # replication, so that we can undo this decision in the future if # we wish, without having destroyed the raw underlying data. sgassoc = signedjson.sign.sign_json( - sgassoc, self.sydent.server_name, self.sydent.keyring.ed25519 + sgassoc, + self.sydent.config.general.server_name, + self.sydent.keyring.ed25519, ) return sgassoc diff --git a/sydent/replication/peer.py b/sydent/replication/peer.py index 8d20a93e..037460ee 100644 --- a/sydent/replication/peer.py +++ b/sydent/replication/peer.py @@ -62,7 +62,7 @@ class LocalPeer(Peer): """ def __init__(self, sydent: "Sydent") -> None: - super().__init__(sydent.server_name, {}) + super().__init__(sydent.config.general.server_name, {}) self.sydent = sydent self.hashing_store = HashingMetadataStore(sydent) @@ -103,7 +103,7 @@ def pushUpdates(self, sgAssocs: Dict[int, Dict[str, Any]]) -> "Deferred": globalAssocStore.addAssociation( assocObj, json.dumps(sgAssocs[localId]), - self.sydent.server_name, + self.sydent.config.general.server_name, localId, ) else: diff --git a/sydent/sydent.py b/sydent/sydent.py index 11bce48e..2693514f 100644 --- a/sydent/sydent.py +++ b/sydent/sydent.py @@ -220,20 +220,6 @@ def __init__( self.db = SqliteDatabase(self).db - self.server_name = self.cfg.get("general", "server.name") - if self.server_name == "": - self.server_name = os.uname()[1] - logger.warning( - ( - "You had not specified a server name. I have guessed that this server is called '%s' " - + "and saved this in the config file. If this is incorrect, you should edit server.name in " - + "the config file." - ) - % (self.server_name,) - ) - self.cfg.set("general", "server.name", self.server_name) - self.save_config() - if self.cfg.has_option("general", "sentry_dsn"): # Only import and start sentry SDK if configured. import sentry_sdk @@ -242,7 +228,7 @@ def __init__( dsn=self.cfg.get("general", "sentry_dsn"), ) with sentry_sdk.configure_scope() as scope: - scope.set_tag("sydent_server_name", self.server_name) + scope.set_tag("sydent_server_name", self.config.general.server_name) if self.cfg.has_option("general", "prometheus_port"): import prometheus_client diff --git a/sydent/threepid/bind.py b/sydent/threepid/bind.py index 31e5af3c..7f7441a5 100644 --- a/sydent/threepid/bind.py +++ b/sydent/threepid/bind.py @@ -99,7 +99,9 @@ def addBinding(self, medium: str, address: str, mxid: str) -> Dict[str, Any]: "token": cast(str, token["token"]), } token["signed"] = signedjson.sign.sign_json( - token["signed"], self.sydent.server_name, self.sydent.keyring.ed25519 + token["signed"], + self.sydent.config.general.server_name, + self.sydent.keyring.ed25519, ) invites.append(token) if invites: diff --git a/sydent/threepid/signer.py b/sydent/threepid/signer.py index 4cc37913..cc7ad8fd 100644 --- a/sydent/threepid/signer.py +++ b/sydent/threepid/signer.py @@ -43,6 +43,6 @@ def signedThreePidAssociation(self, assoc: "ThreepidAssociation") -> Dict[str, A } sgassoc.update(assoc.extra_fields) sgassoc = signedjson.sign.sign_json( - sgassoc, self.sydent.server_name, self.sydent.keyring.ed25519 + sgassoc, self.sydent.config.general.server_name, self.sydent.keyring.ed25519 ) return sgassoc diff --git a/tests/test_casefold_migration.py b/tests/test_casefold_migration.py index d0916615..74a1929d 100644 --- a/tests/test_casefold_migration.py +++ b/tests/test_casefold_migration.py @@ -95,7 +95,7 @@ def setUp(self): # create some global associations associations = [] - originServer = self.sydent.server_name + originServer = self.sydent.config.general.server_name for i in range(10): address = "bob%d@example.com" % i From bb02656ff3e398009ce457ac10ea3fe439cb61c9 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Tue, 7 Sep 2021 14:41:33 +0100 Subject: [PATCH 11/25] Move 'general' template config handling over to SydentConfig --- sydent/config/general.py | 28 ++++++++++++++++++++++++++++ sydent/sydent.py | 27 +++------------------------ sydent/util/emailutils.py | 2 +- tests/test_jinja_templates.py | 2 +- 4 files changed, 33 insertions(+), 26 deletions(-) diff --git a/sydent/config/general.py b/sydent/config/general.py index c3de58ca..0dcfce9a 100644 --- a/sydent/config/general.py +++ b/sydent/config/general.py @@ -12,10 +12,14 @@ # See the License for the specific language governing permissions and # limitations under the License. + import logging import os from typing import TYPE_CHECKING +from jinja2.environment import Environment +from jinja2.loaders import FileSystemLoader + if TYPE_CHECKING: from configparser import ConfigParser @@ -37,3 +41,27 @@ def parse_config(self, cfg: "ConfigParser") -> None: "If this is incorrect, you should edit 'general.server.name' in the config file." % (self.server_name,) ) + + # Get the possible brands by looking at directories under the + # templates.path directory. + self.templates_path = cfg.get("general", "templates.path") + if os.path.exists(self.templates_path): + self.valid_brands = { + p + for p in os.listdir(self.templates_path) + if os.path.isdir(os.path.join(self.templates_path, p)) + } + else: + logging.warning( + "The path specified by 'general.templates.path' does not exist." + ) + # This is a legacy code-path and assumes that verify_response_template, + # email.template, and email.invite_template are defined. + self.valid_brands = set() + + self.template_environment = Environment( + loader=FileSystemLoader(cfg.get("general", "templates.path")), + autoescape=True, + ) + + self.default_brand = cfg.get("general", "brand.default") diff --git a/sydent/sydent.py b/sydent/sydent.py index 2693514f..9e6d34ff 100644 --- a/sydent/sydent.py +++ b/sydent/sydent.py @@ -23,7 +23,6 @@ from typing import Set import twisted.internet.reactor -from jinja2 import Environment, FileSystemLoader from twisted.internet import address, task from twisted.python import log @@ -238,21 +237,6 @@ def __init__( addr=self.cfg.get("general", "prometheus_addr"), ) - if self.cfg.has_option("general", "templates.path"): - # Get the possible brands by looking at directories under the - # templates.path directory. - root_template_path = self.cfg.get("general", "templates.path") - if os.path.exists(root_template_path): - self.valid_brands = { - p - for p in os.listdir(root_template_path) - if os.path.isdir(os.path.join(root_template_path, p)) - } - else: - # This is a legacy code-path and assumes that verify_response_template, - # email.template, and email.invite_template are defined. - self.valid_brands = set() - self.enable_v1_associations = parse_cfg_bool( self.cfg.get("general", "enable_v1_associations") ) @@ -274,11 +258,6 @@ def __init__( self.ip_blacklist = generate_ip_set(ip_blacklist) self.ip_whitelist = generate_ip_set(ip_whitelist) - self.template_environment = Environment( - loader=FileSystemLoader(self.cfg.get("general", "templates.path")), - autoescape=True, - ) - # See if a pepper already exists in the database # Note: This MUST be run before we start serving requests, otherwise lookups for # 3PID hashes may come in before we've completed generating them @@ -446,14 +425,14 @@ def get_branded_template(self, brand, template_name, deprecated_template_name): # If a brand hint is provided, attempt to use it if it is valid. if brand: - if brand not in self.valid_brands: + if brand not in self.config.general.valid_brands: brand = None # If the brand hint is not valid, or not provided, fallback to the default brand. if not brand: - brand = self.cfg.get("general", "brand.default") + brand = self.config.general.default_brand - root_template_path = self.cfg.get("general", "templates.path") + root_template_path = self.config.general.templates_path # Grab jinja template if it exists if os.path.exists( diff --git a/sydent/util/emailutils.py b/sydent/util/emailutils.py index da54548e..0c258927 100644 --- a/sydent/util/emailutils.py +++ b/sydent/util/emailutils.py @@ -65,7 +65,7 @@ def sendEmail( # We add randomize the multipart boundary to stop user input from # conflicting with it. substitutions["multipart_boundary"] = generateAlphanumericTokenOfLength(32) - template = sydent.template_environment.get_template(templateFile) + template = sydent.config.general.template_environment.get_template(templateFile) mailString = template.render(substitutions) else: allSubstitutions = {} diff --git a/tests/test_jinja_templates.py b/tests/test_jinja_templates.py index 78b71872..bc0490f4 100644 --- a/tests/test_jinja_templates.py +++ b/tests/test_jinja_templates.py @@ -218,7 +218,7 @@ def test_jinja_vector_verification(self): email_contents = smtp.sendmail.call_args[0][2].decode("utf-8") path = os.path.join( - self.sydent.cfg.get("general", "templates.path"), + self.sydent.config.general.templates_path, "vector_verification_sample.txt", ) From 2d460ee6a390e9a054ee2fc8a60f8164f7e5938e Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Tue, 7 Sep 2021 15:37:28 +0100 Subject: [PATCH 12/25] Move rest of 'general' config handling over to SydentConfig --- sydent/config/general.py | 57 ++++++++++++++++++++++++- sydent/http/httpclient.py | 8 ++-- sydent/http/httpserver.py | 2 +- sydent/http/servlets/lookupv2servlet.py | 2 +- sydent/sydent.py | 41 +++--------------- sydent/terms/terms.py | 13 ++++-- sydent/threepid/bind.py | 2 +- tests/test_blacklisting.py | 4 +- 8 files changed, 81 insertions(+), 48 deletions(-) diff --git a/sydent/config/general.py b/sydent/config/general.py index 0dcfce9a..a2aa8e29 100644 --- a/sydent/config/general.py +++ b/sydent/config/general.py @@ -15,11 +15,13 @@ import logging import os -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Set from jinja2.environment import Environment from jinja2.loaders import FileSystemLoader +from sydent.util.ip_range import DEFAULT_IP_RANGE_BLACKLIST, generate_ip_set + if TYPE_CHECKING: from configparser import ConfigParser @@ -65,3 +67,56 @@ def parse_config(self, cfg: "ConfigParser") -> None: ) self.default_brand = cfg.get("general", "brand.default") + + self.pidfile = cfg.get("general", "pidfile.path") + + self.terms_path = cfg.get("general", "terms.path") + + self.address_lookup_limit = cfg.getint("general", "address_lookup_limit") + + self.prometheus_enabled = cfg.has_option("general", "prometheus_port") + if self.prometheus_enabled: + self.prometheus_port = cfg.getint("general", "prometheus_port") + self.prometheus_addr = cfg.get("general", "prometheus_addr") + + self.sentry_enabled = cfg.has_option("general", "sentry_dsn") + if self.sentry_enabled: + self.sentry_dsn = cfg.get("general", "sentry_dsn") + + self.enable_v1_associations = parse_cfg_bool( + cfg.get("general", "enable_v1_associations") + ) + + self.delete_tokens_on_bind = parse_cfg_bool( + cfg.get("general", "delete_tokens_on_bind") + ) + + ip_blacklist = set_from_comma_sep_string(cfg.get("general", "ip.blacklist")) + if not ip_blacklist: + ip_blacklist = DEFAULT_IP_RANGE_BLACKLIST + + ip_whitelist = set_from_comma_sep_string(cfg.get("general", "ip.whitelist")) + + self.ip_blacklist = generate_ip_set(ip_blacklist) + self.ip_whitelist = generate_ip_set(ip_whitelist) + + +def set_from_comma_sep_string(rawstr: str) -> Set[str]: + """ + Parse the a comma seperated string into a set + + :param rawstr: the string to be parsed + """ + if rawstr == "": + return set() + return {x.strip() for x in rawstr.split(",")} + + +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" diff --git a/sydent/http/httpclient.py b/sydent/http/httpclient.py index eea4b9bb..9b4eaedd 100644 --- a/sydent/http/httpclient.py +++ b/sydent/http/httpclient.py @@ -125,8 +125,8 @@ def __init__(self, sydent: "Sydent") -> None: self.agent = Agent( BlacklistingReactorWrapper( reactor=self.sydent.reactor, - ip_whitelist=sydent.ip_whitelist, - ip_blacklist=sydent.ip_blacklist, + ip_whitelist=sydent.config.general.ip_whitelist, + ip_blacklist=sydent.config.general.ip_blacklist, ), connectTimeout=15, ) @@ -142,8 +142,8 @@ def __init__(self, sydent: "Sydent") -> None: self.agent = MatrixFederationAgent( BlacklistingReactorWrapper( reactor=self.sydent.reactor, - ip_whitelist=sydent.ip_whitelist, - ip_blacklist=sydent.ip_blacklist, + ip_whitelist=sydent.config.general.ip_whitelist, + ip_blacklist=sydent.config.general.ip_blacklist, ), ClientTLSOptionsFactory(sydent.config.http.verify_federation_certs) if sydent.use_tls_for_federation diff --git a/sydent/http/httpserver.py b/sydent/http/httpserver.py index 541986c3..982d196a 100644 --- a/sydent/http/httpserver.py +++ b/sydent/http/httpserver.py @@ -93,7 +93,7 @@ def __init__(self, sydent: "Sydent") -> None: threepid_v1.putChild(b"getValidated3pid", self.sydent.servlets.getValidated3pid) threepid_v1.putChild(b"unbind", unbind) - if self.sydent.enable_v1_associations: + if self.sydent.config.general.enable_v1_associations: threepid_v1.putChild(b"bind", self.sydent.servlets.threepidBind) v1.putChild(b"3pid", threepid_v1) diff --git a/sydent/http/servlets/lookupv2servlet.py b/sydent/http/servlets/lookupv2servlet.py index 0789de02..3b1d2a72 100644 --- a/sydent/http/servlets/lookupv2servlet.py +++ b/sydent/http/servlets/lookupv2servlet.py @@ -80,7 +80,7 @@ def render_POST(self, request: Request) -> JsonDict: return {"errcode": "M_INVALID_PARAM", "error": "algorithm is not supported"} # Ensure address count is under the configured limit - limit = int(self.sydent.cfg.get("general", "address_lookup_limit")) + limit = self.sydent.config.general.address_lookup_limit if len(addresses) > limit: request.setResponseCode(400) return { diff --git a/sydent/sydent.py b/sydent/sydent.py index 9e6d34ff..7d4b78b5 100644 --- a/sydent/sydent.py +++ b/sydent/sydent.py @@ -70,7 +70,6 @@ from sydent.replication.pusher import Pusher from sydent.threepid.bind import ThreepidBinder from sydent.util.hash import sha256_and_url_safe_base64 -from sydent.util.ip_range import DEFAULT_IP_RANGE_BLACKLIST, generate_ip_set from sydent.util.tokenutils import generateAlphanumericTokenOfLength from sydent.validators.emailvalidator import EmailValidator from sydent.validators.msisdnvalidator import MsisdnValidator @@ -215,49 +214,23 @@ def __init__( logger.info("Starting Sydent server") - self.pidfile = self.cfg.get("general", "pidfile.path") - self.db = SqliteDatabase(self).db - if self.cfg.has_option("general", "sentry_dsn"): - # Only import and start sentry SDK if configured. + if self.config.general.sentry_enabled: import sentry_sdk - sentry_sdk.init( - dsn=self.cfg.get("general", "sentry_dsn"), - ) + sentry_sdk.init(dsn=self.config.general.sentry_dsn) with sentry_sdk.configure_scope() as scope: scope.set_tag("sydent_server_name", self.config.general.server_name) - if self.cfg.has_option("general", "prometheus_port"): + if self.config.general.prometheus_enabled: import prometheus_client prometheus_client.start_http_server( - port=self.cfg.getint("general", "prometheus_port"), - addr=self.cfg.get("general", "prometheus_addr"), + port=self.config.general.prometheus_port, + addr=self.config.general.prometheus_addr, ) - self.enable_v1_associations = parse_cfg_bool( - self.cfg.get("general", "enable_v1_associations") - ) - - self.delete_tokens_on_bind = parse_cfg_bool( - self.cfg.get("general", "delete_tokens_on_bind") - ) - - ip_blacklist = set_from_comma_sep_string( - self.cfg.get("general", "ip.blacklist") - ) - if not ip_blacklist: - ip_blacklist = DEFAULT_IP_RANGE_BLACKLIST - - ip_whitelist = set_from_comma_sep_string( - self.cfg.get("general", "ip.whitelist") - ) - - self.ip_blacklist = generate_ip_set(ip_blacklist) - self.ip_whitelist = generate_ip_set(ip_whitelist) - # See if a pepper already exists in the database # Note: This MUST be run before we start serving requests, otherwise lookups for # 3PID hashes may come in before we've completed generating them @@ -367,8 +340,8 @@ def run(self): self.internalApiHttpServer = InternalApiHttpServer(self) self.internalApiHttpServer.setup(interface, internalport) - if self.pidfile: - with open(self.pidfile, "w") as pidfile: + if self.config.general.pidfile: + with open(self.config.general.pidfile, "w") as pidfile: pidfile.write(str(os.getpid()) + "\n") self.reactor.run() diff --git a/sydent/terms/terms.py b/sydent/terms/terms.py index dc16108c..6204a676 100644 --- a/sydent/terms/terms.py +++ b/sydent/terms/terms.py @@ -13,10 +13,13 @@ # limitations under the License. import logging -from typing import Any, Dict, List, Optional, Set +from typing import TYPE_CHECKING, Any, Dict, List, Optional, Set import yaml +if TYPE_CHECKING: + from sydent.sydent import Sydent + logger = logging.getLogger(__name__) @@ -95,14 +98,16 @@ def urlListIsSufficient(self, urls: List[str]) -> bool: return agreed == required -def get_terms(sydent) -> Optional[Terms]: +def get_terms(sydent: "Sydent") -> Optional[Terms]: """Read and parse terms as specified in the config. :returns Terms """ try: termsYaml = None - termsPath = sydent.cfg.get("general", "terms.path") + + # TODO - move some of this to parse_config + termsPath = sydent.config.general.terms_path if termsPath == "": return Terms(None) @@ -130,6 +135,6 @@ def get_terms(sydent) -> Optional[Terms]: return Terms(termsYaml) except Exception: logger.exception( - "Couldn't read terms file '%s'", sydent.cfg.get("general", "terms.path") + "Couldn't read terms file '%s'", sydent.config.general.terms_path ) return None diff --git a/sydent/threepid/bind.py b/sydent/threepid/bind.py index 7f7441a5..8983c12c 100644 --- a/sydent/threepid/bind.py +++ b/sydent/threepid/bind.py @@ -178,7 +178,7 @@ async def _notify(self, assoc: Dict[str, Any], attempt: int) -> None: logger.info("Successfully notified on bind for %s" % (mxid,)) # Skip the deletion step if instructed so by the config. - if not self.sydent.delete_tokens_on_bind: + if not self.sydent.config.general.delete_tokens_on_bind: return # Only remove sent tokens when they've been successfully sent. diff --git a/tests/test_blacklisting.py b/tests/test_blacklisting.py index 69a3d531..922cd5a0 100644 --- a/tests/test_blacklisting.py +++ b/tests/test_blacklisting.py @@ -49,8 +49,8 @@ def setUp(self): self.reactor.lookups[domain.decode()] = ip.decode() self.reactor.lookups[ip.decode()] = ip.decode() - self.ip_whitelist = self.sydent.ip_whitelist - self.ip_blacklist = self.sydent.ip_blacklist + self.ip_whitelist = self.sydent.config.general.ip_whitelist + self.ip_blacklist = self.sydent.config.general.ip_blacklist def test_reactor(self): """Apply the blacklisting reactor and ensure it properly blocks From 4b0eb6770f8b4a37bf6781520e388b533cb8717c Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Wed, 8 Sep 2021 10:04:48 +0100 Subject: [PATCH 13/25] Remove deprecated template argument from get_branded_template test_jinja_templates used `("email", "email.verification_template")` for this argument, however this config option isn't mentioned anywhere else in the code (or even in the test's config) so it's ok to remove it --- scripts/casefold_db.py | 1 - sydent/http/servlets/emailservlet.py | 1 - sydent/http/servlets/msisdnservlet.py | 1 - sydent/http/servlets/store_invite_servlet.py | 1 - sydent/sydent.py | 23 ++++++++------------ sydent/validators/emailvalidator.py | 1 - tests/test_casefold_migration.py | 1 - tests/test_jinja_templates.py | 4 ---- 8 files changed, 9 insertions(+), 24 deletions(-) diff --git a/scripts/casefold_db.py b/scripts/casefold_db.py index daa5fa04..f71abe92 100755 --- a/scripts/casefold_db.py +++ b/scripts/casefold_db.py @@ -109,7 +109,6 @@ def update_local_associations( templateFile = sydent.get_branded_template( None, "migration_template.eml", - ("email", "email.template"), ) else: templateFile = sydent.config.email.template diff --git a/sydent/http/servlets/emailservlet.py b/sydent/http/servlets/emailservlet.py index 45c15219..3515820e 100644 --- a/sydent/http/servlets/emailservlet.py +++ b/sydent/http/servlets/emailservlet.py @@ -125,7 +125,6 @@ def render_GET(self, request: Request) -> bytes: templateFile = self.sydent.get_branded_template( brand, "verify_response_template.html", - ("http", "verify_response_template"), ) else: templateFile = self.sydent.config.http.verify_response_template diff --git a/sydent/http/servlets/msisdnservlet.py b/sydent/http/servlets/msisdnservlet.py index c223c933..a3066374 100644 --- a/sydent/http/servlets/msisdnservlet.py +++ b/sydent/http/servlets/msisdnservlet.py @@ -148,7 +148,6 @@ def render_GET(self, request: Request) -> str: templateFile = self.sydent.get_branded_template( brand, "verify_response_template.html", - ("http", "verify_response_template"), ) else: templateFile = self.sydent.config.http.verify_response_template diff --git a/sydent/http/servlets/store_invite_servlet.py b/sydent/http/servlets/store_invite_servlet.py index c6ee1c42..4664829e 100644 --- a/sydent/http/servlets/store_invite_servlet.py +++ b/sydent/http/servlets/store_invite_servlet.py @@ -159,7 +159,6 @@ def render_POST(self, request: Request) -> JsonDict: templateFile = self.sydent.get_branded_template( brand, "invite_template.eml", - ("email", "email.invite_template"), ) else: templateFile = self.sydent.config.email.invite_template diff --git a/sydent/sydent.py b/sydent/sydent.py index 7d4b78b5..60d48e44 100644 --- a/sydent/sydent.py +++ b/sydent/sydent.py @@ -20,7 +20,7 @@ import logging import logging.handlers import os -from typing import Set +from typing import Optional, Set import twisted.internet.reactor from twisted.internet import address, task @@ -371,31 +371,26 @@ def brand_from_request(self, request): return request.args[b"brand"][0].decode("utf-8") return None - def get_branded_template(self, brand, template_name, deprecated_template_name): + def get_branded_template( + self, + brand: Optional[str], + template_name: str, + ) -> str: """ - Calculate a (maybe) branded template filename to use. + Calculate a branded template filename to use. - If the deprecated email.template setting is defined, always use it. - Otherwise, attempt to use the hinted brand from the request if the brand + Attempt to use the hinted brand from the request if the brand is valid. Otherwise, fallback to the default brand. :param brand: The hint of which brand to use. :type brand: str or None :param template_name: The name of the template file to load. :type template_name: str - :param deprecated_template_name: The deprecated setting to use, if provided. - :type deprecated_template_name: Tuple[str] - + ... :return: The template filename to use. :rtype: str """ - # If the deprecated setting is defined, return it. - try: - return self.cfg.get(*deprecated_template_name) - except configparser.NoOptionError: - pass - # If a brand hint is provided, attempt to use it if it is valid. if brand: if brand not in self.config.general.valid_brands: diff --git a/sydent/validators/emailvalidator.py b/sydent/validators/emailvalidator.py index b2ef9664..b3057256 100644 --- a/sydent/validators/emailvalidator.py +++ b/sydent/validators/emailvalidator.py @@ -67,7 +67,6 @@ def requestToken( templateFile = self.sydent.get_branded_template( brand, "verification_template.eml", - ("email", "email.template"), ) else: templateFile = self.sydent.config.email.template diff --git a/tests/test_casefold_migration.py b/tests/test_casefold_migration.py index 74a1929d..1c9e7de2 100644 --- a/tests/test_casefold_migration.py +++ b/tests/test_casefold_migration.py @@ -175,7 +175,6 @@ def test_migration_email(self): templateFile = self.sydent.get_branded_template( None, "migration_template.eml", - ("email", "email.template"), ) else: templateFile = self.sydent.config.email.template diff --git a/tests/test_jinja_templates.py b/tests/test_jinja_templates.py index bc0490f4..cac01323 100644 --- a/tests/test_jinja_templates.py +++ b/tests/test_jinja_templates.py @@ -58,7 +58,6 @@ def test_jinja_vector_invite(self): templateFile = self.sydent.get_branded_template( "vector-im", "invite_template.eml", - ("email", "email.invite_template"), ) else: templateFile = self.sydent.config.email.invite_template @@ -124,7 +123,6 @@ def test_jinja_matrix_invite(self): templateFile = self.sydent.get_branded_template( "matrix-org", "invite_template.eml", - ("email", "email.invite_template"), ) else: templateFile = self.sydent.config.email.invite_template @@ -178,7 +176,6 @@ def test_jinja_matrix_verification(self): templateFile = self.sydent.get_branded_template( "matrix-org", "verification_template.eml", - ("email", "email.verification_template"), ) with patch("sydent.util.emailutils.smtplib") as smtplib: @@ -208,7 +205,6 @@ def test_jinja_vector_verification(self): templateFile = self.sydent.get_branded_template( "vector-im", "verification_template.eml", - ("email", "email.verification_template"), ) with patch("sydent.util.emailutils.smtplib") as smtplib: From 37b928b42d075629eb57f8e201d4de26b18ece3c Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Wed, 8 Sep 2021 10:38:49 +0100 Subject: [PATCH 14/25] Remove old cfg argument from Sydent constructor - Move config file and dict handling over to SydentConfig - Alter all places where Sydent object is constructed - Remove parse_cfg_bool and set_from_comma_set_string from sydent.py as these functions were only used for config parsing - Remove save_config from sydent.py as this can now be done from SydentConfig --- scripts/casefold_db.py | 8 +- sydent/config/__init__.py | 228 ++++++++++++++++++++++++++++++++++++- sydent/sydent.py | 231 +------------------------------------- tests/utils.py | 7 +- 4 files changed, 235 insertions(+), 239 deletions(-) diff --git a/scripts/casefold_db.py b/scripts/casefold_db.py index f71abe92..a0d4661b 100755 --- a/scripts/casefold_db.py +++ b/scripts/casefold_db.py @@ -23,7 +23,7 @@ import signedjson.sign from sydent.config import SydentConfig -from sydent.sydent import Sydent, parse_config_file +from sydent.sydent import Sydent from sydent.util import json_decoder from sydent.util.emailutils import sendEmail from sydent.util.hash import sha256_and_url_safe_base64 @@ -252,13 +252,11 @@ def update_global_associations( print(f"The config file '{args.config_path}' does not exist.") sys.exit(1) - config = parse_config_file(args.config_path) - sydent_config = SydentConfig() - sydent_config.parse_from_config_parser(config) + sydent_config.parse_config_file(args.config_path) reactor = ResolvingMemoryReactorClock() - sydent = Sydent(config, sydent_config, reactor, False) + sydent = Sydent(sydent_config, reactor, False) update_global_associations(sydent, sydent.db, not args.no_email, args.dry_run) update_local_associations(sydent, sydent.db, not args.no_email, args.dry_run) diff --git a/sydent/config/__init__.py b/sydent/config/__init__.py index babc28b4..a89f0d13 100644 --- a/sydent/config/__init__.py +++ b/sydent/config/__init__.py @@ -12,7 +12,13 @@ # See the License for the specific language governing permissions and # limitations under the License. -from configparser import ConfigParser +import copy +import logging +import os +from configparser import DEFAULTSECT, ConfigParser +from typing import Dict + +from twisted.python import log from sydent.config.crypto import CryptoConfig from sydent.config.database import DatabaseConfig @@ -21,6 +27,128 @@ from sydent.config.http import HTTPConfig from sydent.config.sms import SMSConfig +logger = logging.getLogger(__name__) + +CONFIG_DEFAULTS = { + "general": { + "server.name": os.environ.get("SYDENT_SERVER_NAME", ""), + "log.path": "", + "log.level": "INFO", + "pidfile.path": os.environ.get("SYDENT_PID_FILE", "sydent.pid"), + "terms.path": "", + "address_lookup_limit": "10000", # Maximum amount of addresses in a single /lookup request + # The root path to use for load templates. This should contain branded + # directories. Each directory should contain the following templates: + # + # * invite_template.eml + # * verification_template.eml + # * verify_response_template.html + "templates.path": "res", + # The brand directory to use if no brand hint (or an invalid brand hint) + # is provided by the request. + "brand.default": "matrix-org", + # The following can be added to your local config file to enable prometheus + # support. + # 'prometheus_port': '8080', # The port to serve metrics on + # 'prometheus_addr': '', # The address to bind to. Empty string means bind to all. + # The following can be added to your local config file to enable sentry support. + # 'sentry_dsn': 'https://...' # The DSN has configured in the sentry instance project. + # Whether clients and homeservers can register an association using v1 endpoints. + "enable_v1_associations": "true", + "delete_tokens_on_bind": "true", + # Prevent outgoing requests from being sent to the following blacklisted + # IP address CIDR ranges. If this option is not specified or empty then + # it defaults to private IP address ranges. + # + # The blacklist applies to all outbound requests except replication + # requests. + # + # (0.0.0.0 and :: are always blacklisted, whether or not they are + # explicitly listed here, since they correspond to unroutable + # addresses.) + "ip.blacklist": "", + # List of IP address CIDR ranges that should be allowed for outbound + # requests. This is useful for specifying exceptions to wide-ranging + # blacklisted target IP ranges. + # + # This whitelist overrides `ip.blacklist` and defaults to an empty + # list. + "ip.whitelist": "", + }, + "db": { + "db.file": os.environ.get("SYDENT_DB_PATH", "sydent.db"), + }, + "http": { + "clientapi.http.bind_address": "::", + "clientapi.http.port": "8090", + "internalapi.http.bind_address": "::1", + "internalapi.http.port": "", + "replication.https.certfile": "", + "replication.https.cacert": "", # This should only be used for testing + "replication.https.bind_address": "::", + "replication.https.port": "4434", + "obey_x_forwarded_for": "False", + "federation.verifycerts": "True", + # verify_response_template is deprecated, but still used if defined. Define + # templates.path and brand.default under general instead. + # + # 'verify_response_template': 'res/verify_response_page_template', + "client_http_base": "", + }, + "email": { + # email.template and email.invite_template are deprecated, but still used + # if defined. Define templates.path and brand.default under general instead. + # + # 'email.template': 'res/verification_template.eml', + # 'email.invite_template': 'res/invite_template.eml', + "email.from": "Sydent Validation ", + "email.subject": "Your Validation Token", + "email.invite.subject": "%(sender_display_name)s has invited you to chat", + "email.invite.subject_space": "%(sender_display_name)s has invited you to a space", + "email.smtphost": "localhost", + "email.smtpport": "25", + "email.smtpusername": "", + "email.smtppassword": "", + "email.hostname": "", + "email.tlsmode": "0", + # The web client location which will be used if it is not provided by + # the homeserver. + # + # This should be the scheme and hostname only, see res/invite_template.eml + # for the full URL that gets generated. + "email.default_web_client_location": "https://app.element.io", + # When a user is invited to a room via their email address, that invite is + # displayed in the room list using an obfuscated version of the user's email + # address. These config options determine how much of the email address to + # obfuscate. Note that the '@' sign is always included. + # + # If the string is longer than a configured limit below, it is truncated to that limit + # with '...' added. Otherwise: + # + # * If the string is longer than 5 characters, it is truncated to 3 characters + '...' + # * If the string is longer than 1 character, it is truncated to 1 character + '...' + # * If the string is 1 character long, it is converted to '...' + # + # This ensures that a full email address is never shown, even if it is extremely + # short. + # + # The number of characters from the beginning to reveal of the email's username + # portion (left of the '@' sign) + "email.third_party_invite_username_obfuscate_characters": "3", + # The number of characters from the beginning to reveal of the email's domain + # portion (right of the '@' sign) + "email.third_party_invite_domain_obfuscate_characters": "3", + }, + "sms": { + "bodyTemplate": "Your code is {token}", + "username": "", + "password": "", + }, + "crypto": { + "ed25519.signingkey": "", + }, +} + class ConfigError(Exception): pass @@ -30,6 +158,10 @@ class SydentConfig: """This is the class in charge of handling Sydent's configuration. Handling of each individual section is delegated to other classes stored in a `config_sections` list. + + To use this class, create a new object and then call one of + `parse_config_file` or `parse_config_dict` before creating the + Sydent object that uses it. """ def __init__(self): @@ -73,3 +205,97 @@ def parse_from_config_parser(self, cfg: ConfigParser) -> bool: # user has asked for this specifially (e.g. on first # run only, or when specify --generate-config) return self.crypto.save_key + + def parse_config_file(self, config_file: str) -> None: + """ + Parse the given config from a filepath, populating missing items and + sections + + :param config_file: the file to be parsed + """ + # If the config file doesn't exist, prepopulate the config object + # with the defaults, in the right section. + # + # Otherwise, we have to put the defaults in the DEFAULT section, + # to ensure that they don't override anyone's settings which are + # in their config file in the default section (which is likely, + # because sydent used to be braindead). + use_defaults = not os.path.exists(config_file) + + cfg = ConfigParser() + for sect, entries in CONFIG_DEFAULTS.items(): + cfg.add_section(sect) + for k, v in entries.items(): + cfg.set(DEFAULTSECT if use_defaults else 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 + setup_logging(cfg) + + needs_saving = self.parse_from_config_parser(cfg) + + if needs_saving: + fp = open(config_file, "w") + cfg.write(fp) + fp.close() + + def parse_config_dict(self, config_dict: Dict) -> None: + """ + Parse the given config from a dictionary, populating missing items and sections + + :param config_dict: the configuration dictionary to be parsed + """ + # Build a config dictionary from the defaults merged with the given dictionary + config = copy.deepcopy(CONFIG_DEFAULTS) + for section, section_dict in config_dict.items(): + if section not in config: + config[section] = {} + 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) + + +def setup_logging(cfg: ConfigParser) -> None: + """ + Setup logging using the options selected in the config + + :param cfg: the configuration + """ + log_format = "%(asctime)s - %(name)s - %(lineno)d - %(levelname)s" " - %(message)s" + formatter = logging.Formatter(log_format) + + logPath = cfg.get("general", "log.path") + if logPath != "": + handler = logging.handlers.TimedRotatingFileHandler( + logPath, when="midnight", backupCount=365 + ) + handler.setFormatter(formatter) + + def sighup(signum, stack): + logger.info("Closing log file due to SIGHUP") + handler.doRollover() + logger.info("Opened new log file due to SIGHUP") + + else: + handler = logging.StreamHandler() + + handler.setFormatter(formatter) + rootLogger = logging.getLogger("") + rootLogger.setLevel(cfg.get("general", "log.level")) + rootLogger.addHandler(handler) + + observer = log.PythonLoggingObserver() + observer.start() diff --git a/sydent/sydent.py b/sydent/sydent.py index 60d48e44..1652f0c2 100644 --- a/sydent/sydent.py +++ b/sydent/sydent.py @@ -14,17 +14,14 @@ # See the License for the specific language governing permissions and # limitations under the License. -import configparser -import copy import gc import logging import logging.handlers import os -from typing import Optional, Set +from typing import Optional import twisted.internet.reactor from twisted.internet import address, task -from twisted.python import log from sydent.config import SydentConfig from sydent.db.hashing_metadata import HashingMetadataStore @@ -76,140 +73,17 @@ logger = logging.getLogger(__name__) -CONFIG_DEFAULTS = { - "general": { - "server.name": os.environ.get("SYDENT_SERVER_NAME", ""), - "log.path": "", - "log.level": "INFO", - "pidfile.path": os.environ.get("SYDENT_PID_FILE", "sydent.pid"), - "terms.path": "", - "address_lookup_limit": "10000", # Maximum amount of addresses in a single /lookup request - # The root path to use for load templates. This should contain branded - # directories. Each directory should contain the following templates: - # - # * invite_template.eml - # * verification_template.eml - # * verify_response_template.html - "templates.path": "res", - # The brand directory to use if no brand hint (or an invalid brand hint) - # is provided by the request. - "brand.default": "matrix-org", - # The following can be added to your local config file to enable prometheus - # support. - # 'prometheus_port': '8080', # The port to serve metrics on - # 'prometheus_addr': '', # The address to bind to. Empty string means bind to all. - # The following can be added to your local config file to enable sentry support. - # 'sentry_dsn': 'https://...' # The DSN has configured in the sentry instance project. - # Whether clients and homeservers can register an association using v1 endpoints. - "enable_v1_associations": "true", - "delete_tokens_on_bind": "true", - # Prevent outgoing requests from being sent to the following blacklisted - # IP address CIDR ranges. If this option is not specified or empty then - # it defaults to private IP address ranges. - # - # The blacklist applies to all outbound requests except replication - # requests. - # - # (0.0.0.0 and :: are always blacklisted, whether or not they are - # explicitly listed here, since they correspond to unroutable - # addresses.) - "ip.blacklist": "", - # List of IP address CIDR ranges that should be allowed for outbound - # requests. This is useful for specifying exceptions to wide-ranging - # blacklisted target IP ranges. - # - # This whitelist overrides `ip.blacklist` and defaults to an empty - # list. - "ip.whitelist": "", - }, - "db": { - "db.file": os.environ.get("SYDENT_DB_PATH", "sydent.db"), - }, - "http": { - "clientapi.http.bind_address": "::", - "clientapi.http.port": "8090", - "internalapi.http.bind_address": "::1", - "internalapi.http.port": "", - "replication.https.certfile": "", - "replication.https.cacert": "", # This should only be used for testing - "replication.https.bind_address": "::", - "replication.https.port": "4434", - "obey_x_forwarded_for": "False", - "federation.verifycerts": "True", - # verify_response_template is deprecated, but still used if defined Define - # templates.path and brand.default under general instead. - # - # 'verify_response_template': 'res/verify_response_page_template', - "client_http_base": "", - }, - "email": { - # email.template and email.invite_template are deprecated, but still used - # if defined. Define templates.path and brand.default under general instead. - # - # 'email.template': 'res/verification_template.eml', - # 'email.invite_template': 'res/invite_template.eml', - "email.from": "Sydent Validation ", - "email.subject": "Your Validation Token", - "email.invite.subject": "%(sender_display_name)s has invited you to chat", - "email.invite.subject_space": "%(sender_display_name)s has invited you to a space", - "email.smtphost": "localhost", - "email.smtpport": "25", - "email.smtpusername": "", - "email.smtppassword": "", - "email.hostname": "", - "email.tlsmode": "0", - # The web client location which will be used if it is not provided by - # the homeserver. - # - # This should be the scheme and hostname only, see res/invite_template.eml - # for the full URL that gets generated. - "email.default_web_client_location": "https://app.element.io", - # When a user is invited to a room via their email address, that invite is - # displayed in the room list using an obfuscated version of the user's email - # address. These config options determine how much of the email address to - # obfuscate. Note that the '@' sign is always included. - # - # If the string is longer than a configured limit below, it is truncated to that limit - # with '...' added. Otherwise: - # - # * If the string is longer than 5 characters, it is truncated to 3 characters + '...' - # * If the string is longer than 1 character, it is truncated to 1 character + '...' - # * If the string is 1 character long, it is converted to '...' - # - # This ensures that a full email address is never shown, even if it is extremely - # short. - # - # The number of characters from the beginning to reveal of the email's username - # portion (left of the '@' sign) - "email.third_party_invite_username_obfuscate_characters": "3", - # The number of characters from the beginning to reveal of the email's domain - # portion (right of the '@' sign) - "email.third_party_invite_domain_obfuscate_characters": "3", - }, - "sms": { - "bodyTemplate": "Your code is {token}", - "username": "", - "password": "", - }, - "crypto": { - "ed25519.signingkey": "", - }, -} - class Sydent: def __init__( self, - cfg, sydent_config: SydentConfig, reactor=twisted.internet.reactor, use_tls_for_federation=True, ): - self.cfg = cfg self.config = sydent_config self.reactor = reactor - self.config_file = get_config_file_path() self.use_tls_for_federation = use_tls_for_federation logger.info("Starting Sydent server") @@ -323,11 +197,6 @@ def __init__( cb.clock = self.reactor cb.start(1.0) - def save_config(self): - fp = open(self.config_file, "w") - self.cfg.write(fp) - fp.close() - def run(self): self.clientApiHttpServer.setup() self.replicationHttpsServer.setup() @@ -423,97 +292,10 @@ class Keyring: pass -def parse_config_dict(config_dict): - """Parse the given config from a dictionary, populating missing items and sections - - Args: - config_dict (dict): the configuration dictionary to be parsed - """ - # Build a config dictionary from the defaults merged with the given dictionary - config = copy.deepcopy(CONFIG_DEFAULTS) - for section, section_dict in config_dict.items(): - if section not in config: - config[section] = {} - for option in section_dict.keys(): - config[section][option] = config_dict[section][option] - - # Build a ConfigParser from the merged dictionary - cfg = configparser.ConfigParser() - for section, section_dict in config.items(): - cfg.add_section(section) - for option, value in section_dict.items(): - cfg.set(section, option, value) - - return cfg - - -def parse_config_file(config_file): - """Parse the given config from a filepath, populating missing items and - sections - Args: - config_file (str): the file to be parsed - """ - # if the config file doesn't exist, prepopulate the config object - # with the defaults, in the right section. - # - # otherwise, we have to put the defaults in the DEFAULT section, - # to ensure that they don't override anyone's settings which are - # in their config file in the default section (which is likely, - # because sydent used to be braindead). - use_defaults = not os.path.exists(config_file) - cfg = configparser.ConfigParser() - for sect, entries in CONFIG_DEFAULTS.items(): - cfg.add_section(sect) - for k, v in entries.items(): - cfg.set(configparser.DEFAULTSECT if use_defaults else sect, k, v) - - cfg.read(config_file) - - return cfg - - -def setup_logging(cfg): - log_format = "%(asctime)s - %(name)s - %(lineno)d - %(levelname)s" " - %(message)s" - formatter = logging.Formatter(log_format) - - logPath = cfg.get("general", "log.path") - if logPath != "": - handler = logging.handlers.TimedRotatingFileHandler( - logPath, when="midnight", backupCount=365 - ) - handler.setFormatter(formatter) - - def sighup(signum, stack): - logger.info("Closing log file due to SIGHUP") - handler.doRollover() - logger.info("Opened new log file due to SIGHUP") - - else: - handler = logging.StreamHandler() - - handler.setFormatter(formatter) - rootLogger = logging.getLogger("") - rootLogger.setLevel(cfg.get("general", "log.level")) - rootLogger.addHandler(handler) - - observer = log.PythonLoggingObserver() - observer.start() - - def get_config_file_path(): return os.environ.get("SYDENT_CONF", "sydent.conf") -def parse_cfg_bool(value): - return value.lower() == "true" - - -def set_from_comma_sep_string(rawstr: str) -> Set[str]: - if rawstr == "": - return set() - return {x.strip() for x in rawstr.split(",")} - - def run_gc(): threshold = gc.get_threshold() counts = gc.get_count() @@ -523,15 +305,8 @@ def run_gc(): if __name__ == "__main__": - cfg = parse_config_file(get_config_file_path()) - setup_logging(cfg) - sydent_config = SydentConfig() - cfg_needs_saving = sydent_config.parse_from_config_parser(cfg) - - syd = Sydent(cfg, sydent_config=sydent_config) - - if cfg_needs_saving: - syd.save_config() + sydent_config.parse_config_file(get_config_file_path()) + syd = Sydent(sydent_config) syd.run() diff --git a/tests/utils.py b/tests/utils.py index 408269ba..902b3bef 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -24,7 +24,7 @@ from zope.interface import implementer from sydent.config import SydentConfig -from sydent.sydent import Sydent, parse_config_dict +from sydent.sydent import Sydent # Expires on Jan 11 2030 at 17:53:40 GMT FAKE_SERVER_CERT_PEM = """ @@ -70,14 +70,11 @@ def make_sydent(test_config={}): reactor = ResolvingMemoryReactorClock() - cfg = parse_config_dict(test_config) - sydent_config = SydentConfig() - sydent_config.parse_from_config_parser(cfg) + sydent_config.parse_config_dict(test_config) return Sydent( reactor=reactor, - cfg=cfg, sydent_config=sydent_config, use_tls_for_federation=False, ) From a9415f4906b47f1523b734ff1a6685a85ca3f408 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Wed, 8 Sep 2021 11:16:57 +0100 Subject: [PATCH 15/25] Add changelog --- changelog.d/385.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/385.misc diff --git a/changelog.d/385.misc b/changelog.d/385.misc new file mode 100644 index 00000000..e509b08b --- /dev/null +++ b/changelog.d/385.misc @@ -0,0 +1 @@ +Move the configuration file handling code into a seperate module. \ No newline at end of file From cf58f3a8a0ea712a22f75811909ced43eace82d8 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Thu, 9 Sep 2021 16:29:07 +0100 Subject: [PATCH 16/25] Add file that got lost while fixing merge conflicts --- docs/casefold_migration.md | 50 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 docs/casefold_migration.md diff --git a/docs/casefold_migration.md b/docs/casefold_migration.md new file mode 100644 index 00000000..77a53ae1 --- /dev/null +++ b/docs/casefold_migration.md @@ -0,0 +1,50 @@ +# Migrating to case-insensitive email addresses + +**Note: the operation described in this documentation is only needed if your server was +running a version of Sydent earlier than 2.4.0 at some point, and only needs to be run +once. If the first version of Sydent you have set up is 2.4.0 or later, or if you have +already run this operation, you don't need to do it again.** + +In the past, the Matrix specification would consider email addresses as case-sensitive. This means +`alice@example.com` and `Alice@example.com` would be seen as two different email addresses +which could each be associated with a different Matrix user ID. + +With [MSC2265](https://github.com/matrix-org/matrix-doc/pull/2265), the Matrix +specification was updated so that email addresses are considered without any case sensitivity (so the two +addresses mentioned in the previous paragraph would be considered as being one and the +same). + +As of version 2.4.0, Sydent supports this change by processing each new association +without case sensitivity. However, some data might remain in the database from earlier +versions when Sydent would support multiple associations for a given email address (by +using variations of the same address with a different case). This means some addresses in +your identity server's database might not have been stored in a format that allows for +case-insensitive processing, or might have duplicate associations. + +To correct this, Sydent 2.4.0 introduces a [script](https://github.com/matrix-org/sydent/blob/main/scripts/casefold_db.py) +that inspects an identity server's database and fixes it to be compatible with this change: + +``` +Usage: /path/to/sydent/scripts/casefold_db.py --config_path=/path/to/sydent.conf [--no-email] [--dry-run] + +Arguments: + * --config_path: path to Sydent's sydent.conf config file + * --no-email: don't send out emails when deleting associations due to duplicates + * --dry-run: don't update database rows and don't send out emails +``` + +If the script finds a duplicate (i.e. an email address with multiple associations), it +keeps the most recent association and deletes the others. If one or more of the Matrix +user IDs that are being dissociated don't match the one being kept, the script also sends an +email to the address to inform the user of the dissocation. + +The default template for this email can be found [here](https://github.com/matrix-org/sydent/blob/main/res/matrix-org/migration_template.eml.j2) +and can be overriden by configuring a custom template directory (by changing the +`templates.path` configuration setting). The custom template must be named `migration_template.eml.j2` +(or `migration_template.eml` if not using Jinja 2 syntax), and will be given the Matrix +user ID being dissociated at render through the variable `mxid`. + +This script is safe to run whilst Sydent is running. + +If the script is not run, there may be associations in your database that can no +longer be looked up and duplicate associations may be registered. From 85bd37639d01c80e9247bef9a67707da20c41dd5 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Fri, 10 Sep 2021 10:32:31 +0100 Subject: [PATCH 17/25] Apply suggestions from code review - Fix typo in changelog - Remove ... between params and return in docstrings - Update copyright year - Remove conditional import for type checking if conditional not needed - Use fallback for email templates instead of has_option - Fix typo in blank server.name warning - Add template path used to template path not found warning - Set values for prometheus and sentry settings even when not enabled - Use fallback for verify_response_template instead of has_option - Set value for internal_bind_address even when API not enabled - Remove random capitilisation in ca_cert_file - Add type hint to ClientTLSOptionsFactory constructor - Add 'is deprecated' comments near old template usages - Move terms_path config option read outside of try: --- changelog.d/385.misc | 2 +- sydent/config/__init__.py | 4 +-- sydent/config/crypto.py | 5 +--- sydent/config/database.py | 4 +-- sydent/config/email.py | 13 +++------- sydent/config/general.py | 19 ++++++-------- sydent/config/http.py | 26 ++++++++------------ sydent/config/sms.py | 6 ++--- sydent/http/federation_tls_options.py | 2 +- sydent/http/httpcommon.py | 2 +- sydent/http/servlets/emailservlet.py | 2 ++ sydent/http/servlets/msisdnservlet.py | 2 ++ sydent/http/servlets/store_invite_servlet.py | 1 + sydent/sydent.py | 2 +- sydent/terms/terms.py | 8 +++--- sydent/validators/emailvalidator.py | 1 + tests/test_casefold_migration.py | 1 + tests/test_jinja_templates.py | 4 ++- 18 files changed, 45 insertions(+), 59 deletions(-) diff --git a/changelog.d/385.misc b/changelog.d/385.misc index e509b08b..08af10f1 100644 --- a/changelog.d/385.misc +++ b/changelog.d/385.misc @@ -1 +1 @@ -Move the configuration file handling code into a seperate module. \ No newline at end of file +Move the configuration file handling code into a separate module. \ No newline at end of file diff --git a/sydent/config/__init__.py b/sydent/config/__init__.py index a89f0d13..4db67cd0 100644 --- a/sydent/config/__init__.py +++ b/sydent/config/__init__.py @@ -1,4 +1,4 @@ -# Copyright 2019 New Vector Ltd +# Copyright 2021 New Vector Ltd # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -196,7 +196,7 @@ def parse_from_config_parser(self, cfg: ConfigParser) -> bool: Parse the configuration from a ConfigParser object :param cfg: the configuration to be parsed - ... + :return: Whether or not cfg has been changed and needs saving """ self._parse_config(cfg) diff --git a/sydent/config/crypto.py b/sydent/config/crypto.py index 363a111f..8351aeba 100644 --- a/sydent/config/crypto.py +++ b/sydent/config/crypto.py @@ -14,14 +14,11 @@ import logging -from typing import TYPE_CHECKING +from configparser import ConfigParser import nacl import signedjson.key -if TYPE_CHECKING: - from configparser import ConfigParser - logger = logging.getLogger(__name__) diff --git a/sydent/config/database.py b/sydent/config/database.py index 5be2d4e8..90e8bf41 100644 --- a/sydent/config/database.py +++ b/sydent/config/database.py @@ -12,10 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import TYPE_CHECKING -if TYPE_CHECKING: - from configparser import ConfigParser +from configparser import ConfigParser class DatabaseConfig: diff --git a/sydent/config/email.py b/sydent/config/email.py index d54ba11f..5799a720 100644 --- a/sydent/config/email.py +++ b/sydent/config/email.py @@ -13,10 +13,7 @@ # limitations under the License. import socket -from typing import TYPE_CHECKING - -if TYPE_CHECKING: - from configparser import ConfigParser +from configparser import ConfigParser class EmailConfig: @@ -28,13 +25,9 @@ def parse_config(self, cfg: "ConfigParser") -> None: """ # These two options are deprecated - self.template = None - if cfg.has_option("email", "email.template"): - self.template = cfg.get("email", "email.template") + self.template = self.template = cfg.get("email", "email.template", fallback=None) - self.invite_template = None - if cfg.has_option("email", "email.invite_template"): - self.invite_template = cfg.get("email", "email.invite_template") + self.invite_template = cfg.get("email", "email.invite_template", fallback=None) # This isn't used anywhere... self.validation_subject = cfg.get("email", "email.subject") diff --git a/sydent/config/general.py b/sydent/config/general.py index a2aa8e29..81240f27 100644 --- a/sydent/config/general.py +++ b/sydent/config/general.py @@ -12,19 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. - import logging import os -from typing import TYPE_CHECKING, Set +from configparser import ConfigParser +from typing import Set from jinja2.environment import Environment from jinja2.loaders import FileSystemLoader from sydent.util.ip_range import DEFAULT_IP_RANGE_BLACKLIST, generate_ip_set -if TYPE_CHECKING: - from configparser import ConfigParser - logger = logging.getLogger(__name__) @@ -39,7 +36,7 @@ def parse_config(self, cfg: "ConfigParser") -> None: if self.server_name == "": self.server_name = os.uname()[1] logger.warning( - "You have not specified a server name. I have guessed that this server is called '%s' ." + "You have not specified a server name. I have guessed that this server is called '%s'. " "If this is incorrect, you should edit 'general.server.name' in the config file." % (self.server_name,) ) @@ -55,7 +52,7 @@ def parse_config(self, cfg: "ConfigParser") -> None: } else: logging.warning( - "The path specified by 'general.templates.path' does not exist." + f"The path specified by 'general.templates.path' ({self.templates_path}) does not exist." ) # This is a legacy code-path and assumes that verify_response_template, # email.template, and email.invite_template are defined. @@ -75,13 +72,11 @@ def parse_config(self, cfg: "ConfigParser") -> None: self.address_lookup_limit = cfg.getint("general", "address_lookup_limit") self.prometheus_enabled = cfg.has_option("general", "prometheus_port") - if self.prometheus_enabled: - self.prometheus_port = cfg.getint("general", "prometheus_port") - self.prometheus_addr = cfg.get("general", "prometheus_addr") + self.prometheus_port = cfg.getint("general", "prometheus_port", fallback=None) + self.prometheus_addr = cfg.get("general", "prometheus_addr", fallback=None) self.sentry_enabled = cfg.has_option("general", "sentry_dsn") - if self.sentry_enabled: - self.sentry_dsn = cfg.get("general", "sentry_dsn") + self.sentry_dsn = self.sentry_dsn = cfg.get("general", "sentry_dsn", fallback=None) self.enable_v1_associations = parse_cfg_bool( cfg.get("general", "enable_v1_associations") diff --git a/sydent/config/http.py b/sydent/config/http.py index d145e75e..b57d7ddc 100644 --- a/sydent/config/http.py +++ b/sydent/config/http.py @@ -12,11 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from configparser import NoOptionError -from typing import TYPE_CHECKING - -if TYPE_CHECKING: - from configparser import ConfigParser +from configparser import ConfigParser class HTTPConfig: @@ -27,29 +23,27 @@ def parse_config(self, cfg: "ConfigParser") -> None: :param cfg: the configuration to be parsed """ # This option is deprecated - self.verify_response_template = None - if cfg.has_option("http", "verify_response_template"): - self.verify_response_template = cfg.get("http", "verify_response_template") + self.verify_response_template = cfg.get( + "http", "verify_response_template", fallback=None + ) self.client_bind_address = cfg.get("http", "clientapi.http.bind_address") self.client_port = cfg.getint("http", "clientapi.http.port") # internal port is allowed to be set to an empty string in the config self.internal_port = cfg.get("http", "internalapi.http.port") - if self.internal_port: + self.internal_bind_address = cfg.get( + "http", "internalapi.http.bind_address", fallback="::1" + ) + if self.internal_port != "": self.internal_api_enabled = True self.internal_port = int(self.internal_port) - try: - self.internal_bind_address = cfg.get( - "http", "internalapi.http.bind_address" - ) - except NoOptionError: - self.internal_bind_address = "::1" else: self.internal_api_enabled = False + self.internal_port = None self.cert_file = cfg.get("http", "replication.https.certfile") - self.ca_cert_File = cfg.get("http", "replication.https.cacert") + self.ca_cert_file = cfg.get("http", "replication.https.cacert") self.replication_bind_address = cfg.get( "http", "replication.https.bind_address" diff --git a/sydent/config/sms.py b/sydent/config/sms.py index f7368240..60a70136 100644 --- a/sydent/config/sms.py +++ b/sydent/config/sms.py @@ -12,10 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import TYPE_CHECKING, Dict, List - -if TYPE_CHECKING: - from configparser import ConfigParser +from configparser import ConfigParser +from typing import Dict, List class SMSConfig: diff --git a/sydent/http/federation_tls_options.py b/sydent/http/federation_tls_options.py index c7e55042..76ff0814 100644 --- a/sydent/http/federation_tls_options.py +++ b/sydent/http/federation_tls_options.py @@ -89,7 +89,7 @@ class ClientTLSOptionsFactory: """Factory for Twisted ClientTLSOptions that are used to make connections to remote servers for federation.""" - def __init__(self, verify_requests): + def __init__(self, verify_requests: bool): if verify_requests: self._options = ssl.CertificateOptions(trustRoot=ssl.platformTrust()) else: diff --git a/sydent/http/httpcommon.py b/sydent/http/httpcommon.py index c4cf3e88..330f83f6 100644 --- a/sydent/http/httpcommon.py +++ b/sydent/http/httpcommon.py @@ -69,7 +69,7 @@ def makeMyCertificate(self): def makeTrustRoot(self): # If this option is specified, use a specific root CA cert. This is useful for testing when it's not # practical to get the client cert signed by a real root CA but should never be used on a production server. - caCertFilename = self.sydent.config.http.ca_cert_File + caCertFilename = self.sydent.config.http.ca_cert_file if len(caCertFilename) > 0: try: fp = open(caCertFilename) diff --git a/sydent/http/servlets/emailservlet.py b/sydent/http/servlets/emailservlet.py index 3515820e..825a8cda 100644 --- a/sydent/http/servlets/emailservlet.py +++ b/sydent/http/servlets/emailservlet.py @@ -121,6 +121,8 @@ def render_GET(self, request: Request) -> bytes: msg = "Verification failed: you may need to request another verification email" brand = self.sydent.brand_from_request(request) + + # self.sydent.config.http.verify_response_template is deprecated if self.sydent.config.http.verify_response_template is None: templateFile = self.sydent.get_branded_template( brand, diff --git a/sydent/http/servlets/msisdnservlet.py b/sydent/http/servlets/msisdnservlet.py index a3066374..5803a5ab 100644 --- a/sydent/http/servlets/msisdnservlet.py +++ b/sydent/http/servlets/msisdnservlet.py @@ -144,6 +144,8 @@ def render_GET(self, request: Request) -> str: msg = "Verification failed: you may need to request another verification text" brand = self.sydent.brand_from_request(request) + + # self.sydent.config.http.verify_response_template is deprecated if self.sydent.config.http.verify_response_template is None: templateFile = self.sydent.get_branded_template( brand, diff --git a/sydent/http/servlets/store_invite_servlet.py b/sydent/http/servlets/store_invite_servlet.py index 4664829e..90aed29d 100644 --- a/sydent/http/servlets/store_invite_servlet.py +++ b/sydent/http/servlets/store_invite_servlet.py @@ -155,6 +155,7 @@ def render_POST(self, request: Request) -> JsonDict: brand = self.sydent.brand_from_request(request) + # self.sydent.config.email.invite_template is deprecated if self.sydent.config.email.invite_template is None: templateFile = self.sydent.get_branded_template( brand, diff --git a/sydent/sydent.py b/sydent/sydent.py index 1652f0c2..4f348676 100644 --- a/sydent/sydent.py +++ b/sydent/sydent.py @@ -255,7 +255,7 @@ def get_branded_template( :type brand: str or None :param template_name: The name of the template file to load. :type template_name: str - ... + :return: The template filename to use. :rtype: str """ diff --git a/sydent/terms/terms.py b/sydent/terms/terms.py index 6204a676..7ff6da18 100644 --- a/sydent/terms/terms.py +++ b/sydent/terms/terms.py @@ -103,11 +103,13 @@ def get_terms(sydent: "Sydent") -> Optional[Terms]: :returns Terms """ + # TODO - move some of this to parse_config + + termsPath = sydent.config.general.terms_path + try: termsYaml = None - # TODO - move some of this to parse_config - termsPath = sydent.config.general.terms_path if termsPath == "": return Terms(None) @@ -135,6 +137,6 @@ def get_terms(sydent: "Sydent") -> Optional[Terms]: return Terms(termsYaml) except Exception: logger.exception( - "Couldn't read terms file '%s'", sydent.config.general.terms_path + "Couldn't read terms file '%s'", termsPath ) return None diff --git a/sydent/validators/emailvalidator.py b/sydent/validators/emailvalidator.py index b3057256..5b057a70 100644 --- a/sydent/validators/emailvalidator.py +++ b/sydent/validators/emailvalidator.py @@ -63,6 +63,7 @@ def requestToken( valSessionStore.setMtime(valSession.id, time_msec()) + # self.sydent.config.email.template is deprecated if self.sydent.config.email.template is None: templateFile = self.sydent.get_branded_template( brand, diff --git a/tests/test_casefold_migration.py b/tests/test_casefold_migration.py index befc545e..73db5b24 100644 --- a/tests/test_casefold_migration.py +++ b/tests/test_casefold_migration.py @@ -171,6 +171,7 @@ def setUp(self): def test_migration_email(self): with patch("sydent.util.emailutils.smtplib") as smtplib: + # self.sydent.config.email.template is deprecated if self.sydent.config.email.template is None: templateFile = self.sydent.get_branded_template( None, diff --git a/tests/test_jinja_templates.py b/tests/test_jinja_templates.py index cac01323..7eecbc6a 100644 --- a/tests/test_jinja_templates.py +++ b/tests/test_jinja_templates.py @@ -54,6 +54,7 @@ def test_jinja_vector_invite(self): "room_type": "", } + # self.sydent.config.email.invite_template is deprecated if self.sydent.config.email.invite_template is None: templateFile = self.sydent.get_branded_template( "vector-im", @@ -118,7 +119,8 @@ def test_jinja_matrix_invite(self): "web_client_location": "https://matrix.org", "room_type": "", } - + + # self.sydent.config.email.invite_template is deprecated if self.sydent.config.email.invite_template is None: templateFile = self.sydent.get_branded_template( "matrix-org", From 6cf0090eec5600afbc1acb3a740e713de5ef0a11 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Fri, 10 Sep 2021 10:40:40 +0100 Subject: [PATCH 18/25] Run linters --- sydent/config/email.py | 4 +++- sydent/config/general.py | 4 +++- sydent/terms/terms.py | 4 +--- tests/test_jinja_templates.py | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/sydent/config/email.py b/sydent/config/email.py index 5799a720..3ee2ad83 100644 --- a/sydent/config/email.py +++ b/sydent/config/email.py @@ -25,7 +25,9 @@ def parse_config(self, cfg: "ConfigParser") -> None: """ # These two options are deprecated - self.template = self.template = cfg.get("email", "email.template", fallback=None) + self.template = self.template = cfg.get( + "email", "email.template", fallback=None + ) self.invite_template = cfg.get("email", "email.invite_template", fallback=None) diff --git a/sydent/config/general.py b/sydent/config/general.py index 81240f27..aff21d81 100644 --- a/sydent/config/general.py +++ b/sydent/config/general.py @@ -76,7 +76,9 @@ def parse_config(self, cfg: "ConfigParser") -> None: self.prometheus_addr = cfg.get("general", "prometheus_addr", fallback=None) self.sentry_enabled = cfg.has_option("general", "sentry_dsn") - self.sentry_dsn = self.sentry_dsn = cfg.get("general", "sentry_dsn", fallback=None) + self.sentry_dsn = self.sentry_dsn = cfg.get( + "general", "sentry_dsn", fallback=None + ) self.enable_v1_associations = parse_cfg_bool( cfg.get("general", "enable_v1_associations") diff --git a/sydent/terms/terms.py b/sydent/terms/terms.py index 7ff6da18..cd78187c 100644 --- a/sydent/terms/terms.py +++ b/sydent/terms/terms.py @@ -136,7 +136,5 @@ def get_terms(sydent: "Sydent") -> Optional[Terms]: return Terms(termsYaml) except Exception: - logger.exception( - "Couldn't read terms file '%s'", termsPath - ) + logger.exception("Couldn't read terms file '%s'", termsPath) return None diff --git a/tests/test_jinja_templates.py b/tests/test_jinja_templates.py index 7eecbc6a..030e6e90 100644 --- a/tests/test_jinja_templates.py +++ b/tests/test_jinja_templates.py @@ -119,7 +119,7 @@ def test_jinja_matrix_invite(self): "web_client_location": "https://matrix.org", "room_type": "", } - + # self.sydent.config.email.invite_template is deprecated if self.sydent.config.email.invite_template is None: templateFile = self.sydent.get_branded_template( From b0ad7d00c8c5afdb9428a882ddcbc694642b9be9 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Fri, 10 Sep 2021 10:49:18 +0100 Subject: [PATCH 19/25] Make initial read of internalapi.http.port a local variable --- sydent/config/http.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sydent/config/http.py b/sydent/config/http.py index b57d7ddc..544b7530 100644 --- a/sydent/config/http.py +++ b/sydent/config/http.py @@ -31,13 +31,13 @@ def parse_config(self, cfg: "ConfigParser") -> None: self.client_port = cfg.getint("http", "clientapi.http.port") # internal port is allowed to be set to an empty string in the config - self.internal_port = cfg.get("http", "internalapi.http.port") + internal_api_port = cfg.get("http", "internalapi.http.port") self.internal_bind_address = cfg.get( "http", "internalapi.http.bind_address", fallback="::1" ) - if self.internal_port != "": + if internal_api_port != "": self.internal_api_enabled = True - self.internal_port = int(self.internal_port) + self.internal_port = int(internal_api_port) else: self.internal_api_enabled = False self.internal_port = None From 760963e7aad63b43319e2f3b0c92abec60f6aed0 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Fri, 10 Sep 2021 10:53:01 +0100 Subject: [PATCH 20/25] Standardize it being one empty line after licence --- sydent/config/crypto.py | 1 - sydent/config/database.py | 1 - sydent/http/blacklisting_reactor.py | 1 + sydent/threepid/__init__.py | 1 - sydent/users/accounts.py | 1 - sydent/util/stringutils.py | 1 + sydent/validators/__init__.py | 1 - 7 files changed, 2 insertions(+), 5 deletions(-) diff --git a/sydent/config/crypto.py b/sydent/config/crypto.py index 8351aeba..b6989ad0 100644 --- a/sydent/config/crypto.py +++ b/sydent/config/crypto.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. - import logging from configparser import ConfigParser diff --git a/sydent/config/database.py b/sydent/config/database.py index 90e8bf41..713857fe 100644 --- a/sydent/config/database.py +++ b/sydent/config/database.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. - from configparser import ConfigParser diff --git a/sydent/http/blacklisting_reactor.py b/sydent/http/blacklisting_reactor.py index 488ab636..6e1e88c6 100644 --- a/sydent/http/blacklisting_reactor.py +++ b/sydent/http/blacklisting_reactor.py @@ -11,6 +11,7 @@ # 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. + import logging from typing import Any, List, Optional diff --git a/sydent/threepid/__init__.py b/sydent/threepid/__init__.py index 61200ee3..4b9eb3df 100644 --- a/sydent/threepid/__init__.py +++ b/sydent/threepid/__init__.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. - def threePidAssocFromDict(d): """ Instantiates a ThreepidAssociation from the given dict. diff --git a/sydent/users/accounts.py b/sydent/users/accounts.py index 18f3fda8..f20c24e8 100644 --- a/sydent/users/accounts.py +++ b/sydent/users/accounts.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. - class Account: def __init__(self, user_id: str, creation_ts: int, consent_version: str) -> None: """ diff --git a/sydent/util/stringutils.py b/sydent/util/stringutils.py index 267c2d05..c49c400b 100644 --- a/sydent/util/stringutils.py +++ b/sydent/util/stringutils.py @@ -11,6 +11,7 @@ # 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. + import re from typing import Optional, Tuple diff --git a/sydent/validators/__init__.py b/sydent/validators/__init__.py index 105c66fe..a658be30 100644 --- a/sydent/validators/__init__.py +++ b/sydent/validators/__init__.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. - class ValidationSession: # how long a user can wait before validating a session after starting it THREEPID_SESSION_VALIDATION_TIMEOUT_MS = 24 * 60 * 60 * 1000 From d2c24c35a4c151d26c286b07afd98b5a19ef15c4 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Fri, 10 Sep 2021 11:04:01 +0100 Subject: [PATCH 21/25] Document more clearly that parse_config_file sets up logging --- sydent/config/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sydent/config/__init__.py b/sydent/config/__init__.py index 4db67cd0..b1082c94 100644 --- a/sydent/config/__init__.py +++ b/sydent/config/__init__.py @@ -209,7 +209,7 @@ def parse_from_config_parser(self, cfg: ConfigParser) -> bool: def parse_config_file(self, config_file: str) -> None: """ Parse the given config from a filepath, populating missing items and - sections + sections. NOTE: this method also sets up logging. :param config_file: the file to be parsed """ From 64d3d343d02cbebf245220a5ac35b0fc336167c4 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Fri, 10 Sep 2021 11:39:11 +0100 Subject: [PATCH 22/25] Readd lines between licence and code to make linters happy Apparently only have one line if it's between licence and imports! --- sydent/threepid/__init__.py | 1 + sydent/users/accounts.py | 1 + sydent/validators/__init__.py | 1 + 3 files changed, 3 insertions(+) diff --git a/sydent/threepid/__init__.py b/sydent/threepid/__init__.py index 4b9eb3df..61200ee3 100644 --- a/sydent/threepid/__init__.py +++ b/sydent/threepid/__init__.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. + def threePidAssocFromDict(d): """ Instantiates a ThreepidAssociation from the given dict. diff --git a/sydent/users/accounts.py b/sydent/users/accounts.py index f20c24e8..18f3fda8 100644 --- a/sydent/users/accounts.py +++ b/sydent/users/accounts.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. + class Account: def __init__(self, user_id: str, creation_ts: int, consent_version: str) -> None: """ diff --git a/sydent/validators/__init__.py b/sydent/validators/__init__.py index a658be30..105c66fe 100644 --- a/sydent/validators/__init__.py +++ b/sydent/validators/__init__.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. + class ValidationSession: # how long a user can wait before validating a session after starting it THREEPID_SESSION_VALIDATION_TIMEOUT_MS = 24 * 60 * 60 * 1000 From e57fa59637b77f5cce509e3470b14eadddad6ed4 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Fri, 10 Sep 2021 13:45:13 +0100 Subject: [PATCH 23/25] Apply suggestions from code review - `parse_config` now returns bool - Add BaseConfig class - Fix typo - Update licence --- sydent/config/__init__.py | 28 ++++++++++++++++++---------- sydent/config/crypto.py | 17 +++++++++++------ sydent/config/database.py | 4 +++- sydent/config/email.py | 8 ++++++-- sydent/config/general.py | 11 ++++++----- sydent/config/http.py | 6 +++++- sydent/config/sms.py | 6 +++++- 7 files changed, 54 insertions(+), 26 deletions(-) diff --git a/sydent/config/__init__.py b/sydent/config/__init__.py index b1082c94..f93d0eae 100644 --- a/sydent/config/__init__.py +++ b/sydent/config/__init__.py @@ -1,4 +1,4 @@ -# Copyright 2021 New Vector Ltd +# Copyright 2019-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. @@ -181,15 +181,23 @@ def __init__(self): self.http, ] - def _parse_config(self, cfg: ConfigParser) -> None: + def _parse_config(self, cfg: ConfigParser) -> bool: """ Run the parse_config method on each of the objects in self.config_sections :param cfg: the configuration to be parsed + + :return: whether or not cfg has been altered. This method CAN + return True, but it *shouldn't* as this leads to altering the + config file. """ + needs_saving = False for section in self.config_sections: - section.parse_config(cfg) + if section.parse_config(cfg): + needs_saving = True + + return needs_saving def parse_from_config_parser(self, cfg: ConfigParser) -> bool: """ @@ -197,14 +205,11 @@ def parse_from_config_parser(self, cfg: ConfigParser) -> bool: :param cfg: the configuration to be parsed - :return: Whether or not cfg has been changed and needs saving + :return: whether or not cfg has been altered. This method CAN + return True, but it *shouldn't* as this leads to altering the + config file. """ - self._parse_config(cfg) - - # TODO: Don't alter config file when starting Sydent unless - # user has asked for this specifially (e.g. on first - # run only, or when specify --generate-config) - return self.crypto.save_key + return self._parse_config(cfg) def parse_config_file(self, config_file: str) -> None: """ @@ -234,6 +239,9 @@ def parse_config_file(self, config_file: str) -> None: # so that we can log while parsing the rest setup_logging(cfg) + # TODO: Don't alter config file when starting Sydent so that + # it can be set to read-only + needs_saving = self.parse_from_config_parser(cfg) if needs_saving: diff --git a/sydent/config/crypto.py b/sydent/config/crypto.py index b6989ad0..80e9a39c 100644 --- a/sydent/config/crypto.py +++ b/sydent/config/crypto.py @@ -18,11 +18,13 @@ import nacl import signedjson.key +from sydent.config._base import BaseConfig + logger = logging.getLogger(__name__) -class CryptoConfig: - def parse_config(self, cfg: "ConfigParser") -> None: +class CryptoConfig(BaseConfig): + def parse_config(self, cfg: "ConfigParser") -> bool: """ Parse the crypto section of the config :param cfg: the configuration to be parsed @@ -31,7 +33,7 @@ def parse_config(self, cfg: "ConfigParser") -> None: signing_key_str = cfg.get("crypto", "ed25519.signingkey") signing_key_parts = signing_key_str.split(" ") - self.save_key = False + save_key = False if signing_key_str == "": logger.info( @@ -41,7 +43,7 @@ def parse_config(self, cfg: "ConfigParser") -> None: self.signing_key = signedjson.key.generate_signing_key("0") - self.save_key = True + save_key = True elif len(signing_key_parts) == 1: # old format key logger.info("Updating signing key format: brace yourselves") @@ -52,16 +54,19 @@ def parse_config(self, cfg: "ConfigParser") -> None: self.signing_key.version = "0" self.signing_key.alg = signedjson.key.NACL_ED25519 - self.save_key = True + save_key = True else: self.signing_key = signedjson.key.decode_signing_key_base64( signing_key_parts[0], signing_key_parts[1], signing_key_parts[2] ) - if self.save_key: + if save_key: signing_key_str = "%s %s %s" % ( self.signing_key.alg, self.signing_key.version, signedjson.key.encode_signing_key_base64(self.signing_key), ) cfg.set("crypto", "ed25519.signingkey", signing_key_str) + return True + else: + return False diff --git a/sydent/config/database.py b/sydent/config/database.py index 713857fe..709fe98e 100644 --- a/sydent/config/database.py +++ b/sydent/config/database.py @@ -14,8 +14,10 @@ from configparser import ConfigParser +from sydent.config._base import BaseConfig -class DatabaseConfig: + +class DatabaseConfig(BaseConfig): def parse_config(self, cfg: "ConfigParser") -> None: """ Parse the database section of the config diff --git a/sydent/config/email.py b/sydent/config/email.py index 3ee2ad83..94ba9418 100644 --- a/sydent/config/email.py +++ b/sydent/config/email.py @@ -15,9 +15,11 @@ import socket from configparser import ConfigParser +from sydent.config._base import BaseConfig -class EmailConfig: - def parse_config(self, cfg: "ConfigParser") -> None: + +class EmailConfig(BaseConfig): + def parse_config(self, cfg: "ConfigParser") -> bool: """ Parse the email section of the config @@ -63,3 +65,5 @@ def parse_config(self, cfg: "ConfigParser") -> None: self.domain_obfuscate_characters = cfg.getint( "email", "email.third_party_invite_domain_obfuscate_characters" ) + + return False diff --git a/sydent/config/general.py b/sydent/config/general.py index aa339b90..f2a13c9d 100644 --- a/sydent/config/general.py +++ b/sydent/config/general.py @@ -20,13 +20,14 @@ from jinja2.environment import Environment from jinja2.loaders import FileSystemLoader +from sydent.config._base import BaseConfig from sydent.util.ip_range import DEFAULT_IP_RANGE_BLACKLIST, generate_ip_set logger = logging.getLogger(__name__) -class GeneralConfig: - def parse_config(self, cfg: "ConfigParser") -> None: +class GeneralConfig(BaseConfig): + def parse_config(self, cfg: "ConfigParser") -> bool: """ Parse the 'general' section of the config @@ -78,9 +79,7 @@ def parse_config(self, cfg: "ConfigParser") -> None: ) self.sentry_enabled = cfg.has_option("general", "sentry_dsn") - self.sentry_dsn = self.sentry_dsn = cfg.get( - "general", "sentry_dsn", fallback=None - ) + self.sentry_dsn = cfg.get("general", "sentry_dsn", fallback=None) self.enable_v1_associations = parse_cfg_bool( cfg.get("general", "enable_v1_associations") @@ -99,6 +98,8 @@ def parse_config(self, cfg: "ConfigParser") -> None: self.ip_blacklist = generate_ip_set(ip_blacklist) self.ip_whitelist = generate_ip_set(ip_whitelist) + return False + def set_from_comma_sep_string(rawstr: str) -> Set[str]: """ diff --git a/sydent/config/http.py b/sydent/config/http.py index 544b7530..b4c9e279 100644 --- a/sydent/config/http.py +++ b/sydent/config/http.py @@ -14,8 +14,10 @@ from configparser import ConfigParser +from sydent.config._base import BaseConfig -class HTTPConfig: + +class HTTPConfig(BaseConfig): def parse_config(self, cfg: "ConfigParser") -> None: """ Parse the http section of the config @@ -65,3 +67,5 @@ def parse_config(self, cfg: "ConfigParser") -> None: if cfg.has_option(section, "base_replication_url"): base_url = cfg.get(section, "base_replication_url") self.base_replication_urls[peer] = base_url + + return False diff --git a/sydent/config/sms.py b/sydent/config/sms.py index 60a70136..58aa07be 100644 --- a/sydent/config/sms.py +++ b/sydent/config/sms.py @@ -15,8 +15,10 @@ from configparser import ConfigParser from typing import Dict, List +from sydent.config._base import BaseConfig -class SMSConfig: + +class SMSConfig(BaseConfig): def parse_config(self, cfg: "ConfigParser") -> None: """ Parse the sms section of the config @@ -67,3 +69,5 @@ def parse_config(self, cfg: "ConfigParser") -> None: ) self.smsRules[country] = action + + return False From d12d283e7efcafb12e3c221824de0dcca835a0c0 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Fri, 10 Sep 2021 13:46:37 +0100 Subject: [PATCH 24/25] Add missing file from last commit --- sydent/config/_base.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 sydent/config/_base.py diff --git a/sydent/config/_base.py b/sydent/config/_base.py new file mode 100644 index 00000000..1ce575a3 --- /dev/null +++ b/sydent/config/_base.py @@ -0,0 +1,31 @@ +# 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 abc import ABC, abstractmethod +from configparser import ConfigParser + + +class BaseConfig(ABC): + @abstractmethod + def parse_config(self, cfg: ConfigParser) -> bool: + """ + Parse the a section of the config + + :param cfg: the configuration to be parsed + + :return: whether or not cfg has been altered. This method CAN + return True, but it *shouldn't* as this leads to altering the + config file. + """ + pass From e4ce136441091f7d5eb93d7bbf50373afbf13750 Mon Sep 17 00:00:00 2001 From: Azrenbeth <7782548+Azrenbeth@users.noreply.github.com> Date: Mon, 13 Sep 2021 11:35:25 +0100 Subject: [PATCH 25/25] Fix type issues in config code - Update remaining parse-configs to return bool - Add in some type hints - Parse IPs into lists not set (just needs to be Iterable) - Add missing import to __init__.py --- sydent/config/__init__.py | 3 ++- sydent/config/database.py | 4 +++- sydent/config/email.py | 5 ++--- sydent/config/general.py | 14 +++++++------- sydent/config/http.py | 5 +++-- sydent/config/sms.py | 2 +- 6 files changed, 18 insertions(+), 15 deletions(-) diff --git a/sydent/config/__init__.py b/sydent/config/__init__.py index f93d0eae..8e0920cc 100644 --- a/sydent/config/__init__.py +++ b/sydent/config/__init__.py @@ -14,6 +14,7 @@ import copy import logging +import logging.handlers import os from configparser import DEFAULTSECT, ConfigParser from typing import Dict @@ -287,7 +288,7 @@ def setup_logging(cfg: ConfigParser) -> None: logPath = cfg.get("general", "log.path") if logPath != "": - handler = logging.handlers.TimedRotatingFileHandler( + handler: logging.StreamHandler = logging.handlers.TimedRotatingFileHandler( logPath, when="midnight", backupCount=365 ) handler.setFormatter(formatter) diff --git a/sydent/config/database.py b/sydent/config/database.py index 709fe98e..0da8171d 100644 --- a/sydent/config/database.py +++ b/sydent/config/database.py @@ -18,10 +18,12 @@ class DatabaseConfig(BaseConfig): - def parse_config(self, cfg: "ConfigParser") -> None: + def parse_config(self, cfg: "ConfigParser") -> bool: """ Parse the database section of the config :param cfg: the configuration to be parsed """ self.database_path = cfg.get("db", "db.file") + + return False diff --git a/sydent/config/email.py b/sydent/config/email.py index 94ba9418..b6128b9b 100644 --- a/sydent/config/email.py +++ b/sydent/config/email.py @@ -14,6 +14,7 @@ import socket from configparser import ConfigParser +from typing import Optional from sydent.config._base import BaseConfig @@ -27,9 +28,7 @@ def parse_config(self, cfg: "ConfigParser") -> bool: """ # These two options are deprecated - self.template = self.template = cfg.get( - "email", "email.template", fallback=None - ) + self.template: Optional[str] = cfg.get("email", "email.template", fallback=None) self.invite_template = cfg.get("email", "email.invite_template", fallback=None) diff --git a/sydent/config/general.py b/sydent/config/general.py index f2a13c9d..dab20847 100644 --- a/sydent/config/general.py +++ b/sydent/config/general.py @@ -15,7 +15,7 @@ import logging import os from configparser import ConfigParser -from typing import Set +from typing import List from jinja2.environment import Environment from jinja2.loaders import FileSystemLoader @@ -89,11 +89,11 @@ def parse_config(self, cfg: "ConfigParser") -> bool: cfg.get("general", "delete_tokens_on_bind") ) - ip_blacklist = set_from_comma_sep_string(cfg.get("general", "ip.blacklist")) + ip_blacklist = list_from_comma_sep_string(cfg.get("general", "ip.blacklist")) if not ip_blacklist: ip_blacklist = DEFAULT_IP_RANGE_BLACKLIST - ip_whitelist = set_from_comma_sep_string(cfg.get("general", "ip.whitelist")) + ip_whitelist = list_from_comma_sep_string(cfg.get("general", "ip.whitelist")) self.ip_blacklist = generate_ip_set(ip_blacklist) self.ip_whitelist = generate_ip_set(ip_whitelist) @@ -101,15 +101,15 @@ def parse_config(self, cfg: "ConfigParser") -> bool: return False -def set_from_comma_sep_string(rawstr: str) -> Set[str]: +def list_from_comma_sep_string(rawstr: str) -> List[str]: """ - Parse the a comma seperated string into a set + Parse the a comma seperated string into a list :param rawstr: the string to be parsed """ if rawstr == "": - return set() - return {x.strip() for x in rawstr.split(",")} + return [] + return [x.strip() for x in rawstr.split(",")] def parse_cfg_bool(value: str): diff --git a/sydent/config/http.py b/sydent/config/http.py index b4c9e279..a803ab62 100644 --- a/sydent/config/http.py +++ b/sydent/config/http.py @@ -13,12 +13,13 @@ # limitations under the License. from configparser import ConfigParser +from typing import Optional from sydent.config._base import BaseConfig class HTTPConfig(BaseConfig): - def parse_config(self, cfg: "ConfigParser") -> None: + def parse_config(self, cfg: "ConfigParser") -> bool: """ Parse the http section of the config @@ -39,7 +40,7 @@ def parse_config(self, cfg: "ConfigParser") -> None: ) if internal_api_port != "": self.internal_api_enabled = True - self.internal_port = int(internal_api_port) + self.internal_port: Optional[int] = int(internal_api_port) else: self.internal_api_enabled = False self.internal_port = None diff --git a/sydent/config/sms.py b/sydent/config/sms.py index 58aa07be..b0b977cd 100644 --- a/sydent/config/sms.py +++ b/sydent/config/sms.py @@ -19,7 +19,7 @@ class SMSConfig(BaseConfig): - def parse_config(self, cfg: "ConfigParser") -> None: + def parse_config(self, cfg: "ConfigParser") -> bool: """ Parse the sms section of the config