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..1426ef65 100644 --- a/src/utils/auth.js +++ b/src/utils/auth.js @@ -228,6 +228,37 @@ export function pathSorter({ path: path1 }, { path: path2 }) { return sp2.length - sp1.length; } +/** + * 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 resolveConfigKey(daCtx.aclCtx?.pathLookup, daCtx.site); +} + export async function getAclCtx(env, org, users, key, api) { const pathLookup = new Map(); @@ -328,7 +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') { - k = 'CONFIG'; + const [site] = key.split('/').filter((part) => part.length > 0); + k = resolveConfigKey(pathLookup, site); } else { k = key.startsWith('/') ? key : `/${key}`; } @@ -354,7 +386,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..161a2f0a 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,106 @@ describe('DA auth', () => { assert(!aclCtx.actionSet.has('write')); }); + it('configPermissionPath returns CONFIG for org config', () => { + assert.strictEqual(configPermissionPath({}), '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 () => { + 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] } }; + + // 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 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] } }; + + 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, + }; + }; + + // 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 () => { const opsOrg = 'MyOpsOrg'; const envOps = {