fix(list): allow listing an ancestor folder when permission is only on a descendant

src/index.js

@@ -39,13 +39,21 @@ export default {
       return daResp({ status: 500, error: e.message });
     }
 
-    const { users, authorized, key } = daCtx;
+    const {
+      users, authorized, key, api,
+    } = daCtx;
 
     // Anonymous users are not permitted
     const anon = users.some((user) => user.email === 'anonymous');
     if (anon) return daResp({ status: 401 });
 
-    if (!authorized) return daResp({ status: 403 });
+    // `authorized` only reflects permission on the exact requested path. A user
+    // granted permission on some deeper descendant only (e.g. /folder2/a/b/c)
+    // is not "authorized" for a listing of an ancestor folder by that measure,
+    // but should still be able to list it to reach their descendant. The list
+    // route itself knows how to fall back to descendant permission, so this
+    // blanket gate must not shadow it.
+    if (!authorized && api !== 'list') return daResp({ status: 403 });
 
     if (key?.startsWith('.da-versions')) {
       return daResp({ status: 404 });

src/routes/list.js

@@ -11,13 +11,20 @@
  */
 import listBuckets from '../storage/bucket/list.js';
 import listObjects from '../storage/object/list.js';
-import { getChildRules, hasPermission } from '../utils/auth.js';
+import { getChildRules, hasDescendantPermission, hasPermission } from '../utils/auth.js';
 
 export default async function getList({ env, daCtx }) {
   if (!daCtx.org) return listBuckets(env, daCtx);
-  if (!hasPermission(daCtx, daCtx.key, 'read')) return { status: 403 };
+
+  const canReadDir = hasPermission(daCtx, daCtx.key, 'read');
+  if (!canReadDir && !hasDescendantPermission(daCtx, daCtx.key, 'read')) {
+    return { status: 403 };
+  }
 
   // Get the child rules of the current folder and store this in daCtx.aclCtx
   getChildRules(daCtx);
-  return /* await */ listObjects(env, daCtx);
+  // When the user can't read this folder directly but has permission on some
+  // descendant, only the folder itself is authorized as an ancestor - each
+  // child must still be checked individually before being shown.
+  return /* await */ listObjects(env, daCtx, undefined, !canReadDir);
 }

src/storage/object/list.js

@@ -16,6 +16,7 @@ import {
 
 import getS3Config from '../utils/config.js';
 import formatList from '../utils/list.js';
+import { hasDescendantPermission, hasPermission } from '../../utils/auth.js';
 
 function buildInput({
   bucket, org, key, maxKeys, continuationToken,
@@ -30,7 +31,24 @@ function buildInput({
   return input;
 }
 
-export default async function listObjects(env, daCtx, maxKeys) {
+// Only used when the caller couldn't read the listed folder directly, but got
+// in because some descendant is permitted (see src/routes/list.js). Every
+// child then needs its own check: folders may lead to a permitted descendant
+// even if not directly readable, but files have no descendants to fall back on.
+function filterUnauthorized(daCtx, items) {
+  // item.path is bucket-relative (starts with /{org}/...) since one bucket is
+  // shared across orgs, but permissions are org-scoped and keyed off
+  // org-relative paths (like daCtx.key) - so the org segment must be stripped
+  // before checking.
+  const orgPrefix = `/${daCtx.org}`;
+  return items.filter((item) => {
+    const relPath = item.path.slice(orgPrefix.length) || '/';
+    return hasPermission(daCtx, relPath, 'read')
+      || (!item.ext && hasDescendantPermission(daCtx, relPath, 'read'));
+  });
+}
+
+export default async function listObjects(env, daCtx, maxKeys, restrictToPermitted = false) {
   const config = getS3Config(env);
   const client = new S3Client(config);
 
@@ -42,7 +60,9 @@ export default async function listObjects(env, daCtx, maxKeys) {
   try {
     const resp = await client.send(command);
     // console.log(resp);
-    const body = formatList(resp);
+    const body = restrictToPermitted
+      ? filterUnauthorized(daCtx, formatList(resp))
+      : formatList(resp);
     const nextContinuationToken = resp.IsTruncated
       && resp.NextContinuationToken
       && resp.NextContinuationToken !== daCtx.continuationToken

src/utils/auth.js

@@ -453,6 +453,37 @@ export function getChildRules(daCtx) {
   daCtx.aclCtx.childRules = [`${probeDir}**=${[...resultSet].join(',')}`];
 }
 
+/**
+ * Whether the user has `action` on some path at or below `path` (a descendant,
+ * or `path` itself). Used to let a listing of an ancestor folder proceed even
+ * when the user has no permission on the folder itself - e.g. a user granted
+ * read on `/folder2/a/b/c` only should still see `folder2` when listing `/`.
+ * Keyword paths (CONFIG, ACLTRACE) are ignored since they don't represent
+ * content and must not leak directory visibility.
+ */
+export function hasDescendantPermission(daCtx, path, action = 'read') {
+  if (path === null || path === undefined) return false;
+  const { pathLookup } = daCtx.aclCtx;
+  if (pathLookup.size === 0) return true;
+
+  const p = !path.startsWith('/') ? `/${path}` : path;
+  const dirKey = p === '/' ? '/' : `${p.endsWith('/') ? p.slice(0, -1) : p}/`;
+
+  return daCtx.users.every((u) => getIdents(u).some((ident) => (pathLookup.get(ident) || [])
+    .some((r) => {
+      if (!r.path.startsWith('/')) return false;
+      if (r.path === 'CONFIG' || r.path.endsWith('/CONFIG')) return false;
+      if (!r.actions.includes(action)) return false;
+
+      let base = r.path;
+      if (base.endsWith('/+**')) base = base.slice(0, -3);
+      else if (base.endsWith('/**')) base = base.slice(0, -2);
+      else base = base.endsWith('/') ? base : `${base}/`;
+
+      return base.startsWith(dirKey);
+    })));
+}
+
 export function hasPermission(daCtx, path, action, keywordPath = false) {
   if (path === null || path === undefined) return false;
   if (daCtx.aclCtx.pathLookup.size === 0) {

test/index.test.js

@@ -67,6 +67,50 @@ describe('fetch', () => {
     assert.strictEqual(resp.status, 403);
   });
 
+  it('defers to the list route when not authorized on the exact path but api is list', async () => {
+    // daCtx.authorized reflects exact-path permission only. A user who is only
+    // granted access on a deep descendant (e.g. /folder2/a/b/c) has
+    // authorized=false for the root listing request, but routes/list.js knows
+    // how to fall back to descendant permission - so the blanket 403 gate here
+    // must not shadow it for the list api.
+    const hnd = await esmock('../src/index.js', {
+      '../src/utils/daCtx.js': {
+        default: async () => ({
+          authorized: false,
+          users: [{ email: 'acapt@adobe.com' }],
+          path: '/list/kptdobe',
+          api: 'list',
+          org: 'kptdobe',
+          key: '',
+        }),
+      },
+      '../src/handlers/get.js': {
+        default: async () => ({ status: 200, body: '[]', contentType: 'application/json' }),
+      },
+    });
+
+    const resp = await hnd.fetch({ method: 'GET', url: 'http://www.example.com/list/kptdobe' }, {});
+    assert.strictEqual(resp.status, 200);
+  });
+
+  it('still returns 403 for a non-list api when not authorized on the exact path', async () => {
+    const hnd = await esmock('../src/index.js', {
+      '../src/utils/daCtx.js': {
+        default: async () => ({
+          authorized: false,
+          users: [{ email: 'acapt@adobe.com' }],
+          path: '/source/kptdobe/test/index.html',
+          api: 'source',
+          org: 'kptdobe',
+          key: 'test/index.html',
+        }),
+      },
+    });
+
+    const resp = await hnd.fetch({ method: 'GET', url: 'http://www.example.com/source/kptdobe/test/index.html' }, {});
+    assert.strictEqual(resp.status, 403);
+  });
+
   it('return 404 for unknown get route', async () => {
     const hnd = await esmock('../src/index.js', {
       '../src/utils/daCtx.js': {

test/routes/list.test.js

@@ -15,8 +15,10 @@ import esmock from 'esmock';
 describe('List Route', () => {
   it('Test getList with permissions', async () => {
     const loCalled = [];
-    const listObjects = (e, c) => {
-      loCalled.push({ e, c });
+    const listObjects = (e, c, maxKeys, restrictToPermitted) => {
+      loCalled.push({
+        e, c, maxKeys, restrictToPermitted,
+      });
       return {};
     };
 
@@ -27,13 +29,16 @@ describe('List Route', () => {
       }
       return true;
     };
+    // No descendant access either - the 403 must still hold.
+    const hasDescendantPermission = () => false;
 
     const getList = await esmock('../../src/routes/list.js', {
       '../../src/storage/object/list.js': {
         default: listObjects,
       },
       '../../src/utils/auth.js': {
         hasPermission,
+        hasDescendantPermission,
       },
     });
     const resp = await getList({ env: {}, daCtx: ctx, aclCtx: {} });
@@ -49,9 +54,74 @@ describe('List Route', () => {
     });
     assert.strictEqual(1, loCalled.length);
     assert.strictEqual('q/q', loCalled[0].c.key);
+    assert.strictEqual(false, loCalled[0].restrictToPermitted, 'a normally-permitted dir must not be filtered');
 
     const { childRules } = aclCtx;
     assert.strictEqual(1, childRules.length);
     assert(childRules[0].startsWith('/q/q/**='), 'Should have defined some child rule');
   });
+
+  it('lists an ancestor folder when the user only has permission on a descendant', async () => {
+    const loCalled = [];
+    const listObjects = (e, c, maxKeys, restrictToPermitted) => {
+      loCalled.push({ c, restrictToPermitted });
+      return {};
+    };
+
+    // No direct permission on "" (root), but the user has read somewhere below it.
+    const hasPermission = (c, k, a) => !(k === '' && a === 'read');
+    const hasDescendantPermission = (c, k, a) => k === '' && a === 'read';
+
+    const aclCtx = { pathLookup: new Map([['x', []]]) };
+    const getList = await esmock('../../src/routes/list.js', {
+      '../../src/storage/object/list.js': {
+        default: listObjects,
+      },
+      '../../src/utils/auth.js': {
+        hasPermission,
+        hasDescendantPermission,
+      },
+    });
+
+    const resp = await getList({
+      env: {},
+      daCtx: {
+        org: 'bar', key: '', users: [], aclCtx,
+      },
+    });
+
+    assert.notStrictEqual(resp?.status, 403);
+    assert.strictEqual(1, loCalled.length, 'listObjects should still be invoked');
+    assert.strictEqual(true, loCalled[0].restrictToPermitted, 'children must be filtered per-permission');
+  });
+
+  it('still returns 403 when the user has neither direct nor descendant permission', async () => {
+    const loCalled = [];
+    const listObjects = (...args) => {
+      loCalled.push(args);
+      return {};
+    };
+    const hasPermission = () => false;
+    const hasDescendantPermission = () => false;
+
+    const getList = await esmock('../../src/routes/list.js', {
+      '../../src/storage/object/list.js': {
+        default: listObjects,
+      },
+      '../../src/utils/auth.js': {
+        hasPermission,
+        hasDescendantPermission,
+      },
+    });
+
+    const resp = await getList({
+      env: {},
+      daCtx: {
+        org: 'bar', key: '', users: [], aclCtx: { pathLookup: new Map([['x', []]]) },
+      },
+    });
+
+    assert.strictEqual(403, resp.status);
+    assert.strictEqual(0, loCalled.length);
+  });
 });

test/storage/object/list.test.js

@@ -105,6 +105,64 @@ describe('List Objects', () => {
     assert.strictEqual(resp.continuationToken, undefined);
   });
 
+  it('filters out entries the user cannot reach when restrictToPermitted is set', async () => {
+    s3Mock.on(ListObjectsV2Command, {
+      Bucket: 'bkt',
+      Prefix: 'adobe/',
+      Delimiter: '/',
+    }).resolves({
+      $metadata: { httpStatusCode: 200 },
+      CommonPrefixes: [{ Prefix: 'adobe/folder2/' }, { Prefix: 'adobe/folder3/' }],
+      Contents: [{ Key: 'adobe/index.html', LastModified: new Date() }],
+    });
+
+    // The user is only granted read on a page deep inside folder2 - not on
+    // folder2 itself, not on folder3, and not on the root index.html.
+    const pathLookup = new Map([
+      ['deep@bloggs.org', [{ group: 'deep@bloggs.org', path: '/folder2/a/b/c', actions: ['read'] }]],
+    ]);
+    const daCtx = {
+      bucket: 'bkt',
+      org: 'adobe',
+      key: '',
+      users: [{ email: 'deep@bloggs.org' }],
+      aclCtx: { pathLookup },
+    };
+
+    const resp = await listObjects({}, daCtx, undefined, true);
+    const data = JSON.parse(resp.body);
+    assert.deepStrictEqual(data.map((item) => item.name), ['folder2']);
+  });
+
+  it('does not filter entries when restrictToPermitted is not set (default, backward compatible)', async () => {
+    s3Mock.on(ListObjectsV2Command, {
+      Bucket: 'bkt',
+      Prefix: 'adobe/',
+      Delimiter: '/',
+    }).resolves({
+      $metadata: { httpStatusCode: 200 },
+      CommonPrefixes: [{ Prefix: 'adobe/folder2/' }, { Prefix: 'adobe/folder3/' }],
+      Contents: [{ Key: 'adobe/index.html', LastModified: new Date() }],
+    });
+
+    // Same restrictive ACL as above, but the caller did not ask for filtering
+    // (e.g. because the exact-path permission check already passed).
+    const pathLookup = new Map([
+      ['deep@bloggs.org', [{ group: 'deep@bloggs.org', path: '/folder2/a/b/c', actions: ['read'] }]],
+    ]);
+    const daCtx = {
+      bucket: 'bkt',
+      org: 'adobe',
+      key: '',
+      users: [{ email: 'deep@bloggs.org' }],
+      aclCtx: { pathLookup },
+    };
+
+    const resp = await listObjects({}, daCtx);
+    const data = JSON.parse(resp.body);
+    assert.deepStrictEqual(data.map((item) => item.name).sort(), ['folder2', 'folder3', 'index'].sort());
+  });
+
   it('does not return same continuation token again', async () => {
     s3Mock.on(ListObjectsV2Command, {
       Bucket: 'rt-bkt',