From 8da1135365829e77fb5b17cfa4587860f306147f Mon Sep 17 00:00:00 2001 From: Lukas Nagel <69244516+lukasngl@users.noreply.github.com> Date: Tue, 13 Jun 2023 10:59:42 +0200 Subject: [PATCH] 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..extraAccounts` now have the highest precedence. Appropriate tests were added as well. * aerc: write primary account first --- modules/programs/aerc-accounts.nix | 29 ++++- modules/programs/aerc.nix | 123 ++++++++++++------ .../programs/aerc/extraAccounts.expected | 10 ++ tests/modules/programs/aerc/settings.nix | 25 +++- 4 files changed, 139 insertions(+), 48 deletions(-) diff --git a/modules/programs/aerc-accounts.nix b/modules/programs/aerc-accounts.nix index e4c3fa7f0..1d4f4053a 100644 --- a/modules/programs/aerc-accounts.nix +++ b/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 aerc-config5. ''; }; + 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; } diff --git a/modules/programs/aerc.nix b/modules/programs/aerc.nix index 95ca984f3..c3f690509 100644 --- a/modules/programs/aerc.nix +++ b/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: - with builtins; - concatStringsSep "\n" (filter (v: v != "") cfgs); + joinCfg = cfgs: 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,64 +111,94 @@ 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 = - builtins.mapAttrs accounts.mkAccountConfig aerc-accounts; - accountsExtraBinds = - builtins.mapAttrs accounts.mkAccountBinds aerc-accounts; - joinContextual = contextual: - with builtins; - joinCfg (map mkINI (attrValues contextual)); + + primaryAccount = attrsets.filterAttrs (_: v: v.primary) aerc-accounts; + otherAccounts = attrsets.filterAttrs (_: v: !v.primary) aerc-accounts; + + primaryAccountAccounts = mapAttrs accounts.mkAccount primaryAccount; + + accountsExtraAccounts = mapAttrs accounts.mkAccount otherAccounts; + + 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 != { }) - || accountsExtraAccounts != { }) + warnings = if genAccountsConf && (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. - Consider setting the option `extraConfig.general.unsafe-accounts-conf` to true. + This will prevent aerc from starting, see `unsafe-accounts-conf` in the man page aerc-config(5), which states: + > 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 - ((cfg.extraAccounts != "" && cfg.extraAccounts != { }) - || accountsExtraAccounts != { }) { - text = joinCfg [ - header - (mkINI cfg.extraAccounts) - (mkINI accountsExtraAccounts) - ]; - }; - "aerc/aerc.conf" = - mkIf (cfg.extraConfig != "" && cfg.extraConfig != { }) { - text = joinCfg [ - header - (mkINI cfg.extraConfig) - (joinContextual accountsExtraConfig) - ]; - }; - "aerc/binds.conf" = mkIf ((cfg.extraBinds != "" && cfg.extraBinds != { }) - || accountsExtraBinds != { }) { - text = joinCfg [ - header - (mkINI cfg.extraBinds) - (joinContextual accountsExtraBinds) - ]; - }; + "aerc/accounts.conf" = mkIf genAccountsConf { + text = joinCfg [ + header + (mkINI cfg.extraAccounts) + (mkINI primaryAccountAccounts) + (mkINI accountsExtraAccounts) + ]; + }; + + "aerc/aerc.conf" = mkIf genAercConf { + text = joinCfg [ + header + (mkINI cfg.extraConfig) + (joinContextual accountsExtraConfig) + ]; + }; + + "aerc/binds.conf" = mkIf genBindsConf { + text = joinCfg [ + header + (mkINI cfg.extraBinds) + (joinContextual accountsExtraBinds) + ]; + }; } // (mkStyleset cfg.stylesets) // (mkTemplates cfg.templates); }; } diff --git a/tests/modules/programs/aerc/extraAccounts.expected b/tests/modules/programs/aerc/extraAccounts.expected index 9377384fc..a53e6e007 100644 --- a/tests/modules/programs/aerc/extraAccounts.expected +++ b/tests/modules/programs/aerc/extraAccounts.expected @@ -8,6 +8,10 @@ source = maildir:///dev/null [Test2] pgp-key-id = 42 +[primary] +from = Foo Bar +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 outgoing = msmtpq --read-envelope-from --read-recipients + +[p_overwrite_defaults] +from = test +outgoing = imap+plain://intentionallyWrong:PaSsWorD@smtp.host.invalid:1337 +postpone = dRaFts +source = smtp+plain://intentionallyWrong:PaSsWorD@smtp.host.invalid:1337 diff --git a/tests/modules/programs/aerc/settings.nix b/tests/modules/programs/aerc/settings.nix index 1f3c25a94..fa385655a 100644 --- a/tests/modules/programs/aerc/settings.nix +++ b/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 "; + outgoing = + "imap+plain://intentionallyWrong:PaSsWorD@smtp.host.invalid:1337"; + source = + "smtp+plain://intentionallyWrong:PaSsWorD@smtp.host.invalid:1337"; + postpone = "dRaFts"; + }; + }; + }; }; }; }