From 765f081f6fd5595f102efad10ef82beed2c6531b Mon Sep 17 00:00:00 2001 From: holger krekel Date: Tue, 23 Jul 2024 22:59:02 +0200 Subject: [PATCH] refactor password/login-timestamp handling into a User object --- CHANGELOG.md | 9 ++- chatmaild/src/chatmaild/config.py | 21 ++--- .../src/chatmaild/delete_inactive_users.py | 16 +++- chatmaild/src/chatmaild/doveauth.py | 36 +++------ chatmaild/src/chatmaild/lastlogin.py | 44 +---------- chatmaild/src/chatmaild/migrate_db.py | 13 ++-- chatmaild/src/chatmaild/tests/plugin.py | 5 ++ chatmaild/src/chatmaild/tests/test_config.py | 23 ++---- .../tests/test_delete_inactive_users.py | 41 ++++------ .../src/chatmaild/tests/test_lastlogin.py | 35 ++------- .../src/chatmaild/tests/test_metadata.py | 5 -- .../src/chatmaild/tests/test_migrate_db.py | 6 +- chatmaild/src/chatmaild/tests/test_user.py | 32 ++++++++ chatmaild/src/chatmaild/user.py | 77 +++++++++++++++++++ cmdeploy/src/cmdeploy/tests/plugin.py | 1 - 15 files changed, 193 insertions(+), 171 deletions(-) create mode 100644 chatmaild/src/chatmaild/tests/test_user.py create mode 100644 chatmaild/src/chatmaild/user.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 1dfcae38..15bbfa3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## untagged +- Migrate and remove sqlite database in favor of password/lastlogin tracking + in a user's maildir. This removes the need for "passdb" setting in ini file + which was introduced through #351 below. + ([#379](https://github.com/deltachat/chatmail/pull/379)) + - BREAKING: new required chatmail.ini values: mailboxes_dir = /home/vmail/mail/{mail_domain} @@ -14,10 +19,6 @@ which removes users from database and mails after 100 days without any login. ([#350](https://github.com/deltachat/chatmail/pull/350)) -- Fix and refine "last-login" tracking which now happens via a dedicated - dovecot dictproxy with state kept in "$USERDIR/last-login" files. - ([#354](https://github.com/deltachat/chatmail/pull/354)) - - Refine DNS checking to distinguish between "required" and "recommended" settings ([#372](https://github.com/deltachat/chatmail/pull/372)) diff --git a/chatmaild/src/chatmaild/config.py b/chatmaild/src/chatmaild/config.py index ae7727d3..1da2e71c 100644 --- a/chatmaild/src/chatmaild/config.py +++ b/chatmaild/src/chatmaild/config.py @@ -2,6 +2,8 @@ from pathlib import Path import iniconfig +from chatmaild.user import User + echobot_password_path = Path("/run/echobot/password") @@ -38,16 +40,17 @@ class Config: def _getbytefile(self): return open(self._inipath, "rb") - def get_user_maildir(self, addr): - if addr and addr != "." and addr != ".." and "/" not in addr: - return self.mailboxes_dir.joinpath(addr) - raise ValueError(f"invalid address {addr!r}") + def get_user(self, addr): + if not addr or "@" not in addr or "/" in addr: + raise ValueError(f"invalid address {addr!r}") - def get_user_dict(self, addr, enc_password): - home = self.get_user_maildir(addr) - return dict( - addr=addr, home=str(home), uid="vmail", gid="vmail", password=enc_password - ) + maildir = self.mailboxes_dir.joinpath(addr) + if addr.startswith("echo@"): + password_path = echobot_password_path + else: + password_path = maildir.joinpath("password") + + return User(maildir, addr, password_path, uid="vmail", gid="vmail") def write_initial_config(inipath, mail_domain, overrides): diff --git a/chatmaild/src/chatmaild/delete_inactive_users.py b/chatmaild/src/chatmaild/delete_inactive_users.py index c253653f..81467852 100644 --- a/chatmaild/src/chatmaild/delete_inactive_users.py +++ b/chatmaild/src/chatmaild/delete_inactive_users.py @@ -2,19 +2,27 @@ Remove inactive users """ +import os import shutil import sys import time from .config import read_config -from .lastlogin import get_last_login_from_userdir def delete_inactive_users(config): cutoff_date = time.time() - config.delete_inactive_users_after * 86400 - for userdir in config.mailboxes_dir.iterdir(): - if get_last_login_from_userdir(userdir) < cutoff_date: - shutil.rmtree(userdir, ignore_errors=True) + for addr in os.listdir(config.mailboxes_dir): + try: + user = config.get_user(addr) + except ValueError: + continue + + read_timestamp = user.get_last_login_timestamp() + if read_timestamp and read_timestamp < cutoff_date: + path = config.mailboxes_dir.joinpath(addr) + assert path == user.maildir + shutil.rmtree(path, ignore_errors=True) def main(): diff --git a/chatmaild/src/chatmaild/doveauth.py b/chatmaild/src/chatmaild/doveauth.py index 6ee65e71..fadd5fdd 100644 --- a/chatmaild/src/chatmaild/doveauth.py +++ b/chatmaild/src/chatmaild/doveauth.py @@ -4,9 +4,8 @@ import logging import os import sys -from .config import Config, echobot_password_path, read_config +from .config import Config, read_config from .dictproxy import DictProxy -from .lastlogin import set_user_password from .migrate_db import migrate_from_db_to_maildir NOCREATE_FILE = "/etc/chatmail-nocreate" @@ -128,37 +127,22 @@ class AuthDictProxy(DictProxy): def iter_userdb(self) -> list: """Get a list of all user addresses.""" - getuserpaths = self.config.mailboxes_dir.iterdir() - return [x.name for x in getuserpaths if "@" in x.name] + return [x for x in os.listdir(self.config.mailboxes_dir) if "@" in x] - def lookup_userdb(self, user): - userdir = self.config.get_user_maildir(user) - if user.startswith("echo@"): - password_path = echobot_password_path - else: - password_path = userdir.joinpath("password") + def lookup_userdb(self, addr): + return self.config.get_user(addr).get_userdb_dict() - try: - enc_password = password_path.read_text() - except FileNotFoundError: - return {} - else: - if not enc_password: - # writing the password might have crashed and file is empty - return {} - return self.config.get_user_dict(user, enc_password=enc_password) - - def lookup_passdb(self, user, cleartext_password): - userdata = self.lookup_userdb(user) + def lookup_passdb(self, addr, cleartext_password): + user = self.config.get_user(addr) + userdata = user.get_userdb_dict() if userdata: return userdata - if not is_allowed_to_create(self.config, user, cleartext_password): + if not is_allowed_to_create(self.config, addr, cleartext_password): return - enc_password = encrypt_password(cleartext_password) - set_user_password(self.config, user, enc_password=enc_password) + user.set_password(encrypt_password(cleartext_password)) print(f"Created address: {user}", file=sys.stderr) - return self.config.get_user_dict(user, enc_password=enc_password) + return user.get_userdb_dict() def main(): diff --git a/chatmaild/src/chatmaild/lastlogin.py b/chatmaild/src/chatmaild/lastlogin.py index a7a47534..ceb11a67 100644 --- a/chatmaild/src/chatmaild/lastlogin.py +++ b/chatmaild/src/chatmaild/lastlogin.py @@ -1,48 +1,8 @@ -import os import sys -import time - -import filelock from .config import read_config from .dictproxy import DictProxy -# this file's mtime reflects the last login-time for a user -LAST_LOGIN = "password" - - -def get_daytimestamp(timestamp) -> int: - return int(timestamp) // 86400 * 86400 - - -def write_last_login_to_userdir(userdir, timestamp): - target = userdir.joinpath(LAST_LOGIN) - timestamp = get_daytimestamp(timestamp) - try: - s = os.stat(target) - except FileNotFoundError: - pass - else: - if int(s.st_mtime) != timestamp: - os.utime(target, (timestamp, timestamp)) - - -def get_last_login_from_userdir(userdir) -> int: - if "@" not in userdir.name or userdir.name.startswith("echo@"): - return get_daytimestamp(time.time()) - target = userdir.joinpath(LAST_LOGIN) - return int(target.stat().st_mtime) - - -def set_user_password(config, addr, enc_password): - assert not addr.startswith("echo@"), addr - userdir = config.get_user_maildir(addr) - userdir.mkdir(exist_ok=True) - password_path = userdir.joinpath("password") - lock_path = password_path.with_suffix(".lock") - with filelock.FileLock(lock_path): - password_path.write_bytes(enc_password.encode("ascii")) - class LastLoginDictProxy(DictProxy): def __init__(self, config): @@ -58,8 +18,8 @@ class LastLoginDictProxy(DictProxy): return addr = keyname[2] timestamp = int(value) - userdir = self.config.get_user_maildir(addr) - write_last_login_to_userdir(userdir, timestamp) + user = self.config.get_user(addr) + user.set_last_login_timestamp(timestamp) else: # Transaction failed. self.transactions[transaction_id]["res"] = "F\n" diff --git a/chatmaild/src/chatmaild/migrate_db.py b/chatmaild/src/chatmaild/migrate_db.py index adc6d4bc..d616489c 100644 --- a/chatmaild/src/chatmaild/migrate_db.py +++ b/chatmaild/src/chatmaild/migrate_db.py @@ -2,13 +2,13 @@ migration code from old sqlite databases into per-maildir "password" files where mtime reflects and is updated to be the "last-login" time. """ + import logging import os import sqlite3 import sys from chatmaild.config import read_config -from chatmaild.lastlogin import set_user_password, write_last_login_to_userdir def get_all_rows(path): @@ -29,6 +29,7 @@ def migrate_from_db_to_maildir(config, chunking=10000): all_rows = get_all_rows(path) + # don't transfer special/CI accounts rows = [row for row in all_rows if row[0][:3] not in ("ci-", "ac_")] logging.info(f"ignoring {len(all_rows)-len(rows)} CI accounts") @@ -36,15 +37,13 @@ def migrate_from_db_to_maildir(config, chunking=10000): for i, row in enumerate(rows): addr = row[0] - # don't transfer special/CI accounts (IOLO) - if addr.startswith("echo@"): - continue enc_password = row[1] - set_user_password(config, addr, enc_password=enc_password) + user = config.get_user(addr) + user.set_password(enc_password) + if len(row) == 3 and row[2]: - homedir = config.mailboxes_dir.joinpath(addr) timestamp = int(row[2]) - write_last_login_to_userdir(homedir, timestamp) + user.set_last_login_timestamp(timestamp) if i > 0 and i % chunking == 0: logging.info(f"migration-progress: {i} passwords transferred") diff --git a/chatmaild/src/chatmaild/tests/plugin.py b/chatmaild/src/chatmaild/tests/plugin.py index 16dcb07d..c704686b 100644 --- a/chatmaild/src/chatmaild/tests/plugin.py +++ b/chatmaild/src/chatmaild/tests/plugin.py @@ -35,6 +35,11 @@ def maildomain(example_config): return example_config.mail_domain +@pytest.fixture +def testaddr(maildomain): + return f"user.name@{maildomain}" + + @pytest.fixture def gencreds(maildomain): count = itertools.count() diff --git a/chatmaild/src/chatmaild/tests/test_config.py b/chatmaild/src/chatmaild/tests/test_config.py index 25f870f2..696f1e0a 100644 --- a/chatmaild/src/chatmaild/tests/test_config.py +++ b/chatmaild/src/chatmaild/tests/test_config.py @@ -41,32 +41,21 @@ def test_config_userstate_paths(make_config, tmp_path): assert passdb_path.name == "passdb.sqlite" assert passdb_path.is_relative_to(tmp_path) assert config.mail_domain == "something.testrun.org" - path = config.get_user_maildir("user1@something.testrun.org") + path = config.get_user("user1@something.testrun.org").maildir assert not path.exists() assert path == mailboxes_dir.joinpath("user1@something.testrun.org") with pytest.raises(ValueError): - config.get_user_maildir("") + config.get_user("") with pytest.raises(ValueError): - config.get_user_maildir(None) + config.get_user(None) with pytest.raises(ValueError): - config.get_user_maildir("../some@something.testrun.org") + config.get_user("../some@something.testrun.org").maildir with pytest.raises(ValueError): - config.get_user_maildir("..") + config.get_user("..").maildir with pytest.raises(ValueError): - config.get_user_maildir(".") - - -def test_config_get_user_dict(make_config, tmp_path): - config = make_config("something.testrun.org") - addr = "user1@something.org" - enc_password = "l1k2j31lk2j3l1k23j123" - data = config.get_user_dict(addr, enc_password=enc_password) - assert addr in str(data["home"]) - assert data["uid"] == "vmail" - assert data["gid"] == "vmail" - assert data["password"] == enc_password + config.get_user(".") diff --git a/chatmaild/src/chatmaild/tests/test_delete_inactive_users.py b/chatmaild/src/chatmaild/tests/test_delete_inactive_users.py index 232b9682..937237b4 100644 --- a/chatmaild/src/chatmaild/tests/test_delete_inactive_users.py +++ b/chatmaild/src/chatmaild/tests/test_delete_inactive_users.py @@ -2,28 +2,17 @@ import time from chatmaild.delete_inactive_users import delete_inactive_users from chatmaild.doveauth import AuthDictProxy -from chatmaild.lastlogin import get_last_login_from_userdir, write_last_login_to_userdir -def test_login_timestamps(tmp_path): - userdir = tmp_path.joinpath("someuser@chat.example.org") - userdir.mkdir() - userdir.joinpath("password").touch() - write_last_login_to_userdir(userdir, timestamp=100000) - assert get_last_login_from_userdir(userdir) == 86400 +def test_login_timestamps(example_config): + testaddr = "someuser@chat.example.org" + user = example_config.get_user(testaddr) - write_last_login_to_userdir(userdir, timestamp=200000) - assert get_last_login_from_userdir(userdir) == 86400 * 2 - - write_last_login_to_userdir(userdir, timestamp=200000) - assert get_last_login_from_userdir(userdir) == 86400 * 2 - - -def test_delete_skips_non_email_dir(example_config): - userdir = example_config.get_user_maildir("something") - userdir.mkdir() - get_last_login_from_userdir(userdir) - assert not list(userdir.iterdir()) + # password file needs to be set because it's mtime tracks last-login time + user.set_password("1l2k3j1l2k3j123") + for i in range(10): + user.set_last_login_timestamp(86400 * 4 + i) + assert user.get_last_login_timestamp() == 86400 * 4 def test_delete_inactive_users(example_config): @@ -33,10 +22,10 @@ def test_delete_inactive_users(example_config): def create_user(addr, last_login): dictproxy.lookup_passdb(addr, "q9mr3faue") - md = example_config.get_user_maildir(addr) - md.joinpath("cur").mkdir() - md.joinpath("cur", "something").mkdir() - write_last_login_to_userdir(md, timestamp=last_login) + user = example_config.get_user(addr) + user.maildir.joinpath("cur").mkdir() + user.maildir.joinpath("cur", "something").mkdir() + user.set_last_login_timestamp(timestamp=last_login) # create some stale and some new accounts to_remove = [] @@ -54,7 +43,7 @@ def test_delete_inactive_users(example_config): # check pre and post-conditions for delete_inactive_users() for addr in to_remove: - assert example_config.get_user_maildir(addr).exists() + assert example_config.get_user(addr).maildir.exists() delete_inactive_users(example_config) @@ -62,9 +51,9 @@ def test_delete_inactive_users(example_config): assert not p.name.startswith("old") for addr in to_remove: - assert not example_config.get_user_maildir(addr).exists() + assert not example_config.get_user(addr).maildir.exists() for addr in remain: - userdir = example_config.get_user_maildir(addr) + userdir = example_config.get_user(addr).maildir assert userdir.exists() assert userdir.joinpath("password").read_text() diff --git a/chatmaild/src/chatmaild/tests/test_lastlogin.py b/chatmaild/src/chatmaild/tests/test_lastlogin.py index 9d379e2b..bb8795c5 100644 --- a/chatmaild/src/chatmaild/tests/test_lastlogin.py +++ b/chatmaild/src/chatmaild/tests/test_lastlogin.py @@ -1,42 +1,35 @@ import time -import pytest from chatmaild.doveauth import AuthDictProxy from chatmaild.lastlogin import ( LastLoginDictProxy, - get_last_login_from_userdir, - write_last_login_to_userdir, ) -@pytest.fixture -def testaddr(): - return "user.name@example.org" - - def test_handle_dovecot_request_last_login(testaddr, example_config): dictproxy = LastLoginDictProxy(config=example_config) authproxy = AuthDictProxy(config=example_config) authproxy.lookup_passdb(testaddr, "1l2k3j1l2k3jl123") - userdir = dictproxy.config.get_user_maildir(testaddr) - - # set last-login info for user + # Begin transaction tx = "1111" msg = f"B{tx}\t{testaddr}" res = dictproxy.handle_dovecot_request(msg) assert not res assert dictproxy.transactions == {tx: dict(addr=testaddr, res="O\n")} + # set last-login info for user + user = dictproxy.config.get_user(testaddr) timestamp = int(time.time()) msg = f"S{tx}\tshared/last-login/{testaddr}\t{timestamp}" res = dictproxy.handle_dovecot_request(msg) assert not res assert len(dictproxy.transactions) == 1 - read_timestamp = get_last_login_from_userdir(userdir) + read_timestamp = user.get_last_login_timestamp() assert read_timestamp == timestamp // 86400 * 86400 + # finish transaction msg = f"C{tx}" res = dictproxy.handle_dovecot_request(msg) assert res == "O\n" @@ -49,7 +42,7 @@ def test_handle_dovecot_request_last_login_echobot(example_config): authproxy = AuthDictProxy(config=example_config) testaddr = f"echo@{example_config.mail_domain}" authproxy.lookup_passdb(testaddr, "ignore") - userdir = dictproxy.config.get_user_maildir(testaddr) + user = dictproxy.config.get_user(testaddr) # set last-login info for user tx = "1111" @@ -63,17 +56,5 @@ def test_handle_dovecot_request_last_login_echobot(example_config): res = dictproxy.handle_dovecot_request(msg) assert not res assert len(dictproxy.transactions) == 1 - read_timestamp = get_last_login_from_userdir(userdir) - assert read_timestamp == time.time() // 86400 * 86400 - - -def test_login_timestamp(testaddr, example_config): - dictproxy = LastLoginDictProxy(config=example_config) - authproxy = AuthDictProxy(config=example_config) - authproxy.lookup_passdb(testaddr, "1l2k3j1l2k3jl123") - userdir = dictproxy.config.get_user_maildir(testaddr) - write_last_login_to_userdir(userdir, timestamp=100000) - assert get_last_login_from_userdir(userdir) == 86400 - - write_last_login_to_userdir(userdir, timestamp=200000) - assert get_last_login_from_userdir(userdir) == 86400 * 2 + read_timestamp = user.get_last_login_timestamp() + assert read_timestamp is None diff --git a/chatmaild/src/chatmaild/tests/test_metadata.py b/chatmaild/src/chatmaild/tests/test_metadata.py index 7a0573d2..bfba1e75 100644 --- a/chatmaild/src/chatmaild/tests/test_metadata.py +++ b/chatmaild/src/chatmaild/tests/test_metadata.py @@ -33,11 +33,6 @@ def dictproxy(notifier, metadata): return MetadataDictProxy(notifier=notifier, metadata=metadata) -@pytest.fixture -def testaddr(): - return "user.name@example.org" - - @pytest.fixture def testaddr2(): return "user2@example.org" diff --git a/chatmaild/src/chatmaild/tests/test_migrate_db.py b/chatmaild/src/chatmaild/tests/test_migrate_db.py index 0ea0e4f0..e992bf6d 100644 --- a/chatmaild/src/chatmaild/tests/test_migrate_db.py +++ b/chatmaild/src/chatmaild/tests/test_migrate_db.py @@ -1,6 +1,5 @@ import sqlite3 -from chatmaild.lastlogin import get_last_login_from_userdir from chatmaild.migrate_db import migrate_from_db_to_maildir @@ -59,9 +58,10 @@ def test_migration(tmp_path, example_config, caplog): if "@" not in path.name: continue password, last_login = all.pop(path.name) + user = example_config.get_user(path.name) if last_login: - assert get_last_login_from_userdir(path) == last_login - assert password == path.joinpath("password").read_text() + assert user.get_last_login_timestamp() == last_login + assert password == user.get_userdb_dict()["password"] assert not all assert not example_config.passdb_path.exists() diff --git a/chatmaild/src/chatmaild/tests/test_user.py b/chatmaild/src/chatmaild/tests/test_user.py new file mode 100644 index 00000000..df0a19c3 --- /dev/null +++ b/chatmaild/src/chatmaild/tests/test_user.py @@ -0,0 +1,32 @@ +def test_login_timestamp(testaddr, example_config): + user = example_config.get_user(testaddr) + user.set_password("someeqkjwelkqwjleqwe") + user.set_last_login_timestamp(100000) + assert user.get_last_login_timestamp() == 86400 + + user.set_last_login_timestamp(200000) + assert user.get_last_login_timestamp() == 86400 * 2 + + +def test_get_user_dict_not_set(testaddr, example_config, caplog): + user = example_config.get_user(testaddr) + assert not caplog.records + assert user.get_userdb_dict() == {} + assert len(caplog.records) == 1 + + user.set_password("") + assert user.get_userdb_dict() == {} + assert len(caplog.records) == 2 + + +def test_get_user_dict(make_config, tmp_path): + config = make_config("something.testrun.org") + addr = "user1@something.org" + user = config.get_user(addr) + enc_password = "l1k2j31lk2j3l1k23j123" + user.set_password(enc_password) + data = user.get_userdb_dict() + assert addr in str(data["home"]) + assert data["uid"] == "vmail" + assert data["gid"] == "vmail" + assert data["password"] == enc_password diff --git a/chatmaild/src/chatmaild/user.py b/chatmaild/src/chatmaild/user.py new file mode 100644 index 00000000..25bc2898 --- /dev/null +++ b/chatmaild/src/chatmaild/user.py @@ -0,0 +1,77 @@ +import logging +import os + +import filelock + + +def get_daytimestamp(timestamp) -> int: + return int(timestamp) // 86400 * 86400 + + +class User: + def __init__(self, maildir, addr, password_path, uid, gid): + self.maildir = maildir + self.addr = addr + self.password_path = password_path + self.uid = uid + self.gid = gid + + @property + def can_track(self): + return "@" in self.addr and not self.addr.startswith("echo@") + + def get_userdb_dict(self): + """Return a non-empty dovecot 'userdb' style dict + if the user has an existing non-empty password""" + try: + pw = self.password_path.read_text() + except FileNotFoundError: + logging.error(f"password not set for: {self.addr}") + return {} + + if not pw: + logging.error(f"password is empty for: {self.addr}") + return {} + + home = str(self.maildir) + return dict(addr=self.addr, home=home, uid=self.uid, gid=self.gid, password=pw) + + def set_password(self, enc_password): + """Set the specified password for this user. + + If called concurrently from multiple threads + the last password set call will be persisted. + """ + self.maildir.mkdir(exist_ok=True) + lock_path = self.maildir.joinpath("password.lock") + password = enc_password.encode("ascii") + + with filelock.FileLock(lock_path): + try: + self.password_path.write_bytes(password) + except PermissionError: + if not self.addr.startswith("echo@"): + logging.error(f"could not write password for: {self.addr}") + raise + + def set_last_login_timestamp(self, timestamp): + """Track login time with daily granularity + to minimize touching files and to minimize metadata leakage.""" + if not self.can_track: + return + try: + mtime = int(os.stat(self.password_path).st_mtime) + except FileNotFoundError: + logging.error(f"Can not get last login timestamp for {self.addr}") + return + + timestamp = get_daytimestamp(timestamp) + if mtime != timestamp: + os.utime(self.password_path, (timestamp, timestamp)) + + def get_last_login_timestamp(self): + if self.can_track: + try: + return int(self.password_path.stat().st_mtime) + except FileNotFoundError: + pass diff --git a/cmdeploy/src/cmdeploy/tests/plugin.py b/cmdeploy/src/cmdeploy/tests/plugin.py index bb73a76f..beae6e5a 100644 --- a/cmdeploy/src/cmdeploy/tests/plugin.py +++ b/cmdeploy/src/cmdeploy/tests/plugin.py @@ -261,7 +261,6 @@ def gencreds(chatmail_config): return lambda domain=None: next(gen(domain)) - # # Delta Chat testplugin re-use # use the cmfactory fixture to get chatmail instance accounts