postfix: forwarding emails of login accounts with keeping local copy

When a local account address is forwarded, the mails were not locally
kept. This was due to the way lookup tables were internally managed.

Instead of using lists to represent Postfix lookup tables, we now use
attribute sets: they can then be easily merged.

A regression test for
https://gitlab.com/simple-nixos-mailserver/nixos-mailserver/-/issues/
has been added: it sets a forward on a local address and ensure an
email sent to this address is locally kept.

Fixes #205

3 files changed, 122 insertions, 54 deletions

diff --git a/mail-server/postfix.nix b/mail-server/postfix.nix
index 4894a57..971c833 100644
--- a/mail-server/postfix.nix
+++ b/mail-server/postfix.nix
@@ -22,25 +22,48 @@ let
   inherit (lib.strings) concatStringsSep;
   cfg = config.mailserver;
 
-  # valiases_postfix :: [ String ]
-  valiases_postfix = lib.flatten (lib.mapAttrsToList
+  # Merge several lookup tables. A lookup table is a attribute set where
+  # - the key is an address (user@example.com) or a domain (@example.com)
+  # - the value is a list of addresses
+  mergeLookupTables = tables: lib.zipAttrsWith (n: v: lib.flatten v) tables;
+
+  # valiases_postfix :: Map String [String]
+  valiases_postfix = mergeLookupTables (lib.flatten (lib.mapAttrsToList
     (name: value:
       let to = name;
-          in map (from: "${from} ${to}") (value.aliases ++ lib.singleton name))
-    cfg.loginAccounts);
+      in map (from: {"${from}" = to;}) (value.aliases ++ lib.singleton name))
+    cfg.loginAccounts));
 
-  # catchAllPostfix :: [ String ]
-  catchAllPostfix = lib.flatten (lib.mapAttrsToList
+  # catchAllPostfix :: Map String [String]
+  catchAllPostfix =  mergeLookupTables (lib.flatten (lib.mapAttrsToList
     (name: value:
       let to = name;
-          in map (from: "@${from} ${to}") value.catchAll)
-    cfg.loginAccounts);
+      in map (from: {"@${from}" = to;}) value.catchAll)
+      cfg.loginAccounts));
+
+  # all_valiases_postfix :: Map String [String]
+  all_valiases_postfix = mergeLookupTables [valiases_postfix extra_valiases_postfix];
+
+  # attrsToLookupTable :: Map String (Either String [ String ]) -> Map String [String]
+  attrsToLookupTable = aliases: let
+    lookupTables = lib.mapAttrsToList (from: to: {"${from}" = to;}) aliases;
+  in mergeLookupTables lookupTables;
 
-  # extra_valiases_postfix :: [ String ]
-  extra_valiases_postfix = attrsToAliasList cfg.extraVirtualAliases;
+  # extra_valiases_postfix :: Map String [String]
+  extra_valiases_postfix = attrsToLookupTable cfg.extraVirtualAliases;
 
-  # all_valiases_postfix :: [ String ]
-  all_valiases_postfix = valiases_postfix ++ extra_valiases_postfix;
+  # forwards :: Map String [String]
+  forwards = attrsToLookupTable cfg.forwards;
+
+  # lookupTableToString :: Map String [String] -> String
+  lookupTableToString = attrs: let
+    valueToString = value: lib.concatStringsSep ", " value;
+  in lib.concatStringsSep "\n" (lib.mapAttrsToList (name: value: "${name} ${valueToString value}") attrs);
+
+  # valiases_file :: Path
+  valiases_file = let
+    content = lookupTableToString (mergeLookupTables [all_valiases_postfix catchAllPostfix]);
+  in builtins.toFile "valias" content;
 
   # denied_recipients_postfix :: [ String ]
   denied_recipients_postfix = (map
@@ -48,29 +71,12 @@ let
     (lib.filter (acct: acct.sendOnly) (lib.attrValues cfg.loginAccounts)));
   denied_recipients_file = builtins.toFile "denied_recipients" (lib.concatStringsSep "\n" denied_recipients_postfix);
 
-  # attrsToAliasList :: Map String (Either String [ String ]) -> [ String ]
-  attrsToAliasList = aliases:
-    let
-      toList = to: if builtins.isList to then to else [to];
-    in lib.mapAttrsToList
-         (from: to: "${from} " + (lib.concatStringsSep ", " (toList to)))
-         aliases;
-
-  # forwards :: [ String ]
-  forwards = attrsToAliasList cfg.forwards;
-
-  # valiases_file :: Path
-  valiases_file = builtins.toFile "valias"
-                      (lib.concatStringsSep "\n" (all_valiases_postfix ++
-                                                  catchAllPostfix));
-
   reject_senders_postfix = (map
     (sender:
       "${sender} REJECT")
     (cfg.rejectSender));
   reject_senders_file = builtins.toFile "reject_senders" (lib.concatStringsSep "\n" (reject_senders_postfix))  ;
 
-
   reject_recipients_postfix = (map
     (recipient:
       "${recipient} REJECT")
@@ -87,7 +93,7 @@ let
   # for details on how this file looks. By using the same file as valiases,
   # every alias is owned (uniquely) by its user.
   # The user's own address is already in all_valiases_postfix.
-  vaccounts_file = builtins.toFile "vaccounts" (lib.concatStringsSep "\n" all_valiases_postfix);
+  vaccounts_file = builtins.toFile "vaccounts" (lookupTableToString all_valiases_postfix);
 
   submissionHeaderCleanupRules = pkgs.writeText "submission_header_cleanup_rules" (''
      # Removes sensitive headers from mails handed in via the submission port.
@@ -149,8 +155,7 @@ in
       sslKey = keyPath;
       enableSubmission = cfg.enableSubmission;
       enableSubmissions = cfg.enableSubmissionSsl;
-      virtual =
-        (lib.concatStringsSep "\n" (all_valiases_postfix ++ catchAllPostfix ++ forwards));
+      virtual = lookupTableToString (mergeLookupTables [all_valiases_postfix catchAllPostfix forwards]);
 
       config = {
         # Extra Config
diff --git a/scripts/mail-check.py b/scripts/mail-check.py
index 47629fb..f5a97a5 100644
--- a/scripts/mail-check.py
+++ b/scripts/mail-check.py
@@ -24,28 +24,36 @@ def _send_mail(smtp_host, smtp_port, from_addr, from_pwd, to_addr, subject, star
 
     retry = RETRY
     while True:
-        with smtplib.SMTP(smtp_host, port=smtp_port) as smtp:
-            if starttls:
-                smtp.starttls()
-            if from_pwd is not None:
-                smtp.login(from_addr, from_pwd)
-            try:
-                smtp.sendmail(from_addr, [to_addr], message)
-                return
-            except smtplib.SMTPResponseException as e:
-                # This is a service unavailable error
-                # In this situation, we want to retry.
-                if e.smtp_code == 451:
-                    if retry > 0:
-                        retry = retry - 1
-                        time.sleep(1)
-                        continue
+        try:
+            with smtplib.SMTP(smtp_host, port=smtp_port) as smtp:
+                try:
+                    if starttls:
+                        smtp.starttls()
+                    if from_pwd is not None:
+                        smtp.login(from_addr, from_pwd)
+
+                    smtp.sendmail(from_addr, [to_addr], message)
+                    return
+                except smtplib.SMTPResponseException as e:
+                    if e.smtp_code == 451:  # service unavailable error
+                        print(e)
+                    elif e.smtp_code == 454:  # smtplib.SMTPResponseException: (454, b'4.3.0 Try again later')
+                        print(e)
                     else:
-                        print("Error while sending mail: %s" % e)
-                        exit(5)
-            except Exception as e:
-                print("Error while sending mail: %s" % e)
-                exit(4)
+                        raise
+        except OSError as e:
+            if e.errno in [16, -2]:
+                print("OSError exception message: ", e)
+            else:
+                raise
+
+        if retry > 0:
+            retry = retry - 1
+            time.sleep(1)
+            print("Retrying")
+        else:
+            print("Retry attempts exhausted")
+            exit(5)
 
 def _read_mail(
         imap_host,
diff --git a/tests/intern.nix b/tests/intern.nix
index 1d1816b..b357ed9 100644
--- a/tests/intern.nix
+++ b/tests/intern.nix
@@ -33,6 +33,8 @@ let
       htpasswd -nbB "" "${password}" | cut -d: -f2 > $out
     '';
 
+  hashedPasswordFile = hashPassword "my-password";
+  passwordFile = pkgs.writeText "password" "my-password";
 in
 pkgs.nixosTest {
   name = "intern";
@@ -45,20 +47,34 @@ pkgs.nixosTest {
 
       virtualisation.memorySize = 1024;
 
+      environment.systemPackages = [
+        (pkgs.writeScriptBin "mail-check" ''
+          ${pkgs.python3}/bin/python ${../scripts/mail-check.py} $@
+        '')];
+
       mailserver = {
         enable = true;
         fqdn = "mail.example.com";
         domains = [ "example.com" ];
+        localDnsResolver = false;
 
         loginAccounts = {
           "user1@example.com" = {
-            hashedPassword = "$6$/z4n8AQl6K$kiOkBTWlZfBd7PvF5GsJ8PmPgdZsFGN1jPGZufxxr60PoR0oUsrvzm2oQiflyz5ir9fFJ.d/zKm/NgLXNUsNX/";
+            hashedPasswordFile = hashedPasswordFile;
+          };
+          "user2@example.com" = {
+            hashedPasswordFile = hashedPasswordFile;
           };
           "send-only@example.com" = {
             hashedPasswordFile = hashPassword "send-only";
             sendOnly = true;
           };
         };
+        forwards = {
+          # user2@example.com is a local account and its mails are
+          # also forwarded to user1@example.com
+          "user2@example.com" = "user1@example.com";
+        };
 
         vmailGroupName = "vmail";
         vmailUID = 5000;
@@ -71,6 +87,45 @@ pkgs.nixosTest {
     machine.start()
     machine.wait_for_unit("multi-user.target")
 
+    # Regression test for https://gitlab.com/simple-nixos-mailserver/nixos-mailserver/-/issues/205
+    with subtest("mail forwarded can are locally kept"):
+        # A mail sent to user2@example.com is in the user1@example.com mailbox
+        machine.succeed(
+            " ".join(
+                [
+                    "mail-check send-and-read",
+                    "--smtp-port 587",
+                    "--smtp-starttls",
+                    "--smtp-host localhost",
+                    "--imap-host localhost",
+                    "--imap-username user1@example.com",
+                    "--from-addr user1@example.com",
+                    "--to-addr user2@example.com",
+                    "--src-password-file ${passwordFile}",
+                    "--dst-password-file ${passwordFile}",
+                    "--ignore-dkim-spf",
+                ]
+            )
+        )
+        # A mail sent to user2@example.com is in the user2@example.com mailbox
+        machine.succeed(
+            " ".join(
+                [
+                    "mail-check send-and-read",
+                    "--smtp-port 587",
+                    "--smtp-starttls",
+                    "--smtp-host localhost",
+                    "--imap-host localhost",
+                    "--imap-username user2@example.com",
+                    "--from-addr user1@example.com",
+                    "--to-addr user2@example.com",
+                    "--src-password-file ${passwordFile}",
+                    "--dst-password-file ${passwordFile}",
+                    "--ignore-dkim-spf",
+                ]
+            )
+        )
+
     with subtest("vmail gid is set correctly"):
         machine.succeed("getent group vmail | grep 5000")