Added abitlity to make VFS group checking non-strict (#22) (#23)

The `strictGroups: false` mountpoint attribute can now make it so group
matching is less strict. I.e. user can belong to *some* of the groups,
not *all* of them.

Author: andersevenrud

__tests__/utils/vfs.js

@@ -2,6 +2,31 @@ const {Readable, Stream} = require('stream');
 const temp = require('temp');
 const utils = require('../../src/utils/vfs.js');
 
+const checkMountpointGroupPermission = (
+  userGroups = [],
+  mountpointGroups = [],
+  strict
+) => {
+  const check = utils.checkMountpointPermission({
+    session: {
+      user: {
+        groups: userGroups
+      }
+    }
+  }, {}, 'readdir', false, strict);
+
+  const mount = {
+    name: 'osjs',
+    root: 'osjs:/',
+    attributes: {
+      readOnly: true,
+      groups: mountpointGroups
+    }
+  };
+
+  return check({mount});
+};
+
 describe('VFS Utils', () => {
   afterAll(() => {
     temp.cleanupSync();
@@ -42,21 +67,37 @@ describe('VFS Utils', () => {
   });
 
   test('validateGroups - flat groups', () => {
-    const mount = {
+    expect(utils.validateGroups([
+      'successful',
+      'failure'
+    ], '', {
       attributes: {
         groups: [
           'successful'
         ]
       }
-    };
+    })).toBe(true);
+
     expect(utils.validateGroups([
-      'successful',
       'failure'
-    ], '', mount)).toBe(true);
+    ], '', {
+      attributes: {
+        groups: [
+          'successful'
+        ]
+      }
+    })).toBe(false);
 
     expect(utils.validateGroups([
-      'failure'
-    ], '', mount)).toBe(false);
+      'successful'
+    ], '', {
+      attributes: {
+        groups: [
+          'successful',
+          'successful2'
+        ]
+      }
+    }, false)).toBe(true);
   });
 
   test('validateGroups - method maps', () => {
@@ -99,27 +140,43 @@ describe('VFS Utils', () => {
       .toThrowError('Mountpoint \'osjs\' is read-only');
   });
 
-  test('checkMountpointPermission - groups', () => {
-    const check = utils.checkMountpointPermission({
-      session: {
-        user: {
-          groups: []
-        }
-      }
-    }, {}, 'readdir', false);
+  test('checkMountpointPermission - groups', async () => {
+    await expect(checkMountpointGroupPermission(
+      [],
+      ['required']
+    ))
+      .rejects
+      .toThrowError('Permission was denied for \'readdir\' in \'osjs\'');
 
-    const mount = {
-      name: 'osjs',
-      root: 'osjs:/',
-      attributes: {
-        readOnly: true,
-        groups: ['required']
-      }
-    };
+    await expect(checkMountpointGroupPermission(
+      ['missing'],
+      ['required']
+    ))
+      .rejects
+      .toThrowError('Permission was denied for \'readdir\' in \'osjs\'');
 
-    return expect(check({mount}))
+    await expect(checkMountpointGroupPermission(
+      ['required'],
+      ['required', 'some-other']
+    ))
       .rejects
       .toThrowError('Permission was denied for \'readdir\' in \'osjs\'');
+
+    await expect(checkMountpointGroupPermission(
+      ['required'],
+      ['required']
+    )).resolves.toBe(true);
+
+    await expect(checkMountpointGroupPermission(
+      ['required', 'some-other'],
+      ['required']
+    )).resolves.toBe(true);
+
+    await expect(checkMountpointGroupPermission(
+      ['required'],
+      ['required', 'some-other'],
+      false
+    )).resolves.toBe(true);
   });
 
   test('checkMountpointPermission', () => {

src/utils/vfs.js

@@ -73,40 +73,40 @@ const streamFromRequest = req => {
     : fs.createReadStream(req.files.upload.path);
 };
 
-const validateAll = (arr, compare) => arr.every(g => compare.indexOf(g) !== -1);
+const validateAll = (arr, compare, strict = true) => arr[strict ? 'every' : 'some'](g => compare.indexOf(g) !== -1);
 
 /**
  * Validates array groups
  */
-const validateNamedGroups = (groups, userGroups) => {
+const validateNamedGroups = (groups, userGroups, strict) => {
   const namedGroups = groups
     .filter(g => typeof g === 'string');
 
   return namedGroups.length
-    ? validateAll(namedGroups, userGroups)
+    ? validateAll(namedGroups, userGroups, strict)
     : true;
 };
 
 /**
  * Validates matp of groups based on method:[group,...]
  */
-const validateMethodGroups = (groups, userGroups, method) => {
+const validateMethodGroups = (groups, userGroups, method, strict) => {
   const methodGroups = groups
     .find(g => typeof g === 'string' ? false : (method in g));
 
   return methodGroups
-    ? validateAll(methodGroups[method], userGroups)
+    ? validateAll(methodGroups[method], userGroups, strict)
     : true;
 };
 
 /**
  * Validates groups
  */
-const validateGroups = (userGroups, method, mountpoint) => {
+const validateGroups = (userGroups, method, mountpoint, strict) => {
   const groups = mountpoint.attributes.groups || [];
   if (groups.length) {
-    const namedValid = validateNamedGroups(groups, userGroups, method);
-    const methodValid = validateMethodGroups(groups, userGroups, method);
+    const namedValid = validateNamedGroups(groups, userGroups, strict);
+    const methodValid = validateMethodGroups(groups, userGroups, method, strict);
 
     return namedValid && methodValid;
   }
@@ -117,7 +117,7 @@ const validateGroups = (userGroups, method, mountpoint) => {
 /**
  * Checks permissions for given mountpoint
  */
-const checkMountpointPermission = (req, res, method, readOnly) => {
+const checkMountpointPermission = (req, res, method, readOnly, strict) => {
   const userGroups = req.session.user.groups;
 
   return ({mount}) => {
@@ -135,7 +135,7 @@ const checkMountpointPermission = (req, res, method, readOnly) => {
       }
     }
 
-    if (validateGroups(userGroups, method, mount)) {
+    if (validateGroups(userGroups, method, mount, strict)) {
       return Promise.resolve(true);
     }
 

src/vfs.js

@@ -125,7 +125,8 @@ const createRequestFactory = findMountpoint => (getter, method, readOnly, respon
     }
   }
 
-  await checkMountpointPermission(req, res, method, readOnly)(found);
+  const strict = found.mount.attributes.strictGroups !== false;
+  await checkMountpointPermission(req, res, method, readOnly, strict)(found);
 
   const result = await found.adapter[method]({req, res, adapter: found.adapter, mount: found.mount})(...args);
 
@@ -145,8 +146,10 @@ const createCrossRequestFactory = findMountpoint => (getter, method, respond) =>
   const sameAdapter = srcMount.adapter === destMount.adapter;
   const createArgs = t => ({req, res, adapter: t.adapter, mount: t.mount});
 
-  await checkMountpointPermission(req, res, 'readfile', false)(srcMount);
-  await checkMountpointPermission(req, res, 'writefile', true)(destMount);
+  const srcStrict = srcMount.mount.attributes.strictGroups !== false;
+  const destStrict = destMount.mount.attributes.strictGroups !== false;
+  await checkMountpointPermission(req, res, 'readfile', false, srcStrict)(srcMount);
+  await checkMountpointPermission(req, res, 'writefile', true, destStrict)(destMount);
 
   if (sameAdapter) {
     const result = await srcMount