nextcloud: repair only the module's own account, read its existence from the user list, no admin group (issue 1253, review 1)

occ's user:info says 'user not found' on stdout with an empty stderr, so the
error read as 'occ produced no output' and mesh-admin was never made on novox:
occ now says stderr and stdout both, and existence is read from user:list.
The repair could reset whatever account the user setting names, the
operator's included, and the 'admin' fallback is a hand-made install's human
administrator: only mesh-admin is repaired now, and it is the default. The
shares poll needs no admin rights, so the account gets no group. After a
repair the throttle of the caller's address is reset, and a failing repair
reaches the back-off's one loud line with occ's message.

modules/nextcloud/client.ts

@@ -61,6 +61,13 @@ const ONLYOFFICE_KEYS: ReadonlyArray<readonly [occKey: string, configKey: string
 
 let saidOnlyOfficeNotGiven = false;
 
+/**
+ * The one Nextcloud account the module makes and keeps for itself: the same name files/server.env
+ * gives NEXTCLOUD_ADMIN_USER (novox/hq issue 1253). Only this account is ever repaired; a `user`
+ * setting naming any other account is used as given and never written to.
+ */
+export const MESH_ACCOUNT = "mesh-admin";
+
 /**
  * Nextcloud refused the mesh's own login on its OCS surface (HTTP 401), or throttled it after
  * refusals (HTTP 429, its brute-force protection). Said by its status and the account, never a
@@ -133,7 +140,7 @@ export class NextcloudClient {
   /**
    * Build from the module's resolved environment. occ needs only the container name (default
    * "nextcloud"); the OCS surface needs the admin password the module keeps as its own secret
-   * (MESH_NEXTCLOUD_ADMIN_PASSWORD, user MESH_NEXTCLOUD_ADMIN_USER default admin, URL the local
+   * (MESH_NEXTCLOUD_ADMIN_PASSWORD, user MESH_NEXTCLOUD_ADMIN_USER default mesh-admin, URL the local
    * container). The admin password is treated as the "configured for mesh administration" signal:
    * throws without it, and the module then contributes nothing rather than failing.
    */
@@ -142,7 +149,8 @@ export class NextcloudClient {
     // An empty setting is the manifest's "empty is the default", never an empty name.
     const container = cfg.container || env.MESH_NEXTCLOUD_CONTAINER || "nextcloud";
     const ocsUrl = cfg.url || env.MESH_NEXTCLOUD_URL || `http://127.0.0.1:${env.NEXTCLOUD_PORT ?? "80"}`;
-    const adminUser = cfg.user || env.MESH_NEXTCLOUD_ADMIN_USER || "admin";
+    // No "admin" fallback: that is what a hand-made install calls its human administrator.
+    const adminUser = cfg.user || env.MESH_NEXTCLOUD_ADMIN_USER || MESH_ACCOUNT;
     const passwordFile = env.MESH_NEXTCLOUD_ADMIN_PASSWORD_FILE;
     ignoreSecretSetting(cfg, "password");
     const adminPassword = env.MESH_NEXTCLOUD_ADMIN_PASSWORD
@@ -174,7 +182,9 @@ export class NextcloudClient {
         ...(opts.env ? { env: { ...process.env, ...opts.env } } : {}),
       }).trim();
     } catch (err: any) {
-      const detail = String(err?.stderr ?? err?.stdout ?? err?.message ?? "").trim();
+      // occ writes many refusals ("user not found") to stdout with an empty stderr: say both.
+      const detail = [err?.stderr, err?.stdout].map((x) => String(x ?? "").trim()).filter(Boolean).join("\n")
+        || String(err?.message ?? "").trim();
       throw new Error(detail || `occ produced no output — is the ${this.container} container running?`);
     }
   }
@@ -253,31 +263,51 @@ export class NextcloudClient {
   /**
    * Makes the mesh's own account in Nextcloud match the module's own secret (novox/hq issue 1253).
    * The image creates NEXTCLOUD_ADMIN_USER only when it installs a fresh instance, so an instance
-   * the mesh adopted never has it, and a secret made anew never reaches it. Through occ: the
-   * account is added in the admin group when missing; otherwise its password is set to the secret
-   * and it is put in the admin group. The password goes by OC_PASS (`--password-from-env`), never
-   * on an argv. Answers what it did; throws occ's own message when occ fails.
+   * the mesh adopted never has it, and a secret made anew never reaches it. Only the module's own
+   * account (MESH_ACCOUNT) is touched: under any other name it writes nothing and throws
+   * LoginRefusedError. Through occ: whether the account exists is read from user:list's
+   * membership; a missing one is added, an existing one gets its password set to the secret. It
+   * gets no admin group: the shares poll needs no admin rights (least privilege). The password
+   * goes by OC_PASS (`--password-from-env`), never on an argv. Then the brute-force record of the
+   * addresses the module's calls arrive from is reset, so the first good login is not held behind
+   * the throttle. A failing occ write is thrown as LoginRefusedError carrying occ's own message.
    */
-  ensureAdminAccount(): "created" | "password-set" {
+  ensureAdminAccount(): { account: "created" | "password-set"; throttleReset: boolean } {
+    if (this.adminUser !== MESH_ACCOUNT) {
+      throw new LoginRefusedError(401, this.adminUser, `not repaired: only ${MESH_ACCOUNT}, the module's own account, is`);
+    }
     const pass = { env: { OC_PASS: this.adminPassword } };
-    let exists = true;
+    let account: "created" | "password-set";
     try {
-      this.occ(["user:info", this.adminUser, "--output=json"]);
+      if (!this.listUsers().some((u) => u.uid === this.adminUser)) {
+        this.occ(["user:add", "--password-from-env", "--display-name=Mesh administration", this.adminUser], pass);
+        account = "created";
+      } else {
+        this.occ(["user:resetpassword", "--password-from-env", this.adminUser], pass);
+        account = "password-set";
+      }
     } catch (err) {
-      if (!/not found|does not exist/i.test(String((err as Error)?.message ?? err))) throw err;
-      exists = false;
+      throw new LoginRefusedError(401, this.adminUser, `repair failed: ${String((err as Error)?.message ?? err).split("\n")[0]}`);
     }
-    if (!exists) {
-      this.occ(["user:add", "--password-from-env", "--display-name=Mesh administration", "--group=admin", this.adminUser], pass);
-      return "created";
-    }
-    this.occ(["user:resetpassword", "--password-from-env", this.adminUser], pass);
+    let throttleReset = true;
     try {
-      this.occ(["group:adduser", "admin", this.adminUser]);
-    } catch (err) {
-      if (!/already/i.test(String((err as Error)?.message ?? err))) throw err;
+      for (const address of this.callerAddresses()) this.occ(["security:bruteforce:reset", address]);
+    } catch {
+      throttleReset = false;
     }
-    return "password-set";
+    return { account, throttleReset };
+  }
+
+  /**
+   * The addresses the module's calls reach Nextcloud from, as Nextcloud sees them: the gateway of
+   * each network the container is on (a published port arrives from it; the access log shows
+   * 172.17.0.1 on the default bridge).
+   */
+  callerAddresses(): string[] {
+    const out = execFileSync("docker", ["inspect", "-f", "{{range .NetworkSettings.Networks}}{{.Gateway}} {{end}}", this.container], {
+      encoding: "utf8", timeout: 30_000,
+    });
+    return out.split(/\s+/).filter(Boolean);
   }
 
   private fetchShares(): Promise<Response> {

modules/nextcloud/module.json

@@ -72,7 +72,7 @@
     },
     "user": {
       "type": "string",
-      "description": "The Nextcloud account the module's tools act as; empty is admin.",
+      "description": "The Nextcloud account the module's tools act as; empty is mesh-admin, the account the module makes and keeps for itself. Any other account is used as given and never repaired.",
       "trusted": true,
       "default": ""
     }

modules/nextcloud/test/admin-login.test.ts

@@ -15,8 +15,11 @@ const PASSWORD = "the-modules-own-secret";
 const REFUSED_XML = `<?xml version="1.0"?>\n<ocs>\n <meta>\n  <status>failure</status>\n  <statuscode>997</statuscode>\n  <message>Unauthorised</message>\n </meta>\n <data/>\n</ocs>\n`;
 const SHARES = { ocs: { data: [{ id: 7, path: "/Photos", share_type: 3, share_with: "", uid_owner: "jochens" }] } };
 
-function client(): any {
-  return new (NextcloudClient as any)("nextcloud", "http://127.0.0.1:20003", "mesh-admin", PASSWORD);
+function client(user = "mesh-admin"): any {
+  const c = new (NextcloudClient as any)("nextcloud", "http://127.0.0.1:20003", user, PASSWORD);
+  // The address Nextcloud sees the module's calls arrive from (docker inspect in the real one).
+  mock.method(c, "callerAddresses", () => ["172.17.0.1"]);
+  return c;
 }
 
 /** A stand-in Nextcloud: answers `statuses` in turn (the last one repeating) and counts the asks. */
@@ -32,25 +35,31 @@ function standInFetch(statuses: number[]) {
 }
 
 /** A stand-in occ: knows whether the account exists, records every call and every env passed. */
-function standInOcc(accountExists: boolean) {
+function standInOcc(accountExists: boolean, opts: { failWrite?: string } = {}) {
   const calls: string[][] = [];
   const envs: Array<Record<string, string> | undefined> = [];
-  const impl = (args: string[], opts: { env?: Record<string, string> } = {}): string => {
+  const impl = (args: string[], occOpts: { env?: Record<string, string> } = {}): string => {
     calls.push(args);
-    envs.push(opts.env);
-    if (args[0] === "user:info") {
-      if (!accountExists) throw new Error("user not found");
-      return "{}";
+    envs.push(occOpts.env);
+    if (args[0] === "user:list") {
+      return JSON.stringify(accountExists ? { jochens: "Jochen", "mesh-admin": "Mesh administration" } : { jochens: "Jochen" });
     }
-    if (["user:add", "user:resetpassword", "group:adduser"].includes(args[0])) return "";
+    if (["user:add", "user:resetpassword"].includes(args[0])) {
+      if (opts.failWrite) throw new Error(opts.failWrite);
+      return "";
+    }
+    if (args[0] === "security:bruteforce:reset") return "";
     throw new Error("unexpected occ call: " + args.join(" "));
   };
   return { calls, envs, impl };
 }
 
+/** The occ calls that write: anything but a read of the user list. */
+const writes = (calls: string[][]) => calls.filter((a) => a[0] !== "user:list");
+
 // 1253/AC1: given an adopted instance without the mesh's account, when Nextcloud refuses the
-// login, then the account is added in the admin group with the module's own secret, by
-// OC_PASS and never on an argv, and the shares are asked again and listed.
+// login, then the account is added (no admin group) with the module's own secret, by OC_PASS and
+// never on an argv, the throttle of the caller's address is reset, and the shares are listed.
 test("a refused login adds the missing account, password by environment, and lists the shares", async () => {
   const c = client();
   const occ = standInOcc(false);
@@ -63,7 +72,9 @@ test("a refused login adds the missing account, password by environment, and lis
     assert.deepEqual(shares.map((s: any) => s.id), ["7"]);
     const add = occ.calls.find((a) => a[0] === "user:add");
     assert.ok(add, "user:add ran");
-    assert.ok(add.includes("--password-from-env") && add.includes("--group=admin") && add.includes("mesh-admin"));
+    assert.ok(add.includes("--password-from-env") && add.includes("mesh-admin"));
+    assert.ok(!add.some((a) => a.includes("group")), "no admin group: least privilege");
+    assert.deepEqual(occ.calls.at(-1), ["security:bruteforce:reset", "172.17.0.1"]);
     assert.equal(occ.envs[occ.calls.indexOf(add)]?.OC_PASS, PASSWORD);
     assert.ok(!occ.calls.flat().some((a) => a.includes(PASSWORD)), "the password is on no argv");
   } finally {
@@ -72,15 +83,15 @@ test("a refused login adds the missing account, password by environment, and lis
 });
 
 // 1253/AC2: given the account exists with another password, when Nextcloud refuses the login,
-// then its password is set to the module's own secret and it is put in the admin group.
-test("a refused login resets an existing account's password and puts it in the admin group", async () => {
+// then its password is set to the module's own secret, in no group, and the throttle is reset.
+test("a refused login resets an existing account's password, in no group", async () => {
   const c = client();
   const occ = standInOcc(true);
   mock.method(c, "occ", occ.impl);
   const f = mock.method(globalThis, "fetch", standInFetch([401, 200]).impl);
   try {
     await c.listShares();
-    assert.deepEqual(occ.calls.map((a) => a[0]), ["user:info", "user:resetpassword", "group:adduser"]);
+    assert.deepEqual(occ.calls.map((a) => a[0]), ["user:list", "user:resetpassword", "security:bruteforce:reset"]);
     assert.equal(occ.envs[1]?.OC_PASS, PASSWORD);
     assert.ok(!occ.calls.flat().some((a) => a.includes(PASSWORD)));
   } finally {
@@ -205,3 +216,78 @@ test("an empty user setting falls back to the manifest's account", () => {
   assert.equal(c.container, "nextcloud");
   assert.equal(c.ocsUrl, "http://127.0.0.1:20003");
 });
+
+// 1253/AC7: the repair touches only the module's own account: a refusal under a user setting
+// naming another account (the operator's) runs no occ write and throws LoginRefusedError.
+test("a refusal under another account's name writes nothing", async () => {
+  const c = client("jochens");
+  const occ = standInOcc(true);
+  mock.method(c, "occ", occ.impl);
+  const web = standInFetch([401]);
+  const f = mock.method(globalThis, "fetch", web.impl);
+  try {
+    await assert.rejects(c.listShares(), (err: any) => {
+      assert.ok(err instanceof LoginRefusedError);
+      assert.match(err.message, /refused the login of jochens .*not repaired: only mesh-admin/);
+      return true;
+    });
+    assert.deepEqual(writes(occ.calls), []);
+  } finally {
+    f.mock.restore();
+  }
+});
+
+// 1253/AC8: a repair whose occ write fails is said as a LoginRefusedError carrying occ's own
+// message, so the back-off's one loud line says why.
+test("a failing repair carries occ's message into LoginRefusedError", async () => {
+  const c = client();
+  mock.method(c, "occ", standInOcc(false, { failWrite: "The password policy refused the password" }).impl);
+  const f = mock.method(globalThis, "fetch", standInFetch([401]).impl);
+  try {
+    await assert.rejects(c.listShares(), (err: any) =>
+      err instanceof LoginRefusedError && /repair failed: The password policy refused the password/.test(err.message));
+  } finally {
+    f.mock.restore();
+  }
+});
+
+/** A fake docker on PATH: answers occ like Nextcloud (user:info's refusal on stdout, exit 1). */
+function fakeDocker(users: Record<string, string>): { dir: string; record: string } {
+  const dir = mkdtempSync(join(tmpdir(), "nextcloud-1253-docker-"));
+  const record = join(dir, "record");
+  writeFileSync(join(dir, "docker"), [
+    "#!/bin/sh",
+    `printf '%s\\n' "$*" >> ${record}`,
+    'case "$*" in',
+    "  *' occ user:info '*) echo 'user not found'; exit 1 ;;",
+    `  *' occ user:list '*) printf '%s' '${JSON.stringify(users)}' ;;`,
+    "  inspect*) echo '172.17.0.1 ' ;;",
+    "esac",
+    "exit 0",
+    "",
+  ].join("\n"));
+  chmodSync(join(dir, "docker"), 0o755);
+  return { dir, record };
+}
+
+function withPath<T>(dir: string, fn: () => T): T {
+  const path = process.env.PATH;
+  process.env.PATH = `${dir}:${path}`;
+  try { return fn(); } finally { process.env.PATH = path; }
+}
+
+// 1253/AC9: occ's own refusal on stdout with an empty stderr reaches the error, and the real
+// occ() through a docker answering like Nextcloud makes the missing account.
+test("occ says a refusal printed on stdout, and the real occ makes the missing account", () => {
+  const docker = fakeDocker({ jochens: "Jochen" });
+  const c = new (NextcloudClient as any)("nextcloud", "http://127.0.0.1:20003", "mesh-admin", PASSWORD);
+  withPath(docker.dir, () => {
+    assert.throws(() => c.occ(["user:info", "mesh-admin", "--output=json"]), /user not found/);
+    const did = c.ensureAdminAccount();
+    assert.deepEqual(did, { account: "created", throttleReset: true });
+  });
+  const lines = readFileSync(docker.record, "utf8").trim().split("\n");
+  assert.ok(lines.some((l) => l.endsWith("php occ user:add --password-from-env --display-name=Mesh administration mesh-admin")), lines.join("\n"));
+  assert.ok(lines.some((l) => l.endsWith("php occ security:bruteforce:reset 172.17.0.1")), lines.join("\n"));
+  assert.ok(!lines.join("\n").includes(PASSWORD));
+});