From d2f9309a2e32b69bcbddfe8a44818ade05fef4be Mon Sep 17 00:00:00 2001 From: David Bosschaert Date: Tue, 16 Jun 2026 15:40:09 +0100 Subject: [PATCH 1/2] feat: add per-site /{site}/CONFIG permission for site config Introduce a per-site config permission keyword `/{site}/CONFIG` that governs reading/writing site config (/config/{org}/{site}/...), mirroring how the org-level `CONFIG` keyword governs org config (/config/{org}). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/routes/config.js | 6 ++--- src/utils/auth.js | 17 ++++++++++-- test/routes/config.test.js | 55 ++++++++++++++++++++++++++++++++++++++ test/utils/auth.test.js | 51 +++++++++++++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 5 deletions(-) diff --git a/src/routes/config.js b/src/routes/config.js index 3a78f7ff..9e557e46 100644 --- a/src/routes/config.js +++ b/src/routes/config.js @@ -12,10 +12,10 @@ import putKv from '../storage/kv/put.js'; import getKv from '../storage/kv/get.js'; -import { hasPermission } from '../utils/auth.js'; +import { configPermissionPath, hasPermission } from '../utils/auth.js'; export async function postConfig({ req, env, daCtx }) { - if (!hasPermission(daCtx, 'CONFIG', 'write', true)) { + if (!hasPermission(daCtx, configPermissionPath(daCtx), 'write', true)) { return { status: 403 }; } @@ -23,7 +23,7 @@ export async function postConfig({ req, env, daCtx }) { } export async function getConfig({ env, daCtx }) { - if (!hasPermission(daCtx, 'CONFIG', 'read', true)) { + if (!hasPermission(daCtx, configPermissionPath(daCtx), 'read', true)) { return { status: 403 }; } diff --git a/src/utils/auth.js b/src/utils/auth.js index 9d20a15d..66657faf 100644 --- a/src/utils/auth.js +++ b/src/utils/auth.js @@ -228,6 +228,16 @@ export function pathSorter({ path: path1 }, { path: path2 }) { return sp2.length - sp1.length; } +/** + * The keyword path that governs access to a config resource. Org-level config + * (`/config/{org}`) is governed by the `CONFIG` keyword, while site-level config + * (`/config/{org}/{site}/...`) is governed by a per-site `/{site}/CONFIG` keyword. + * The `CONFIG` portion is always uppercase so it cannot collide with a content path. + */ +export function configPermissionPath(daCtx) { + return daCtx.site ? `/${daCtx.site}/CONFIG` : 'CONFIG'; +} + export async function getAclCtx(env, org, users, key, api) { const pathLookup = new Map(); @@ -328,7 +338,10 @@ export async function getAclCtx(env, org, users, key, api) { // Do a lookup for the base key, we always need this info let k; if (api === 'config') { - k = 'CONFIG'; + // Org config (`/config/{org}`) is governed by the `CONFIG` keyword. Site config + // (`/config/{org}/{site}/...`) is governed by a per-site `/{site}/CONFIG` keyword. + const [site] = key.split('/').filter((part) => part.length > 0); + k = site ? `/${site}/CONFIG` : 'CONFIG'; } else { k = key.startsWith('/') ? key : `/${key}`; } @@ -354,7 +367,7 @@ export async function getAclCtx(env, org, users, key, api) { ? actionTrace : undefined; - if (k === 'CONFIG' || api === 'versionsource') { + if (api === 'config' || api === 'versionsource') { actionSet.add('read'); } diff --git a/test/routes/config.test.js b/test/routes/config.test.js index 0a70a9e6..6aa88812 100644 --- a/test/routes/config.test.js +++ b/test/routes/config.test.js @@ -75,6 +75,61 @@ describe('Config', () => { assert.deepStrictEqual(getKVCalled, [{ e: env, c: ctx }]); }); + it('Test postConfig site config uses /{site}/CONFIG permission', async () => { + const ctx = { site: 'mysite' }; + const env = {}; + const req = {}; + + const putKVCalled = []; + const putKV = async (r, q, c) => { + putKVCalled.push({ r, q, c }); + return 'called'; + }; + + const hasPermission = (c, k, a, kw) => k === '/mysite/CONFIG' && a === 'write' && kw === true; + + const { postConfig } = await esmock('../../src/routes/config.js', { + '../../src/storage/kv/put.js': { + default: putKV, + }, + '../../src/utils/auth.js': { + hasPermission, + configPermissionPath: (c) => `/${c.site}/CONFIG`, + }, + }); + + const res = await postConfig({ req, env, daCtx: ctx }); + assert.strictEqual(res, 'called'); + assert.deepStrictEqual(putKVCalled, [{ r: req, q: env, c: ctx }]); + }); + + it('Test getConfig site config uses /{site}/CONFIG permission', async () => { + const ctx = { site: 'mysite' }; + const env = {}; + + const getKVCalled = []; + const getKV = async (e, c) => { + getKVCalled.push({ e, c }); + return 'called'; + }; + + const hasPermission = (c, k, a, kw) => k === '/mysite/CONFIG' && a === 'read' && kw === true; + + const { getConfig } = await esmock('../../src/routes/config.js', { + '../../src/storage/kv/get.js': { + default: getKV, + }, + '../../src/utils/auth.js': { + hasPermission, + configPermissionPath: (c) => `/${c.site}/CONFIG`, + }, + }); + + const res = await getConfig({ env, daCtx: ctx }); + assert.strictEqual(res, 'called'); + assert.deepStrictEqual(getKVCalled, [{ e: env, c: ctx }]); + }); + it('Test no permission', async () => { const ctx = {}; const env = {}; diff --git a/test/utils/auth.test.js b/test/utils/auth.test.js index fb1c0c05..352de9f5 100644 --- a/test/utils/auth.test.js +++ b/test/utils/auth.test.js @@ -19,6 +19,7 @@ import env from './mocks/env.js'; import jose from './mocks/jose.js'; import fetch from './mocks/fetch.js'; import { + configPermissionPath, getAclCtx, getChildRules, getUserActions, @@ -531,6 +532,56 @@ describe('DA auth', () => { assert(!aclCtx.actionSet.has('write')); }); + it('configPermissionPath returns CONFIG for org config', () => { + assert.strictEqual(configPermissionPath({}), 'CONFIG'); + }); + + it('configPermissionPath returns /{site}/CONFIG for site config', () => { + assert.strictEqual(configPermissionPath({ site: 'mysite' }), '/mysite/CONFIG'); + }); + + it('test site CONFIG governs site config read', async () => { + const siteConfig = { + test: { + ':type': 'sheet', + ':sheetname': 'permissions', + data: [ + { path: '/mysite/CONFIG', groups: 'reader@bloggs.org', actions: 'read' }, + { path: 'CONFIG', groups: 'orgadmin@bloggs.org', actions: 'write' }, + ], + }, + }; + const siteEnv = { DA_CONFIG: { get: (name) => siteConfig[name] } }; + + const reader = [{ email: 'reader@bloggs.org' }]; + const aclCtx = await getAclCtx(siteEnv, 'test', reader, 'mysite/config.json', 'config'); + + // The index.js gate always allows reaching the config route for config requests. + assert(aclCtx.actionSet.has('read')); + + // The reader can read this site's config. + assert(hasPermission({ + users: reader, org: 'test', aclCtx, key: 'mysite/config.json', site: 'mysite', + }, configPermissionPath({ site: 'mysite' }), 'read', true)); + + // The reader cannot write this site's config. + assert(!hasPermission({ + users: reader, org: 'test', aclCtx, key: 'mysite/config.json', site: 'mysite', + }, configPermissionPath({ site: 'mysite' }), 'write', true)); + + // A user without the site CONFIG permission cannot read this site's config. + const stranger = [{ email: 'orgadmin@bloggs.org' }]; + const strangerCtx = await getAclCtx(siteEnv, 'test', stranger, 'mysite/config.json', 'config'); + assert(!hasPermission({ + users: stranger, org: 'test', aclCtx: strangerCtx, key: 'mysite/config.json', site: 'mysite', + }, configPermissionPath({ site: 'mysite' }), 'read', true)); + + // The site CONFIG permission does not grant read on a different site's config. + assert(!hasPermission({ + users: reader, org: 'test', aclCtx, key: 'other/config.json', site: 'other', + }, configPermissionPath({ site: 'other' }), 'read', true)); + }); + it('test DA_OPS_IMS_ORG permissions', async () => { const opsOrg = 'MyOpsOrg'; const envOps = { From 5ed1c4c50093d74a10218367be5b1e72f37c0130 Mon Sep 17 00:00:00 2001 From: David Bosschaert Date: Tue, 16 Jun 2026 16:25:18 +0100 Subject: [PATCH 2/2] feat: fall back to CONFIG rule when no /{site}/CONFIG rule is specified Site config access is governed by the per-site /{site}/CONFIG keyword only when such a rule is present in the permissions sheet. When no /{site}/CONFIG rule is specified, site config access falls back to the org-level CONFIG rule. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/utils/auth.js | 35 +++++++++++---- test/utils/auth.test.js | 94 +++++++++++++++++++++++++++++++---------- 2 files changed, 99 insertions(+), 30 deletions(-) diff --git a/src/utils/auth.js b/src/utils/auth.js index 66657faf..1426ef65 100644 --- a/src/utils/auth.js +++ b/src/utils/auth.js @@ -229,13 +229,34 @@ export function pathSorter({ path: path1 }, { path: path2 }) { } /** - * The keyword path that governs access to a config resource. Org-level config - * (`/config/{org}`) is governed by the `CONFIG` keyword, while site-level config - * (`/config/{org}/{site}/...`) is governed by a per-site `/{site}/CONFIG` keyword. - * The `CONFIG` portion is always uppercase so it cannot collide with a content path. + * Resolve the keyword path that governs access to a config resource. + * + * Org-level config (`/config/{org}`) is always governed by the `CONFIG` keyword. + * Site-level config (`/config/{org}/{site}/...`) is governed by a per-site + * `/{site}/CONFIG` keyword, but only when such a rule is actually present in the + * permissions sheet. When no `/{site}/CONFIG` rule is specified, site config access + * falls back to the org-level `CONFIG` rule. The `CONFIG` portion is always uppercase + * so it cannot collide with a content path. + * + * @param {Map} pathLookup the parsed permissions, keyed by ident + * @param {string} [site] the site, if this is a site config request + * @returns {string} the keyword path governing this config resource + */ +function resolveConfigKey(pathLookup, site) { + if (!site) return 'CONFIG'; + const siteKey = `/${site}/CONFIG`; + for (const entries of pathLookup?.values() ?? []) { + if (entries.some((entry) => entry.path === siteKey)) return siteKey; + } + return 'CONFIG'; +} + +/** + * The keyword path that governs access to the config resource of the given request. + * @see resolveConfigKey */ export function configPermissionPath(daCtx) { - return daCtx.site ? `/${daCtx.site}/CONFIG` : 'CONFIG'; + return resolveConfigKey(daCtx.aclCtx?.pathLookup, daCtx.site); } export async function getAclCtx(env, org, users, key, api) { @@ -338,10 +359,8 @@ export async function getAclCtx(env, org, users, key, api) { // Do a lookup for the base key, we always need this info let k; if (api === 'config') { - // Org config (`/config/{org}`) is governed by the `CONFIG` keyword. Site config - // (`/config/{org}/{site}/...`) is governed by a per-site `/{site}/CONFIG` keyword. const [site] = key.split('/').filter((part) => part.length > 0); - k = site ? `/${site}/CONFIG` : 'CONFIG'; + k = resolveConfigKey(pathLookup, site); } else { k = key.startsWith('/') ? key : `/${key}`; } diff --git a/test/utils/auth.test.js b/test/utils/auth.test.js index 352de9f5..161a2f0a 100644 --- a/test/utils/auth.test.js +++ b/test/utils/auth.test.js @@ -536,11 +536,23 @@ describe('DA auth', () => { assert.strictEqual(configPermissionPath({}), 'CONFIG'); }); - it('configPermissionPath returns /{site}/CONFIG for site config', () => { - assert.strictEqual(configPermissionPath({ site: 'mysite' }), '/mysite/CONFIG'); + it('configPermissionPath returns /{site}/CONFIG when a site rule exists', () => { + const pathLookup = new Map([ + ['someone@bloggs.org', [{ path: '/mysite/CONFIG', actions: ['read'] }]], + ]); + const daCtx = { site: 'mysite', aclCtx: { pathLookup } }; + assert.strictEqual(configPermissionPath(daCtx), '/mysite/CONFIG'); }); - it('test site CONFIG governs site config read', async () => { + it('configPermissionPath falls back to CONFIG when no site rule exists', () => { + const pathLookup = new Map([ + ['someone@bloggs.org', [{ path: 'CONFIG', actions: ['write'] }]], + ]); + const daCtx = { site: 'mysite', aclCtx: { pathLookup } }; + assert.strictEqual(configPermissionPath(daCtx), 'CONFIG'); + }); + + it('test site CONFIG governs site config read when a site rule is specified', async () => { const siteConfig = { test: { ':type': 'sheet', @@ -553,33 +565,71 @@ describe('DA auth', () => { }; const siteEnv = { DA_CONFIG: { get: (name) => siteConfig[name] } }; + // Build a site-config daCtx for the given users. + const ctxFor = async (users, site) => { + const aclCtx = await getAclCtx(siteEnv, 'test', users, `${site}/config.json`, 'config'); + return { + users, org: 'test', aclCtx, key: `${site}/config.json`, site, + }; + }; + const reader = [{ email: 'reader@bloggs.org' }]; - const aclCtx = await getAclCtx(siteEnv, 'test', reader, 'mysite/config.json', 'config'); + const readerCtx = await ctxFor(reader, 'mysite'); // The index.js gate always allows reaching the config route for config requests. - assert(aclCtx.actionSet.has('read')); + assert(readerCtx.aclCtx.actionSet.has('read')); + + // A /mysite/CONFIG rule exists, so the site keyword governs this site's config. + assert.strictEqual(configPermissionPath(readerCtx), '/mysite/CONFIG'); // The reader can read this site's config. - assert(hasPermission({ - users: reader, org: 'test', aclCtx, key: 'mysite/config.json', site: 'mysite', - }, configPermissionPath({ site: 'mysite' }), 'read', true)); + assert(hasPermission(readerCtx, configPermissionPath(readerCtx), 'read', true)); + // ...but cannot write it. + assert(!hasPermission(readerCtx, configPermissionPath(readerCtx), 'write', true)); + + // An org-CONFIG holder without /mysite/CONFIG cannot read this site's config, + // because a /mysite/CONFIG rule is specified (no fallback to the CONFIG rule). + const orgAdmin = [{ email: 'orgadmin@bloggs.org' }]; + const orgAdminCtx = await ctxFor(orgAdmin, 'mysite'); + assert.strictEqual(configPermissionPath(orgAdminCtx), '/mysite/CONFIG'); + assert(!hasPermission(orgAdminCtx, configPermissionPath(orgAdminCtx), 'read', true)); + }); - // The reader cannot write this site's config. - assert(!hasPermission({ - users: reader, org: 'test', aclCtx, key: 'mysite/config.json', site: 'mysite', - }, configPermissionPath({ site: 'mysite' }), 'write', true)); + it('test site config falls back to CONFIG rule when no site rule is specified', async () => { + const siteConfig = { + test: { + ':type': 'sheet', + ':sheetname': 'permissions', + data: [ + // Note: no /othersite/CONFIG rule is specified anywhere. + { path: 'CONFIG', groups: 'orgadmin@bloggs.org', actions: 'write' }, + { path: '/mysite/CONFIG', groups: 'reader@bloggs.org', actions: 'read' }, + ], + }, + }; + const siteEnv = { DA_CONFIG: { get: (name) => siteConfig[name] } }; - // A user without the site CONFIG permission cannot read this site's config. - const stranger = [{ email: 'orgadmin@bloggs.org' }]; - const strangerCtx = await getAclCtx(siteEnv, 'test', stranger, 'mysite/config.json', 'config'); - assert(!hasPermission({ - users: stranger, org: 'test', aclCtx: strangerCtx, key: 'mysite/config.json', site: 'mysite', - }, configPermissionPath({ site: 'mysite' }), 'read', true)); + const ctxFor = async (users, site) => { + const aclCtx = await getAclCtx(siteEnv, 'test', users, `${site}/config.json`, 'config'); + return { + users, org: 'test', aclCtx, key: `${site}/config.json`, site, + }; + }; - // The site CONFIG permission does not grant read on a different site's config. - assert(!hasPermission({ - users: reader, org: 'test', aclCtx, key: 'other/config.json', site: 'other', - }, configPermissionPath({ site: 'other' }), 'read', true)); + // No /othersite/CONFIG rule -> falls back to the org-level CONFIG rule. + const orgAdmin = [{ email: 'orgadmin@bloggs.org' }]; + const orgAdminCtx = await ctxFor(orgAdmin, 'othersite'); + assert.strictEqual(configPermissionPath(orgAdminCtx), 'CONFIG'); + // The CONFIG rule grants orgAdmin write (and therefore read) on this site's config. + assert(hasPermission(orgAdminCtx, configPermissionPath(orgAdminCtx), 'read', true)); + assert(hasPermission(orgAdminCtx, configPermissionPath(orgAdminCtx), 'write', true)); + + // A user with only /mysite/CONFIG does not inherit access to othersite via the + // fallback, since the fallback follows the CONFIG rule which they do not hold. + const reader = [{ email: 'reader@bloggs.org' }]; + const readerCtx = await ctxFor(reader, 'othersite'); + assert.strictEqual(configPermissionPath(readerCtx), 'CONFIG'); + assert(!hasPermission(readerCtx, configPermissionPath(readerCtx), 'read', true)); }); it('test DA_OPS_IMS_ORG permissions', async () => {