From ae545633337ae7a3328f867dc557659692d93a02 Mon Sep 17 00:00:00 2001 From: holger krekel Date: Tue, 9 Jul 2024 14:50:27 +0200 Subject: [PATCH 1/7] introduce "mailboxes_dir" config ini option to avoid hardcoding /home/vmail/mail/.... in source code and to improve testability. --- CHANGELOG.md | 5 +++ chatmaild/src/chatmaild/config.py | 39 ++++++++++++------- chatmaild/src/chatmaild/doveauth.py | 10 ++--- chatmaild/src/chatmaild/ini/chatmail.ini.f | 4 +- chatmaild/src/chatmaild/tests/plugin.py | 5 ++- chatmaild/src/chatmaild/tests/test_config.py | 9 +++-- .../tests/test_delete_inactive_users.py | 14 ++++--- .../src/chatmaild/tests/test_doveauth.py | 5 +-- cmdeploy/src/cmdeploy/dovecot/dovecot.conf.j2 | 2 +- 9 files changed, 59 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bce3485e..26c498db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## untagged +- BREAKING: new required chatmail.ini value + 'mailboxes_dir = /home/vmail/mail/{mail_domain}' + reducing the hardcoding on that directory and improving testability. + ([#351](https://github.com/deltachat/chatmail/pull/351)) + - BREAKING: new required chatmail.ini value 'delete_inactive_users_after = 100' which removes users from database and mails after 100 days without any login. ([#350](https://github.com/deltachat/chatmail/pull/350)) diff --git a/chatmaild/src/chatmaild/config.py b/chatmaild/src/chatmaild/config.py index 48fd98a2..2ab4e901 100644 --- a/chatmaild/src/chatmaild/config.py +++ b/chatmaild/src/chatmaild/config.py @@ -3,17 +3,15 @@ import iniconfig -def read_config(inipath, mail_basedir=None): +def read_config(inipath): assert Path(inipath).exists(), inipath cfg = iniconfig.IniConfig(inipath) params = cfg.sections["params"] - if mail_basedir is None: - mail_basedir = Path(f"/home/vmail/mail/{params['mail_domain']}") - return Config(inipath, params=params, mail_basedir=mail_basedir) + return Config(inipath, params=params) class Config: - def __init__(self, inipath, params, mail_basedir: Path): + def __init__(self, inipath, params): self._inipath = inipath self.mail_domain = params["mail_domain"] self.max_user_send_per_minute = int(params["max_user_send_per_minute"]) @@ -25,6 +23,7 @@ def __init__(self, inipath, params, mail_basedir: Path): self.password_min_length = int(params["password_min_length"]) self.passthrough_senders = params["passthrough_senders"].split() self.passthrough_recipients = params["passthrough_recipients"].split() + self.mailboxes_dir = params["mailboxes_dir"].strip().rstrip("/") self.filtermail_smtp_port = int(params["filtermail_smtp_port"]) self.postfix_reinject_port = int(params["postfix_reinject_port"]) self.iroh_relay = params.get("iroh_relay") @@ -32,26 +31,40 @@ def __init__(self, inipath, params, mail_basedir: Path): self.privacy_mail = params.get("privacy_mail") self.privacy_pdo = params.get("privacy_pdo") self.privacy_supervisor = params.get("privacy_supervisor") - self.mail_basedir = mail_basedir def _getbytefile(self): return open(self._inipath, "rb") def get_user_maildir(self, addr): if addr and addr != "." and "/" not in addr: - res = self.mail_basedir.joinpath(addr).resolve() - if res.is_relative_to(self.mail_basedir): - return res + res = Path(self.mailboxes_dir).joinpath(addr).resolve() + if res.is_relative_to(self.mailboxes_dir): + return str(res) raise ValueError(f"invalid address {addr!r}") -def write_initial_config(inipath, mail_domain): +def write_initial_config(inipath, mail_domain, **overrides): + """Write out default config file, using the specified config value overrides.""" from importlib.resources import files inidir = files(__package__).joinpath("ini") - content = ( - inidir.joinpath("chatmail.ini.f").read_text().format(mail_domain=mail_domain) - ) + source_inipath = inidir.joinpath("chatmail.ini.f") + content = source_inipath.read_text().format(mail_domain=mail_domain) + + # apply config overrides + newlines = [] + for line in content.split("\n"): + newline = line.strip() + if newline and newline[0] not in "#[": + name, value = newline.split(" =", maxsplit=1) + value = overrides.get(name.strip(), value.strip()) + newline = f"{name.strip()} = {value.strip()}" + newlines.append(newline) + + content = "\n".join(newlines) + + # apply testrun privacy overrides + if mail_domain.endswith(".testrun.org"): override_inipath = inidir.joinpath("override-testrun.ini") privacy = iniconfig.IniConfig(override_inipath)["privacy"] diff --git a/chatmaild/src/chatmaild/doveauth.py b/chatmaild/src/chatmaild/doveauth.py index 80fd4235..628c1449 100644 --- a/chatmaild/src/chatmaild/doveauth.py +++ b/chatmaild/src/chatmaild/doveauth.py @@ -68,7 +68,7 @@ def is_allowed_to_create(config: Config, user, cleartext_password) -> bool: def get_user_data(db, config: Config, user): if user == f"echo@{config.mail_domain}": return dict( - home=f"/home/vmail/mail/{config.mail_domain}/echo@{config.mail_domain}", + home=config.get_user_maildir(user), uid="vmail", gid="vmail", ) @@ -76,7 +76,7 @@ def get_user_data(db, config: Config, user): with db.read_connection() as conn: result = conn.get_user(user) if result: - result["home"] = f"/home/vmail/mail/{config.mail_domain}/{user}" + result["home"] = config.get_user_maildir(user) result["uid"] = "vmail" result["gid"] = "vmail" return result @@ -96,7 +96,7 @@ def lookup_passdb(db, config: Config, user, cleartext_password, last_login=None) return None return dict( - home=f"/home/vmail/mail/{config.mail_domain}/echo@{config.mail_domain}", + home=config.get_user_maildir(user), uid="vmail", gid="vmail", password=encrypt_password(password), @@ -114,7 +114,7 @@ def lookup_passdb(db, config: Config, user, cleartext_password, last_login=None) "UPDATE users SET last_login=? WHERE addr=?", (last_login, user) ) - userdata["home"] = f"/home/vmail/mail/{config.mail_domain}/{user}" + userdata["home"] = config.get_user_maildir(user) userdata["uid"] = "vmail" userdata["gid"] = "vmail" return userdata @@ -127,7 +127,7 @@ def lookup_passdb(db, config: Config, user, cleartext_password, last_login=None) conn.execute(q, (user, encrypted_password, last_login)) print(f"Created address: {user}", file=sys.stderr) return dict( - home=f"/home/vmail/mail/{config.mail_domain}/{user}", + home=config.get_user_maildir(user), uid="vmail", gid="vmail", password=encrypted_password, diff --git a/chatmaild/src/chatmaild/ini/chatmail.ini.f b/chatmaild/src/chatmaild/ini/chatmail.ini.f index 181c68cc..fea7b9cd 100644 --- a/chatmaild/src/chatmaild/ini/chatmail.ini.f +++ b/chatmaild/src/chatmaild/ini/chatmail.ini.f @@ -42,6 +42,9 @@ # Deployment Details # +# Directory where user mailboxes are stored +mailboxes_dir = /home/vmail/mail/{mail_domain} + # where the filtermail SMTP service listens filtermail_smtp_port = 10080 @@ -63,4 +66,3 @@ # postal address of the privacy supervisor privacy_supervisor = - diff --git a/chatmaild/src/chatmaild/tests/plugin.py b/chatmaild/src/chatmaild/tests/plugin.py index f83c8f58..ac2ebdcc 100644 --- a/chatmaild/src/chatmaild/tests/plugin.py +++ b/chatmaild/src/chatmaild/tests/plugin.py @@ -16,10 +16,11 @@ def make_config(tmp_path): inipath = tmp_path.joinpath("chatmail.ini") def make_conf(mail_domain): - write_initial_config(inipath, mail_domain=mail_domain) basedir = tmp_path.joinpath(f"vmail/{mail_domain}") basedir.mkdir(parents=True, exist_ok=True) - return read_config(inipath, mail_basedir=basedir) + overrides = dict(mailboxes_dir=str(basedir)) + write_initial_config(inipath, mail_domain=mail_domain, **overrides) + return read_config(inipath) return make_conf diff --git a/chatmaild/src/chatmaild/tests/test_config.py b/chatmaild/src/chatmaild/tests/test_config.py index 38dca6f6..99838e49 100644 --- a/chatmaild/src/chatmaild/tests/test_config.py +++ b/chatmaild/src/chatmaild/tests/test_config.py @@ -1,3 +1,5 @@ +from pathlib import Path + import pytest from chatmaild.config import read_config @@ -35,11 +37,12 @@ def test_read_config_testrun(make_config): def test_get_user_maildir(make_config): config = make_config("something.testrun.org") - assert config.mail_basedir.name == "something.testrun.org" + mailboxes_dir = Path(config.mailboxes_dir) + assert mailboxes_dir.name == "something.testrun.org" assert config.mail_domain == "something.testrun.org" - path = config.get_user_maildir("user1@something.testrun.org") + path = Path(config.get_user_maildir("user1@something.testrun.org")) assert not path.exists() - assert path == config.mail_basedir.joinpath("user1@something.testrun.org") + assert path == mailboxes_dir.joinpath("user1@something.testrun.org") with pytest.raises(ValueError): config.get_user_maildir("") diff --git a/chatmaild/src/chatmaild/tests/test_delete_inactive_users.py b/chatmaild/src/chatmaild/tests/test_delete_inactive_users.py index df22bebe..560e74ef 100644 --- a/chatmaild/src/chatmaild/tests/test_delete_inactive_users.py +++ b/chatmaild/src/chatmaild/tests/test_delete_inactive_users.py @@ -1,4 +1,5 @@ import time +from pathlib import Path from chatmaild.delete_inactive_users import delete_inactive_users from chatmaild.doveauth import lookup_passdb @@ -8,9 +9,12 @@ def test_remove_stale_users(db, example_config): new = time.time() old = new - (example_config.delete_inactive_users_after * 86400) - 1 + def get_user_path(addr): + return Path(example_config.get_user_maildir(addr)) + def create_user(addr, last_login): lookup_passdb(db, example_config, addr, "q9mr3faue", last_login=last_login) - md = example_config.get_user_maildir(addr) + md = get_user_path(addr) md.mkdir(parents=True) md.joinpath("cur").mkdir() md.joinpath("cur", "something").mkdir() @@ -33,19 +37,19 @@ def create_user(addr, last_login): # check pre and post-conditions for delete_inactive_users() for addr in to_remove: - assert example_config.get_user_maildir(addr).exists() + assert get_user_path(addr).exists() delete_inactive_users(db, example_config) - for p in example_config.mail_basedir.iterdir(): + for p in Path(example_config.mailboxes_dir).iterdir(): assert not p.name.startswith("old") for addr in to_remove: + assert not get_user_path(addr).exists() with db.read_connection() as conn: assert not conn.get_user(addr) - assert not example_config.get_user_maildir(addr).exists() for addr in remain: - assert example_config.get_user_maildir(addr).exists() + assert get_user_path(addr).exists() with db.read_connection() as conn: assert conn.get_user(addr) diff --git a/chatmaild/src/chatmaild/tests/test_doveauth.py b/chatmaild/src/chatmaild/tests/test_doveauth.py index 3c49c794..2ac9e239 100644 --- a/chatmaild/src/chatmaild/tests/test_doveauth.py +++ b/chatmaild/src/chatmaild/tests/test_doveauth.py @@ -114,10 +114,7 @@ def test_handle_dovecot_request(db, example_config): assert res assert res[0] == "O" and res.endswith("\n") userdata = json.loads(res[1:].strip()) - assert ( - userdata["home"] - == "/home/vmail/mail/chat.example.org/some42123@chat.example.org" - ) + assert userdata["home"].endswith("chat.example.org/some42123@chat.example.org") assert userdata["uid"] == userdata["gid"] == "vmail" assert userdata["password"].startswith("{SHA512-CRYPT}") diff --git a/cmdeploy/src/cmdeploy/dovecot/dovecot.conf.j2 b/cmdeploy/src/cmdeploy/dovecot/dovecot.conf.j2 index 171300ba..0eb7ba6a 100644 --- a/cmdeploy/src/cmdeploy/dovecot/dovecot.conf.j2 +++ b/cmdeploy/src/cmdeploy/dovecot/dovecot.conf.j2 @@ -67,7 +67,7 @@ userdb { ## # Mailboxes are stored in the "mail" directory of the vmail user home. -mail_location = maildir:/home/vmail/mail/%d/%u +mail_location = maildir:{{ config.mailboxes_dir }}/%u namespace inbox { inbox = yes From e4ddf059522d9f1236a7c1f858c26791b0991c08 Mon Sep 17 00:00:00 2001 From: holger krekel Date: Tue, 9 Jul 2024 20:35:57 +0200 Subject: [PATCH 2/7] remove all occurences of hardcoded /home/vmail for database and mailbox dirs --- CHANGELOG.md | 9 ++++++--- chatmaild/src/chatmaild/config.py | 1 + chatmaild/src/chatmaild/delete_inactive_users.py | 5 +++-- chatmaild/src/chatmaild/doveauth.py | 6 +++--- chatmaild/src/chatmaild/ini/chatmail.ini.f | 3 +++ chatmaild/src/chatmaild/metadata.py | 4 ++-- chatmaild/src/chatmaild/tests/plugin.py | 3 ++- chatmaild/src/chatmaild/tests/test_config.py | 5 ++++- cmdeploy/src/cmdeploy/__init__.py | 2 +- cmdeploy/src/cmdeploy/dovecot/expunge.cron.j2 | 16 ++++++++-------- cmdeploy/src/cmdeploy/metrics.cron.j2 | 2 +- .../cmdeploy/service/chatmail-metadata.service.f | 2 +- cmdeploy/src/cmdeploy/service/doveauth.service.f | 2 +- .../src/cmdeploy/tests/online/test_1_basic.py | 12 ++++++------ 14 files changed, 42 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26c498db..88354e7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,12 @@ ## untagged -- BREAKING: new required chatmail.ini value - 'mailboxes_dir = /home/vmail/mail/{mail_domain}' - reducing the hardcoding on that directory and improving testability. +- BREAKING: new required chatmail.ini values: + + mailboxes_dir = /home/vmail/mail/{mail_domain} + passdb = /home/vmail/passdb.sqlite + + reducing hardcoding these two paths all over the files, also improving testability. ([#351](https://github.com/deltachat/chatmail/pull/351)) - BREAKING: new required chatmail.ini value 'delete_inactive_users_after = 100' diff --git a/chatmaild/src/chatmaild/config.py b/chatmaild/src/chatmaild/config.py index 2ab4e901..c3562472 100644 --- a/chatmaild/src/chatmaild/config.py +++ b/chatmaild/src/chatmaild/config.py @@ -24,6 +24,7 @@ def __init__(self, inipath, params): self.passthrough_senders = params["passthrough_senders"].split() self.passthrough_recipients = params["passthrough_recipients"].split() self.mailboxes_dir = params["mailboxes_dir"].strip().rstrip("/") + self.passdb_path = params["passdb_path"].strip().rstrip("/") self.filtermail_smtp_port = int(params["filtermail_smtp_port"]) self.postfix_reinject_port = int(params["postfix_reinject_port"]) self.iroh_relay = params.get("iroh_relay") diff --git a/chatmaild/src/chatmaild/delete_inactive_users.py b/chatmaild/src/chatmaild/delete_inactive_users.py index 2db1d37f..4d23d2e6 100644 --- a/chatmaild/src/chatmaild/delete_inactive_users.py +++ b/chatmaild/src/chatmaild/delete_inactive_users.py @@ -27,6 +27,7 @@ def delete_inactive_users(db, config, CHUNK=100): def main(): - db = Database(sys.argv[1]) - config = read_config(sys.argv[2]) + (cfgpath,) = sys.argv[1:] + config = read_config(cfgpath) + db = Database(config.passdb_path) delete_inactive_users(db, config) diff --git a/chatmaild/src/chatmaild/doveauth.py b/chatmaild/src/chatmaild/doveauth.py index 628c1449..d1bd938d 100644 --- a/chatmaild/src/chatmaild/doveauth.py +++ b/chatmaild/src/chatmaild/doveauth.py @@ -245,9 +245,9 @@ class ThreadedUnixStreamServer(ThreadingMixIn, UnixStreamServer): def main(): - socket = sys.argv[1] - db = Database(sys.argv[2]) - config = read_config(sys.argv[3]) + socket, cfgpath = sys.argv[1:] + config = read_config(cfgpath) + db = Database(config.passdb_path) class Handler(StreamRequestHandler): def handle(self): diff --git a/chatmaild/src/chatmaild/ini/chatmail.ini.f b/chatmaild/src/chatmaild/ini/chatmail.ini.f index fea7b9cd..c78f8059 100644 --- a/chatmaild/src/chatmaild/ini/chatmail.ini.f +++ b/chatmaild/src/chatmaild/ini/chatmail.ini.f @@ -45,6 +45,9 @@ # Directory where user mailboxes are stored mailboxes_dir = /home/vmail/mail/{mail_domain} +# user address sqlite database path +passdb_path = /home/vmail/passdb.sqlite + # where the filtermail SMTP service listens filtermail_smtp_port = 10080 diff --git a/chatmaild/src/chatmaild/metadata.py b/chatmaild/src/chatmaild/metadata.py index 5b6d62ed..e7f02478 100644 --- a/chatmaild/src/chatmaild/metadata.py +++ b/chatmaild/src/chatmaild/metadata.py @@ -128,12 +128,12 @@ class ThreadedUnixStreamServer(ThreadingMixIn, UnixStreamServer): def main(): - socket, vmail_dir, config_path = sys.argv[1:] + socket, config_path = sys.argv[1:] config = read_config(config_path) iroh_relay = config.iroh_relay - vmail_dir = Path(vmail_dir) + vmail_dir = Path(config.mailboxes_dir) if not vmail_dir.exists(): logging.error("vmail dir does not exist: %r", vmail_dir) return 1 diff --git a/chatmaild/src/chatmaild/tests/plugin.py b/chatmaild/src/chatmaild/tests/plugin.py index ac2ebdcc..8f43ccd0 100644 --- a/chatmaild/src/chatmaild/tests/plugin.py +++ b/chatmaild/src/chatmaild/tests/plugin.py @@ -18,7 +18,8 @@ def make_config(tmp_path): def make_conf(mail_domain): basedir = tmp_path.joinpath(f"vmail/{mail_domain}") basedir.mkdir(parents=True, exist_ok=True) - overrides = dict(mailboxes_dir=str(basedir)) + passdb = tmp_path.joinpath("vmail/passdb.sqlite") + overrides = dict(mailboxes_dir=str(basedir), passdb_path=str(passdb)) write_initial_config(inipath, mail_domain=mail_domain, **overrides) return read_config(inipath) diff --git a/chatmaild/src/chatmaild/tests/test_config.py b/chatmaild/src/chatmaild/tests/test_config.py index 99838e49..0310f54b 100644 --- a/chatmaild/src/chatmaild/tests/test_config.py +++ b/chatmaild/src/chatmaild/tests/test_config.py @@ -35,10 +35,13 @@ def test_read_config_testrun(make_config): assert config.passthrough_senders == [] -def test_get_user_maildir(make_config): +def test_config_userstate_paths(make_config, tmp_path): config = make_config("something.testrun.org") mailboxes_dir = Path(config.mailboxes_dir) + passdb_path = Path(config.passdb_path) assert mailboxes_dir.name == "something.testrun.org" + assert passdb_path.name == "passdb.sqlite" + assert passdb_path.is_relative_to(tmp_path) assert config.mail_domain == "something.testrun.org" path = Path(config.get_user_maildir("user1@something.testrun.org")) assert not path.exists() diff --git a/cmdeploy/src/cmdeploy/__init__.py b/cmdeploy/src/cmdeploy/__init__.py index 36548543..84bec704 100644 --- a/cmdeploy/src/cmdeploy/__init__.py +++ b/cmdeploy/src/cmdeploy/__init__.py @@ -92,7 +92,7 @@ def _install_remote_venv_with_chatmaild(config) -> None: group="root", mode="644", config={ - "mail_domain": config.mail_domain, + "mailboxes_dir": config.mailboxes_dir, "execpath": f"{remote_venv_dir}/bin/chatmail-metrics", }, ) diff --git a/cmdeploy/src/cmdeploy/dovecot/expunge.cron.j2 b/cmdeploy/src/cmdeploy/dovecot/expunge.cron.j2 index 64aa6550..13b170cb 100644 --- a/cmdeploy/src/cmdeploy/dovecot/expunge.cron.j2 +++ b/cmdeploy/src/cmdeploy/dovecot/expunge.cron.j2 @@ -1,12 +1,12 @@ # delete all mails after {{ config.delete_mails_after }} days, in the Inbox -2 0 * * * vmail find /home/vmail/mail/{{ config.mail_domain }} -path '*/cur/*' -mtime +{{ config.delete_mails_after }} -type f -delete +2 0 * * * vmail find {{ config.mailboxes_dir }} -path '*/cur/*' -mtime +{{ config.delete_mails_after }} -type f -delete # or in any IMAP subfolder -2 0 * * * vmail find /home/vmail/mail/{{ config.mail_domain }} -path '*/.*/cur/*' -mtime +{{ config.delete_mails_after }} -type f -delete +2 0 * * * vmail find {{ config.mailboxes_dir }} -path '*/.*/cur/*' -mtime +{{ config.delete_mails_after }} -type f -delete # even if they are unseen -2 0 * * * vmail find /home/vmail/mail/{{ config.mail_domain }} -path '*/new/*' -mtime +{{ config.delete_mails_after }} -type f -delete -2 0 * * * vmail find /home/vmail/mail/{{ config.mail_domain }} -path '*/.*/new/*' -mtime +{{ config.delete_mails_after }} -type f -delete +2 0 * * * vmail find {{ config.mailboxes_dir }} -path '*/new/*' -mtime +{{ config.delete_mails_after }} -type f -delete +2 0 * * * vmail find {{ config.mailboxes_dir }} -path '*/.*/new/*' -mtime +{{ config.delete_mails_after }} -type f -delete # or only temporary (but then they shouldn't be around after {{ config.delete_mails_after }} days anyway). -2 0 * * * vmail find /home/vmail/mail/{{ config.mail_domain }} -path '*/tmp/*' -mtime +{{ config.delete_mails_after }} -type f -delete -2 0 * * * vmail find /home/vmail/mail/{{ config.mail_domain }} -path '*/.*/tmp/*' -mtime +{{ config.delete_mails_after }} -type f -delete -3 0 * * * vmail find /home/vmail/mail/{{ config.mail_domain }} -name 'maildirsize' -type f -delete -4 0 * * * vmail /usr/local/lib/chatmaild/venv/bin/delete_inactive_users /home/vmail/passdb.sqlite /usr/local/lib/chatmaild/chatmail.ini +2 0 * * * vmail find {{ config.mailboxes_dir }} -path '*/tmp/*' -mtime +{{ config.delete_mails_after }} -type f -delete +2 0 * * * vmail find {{ config.mailboxes_dir }} -path '*/.*/tmp/*' -mtime +{{ config.delete_mails_after }} -type f -delete +3 0 * * * vmail find {{ config.mailboxes_dir }} -name 'maildirsize' -type f -delete +4 0 * * * vmail /usr/local/lib/chatmaild/venv/bin/delete_inactive_users /usr/local/lib/chatmaild/chatmail.ini diff --git a/cmdeploy/src/cmdeploy/metrics.cron.j2 b/cmdeploy/src/cmdeploy/metrics.cron.j2 index fbf768ea..8bf036ad 100644 --- a/cmdeploy/src/cmdeploy/metrics.cron.j2 +++ b/cmdeploy/src/cmdeploy/metrics.cron.j2 @@ -1 +1 @@ -*/5 * * * * root {{ config.execpath }} /home/vmail/mail/{{ config.mail_domain }} >/var/www/html/metrics +*/5 * * * * root {{ config.execpath }} {{ config.mailboxes_dir }} >/var/www/html/metrics diff --git a/cmdeploy/src/cmdeploy/service/chatmail-metadata.service.f b/cmdeploy/src/cmdeploy/service/chatmail-metadata.service.f index a3ed9052..b178819d 100644 --- a/cmdeploy/src/cmdeploy/service/chatmail-metadata.service.f +++ b/cmdeploy/src/cmdeploy/service/chatmail-metadata.service.f @@ -2,7 +2,7 @@ Description=Chatmail dict proxy for IMAP METADATA [Service] -ExecStart={execpath} /run/chatmail-metadata/metadata.socket /home/vmail/mail/{mail_domain} {config_path} +ExecStart={execpath} /run/chatmail-metadata/metadata.socket {config_path} Restart=always RestartSec=30 User=vmail diff --git a/cmdeploy/src/cmdeploy/service/doveauth.service.f b/cmdeploy/src/cmdeploy/service/doveauth.service.f index 43b6181a..657430d3 100644 --- a/cmdeploy/src/cmdeploy/service/doveauth.service.f +++ b/cmdeploy/src/cmdeploy/service/doveauth.service.f @@ -2,7 +2,7 @@ Description=Chatmail dict authentication proxy for dovecot [Service] -ExecStart={execpath} /run/doveauth/doveauth.socket /home/vmail/passdb.sqlite {config_path} +ExecStart={execpath} /run/doveauth/doveauth.socket {config_path} Restart=always RestartSec=30 User=vmail diff --git a/cmdeploy/src/cmdeploy/tests/online/test_1_basic.py b/cmdeploy/src/cmdeploy/tests/online/test_1_basic.py index 84010b6a..eea07081 100644 --- a/cmdeploy/src/cmdeploy/tests/online/test_1_basic.py +++ b/cmdeploy/src/cmdeploy/tests/online/test_1_basic.py @@ -108,12 +108,12 @@ def test_exceed_rate_limit(cmsetup, gencreds, maildata, chatmail_config): def test_expunged(remote, chatmail_config): outdated_days = int(chatmail_config.delete_mails_after) + 1 find_cmds = [ - f"find /home/vmail/mail/{chatmail_config.mail_domain} -path '*/cur/*' -mtime +{outdated_days} -type f", - f"find /home/vmail/mail/{chatmail_config.mail_domain} -path '*/.*/cur/*' -mtime +{outdated_days} -type f", - f"find /home/vmail/mail/{chatmail_config.mail_domain} -path '*/new/*' -mtime +{outdated_days} -type f", - f"find /home/vmail/mail/{chatmail_config.mail_domain} -path '*/.*/new/*' -mtime +{outdated_days} -type f", - f"find /home/vmail/mail/{chatmail_config.mail_domain} -path '*/tmp/*' -mtime +{outdated_days} -type f", - f"find /home/vmail/mail/{chatmail_config.mail_domain} -path '*/.*/tmp/*' -mtime +{outdated_days} -type f", + f"find {chatmail_config.mailboxes_dir} -path '*/cur/*' -mtime +{outdated_days} -type f", + f"find {chatmail_config.mailboxes_dir} -path '*/.*/cur/*' -mtime +{outdated_days} -type f", + f"find {chatmail_config.mailboxes_dir} -path '*/new/*' -mtime +{outdated_days} -type f", + f"find {chatmail_config.mailboxes_dir} -path '*/.*/new/*' -mtime +{outdated_days} -type f", + f"find {chatmail_config.mailboxes_dir} -path '*/tmp/*' -mtime +{outdated_days} -type f", + f"find {chatmail_config.mailboxes_dir} -path '*/.*/tmp/*' -mtime +{outdated_days} -type f", ] for cmd in find_cmds: for line in remote.iter_output(cmd): From 7d3dad76f29afc4beb2b757309f2fca2c92c700f Mon Sep 17 00:00:00 2001 From: holger krekel Date: Wed, 10 Jul 2024 11:50:10 +0200 Subject: [PATCH 3/7] Path-ify config.mailboxes_dir --- chatmaild/src/chatmaild/config.py | 4 ++-- chatmaild/src/chatmaild/metadata.py | 3 +-- chatmaild/src/chatmaild/tests/test_config.py | 2 +- chatmaild/src/chatmaild/tests/test_delete_inactive_users.py | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/chatmaild/src/chatmaild/config.py b/chatmaild/src/chatmaild/config.py index c3562472..06458aaf 100644 --- a/chatmaild/src/chatmaild/config.py +++ b/chatmaild/src/chatmaild/config.py @@ -23,7 +23,7 @@ def __init__(self, inipath, params): self.password_min_length = int(params["password_min_length"]) self.passthrough_senders = params["passthrough_senders"].split() self.passthrough_recipients = params["passthrough_recipients"].split() - self.mailboxes_dir = params["mailboxes_dir"].strip().rstrip("/") + self.mailboxes_dir = Path(params["mailboxes_dir"].strip()) self.passdb_path = params["passdb_path"].strip().rstrip("/") self.filtermail_smtp_port = int(params["filtermail_smtp_port"]) self.postfix_reinject_port = int(params["postfix_reinject_port"]) @@ -38,7 +38,7 @@ def _getbytefile(self): def get_user_maildir(self, addr): if addr and addr != "." and "/" not in addr: - res = Path(self.mailboxes_dir).joinpath(addr).resolve() + res = self.mailboxes_dir.joinpath(addr).resolve() if res.is_relative_to(self.mailboxes_dir): return str(res) raise ValueError(f"invalid address {addr!r}") diff --git a/chatmaild/src/chatmaild/metadata.py b/chatmaild/src/chatmaild/metadata.py index e7f02478..8ef66f1e 100644 --- a/chatmaild/src/chatmaild/metadata.py +++ b/chatmaild/src/chatmaild/metadata.py @@ -1,7 +1,6 @@ import logging import os import sys -from pathlib import Path from socketserver import ( StreamRequestHandler, ThreadingMixIn, @@ -133,7 +132,7 @@ def main(): config = read_config(config_path) iroh_relay = config.iroh_relay - vmail_dir = Path(config.mailboxes_dir) + vmail_dir = config.mailboxes_dir if not vmail_dir.exists(): logging.error("vmail dir does not exist: %r", vmail_dir) return 1 diff --git a/chatmaild/src/chatmaild/tests/test_config.py b/chatmaild/src/chatmaild/tests/test_config.py index 0310f54b..94f671c8 100644 --- a/chatmaild/src/chatmaild/tests/test_config.py +++ b/chatmaild/src/chatmaild/tests/test_config.py @@ -37,7 +37,7 @@ def test_read_config_testrun(make_config): def test_config_userstate_paths(make_config, tmp_path): config = make_config("something.testrun.org") - mailboxes_dir = Path(config.mailboxes_dir) + mailboxes_dir = config.mailboxes_dir passdb_path = Path(config.passdb_path) assert mailboxes_dir.name == "something.testrun.org" assert passdb_path.name == "passdb.sqlite" diff --git a/chatmaild/src/chatmaild/tests/test_delete_inactive_users.py b/chatmaild/src/chatmaild/tests/test_delete_inactive_users.py index 560e74ef..418894e0 100644 --- a/chatmaild/src/chatmaild/tests/test_delete_inactive_users.py +++ b/chatmaild/src/chatmaild/tests/test_delete_inactive_users.py @@ -41,7 +41,7 @@ def create_user(addr, last_login): delete_inactive_users(db, example_config) - for p in Path(example_config.mailboxes_dir).iterdir(): + for p in example_config.mailboxes_dir.iterdir(): assert not p.name.startswith("old") for addr in to_remove: From 5b2dfbee4e781e49471806a2e9650640178abacc Mon Sep 17 00:00:00 2001 From: holger krekel Date: Wed, 10 Jul 2024 12:08:48 +0200 Subject: [PATCH 4/7] let config.get_user_maildir return a Path --- chatmaild/src/chatmaild/config.py | 2 +- chatmaild/src/chatmaild/doveauth.py | 10 +++++----- .../chatmaild/tests/test_delete_inactive_users.py | 12 ++++-------- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/chatmaild/src/chatmaild/config.py b/chatmaild/src/chatmaild/config.py index 06458aaf..453efea4 100644 --- a/chatmaild/src/chatmaild/config.py +++ b/chatmaild/src/chatmaild/config.py @@ -40,7 +40,7 @@ def get_user_maildir(self, addr): if addr and addr != "." and "/" not in addr: res = self.mailboxes_dir.joinpath(addr).resolve() if res.is_relative_to(self.mailboxes_dir): - return str(res) + return res raise ValueError(f"invalid address {addr!r}") diff --git a/chatmaild/src/chatmaild/doveauth.py b/chatmaild/src/chatmaild/doveauth.py index d1bd938d..f56decc5 100644 --- a/chatmaild/src/chatmaild/doveauth.py +++ b/chatmaild/src/chatmaild/doveauth.py @@ -68,7 +68,7 @@ def is_allowed_to_create(config: Config, user, cleartext_password) -> bool: def get_user_data(db, config: Config, user): if user == f"echo@{config.mail_domain}": return dict( - home=config.get_user_maildir(user), + home=str(config.get_user_maildir(user)), uid="vmail", gid="vmail", ) @@ -76,7 +76,7 @@ def get_user_data(db, config: Config, user): with db.read_connection() as conn: result = conn.get_user(user) if result: - result["home"] = config.get_user_maildir(user) + result["home"] = str(config.get_user_maildir(user)) result["uid"] = "vmail" result["gid"] = "vmail" return result @@ -96,7 +96,7 @@ def lookup_passdb(db, config: Config, user, cleartext_password, last_login=None) return None return dict( - home=config.get_user_maildir(user), + home=str(config.get_user_maildir(user)), uid="vmail", gid="vmail", password=encrypt_password(password), @@ -114,7 +114,7 @@ def lookup_passdb(db, config: Config, user, cleartext_password, last_login=None) "UPDATE users SET last_login=? WHERE addr=?", (last_login, user) ) - userdata["home"] = config.get_user_maildir(user) + userdata["home"] = str(config.get_user_maildir(user)) userdata["uid"] = "vmail" userdata["gid"] = "vmail" return userdata @@ -127,7 +127,7 @@ def lookup_passdb(db, config: Config, user, cleartext_password, last_login=None) conn.execute(q, (user, encrypted_password, last_login)) print(f"Created address: {user}", file=sys.stderr) return dict( - home=config.get_user_maildir(user), + home=str(config.get_user_maildir(user)), uid="vmail", gid="vmail", password=encrypted_password, diff --git a/chatmaild/src/chatmaild/tests/test_delete_inactive_users.py b/chatmaild/src/chatmaild/tests/test_delete_inactive_users.py index 418894e0..0cdc163f 100644 --- a/chatmaild/src/chatmaild/tests/test_delete_inactive_users.py +++ b/chatmaild/src/chatmaild/tests/test_delete_inactive_users.py @@ -1,5 +1,4 @@ import time -from pathlib import Path from chatmaild.delete_inactive_users import delete_inactive_users from chatmaild.doveauth import lookup_passdb @@ -9,12 +8,9 @@ def test_remove_stale_users(db, example_config): new = time.time() old = new - (example_config.delete_inactive_users_after * 86400) - 1 - def get_user_path(addr): - return Path(example_config.get_user_maildir(addr)) - def create_user(addr, last_login): lookup_passdb(db, example_config, addr, "q9mr3faue", last_login=last_login) - md = get_user_path(addr) + md = example_config.get_user_maildir(addr) md.mkdir(parents=True) md.joinpath("cur").mkdir() md.joinpath("cur", "something").mkdir() @@ -37,7 +33,7 @@ def create_user(addr, last_login): # check pre and post-conditions for delete_inactive_users() for addr in to_remove: - assert get_user_path(addr).exists() + assert example_config.get_user_maildir(addr).exists() delete_inactive_users(db, example_config) @@ -45,11 +41,11 @@ def create_user(addr, last_login): assert not p.name.startswith("old") for addr in to_remove: - assert not get_user_path(addr).exists() + assert not example_config.get_user_maildir(addr).exists() with db.read_connection() as conn: assert not conn.get_user(addr) for addr in remain: - assert get_user_path(addr).exists() + assert example_config.get_user_maildir(addr).exists() with db.read_connection() as conn: assert conn.get_user(addr) From 31df0a24461f70841c946aea1aa569bcf7187e7f Mon Sep 17 00:00:00 2001 From: holger krekel Date: Wed, 10 Jul 2024 12:12:18 +0200 Subject: [PATCH 5/7] don't use kwargs for overrides parameter --- chatmaild/src/chatmaild/config.py | 2 +- chatmaild/src/chatmaild/tests/plugin.py | 2 +- cmdeploy/src/cmdeploy/cmdeploy.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/chatmaild/src/chatmaild/config.py b/chatmaild/src/chatmaild/config.py index 453efea4..312c333a 100644 --- a/chatmaild/src/chatmaild/config.py +++ b/chatmaild/src/chatmaild/config.py @@ -44,7 +44,7 @@ def get_user_maildir(self, addr): raise ValueError(f"invalid address {addr!r}") -def write_initial_config(inipath, mail_domain, **overrides): +def write_initial_config(inipath, mail_domain, overrides): """Write out default config file, using the specified config value overrides.""" from importlib.resources import files diff --git a/chatmaild/src/chatmaild/tests/plugin.py b/chatmaild/src/chatmaild/tests/plugin.py index 8f43ccd0..2aa96149 100644 --- a/chatmaild/src/chatmaild/tests/plugin.py +++ b/chatmaild/src/chatmaild/tests/plugin.py @@ -20,7 +20,7 @@ def make_conf(mail_domain): basedir.mkdir(parents=True, exist_ok=True) passdb = tmp_path.joinpath("vmail/passdb.sqlite") overrides = dict(mailboxes_dir=str(basedir), passdb_path=str(passdb)) - write_initial_config(inipath, mail_domain=mail_domain, **overrides) + write_initial_config(inipath, mail_domain, overrides=overrides) return read_config(inipath) return make_conf diff --git a/cmdeploy/src/cmdeploy/cmdeploy.py b/cmdeploy/src/cmdeploy/cmdeploy.py index 3b26a6ae..bbe9e6ee 100644 --- a/cmdeploy/src/cmdeploy/cmdeploy.py +++ b/cmdeploy/src/cmdeploy/cmdeploy.py @@ -38,7 +38,7 @@ def init_cmd(args, out): if args.inipath.exists(): print(f"Path exists, not modifying: {args.inipath}") else: - write_initial_config(args.inipath, mail_domain) + write_initial_config(args.inipath, mail_domain, overrides={}) out.green(f"created config file for {mail_domain} in {args.inipath}") From e98f9c86bd6f80a6d7df3702bdcc2eedde9c5b30 Mon Sep 17 00:00:00 2001 From: holger krekel Date: Wed, 10 Jul 2024 12:21:31 +0200 Subject: [PATCH 6/7] changing newline-naming as suggested --- chatmaild/src/chatmaild/config.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/chatmaild/src/chatmaild/config.py b/chatmaild/src/chatmaild/config.py index 312c333a..05afd1c9 100644 --- a/chatmaild/src/chatmaild/config.py +++ b/chatmaild/src/chatmaild/config.py @@ -53,16 +53,16 @@ def write_initial_config(inipath, mail_domain, overrides): content = source_inipath.read_text().format(mail_domain=mail_domain) # apply config overrides - newlines = [] + new_lines = [] for line in content.split("\n"): - newline = line.strip() - if newline and newline[0] not in "#[": - name, value = newline.split(" =", maxsplit=1) - value = overrides.get(name.strip(), value.strip()) - newline = f"{name.strip()} = {value.strip()}" - newlines.append(newline) + new_line = line.strip() + if new_line and new_line[0] not in "#[": + name, value = map(str.strip, new_line.split("=", maxsplit=1)) + value = overrides.get(name, value) + new_line = f"{name} = {value}" + new_lines.append(new_line) - content = "\n".join(newlines) + content = "\n".join(new_lines) # apply testrun privacy overrides From 07f59389b3ab75252d653fa26454f70223c559b9 Mon Sep 17 00:00:00 2001 From: holger krekel Date: Wed, 10 Jul 2024 19:10:57 +0200 Subject: [PATCH 7/7] apply last review suggestions --- chatmaild/src/chatmaild/config.py | 2 +- chatmaild/src/chatmaild/tests/test_config.py | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/chatmaild/src/chatmaild/config.py b/chatmaild/src/chatmaild/config.py index 05afd1c9..2076d1fe 100644 --- a/chatmaild/src/chatmaild/config.py +++ b/chatmaild/src/chatmaild/config.py @@ -24,7 +24,7 @@ def __init__(self, inipath, params): self.passthrough_senders = params["passthrough_senders"].split() self.passthrough_recipients = params["passthrough_recipients"].split() self.mailboxes_dir = Path(params["mailboxes_dir"].strip()) - self.passdb_path = params["passdb_path"].strip().rstrip("/") + self.passdb_path = Path(params["passdb_path"].strip()) self.filtermail_smtp_port = int(params["filtermail_smtp_port"]) self.postfix_reinject_port = int(params["postfix_reinject_port"]) self.iroh_relay = params.get("iroh_relay") diff --git a/chatmaild/src/chatmaild/tests/test_config.py b/chatmaild/src/chatmaild/tests/test_config.py index 94f671c8..5a26a01f 100644 --- a/chatmaild/src/chatmaild/tests/test_config.py +++ b/chatmaild/src/chatmaild/tests/test_config.py @@ -1,5 +1,3 @@ -from pathlib import Path - import pytest from chatmaild.config import read_config @@ -38,12 +36,12 @@ def test_read_config_testrun(make_config): def test_config_userstate_paths(make_config, tmp_path): config = make_config("something.testrun.org") mailboxes_dir = config.mailboxes_dir - passdb_path = Path(config.passdb_path) + passdb_path = config.passdb_path assert mailboxes_dir.name == "something.testrun.org" assert passdb_path.name == "passdb.sqlite" assert passdb_path.is_relative_to(tmp_path) assert config.mail_domain == "something.testrun.org" - path = Path(config.get_user_maildir("user1@something.testrun.org")) + path = config.get_user_maildir("user1@something.testrun.org") assert not path.exists() assert path == mailboxes_dir.joinpath("user1@something.testrun.org")