Merge pull request #28 from downtownallday/merge-upstream

Merge upstream

.github/workflows/commit-tests.yml

@@ -22,13 +22,14 @@ jobs:
           path: tests/out/**/screenshot.png
           if-no-files-found: ignore
           retention-days: 5
-    
+
   # install upstream miab, then migrate to miabldap
   upgrade-from-upstream:
     runs-on: ubuntu-22.04
     env:
       PRIMARY_HOSTNAME: box2.abc.com
-      UPSTREAM_TAG: main
+      # TODO: change UPSTREAM_TAG to 'main' once upstream is installable
+      UPSTREAM_TAG: v67
     steps:
       - uses: actions/checkout@v2
       - name: setup

.gitignore

@@ -6,3 +6,4 @@ externals/
 api/docs/api-docs.html
 downloads/
 MAINTAIN/
+*.code-workspace

conf/fail2ban/filter.d/miab-munin.conf

@@ -3,5 +3,5 @@
 before = common.conf
 
 [Definition]
-failregex=<HOST> - .*GET /admin/munin/.* HTTP/1.1\" 401.*
+failregex=<HOST> - .*GET /admin/munin/.* HTTP/\d+\.\d+\" 401.*
 ignoreregex =

management/auth.py

@@ -8,7 +8,7 @@
 ##### details.
 #####
 
-import base64, os, os.path, hmac, json, secrets
+import base64, hmac, json, secrets
 from datetime import timedelta
 
 from expiringdict import ExpiringDict
@@ -32,7 +32,7 @@ def __init__(self):
 	def init_system_api_key(self):
 		"""Write an API key to a local file so local processes can use the API"""
 
-		with open(self.key_path, 'r') as file:
+		with open(self.key_path, encoding='utf-8') as file:
 			self.key = file.read()
 
 	def authenticate(self, request, env, login_only=False, logout=False):
@@ -58,11 +58,13 @@ def decode(s):
 			return username, password
 
 		username, password = parse_http_authorization_basic(request.headers.get('Authorization', ''))
-		if username in (None, ""):
-			raise ValueError("Authorization header invalid.")
+		if username in {None, ""}:
+			msg = "Authorization header invalid."
+			raise ValueError(msg)
 
 		if username.strip() == "" and password.strip() == "":
-			raise ValueError("No email address, password, session key, or API key provided.")
+			msg = "No email address, password, session key, or API key provided."
+			raise ValueError(msg)
 
 		# If user passed the system API key, grant administrative privs. This key
 		# is not associated with a user.
@@ -82,7 +84,8 @@ def decode(s):
 
 		# If no password was given, but a username was given, we're missing some information.
 		elif password.strip() == "":
-			raise ValueError("Enter a password.")
+			msg = "Enter a password."
+			raise ValueError(msg)
 
 		else:
 			# The user is trying to log in with a username and a password
@@ -109,7 +112,8 @@ def check_user_auth(self, email, pw, request, env):
 		# Authenticate.
 		if not validate_login(email, pw, env):
 			# Login failed.
-			raise ValueError("Incorrect email address or password.")
+			msg = "Incorrect email address or password."
+			raise ValueError(msg)
 
 		# If MFA is enabled, check that MFA passes.
 		status, hints = validate_auth_mfa(email, request, env)

management/backup.py

@@ -17,7 +17,7 @@
 # 4) The stopped services are restarted.
 # 5) STORAGE_ROOT/backup/after-backup is executed if it exists.
 
-import os, os.path, shutil, glob, re, datetime, sys
+import os, os.path, re, datetime, sys
 import dateutil.parser, dateutil.relativedelta, dateutil.tz
 import rtyaml
 from exclusiveprocess import Lock
@@ -69,7 +69,7 @@ def parse_line(line):
 		"--archive-dir", backup_cache_dir,
 		"--gpg-options", "'--cipher-algo=AES256'",
 		"--log-fd", "1",
-		] + get_duplicity_additional_args(env) + [
+		*get_duplicity_additional_args(env),
 		get_duplicity_target_url(config)
 		],
 		get_duplicity_env_vars(env),
@@ -79,7 +79,7 @@ def parse_line(line):
 		# destination for the backups or the last backup job terminated unexpectedly.
 		raise Exception("Something is wrong with the backup: " + collection_status)
 	for line in collection_status.split('\n'):
-		if line.startswith(" full") or line.startswith(" inc"):
+		if line.startswith((" full", " inc")):
 			backup = parse_line(line)
 			backups[backup["date"]] = backup
 
@@ -195,7 +195,7 @@ def get_passphrase(env):
 	# only needs to be 43 base64-characters to match AES256's key
 	# length of 32 bytes.
 	backup_root = os.path.join(env["STORAGE_ROOT"], 'backup')
-	with open(os.path.join(backup_root, 'secret_key.txt')) as f:
+	with open(os.path.join(backup_root, 'secret_key.txt'), encoding="utf-8") as f:
 		passphrase = f.readline().strip()
 	if len(passphrase) < 43: raise Exception("secret_key.txt's first line is too short!")
 
@@ -236,7 +236,7 @@ def get_duplicity_additional_args(env):
 			port = 22
 		if port is None:
 			port = 22
-						
+
 		return [
 			f"--ssh-options='-i /root/.ssh/id_rsa_miab -p {port}'",
 			f"--rsync-options='-e \"/usr/bin/ssh -oStrictHostKeyChecking=no -oBatchMode=yes -p {port} -i /root/.ssh/id_rsa_miab\"'",
@@ -267,8 +267,7 @@ def get_duplicity_env_vars(env):
 	return env
 
 def get_target_type(config):
-	protocol = config["target"].split(":")[0]
-	return protocol
+	return config["target"].split(":")[0]
 
 def nuke_local_files(backup_dir, backup_cache_dir, config, env):
 	# the files must be removed manually, duplicity won't do
@@ -351,8 +350,8 @@ def service_command(service, command, quit=None):
 			"--exclude", backup_root,
 			"--volsize", "250",
 			"--gpg-options", "'--cipher-algo=AES256'",
-			"--allow-source-mismatch"
-			] + get_duplicity_additional_args(env) + [
+			"--allow-source-mismatch",
+			*get_duplicity_additional_args(env),
 			env["STORAGE_ROOT"],
 			get_duplicity_target_url(config),
 			],
@@ -375,7 +374,7 @@ def service_command(service, command, quit=None):
 		"--verbosity", "error",
 		"--archive-dir", backup_cache_dir,
 		"--force",
-		] + get_duplicity_additional_args(env) + [
+		*get_duplicity_additional_args(env),
 		get_duplicity_target_url(config)
 		],
 		get_duplicity_env_vars(env))
@@ -391,7 +390,7 @@ def service_command(service, command, quit=None):
 		"--verbosity", "error",
 		"--archive-dir", backup_cache_dir,
 		"--force",
-		] + get_duplicity_additional_args(env) + [
+		*get_duplicity_additional_args(env),
 		get_duplicity_target_url(config)
 		],
 		get_duplicity_env_vars(env))
@@ -430,7 +429,7 @@ def run_duplicity_verification():
 		"--compare-data",
 		"--archive-dir", backup_cache_dir,
 		"--exclude", backup_root,
-		] + get_duplicity_additional_args(env) + [
+		*get_duplicity_additional_args(env),
 		get_duplicity_target_url(config),
 		env["STORAGE_ROOT"],
 	], get_duplicity_env_vars(env))
@@ -443,9 +442,9 @@ def run_duplicity_restore(args):
 		"/usr/bin/duplicity",
 		"restore",
 		"--archive-dir", backup_cache_dir,
-		] + get_duplicity_additional_args(env) + [
-		get_duplicity_target_url(config)
-		] + args,
+		*get_duplicity_additional_args(env),
+		get_duplicity_target_url(config),
+		*args],
 		get_duplicity_env_vars(env))
 
 def print_duplicity_command():
@@ -457,7 +456,7 @@ def print_duplicity_command():
 		print(f"export {k}={shlex.quote(v)}")
 	print("duplicity", "{command}", shlex.join([
 		"--archive-dir", backup_cache_dir,
-		] + get_duplicity_additional_args(env) + [
+		*get_duplicity_additional_args(env),
 		get_duplicity_target_url(config)
 		]))
 
@@ -513,21 +512,22 @@ def list_target_files(config):
 			if 'Permission denied (publickey).' in listing:
 				reason = "Invalid user or check you correctly copied the SSH key."
 			elif 'No such file or directory' in listing:
-				reason = "Provided path {} is invalid.".format(target_path)
+				reason = f"Provided path {target_path} is invalid."
 			elif 'Network is unreachable' in listing:
-				reason = "The IP address {} is unreachable.".format(target.hostname)
+				reason = f"The IP address {target.hostname} is unreachable."
 			elif 'Could not resolve hostname' in listing:
-				reason = "The hostname {} cannot be resolved.".format(target.hostname)
+				reason = f"The hostname {target.hostname} cannot be resolved."
 			else:
-				reason = "Unknown error." \
-						"Please check running 'management/backup.py --verify'" \
-						"from mailinabox sources to debug the issue."
-			raise ValueError("Connection to rsync host failed: {}".format(reason))
+				reason = ("Unknown error."
+						"Please check running 'management/backup.py --verify'"
+						"from mailinabox sources to debug the issue.")
+			msg = f"Connection to rsync host failed: {reason}"
+			raise ValueError(msg)
 
 	elif target.scheme == "s3":
 		import boto3.s3
 		from botocore.exceptions import ClientError
-		
+
 		# separate bucket from path in target
 		bucket = target.path[1:].split('/')[0]
 		path = '/'.join(target.path[1:].split('/')[1:]) + '/'
@@ -537,7 +537,8 @@ def list_target_files(config):
 			path = ''
 
 		if bucket == "":
-			raise ValueError("Enter an S3 bucket name.")
+			msg = "Enter an S3 bucket name."
+			raise ValueError(msg)
 
 		# connect to the region & bucket
 		try:
@@ -555,7 +556,7 @@ def list_target_files(config):
 		from b2sdk.v1.exception import NonExistentBucket
 		info = InMemoryAccountInfo()
 		b2_api = B2Api(info)
-		
+
 		# Extract information from target
 		b2_application_keyid = target.netloc[:target.netloc.index(':')]
 		b2_application_key = urllib.parse.unquote(target.netloc[target.netloc.index(':')+1:target.netloc.index('@')])
@@ -564,8 +565,9 @@ def list_target_files(config):
 		try:
 			b2_api.authorize_account("production", b2_application_keyid, b2_application_key)
 			bucket = b2_api.get_bucket_by_name(b2_bucket)
-		except NonExistentBucket as e:
-			raise ValueError("B2 Bucket does not exist. Please double check your information!")
+		except NonExistentBucket:
+			msg = "B2 Bucket does not exist. Please double check your information!"
+			raise ValueError(msg)
 		return [(key.file_name, key.size) for key, _ in bucket.ls()]
 
 	else:
@@ -586,7 +588,7 @@ def backup_set_custom(env, target, target_user, target_pass, min_age):
 
 	# Validate.
 	try:
-		if config["target"] not in ("off", "local"):
+		if config["target"] not in {"off", "local"}:
 			# these aren't supported by the following function, which expects a full url in the target key,
 			# which is what is there except when loading the config prior to saving
 			list_target_files(config)
@@ -608,9 +610,9 @@ def get_backup_config(env, for_save=False, for_ui=False):
 
 	# Merge in anything written to custom.yaml.
 	try:
-		with open(os.path.join(backup_root, 'custom.yaml'), 'r') as f:
+		with open(os.path.join(backup_root, 'custom.yaml'), encoding="utf-8") as f:
 			custom_config = rtyaml.load(f)
-		if not isinstance(custom_config, dict): raise ValueError() # caught below
+		if not isinstance(custom_config, dict): raise ValueError # caught below
 		config.update(custom_config)
 	except:
 		pass
@@ -634,18 +636,17 @@ def get_backup_config(env, for_save=False, for_ui=False):
 		config["target"] = "file://" + config["file_target_directory"]
 	ssh_pub_key = os.path.join('/root', '.ssh', 'id_rsa_miab.pub')
 	if os.path.exists(ssh_pub_key):
-		with open(ssh_pub_key, 'r') as f:
+		with open(ssh_pub_key, encoding="utf-8") as f:
 			config["ssh_pub_key"] = f.read()
 
 	return config
 
 def write_backup_config(env, newconfig):
 	backup_root = os.path.join(env["STORAGE_ROOT"], 'backup')
-	with open(os.path.join(backup_root, 'custom.yaml'), "w") as f:
+	with open(os.path.join(backup_root, 'custom.yaml'), "w", encoding="utf-8") as f:
 		f.write(rtyaml.dump(newconfig))
 
 if __name__ == "__main__":
-	import sys
 	if sys.argv[-1] == "--verify":
 		# Run duplicity's verification command to check a) the backup files
 		# are readable, and b) report if they are up to date.
@@ -654,7 +655,7 @@ def write_backup_config(env, newconfig):
 	elif sys.argv[-1] == "--list":
 		# List the saved backup files.
 		for fn, size in list_target_files(get_backup_config(load_environment())):
-			print("{}\t{}".format(fn, size))
+			print(f"{fn}\t{size}")
 
 	elif sys.argv[-1] == "--status":
 		# Show backup status.

management/cli.py

@@ -15,7 +15,8 @@
 # root API key. This file is readable only by root, so this
 # tool can only be used as root.
 
-import sys, getpass, urllib.request, urllib.error, json, re, csv
+import sys, getpass, urllib.request, urllib.error, json, csv
+import contextlib
 
 def mgmt(cmd, data=None, is_json=False):
 	# The base URL for the management daemon. (Listens on IPv4 only.)
@@ -28,10 +29,8 @@ def mgmt(cmd, data=None, is_json=False):
 		response = urllib.request.urlopen(req)
 	except urllib.error.HTTPError as e:
 		if e.code == 401:
-			try:
+			with contextlib.suppress(Exception):
 				print(e.read().decode("utf8"))
-			except:
-				pass
 			print("The management daemon refused access. The API key file may be out of sync. Try 'service mailinabox restart'.", file=sys.stderr)
 		elif hasattr(e, 'read'):
 			print(e.read().decode('utf8'), file=sys.stderr)
@@ -56,7 +55,7 @@ def read_password():
     return first
 
 def setup_key_auth(mgmt_uri):
-	with open('/var/lib/mailinabox/api.key', 'r') as f:
+	with open('/var/lib/mailinabox/api.key', encoding='utf-8') as f:
 		key = f.read().strip()
 
 	auth_handler = urllib.request.HTTPBasicAuthHandler()
@@ -100,12 +99,9 @@ def setup_key_auth(mgmt_uri):
 				print("*", end='')
 			print()
 
-elif sys.argv[1] == "user" and sys.argv[2] in ("add", "password"):
+elif sys.argv[1] == "user" and sys.argv[2] in {"add", "password"}:
 	if len(sys.argv) < 5:
-		if len(sys.argv) < 4:
-			email = input("email: ")
-		else:
-			email = sys.argv[3]
+		email = input('email: ') if len(sys.argv) < 4 else sys.argv[3]
 		pw = read_password()
 	else:
 		email, pw = sys.argv[3:5]
@@ -118,11 +114,8 @@ def setup_key_auth(mgmt_uri):
 elif sys.argv[1] == "user" and sys.argv[2] == "remove" and len(sys.argv) == 4:
 	print(mgmt("/mail/users/remove", { "email": sys.argv[3] }))
 
-elif sys.argv[1] == "user" and sys.argv[2] in ("make-admin", "remove-admin") and len(sys.argv) == 4:
-	if sys.argv[2] == "make-admin":
-		action = "add"
-	else:
-		action = "remove"
+elif sys.argv[1] == "user" and sys.argv[2] in {"make-admin", "remove-admin"} and len(sys.argv) == 4:
+	action = 'add' if sys.argv[2] == 'make-admin' else 'remove'
 	print(mgmt("/mail/users/privileges/" + action, { "email": sys.argv[3], "privilege": "admin" }))
 
 elif sys.argv[1] == "user" and sys.argv[2] == "admins":
@@ -141,7 +134,7 @@ def setup_key_auth(mgmt_uri):
 	for mfa in status["enabled_mfa"]:
 		W.writerow([mfa["id"], mfa["type"], mfa["label"]])
 
-elif sys.argv[1] == "user" and len(sys.argv) in (5, 6) and sys.argv[2:4] == ["mfa", "disable"]:
+elif sys.argv[1] == "user" and len(sys.argv) in {5, 6} and sys.argv[2:4] == ["mfa", "disable"]:
 	# Disable MFA (all or a particular device) for a user.
 	print(mgmt("/mfa/disable", { "user": sys.argv[4], "mfa-id": sys.argv[5] if len(sys.argv) == 6 else None }))