fix: derive config site from key and let site wildcards govern site config

src/routes/config.js

@@ -14,16 +14,25 @@ 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 };
   }
 
   return putKv(req, env, daCtx);
 }
 
 export async function getConfig({ env, daCtx }) {
-  if (!hasPermission(daCtx, configPermissionPath(daCtx), 'read', true)) {
+  if (!hasConfigPermission(daCtx, 'read')) {
     return { status: 403 };
   }
 

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}`;
   }

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 = {};

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 () => {