Merge pull request #140775 from Ma27/nextcloud-docs-and-secrets

nixos/nextcloud: manual improvements & pass secrets via the environment to `nextcloud-occ`

3 files changed, 39 insertions, 55 deletions

diff --git a/nixos/modules/services/web-apps/nextcloud.nix b/nixos/modules/services/web-apps/nextcloud.nix
index 886e030b80c..4d7f16b1e1a 100644
--- a/nixos/modules/services/web-apps/nextcloud.nix
+++ b/nixos/modules/services/web-apps/nextcloud.nix
@@ -51,6 +51,12 @@ let
 in {
 
   imports = [
+    (mkRemovedOptionModule [ "services" "nextcloud" "config" "adminpass" ] ''
+      Please use `services.nextcloud.config.adminpassFile' instead!
+    '')
+    (mkRemovedOptionModule [ "services" "nextcloud" "config" "dbpass" ] ''
+      Please use `services.nextcloud.config.dbpassFile' instead!
+    '')
     (mkRemovedOptionModule [ "services" "nextcloud" "nginx" "enable" ] ''
       The nextcloud module supports `nginx` as reverse-proxy by default and doesn't
       support other reverse-proxies officially.
@@ -206,14 +212,6 @@ in {
         default = "nextcloud";
         description = "Database user.";
       };
-      dbpass = mkOption {
-        type = types.nullOr types.str;
-        default = null;
-        description = ''
-          Database password.  Use <literal>dbpassFile</literal> to avoid this
-          being world-readable in the <literal>/nix/store</literal>.
-        '';
-      };
       dbpassFile = mkOption {
         type = types.nullOr types.str;
         default = null;
@@ -246,17 +244,8 @@ in {
         default = "root";
         description = "Admin username.";
       };
-      adminpass = mkOption {
-        type = types.nullOr types.str;
-        default = null;
-        description = ''
-          Admin password.  Use <literal>adminpassFile</literal> to avoid this
-          being world-readable in the <literal>/nix/store</literal>.
-        '';
-      };
       adminpassFile = mkOption {
-        type = types.nullOr types.str;
-        default = null;
+        type = types.str;
         description = ''
           The full path to a file that contains the admin's password. Must be
           readable by user <literal>nextcloud</literal>.
@@ -321,8 +310,8 @@ in {
             This mounts a bucket on an Amazon S3 object storage or compatible
             implementation into the virtual filesystem.
 
-            See nextcloud's documentation on "Object Storage as Primary
-            Storage" for more details.
+            Further details about this feature can be found in the
+            <link xlink:href="https://docs.nextcloud.com/server/22/admin_manual/configuration_files/primary_storage.html">upstream documentation</link>.
           '';
           bucket = mkOption {
             type = types.str;
@@ -389,9 +378,9 @@ in {
               Required for some non-Amazon S3 implementations.
 
               Ordinarily, requests will be made with
-              http://bucket.hostname.domain/, but with path style
+              <literal>http://bucket.hostname.domain/</literal>, but with path style
               enabled requests are made with
-              http://hostname.domain/bucket instead.
+              <literal>http://hostname.domain/bucket</literal> instead.
             '';
           };
         };
@@ -399,11 +388,11 @@ in {
     };
 
     enableImagemagick = mkEnableOption ''
-        Whether to load the ImageMagick module into PHP.
+        the ImageMagick module for PHP.
         This is used by the theming app and for generating previews of certain images (e.g. SVG and HEIF).
         You may want to disable it for increased security. In that case, previews will still be available
         for some images (e.g. JPEG and PNG).
-        See https://github.com/nextcloud/server/issues/13099
+        See <link xlink:href="https://github.com/nextcloud/server/issues/13099" />.
     '' // {
       default = true;
     };
@@ -464,13 +453,6 @@ in {
 
   config = mkIf cfg.enable (mkMerge [
     { assertions = let acfg = cfg.config; in [
-        { assertion = !(acfg.dbpass != null && acfg.dbpassFile != null);
-          message = "Please specify no more than one of dbpass or dbpassFile";
-        }
-        { assertion = ((acfg.adminpass != null || acfg.adminpassFile != null)
-            && !(acfg.adminpass != null && acfg.adminpassFile != null));
-          message = "Please specify exactly one of adminpass or adminpassFile";
-        }
         { assertion = versionOlder cfg.package.version "21" -> cfg.config.defaultPhoneRegion == null;
           message = "The `defaultPhoneRegion'-setting is only supported for Nextcloud >=21!";
         }
@@ -613,7 +595,6 @@ in {
               ${optionalString (c.dbport != null) "'dbport' => '${toString c.dbport}',"}
               ${optionalString (c.dbuser != null) "'dbuser' => '${c.dbuser}',"}
               ${optionalString (c.dbtableprefix != null) "'dbtableprefix' => '${toString c.dbtableprefix}',"}
-              ${optionalString (c.dbpass != null) "'dbpassword' => '${c.dbpass}',"}
               ${optionalString (c.dbpassFile != null) "'dbpassword' => nix_read_secret('${c.dbpassFile}'),"}
               'dbtype' => '${c.dbtype}',
               'trusted_domains' => ${writePhpArrary ([ cfg.hostName ] ++ c.extraTrustedDomains)},
@@ -623,14 +604,17 @@ in {
             ];
           '';
           occInstallCmd = let
-            dbpass = if c.dbpassFile != null
-              then ''"$(<"${toString c.dbpassFile}")"''
-              else if c.dbpass != null
-              then ''"${toString c.dbpass}"''
-              else ''""'';
-            adminpass = if c.adminpassFile != null
-              then ''"$(<"${toString c.adminpassFile}")"''
-              else ''"${toString c.adminpass}"'';
+            mkExport = { arg, value }: "export ${arg}=${value}";
+            dbpass = {
+              arg = "DBPASS";
+              value = if c.dbpassFile != null
+                then ''"$(<"${toString c.dbpassFile}")"''
+                else ''""'';
+            };
+            adminpass = {
+              arg = "ADMINPASS";
+              value = ''"$(<"${toString c.adminpassFile}")"'';
+            };
             installFlags = concatStringsSep " \\\n    "
               (mapAttrsToList (k: v: "${k} ${toString v}") {
               "--database" = ''"${c.dbtype}"'';
@@ -641,12 +625,14 @@ in {
               ${if c.dbhost != null then "--database-host" else null} = ''"${c.dbhost}"'';
               ${if c.dbport != null then "--database-port" else null} = ''"${toString c.dbport}"'';
               ${if c.dbuser != null then "--database-user" else null} = ''"${c.dbuser}"'';
-              "--database-pass" = dbpass;
+              "--database-pass" = "\$${dbpass.arg}";
               "--admin-user" = ''"${c.adminuser}"'';
-              "--admin-pass" = adminpass;
+              "--admin-pass" = "\$${adminpass.arg}";
               "--data-dir" = ''"${cfg.home}/data"'';
             });
           in ''
+            ${mkExport dbpass}
+            ${mkExport adminpass}
             ${occ}/bin/nextcloud-occ maintenance:install \
                 ${installFlags}
           '';
@@ -673,16 +659,14 @@ in {
                 exit 1
               fi
             ''}
-            ${optionalString (c.adminpassFile != null) ''
-              if [ ! -r "${c.adminpassFile}" ]; then
-                echo "adminpassFile ${c.adminpassFile} is not readable by nextcloud:nextcloud! Aborting..."
-                exit 1
-              fi
-              if [ -z "$(<${c.adminpassFile})" ]; then
-                echo "adminpassFile ${c.adminpassFile} is empty!"
-                exit 1
-              fi
-            ''}
+            if [ ! -r "${c.adminpassFile}" ]; then
+              echo "adminpassFile ${c.adminpassFile} is not readable by nextcloud:nextcloud! Aborting..."
+              exit 1
+            fi
+            if [ -z "$(<${c.adminpassFile})" ]; then
+              echo "adminpassFile ${c.adminpassFile} is empty!"
+              exit 1
+            fi
 
             ln -sf ${cfg.package}/apps ${cfg.home}/
 
diff --git a/nixos/tests/nextcloud/basic.nix b/nixos/tests/nextcloud/basic.nix
index 40ee9d5184c..1a7b25d5a49 100644
--- a/nixos/tests/nextcloud/basic.nix
+++ b/nixos/tests/nextcloud/basic.nix
@@ -38,7 +38,7 @@ in {
         hostName = "nextcloud";
         config = {
           # Don't inherit adminuser since "root" is supposed to be the default
-          inherit adminpass;
+          adminpassFile = "${pkgs.writeText "adminpass" adminpass}"; # Don't try this at home!
           dbtableprefix = "nixos_";
         };
         package = pkgs.${"nextcloud" + (toString nextcloudVersion)};
diff --git a/nixos/tests/nextcloud/with-mysql-and-memcached.nix b/nixos/tests/nextcloud/with-mysql-and-memcached.nix
index c0df773eaaa..80cb63df5db 100644
--- a/nixos/tests/nextcloud/with-mysql-and-memcached.nix
+++ b/nixos/tests/nextcloud/with-mysql-and-memcached.nix
@@ -32,9 +32,9 @@ in {
           dbuser = "nextcloud";
           dbhost = "127.0.0.1";
           dbport = 3306;
-          dbpass = "hunter2";
+          dbpassFile = "${pkgs.writeText "dbpass" "hunter2" }";
           # Don't inherit adminuser since "root" is supposed to be the default
-          inherit adminpass;
+          adminpassFile = "${pkgs.writeText "adminpass" adminpass}"; # Don't try this at home!
         };
       };