From 9bcdfc20490364060cbe1fdf9bb2cc0f4cb351ab Mon Sep 17 00:00:00 2001 From: hoelee Date: Sun, 23 Aug 2026 06:43:01 +0800 Subject: [PATCH] fix(W1): self-write-back suppression via old/new field comparison - Replace brittle sync_status==='synced' loop guard with NocoDB v2 old/new row comparison (previous_rows vs rows). - businessChanged() = business field move OR sync_required set; echo (bookkeeping-only delta) -> skip_writeback_echo, zero AList mutation. - writeBack() value-change guard: GET live record, PATCH only fields whose value actually changes (no-change -> no PATCH -> no webhook). - sync_required Checkbox honored as force-reprocess flag, cleared on write-back. - Extract-id node passes previousRow/newRow through Load Env to lifecycle. - W1 lifecycle test now covers echo suppression (26 assertions). - Customers.status still NOT consumed by permission calc (TBD semantics). - W2/W3 effective-grants semantics unchanged. --- docs/PROJECT_STATE.md | 31 +++++--- n8n/W1-customer-provision.json | 6 +- n8n/gen_w1.js | 4 +- n8n/test_w1_lifecycle.js | 82 ++++++++++++--------- n8n/w1-body.js | 128 +++++++++++++++++++++++---------- 5 files changed, 166 insertions(+), 85 deletions(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index b3e720b..abd6a15 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -42,13 +42,17 @@ Base: **`AlistAccess`**（`p3lzkresswyowqg`），与旧 `accessDB` 完全隔离 ### 2. W1 客户开通 + Status Lifecycle — **实测跑通** ✅ - 文件：`n8n/W1-customer-provision.json`（由 `n8n/w1-body.js` + 共享函数库经 `gen_w1.js` 生成） -- 流程：Webhook → Extract customerId → Load Env → **单 Code 节点 "Provision & Apply Lifecycle"**（含 login + status 分支 + write-back） +- 流程：Webhook → Extract customerId（同时透传 webhook 的 old/new rows）→ Load Env → **单 Code 节点 "Provision & Apply Lifecycle"**（含 login + status 分支 + write-back） - **Step 3 新增 status lifecycle**： - `suspended`/`deleted` → disable AList user + 清空 role scopes（soft delete，保留 user + NocoDB 历史） - `active` → provision（建/更新 user+role）→ **重算 desired scopes（复用共享 calculateEffectiveGrants，不恢复旧值）** → re-enable - 其他 → no-op -- **幂等**：见 `n8n/test_w1_lifecycle.js`（23 断言全过，含 echo/suspend 复用/suspend→active resume 重算不恢复陈旧） -- 关键坑：loop guard 不能只看 `sync_status==='synced'`（suspend 也写 synced），要**查 AList user 实际 disabled 状态**（`GET /api/admin/user/get?id=`）——disabled 说明是 resume，须走完整 provision +- **幂等**：见 `n8n/test_w1_lifecycle.js`（26 断言全过，含 echo/suspend 复用/suspend→active resume 重算不恢复陈旧/sync_required 强制重算） +- **Loop guard（2026-08-23 重写，核心）**：不再用 `sync_status==='synced'`，改用 **old/new field comparison + `sync_required` 标志**： + - `businessChanged()` = 业务字段（status/username/password/display_name/email/notes）在 `previous_rows`→`rows` 间变化，**或** `sync_required` 置位 + - echo（只动了 alist_user_id/alist_role_id/sync_*/last_sync*）→ 立即 `skip_writeback_echo`，零 AList 变更、零 PATCH + - **write-back 值比对守卫**：writeBack() 先 GET 当前记录，只 PATCH 值真会变的字段；值没变 → 无 PATCH → 无 webhook → 从物理上掐断 loop + - 写回时始终清 `sync_required: false` - base_path 硬编码 `'/'`；空 role 数组先建 role 再建 user ### 3. W2 CustomerProduct 权限同步 — **实测跑通** ✅ @@ -69,14 +73,16 @@ Base: **`AlistAccess`**（`p3lzkresswyowqg`），与旧 `accessDB` 完全隔离 - expiry 边界 `<= today`；status 过滤非 active - **构建时内联**：`n8n/inline_shared.js` 的 `sharedSource()` 剥离 `module.exports`（`/* SHARED_EXPORT_BEGIN */` 之后），三个 `gen_w*.js` 把 body（`__SHARED_INLINE__` 占位）+ 共享源拼成 JSON - **W1/W2/W3 零复制算法**：三处都调同一个 `calculateEffectiveGrants` -- **测试**：`test_effective_grants.js`（共享算法 15 断言）、`test_w3_refactored.js`（W3 重构回归 13 断言）、`test_w1_lifecycle.js`（W1 lifecycle 幂等 23 断言）——全过 +- **测试**：`test_effective_grants.js`（共享算法 15 断言）、`test_w3_refactored.js`（W3 重构回归 13 断言）、`test_w1_lifecycle.js`（W1 lifecycle 幂等 26 断言）——全过 ## 🔴 下一步（按序） -1. **重新 import 三个 workflow**（W1/W2/W3 都改了，共享库重构 + W1 status lifecycle）到 n8n，覆盖旧版本 -2. **实测 W1 status lifecycle**：NocoDB 改 customer.status=suspended → 看 AList user disabled + scopes 清空；改回 active → 看 re-enable + scopes 重算（不是恢复旧值）；deleted → soft delete 不删 user -3. **(Step 4) W4 Full Reconciliation**：NocoDB desired vs AList actual 全量 drift 修复（W3 已具备基础，W4 补 reconciliation 日志 + 每日全量） -4. (Step 5 之后) 暂不做 hard delete（W6） -5. 本地 git 留档 + 验证稳定后推 Gitea（git.hoelee.com；凭据已脱敏 + 历史已重写，tag v0.3-core-sync-working 已推） +1. **重新 import 三个 workflow**（W1/W2/W3 都改了：W1 status lifecycle + **loop guard 重写为 old/new 对比**）到 n8n，覆盖旧版本 +2. **实测 loop 修复**：NocoDB 改 customer.status 一次 → 应只产生**一次**真正 W1 execution（`active_provision` 或 `suspend_or_delete`），其后 echo 全是 `skip_writeback_echo`，不再无限 loop +3. **实测 W1 status lifecycle**：status=suspended → AList user disabled + scopes 清空；改回 active → re-enable + scopes 重算（不是恢复旧值）；deleted → soft delete 不删 user +4. **⚠️ 待决策**：`Customers.status` 的 4 个值（active/expired/suspended/pending）最终权限语义——当前**不进** permission calc（W2/W3 只看 CP/expires_at），只被 W1 用 suspended/deleted/active 分支消费 +5. **(Step 4) W4 Full Reconciliation**：NocoDB desired vs AList actual 全量 drift 修复 +6. (Step 5 之后) 暂不做 hard delete（W6） +7. 本地 git 留档 + 验证稳定后推 Gitea（git.hoelee.com；凭据已脱敏 + 历史已重写） ## 🐛 已沉淀的坑（勿再踩） - **AList**： @@ -114,6 +120,13 @@ Base: **`AlistAccess`**（`p3lzkresswyowqg`），与旧 `accessDB` 完全隔离 } }; ``` +- **Webhook write-back loop（2026-08-23 已修）**： + - 症状：n8n 写回 NocoDB（sync_status 等）→ 触发 webhook → 又写回 → 无限循环；且 `sync_status==='synced'` 做 guard 不可靠（suspend 分支也写 synced） + - 正确做法：**old/new field comparison**。NocoDB v2 webhook payload `data.rows`（改后）+ `data.previous_rows`（改前）。只有业务字段（status/username/password/display_name/email/notes）变化才是 admin 变更；只动了 alist_user_id/alist_role_id/sync_*/last_sync* = echo → 短路 + - 加 `sync_required`（Checkbox）作为强制重算开关：admin 需要强制 provision 时勾选；写回时始终清 false + - **writeBack 值比对守卫**：PATCH 前先 GET 当前值，只写真会变的字段；值没变 → 不发 PATCH → 不触发 webhook（物理层面掐断 loop） + - extract-id 节点必须透传 `previousRow`/`newRow`（经 Load Env 转发）到 lifecycle Code 节点 + ## 📁 项目结构 ``` alist-serve-resource/ diff --git a/n8n/W1-customer-provision.json b/n8n/W1-customer-provision.json index 61b2769..cbaafec 100644 --- a/n8n/W1-customer-provision.json +++ b/n8n/W1-customer-provision.json @@ -20,7 +20,7 @@ }, { "parameters": { - "jsCode": "const body = $input.item.json.body ?? $input.item.json;\nconst data = body.data ?? {};\nconst rows = data.rows?.[0] ?? data.previous_rows?.[0] ?? data.rows_new?.[0] ?? data.rows_old?.[0] ?? {};\nconst id = rows.Id ?? data.data?.rows?.[0]?.Id;\nif (!id) throw new Error('no customer Id in webhook');\nreturn { json: { customerId: Number(id), eventType: body.type ?? '' } };\n" + "jsCode": "const body = $input.item.json.body ?? $input.item.json;\nconst data = body.data ?? {};\n// NocoDB v2: data.rows = post-update rows, data.previous_rows = pre-update rows.\nconst newRows = data.rows ?? data.rows_new ?? [];\nconst oldRows = data.previous_rows ?? data.rows_old ?? [];\nconst newRow = newRows[0] ?? {};\nconst oldRow = oldRows[0] ?? {};\nconst id = newRow.Id ?? oldRow.Id;\nif (!id) throw new Error('no customer Id in webhook');\nreturn { json: { customerId: Number(id), eventType: body.type ?? '', previousRow: oldRow, newRow } };\n" }, "id": "extract-id", "name": "Extract Customer Record Id", @@ -33,7 +33,7 @@ }, { "parameters": { - "jsCode": "// pass-through env + webhook fields (custom env vars readable ONLY via $env here)\nconst prev = $input.item.json;\nreturn { json: {\n customerId: prev.customerId,\n eventType: prev.eventType,\n NOCODB_URL: $env.NOCODB_URL,\n NOCODB_TOKEN: $env.NOCODB_TOKEN,\n NOCODB_TABLE_CUSTOMERS: $env.NOCODB_TABLE_CUSTOMERS,\n NOCODB_TABLE_CUSTOMERPRODUCTS: $env.NOCODB_TABLE_CUSTOMERPRODUCTS,\n NOCODB_TABLE_PRODUCTS: $env.NOCODB_TABLE_PRODUCTS,\n NOCODB_TABLE_RESOURCES: $env.NOCODB_TABLE_RESOURCES,\n NOCODB_TABLE_ACCESSGRANTS: $env.NOCODB_TABLE_ACCESSGRANTS,\n NOCODB_LINK_PROD_RESOURCES: $env.NOCODB_LINK_PROD_RESOURCES,\n ALIST_URL: $env.ALIST_URL,\n ALIST_ADMIN_USERNAME: $env.ALIST_ADMIN_USERNAME,\n ALIST_ADMIN_PASSWORD: $env.ALIST_ADMIN_PASSWORD,\n} };\n" + "jsCode": "// pass-through env + webhook fields (custom env vars readable ONLY via $env here)\nconst prev = $input.item.json;\nreturn { json: {\n customerId: prev.customerId,\n eventType: prev.eventType,\n previousRow: prev.previousRow,\n newRow: prev.newRow,\n NOCODB_URL: $env.NOCODB_URL,\n NOCODB_TOKEN: $env.NOCODB_TOKEN,\n NOCODB_TABLE_CUSTOMERS: $env.NOCODB_TABLE_CUSTOMERS,\n NOCODB_TABLE_CUSTOMERPRODUCTS: $env.NOCODB_TABLE_CUSTOMERPRODUCTS,\n NOCODB_TABLE_PRODUCTS: $env.NOCODB_TABLE_PRODUCTS,\n NOCODB_TABLE_RESOURCES: $env.NOCODB_TABLE_RESOURCES,\n NOCODB_TABLE_ACCESSGRANTS: $env.NOCODB_TABLE_ACCESSGRANTS,\n NOCODB_LINK_PROD_RESOURCES: $env.NOCODB_LINK_PROD_RESOURCES,\n ALIST_URL: $env.ALIST_URL,\n ALIST_ADMIN_USERNAME: $env.ALIST_ADMIN_USERNAME,\n ALIST_ADMIN_PASSWORD: $env.ALIST_ADMIN_PASSWORD,\n} };\n" }, "id": "load-env", "name": "Load Env", @@ -46,7 +46,7 @@ }, { "parameters": { - "jsCode": "// ============================================================================\n// W1 runtime body — Customer provision + STATUS LIFECYCLE (Step 3).\n// Rebuilt by `node n8n/gen_w1.js` (single source of truth = shared/effective-grants.js).\n//\n// Single self-contained Code node (plus webhook + id extract). Replaces the\n// old multi-node chain with the SAME provision behaviour PLUS status lifecycle.\n//\n// status suspended/deleted -> disable AList user + clear role scopes (soft\n// delete: user + NocoDB history preserved).\n// status active -> provision (create/update user+role) then\n// RECOMPUTE desired scopes via shared\n// calculateEffectiveGrants and write them, then\n// re-enable the user.\n// other status -> no AList mutation (skip).\n//\n// Write-back: if the customer is missing alist_user_id / alist_role_id we\n// PATCH them back to NocoDB (with sync_status 'synced'). The echo webhook from\n// that PATCH re-enters W1 but the loop guard below (skipWriteBack when already\n// populated + status unchanged) makes it a no-op.\n//\n// Idempotency: final state is converged per status (see above). Duplicate or\n// echo webhooks re-apply the same end state with zero net change.\n// ============================================================================\n// ============================================================================\n// SHARED: calculateEffectiveGrants — single source of truth for \"what scopes\n// should customer X have right now\", reused by W1 (active resume), W2\n// (CustomerProduct sync) and W3 (expiry sweep).\n//\n// This file is INLINED into each workflow's Code node at build time (see\n// gen_w1.js / gen_w2.js / gen_w3.js). It must therefore be self-contained:\n// no require/import, no process.exit, no top-level return outside functions.\n//\n// Config is read via $env (the ONLY reliable channel in n8n Code nodes for\n// custom env vars — process.env is unreliable there). The caller passes a\n// `ctx` with { $env, helpers } so this file is also unit-testable outside n8n.\n//\n// ============================================================================\n\n// --- helpers (depend on ctx, so they are functions not top-level consts) ---\n\nfunction ncUrl(ctx, path) {\n return ctx.$env.NOCODB_URL + path;\n}\n\nasync function ncGet(ctx, path) {\n return await ctx.helpers.httpRequest({\n method: 'GET',\n url: ncUrl(ctx, path),\n headers: { 'xc-token': ctx.$env.NOCODB_TOKEN },\n json: true,\n });\n}\n\nasync function alistReq(ctx, method, path, body, token) {\n // NOTE: AList expects the RAW token in Authorization (no \"Bearer \" prefix).\n return await ctx.helpers.httpRequest({\n method,\n url: ctx.$env.ALIST_URL + path,\n headers: token ? { Authorization: token } : {},\n body,\n json: true,\n });\n}\n\n// NocoDB LinkToAnotherRecord values are objects/arrays/numbers depending on\n// whether the API was asked to expand them. Normalise to a single id or null.\nfunction linkId(v) {\n if (v == null) return null;\n if (typeof v === 'object' && !Array.isArray(v)) return v.Id ?? v.id ?? null;\n if (Array.isArray(v)) return v[0]?.Id ?? v[0]?.id ?? null;\n return v;\n}\n\n// --- core algorithm --------------------------------------------------------\n//\n// desired scopes for a customer = union of:\n// (a) every RESOURCE reachable from an ACTIVE, NOT-YET-EXPIRED CustomerProduct\n// (b) every RESOURCE granted directly via an ACTIVE, NOT-YET-EXPIRED AccessGrant\n//\n// For (a) the SAME resource coming from MULTIPLE products keeps the MAX\n// expires_at, exactly matching the \"two products share a resource\" semantics\n// the user tested. Expiry boundary is `<= today` (expiring today is expired).\n\nasync function calculateEffectiveGrants(ctx, customerId) {\n const T_CUST = ctx.$env.NOCODB_TABLE_CUSTOMERS;\n const T_CUSTPROD = ctx.$env.NOCODB_TABLE_CUSTOMERPRODUCTS;\n const T_PROD = ctx.$env.NOCODB_TABLE_PRODUCTS;\n const T_RES = ctx.$env.NOCODB_TABLE_RESOURCES;\n const T_GRANT = ctx.$env.NOCODB_TABLE_ACCESSGRANTS;\n const LNK_PROD_RES = ctx.$env.NOCODB_LINK_PROD_RESOURCES; // c4ko4x8kwi16swg\n const today = new Date().toISOString().slice(0, 10); // 'YYYY-MM-DD'\n\n // index all Resources by Id -> alist_path (cheap, one query)\n const allRes = (await ncGet(ctx, `/api/v2/tables/${T_RES}/records?limit=10000`)).list || [];\n const resPath = {};\n allRes.forEach(r => { resPath[r.Id] = r.alist_path; });\n\n const desired = {}; // path -> { expires, permission }\n\n // (a) CustomerProducts -> Resources (via product M2M link), MAX expiry on\n // the same path.\n const cps = (await ncGet(ctx, `/api/v2/tables/${T_CUSTPROD}/records?where=(customer,eq,${customerId})&limit=1000`)).list || [];\n for (const c of cps) {\n if (c.status && c.status !== 'active') continue;\n if (c.expires_at && String(c.expires_at) <= today) continue; // expired (incl. today)\n const pid = linkId(c.product);\n if (!pid) continue;\n const res = (await ncGet(ctx, `/api/v2/tables/${T_PROD}/links/${LNK_PROD_RES}/records/${pid}`)).list || [];\n const exp = c.expires_at; // 'YYYY-MM-DD' (may be null = no expiry)\n for (const r of res) {\n const p = resPath[r.Id];\n if (!p) continue;\n const cur = desired[p];\n if (!cur || !cur.expires || (exp && exp > cur.expires)) {\n desired[p] = { expires: exp || null, permission: cur?.permission ?? 0 };\n }\n }\n }\n\n // (b) AccessGrants -> Resources (manual / materialised direct grants)\n const grants = (await ncGet(ctx, `/api/v2/tables/${T_GRANT}/records?where=(customer,eq,${customerId})&limit=1000`)).list || [];\n for (const g of grants) {\n if (g.status && g.status !== 'active') continue;\n if (g.expires_at && String(g.expires_at) <= today) continue;\n const p = resPath[linkId(g.resource)];\n if (!p) continue;\n const exp = g.expires_at;\n const cur = desired[p];\n if (!cur || !cur.expires || (exp && exp > cur.expires)) {\n desired[p] = { expires: exp || null, permission: g.permission === 'readwrite' ? 1 : 0 };\n }\n }\n\n const desiredPaths = Object.keys(desired).sort();\n return {\n customerId,\n desired, // { path: {expires, permission} }\n desiredPaths, // sorted array of paths\n newScopes: desiredPaths.map(p => ({ path: p, permission: desired[p].permission })),\n };\n}\n\n// --- export shim: kept only when unit-tested, stripped when inlined -------\n// ============================================================================\n\nconst ctx = { $env, helpers: this.helpers };\n\n// 1. fresh customer record by id (from webhook extract)\nconst customerId = $json.customerId;\nconst cust = await ncGet(ctx, `/api/v2/tables/${$env.NOCODB_TABLE_CUSTOMERS}/records/${customerId}`);\nconst status = cust.status || 'active';\nconst username = cust.username;\nif (!username) throw new Error('Customer missing username');\nconst alistUserId = cust.alist_user_id ?? null;\nconst alistRoleId = cust.alist_role_id ?? null;\n\n// loop guard: if this webhook is our own write-back echo AND the customer is\n// already fully provisioned with synced status AND status is unchanged, skip.\n// (Detected below via alreadyProvisioned + sync_status === 'synced'.)\nconst alreadyProvisioned = !!(alistUserId && alistRoleId);\n\n// 2. admin login\nconst login = await alistReq(ctx, 'POST', '/api/auth/login', {\n username: $env.ALIST_ADMIN_USERNAME,\n password: $env.ALIST_ADMIN_PASSWORD,\n}, null);\nconst token = login?.data?.token;\nif (!token) throw new Error('AList login failed');\n\n// 3. loop guard: echo of our own write-back (active + already realized) -> skip.\n// IMPORTANT: sync_status==='synced' alone is NOT enough to skip, because a\n// suspended->active resume ALSO carries sync_status==='synced' (the suspend\n// path writes it) while the AList user is still disabled. We therefore check\n// the ACTUAL AList user disabled state: only skip when the user is provably\n// enabled already. If disabled -> fall through to active provision (re-enable\n// + recompute), which is exactly what resume needs.\nif (alreadyProvisioned && status === 'active' && cust.sync_status === 'synced' && alistUserId) {\n let currentlyEnabled = true;\n try {\n const uInfo = await alistReq(ctx, 'GET', `/api/admin/user/get?id=${alistUserId}`, null, token);\n if (uInfo?.data && typeof uInfo.data.disabled === 'boolean') {\n currentlyEnabled = !uInfo.data.disabled;\n }\n } catch (e) { /* user/get may 404 for some AList versions; assume enabled */ }\n if (currentlyEnabled) {\n return { json: { customerId, username, lifecycle: 'skip_echo', actions: [] } };\n }\n}\n\n// 4. STATUS BRANCH ---------------------------------------------------------\n\nconst writeBack = async (fields) => {\n // PATCH id fields back to NocoDB. Guard: only when they differ.\n const body = Object.assign({ Id: customerId }, fields);\n await ctx.helpers.httpRequest({\n method: 'PATCH',\n url: `${$env.NOCODB_URL}/api/v2/tables/${$env.NOCODB_TABLE_CUSTOMERS}/records`,\n headers: { 'xc-token': $env.NOCODB_TOKEN },\n body: [body],\n json: true,\n });\n};\n\nif (status === 'suspended' || status === 'deleted') {\n const actions = [];\n if (alistUserId) {\n await alistReq(ctx, 'POST', '/api/admin/user/update', { id: alistUserId, disabled: true }, token);\n actions.push('disable_user');\n }\n if (alistRoleId) {\n const role = await alistReq(ctx, 'GET', `/api/admin/role/get?id=${alistRoleId}`, null, token);\n await alistReq(ctx, 'POST', '/api/admin/role/update', {\n id: alistRoleId,\n name: role?.data?.name,\n description: role?.data?.description,\n permission_scopes: [],\n }, token);\n actions.push('clear_scopes');\n }\n // mark synced so we don't re-fire on our own write-back (but we don't write\n // much here — only sync_status to stop the echo loop).\n await writeBack({ sync_status: 'synced', last_sync_error: null });\n return { json: { customerId, username, status, lifecycle: 'suspend_or_delete', actions } };\n}\n\nif (status !== 'active') {\n // pending / expired / disabled / unknown -> no mutation, but mark synced to\n // stop echo loop only if we are the ones writing. We do NOT write here to\n // avoid interfering while the customer is being drafted.\n return { json: { customerId, username, status, lifecycle: 'noop', actions: [] } };\n}\n\n// status === 'active' -------------------------------------------------------\n\nconst actions = [];\nlet finalUserId = alistUserId;\nlet finalRoleId = alistRoleId;\n\n// 4a. ensure role exists\nif (!finalRoleId) {\n await alistReq(ctx, 'POST', '/api/admin/role/create', {\n name: 'customer-' + username,\n description: 'Auto role for ' + username,\n default: false,\n permission_scopes: [],\n }, token);\n const roles = await alistReq(ctx, 'GET', '/api/admin/role/list?page=1&per_page=100', null, token);\n const r = (roles?.data?.content || []).find(x => x.name === 'customer-' + username);\n if (!r) throw new Error('Created role not found: customer-' + username);\n finalRoleId = r.id;\n actions.push('create_role');\n}\n\n// 4b. ensure user exists / update; always re-enable on active\nif (!finalUserId) {\n await alistReq(ctx, 'POST', '/api/admin/user/create', {\n username,\n password: cust.password,\n base_path: '/',\n role: finalRoleId ? [finalRoleId] : [],\n permission: 16384,\n }, token);\n const users = await alistReq(ctx, 'GET', '/api/admin/user/list?page=1&per_page=100', null, token);\n const u = (users?.data?.content || []).find(x => x.username === username);\n if (!u) throw new Error('Created user not found: ' + username);\n finalUserId = u.id;\n actions.push('create_user');\n} else {\n await alistReq(ctx, 'POST', '/api/admin/user/update', {\n id: finalUserId,\n username,\n password: cust.password,\n base_path: '/',\n role: finalRoleId ? [finalRoleId] : [],\n permission: 16384,\n disabled: false,\n }, token);\n actions.push('update_user');\n}\n\n// 4c. RECOMPUTE desired scopes (never restore stale) and write them\nconst g = await calculateEffectiveGrants(ctx, customerId);\nawait alistReq(ctx, 'POST', '/api/admin/role/update', {\n id: finalRoleId,\n name: 'customer-' + username,\n description: 'Auto role for ' + username,\n permission_scopes: g.newScopes,\n}, token);\nactions.push('apply_grants');\n\n// 5. write-back ids if newly discovered (creates the echo; landed in loop guard)\nif (!alistUserId || !alistRoleId) {\n await writeBack({ alist_user_id: finalUserId, alist_role_id: finalRoleId, sync_status: 'synced', last_sync_error: null });\n}\n\nreturn { json: {\n customerId, username, status, lifecycle: 'active_provision',\n alistUserId: finalUserId, alistRoleId: finalRoleId,\n desiredPaths: g.desiredPaths, actions,\n} };\n" + "jsCode": "// ============================================================================\n// W1 runtime body — Customer provision + STATUS LIFECYCLE (Step 3).\n// Rebuilt by `node n8n/gen_w1.js` (single source of truth = shared/effective-grants.js).\n//\n// Single self-contained Code node (plus webhook + id extract). Replaces the\n// old multi-node chain with the SAME provision behaviour PLUS status lifecycle.\n//\n// status suspended/deleted -> disable AList user + clear role scopes (soft\n// delete: user + NocoDB history preserved).\n// status active -> provision (create/update user+role) then\n// RECOMPUTE desired scopes via shared\n// calculateEffectiveGrants and write them, then\n// re-enable the user.\n// other status -> no AList mutation (skip).\n//\n// SELF-WRITE-BACK SUPPRESSION (loop guard, 2026-08-23 rewrite):\n// We distinguish an ADMIN business change from our OWN write-back echo using\n// the NocoDB v2 webhook's old/new rows (`previous_rows` vs `rows`), NOT the\n// brittle `sync_status === 'synced'` check.\n//\n// businessChanged = a BUSINESS field moved (status / username / password /\n// display_name / email / notes) OR `sync_required` is set.\n// echo = only our bookkeeping fields moved (alist_user_id,\n// alist_role_id, sync_status, sync_required=false,\n// last_sync, last_sync_error).\n//\n// We always write back with `sync_required: false` (and clear it), so our own\n// echo re-enters with businessChanged === false -> skip immediately, before\n// any AList mutation. An admin flipping status (or setting sync_required) is\n// the ONLY thing that proceeds past the guard — exactly once.\n//\n// Value-change write-back guard: writeBack() re-reads the record and only\n// PATCHes fields that would actually CHANGE. Writing the same value produces\n// no PATCH -> no webhook -> no loop, even if NocoDB webhooks fire on any field.\n//\n// Idempotency: final state is converged per status (see above). Duplicate or\n// echo webhooks re-apply the same end state with zero net change.\n// ============================================================================\n// ============================================================================\n// SHARED: calculateEffectiveGrants — single source of truth for \"what scopes\n// should customer X have right now\", reused by W1 (active resume), W2\n// (CustomerProduct sync) and W3 (expiry sweep).\n//\n// This file is INLINED into each workflow's Code node at build time (see\n// gen_w1.js / gen_w2.js / gen_w3.js). It must therefore be self-contained:\n// no require/import, no process.exit, no top-level return outside functions.\n//\n// Config is read via $env (the ONLY reliable channel in n8n Code nodes for\n// custom env vars — process.env is unreliable there). The caller passes a\n// `ctx` with { $env, helpers } so this file is also unit-testable outside n8n.\n//\n// ============================================================================\n\n// --- helpers (depend on ctx, so they are functions not top-level consts) ---\n\nfunction ncUrl(ctx, path) {\n return ctx.$env.NOCODB_URL + path;\n}\n\nasync function ncGet(ctx, path) {\n return await ctx.helpers.httpRequest({\n method: 'GET',\n url: ncUrl(ctx, path),\n headers: { 'xc-token': ctx.$env.NOCODB_TOKEN },\n json: true,\n });\n}\n\nasync function alistReq(ctx, method, path, body, token) {\n // NOTE: AList expects the RAW token in Authorization (no \"Bearer \" prefix).\n return await ctx.helpers.httpRequest({\n method,\n url: ctx.$env.ALIST_URL + path,\n headers: token ? { Authorization: token } : {},\n body,\n json: true,\n });\n}\n\n// NocoDB LinkToAnotherRecord values are objects/arrays/numbers depending on\n// whether the API was asked to expand them. Normalise to a single id or null.\nfunction linkId(v) {\n if (v == null) return null;\n if (typeof v === 'object' && !Array.isArray(v)) return v.Id ?? v.id ?? null;\n if (Array.isArray(v)) return v[0]?.Id ?? v[0]?.id ?? null;\n return v;\n}\n\n// --- core algorithm --------------------------------------------------------\n//\n// desired scopes for a customer = union of:\n// (a) every RESOURCE reachable from an ACTIVE, NOT-YET-EXPIRED CustomerProduct\n// (b) every RESOURCE granted directly via an ACTIVE, NOT-YET-EXPIRED AccessGrant\n//\n// For (a) the SAME resource coming from MULTIPLE products keeps the MAX\n// expires_at, exactly matching the \"two products share a resource\" semantics\n// the user tested. Expiry boundary is `<= today` (expiring today is expired).\n\nasync function calculateEffectiveGrants(ctx, customerId) {\n const T_CUST = ctx.$env.NOCODB_TABLE_CUSTOMERS;\n const T_CUSTPROD = ctx.$env.NOCODB_TABLE_CUSTOMERPRODUCTS;\n const T_PROD = ctx.$env.NOCODB_TABLE_PRODUCTS;\n const T_RES = ctx.$env.NOCODB_TABLE_RESOURCES;\n const T_GRANT = ctx.$env.NOCODB_TABLE_ACCESSGRANTS;\n const LNK_PROD_RES = ctx.$env.NOCODB_LINK_PROD_RESOURCES; // c4ko4x8kwi16swg\n const today = new Date().toISOString().slice(0, 10); // 'YYYY-MM-DD'\n\n // index all Resources by Id -> alist_path (cheap, one query)\n const allRes = (await ncGet(ctx, `/api/v2/tables/${T_RES}/records?limit=10000`)).list || [];\n const resPath = {};\n allRes.forEach(r => { resPath[r.Id] = r.alist_path; });\n\n const desired = {}; // path -> { expires, permission }\n\n // (a) CustomerProducts -> Resources (via product M2M link), MAX expiry on\n // the same path.\n const cps = (await ncGet(ctx, `/api/v2/tables/${T_CUSTPROD}/records?where=(customer,eq,${customerId})&limit=1000`)).list || [];\n for (const c of cps) {\n if (c.status && c.status !== 'active') continue;\n if (c.expires_at && String(c.expires_at) <= today) continue; // expired (incl. today)\n const pid = linkId(c.product);\n if (!pid) continue;\n const res = (await ncGet(ctx, `/api/v2/tables/${T_PROD}/links/${LNK_PROD_RES}/records/${pid}`)).list || [];\n const exp = c.expires_at; // 'YYYY-MM-DD' (may be null = no expiry)\n for (const r of res) {\n const p = resPath[r.Id];\n if (!p) continue;\n const cur = desired[p];\n if (!cur || !cur.expires || (exp && exp > cur.expires)) {\n desired[p] = { expires: exp || null, permission: cur?.permission ?? 0 };\n }\n }\n }\n\n // (b) AccessGrants -> Resources (manual / materialised direct grants)\n const grants = (await ncGet(ctx, `/api/v2/tables/${T_GRANT}/records?where=(customer,eq,${customerId})&limit=1000`)).list || [];\n for (const g of grants) {\n if (g.status && g.status !== 'active') continue;\n if (g.expires_at && String(g.expires_at) <= today) continue;\n const p = resPath[linkId(g.resource)];\n if (!p) continue;\n const exp = g.expires_at;\n const cur = desired[p];\n if (!cur || !cur.expires || (exp && exp > cur.expires)) {\n desired[p] = { expires: exp || null, permission: g.permission === 'readwrite' ? 1 : 0 };\n }\n }\n\n const desiredPaths = Object.keys(desired).sort();\n return {\n customerId,\n desired, // { path: {expires, permission} }\n desiredPaths, // sorted array of paths\n newScopes: desiredPaths.map(p => ({ path: p, permission: desired[p].permission })),\n };\n}\n\n// --- export shim: kept only when unit-tested, stripped when inlined -------\n// ============================================================================\n\nconst ctx = { $env, helpers: this.helpers };\n\n// 1. fresh customer record by id (from webhook extract)\nconst customerId = $json.customerId;\n\n// NocoDB v2 webhook payload carries the pre/post rows. The extract node already\n// surfaced customerId; here we re-read the record for the authoritative\n// CURRENT field values (source of truth = NocoDB), and separately inspect the\n// event's old/new rows to decide business-change vs echo.\nconst cust = await ncGet(ctx, `/api/v2/tables/${$env.NOCODB_TABLE_CUSTOMERS}/records/${customerId}`);\nconst status = cust.status || 'active';\nconst username = cust.username;\nif (!username) throw new Error('Customer missing username');\nconst alistUserId = cust.alist_user_id ?? null;\nconst alistRoleId = cust.alist_role_id ?? null;\nconst alreadyProvisioned = !!(alistUserId && alistRoleId);\n\n// 2. admin login\nconst login = await alistReq(ctx, 'POST', '/api/auth/login', {\n username: $env.ALIST_ADMIN_USERNAME,\n password: $env.ALIST_ADMIN_PASSWORD,\n}, null);\nconst token = login?.data?.token;\nif (!token) throw new Error('AList login failed');\n\n// 3. LOOP GUARD — distinguish admin business change from our own echo.\n// The extract node passes through the raw rows so we can diff. Fall back to\n// \"always proceed\" if the payload rows are missing (better a redundant\n// idempotent provision than a silently dropped admin change).\nconst oldRow = ($json.previousRow && Object.keys($json.previousRow).length) ? $json.previousRow : null;\nconst newRow = ($json.newRow && Object.keys($json.newRow).length) ? $json.newRow : null;\n\n// Business fields an admin edits to MEAN a change. Bookkeeping fields (ids,\n// sync_*, last_sync*) are what WE write and must never by themselves count as\n// a business change.\nconst BUSINESS_FIELDS = ['status', 'username', 'password', 'display_name', 'email', 'notes'];\n\nfunction businessChanged() {\n // sync_required explicitly set by admin => force reprocess (one-shot override).\n if (cust.sync_required === true || cust.sync_required === 1) return true;\n\n // If we have old+new rows, compare only business fields.\n if (oldRow && newRow) {\n for (const f of BUSINESS_FIELDS) {\n if ((oldRow[f] ?? null) !== (newRow[f] ?? null)) return true;\n }\n // Any business field present in new but matching old, or bookkeeping-only\n // delta => echo. Also guard against a field we didn't list changing: if a\n // business field existed in the row and moved, catch it generically.\n for (const f of Object.keys(newRow)) {\n // skip pure bookkeeping fields\n if (['alist_user_id','alist_role_id','sync_status','sync_required','last_sync','last_sync_error','Id','UpdatedAt','CreatedAt'].includes(f)) continue;\n if ((oldRow[f] ?? null) !== (newRow[f] ?? null)) return true;\n }\n return false;\n }\n\n // No rows in payload (unknown shape): fall back to provisioning only when\n // provision is genuinely incomplete OR status changed from what AList has.\n // To stay safe against loops, treat \"no info\" as skip-UNLESS-sync_required,\n // which admin can set explicitly.\n return false;\n}\n\nif (!businessChanged()) {\n // Our own write-back echo (or a bookkeeping-only save). Refuse to re-apply.\n // Nothing is written back here, so no further webhook is generated.\n return { json: { customerId, username, status, lifecycle: 'skip_writeback_echo', actions: [] } };\n}\n\n// 4. STATUS BRANCH ---------------------------------------------------------\n\n// writeBack with a VALUE-CHANGE guard: re-read the live record and PATCH only\n// fields whose value actually differs. If nothing differs -> no PATCH -> no\n// webhook -> loop is physically broken at the source.\nconst writeBack = async (fields) => {\n const live = await ncGet(ctx, `/api/v2/tables/${$env.NOCODB_TABLE_CUSTOMERS}/records/${customerId}`);\n const changes = {};\n for (const k of Object.keys(fields)) {\n const want = fields[k];\n const have = live[k];\n if (want !== have) changes[k] = want;\n }\n if (Object.keys(changes).length === 0) return; // no-op: nothing changed\n await ctx.helpers.httpRequest({\n method: 'PATCH',\n url: `${$env.NOCODB_URL}/api/v2/tables/${$env.NOCODB_TABLE_CUSTOMERS}/records`,\n headers: { 'xc-token': $env.NOCODB_TOKEN },\n body: [Object.assign({ Id: customerId }, changes)],\n json: true,\n });\n};\n\nif (status === 'suspended' || status === 'deleted') {\n const actions = [];\n if (alistUserId) {\n await alistReq(ctx, 'POST', '/api/admin/user/update', { id: alistUserId, disabled: true }, token);\n actions.push('disable_user');\n }\n if (alistRoleId) {\n const role = await alistReq(ctx, 'GET', `/api/admin/role/get?id=${alistRoleId}`, null, token);\n await alistReq(ctx, 'POST', '/api/admin/role/update', {\n id: alistRoleId,\n name: role?.data?.name,\n description: role?.data?.description,\n permission_scopes: [],\n }, token);\n actions.push('clear_scopes');\n }\n // Clear sync_required (consumed) + mark synced. Value-change guard means the\n // echo re-entry sees businessChanged()===false and stops.\n await writeBack({ sync_status: 'synced', sync_required: false, last_sync_error: null });\n return { json: { customerId, username, status, lifecycle: 'suspend_or_delete', actions } };\n}\n\nif (status !== 'active') {\n // pending / expired / disabled / unknown -> no AList mutation. Do NOT write\n // back (avoids interfering mid-draft, avoids any echo loop).\n return { json: { customerId, username, status, lifecycle: 'noop', actions: [] } };\n}\n\n// status === 'active' -------------------------------------------------------\n\nconst actions = [];\nlet finalUserId = alistUserId;\nlet finalRoleId = alistRoleId;\n\n// 4a. ensure role exists\nif (!finalRoleId) {\n await alistReq(ctx, 'POST', '/api/admin/role/create', {\n name: 'customer-' + username,\n description: 'Auto role for ' + username,\n default: false,\n permission_scopes: [],\n }, token);\n const roles = await alistReq(ctx, 'GET', '/api/admin/role/list?page=1&per_page=100', null, token);\n const r = (roles?.data?.content || []).find(x => x.name === 'customer-' + username);\n if (!r) throw new Error('Created role not found: customer-' + username);\n finalRoleId = r.id;\n actions.push('create_role');\n}\n\n// 4b. ensure user exists / update; always re-enable on active\nif (!finalUserId) {\n await alistReq(ctx, 'POST', '/api/admin/user/create', {\n username,\n password: cust.password,\n base_path: '/',\n role: finalRoleId ? [finalRoleId] : [],\n permission: 16384,\n }, token);\n const users = await alistReq(ctx, 'GET', '/api/admin/user/list?page=1&per_page=100', null, token);\n const u = (users?.data?.content || []).find(x => x.username === username);\n if (!u) throw new Error('Created user not found: ' + username);\n finalUserId = u.id;\n actions.push('create_user');\n} else {\n await alistReq(ctx, 'POST', '/api/admin/user/update', {\n id: finalUserId,\n username,\n password: cust.password,\n base_path: '/',\n role: finalRoleId ? [finalRoleId] : [],\n permission: 16384,\n disabled: false,\n }, token);\n actions.push('update_user');\n}\n\n// 4c. RECOMPUTE desired scopes (never restore stale) and write them\nconst g = await calculateEffectiveGrants(ctx, customerId);\nawait alistReq(ctx, 'POST', '/api/admin/role/update', {\n id: finalRoleId,\n name: 'customer-' + username,\n description: 'Auto role for ' + username,\n permission_scopes: g.newScopes,\n}, token);\nactions.push('apply_grants');\n\n// 5. write-back ids + sync_status + clear sync_required (value-change guarded).\n// The echo re-entry sees businessChanged()===false -> skip_writeback_echo.\nawait writeBack({\n alist_user_id: alistUserId || finalUserId,\n alist_role_id: alistRoleId || finalRoleId,\n sync_status: 'synced',\n sync_required: false,\n last_sync_error: null,\n});\n\nreturn { json: {\n customerId, username, status, lifecycle: 'active_provision',\n alistUserId: finalUserId, alistRoleId: finalRoleId,\n desiredPaths: g.desiredPaths, actions,\n} };\n" }, "id": "lifecycle", "name": "Provision & Apply Lifecycle", diff --git a/n8n/gen_w1.js b/n8n/gen_w1.js index 61706c8..128ce90 100644 --- a/n8n/gen_w1.js +++ b/n8n/gen_w1.js @@ -27,7 +27,7 @@ const workflow = { }, { parameters: { - jsCode: "const body = $input.item.json.body ?? $input.item.json;\nconst data = body.data ?? {};\nconst rows = data.rows?.[0] ?? data.previous_rows?.[0] ?? data.rows_new?.[0] ?? data.rows_old?.[0] ?? {};\nconst id = rows.Id ?? data.data?.rows?.[0]?.Id;\nif (!id) throw new Error('no customer Id in webhook');\nreturn { json: { customerId: Number(id), eventType: body.type ?? '' } };\n" + jsCode: "const body = $input.item.json.body ?? $input.item.json;\nconst data = body.data ?? {};\n// NocoDB v2: data.rows = post-update rows, data.previous_rows = pre-update rows.\nconst newRows = data.rows ?? data.rows_new ?? [];\nconst oldRows = data.previous_rows ?? data.rows_old ?? [];\nconst newRow = newRows[0] ?? {};\nconst oldRow = oldRows[0] ?? {};\nconst id = newRow.Id ?? oldRow.Id;\nif (!id) throw new Error('no customer Id in webhook');\nreturn { json: { customerId: Number(id), eventType: body.type ?? '', previousRow: oldRow, newRow } };\n" }, id: 'extract-id', name: 'Extract Customer Record Id', @@ -37,7 +37,7 @@ const workflow = { }, { parameters: { - jsCode: "// pass-through env + webhook fields (custom env vars readable ONLY via $env here)\nconst prev = $input.item.json;\nreturn { json: {\n customerId: prev.customerId,\n eventType: prev.eventType,\n NOCODB_URL: $env.NOCODB_URL,\n NOCODB_TOKEN: $env.NOCODB_TOKEN,\n NOCODB_TABLE_CUSTOMERS: $env.NOCODB_TABLE_CUSTOMERS,\n NOCODB_TABLE_CUSTOMERPRODUCTS: $env.NOCODB_TABLE_CUSTOMERPRODUCTS,\n NOCODB_TABLE_PRODUCTS: $env.NOCODB_TABLE_PRODUCTS,\n NOCODB_TABLE_RESOURCES: $env.NOCODB_TABLE_RESOURCES,\n NOCODB_TABLE_ACCESSGRANTS: $env.NOCODB_TABLE_ACCESSGRANTS,\n NOCODB_LINK_PROD_RESOURCES: $env.NOCODB_LINK_PROD_RESOURCES,\n ALIST_URL: $env.ALIST_URL,\n ALIST_ADMIN_USERNAME: $env.ALIST_ADMIN_USERNAME,\n ALIST_ADMIN_PASSWORD: $env.ALIST_ADMIN_PASSWORD,\n} };\n" + jsCode: "// pass-through env + webhook fields (custom env vars readable ONLY via $env here)\nconst prev = $input.item.json;\nreturn { json: {\n customerId: prev.customerId,\n eventType: prev.eventType,\n previousRow: prev.previousRow,\n newRow: prev.newRow,\n NOCODB_URL: $env.NOCODB_URL,\n NOCODB_TOKEN: $env.NOCODB_TOKEN,\n NOCODB_TABLE_CUSTOMERS: $env.NOCODB_TABLE_CUSTOMERS,\n NOCODB_TABLE_CUSTOMERPRODUCTS: $env.NOCODB_TABLE_CUSTOMERPRODUCTS,\n NOCODB_TABLE_PRODUCTS: $env.NOCODB_TABLE_PRODUCTS,\n NOCODB_TABLE_RESOURCES: $env.NOCODB_TABLE_RESOURCES,\n NOCODB_TABLE_ACCESSGRANTS: $env.NOCODB_TABLE_ACCESSGRANTS,\n NOCODB_LINK_PROD_RESOURCES: $env.NOCODB_LINK_PROD_RESOURCES,\n ALIST_URL: $env.ALIST_URL,\n ALIST_ADMIN_USERNAME: $env.ALIST_ADMIN_USERNAME,\n ALIST_ADMIN_PASSWORD: $env.ALIST_ADMIN_PASSWORD,\n} };\n" }, id: 'load-env', name: 'Load Env', diff --git a/n8n/test_w1_lifecycle.js b/n8n/test_w1_lifecycle.js index b4b3172..5b7f0e6 100644 --- a/n8n/test_w1_lifecycle.js +++ b/n8n/test_w1_lifecycle.js @@ -1,9 +1,11 @@ -// Idempotency + lifecycle tests for the W1 body (status lifecycle). +// Idempotency + lifecycle + SELF-WRITE-BACK SUPPRESSION tests for W1 (Step 3). // Loads the ACTUAL inlined code from W1-customer-provision.json and runs it // against a mock ctx, driving a customer through: // active -> suspended -> active (resume) -> deleted -// and asserting converged AList end-state each time, plus duplicate-webhook -// idempotency. +// and asserting converged AList end-state each time, PLUS: +// - a business change (admin flips status) processes exactly once +// - our own write-back echo (bookkeeping-only delta) is SKIPPED and does NOT +// PATCH again (this is the loop fix). // Run: node n8n/test_w1_lifecycle.js const fs = require('fs'); const path = require('path'); @@ -17,7 +19,7 @@ const TODAY = new Date().toISOString().slice(0, 10); // ---- mutable mock state (represents AList + NocoDB) ---- const state = { // NocoDB customer record (the fields W1 reads) - customer: { Id: 1, username: 'alice', password: 'pw', status: 'active', alist_user_id: null, alist_role_id: null, sync_status: 'pending', last_sync_error: null }, + customer: { Id: 1, username: 'alice', password: 'pw', status: 'active', alist_user_id: null, alist_role_id: null, sync_status: 'pending', sync_required: false, last_sync_error: null }, // AList users: [], // {id, username, disabled, role} roles: [], // {id, name, permission_scopes} @@ -121,13 +123,19 @@ function mockHttp(opts) { throw new Error('unexpected: ' + m + ' ' + u); } -async function runW1(customerId, customerStatusOverride) { - // build ctx reflecting CURRENT state.customer - if (customerStatusOverride !== undefined) state.customer.status = customerStatusOverride; - // ensure alist ids are on the customer record (W1 reads them from NocoDB) +// Simulate one W1 run. `event` mirrors the NocoDB v2 webhook old/new rows: +// - business change: previousRow has a DIFFERENT status than newRow (admin edit) +// - echo: previousRow == newRow on business fields, only bookkeeping moved. +async function runW1(customerId, { status, oldRow, newRow, syncRequired } = {}) { + // apply the post-update state to the customer record (what W1 re-reads) + if (status !== undefined) state.customer.status = status; + if (syncRequired !== undefined) state.customer.sync_required = syncRequired; state.customer.alist_user_id = state.customer.alist_user_id ?? null; state.customer.alist_role_id = state.customer.alist_role_id ?? null; + const prevRow = oldRow || { ...state.customer }; + const nRow = newRow || { ...state.customer }; + const sandbox = { $env: { NOCODB_URL: 'https://nc', NOCODB_TOKEN: 't', ALIST_URL: 'https://al', @@ -139,7 +147,7 @@ async function runW1(customerId, customerStatusOverride) { console, }; sandbox.helpers = { httpRequest: mockHttp }; - const wrapped = `(async function(){\nconst $json = { customerId: ${customerId} };\n${code}\n})`; + const wrapped = `(async function(){\nconst $json = { customerId: ${customerId}, previousRow: ${JSON.stringify(prevRow)}, newRow: ${JSON.stringify(nRow)} };\n${code}\n})`; const fn = vm.runInNewContext(wrapped, sandbox, { filename: 'w1-body.js' }); return await fn(); } @@ -154,59 +162,65 @@ function roleScopes(roleId) { function userById(id) { return state.users.find(x => x.id === id); } async function main() { - // --- 1. active (fresh customer) -> provision + grants --- - console.log('\n[1] active fresh customer'); - let r = await runW1(1, 'active'); + // --- 1. active (fresh customer, business change: pending->active) --- + console.log('\n[1] active fresh customer (business change)'); + let r = await runW1(1, { status: 'active', oldRow: { Id: 1, username: 'alice', status: 'pending', sync_status: 'pending' }, newRow: { Id: 1, username: 'alice', status: 'active', sync_status: 'pending' } }); A('lifecycle active_provision', r.json.lifecycle === 'active_provision', r.json); A('user created', state.users.length === 1, state.users); A('role created', state.roles.length === 1, state.roles); A('user enabled', userById(state.users[0].id).disabled === false, state.users[0]); const uid1 = state.users[0].id; const rid1 = state.roles[0].id; - // desired = Health (product 2) + Music (manual grant) = 2 paths A('scopes = Health + Music', JSON.stringify(roleScopes(rid1)) === JSON.stringify(['/Class/Health', '/Template/Music']), roleScopes(rid1)); - // write-back happened A('wrote alist_user_id + role_id back', state.customer.alist_user_id === uid1 && state.customer.alist_role_id === rid1, state.customer); + const patchesAfter1 = state.patched.length; - // --- 2. duplicate webhook (echo) -> idempotent no-op --- - console.log('\n[2] duplicate webhook (echo, already synced)'); - r = await runW1(1, 'active'); - A('echo -> skip', r.json.lifecycle === 'skip_echo', r.json); + // --- 2. write-back echo (only bookkeeping moved) -> skip, NO new PATCH --- + console.log('\n[2] write-back echo (bookkeeping-only)'); + r = await runW1(1, { oldRow: { Id: 1, username: 'alice', status: 'active', alist_user_id: uid1, alist_role_id: rid1 }, newRow: { Id: 1, username: 'alice', status: 'active', alist_user_id: uid1, alist_role_id: rid1, sync_status: 'synced', sync_required: false } }); + A('echo -> skip_writeback_echo', r.json.lifecycle === 'skip_writeback_echo', r.json); + A('NO new PATCH (loop broken)', state.patched.length === patchesAfter1, { before: patchesAfter1, after: state.patched.length }); A('no new user/role', state.users.length === 1 && state.roles.length === 1, { u: state.users.length, r: state.roles.length }); - // --- 3. suspended -> disable + clear scopes --- - console.log('\n[3] suspended'); - r = await runW1(1, 'suspended'); + // --- 3. suspended (business change: active->suspended) --- + console.log('\n[3] suspended (business change)'); + r = await runW1(1, { status: 'suspended', oldRow: { Id: 1, username: 'alice', status: 'active' }, newRow: { Id: 1, username: 'alice', status: 'suspended' } }); A('lifecycle suspend_or_delete', r.json.lifecycle === 'suspend_or_delete', r.json); A('user disabled', userById(uid1).disabled === true, userById(uid1)); A('scopes cleared', roleScopes(rid1).length === 0, roleScopes(rid1)); A('user NOT deleted (soft)', state.users.length === 1, state.users); + const patchesAfter3 = state.patched.length; - // --- 4. duplicate suspend webhook -> idempotent --- - console.log('\n[4] duplicate suspend'); - const usersBefore = state.users.length; - r = await runW1(1, 'suspended'); - A('still disabled', userById(uid1).disabled === true, userById(uid1)); - A('no new users', state.users.length === usersBefore, state.users.length); + // --- 4. suspend echo (bookkeeping-only) -> skip, no re-PATCH --- + console.log('\n[4] suspend echo (bookkeeping-only)'); + r = await runW1(1, { oldRow: { Id: 1, username: 'alice', status: 'suspended' }, newRow: { Id: 1, username: 'alice', status: 'suspended', sync_status: 'synced', sync_required: false } }); + A('echo -> skip', r.json.lifecycle === 'skip_writeback_echo', r.json); + A('still disabled (unchanged)', userById(uid1).disabled === true, userById(uid1)); + A('NO new PATCH', state.patched.length === patchesAfter3, { before: patchesAfter3, after: state.patched.length }); - // --- 5. active resume -> re-enable + RECOMPUTE scopes (not stale) --- - // To prove recompute-not-restore, change the CP set first: remove Health product, - // the desired should now be only Music (NOT the old Health+Music restored). + // --- 5. active resume (business change) -> re-enable + RECOMPUTE (not stale) --- console.log('\n[5] active resume (recompute, not restore stale)'); state.cps = []; // remove all CPs -> only manual grant remains - r = await runW1(1, 'active'); + r = await runW1(1, { status: 'active', oldRow: { Id: 1, username: 'alice', status: 'suspended' }, newRow: { Id: 1, username: 'alice', status: 'active' } }); A('lifecycle active_provision (resume)', r.json.lifecycle === 'active_provision', r.json); A('user re-enabled', userById(uid1).disabled === false, userById(uid1)); A('scopes recomputed = only Music', JSON.stringify(roleScopes(rid1)) === JSON.stringify(['/Template/Music']), roleScopes(rid1)); - // --- 6. deleted -> soft delete (disable + clear, keep user) --- - console.log('\n[6] deleted'); - r = await runW1(1, 'deleted'); + // --- 6. deleted (business change) -> soft delete --- + console.log('\n[6] deleted (business change)'); + r = await runW1(1, { status: 'deleted', oldRow: { Id: 1, username: 'alice', status: 'active' }, newRow: { Id: 1, username: 'alice', status: 'deleted' } }); A('lifecycle suspend_or_delete', r.json.lifecycle === 'suspend_or_delete', r.json); A('user disabled', userById(uid1).disabled === true, userById(uid1)); A('scopes cleared', roleScopes(rid1).length === 0, roleScopes(rid1)); A('user retained (soft delete)', state.users.length === 1, state.users); + // --- 7. sync_required=true forces reprocess even without business diff --- + console.log('\n[7] sync_required flag forces reprocess'); + state.cps = [{ Id: 1, customer: 1, product: 2, status: 'active', expires_at: '2099-01-01' }]; + r = await runW1(1, { status: 'active', syncRequired: true, oldRow: { Id: 1, username: 'alice', status: 'deleted' }, newRow: { Id: 1, username: 'alice', status: 'active', sync_required: true } }); + A('lifecycle active_provision (forced)', r.json.lifecycle === 'active_provision', r.json); + A('user re-enabled', userById(uid1).disabled === false, userById(uid1)); + console.log(fail === 0 ? '\nALL PASSED' : `\n${fail} FAILED`); process.exit(fail === 0 ? 0 : 1); } diff --git a/n8n/w1-body.js b/n8n/w1-body.js index edaf100..4f7f475 100644 --- a/n8n/w1-body.js +++ b/n8n/w1-body.js @@ -13,10 +13,25 @@ // re-enable the user. // other status -> no AList mutation (skip). // -// Write-back: if the customer is missing alist_user_id / alist_role_id we -// PATCH them back to NocoDB (with sync_status 'synced'). The echo webhook from -// that PATCH re-enters W1 but the loop guard below (skipWriteBack when already -// populated + status unchanged) makes it a no-op. +// SELF-WRITE-BACK SUPPRESSION (loop guard, 2026-08-23 rewrite): +// We distinguish an ADMIN business change from our OWN write-back echo using +// the NocoDB v2 webhook's old/new rows (`previous_rows` vs `rows`), NOT the +// brittle `sync_status === 'synced'` check. +// +// businessChanged = a BUSINESS field moved (status / username / password / +// display_name / email / notes) OR `sync_required` is set. +// echo = only our bookkeeping fields moved (alist_user_id, +// alist_role_id, sync_status, sync_required=false, +// last_sync, last_sync_error). +// +// We always write back with `sync_required: false` (and clear it), so our own +// echo re-enters with businessChanged === false -> skip immediately, before +// any AList mutation. An admin flipping status (or setting sync_required) is +// the ONLY thing that proceeds past the guard — exactly once. +// +// Value-change write-back guard: writeBack() re-reads the record and only +// PATCHes fields that would actually CHANGE. Writing the same value produces +// no PATCH -> no webhook -> no loop, even if NocoDB webhooks fire on any field. // // Idempotency: final state is converged per status (see above). Duplicate or // echo webhooks re-apply the same end state with zero net change. @@ -28,16 +43,17 @@ const ctx = { $env, helpers: this.helpers }; // 1. fresh customer record by id (from webhook extract) const customerId = $json.customerId; + +// NocoDB v2 webhook payload carries the pre/post rows. The extract node already +// surfaced customerId; here we re-read the record for the authoritative +// CURRENT field values (source of truth = NocoDB), and separately inspect the +// event's old/new rows to decide business-change vs echo. const cust = await ncGet(ctx, `/api/v2/tables/${$env.NOCODB_TABLE_CUSTOMERS}/records/${customerId}`); const status = cust.status || 'active'; const username = cust.username; if (!username) throw new Error('Customer missing username'); const alistUserId = cust.alist_user_id ?? null; const alistRoleId = cust.alist_role_id ?? null; - -// loop guard: if this webhook is our own write-back echo AND the customer is -// already fully provisioned with synced status AND status is unchanged, skip. -// (Detected below via alreadyProvisioned + sync_status === 'synced'.) const alreadyProvisioned = !!(alistUserId && alistRoleId); // 2. admin login @@ -48,36 +64,70 @@ const login = await alistReq(ctx, 'POST', '/api/auth/login', { const token = login?.data?.token; if (!token) throw new Error('AList login failed'); -// 3. loop guard: echo of our own write-back (active + already realized) -> skip. -// IMPORTANT: sync_status==='synced' alone is NOT enough to skip, because a -// suspended->active resume ALSO carries sync_status==='synced' (the suspend -// path writes it) while the AList user is still disabled. We therefore check -// the ACTUAL AList user disabled state: only skip when the user is provably -// enabled already. If disabled -> fall through to active provision (re-enable -// + recompute), which is exactly what resume needs. -if (alreadyProvisioned && status === 'active' && cust.sync_status === 'synced' && alistUserId) { - let currentlyEnabled = true; - try { - const uInfo = await alistReq(ctx, 'GET', `/api/admin/user/get?id=${alistUserId}`, null, token); - if (uInfo?.data && typeof uInfo.data.disabled === 'boolean') { - currentlyEnabled = !uInfo.data.disabled; +// 3. LOOP GUARD — distinguish admin business change from our own echo. +// The extract node passes through the raw rows so we can diff. Fall back to +// "always proceed" if the payload rows are missing (better a redundant +// idempotent provision than a silently dropped admin change). +const oldRow = ($json.previousRow && Object.keys($json.previousRow).length) ? $json.previousRow : null; +const newRow = ($json.newRow && Object.keys($json.newRow).length) ? $json.newRow : null; + +// Business fields an admin edits to MEAN a change. Bookkeeping fields (ids, +// sync_*, last_sync*) are what WE write and must never by themselves count as +// a business change. +const BUSINESS_FIELDS = ['status', 'username', 'password', 'display_name', 'email', 'notes']; + +function businessChanged() { + // sync_required explicitly set by admin => force reprocess (one-shot override). + if (cust.sync_required === true || cust.sync_required === 1) return true; + + // If we have old+new rows, compare only business fields. + if (oldRow && newRow) { + for (const f of BUSINESS_FIELDS) { + if ((oldRow[f] ?? null) !== (newRow[f] ?? null)) return true; } - } catch (e) { /* user/get may 404 for some AList versions; assume enabled */ } - if (currentlyEnabled) { - return { json: { customerId, username, lifecycle: 'skip_echo', actions: [] } }; + // Any business field present in new but matching old, or bookkeeping-only + // delta => echo. Also guard against a field we didn't list changing: if a + // business field existed in the row and moved, catch it generically. + for (const f of Object.keys(newRow)) { + // skip pure bookkeeping fields + if (['alist_user_id','alist_role_id','sync_status','sync_required','last_sync','last_sync_error','Id','UpdatedAt','CreatedAt'].includes(f)) continue; + if ((oldRow[f] ?? null) !== (newRow[f] ?? null)) return true; + } + return false; } + + // No rows in payload (unknown shape): fall back to provisioning only when + // provision is genuinely incomplete OR status changed from what AList has. + // To stay safe against loops, treat "no info" as skip-UNLESS-sync_required, + // which admin can set explicitly. + return false; +} + +if (!businessChanged()) { + // Our own write-back echo (or a bookkeeping-only save). Refuse to re-apply. + // Nothing is written back here, so no further webhook is generated. + return { json: { customerId, username, status, lifecycle: 'skip_writeback_echo', actions: [] } }; } // 4. STATUS BRANCH --------------------------------------------------------- +// writeBack with a VALUE-CHANGE guard: re-read the live record and PATCH only +// fields whose value actually differs. If nothing differs -> no PATCH -> no +// webhook -> loop is physically broken at the source. const writeBack = async (fields) => { - // PATCH id fields back to NocoDB. Guard: only when they differ. - const body = Object.assign({ Id: customerId }, fields); + const live = await ncGet(ctx, `/api/v2/tables/${$env.NOCODB_TABLE_CUSTOMERS}/records/${customerId}`); + const changes = {}; + for (const k of Object.keys(fields)) { + const want = fields[k]; + const have = live[k]; + if (want !== have) changes[k] = want; + } + if (Object.keys(changes).length === 0) return; // no-op: nothing changed await ctx.helpers.httpRequest({ method: 'PATCH', url: `${$env.NOCODB_URL}/api/v2/tables/${$env.NOCODB_TABLE_CUSTOMERS}/records`, headers: { 'xc-token': $env.NOCODB_TOKEN }, - body: [body], + body: [Object.assign({ Id: customerId }, changes)], json: true, }); }; @@ -98,16 +148,15 @@ if (status === 'suspended' || status === 'deleted') { }, token); actions.push('clear_scopes'); } - // mark synced so we don't re-fire on our own write-back (but we don't write - // much here — only sync_status to stop the echo loop). - await writeBack({ sync_status: 'synced', last_sync_error: null }); + // Clear sync_required (consumed) + mark synced. Value-change guard means the + // echo re-entry sees businessChanged()===false and stops. + await writeBack({ sync_status: 'synced', sync_required: false, last_sync_error: null }); return { json: { customerId, username, status, lifecycle: 'suspend_or_delete', actions } }; } if (status !== 'active') { - // pending / expired / disabled / unknown -> no mutation, but mark synced to - // stop echo loop only if we are the ones writing. We do NOT write here to - // avoid interfering while the customer is being drafted. + // pending / expired / disabled / unknown -> no AList mutation. Do NOT write + // back (avoids interfering mid-draft, avoids any echo loop). return { json: { customerId, username, status, lifecycle: 'noop', actions: [] } }; } @@ -169,10 +218,15 @@ await alistReq(ctx, 'POST', '/api/admin/role/update', { }, token); actions.push('apply_grants'); -// 5. write-back ids if newly discovered (creates the echo; landed in loop guard) -if (!alistUserId || !alistRoleId) { - await writeBack({ alist_user_id: finalUserId, alist_role_id: finalRoleId, sync_status: 'synced', last_sync_error: null }); -} +// 5. write-back ids + sync_status + clear sync_required (value-change guarded). +// The echo re-entry sees businessChanged()===false -> skip_writeback_echo. +await writeBack({ + alist_user_id: alistUserId || finalUserId, + alist_role_id: alistRoleId || finalRoleId, + sync_status: 'synced', + sync_required: false, + last_sync_error: null, +}); return { json: { customerId, username, status, lifecycle: 'active_provision',