Merge pull request #13499 from meteor/allow-deny-tweaks

Tweaks on allow/deny rules

packages/allow-deny/allow-deny.js

@@ -174,9 +174,14 @@ CollectionPrototype._defineMutationMethods = function(options) {
           // single-ID selectors.
           if (!isInsert(method)) throwIfSelectorIsNotId(args[0], method);
 
+          const syncMethodName = method.replace('Async', '');
+          const syncValidatedMethodName = '_validated' + method.charAt(0).toUpperCase() + syncMethodName.slice(1);
+          // it forces to use async validated behavior
+          const validatedMethodName = syncValidatedMethodName + 'Async';
+
           if (self._restricted) {
             // short circuit if there is no way it will pass.
-            if (self._validators[method].allow.length === 0) {
+            if (self._validators[syncMethodName].allow.length === 0) {
               throw new Meteor.Error(
                 403,
                 'Access denied. No allow validators set on restricted ' +
@@ -186,11 +191,6 @@ CollectionPrototype._defineMutationMethods = function(options) {
               );
             }
 
-            const syncMethodName = method.replace('Async', '');
-            const syncValidatedMethodName = '_validated' + method.charAt(0).toUpperCase() + syncMethodName.slice(1);
-            // it forces to use async validated behavior on the server
-            const validatedMethodName = Meteor.isServer ? syncValidatedMethodName + 'Async' : syncValidatedMethodName;
-
             args.unshift(this.userId);
             isInsert(method) && args.push(generatedId);
             return self[validatedMethodName].apply(self, args);
@@ -292,7 +292,7 @@ CollectionPrototype._validatedInsertAsync = async function(userId, doc,
   const self = this;
   // call user validators.
   // Any deny returns true means denied.
-  if (await asyncSome(self._validators.insertAsync.deny, async (validator) => {
+  if (await asyncSome(self._validators.insert.deny, async (validator) => {
     const result = validator(userId, docToValidate(validator, doc, generatedId));
     return Meteor._isPromise(result) ? await result : result;
   })) {
@@ -300,7 +300,7 @@ CollectionPrototype._validatedInsertAsync = async function(userId, doc,
   }
   // Any allow returns true means proceed. Throw error if they all fail.
 
-  if (await asyncEvery(self._validators.insertAsync.allow, async (validator) => {
+  if (await asyncEvery(self._validators.insert.allow, async (validator) => {
     const result = validator(userId, docToValidate(validator, doc, generatedId));
     return !(Meteor._isPromise(result) ? await result : result);
   })) {
@@ -315,36 +315,6 @@ CollectionPrototype._validatedInsertAsync = async function(userId, doc,
   return self._collection.insertAsync.call(self._collection, doc);
 };
 
-CollectionPrototype._validatedInsert = function (userId, doc,
-                                                         generatedId) {
-  const self = this;
-
-  // call user validators.
-  // Any deny returns true means denied.
-  if (self._validators.insert.deny.some((validator) => {
-    return validator(userId, docToValidate(validator, doc, generatedId));
-  })) {
-    throw new Meteor.Error(403, "Access denied");
-  }
-  // Any allow returns true means proceed. Throw error if they all fail.
-
-  if (self._validators.insert.allow.every((validator) => {
-    return !validator(userId, docToValidate(validator, doc, generatedId));
-  })) {
-    throw new Meteor.Error(403, "Access denied");
-  }
-
-  // If we generated an ID above, insert it now: after the validation, but
-  // before actually inserting.
-  if (generatedId !== null)
-    doc._id = generatedId;
-
-  return (Meteor.isServer
-    ? self._collection.insertAsync
-    : self._collection.insert
-  ).call(self._collection, doc);
-};
-
 // Simulate a mongo `update` operation while validating that the access
 // control rules set by calls to `allow/deny` are satisfied. If all
 // pass, rewrite the mongo operation to use $in to set the list of
@@ -414,7 +384,7 @@ CollectionPrototype._validatedUpdateAsync = async function(
 
   // call user validators.
   // Any deny returns true means denied.
-  if (await asyncSome(self._validators.updateAsync.deny, async (validator) => {
+  if (await asyncSome(self._validators.update.deny, async (validator) => {
     const factoriedDoc = transformDoc(validator, doc);
     const result = validator(userId,
       factoriedDoc,
@@ -424,8 +394,9 @@ CollectionPrototype._validatedUpdateAsync = async function(
   })) {
     throw new Meteor.Error(403, "Access denied");
   }
+
   // Any allow returns true means proceed. Throw error if they all fail.
-  if (await asyncEvery(self._validators.updateAsync.allow, async (validator) => {
+  if (await asyncEvery(self._validators.update.allow, async (validator) => {
     const factoriedDoc = transformDoc(validator, doc);
     const result = validator(userId,
       factoriedDoc,
@@ -447,102 +418,6 @@ CollectionPrototype._validatedUpdateAsync = async function(
     self._collection, selector, mutator, options);
 };
 
-CollectionPrototype._validatedUpdate = function(
-    userId, selector, mutator, options) {
-  const self = this;
-
-  check(mutator, Object);
-
-  options = Object.assign(Object.create(null), options);
-
-  if (!LocalCollection._selectorIsIdPerhapsAsObject(selector))
-    throw new Error("validated update should be of a single ID");
-
-  // We don't support upserts because they don't fit nicely into allow/deny
-  // rules.
-  if (options.upsert)
-    throw new Meteor.Error(403, "Access denied. Upserts not " +
-                           "allowed in a restricted collection.");
-
-  const noReplaceError = "Access denied. In a restricted collection you can only" +
-        " update documents, not replace them. Use a Mongo update operator, such " +
-        "as '$set'.";
-
-  const mutatorKeys = Object.keys(mutator);
-
-  // compute modified fields
-  const modifiedFields = {};
-
-  if (mutatorKeys.length === 0) {
-    throw new Meteor.Error(403, noReplaceError);
-  }
-  mutatorKeys.forEach((op) => {
-    const params = mutator[op];
-    if (op.charAt(0) !== '$') {
-      throw new Meteor.Error(403, noReplaceError);
-    } else if (!hasOwn.call(ALLOWED_UPDATE_OPERATIONS, op)) {
-      throw new Meteor.Error(
-        403, "Access denied. Operator " + op + " not allowed in a restricted collection.");
-    } else {
-      Object.keys(params).forEach((field) => {
-        // treat dotted fields as if they are replacing their
-        // top-level part
-        if (field.indexOf('.') !== -1)
-          field = field.substring(0, field.indexOf('.'));
-
-        // record the field we are trying to change
-        modifiedFields[field] = true;
-      });
-    }
-  });
-
-  const fields = Object.keys(modifiedFields);
-
-  const findOptions = {transform: null};
-  if (!self._validators.fetchAllFields) {
-    findOptions.fields = {};
-    self._validators.fetch.forEach((fieldName) => {
-      findOptions.fields[fieldName] = 1;
-    });
-  }
-
-  const doc = self._collection.findOne(selector, findOptions);
-  if (!doc)  // none satisfied!
-    return 0;
-
-  // call user validators.
-  // Any deny returns true means denied.
-  if (self._validators.update.deny.some((validator) => {
-    const factoriedDoc = transformDoc(validator, doc);
-    return validator(userId,
-                     factoriedDoc,
-                     fields,
-                     mutator);
-  })) {
-    throw new Meteor.Error(403, "Access denied");
-  }
-  // Any allow returns true means proceed. Throw error if they all fail.
-  if (self._validators.update.allow.every((validator) => {
-    const factoriedDoc = transformDoc(validator, doc);
-    return !validator(userId,
-                      factoriedDoc,
-                      fields,
-                      mutator);
-  })) {
-    throw new Meteor.Error(403, "Access denied");
-  }
-
-  options._forbidReplace = true;
-
-  // Back when we supported arbitrary client-provided selectors, we actually
-  // rewrote the selector to include an _id clause before passing to Mongo to
-  // avoid races, but since selector is guaranteed to already just be an ID, we
-  // don't have to any more.
-
-  return self._collection.update.call(
-    self._collection, selector, mutator, options);
-};
-
 // Only allow these operations in validated updates. Specifically
 // whitelist operations, rather than blacklist, so new complex
 // operations that are added aren't automatically allowed. A complex
@@ -573,14 +448,14 @@ CollectionPrototype._validatedRemoveAsync = async function(userId, selector) {
 
   // call user validators.
   // Any deny returns true means denied.
-  if (await asyncSome(self._validators.removeAsync.deny, async (validator) => {
+  if (await asyncSome(self._validators.remove.deny, async (validator) => {
     const result = validator(userId, transformDoc(validator, doc));
     return Meteor._isPromise(result) ? await result : result;
   })) {
     throw new Meteor.Error(403, "Access denied");
   }
   // Any allow returns true means proceed. Throw error if they all fail.
-  if (await asyncEvery(self._validators.removeAsync.allow, async (validator) => {
+  if (await asyncEvery(self._validators.remove.allow, async (validator) => {
     const result = validator(userId, transformDoc(validator, doc));
     return !(Meteor._isPromise(result) ? await result : result);
   })) {
@@ -595,43 +470,6 @@ CollectionPrototype._validatedRemoveAsync = async function(userId, selector) {
   return self._collection.removeAsync.call(self._collection, selector);
 };
 
-CollectionPrototype._validatedRemove = function(userId, selector) {
-  const self = this;
-
-  const findOptions = {transform: null};
-  if (!self._validators.fetchAllFields) {
-    findOptions.fields = {};
-    self._validators.fetch.forEach((fieldName) => {
-      findOptions.fields[fieldName] = 1;
-    });
-  }
-
-  const doc = self._collection.findOne(selector, findOptions);
-  if (!doc)
-    return 0;
-
-  // call user validators.
-  // Any deny returns true means denied.
-  if (self._validators.remove.deny.some((validator) => {
-    return validator(userId, transformDoc(validator, doc));
-  })) {
-    throw new Meteor.Error(403, "Access denied");
-  }
-  // Any allow returns true means proceed. Throw error if they all fail.
-  if (self._validators.remove.allow.every((validator) => {
-    return !validator(userId, transformDoc(validator, doc));
-  })) {
-    throw new Meteor.Error(403, "Access denied");
-  }
-
-  // Back when we supported arbitrary client-provided selectors, we actually
-  // rewrote the selector to {_id: {$in: [ids that we found]}} before passing to
-  // Mongo to avoid races, but since selector is guaranteed to already just be
-  // an ID, we don't have to any more.
-
-  return self._collection.remove.call(self._collection, selector);
-};
-
 CollectionPrototype._callMutatorMethodAsync = function _callMutatorMethodAsync(name, args, options = {}) {
 
   // For two out of three mutator methods, the first argument is a selector
@@ -711,6 +549,13 @@ function addValidator(collection, allowOrDeny, options) {
   Object.keys(options).forEach((key) => {
     if (!validKeysRegEx.test(key))
       throw new Error(allowOrDeny + ": Invalid key: " + key);
+
+    // TODO deprecated async config on future versions
+    const isAsyncKey = key.includes('Async');
+    if (isAsyncKey) {
+      const syncKey = key.replace('Async', '');
+      console.warn(allowOrDeny + `: The "${key}" key is deprecated. Use "${syncKey}" instead.`);
+    }
   });
 
   collection._restricted = true;
@@ -740,7 +585,9 @@ function addValidator(collection, allowOrDeny, options) {
           options.transform
         );
       }
-      collection._validators[name][allowOrDeny].push(options[name]);
+      const isAsyncName = name.includes('Async');
+      const validatorSyncName = isAsyncName ? name.replace('Async', '') : name;
+      collection._validators[validatorSyncName][allowOrDeny].push(options[name]);
     }
   });
 

packages/ddp-client/package.js

@@ -67,4 +67,5 @@ Package.onTest((api) => {
   api.addFiles("test/async_stubs/client.js", "client");
   api.addFiles("test/async_stubs/server_setup.js", "server");
   api.addFiles("test/livedata_callAsync_tests.js");
+  api.addFiles("test/allow_deny_setup.js");
 });

packages/ddp-client/test/allow_deny_setup.js

@@ -0,0 +1,17 @@
+export const FlickerCollectionName = `allow_deny_flicker`;
+export const FlickerCollection = new Mongo.Collection(FlickerCollectionName);
+
+if (Meteor.isServer) {
+  FlickerCollection.allow({
+    insert: () => true,
+    update: () => true,
+    remove: () => true,
+    insertAsync: () => true,
+    updateAsync: () => true,
+    removeAsync: () => true,
+  });
+
+  Meteor.publish(`pub-${FlickerCollectionName}`, function() {
+    return FlickerCollection.find();
+  });
+}

packages/ddp-client/test/livedata_tests.js

@@ -1,5 +1,7 @@
-import { DDP } from '../common/namespace.js';
+import { Meteor } from 'meteor/meteor';
 import { Connection } from '../common/livedata_connection.js';
+import { DDP } from '../common/namespace.js';
+import { FlickerCollection, FlickerCollectionName } from './allow_deny_setup.js';
 
 const callWhenSubReady = async (subName, handle, cb = () => {}) => {
   let control = 0;
@@ -1295,6 +1297,66 @@ if (Meteor.isClient) {
   });
 }
 
+if (Meteor.isClient) {
+  testAsyncMulti('livedata - allow/deny - no flicker with isomorphic calls', [
+    async function(test, expect) {
+      const docId = await FlickerCollection.insertAsync({
+        value: ['initial'],
+        test: test.runId()
+      });
+
+      let changeCount = 0;
+      const messages = [];
+
+      const handle = await FlickerCollection.find({ _id: docId }).observeChanges({
+        added(id, fields) {
+          messages.push(['added', id, fields]);
+        },
+        changed(id, fields) {
+          changeCount++;
+          messages.push(['changed', id, fields]);
+          
+          if (changeCount > 1) {
+            test.fail('Multiple changes detected - flicker occurred');
+          }
+
+          test.equal(fields.value.length, 2);
+          test.isTrue(fields.value.includes('updated'));
+        }
+      });
+
+      const sub = Meteor.subscribe(`pub-${FlickerCollectionName}`);
+      
+      await new Promise(resolve => {
+        const checkReady = setInterval(() => {
+          console.log('sub.ready()', sub.ready());
+          if (sub.ready()) {
+            clearInterval(checkReady);
+            resolve();
+          }
+        }, 10);
+      });
+
+      await FlickerCollection.updateAsync(docId, {
+        $addToSet: {
+          value: 'updated'
+        }
+      });
+
+      await Meteor._sleepForMs(200);
+
+      handle.stop();
+      sub.stop();
+      
+      test.equal(changeCount, 1, 'Expected exactly one change notification');
+      
+      test.equal(messages.length, 2);
+      test.equal(messages[0][0], 'added');
+      test.equal(messages[1][0], 'changed');
+    }
+  ]);
+}
+
 // TODO [FIBERS] - check if this still makes sense to have
 
 //  Tinytest.addAsync('livedata - isAsync call', async function (test) {

packages/mongo/tests/allow_tests.js

@@ -337,22 +337,22 @@ if (Meteor.isClient) {
         },{
           returnServerResultPromise: true,
         });
-       await restrictedCollectionForFetchTest
-         .updateAsync(
-           fetchId,
-           { $set: { updated: true } },
-           {
-             returnServerResultPromise: true,
-           }
-         )
-         .catch(
-           expect(function(err) {
-             test.equal(
-               err.reason,
-               'Test: Fields in doc: _id,field1,field2,field3'
-             );
-           })
-         );
+        await restrictedCollectionForFetchTest
+          .updateAsync(
+            fetchId,
+            { $set: { updated: true } },
+            {
+              returnServerResultPromise: true,
+            }
+          )
+          .catch(
+            expect(function(err) {
+              test.equal(
+                err.reason,
+                'Test: Fields in doc: _id,field1,field2,field3'
+              );
+            })
+          );
 
         await restrictedCollectionForFetchTest
           .removeAsync(fetchId, {
@@ -732,8 +732,8 @@ if (Meteor.isClient) {
       ]);
     })();
 
-    
-      [restrictedCollectionDefaultInsecure, restrictedCollectionDefaultSecure].forEach(
+
+    [restrictedCollectionDefaultInsecure, restrictedCollectionDefaultSecure].forEach(
       function(collection) {
         var canUpdateId, canRemoveId;
 
@@ -874,6 +874,7 @@ if (Meteor.isClient) {
                 { returnServerResultPromise: true }
               )
               .then(async function(res) {
+                console.log('res', res);
                 test.equal(res, 0);
                 // nothing has changed
                 test.equal(await collection.find().countAsync(), 3);
@@ -1043,7 +1044,7 @@ if (Meteor.isClient) {
     );
     testAsyncMulti(
       'collection - restricted collection allows client-side id, ' +
-        idGeneration,
+      idGeneration,
       [
         async function(test, expect) {
           var self = this;
@@ -1202,6 +1203,7 @@ Tinytest.addAsync(
           error ? reject(error) : resolve(result)
         );
       });
+      console.log('id', id);
       await new Promise((resolve, reject) => {
         AllowAsyncValidateCollection.update(
           id,
@@ -1223,88 +1225,181 @@ Tinytest.addAsync(
   }
 );
 
-function configAllAsyncAllowDeny(collection, configType = 'allow', enabled) {
-  collection[configType]({
-    async insertAsync(selector, doc) {
-      if (doc.force) return true;
-      await Meteor._sleepForMs(100);
-      return enabled;
-    },
-    async updateAsync() {
-      await Meteor._sleepForMs(100);
-      return enabled;
-    },
-    async removeAsync() {
-      await Meteor._sleepForMs(100);
-      return enabled;
-    },
-  });
+function configAllAllowDeny(collection, configType = 'allow', enabled, isAsync = true) {
+  const handler = isAsync
+    ? {
+      async insertAsync(selector, doc) {
+        if (doc.force) return configType === 'allow';
+        await Meteor._sleepForMs(100);
+        return enabled;
+      },
+      async updateAsync() {
+        await Meteor._sleepForMs(100);
+        return enabled;
+      },
+      async removeAsync() {
+        await Meteor._sleepForMs(100);
+        return enabled;
+      },
+    }
+    : {
+      insert(selector, doc) {
+        if (doc.force) return configType === 'allow';
+        return enabled;
+      },
+      update() {
+        return enabled;
+      },
+      remove() {
+        return enabled;
+      },
+    };
+
+  collection[configType](handler);
 }
 
-async function runAllAsyncExpect(test, collection, allow) {
+async function runAllExpect(test, collection, allow, isAsync = true) {
   let id;
-  /* async tests */
+
+  const resolveSyncCallback = (resolve, reject) =>
+    (error, result) => {
+    if (error) {
+      reject(error);
+      return;
+    }
+    resolve(result);
+  };
+  const methods = isAsync
+    ? {
+      insert: async (doc) => await collection.insertAsync(doc),
+      update: async (id, modifier) => await collection.updateAsync(id, modifier),
+      remove: async (id) => await collection.removeAsync(id),
+    }
+    : {
+      insert: (doc) =>
+        new Promise((resolve, reject) =>
+          collection.insert(doc, resolveSyncCallback(resolve, reject))
+        ),
+      update: (id, modifier) =>
+        new Promise((resolve, reject) =>
+          collection.update(id, modifier, resolveSyncCallback(resolve, reject))
+        ),
+      remove: (id) =>
+        new Promise((resolve, reject) =>
+          collection.remove(id, resolveSyncCallback(resolve, reject))
+        ),
+    };
+
   try {
-    id = await collection.insertAsync({ num: 2 });
+    id = await methods.insert({ num: 2 });
     test.isTrue(allow);
   } catch (e) {
     test.isTrue(!allow);
   }
+
   try {
-    id = await collection.insertAsync({ force: true });
-    await collection.updateAsync(id, { $set: { num: 22 } });
+    id = await methods.insert({ force: true });
+    await methods.update(id, { $set: { num: 22 } });
     test.isTrue(allow);
   } catch (e) {
     test.isTrue(!allow);
   }
+
   try {
-    await collection.removeAsync(id);
+    id = await methods.insert({ force: true });
+    await methods.remove(id);
     test.isTrue(allow);
   } catch (e) {
     test.isTrue(!allow);
   }
 }
 
-var AllowDenyAsyncRulesCollections = {};
+const AllowDenyRulesCollections = {};
 
-testAsyncMulti("collection - async definitions on allow/deny rules", [
-  async function (test) {
-    AllowDenyAsyncRulesCollections.allowed =
-      AllowDenyAsyncRulesCollections.allowed ||
-      new Mongo.Collection(`allowdeny-async-rules-allowed`);
-    if (Meteor.isServer) {
-      await AllowDenyAsyncRulesCollections.allowed.removeAsync();
-    }
+function createAllowDenyRulesTest(collections, isAsync = true) {
+  return [
+    async function (test) {
+      const collectionName = `allowdeny-${isAsync ? "async" : "sync"}-rules-noRules`;
+      collections.noRules =
+        collections.noRules || new Mongo.Collection(collectionName);
+      if (Meteor.isServer) {
+        await collections.noRules.removeAsync();
+      }
 
-    configAllAsyncAllowDeny(AllowDenyAsyncRulesCollections.allowed, 'allow', true);
-    if (Meteor.isClient) {
-      await runAllAsyncExpect(test, AllowDenyAsyncRulesCollections.allowed, true);
-    }
-  },
-  async function (test) {
-    AllowDenyAsyncRulesCollections.notAllowed =
-      AllowDenyAsyncRulesCollections.notAllowed ||
-      new Mongo.Collection(`allowdeny-async-rules-notAllowed`);
-    if (Meteor.isServer) {
-      await AllowDenyAsyncRulesCollections.notAllowed.removeAsync();
-    }
+      if (Meteor.isClient) {
+        await runAllExpect(test, collections.noRules, collections.noRules._isInsecure(), isAsync);
+      }
+    },
+    async function (test) {
+      const collectionName = `allowdeny-${isAsync ? "async" : "sync"}-rules-allowed`;
+      collections.allowed =
+        collections.allowed || new Mongo.Collection(collectionName);
+      if (Meteor.isServer) {
+        await collections.allowed.removeAsync();
+      }
 
-    configAllAsyncAllowDeny(AllowDenyAsyncRulesCollections.notAllowed, 'allow', false);
-    if (Meteor.isClient) {
-      await runAllAsyncExpect(test, AllowDenyAsyncRulesCollections.notAllowed, false);
-    }
-  },
-  async function (test) {
-    AllowDenyAsyncRulesCollections.denied =
-      AllowDenyAsyncRulesCollections.denied ||
-      new Mongo.Collection(`allowdeny-async-rules-denied`);
-    if (Meteor.isServer) {
-      await AllowDenyAsyncRulesCollections.denied.removeAsync();
-    }
+      configAllAllowDeny(collections.allowed, 'allow', true, isAsync);
+      if (Meteor.isClient) {
+        await runAllExpect(test, collections.allowed, true, isAsync);
+      }
+    },
+    async function (test) {
+      const collectionName = `allowdeny-${isAsync ? "async" : "sync"}-rules-notAllowed`;
+      collections.notAllowed =
+        collections.notAllowed || new Mongo.Collection(collectionName);
+      if (Meteor.isServer) {
+        await collections.notAllowed.removeAsync();
+      }
 
-    configAllAsyncAllowDeny(AllowDenyAsyncRulesCollections.denied, 'deny', true);
-    if (Meteor.isClient) {
-      await runAllAsyncExpect(test, AllowDenyAsyncRulesCollections.denied, false);
-    }
-  },
-]);
+      configAllAllowDeny(collections.notAllowed, 'allow', false, isAsync);
+      if (Meteor.isClient) {
+        await runAllExpect(test, collections.notAllowed, false, isAsync);
+      }
+    },
+    async function (test) {
+      const collectionName = `allowdeny-${isAsync ? "async" : "sync"}-rules-denied`;
+      collections.denied =
+        collections.denied || new Mongo.Collection(collectionName);
+      if (Meteor.isServer) {
+        await collections.denied.removeAsync();
+      }
+
+      configAllAllowDeny(collections.denied, 'deny', true, isAsync);
+      if (Meteor.isClient) {
+        await runAllExpect(test, collections.denied, false, isAsync);
+      }
+    },
+    async function (test) {
+      const collectionName = `allowdeny-${isAsync ? "async" : "sync"}-rules-allowThenDeny`;
+      collections.allowThenDeny =
+        collections.allowThenDeny || new Mongo.Collection(collectionName);
+      if (Meteor.isServer) {
+        await collections.allowThenDeny.removeAsync();
+      }
+
+      configAllAllowDeny(collections.allowThenDeny, 'allow', true, isAsync);
+      configAllAllowDeny(collections.allowThenDeny, 'deny', true, isAsync);
+      if (Meteor.isClient) {
+        await runAllExpect(test, collections.allowThenDeny, false, isAsync);
+      }
+    },
+    async function (test) {
+      const collectionName = `allowdeny-${isAsync ? "async" : "sync"}-rules-allowThenNotDenied`;
+      collections.allowThenNotDenied =
+        collections.allowThenNotDenied || new Mongo.Collection(collectionName);
+      if (Meteor.isServer) {
+        await collections.allowThenNotDenied.removeAsync();
+      }
+
+      configAllAllowDeny(collections.allowThenNotDenied, 'allow', true, isAsync);
+      configAllAllowDeny(collections.allowThenNotDenied, 'deny', false, isAsync);
+      if (Meteor.isClient) {
+        await runAllExpect(test, collections.allowThenNotDenied, true, isAsync);
+      }
+    },
+  ];
+}
+
+testAsyncMulti("collection - async definitions on allow/deny rules", createAllowDenyRulesTest(AllowDenyRulesCollections, true));
+
+testAsyncMulti("collection - sync definitions on allow/deny rules", createAllowDenyRulesTest(AllowDenyRulesCollections, false));

v3-docs/docs/api/collections.md

@@ -526,8 +526,8 @@ restrictions. That includes methods that are called with `Meteor.call`
 relying on `allow` and `deny`.
 
 You can call `allow` as many times as you like, and each call can
-include any combination of `insert`/`insertAsync`, `update`/`updateAsync`,
-and `remove`/`removeAsync` functions. The functions should return `true`
+include any combination of `insert`, `update`,
+and `remove` functions. The functions should return `true`
 if they think the operation should be allowed. Otherwise they should
 return `false`, or nothing at all (`undefined`). In that case Meteor
 will continue searching through any other `allow` rules on the collection.
@@ -536,18 +536,18 @@ The available callbacks are:
 
 ### Callbacks
 
-- `insert(userId, doc)`/`insertAsync(userId, doc)` - The user `userId` wants to insert the
+- `insert(userId, doc)` - The user `userId` wants to insert the
   document `doc` into the collection. Return `true` if this should be
-  allowed.
+  allowed. Supports async validations.
 
   `doc` will contain the `_id` field if one was explicitly set by the client, or
   if there is an active `transform`. You can use this to prevent users from
   specifying arbitrary `_id` fields.
 
-- `update(userId, doc, fieldNames, modifier)`/`updateAsync(userId, doc, fieldNames, modifier)` - The user `userId`
+- `update(userId, doc, fieldNames, modifier)` - The user `userId`
   wants to update a document `doc` in the database. (`doc` is the
   current version of the document from the database, without the
-  proposed update.) Return `true` to permit the change.
+  proposed update.) Return `true` to permit the change. Supports async validations.
 
   `fieldNames` is an array of the (top-level) fields in `doc` that the
   client wants to modify, for example
@@ -562,8 +562,8 @@ The available callbacks are:
   \$-modifiers, the request will be denied without checking the `allow`
   functions.
 
-- `remove(userId, doc)`/`removeAsync(userId, doc)` - the user `userId` wants to remove `doc` from the database. Return
-  `true` to permit this.
+- `remove(userId, doc)` - the user `userId` wants to remove `doc` from the database. Return
+  `true` to permit this. Supports async validations.
 
 
 When calling `update`/`updateAsync` or `remove`/`removeAsync` Meteor will by default fetch the
@@ -593,28 +593,12 @@ Posts.allow({
     return doc.owner === userId;
   },
 
-  remove(userId, doc) {
+  async remove(userId, doc) {
+    // Any custom async validation is supported
+    await Meteor.sleep(100);
     // Can only remove your own documents.
     return doc.owner === userId;
   },
-  
-  async insertAsync(userId, doc) {
-    // Any custom async validation is supported
-    const allowed = await allowInsertAsync(userId, doc);
-    return userId && allowed;
-  },
-
-  async updateAsync(userId, doc, fields, modifier) {
-    // Any custom async validation is supported
-    const allowed = await allowUpdateAsync(userId, doc);
-    return userId && allowed;
-  },
-
-  async removeAsync(userId, doc) {
-    // Any custom async validation is supported
-    const allowed = await allowRemoveAsync(userId, doc);
-    return userId && allowed;
-  },
 
   fetch: ["owner"],
 });
@@ -625,22 +609,12 @@ Posts.deny({
     return _.contains(fields, "owner");
   },
 
-  remove(userId, doc) {
+  async remove(userId, doc) {
+    // Any custom async validation is supported
+    await Meteor.sleep(100);
     // Can't remove locked documents.
     return doc.locked;
   },
-  
-  async updateAsync(userId, doc, fields, modifier) {
-    // Any custom async validation is supported
-    const denied = await denyUpdateAsync(userId, doc);
-    return userId && denied;
-  },
-
-  async removeAsync(userId, doc) {
-    // Any custom async validation is supported
-    const denied = await denyRemoveAsync(userId, doc);
-    return userId && denied;
-  },
 
   fetch: ["locked"], // No need to fetch `owner`
 });