Merge pull request 'Join nextcloud to only-office through a document-server provision, so OnlyOffice opens documents again (issue 1830)' (#438) from fix/1830-onlyoffice-link into main

modules/nextcloud/client.ts

@@ -43,6 +43,24 @@ function ignoreSecretSetting(cfg: Record<string, unknown>, key: string): void {
   console.error(`the setting ${key} is ignored: it comes from the vault`);
 }
 
+/** What applyOnlyOffice changed and whether it checked the connection (novox/hq issue 1830). */
+export interface OnlyOfficeResult {
+  /** The onlyoffice occ keys that were set this call — key names only, never a value. */
+  changed: string[];
+  /** Whether `onlyoffice:documentserver --check` ran, and passed; absent when nothing changed. */
+  checkOk?: boolean;
+}
+
+// occ's config key, and the mesh-delivered config.json key it comes from (files/config.json).
+const ONLYOFFICE_KEYS: ReadonlyArray<readonly [occKey: string, configKey: string]> = [
+  ["DocumentServerUrl", "onlyofficeUrl"],
+  ["DocumentServerInternalUrl", "onlyofficeInternalUrl"],
+  ["StorageUrl", "onlyofficeStorageUrl"],
+  ["jwt_secret", "onlyofficeSecret"],
+];
+
+let saidOnlyOfficeNotGiven = false;
+
 export class NextcloudClient {
   constructor(
     private readonly container: string,
@@ -86,6 +104,56 @@ export class NextcloudClient {
     }
   }
 
+  /**
+   * Applies OnlyOffice's settings from the mesh-delivered config (novox/hq issue 1830), read fresh
+   * from the config file on every call. Sets only the occ keys that differ from Nextcloud's current
+   * ones, then runs the connection check once something changed. When any of the four values is not
+   * given yet (document-server not bound, or its secret not given), changes nothing and says so once
+   * — never on every call. Never logs, returns or emits the secret's value: `changed` holds key names
+   * only, `jwt_secret` among them, never jwt_secret's value.
+   */
+  applyOnlyOffice(configFile: string | undefined = process.env.MESH_NEXTCLOUD_CONFIG_FILE): OnlyOfficeResult {
+    const cfg = meshConfig(configFile);
+    const desired = ONLYOFFICE_KEYS.map(([occKey, configKey]) => [occKey, cfg[configKey] ?? ""] as const);
+    if (desired.some(([, value]) => !value)) {
+      if (!saidOnlyOfficeNotGiven) {
+        saidOnlyOfficeNotGiven = true;
+        console.error("onlyoffice: document-server is not bound yet, or its secret is not given yet — nothing applied");
+      }
+      return { changed: [] };
+    }
+
+    const changed: string[] = [];
+    for (const [occKey, value] of desired) {
+      if (this.currentAppConfig(occKey) !== value) {
+        this.occ(["config:app:set", "onlyoffice", occKey, `--value=${value}`]);
+        changed.push(occKey);
+      }
+    }
+    if (changed.length === 0) return { changed };
+
+    let checkOk = true;
+    try {
+      this.occ(["onlyoffice:documentserver", "--check"]);
+    } catch {
+      checkOk = false;
+    }
+    return { changed, checkOk };
+  }
+
+  /**
+   * config:app:get of a key onlyoffice never set can throw occ's own "produced no output" error
+   * (occ above) rather than answer an empty string; read either as unset, so a first run sets it
+   * rather than aborting the loop.
+   */
+  private currentAppConfig(key: string): string {
+    try {
+      return this.occ(["config:app:get", "onlyoffice", key]);
+    } catch {
+      return "";
+    }
+  }
+
   listUsers(): NextcloudUser[] {
     // user:list --output=json answers an object of uid → display name.
     const raw = this.occ(["user:list", "--output=json"]);

modules/nextcloud/files/config.json

@@ -1 +1,6 @@
-{}
+{
+  "onlyofficeUrl": "${bound:document-server:public-root}",
+  "onlyofficeInternalUrl": "http://${bound:document-server:at}:${bound:document-server:port}/",
+  "onlyofficeStorageUrl": "https://${bound:route:name}/",
+  "onlyofficeSecret": "${secret:document-server}"
+}

modules/nextcloud/index.ts

@@ -44,6 +44,15 @@ async function pollShares(client: NextcloudClient): Promise<void> {
   sharesPrimed = true;
 }
 
+// Keeps Nextcloud's onlyoffice settings matched to the document server the mesh bound (novox/hq
+// issue 1830). The client never logs; this says what it did.
+async function applyOnlyOffice(client: NextcloudClient): Promise<void> {
+  const result = client.applyOnlyOffice();
+  if (result.changed.length === 0) return;
+  console.log(`[nextcloud] onlyoffice: set ${result.changed.join(", ")}`);
+  if (result.checkOk === false) console.error("[nextcloud] onlyoffice: connection check failed after applying settings");
+}
+
 if (nextcloud) {
   const client = nextcloud;
   const tick = (fn: (c: NextcloudClient) => Promise<void>): void => {
@@ -53,5 +62,6 @@ if (nextcloud) {
   };
   tick(pollUsers);
   tick(pollShares);
-  console.log("[nextcloud] watching users and shares");
+  tick(applyOnlyOffice);
+  console.log("[nextcloud] watching users, shares and onlyoffice settings");
 }

modules/nextcloud/module.json

@@ -5,7 +5,8 @@
   "requires": [
     "postgres-database",
     "s3-bucket",
-    "route"
+    "route",
+    "document-server"
   ],
   "contributes": {
     "postgres-database": {
@@ -19,11 +20,13 @@
   "binds": {
     "postgres-database": "${dir:state}/database.json",
     "s3-bucket": "${dir:state}/store.json",
-    "route": "${dir:state}/route.json"
+    "route": "${dir:state}/route.json",
+    "document-server": "${dir:state}/document-server.json"
   },
   "secrets": {
     "postgres-database": "${dir:state}/database.secret",
-    "s3-bucket": "${dir:state}/store.secret"
+    "s3-bucket": "${dir:state}/store.secret",
+    "document-server": "${dir:state}/document-server.secret"
   },
   "emits": [
     "user.created",
@@ -134,6 +137,7 @@
       "mode": "0600",
       "template": "files/config.json",
       "takes-config": ["container", "url", "user"],
+      "protected": ["onlyofficeSecret"],
       "merge": "json"
     }
   ],

modules/nextcloud/test/manifest.test.ts

@@ -0,0 +1,35 @@
+// 1830/AC2 (unit: manifest checks)
+// Given the catalogue, when nextcloud's manifest is parsed, then it requires document-server,
+// binds it and receives its secret, and its config.json template carries the document server's
+// public address, bound address and port, its own route name and the shared secret.
+// Run against the manifest and its templates directly — no build needed, they are read as data.
+
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { readFileSync } from "node:fs";
+import { fileURLToPath } from "node:url";
+import { dirname, join } from "node:path";
+
+const moduleDir = join(dirname(fileURLToPath(import.meta.url)), "..");
+
+function readJson(path: string): any {
+  return JSON.parse(readFileSync(join(moduleDir, path), "utf8"));
+}
+
+test("the manifest requires document-server, and binds and receives its secret", () => {
+  const manifest = readJson("module.json");
+  assert.ok(
+    Array.isArray(manifest.requires) && manifest.requires.includes("document-server"),
+    "requires names document-server",
+  );
+  assert.equal(manifest.binds?.["document-server"], "${dir:state}/document-server.json");
+  assert.equal(manifest.secrets?.["document-server"], "${dir:state}/document-server.secret");
+});
+
+test("config.json carries the document server's public address, bound address, port, route name and secret", () => {
+  const config = readJson("files/config.json");
+  assert.equal(config.onlyofficeUrl, "${bound:document-server:public-root}");
+  assert.equal(config.onlyofficeInternalUrl, "http://${bound:document-server:at}:${bound:document-server:port}/");
+  assert.equal(config.onlyofficeStorageUrl, "https://${bound:route:name}/");
+  assert.equal(config.onlyofficeSecret, "${secret:document-server}");
+});

modules/nextcloud/test/onlyoffice.test.ts

@@ -0,0 +1,142 @@
+// novox/hq issue 1830: nextcloud reaches only-office's document server by its mesh-delivered
+// config, not by hand-set settings, and the loop that applies it never leaks the shared secret.
+// Run against the compiled module, the way the runtime loads it.
+
+import { test, mock } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+
+import { NextcloudClient } from "../dist/client.js";
+
+const SECRET = "the-shared-jwt-secret";
+
+const FULL_CONFIG = {
+  onlyofficeUrl: "https://office.example/",
+  onlyofficeInternalUrl: "http://10.10.0.1:9070/",
+  onlyofficeStorageUrl: "https://drive.example/",
+  onlyofficeSecret: SECRET,
+};
+
+function configFile(values: Record<string, string>): string {
+  const dir = mkdtempSync(join(tmpdir(), "nextcloud-1830-"));
+  const path = join(dir, "config.json");
+  writeFileSync(path, JSON.stringify(values));
+  return path;
+}
+
+function client(): any {
+  return new (NextcloudClient as any)("nextcloud", "http://127.0.0.1", "admin", "irrelevant");
+}
+
+/** A stand-in occ: config:app:get answers `current`, or throws like the real occ when unset. */
+function standInOcc(current: Record<string, string>, opts: { checkFails?: boolean } = {}) {
+  const calls: string[][] = [];
+  const impl = (args: string[]): string => {
+    calls.push(args);
+    if (args[0] === "config:app:get") {
+      const key = args[2];
+      if (!(key in current)) throw new Error("occ produced no output");
+      return current[key];
+    }
+    if (args[0] === "config:app:set") {
+      const key = args[2];
+      const value = args[3].slice("--value=".length);
+      current[key] = value;
+      return "System config value onlyoffice => " + key + " set to string " + value;
+    }
+    if (args[0] === "onlyoffice:documentserver") {
+      if (opts.checkFails) throw new Error("occ produced no output");
+      return "";
+    }
+    throw new Error("unexpected occ call: " + args.join(" "));
+  };
+  return { calls, impl };
+}
+
+// 1830/AC3a: when the delivered values differ from Nextcloud's settings, it sets exactly the
+// differing ones and runs OnlyOffice's connection check.
+test("sets exactly the keys that differ, and runs the connection check", () => {
+  const c = client();
+  const current: Record<string, string> = {
+    DocumentServerUrl: "https://old-office.example/",
+    StorageUrl: FULL_CONFIG.onlyofficeStorageUrl, // already correct
+    jwt_secret: "a-stale-secret",
+    // DocumentServerInternalUrl absent: occ's config:app:get throws, as for a never-set key.
+  };
+  const stand = standInOcc(current);
+  mock.method(c, "occ", stand.impl);
+
+  const result = c.applyOnlyOffice(configFile(FULL_CONFIG));
+
+  assert.deepEqual(result.changed, ["DocumentServerUrl", "DocumentServerInternalUrl", "jwt_secret"]);
+  assert.equal(result.checkOk, true);
+  assert.ok(stand.calls.some((a) => a[0] === "onlyoffice:documentserver" && a[1] === "--check"),
+    "the connection check ran");
+  assert.equal(current.StorageUrl, FULL_CONFIG.onlyofficeStorageUrl, "the matching key was left alone");
+});
+
+// 1830/AC3b: when they match, it changes nothing and runs no check.
+test("changes nothing and runs no check when every value already matches", () => {
+  const c = client();
+  const current: Record<string, string> = {
+    DocumentServerUrl: FULL_CONFIG.onlyofficeUrl,
+    DocumentServerInternalUrl: FULL_CONFIG.onlyofficeInternalUrl,
+    StorageUrl: FULL_CONFIG.onlyofficeStorageUrl,
+    jwt_secret: FULL_CONFIG.onlyofficeSecret,
+  };
+  const stand = standInOcc(current);
+  mock.method(c, "occ", stand.impl);
+
+  const result = c.applyOnlyOffice(configFile(FULL_CONFIG));
+
+  assert.deepEqual(result.changed, []);
+  assert.equal(result.checkOk, undefined);
+  assert.ok(!stand.calls.some((a) => a[0] === "config:app:set"), "nothing was set");
+  assert.ok(!stand.calls.some((a) => a[0] === "onlyoffice:documentserver"), "no check ran");
+});
+
+// 1830/AC3c: when a value is empty (not bound or not given yet), it changes nothing, and says so
+// once rather than on every call.
+test("changes nothing when a value is not given yet, and says so only once", () => {
+  const c = client();
+  const stand = standInOcc({});
+  mock.method(c, "occ", stand.impl);
+  const err = mock.method(console, "error", () => {});
+  try {
+    const partial = { ...FULL_CONFIG, onlyofficeSecret: "" };
+    const file = configFile(partial);
+
+    const first = c.applyOnlyOffice(file);
+    const second = c.applyOnlyOffice(file);
+
+    assert.deepEqual(first.changed, []);
+    assert.deepEqual(second.changed, []);
+    assert.equal(stand.calls.length, 0, "occ was never called");
+    assert.equal(err.mock.callCount(), 1, "said so once, not on every call");
+  } finally {
+    err.mock.restore();
+  }
+});
+
+// 1830/AC3d: the secret appears in no log line and no return value.
+test("the secret's value never appears in what is logged or returned", () => {
+  const c = client();
+  const current: Record<string, string> = {}; // every key unset, so jwt_secret is among those changed
+  const stand = standInOcc(current);
+  mock.method(c, "occ", stand.impl);
+  const log = mock.method(console, "log", () => {});
+  const err = mock.method(console, "error", () => {});
+  try {
+    const result = c.applyOnlyOffice(configFile(FULL_CONFIG));
+
+    assert.ok(result.changed.includes("jwt_secret"), "the key name is returned");
+    assert.ok(!result.changed.some((k: string) => k.includes(SECRET)), "never the value, in changed");
+    const said = [...log.mock.calls, ...err.mock.calls].map((c) => c.arguments.join(" ")).join("\n");
+    assert.ok(!said.includes(SECRET), "never the value, in a log line");
+  } finally {
+    log.mock.restore();
+    err.mock.restore();
+  }
+});

modules/only-office/manifest_test.go

@@ -0,0 +1,63 @@
+package office
+
+import (
+	"os"
+	"testing"
+
+	"git.novox.be/novox/mesh-sdk/go/catalogue"
+)
+
+// 1830/AC1 (unit: manifest check)
+// Given the catalogue, when only-office's manifest is parsed, then it offers document-server with
+// credential own jwt, jwt says taken at-start, and serves document-server on port 9070 with a
+// public-root.
+func TestManifestOffersDocumentServer(t *testing.T) {
+	raw, err := os.ReadFile("module.json")
+	if err != nil {
+		t.Fatalf("reading module.json: %v", err)
+	}
+
+	m, err := catalogue.ParseManifest(raw)
+	if err != nil {
+		t.Fatalf("parsing module.json: %v", err)
+	}
+
+	var offer *catalogue.Offer
+	for i := range m.Provides {
+		if m.Provides[i].Name == "document-server" {
+			offer = &m.Provides[i]
+			break
+		}
+	}
+	if offer == nil {
+		t.Fatal("manifest does not provide document-server")
+	}
+	if offer.Scope != catalogue.ScopeMesh {
+		t.Errorf("document-server scope = %q, want %q", offer.Scope, catalogue.ScopeMesh)
+	}
+	if offer.Credential == nil || offer.Credential.Own != "jwt" {
+		t.Errorf("document-server credential = %+v, want own jwt", offer.Credential)
+	}
+
+	own, ok := m.OwnSecrets["jwt"]
+	if !ok {
+		t.Fatal("manifest declares no own-secret jwt")
+	}
+	if own.Taken != catalogue.TakenAtStart {
+		t.Errorf("own-secrets.jwt.taken = %q, want %q", own.Taken, catalogue.TakenAtStart)
+	}
+
+	serve, ok := m.Serves["document-server"]
+	if !ok {
+		t.Fatal("manifest serves no document-server")
+	}
+	if scheme, _ := serve["scheme"].(string); scheme != "http" {
+		t.Errorf("serves.document-server.scheme = %q, want http", scheme)
+	}
+	if port, _ := serve["port"].(float64); port != 9070 {
+		t.Errorf("serves.document-server.port = %v, want 9070", serve["port"])
+	}
+	if root, _ := serve["public-root"].(string); root != "https://${bound:route:name}/" {
+		t.Errorf("serves.document-server.public-root = %q, want https://${bound:route:name}/", root)
+	}
+}

modules/only-office/module.json

@@ -2,6 +2,22 @@
   "module": "only-office",
   "version": "1",
   "slug": "office",
+  "provides": [
+    {
+      "name": "document-server",
+      "scope": "mesh",
+      "credential": {
+        "own": "jwt"
+      }
+    }
+  ],
+  "serves": {
+    "document-server": {
+      "scheme": "http",
+      "port": 9070,
+      "public-root": "https://${bound:route:name}/"
+    }
+  },
   "capabilities": [
     "container-runtime"
   ],
@@ -57,7 +73,8 @@
   "own-secrets": {
     "jwt": {
       "path": "${dir:state}/jwt.secret",
-      "owner": "only-office-converter"
+      "owner": "only-office-converter",
+      "taken": "at-start"
     },
     "broker-converter": {
       "path": "${dir:state}/broker-converter.secret",