aerc: improve module (#3150)

* aerc: add space after definitions

* aerc: only generate files, if options were set

* aerc: improve file permission warning

* aerc: remove redundant access to builtins

* aerc: allow overwriting of derived values

the order of merging the config subsets did not allow the user to specify
outgoing, source and password command values,
if they were previously derived from the SMTP, IMAP, Maildir etc config.

The values from `account.<name>.extraAccounts` now have the highest precedence.
Appropriate tests were added as well.

* aerc: write primary account first

modules/programs/aerc-accounts.nix

@@ -4,7 +4,6 @@ with lib;
 
 let
   mapAttrNames = f: attr:
-    with builtins;
     listToAttrs (attrValues (mapAttrs (k: v: {
       name = f k;
       value = v;
@@ -59,6 +58,7 @@ in {
             See aerc-config(5).
           '';
         };
+
         extraBinds = mkOption {
           type = confSections;
           default = { };
@@ -70,6 +70,7 @@ in {
             See <citerefentry><refentrytitle>aerc-config</refentrytitle><manvolnum>5</manvolnum></citerefentry>.
           '';
         };
+
         extraConfig = mkOption {
           type = confSections;
           default = { };
@@ -110,14 +111,18 @@ in {
       };
     });
   };
+
   mkAccount = name: account:
     let
       nullOrMap = f: v: if v == null then v else f v;
+
       optPort = port: if port != null then ":${toString port}" else "";
+
       optAttr = k: v:
         if v != null && v != [ ] && v != "" then { ${k} = v; } else { };
+
       optPwCmd = k: p:
-        optAttr "${k}-cred-cmd" (nullOrMap (builtins.concatStringsSep " ") p);
+        optAttr "${k}-cred-cmd" (nullOrMap (concatStringsSep " ") p);
 
       useOauth = auth: builtins.elem auth [ "oauthbearer" "xoauth2" ];
 
@@ -133,6 +138,7 @@ in {
           source =
             "maildir://${config.accounts.email.maildirBasePath}/${cfg.maildir.path}";
         };
+
         imap = { userName, imap, passwordCommand, aerc, ... }@cfg:
           let
             loginMethod' =
@@ -147,11 +153,14 @@ in {
               if imap.tls.useStartTls then "imap" else "imaps${loginMethod'}"
             else
               "imap+insecure";
+
             port' = optPort imap.port;
+
           in {
             source =
               "${protocol}://${userName}@${imap.host}${port'}${oauthParams'}";
           } // optPwCmd "source" passwordCommand;
+
         smtp = { userName, smtp, passwordCommand, ... }@cfg:
           let
             loginMethod' =
@@ -166,25 +175,32 @@ in {
               "smtps${loginMethod'}"
             else
               "smtp${loginMethod'}";
+
             port' = optPort smtp.port;
+
             smtp-starttls =
               if smtp.tls.enable && smtp.tls.useStartTls then "yes" else null;
+
           in {
             outgoing =
               "${protocol}://${userName}@${smtp.host}${port'}${oauthParams'}";
           } // optPwCmd "outgoing" passwordCommand
           // optAttr "smtp-starttls" smtp-starttls;
+
         msmtp = cfg: {
           outgoing = "msmtpq --read-envelope-from --read-recipients";
         };
+
       };
+
       basicCfg = account:
         {
           from = "${account.realName} <${account.address}>";
         } // (optAttr "copy-to" account.folders.sent)
         // (optAttr "default" account.folders.inbox)
         // (optAttr "postpone" account.folders.drafts)
-        // (optAttr "aliases" account.aliases) // account.aerc.extraAccounts;
+        // (optAttr "aliases" account.aliases);
+
       sourceCfg = account:
         if account.mbsync.enable || account.offlineimap.enable then
           mkConfig.maildir account
@@ -192,6 +208,7 @@ in {
           mkConfig.imap account
         else
           { };
+
       outgoingCfg = account:
         if account.msmtp.enable then
           mkConfig.msmtp account
@@ -199,9 +216,13 @@ in {
           mkConfig.smtp account
         else
           { };
-    in (basicCfg account) // (sourceCfg account) // (outgoingCfg account);
+
+    in (basicCfg account) // (sourceCfg account) // (outgoingCfg account)
+    // account.aerc.extraAccounts;
+
   mkAccountConfig = name: account:
     mapAttrNames (addAccountName name) account.aerc.extraConfig;
+
   mkAccountBinds = name: account:
     mapAttrNames (addAccountName name) account.aerc.extraBinds;
 }

modules/programs/aerc.nix

@@ -3,24 +3,32 @@
 with lib;
 let
   cfg = config.programs.aerc;
+
   primitive = with types;
     ((type: either type (listOf type)) (nullOr (oneOf [ str int bool float ])))
     // {
       description =
         "values (null, bool, int, string of float) or a list of values, that will be joined with a comma";
     };
+
   confSection = types.attrsOf primitive;
+
   confSections = types.attrsOf confSection;
+
   sectionsOrLines = types.either types.lines confSections;
+
   accounts = import ./aerc-accounts.nix {
     inherit config pkgs lib confSection confSections;
   };
+
   aerc-accounts =
     attrsets.filterAttrs (_: v: v.aerc.enable) config.accounts.email.accounts;
+
 in {
   meta.maintainers = with lib.hm.maintainers; [ lukasngl ];
 
   options.accounts.email.accounts = accounts.type;
+
   options.programs.aerc = {
 
     enable = mkEnableOption "aerc";
@@ -70,6 +78,7 @@ in {
         See aerc-stylesets(7).
       '';
     };
+
     templates = mkOption {
       type = with types; attrsOf lines;
       default = { };
@@ -84,16 +93,14 @@ in {
   };
 
   config = let
-    joinCfg = cfgs:
+    joinCfg = cfgs: concatStringsSep "\n" (filter (v: v != "") cfgs);
-      with builtins;
+
-      concatStringsSep "\n" (filter (v: v != "") cfgs);
     toINI = conf: # quirk: global section is prepended w/o section heading
       let
         global = conf.global or { };
         local = removeAttrs conf [ "global" ];
         optNewLine = if global != { } && local != { } then "\n" else "";
         mkValueString = v:
-          with builtins;
           if isList v then # join with comma
             concatStringsSep "," (map (generators.mkValueStringDefault { }) v)
           else
@@ -104,58 +111,88 @@ in {
         (generators.toKeyValue { inherit mkKeyValue; } global)
         (generators.toINI { inherit mkKeyValue; } local)
       ];
-    mkINI = conf: if builtins.isString conf then conf else toINI conf;
+
+    mkINI = conf: if isString conf then conf else toINI conf;
+
     mkStyleset = attrsets.mapAttrs' (k: v:
-      let value = if builtins.isString v then v else toINI { global = v; };
+      let value = if isString v then v else toINI { global = v; };
       in {
         name = "aerc/stylesets/${k}";
         value.text = joinCfg [ header value ];
       });
+
     mkTemplates = attrsets.mapAttrs' (k: v: {
       name = "aerc/templates/${k}";
       value.text = v;
     });
-    accountsExtraAccounts = builtins.mapAttrs accounts.mkAccount aerc-accounts;
+
-    accountsExtraConfig =
+    primaryAccount = attrsets.filterAttrs (_: v: v.primary) aerc-accounts;
-      builtins.mapAttrs accounts.mkAccountConfig aerc-accounts;
+    otherAccounts = attrsets.filterAttrs (_: v: !v.primary) aerc-accounts;
-    accountsExtraBinds =
+
-      builtins.mapAttrs accounts.mkAccountBinds aerc-accounts;
+    primaryAccountAccounts = mapAttrs accounts.mkAccount primaryAccount;
-    joinContextual = contextual:
+
-      with builtins;
+    accountsExtraAccounts = mapAttrs accounts.mkAccount otherAccounts;
-      joinCfg (map mkINI (attrValues contextual));
+
+    accountsExtraConfig = mapAttrs accounts.mkAccountConfig aerc-accounts;
+
+    accountsExtraBinds = mapAttrs accounts.mkAccountBinds aerc-accounts;
+
+    joinContextual = contextual: joinCfg (map mkINI (attrValues contextual));
+
+    isRecursivelyEmpty = x:
+      if isAttrs x then
+        all (x: x == { } || isRecursivelyEmpty x) (attrValues x)
+      else
+        false;
+
+    genAccountsConf = ((cfg.extraAccounts != "" && cfg.extraAccounts != { })
+      || !(isRecursivelyEmpty accountsExtraAccounts)
+      || !(isRecursivelyEmpty primaryAccountAccounts));
+
+    genAercConf = ((cfg.extraConfig != "" && cfg.extraConfig != { })
+      || !(isRecursivelyEmpty accountsExtraConfig));
+
+    genBindsConf = ((cfg.extraBinds != "" && cfg.extraBinds != { })
+      || !(isRecursivelyEmpty accountsExtraBinds));
+
     header = ''
       # Generated by Home Manager.
     '';
+
   in mkIf cfg.enable {
-    warnings = if ((cfg.extraAccounts != "" && cfg.extraAccounts != { })
+    warnings = if genAccountsConf
-      || accountsExtraAccounts != { })
     && (cfg.extraConfig.general.unsafe-accounts-conf or false) == false then [''
       aerc: An email account was configured, but `extraConfig.general.unsafe-accounts-conf` is set to false or unset.
-      This will prevent aerc from starting, see `unsafe-accounts-conf` in aerc-config(5) for details.
+      This will prevent aerc from starting, see `unsafe-accounts-conf` in the man page aerc-config(5), which states:
-      Consider setting the option `extraConfig.general.unsafe-accounts-conf` to true.
+      > By default, the file permissions of accounts.conf must be restrictive and only allow reading by the file owner (0600).
+      > Set this option to true to ignore this permission check. Use this with care as it may expose your credentials.
+      These file permissions are not possible with home-manger, since the generated file is stored in the nix-store with read-only access for all users (0444).
+      If `passwordCommand` is properly set, no credentials will be stored in the nix store.
+      Therefore, consider setting the option `extraConfig.general.unsafe-accounts-conf` to true.
     ''] else
       [ ];
+
     home.packages = [ cfg.package ];
+
     xdg.configFile = {
-      "aerc/accounts.conf" = mkIf
+      "aerc/accounts.conf" = mkIf genAccountsConf {
-        ((cfg.extraAccounts != "" && cfg.extraAccounts != { })
-          || accountsExtraAccounts != { }) {
         text = joinCfg [
           header
           (mkINI cfg.extraAccounts)
+          (mkINI primaryAccountAccounts)
           (mkINI accountsExtraAccounts)
         ];
       };
-      "aerc/aerc.conf" =
+
-        mkIf (cfg.extraConfig != "" && cfg.extraConfig != { }) {
+      "aerc/aerc.conf" = mkIf genAercConf {
         text = joinCfg [
           header
           (mkINI cfg.extraConfig)
           (joinContextual accountsExtraConfig)
         ];
       };
-      "aerc/binds.conf" = mkIf ((cfg.extraBinds != "" && cfg.extraBinds != { })
+
-        || accountsExtraBinds != { }) {
+      "aerc/binds.conf" = mkIf genBindsConf {
         text = joinCfg [
           header
           (mkINI cfg.extraBinds)

tests/modules/programs/aerc/extraAccounts.expected

@@ -8,6 +8,10 @@ source = maildir:///dev/null
 [Test2]
 pgp-key-id = 42
 
+[primary]
+from = Foo Bar <addr@mail.invalid>
+source = imap://foobar@imap.host.invalid:1337
+
 [a_imap-nopasscmd-tls-starttls-folders]
 copy-to = aercSent
 default = aercInbox
@@ -74,3 +78,9 @@ outgoing = smtps+login://foobar@smtp.host.invalid:42
 [o_msmtp]
 from = Foo Bar <addr@mail.invalid>
 outgoing = msmtpq --read-envelope-from --read-recipients
+
+[p_overwrite_defaults]
+from = test <test@email.invalid>
+outgoing = imap+plain://intentionallyWrong:PaSsWorD@smtp.host.invalid:1337
+postpone = dRaFts
+source = smtp+plain://intentionallyWrong:PaSsWorD@smtp.host.invalid:1337

tests/modules/programs/aerc/settings.nix

@@ -102,7 +102,7 @@ with lib;
         };
       };
     in {
-      a_imap-nopasscmd-tls-starttls-folders = basics // {
+      primary = basics // {
         primary = true;
         imap = {
           host = "imap.host.invalid";
@@ -110,6 +110,14 @@ with lib;
           tls.enable = true;
           tls.useStartTls = true;
         };
+      };
+      a_imap-nopasscmd-tls-starttls-folders = basics // {
+        imap = {
+          host = "imap.host.invalid";
+          port = 1337;
+          tls.enable = true;
+          tls.useStartTls = true;
+        };
         folders = {
           drafts = "aercDrafts";
           inbox = "aercInbox";
@@ -224,6 +232,21 @@ with lib;
         };
       };
       o_msmtp = basics // { msmtp = { enable = true; }; };
+      p_overwrite_defaults = basics // {
+        smtp.host = "should.be.overwritten.invalid";
+        imap.host = "should.be.overwritten.invalid";
+        aerc = {
+          enable = true;
+          extraAccounts = {
+            from = "test <test@email.invalid>";
+            outgoing =
+              "imap+plain://intentionallyWrong:PaSsWorD@smtp.host.invalid:1337";
+            source =
+              "smtp+plain://intentionallyWrong:PaSsWorD@smtp.host.invalid:1337";
+            postpone = "dRaFts";
+          };
+        };
+      };
     };
   };
 }