diff --git a/src/routes/config.js b/src/routes/config.js index 9e557e4..f2b1ddd 100644 --- a/src/routes/config.js +++ b/src/routes/config.js @@ -14,8 +14,17 @@ import putKv from '../storage/kv/put.js'; import getKv from '../storage/kv/get.js'; import { configPermissionPath, hasPermission } from '../utils/auth.js'; +// Config access is granted if the user has the action on the resource's own keyword +// (the per-site `/{site}/CONFIG`, or `CONFIG` for org config) OR on the org-level +// `CONFIG` keyword. The latter lets org admins manage any site's config; site rules +// (including `/{site}/**` wildcards) can grant additional access but never restrict it. +function hasConfigPermission(daCtx, action) { + return hasPermission(daCtx, configPermissionPath(daCtx), action, true) + || hasPermission(daCtx, 'CONFIG', action, true); +} + export async function postConfig({ req, env, daCtx }) { - if (!hasPermission(daCtx, configPermissionPath(daCtx), 'write', true)) { + if (!hasConfigPermission(daCtx, 'write')) { return { status: 403 }; } @@ -23,7 +32,7 @@ export async function postConfig({ req, env, daCtx }) { } export async function getConfig({ env, daCtx }) { - if (!hasPermission(daCtx, configPermissionPath(daCtx), 'read', true)) { + if (!hasConfigPermission(daCtx, 'read')) { return { status: 403 }; } diff --git a/src/utils/auth.js b/src/utils/auth.js index 1426ef6..71199fc 100644 --- a/src/utils/auth.js +++ b/src/utils/auth.js @@ -229,34 +229,27 @@ export function pathSorter({ path: path1 }, { path: path2 }) { } /** - * 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 + * The site of a config request, derived from its key (the path below the org). + * For `/config/{org}` the key is empty (org config, no site). For + * `/config/{org}/{site}` and `/config/{org}/{site}/...` the site is the first + * key segment. Note that `daCtx.site` is unreliable here: for the bare + * `/config/{org}/{site}` request the site name is parsed as the filename, leaving + * `daCtx.site` undefined, so the key is the source of truth. */ -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'; +function configSite(key) { + const [site] = (key || '').split('/').filter((part) => part.length > 0); + return site; } /** - * The keyword path that governs access to the config resource of the given request. - * @see resolveConfigKey + * The keyword path naming the config resource of the given request: the per-site + * `/{site}/CONFIG` for site config, or the org-level `CONFIG` for org config. The + * `CONFIG` portion is always uppercase so it cannot collide with a content path. + * Access is granted via this keyword OR the org `CONFIG` keyword (see the config route). */ export function configPermissionPath(daCtx) { - return resolveConfigKey(daCtx.aclCtx?.pathLookup, daCtx.site); + const site = configSite(daCtx.key); + return site ? `/${site}/CONFIG` : 'CONFIG'; } export async function getAclCtx(env, org, users, key, api) { @@ -359,8 +352,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') { - const [site] = key.split('/').filter((part) => part.length > 0); - k = resolveConfigKey(pathLookup, site); + const site = configSite(key); + k = site ? `/${site}/CONFIG` : 'CONFIG'; } else { k = key.startsWith('/') ? key : `/${key}`; } diff --git a/test/routes/config.test.js b/test/routes/config.test.js index 6aa8881..6362560 100644 --- a/test/routes/config.test.js +++ b/test/routes/config.test.js @@ -130,6 +130,60 @@ describe('Config', () => { assert.deepStrictEqual(getKVCalled, [{ e: env, c: ctx }]); }); + it('Test getConfig site config granted via the site keyword', async () => { + // Real configPermissionPath derives /mysite/CONFIG from the key. + const ctx = { key: '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, + }, + }); + + const res = await getConfig({ env, daCtx: ctx }); + assert.strictEqual(res, 'called'); + assert.deepStrictEqual(getKVCalled, [{ e: env, c: ctx }]); + }); + + it('Test getConfig site config granted via the org CONFIG fallback', async () => { + // No /mysite/CONFIG permission, but the user holds the org CONFIG keyword. + const ctx = { key: 'mysite' }; + const env = {}; + + const getKVCalled = []; + const getKV = async (e, c) => { + getKVCalled.push({ e, c }); + return 'called'; + }; + + const hasPermission = (c, k, a, kw) => k === '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, + }, + }); + + 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 161a2f0..f435950 100644 --- a/test/utils/auth.test.js +++ b/test/utils/auth.test.js @@ -532,104 +532,57 @@ describe('DA auth', () => { assert(!aclCtx.actionSet.has('write')); }); - it('configPermissionPath returns CONFIG for org config', () => { + it('configPermissionPath maps a config request to its keyword', () => { + // Org config (no site) -> the CONFIG keyword. assert.strictEqual(configPermissionPath({}), 'CONFIG'); + assert.strictEqual(configPermissionPath({ key: '' }), 'CONFIG'); + // Site config -> /{site}/CONFIG, derived from the key. (daCtx.site is undefined + // for the bare /config/{org}/{site} URL, so the key is the source of truth.) + assert.strictEqual(configPermissionPath({ key: 'mysite' }), '/mysite/CONFIG'); + assert.strictEqual( + configPermissionPath({ key: 'mysite/public/config.json' }), + '/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('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 () => { + it('site config permission is granted by the site keyword or a site wildcard', async () => { + // Building blocks for the union model: the route grants config access if the user + // matches the /{site}/CONFIG keyword OR the org CONFIG keyword (see config route). 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' }, + { path: '/mysite/+**', groups: 'plus@bloggs.org', actions: 'read' }, + { path: '/mysite/**', groups: 'star@bloggs.org', actions: 'write' }, ], }, }; 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'); + // A bare /config/{org}/{site} request: key is the site, daCtx.site absent. + const ctxFor = async (users) => { + const aclCtx = await getAclCtx(siteEnv, 'test', users, 'mysite', 'config'); return { - users, org: 'test', aclCtx, key: `${site}/config.json`, site, + users, org: 'test', aclCtx, key: 'mysite', }; }; + const siteKey = '/mysite/CONFIG'; - const reader = [{ email: 'reader@bloggs.org' }]; - const readerCtx = await ctxFor(reader, 'mysite'); - - // The index.js gate always allows reaching the config route for config requests. - 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(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)); - }); - - 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] } }; + // An explicit /mysite/CONFIG read rule grants read but not write. + const reader = await ctxFor([{ email: 'reader@bloggs.org' }]); + assert(hasPermission(reader, siteKey, 'read', true)); + assert(!hasPermission(reader, siteKey, 'write', 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, - }; - }; + // A /mysite/+** read wildcard matches the site keyword. + const plus = await ctxFor([{ email: 'plus@bloggs.org' }]); + assert(hasPermission(plus, siteKey, '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)); + // A /mysite/** write wildcard grants read+write on the site keyword. + const star = await ctxFor([{ email: 'star@bloggs.org' }]); + assert(hasPermission(star, siteKey, 'read', true)); + assert(hasPermission(star, siteKey, 'write', true)); }); it('test DA_OPS_IMS_ORG permissions', async () => {