diff --git a/modules/nextcloud/client.ts b/modules/nextcloud/client.ts index e2310704..32ec8a9f 100644 --- a/modules/nextcloud/client.ts +++ b/modules/nextcloud/client.ts @@ -43,24 +43,6 @@ function ignoreSecretSetting(cfg: Record, 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 = [ - ["DocumentServerUrl", "onlyofficeUrl"], - ["DocumentServerInternalUrl", "onlyofficeInternalUrl"], - ["StorageUrl", "onlyofficeStorageUrl"], - ["jwt_secret", "onlyofficeSecret"], -]; - -let saidOnlyOfficeNotGiven = false; - export class NextcloudClient { constructor( private readonly container: string, @@ -104,56 +86,6 @@ 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"]); diff --git a/modules/nextcloud/files/config.json b/modules/nextcloud/files/config.json index fe032343..0967ef42 100644 --- a/modules/nextcloud/files/config.json +++ b/modules/nextcloud/files/config.json @@ -1,6 +1 @@ -{ - "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}" -} +{} diff --git a/modules/nextcloud/index.ts b/modules/nextcloud/index.ts index 6d3302dd..9fcf6d59 100644 --- a/modules/nextcloud/index.ts +++ b/modules/nextcloud/index.ts @@ -44,15 +44,6 @@ async function pollShares(client: NextcloudClient): Promise { 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 { - 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 => { @@ -62,6 +53,5 @@ if (nextcloud) { }; tick(pollUsers); tick(pollShares); - tick(applyOnlyOffice); - console.log("[nextcloud] watching users, shares and onlyoffice settings"); + console.log("[nextcloud] watching users and shares"); } diff --git a/modules/nextcloud/module.json b/modules/nextcloud/module.json index dd28417f..2854a160 100644 --- a/modules/nextcloud/module.json +++ b/modules/nextcloud/module.json @@ -5,8 +5,7 @@ "requires": [ "postgres-database", "s3-bucket", - "route", - "document-server" + "route" ], "contributes": { "postgres-database": { @@ -20,13 +19,11 @@ "binds": { "postgres-database": "${dir:state}/database.json", "s3-bucket": "${dir:state}/store.json", - "route": "${dir:state}/route.json", - "document-server": "${dir:state}/document-server.json" + "route": "${dir:state}/route.json" }, "secrets": { "postgres-database": "${dir:state}/database.secret", - "s3-bucket": "${dir:state}/store.secret", - "document-server": "${dir:state}/document-server.secret" + "s3-bucket": "${dir:state}/store.secret" }, "emits": [ "user.created", @@ -137,7 +134,6 @@ "mode": "0600", "template": "files/config.json", "takes-config": ["container", "url", "user"], - "protected": ["onlyofficeSecret"], "merge": "json" } ], diff --git a/modules/nextcloud/test/manifest.test.ts b/modules/nextcloud/test/manifest.test.ts deleted file mode 100644 index 55a2f60b..00000000 --- a/modules/nextcloud/test/manifest.test.ts +++ /dev/null @@ -1,35 +0,0 @@ -// 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}"); -}); diff --git a/modules/nextcloud/test/onlyoffice.test.ts b/modules/nextcloud/test/onlyoffice.test.ts deleted file mode 100644 index 09241597..00000000 --- a/modules/nextcloud/test/onlyoffice.test.ts +++ /dev/null @@ -1,142 +0,0 @@ -// 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 { - 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, 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 = { - 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 = { - 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 = {}; // 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(); - } -}); diff --git a/modules/only-office/manifest_test.go b/modules/only-office/manifest_test.go deleted file mode 100644 index cbaec2b4..00000000 --- a/modules/only-office/manifest_test.go +++ /dev/null @@ -1,63 +0,0 @@ -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) - } -} diff --git a/modules/only-office/module.json b/modules/only-office/module.json index 157ca954..03ed3cc0 100644 --- a/modules/only-office/module.json +++ b/modules/only-office/module.json @@ -2,22 +2,6 @@ "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" ], @@ -73,8 +57,7 @@ "own-secrets": { "jwt": { "path": "${dir:state}/jwt.secret", - "owner": "only-office-converter", - "taken": "at-start" + "owner": "only-office-converter" }, "broker-converter": { "path": "${dir:state}/broker-converter.secret",