From 6a1a73e285931c036f9128c1f1899efe1db8031e Mon Sep 17 00:00:00 2001 From: Leonardo Venturini Date: Mon, 25 Nov 2024 15:25:41 -0400 Subject: [PATCH 01/10] add allow deny test --- packages/ddp-client/package.js | 1 + packages/ddp-client/test/allow_deny_setup.js | 17 ++++++ packages/ddp-client/test/livedata_tests.js | 64 +++++++++++++++++++- 3 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 packages/ddp-client/test/allow_deny_setup.js diff --git a/packages/ddp-client/package.js b/packages/ddp-client/package.js index e0cb8bffe5..7ccb3cbec4 100644 --- a/packages/ddp-client/package.js +++ b/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"); }); diff --git a/packages/ddp-client/test/allow_deny_setup.js b/packages/ddp-client/test/allow_deny_setup.js new file mode 100644 index 0000000000..ed0eca4b25 --- /dev/null +++ b/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(); + }); +} \ No newline at end of file diff --git a/packages/ddp-client/test/livedata_tests.js b/packages/ddp-client/test/livedata_tests.js index d297aafe58..1b57ecd4b3 100644 --- a/packages/ddp-client/test/livedata_tests.js +++ b/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) { From 754d0060d61d6f356f93abafe9d6fed28d1dcf4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nacho=20Codo=C3=B1er?= Date: Thu, 28 Nov 2024 18:35:43 +0100 Subject: [PATCH 02/10] ensure allow/deny rules describe uniquely operation configs by deprecating async distinction --- packages/allow-deny/allow-deny.js | 53 +- packages/mongo/tests/allow_tests.js | 1272 +++------------------------ 2 files changed, 143 insertions(+), 1182 deletions(-) diff --git a/packages/allow-deny/allow-deny.js b/packages/allow-deny/allow-deny.js index faf7f774c9..d6e58f1f8b 100644 --- a/packages/allow-deny/allow-deny.js +++ b/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); })) { @@ -408,13 +408,12 @@ CollectionPrototype._validatedUpdateAsync = async function( }); } - const doc = await self._collection.findOneAsync(selector, findOptions); - if (!doc) // none satisfied! - return 0; + + const doc = await self._collection.findOneAsync(selector, findOptions) || {}; // 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 +423,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, @@ -436,6 +436,9 @@ CollectionPrototype._validatedUpdateAsync = async function( throw new Meteor.Error(403, "Access denied"); } + if (!doc) // none satisfied! + return 0; + options._forbidReplace = true; // Back when we supported arbitrary client-provided selectors, we actually @@ -567,26 +570,27 @@ CollectionPrototype._validatedRemoveAsync = async function(userId, selector) { }); } - const doc = await self._collection.findOneAsync(selector, findOptions); - if (!doc) - return 0; + const doc = await self._collection.findOneAsync(selector, findOptions) || {}; // 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); })) { throw new Meteor.Error(403, "Access denied"); } + if (!doc) + return 0; + // 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 @@ -713,6 +717,15 @@ function addValidator(collection, allowOrDeny, options) { throw new Error(allowOrDeny + ": Invalid key: " + key); }); + Object.keys(options).forEach((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 +753,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]); } }); diff --git a/packages/mongo/tests/allow_tests.js b/packages/mongo/tests/allow_tests.js index 212a6f6e23..777d9f6b02 100644 --- a/packages/mongo/tests/allow_tests.js +++ b/packages/mongo/tests/allow_tests.js @@ -1,1168 +1,5 @@ import has from 'lodash.has'; -if (Meteor.isServer) { - // Set up allow/deny rules for test collections - - var allowCollections = {}; - - // We create the collections in the publisher (instead of using a method or - // something) because if we made them with a method, we'd need to follow the - // method with some subscribes, and it's possible that the method call would - // be delayed by a wait method and the subscribe messages would be sent before - // it and fail due to the collection not yet existing. So we are very hacky - // and use a publish. - Meteor.publish("allowTests", function (nonce, idGeneration) { - check(nonce, String); - check(idGeneration, String); - var cursors = []; - var needToConfigure; - - // helper for defining a collection. we are careful to create just one - // Mongo.Collection even if the sub body is rerun, by caching them. - var defineCollection = function(name, insecure, transform) { - var fullName = name + idGeneration + nonce; - - var collection; - if (has(allowCollections, fullName)) { - collection = allowCollections[fullName]; - if (needToConfigure === true) - throw new Error("collections inconsistently exist"); - needToConfigure = false; - } else { - collection = new Mongo.Collection( - fullName, {idGeneration: idGeneration, transform: transform}); - allowCollections[fullName] = collection; - if (needToConfigure === false) - throw new Error("collections inconsistently don't exist"); - needToConfigure = true; - collection._insecure = insecure; - var m = {}; - m["clear-collection-" + fullName] = async function() { - await collection.removeAsync({}, { returnServerResultPromise: true }); - }; - Meteor.methods(m); - } - - cursors.push(collection.find()); - return collection; - }; - - var insecureCollection = defineCollection( - "collection-insecure", true /*insecure*/); - // totally locked down collection - var lockedDownCollection = defineCollection( - "collection-locked-down", false /*insecure*/); - // restricted collection with same allowed modifications, both with and - // without the `insecure` package - var restrictedCollectionDefaultSecure = defineCollection( - "collection-restrictedDefaultSecure", false /*insecure*/); - var restrictedCollectionDefaultInsecure = defineCollection( - "collection-restrictedDefaultInsecure", true /*insecure*/); - var restrictedCollectionForUpdateOptionsTest = defineCollection( - "collection-restrictedForUpdateOptionsTest", true /*insecure*/); - var restrictedCollectionForPartialAllowTest = defineCollection( - "collection-restrictedForPartialAllowTest", true /*insecure*/); - var restrictedCollectionForPartialDenyTest = defineCollection( - "collection-restrictedForPartialDenyTest", true /*insecure*/); - var restrictedCollectionForFetchTest = defineCollection( - "collection-restrictedForFetchTest", true /*insecure*/); - var restrictedCollectionForFetchAllTest = defineCollection( - "collection-restrictedForFetchAllTest", true /*insecure*/); - var restrictedCollectionWithTransform = defineCollection( - "withTransform", false, function (doc) { - return doc.a; - }); - var restrictedCollectionForInvalidTransformTest = defineCollection( - "collection-restrictedForInvalidTransform", false /*insecure*/); - var restrictedCollectionForClientIdTest = defineCollection( - "collection-restrictedForClientIdTest", false /*insecure*/); - - if (needToConfigure) { - restrictedCollectionWithTransform.allow({ - insertAsync: function (userId, doc) { - return doc.foo === "foo"; - }, - updateAsync: function (userId, doc) { - return doc.foo === "foo"; - }, - removeAsync: function (userId, doc) { - return doc.bar === "bar"; - } - }); - restrictedCollectionWithTransform.allow({ - // transform: null means that doc here is the top level, not the 'a' - // element. - transform: null, - insertAsync: function (userId, doc) { - return !!doc.topLevelField; - }, - updateAsync: function (userId, doc) { - return !!doc.topLevelField; - } - }); - restrictedCollectionForInvalidTransformTest.allow({ - // transform must return an object which is not a mongo id - transform: function (doc) { return doc._id; }, - insert: function () { return true; } - }); - restrictedCollectionForClientIdTest.allow({ - // This test just requires the collection to trigger the restricted - // case. - insert: function () { return true; }, - insertAsync: function () { return true; } - }); - - // two calls to allow to verify that either validator is sufficient. - var allows = [{ - insertAsync: function(userId, doc) { - return doc.canInsert; - }, - updateAsync: function(userId, doc) { - return doc.canUpdate; - }, - removeAsync: function (userId, doc) { - return doc.canRemove; - } - }, { - insertAsync: function(userId, doc) { - return doc.canInsert2; - }, - updateAsync: function(userId, doc, fields, modifier) { - return -1 !== fields.indexOf('canUpdate2'); - }, - removeAsync: function(userId, doc) { - return doc.canRemove2; - } - }]; - - // two calls to deny to verify that either one blocks the change. - var denies = [{ - insertAsync: function(userId, doc) { - return doc.cantInsert; - }, - removeAsync: function (userId, doc) { - return doc.cantRemove; - } - }, { - insertAsync: function(userId, doc) { - // Don't allow explicit ID to be set by the client. - return has(doc, '_id'); - }, - updateAsync: function(userId, doc, fields, modifier) { - return -1 !== fields.indexOf('verySecret'); - } - }]; - - [ - restrictedCollectionDefaultSecure, - restrictedCollectionDefaultInsecure, - restrictedCollectionForUpdateOptionsTest - ].forEach(function (collection) { - allows.forEach(function (allow) { - collection.allow(allow); - }); - denies.forEach(function (deny) { - collection.deny(deny); - }); - }); - - // just restrict one operation so that we can verify that others - // fail - restrictedCollectionForPartialAllowTest.allow({ - insert: function() {} - }); - restrictedCollectionForPartialDenyTest.deny({ - insert: function() {} - }); - - // verify that we only fetch the fields specified - we should - // be fetching just field1, field2, and field3. - restrictedCollectionForFetchTest.allow({ - insertAsync: function() { return true; }, - updateAsync: function(userId, doc) { - // throw fields in doc so that we can inspect them in test - throw new Meteor.Error( - 999, "Test: Fields in doc: " + Object.keys(doc).sort().join(',')); - }, - removeAsync: function(userId, doc) { - // throw fields in doc so that we can inspect them in test - throw new Meteor.Error( - 999, "Test: Fields in doc: " + Object.keys(doc).sort().join(',')); - }, - fetch: ['field1'] - }); - restrictedCollectionForFetchTest.allow({ - fetch: ['field2'] - }); - restrictedCollectionForFetchTest.deny({ - fetch: ['field3'] - }); - - // verify that not passing fetch to one of the calls to allow - // causes all fields to be fetched - restrictedCollectionForFetchAllTest.allow({ - insertAsync: function() { return true; }, - updateAsync: function(userId, doc) { - // throw fields in doc so that we can inspect them in test - throw new Meteor.Error( - 999, "Test: Fields in doc: " + Object.keys(doc).sort().join(',')); - }, - removeAsync: function(userId, doc) { - // throw fields in doc so that we can inspect them in test - throw new Meteor.Error( - 999, "Test: Fields in doc: " + Object.keys(doc).sort().join(',')); - }, - fetch: ['field1'] - }); - restrictedCollectionForFetchAllTest.allow({ - updateAsync: function() { return true; } - }); - } - - return cursors; - }); -} - -if (Meteor.isClient) { - ['STRING', 'MONGO'].forEach(function (idGeneration) { - // Set up a bunch of test collections... on the client! They match the ones - // created by setUpAllowTestsCollections. - - var nonce = Random.id(); - // Tell the server to make, configure, and publish a set of collections unique - // to our test run. Since the method does not unblock, this will complete - // running on the server before anything else happens. - Meteor.subscribe('allowTests', nonce, idGeneration); - - // helper for defining a collection, subscribing to it, and defining - // a method to clear it - var defineCollection = function(name, transform) { - var fullName = name + idGeneration + nonce; - var collection = new Mongo.Collection( - fullName, {idGeneration: idGeneration, transform: transform}); - - collection.callClearMethod = async function () { - await Meteor.callAsync('clear-collection-' + fullName); - }; - collection.unnoncedName = name + idGeneration; - return collection; - }; - - // totally insecure collection - var insecureCollection = defineCollection("collection-insecure"); - - // totally locked down collection - var lockedDownCollection = defineCollection("collection-locked-down"); - - // restricted collection with same allowed modifications, both with and - // without the `insecure` package - var restrictedCollectionDefaultSecure = defineCollection( - "collection-restrictedDefaultSecure"); - var restrictedCollectionDefaultInsecure = defineCollection( - "collection-restrictedDefaultInsecure"); - var restrictedCollectionForUpdateOptionsTest = defineCollection( - "collection-restrictedForUpdateOptionsTest"); - var restrictedCollectionForPartialAllowTest = defineCollection( - "collection-restrictedForPartialAllowTest"); - var restrictedCollectionForPartialDenyTest = defineCollection( - "collection-restrictedForPartialDenyTest"); - var restrictedCollectionForFetchTest = defineCollection( - "collection-restrictedForFetchTest"); - var restrictedCollectionForFetchAllTest = defineCollection( - "collection-restrictedForFetchAllTest"); - var restrictedCollectionWithTransform = defineCollection( - "withTransform", function (doc) { - return doc.a; - }); - var restrictedCollectionForInvalidTransformTest = defineCollection( - "collection-restrictedForInvalidTransform"); - var restrictedCollectionForClientIdTest = defineCollection( - "collection-restrictedForClientIdTest"); - - // test that if allow is called once then the collection is - // restricted, and that other mutations aren't allowed - testAsyncMulti('collection - partial allow, ' + idGeneration, [ - async function(test, expect) { - try { - await restrictedCollectionForPartialAllowTest.updateAsync( - 'foo', - { $set: { updated: true } }, - { - returnServerResultPromise: true, - } - ); - } catch (err) { - test.equal(err.error, 403); - } - }, - ]); - - // test that if deny is called once then the collection is - // restricted, and that other mutations aren't allowed - testAsyncMulti('collection - partial deny, ' + idGeneration, [ - async function(test, expect) { - try { - await restrictedCollectionForPartialDenyTest.updateAsync( - 'foo', - { - $set: { updated: true }, - }, - { - returnServerResultPromise: true, - } - ); - } catch (err) { - test.equal(err.error, 403); - } - }, - ]); - - - // test that we only fetch the fields specified - testAsyncMulti('collection - fetch, ' + idGeneration, [ - async function(test, expect) { - var fetchId = await restrictedCollectionForFetchTest.insertAsync({ - field1: 1, - field2: 1, - field3: 1, - field4: 1, - },{ - returnServerResultPromise: true, - }); - var fetchAllId = await restrictedCollectionForFetchAllTest.insertAsync({ - field1: 1, - field2: 1, - field3: 1, - field4: 1, - },{ - 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 - .removeAsync(fetchId, { - returnServerResultPromise: true, - }) - .catch( - expect(function(err) { - test.equal( - err.reason, - 'Test: Fields in doc: _id,field1,field2,field3' - ); - }) - ); - - await restrictedCollectionForFetchAllTest - .updateAsync( - fetchAllId, - { $set: { updated: true } }, - { - returnServerResultPromise: true, - } - ) - .catch( - expect(function(err) { - test.equal( - err.reason, - 'Test: Fields in doc: _id,field1,field2,field3,field4' - ); - }) - ); - await restrictedCollectionForFetchAllTest - .removeAsync(fetchAllId, { - returnServerResultPromise: true, - }) - .catch( - expect(function(err) { - test.equal( - err.reason, - 'Test: Fields in doc: _id,field1,field2,field3,field4' - ); - }) - ); - }, - ]); - - (function() { - testAsyncMulti('collection - restricted factories ' + idGeneration, [ - async function(test, expect) { - await restrictedCollectionWithTransform.callClearMethod(); - test.equal( - await restrictedCollectionWithTransform.find().countAsync(), - 0 - ); - }, - async function(test, expect) { - var self = this; - await restrictedCollectionWithTransform - .insertAsync( - { - a: { foo: 'foo', bar: 'bar', baz: 'baz' }, - }, - { - returnServerResultPromise: true, - } - ) - .then( - expect(function(res) { - test.isTrue(res); - self.item1 = res; - }) - ); - await restrictedCollectionWithTransform - .insertAsync( - { - a: { foo: 'foo', bar: 'quux', baz: 'quux' }, - b: 'potato', - }, - { - returnServerResultPromise: true, - } - ) - .then( - expect(function(res) { - test.isTrue(res); - self.item2 = res; - }) - ); - await restrictedCollectionWithTransform - .insertAsync( - { - a: { foo: 'adsfadf', bar: 'quux', baz: 'quux' }, - b: 'potato', - }, - { - returnServerResultPromise: true, - } - ) - .catch( - expect(function(e, res) { - test.isTrue(e); - }) - ); - await restrictedCollectionWithTransform - .insertAsync( - { - a: { foo: 'bar' }, - topLevelField: true, - }, - { - returnServerResultPromise: true, - } - ) - .then( - expect(function(res) { - test.isTrue(res); - self.item3 = res; - }) - ); - }, - async function(test, expect) { - var self = this; - // This should work, because there is an update allow for things with - // topLevelField. - await restrictedCollectionWithTransform - .updateAsync( - self.item3, - { $set: { xxx: true } }, - { - returnServerResultPromise: true, - } - ) - .then( - expect(function(res) { - test.equal(1, res); - }) - ); - }, - async function(test, expect) { - var self = this; - test.equal( - await restrictedCollectionWithTransform.findOneAsync(self.item1), - { - _id: self.item1, - foo: 'foo', - bar: 'bar', - baz: 'baz', - } - ); - await restrictedCollectionWithTransform - .removeAsync(self.item1, { - returnServerResultPromise: true, - }) - .then( - expect(function(res) { - test.isTrue(res); - }) - ); - await restrictedCollectionWithTransform - .removeAsync(self.item2, { - returnServerResultPromise: true, - }) - .catch( - expect(function(e) { - test.isTrue(e); - }) - ); - }, - ]); - })(); - - testAsyncMulti('collection - insecure, ' + idGeneration, [ - async function(test, expect) { - await insecureCollection.callClearMethod(); - test.equal(await insecureCollection.find().countAsync(), 0); - }, - async function(test, expect) { - let idThen; - var id = await insecureCollection - .insertAsync( - { foo: 'bar' }, - { - returnServerResultPromise: true, - } - ) - .then(async function(res) { - idThen = res; - test.equal(await insecureCollection.find(res).countAsync(), 1); - test.equal((await insecureCollection.findOneAsync(res)).foo, 'bar'); - return res; - }); - test.equal(idThen, id); - test.equal(await insecureCollection.find(id).countAsync(), 1); - test.equal((await insecureCollection.findOneAsync(id)).foo, 'bar'); - }, - ]); - - testAsyncMulti('collection - locked down, ' + idGeneration, [ - async function(test, expect) { - await lockedDownCollection.callClearMethod(); - test.equal(await lockedDownCollection.find().countAsync(), 0); - }, - async function(test, expect) { - await lockedDownCollection - .insertAsync( - { foo: 'bar' }, - { - returnServerResultPromise: true, - } - ) - .catch(async function(err, res) { - test.equal(err.error, 403); - test.equal(await lockedDownCollection.find().countAsync(), 0); - }); - }, - ]); - - (function() { - var collection = restrictedCollectionForUpdateOptionsTest; - var id1, id2; - testAsyncMulti('collection - update options, ' + idGeneration, [ - // init - async function(test, expect) { - await collection.callClearMethod().then(async function() { - test.equal(await collection.find().countAsync(), 0); - }); - }, - // put a few objects - async function(test, expect) { - var doc = { canInsert: true, canUpdate: true }; - id1 = await collection.insertAsync(doc); - id2 = await collection.insertAsync(doc); - await collection.insertAsync(doc); - await collection - .insertAsync(doc, { returnServerResultPromise: true }) - .then(async function(res) { - test.equal(await collection.find().countAsync(), 4); - }); - }, - // update by id - async function(test, expect) { - await collection - .updateAsync( - id1, - { $set: { updated: true } }, - { returnServerResultPromise: true } - ) - .then(async function(res) { - test.equal(res, 1); - test.equal( - await collection.find({ updated: true }).countAsync(), - 1 - ); - }); - }, - // update by id in an object - async function(test, expect) { - await collection - .updateAsync( - { _id: id2 }, - { $set: { updated: true } }, - { returnServerResultPromise: true } - ) - .then(async function(res) { - test.equal(res, 1); - test.equal( - await collection.find({ updated: true }).countAsync(), - 2 - ); - }); - }, - // update with replacement operator not allowed, and has nice error. - async function(test, expect) { - await collection - .updateAsync( - { _id: id2 }, - { _id: id2, updated: true }, - { returnServerResultPromise: true } - ) - .catch(async function(err) { - test.equal(err.error, 403); - test.matches(err.reason, /In a restricted/); - // unchanged - test.equal( - await collection.find({ updated: true }).countAsync(), - 2 - ); - }); - }, - // upsert not allowed, and has nice error. - async function(test, expect) { - await collection - .updateAsync( - { _id: id2 }, - { $set: { upserted: true } }, - { upsert: true, returnServerResultPromise: true } - ) - .catch(async function(err) { - test.equal(err.error, 403); - test.matches(err.reason, /in a restricted/); - test.equal( - await collection.find({ upserted: true }).countAsync(), - 0 - ); - }); - }, - // update with rename operator not allowed, and has nice error. - async function(test, expect) { - await collection - .updateAsync( - { _id: id2 }, - { $rename: { updated: 'asdf' } }, - { returnServerResultPromise: true } - ) - .catch(async function(err) { - test.equal(err.error, 403); - test.matches(err.reason, /not allowed/); - // unchanged - test.equal( - await collection.find({ updated: true }).countAsync(), - 2 - ); - }); - }, - // update method with a non-ID selector is not allowed - async function(test, expect) { - // We shouldn't even send the method... - await test.throwsAsync(async function() { - await collection.updateAsync( - { updated: { $exists: false } }, - { $set: { updated: true } } - ); - }); - // ... but if we did, the server would reject it too. - await Meteor.callAsync( - '/' + collection._name + '/updateAsync', - { updated: { $exists: false } }, - { $set: { updated: true } } - ).catch(async function(err, res) { - test.equal(err.error, 403); - // unchanged - test.equal( - await collection.find({ updated: true }).countAsync(), - 2 - ); - }); - }, - // make sure it doesn't think that {_id: 'foo', something: else} is ok. - async function(test, expect) { - await test.throwsAsync(async function() { - await collection.updateAsync( - { _id: id1, updated: { $exists: false } }, - { $set: { updated: true } } - ); - }); - }, - // remove method with a non-ID selector is not allowed - async function(test, expect) { - // We shouldn't even send the method... - await test.throwsAsync(async function() { - await collection.removeAsync({ updated: true }); - }); - // ... but if we did, the server would reject it too. - await Meteor.callAsync( - '/' + collection._name + '/removeAsync', - { - updated: true, - } - ).catch(async function(err) { - test.equal(err.error, 403); - // unchanged - test.equal( - await collection.find({ updated: true }).countAsync(), - 2 - ); - }); - }, - ]); - })(); - - - [restrictedCollectionDefaultInsecure, restrictedCollectionDefaultSecure].forEach( - function(collection) { - var canUpdateId, canRemoveId; - - testAsyncMulti('collection - ' + collection.unnoncedName, [ - // init - async function(test, expect) { - await collection.callClearMethod().then(async function() { - test.equal(await collection.find().countAsync(), 0); - }); - }, - - // insert with no allows passing. request is denied. - async function(test, expect) { - await collection - .insertAsync({}, { returnServerResultPromise: true }) - .catch(async function(err, res) { - test.equal(err.error, 403); - test.equal(await collection.find().countAsync(), 0); - }); - }, - // insert with one allow and one deny. denied. - async function(test, expect) { - await collection - .insertAsync( - { canInsert: true, cantInsert: true }, - { returnServerResultPromise: true } - ) - .catch(async function(err, res) { - test.equal(err.error, 403); - test.equal(await collection.find().countAsync(), 0); - }); - }, - // insert with one allow and other deny. denied. - async function(test, expect) { - await collection - .insertAsync( - { canInsert: true, _id: Random.id() }, - { returnServerResultPromise: true } - ) - .catch(async function(err) { - test.equal(err.error, 403); - test.equal(await collection.find().countAsync(), 0); - }); - }, - // insert one allow passes. allowed. - async function(test, expect) { - await collection - .insertAsync( - { canInsert: true }, - { returnServerResultPromise: true } - ) - .then(async function(err, res) { - test.equal(await collection.find().countAsync(), 1); - }); - }, - // insert other allow passes. allowed. - // includes canUpdate for later. - async function(test, expect) { - canUpdateId = await collection - .insertAsync( - { canInsert2: true, canUpdate: true }, - { returnServerResultPromise: true } - ) - .then(async function(res) { - test.equal(await collection.find().countAsync(), 2); - return res; - }); - }, - // yet a third insert executes. this one has canRemove and - // cantRemove set for later. - async function(test, expect) { - canRemoveId = await collection - .insertAsync( - { canInsert: true, canRemove: true, cantRemove: true }, - { returnServerResultPromise: true } - ) - .then(async function(res) { - test.equal(await collection.find().countAsync(), 3); - return res; - }); - }, - - // can't update with a non-operator mutation - async function(test, expect) { - await collection - .updateAsync( - canUpdateId, - { newObject: 1 }, - { returnServerResultPromise: true } - ) - .catch(async function(err, res) { - test.equal(err.error, 403); - test.equal(await collection.find().countAsync(), 3); - }); - }, - - // updating dotted fields works as if we are changing their - // top part - async function(test, expect) { - await collection - .updateAsync( - canUpdateId, - { $set: { 'dotted.field': 1 } }, - { returnServerResultPromise: true } - ) - .then(async function(res) { - test.equal(res, 1); - test.equal( - (await collection.findOneAsync(canUpdateId)).dotted.field, - 1 - ); - }); - }, - async function(test, expect) { - await collection - .updateAsync( - canUpdateId, - { $set: { 'verySecret.field': 1 } }, - { returnServerResultPromise: true } - ) - .catch(async function(err, res) { - test.equal(err.error, 403); - test.equal( - await collection - .find({ verySecret: { $exists: true } }) - .countAsync(), - 0 - ); - }); - }, - - // update doesn't do anything if no docs match - async function(test, expect) { - await collection - .updateAsync( - "doesn't exist", - { $set: { updated: true } }, - { returnServerResultPromise: true } - ) - .then(async function(res) { - test.equal(res, 0); - // nothing has changed - test.equal(await collection.find().countAsync(), 3); - test.equal( - await collection.find({ updated: true }).countAsync(), - 0 - ); - }); - }, - // update fails when access is denied trying to set `verySecret` - async function(test, expect) { - await collection - .updateAsync( - canUpdateId, - { $set: { verySecret: true } }, - { returnServerResultPromise: true } - ) - .catch(async function(err) { - test.equal(err.error, 403); - // nothing has changed - test.equal(await collection.find().countAsync(), 3); - test.equal( - await collection.find({ updated: true }).countAsync(), - 0 - ); - }); - }, - // update fails when trying to set two fields, one of which is - // `verySecret` - async function(test, expect) { - await collection - .updateAsync( - canUpdateId, - { $set: { updated: true, verySecret: true } }, - { returnServerResultPromise: true } - ) - .catch(async function(err, res) { - test.equal(err.error, 403); - // nothing has changed - test.equal(await collection.find().countAsync(), 3); - test.equal( - await collection.find({ updated: true }).countAsync(), - 0 - ); - }); - }, - // update fails when trying to modify docs that don't - // have `canUpdate` set - async function(test, expect) { - await collection - .updateAsync( - canRemoveId, - { $set: { updated: true } }, - { returnServerResultPromise: true } - ) - .catch(async function(err, res) { - test.equal(err.error, 403); - // nothing has changed - test.equal(await collection.find().countAsync(), 3); - test.equal( - await collection.find({ updated: true }).countAsync(), - 0 - ); - }); - }, - // update executes when it should - async function(test, expect) { - await collection - .updateAsync( - canUpdateId, - { $set: { updated: true } }, - { returnServerResultPromise: true } - ) - .then(async function(res) { - test.equal(res, 1); - test.equal( - await collection.find({ updated: true }).countAsync(), - 1 - ); - }); - }, - - // remove fails when trying to modify a doc with no `canRemove` set - async function(test, expect) { - await collection - .removeAsync(canUpdateId, { returnServerResultPromise: true }) - .catch(async function(err, res) { - test.equal(err.error, 403); - // nothing has changed - test.equal(await collection.find().countAsync(), 3); - }); - }, - // remove fails when trying to modify an doc with `cantRemove` - // set - async function(test, expect) { - await collection - .removeAsync(canRemoveId, { returnServerResultPromise: true }) - .catch(async function(err, res) { - test.equal(err.error, 403); - // nothing has changed - test.equal(await collection.find().countAsync(), 3); - }); - }, - - // update the doc to remove cantRemove. - async function(test, expect) { - await collection - .updateAsync( - canRemoveId, - { $set: { cantRemove: false, canUpdate2: true } }, - { returnServerResultPromise: true } - ) - .then(async function(res) { - test.equal(res, 1); - test.equal( - await collection.find({ cantRemove: true }).countAsync(), - 0 - ); - }); - }, - - // now remove can remove it. - async function(test, expect) { - await collection - .removeAsync(canRemoveId, { returnServerResultPromise: true }) - .then(async function(res) { - test.equal(res, 1); - // successfully removed - test.equal(await collection.find().countAsync(), 2); - }); - }, - - // try to remove a doc that doesn't exist. see we remove no docs. - async function(test, expect) { - await collection - .removeAsync('some-random-id-that-never-matches', { - returnServerResultPromise: true, - }) - .then(async function(res) { - test.equal(res, 0); - // nothing removed - test.equal(await collection.find().countAsync(), 2); - }); - }, - - // methods can still bypass restrictions - async function(test, expect) { - await collection.callClearMethod().then(async function(err, res) { - // successfully removed - test.equal(await collection.find().countAsync(), 0); - }); - }, - ]); - } - ); - testAsyncMulti( - 'collection - allow/deny transform must return object, ' + idGeneration, - [ - async function(test, expect) { - await restrictedCollectionForInvalidTransformTest - .insertAsync({}, { returnServerResultPromise: true }) - .catch(function(err) { - test.isTrue(err); - }); - }, - ] - ); - testAsyncMulti( - 'collection - restricted collection allows client-side id, ' + - idGeneration, - [ - async function(test, expect) { - var self = this; - self.id = Random.id(); - await restrictedCollectionForClientIdTest - .insertAsync({ _id: self.id }, { returnServerResultPromise: true }) - .then(async function(res) { - test.equal(res, self.id); - test.equal( - await restrictedCollectionForClientIdTest.findOneAsync(self.id), - { - _id: self.id, - } - ); - }); - }, - ] - ); - }); // end idGeneration loop -} // end if isClient - - - -// A few simple server-only tests which don't need to coordinate collections -// with the client.. -if (Meteor.isServer) { - Tinytest.add("collection - allow and deny validate options", function (test) { - var collection = new Mongo.Collection(null); - - test.throws(function () { - collection.allow({invalidOption: true}); - }); - test.throws(function () { - collection.deny({invalidOption: true}); - }); - - ['insert', 'update', 'remove', 'fetch'].forEach(function (key) { - var options = {}; - options[key] = true; - test.throws(function () { - collection.allow(options); - }); - test.throws(function () { - collection.deny(options); - }); - }); - - ['insert', 'update', 'remove'].forEach(function (key) { - var options = {}; - options[key] = false; - test.throws(function () { - collection.allow(options); - }); - test.throws(function () { - collection.deny(options); - }); - }); - - ['insert', 'update', 'remove'].forEach(function (key) { - var options = {}; - options[key] = undefined; - test.throws(function () { - collection.allow(options); - }); - test.throws(function () { - collection.deny(options); - }); - }); - - ['insert', 'update', 'remove'].forEach(function (key) { - var options = {}; - options[key] = ['an array']; // this should be a function, not an array - test.throws(function () { - collection.allow(options); - }); - test.throws(function () { - collection.deny(options); - }); - }); - - test.throws(function () { - collection.allow({fetch: function () {}}); // this should be an array - }); - }); - - Tinytest.add("collection - calling allow restricts", function (test) { - var collection = new Mongo.Collection(null); - test.equal(collection._restricted, false); - collection.allow({ - insert: function() {} - }); - test.equal(collection._restricted, true); - }); - - Tinytest.add("collection - global insecure", function (test) { - // note: This test alters the global insecure status, by sneakily hacking - // the global Package object! - var insecurePackage = Package.insecure; - - Package.insecure = {}; - var collection = new Mongo.Collection(null); - test.equal(collection._isInsecure(), true); - - Package.insecure = undefined; - test.equal(collection._isInsecure(), false); - - delete Package.insecure; - test.equal(collection._isInsecure(), false); - - collection._insecure = true; - test.equal(collection._isInsecure(), true); - - if (insecurePackage) - Package.insecure = insecurePackage; - else - delete Package.insecure; - }); -} - var AllowAsyncValidateCollection; Tinytest.addAsync( @@ -1202,6 +39,7 @@ Tinytest.addAsync( error ? reject(error) : resolve(result) ); }); + console.log('id', id); await new Promise((resolve, reject) => { AllowAsyncValidateCollection.update( id, @@ -1308,3 +146,111 @@ testAsyncMulti("collection - async definitions on allow/deny rules", [ } }, ]); + +function configAllSyncAllowDeny(collection, configType = 'allow', enabled) { + collection[configType]({ + insert(selector, doc) { + if (doc.force) return true; + return enabled; + }, + update() { + return enabled; + }, + remove() { + return enabled; + }, + }); +} + +async function runAllSyncExpect(test, collection, expected) { + let id; + /* sync tests */ + const syncCallback = (error) => { + if (error) { + test.isTrue(!expected); + return; + } + test.isTrue(expected); + }; + + await new Promise((resolve) => { + id = collection.insert({ num: 2 }, (error, result) => { + if (error) { + test.isTrue(!expected); + resolve(); + return; + } + test.isTrue(expected && result != null); + resolve(); + }); + }); + + await new Promise((resolve) => { + collection.update(id, { $set: { num: 22 } }, (error, result) => { + if (error) { + test.isTrue(!expected); + resolve(); + return; + } + test.isTrue(expected && result != null); + resolve(); + }); + }); + + await new Promise((resolve) => { + collection.remove(id, (error, result) => { + if (error) { + test.isTrue(!expected); + resolve(); + return; + } + test.isTrue(expected && result != null); + resolve(); + }); + }); +} + +var AllowDenySyncRulesCollections = {}; + +testAsyncMulti("collection - sync definitions on allow/deny rules", [ + async function (test) { + AllowDenySyncRulesCollections.allowed = + AllowDenySyncRulesCollections.allowed || + new Mongo.Collection(`allowdeny-sync-rules-allowed`); + if (Meteor.isServer) { + await AllowDenySyncRulesCollections.allowed.removeAsync(); + } + + configAllAsyncAllowDeny(AllowDenySyncRulesCollections.allowed, 'allow', true); + if (Meteor.isClient) { + await runAllSyncExpect(test, AllowDenySyncRulesCollections.allowed, true); + } + }, + async function (test) { + AllowDenySyncRulesCollections.notAllowed = + AllowDenySyncRulesCollections.notAllowed || + new Mongo.Collection(`allowdeny-sync-rules-notAllowed`); + if (Meteor.isServer) { + await AllowDenySyncRulesCollections.notAllowed.removeAsync(); + await AllowDenySyncRulesCollections.notAllowed.insertAsync({ num: 2 }); + } + + configAllSyncAllowDeny(AllowDenySyncRulesCollections.notAllowed, 'allow', false); + if (Meteor.isClient) { + await runAllSyncExpect(test, AllowDenySyncRulesCollections.notAllowed, false); + } + }, + async function (test) { + AllowDenySyncRulesCollections.denied = + AllowDenySyncRulesCollections.denied || + new Mongo.Collection(`allowdeny-sync-rules-denied`); + if (Meteor.isServer) { + await AllowDenySyncRulesCollections.denied.removeAsync(); + } + + configAllSyncAllowDeny(AllowDenySyncRulesCollections.denied, 'deny', true); + if (Meteor.isClient) { + await runAllSyncExpect(test, AllowDenySyncRulesCollections.denied, false); + } + }, +]); From fa31a9d16aa0cf94a76b81efae51234aa096a70d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nacho=20Codo=C3=B1er?= Date: Thu, 28 Nov 2024 18:45:20 +0100 Subject: [PATCH 03/10] revert removal of allow/deny base tests --- packages/mongo/tests/allow_tests.js | 1163 +++++++++++++++++++++++++++ 1 file changed, 1163 insertions(+) diff --git a/packages/mongo/tests/allow_tests.js b/packages/mongo/tests/allow_tests.js index 777d9f6b02..41abcd32a3 100644 --- a/packages/mongo/tests/allow_tests.js +++ b/packages/mongo/tests/allow_tests.js @@ -1,5 +1,1168 @@ import has from 'lodash.has'; +if (Meteor.isServer) { + // Set up allow/deny rules for test collections + + var allowCollections = {}; + + // We create the collections in the publisher (instead of using a method or + // something) because if we made them with a method, we'd need to follow the + // method with some subscribes, and it's possible that the method call would + // be delayed by a wait method and the subscribe messages would be sent before + // it and fail due to the collection not yet existing. So we are very hacky + // and use a publish. + Meteor.publish("allowTests", function (nonce, idGeneration) { + check(nonce, String); + check(idGeneration, String); + var cursors = []; + var needToConfigure; + + // helper for defining a collection. we are careful to create just one + // Mongo.Collection even if the sub body is rerun, by caching them. + var defineCollection = function(name, insecure, transform) { + var fullName = name + idGeneration + nonce; + + var collection; + if (has(allowCollections, fullName)) { + collection = allowCollections[fullName]; + if (needToConfigure === true) + throw new Error("collections inconsistently exist"); + needToConfigure = false; + } else { + collection = new Mongo.Collection( + fullName, {idGeneration: idGeneration, transform: transform}); + allowCollections[fullName] = collection; + if (needToConfigure === false) + throw new Error("collections inconsistently don't exist"); + needToConfigure = true; + collection._insecure = insecure; + var m = {}; + m["clear-collection-" + fullName] = async function() { + await collection.removeAsync({}, { returnServerResultPromise: true }); + }; + Meteor.methods(m); + } + + cursors.push(collection.find()); + return collection; + }; + + var insecureCollection = defineCollection( + "collection-insecure", true /*insecure*/); + // totally locked down collection + var lockedDownCollection = defineCollection( + "collection-locked-down", false /*insecure*/); + // restricted collection with same allowed modifications, both with and + // without the `insecure` package + var restrictedCollectionDefaultSecure = defineCollection( + "collection-restrictedDefaultSecure", false /*insecure*/); + var restrictedCollectionDefaultInsecure = defineCollection( + "collection-restrictedDefaultInsecure", true /*insecure*/); + var restrictedCollectionForUpdateOptionsTest = defineCollection( + "collection-restrictedForUpdateOptionsTest", true /*insecure*/); + var restrictedCollectionForPartialAllowTest = defineCollection( + "collection-restrictedForPartialAllowTest", true /*insecure*/); + var restrictedCollectionForPartialDenyTest = defineCollection( + "collection-restrictedForPartialDenyTest", true /*insecure*/); + var restrictedCollectionForFetchTest = defineCollection( + "collection-restrictedForFetchTest", true /*insecure*/); + var restrictedCollectionForFetchAllTest = defineCollection( + "collection-restrictedForFetchAllTest", true /*insecure*/); + var restrictedCollectionWithTransform = defineCollection( + "withTransform", false, function (doc) { + return doc.a; + }); + var restrictedCollectionForInvalidTransformTest = defineCollection( + "collection-restrictedForInvalidTransform", false /*insecure*/); + var restrictedCollectionForClientIdTest = defineCollection( + "collection-restrictedForClientIdTest", false /*insecure*/); + + if (needToConfigure) { + restrictedCollectionWithTransform.allow({ + insertAsync: function (userId, doc) { + return doc.foo === "foo"; + }, + updateAsync: function (userId, doc) { + return doc.foo === "foo"; + }, + removeAsync: function (userId, doc) { + return doc.bar === "bar"; + } + }); + restrictedCollectionWithTransform.allow({ + // transform: null means that doc here is the top level, not the 'a' + // element. + transform: null, + insertAsync: function (userId, doc) { + return !!doc.topLevelField; + }, + updateAsync: function (userId, doc) { + return !!doc.topLevelField; + } + }); + restrictedCollectionForInvalidTransformTest.allow({ + // transform must return an object which is not a mongo id + transform: function (doc) { return doc._id; }, + insert: function () { return true; } + }); + restrictedCollectionForClientIdTest.allow({ + // This test just requires the collection to trigger the restricted + // case. + insert: function () { return true; }, + insertAsync: function () { return true; } + }); + + // two calls to allow to verify that either validator is sufficient. + var allows = [{ + insertAsync: function(userId, doc) { + return doc.canInsert; + }, + updateAsync: function(userId, doc) { + return doc.canUpdate; + }, + removeAsync: function (userId, doc) { + return doc.canRemove; + } + }, { + insertAsync: function(userId, doc) { + return doc.canInsert2; + }, + updateAsync: function(userId, doc, fields, modifier) { + return -1 !== fields.indexOf('canUpdate2'); + }, + removeAsync: function(userId, doc) { + return doc.canRemove2; + } + }]; + + // two calls to deny to verify that either one blocks the change. + var denies = [{ + insertAsync: function(userId, doc) { + return doc.cantInsert; + }, + removeAsync: function (userId, doc) { + return doc.cantRemove; + } + }, { + insertAsync: function(userId, doc) { + // Don't allow explicit ID to be set by the client. + return has(doc, '_id'); + }, + updateAsync: function(userId, doc, fields, modifier) { + return -1 !== fields.indexOf('verySecret'); + } + }]; + + [ + restrictedCollectionDefaultSecure, + restrictedCollectionDefaultInsecure, + restrictedCollectionForUpdateOptionsTest + ].forEach(function (collection) { + allows.forEach(function (allow) { + collection.allow(allow); + }); + denies.forEach(function (deny) { + collection.deny(deny); + }); + }); + + // just restrict one operation so that we can verify that others + // fail + restrictedCollectionForPartialAllowTest.allow({ + insert: function() {} + }); + restrictedCollectionForPartialDenyTest.deny({ + insert: function() {} + }); + + // verify that we only fetch the fields specified - we should + // be fetching just field1, field2, and field3. + restrictedCollectionForFetchTest.allow({ + insertAsync: function() { return true; }, + updateAsync: function(userId, doc) { + // throw fields in doc so that we can inspect them in test + throw new Meteor.Error( + 999, "Test: Fields in doc: " + Object.keys(doc).sort().join(',')); + }, + removeAsync: function(userId, doc) { + // throw fields in doc so that we can inspect them in test + throw new Meteor.Error( + 999, "Test: Fields in doc: " + Object.keys(doc).sort().join(',')); + }, + fetch: ['field1'] + }); + restrictedCollectionForFetchTest.allow({ + fetch: ['field2'] + }); + restrictedCollectionForFetchTest.deny({ + fetch: ['field3'] + }); + + // verify that not passing fetch to one of the calls to allow + // causes all fields to be fetched + restrictedCollectionForFetchAllTest.allow({ + insertAsync: function() { return true; }, + updateAsync: function(userId, doc) { + // throw fields in doc so that we can inspect them in test + throw new Meteor.Error( + 999, "Test: Fields in doc: " + Object.keys(doc).sort().join(',')); + }, + removeAsync: function(userId, doc) { + // throw fields in doc so that we can inspect them in test + throw new Meteor.Error( + 999, "Test: Fields in doc: " + Object.keys(doc).sort().join(',')); + }, + fetch: ['field1'] + }); + restrictedCollectionForFetchAllTest.allow({ + updateAsync: function() { return true; } + }); + } + + return cursors; + }); +} + +if (Meteor.isClient) { + ['STRING', 'MONGO'].forEach(function (idGeneration) { + // Set up a bunch of test collections... on the client! They match the ones + // created by setUpAllowTestsCollections. + + var nonce = Random.id(); + // Tell the server to make, configure, and publish a set of collections unique + // to our test run. Since the method does not unblock, this will complete + // running on the server before anything else happens. + Meteor.subscribe('allowTests', nonce, idGeneration); + + // helper for defining a collection, subscribing to it, and defining + // a method to clear it + var defineCollection = function(name, transform) { + var fullName = name + idGeneration + nonce; + var collection = new Mongo.Collection( + fullName, {idGeneration: idGeneration, transform: transform}); + + collection.callClearMethod = async function () { + await Meteor.callAsync('clear-collection-' + fullName); + }; + collection.unnoncedName = name + idGeneration; + return collection; + }; + + // totally insecure collection + var insecureCollection = defineCollection("collection-insecure"); + + // totally locked down collection + var lockedDownCollection = defineCollection("collection-locked-down"); + + // restricted collection with same allowed modifications, both with and + // without the `insecure` package + var restrictedCollectionDefaultSecure = defineCollection( + "collection-restrictedDefaultSecure"); + var restrictedCollectionDefaultInsecure = defineCollection( + "collection-restrictedDefaultInsecure"); + var restrictedCollectionForUpdateOptionsTest = defineCollection( + "collection-restrictedForUpdateOptionsTest"); + var restrictedCollectionForPartialAllowTest = defineCollection( + "collection-restrictedForPartialAllowTest"); + var restrictedCollectionForPartialDenyTest = defineCollection( + "collection-restrictedForPartialDenyTest"); + var restrictedCollectionForFetchTest = defineCollection( + "collection-restrictedForFetchTest"); + var restrictedCollectionForFetchAllTest = defineCollection( + "collection-restrictedForFetchAllTest"); + var restrictedCollectionWithTransform = defineCollection( + "withTransform", function (doc) { + return doc.a; + }); + var restrictedCollectionForInvalidTransformTest = defineCollection( + "collection-restrictedForInvalidTransform"); + var restrictedCollectionForClientIdTest = defineCollection( + "collection-restrictedForClientIdTest"); + + // test that if allow is called once then the collection is + // restricted, and that other mutations aren't allowed + testAsyncMulti('collection - partial allow, ' + idGeneration, [ + async function(test, expect) { + try { + await restrictedCollectionForPartialAllowTest.updateAsync( + 'foo', + { $set: { updated: true } }, + { + returnServerResultPromise: true, + } + ); + } catch (err) { + test.equal(err.error, 403); + } + }, + ]); + + // test that if deny is called once then the collection is + // restricted, and that other mutations aren't allowed + testAsyncMulti('collection - partial deny, ' + idGeneration, [ + async function(test, expect) { + try { + await restrictedCollectionForPartialDenyTest.updateAsync( + 'foo', + { + $set: { updated: true }, + }, + { + returnServerResultPromise: true, + } + ); + } catch (err) { + test.equal(err.error, 403); + } + }, + ]); + + + // test that we only fetch the fields specified + testAsyncMulti('collection - fetch, ' + idGeneration, [ + async function(test, expect) { + var fetchId = await restrictedCollectionForFetchTest.insertAsync({ + field1: 1, + field2: 1, + field3: 1, + field4: 1, + },{ + returnServerResultPromise: true, + }); + var fetchAllId = await restrictedCollectionForFetchAllTest.insertAsync({ + field1: 1, + field2: 1, + field3: 1, + field4: 1, + },{ + 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 + .removeAsync(fetchId, { + returnServerResultPromise: true, + }) + .catch( + expect(function(err) { + test.equal( + err.reason, + 'Test: Fields in doc: _id,field1,field2,field3' + ); + }) + ); + + await restrictedCollectionForFetchAllTest + .updateAsync( + fetchAllId, + { $set: { updated: true } }, + { + returnServerResultPromise: true, + } + ) + .catch( + expect(function(err) { + test.equal( + err.reason, + 'Test: Fields in doc: _id,field1,field2,field3,field4' + ); + }) + ); + await restrictedCollectionForFetchAllTest + .removeAsync(fetchAllId, { + returnServerResultPromise: true, + }) + .catch( + expect(function(err) { + test.equal( + err.reason, + 'Test: Fields in doc: _id,field1,field2,field3,field4' + ); + }) + ); + }, + ]); + + (function() { + testAsyncMulti('collection - restricted factories ' + idGeneration, [ + async function(test, expect) { + await restrictedCollectionWithTransform.callClearMethod(); + test.equal( + await restrictedCollectionWithTransform.find().countAsync(), + 0 + ); + }, + async function(test, expect) { + var self = this; + await restrictedCollectionWithTransform + .insertAsync( + { + a: { foo: 'foo', bar: 'bar', baz: 'baz' }, + }, + { + returnServerResultPromise: true, + } + ) + .then( + expect(function(res) { + test.isTrue(res); + self.item1 = res; + }) + ); + await restrictedCollectionWithTransform + .insertAsync( + { + a: { foo: 'foo', bar: 'quux', baz: 'quux' }, + b: 'potato', + }, + { + returnServerResultPromise: true, + } + ) + .then( + expect(function(res) { + test.isTrue(res); + self.item2 = res; + }) + ); + await restrictedCollectionWithTransform + .insertAsync( + { + a: { foo: 'adsfadf', bar: 'quux', baz: 'quux' }, + b: 'potato', + }, + { + returnServerResultPromise: true, + } + ) + .catch( + expect(function(e, res) { + test.isTrue(e); + }) + ); + await restrictedCollectionWithTransform + .insertAsync( + { + a: { foo: 'bar' }, + topLevelField: true, + }, + { + returnServerResultPromise: true, + } + ) + .then( + expect(function(res) { + test.isTrue(res); + self.item3 = res; + }) + ); + }, + async function(test, expect) { + var self = this; + // This should work, because there is an update allow for things with + // topLevelField. + await restrictedCollectionWithTransform + .updateAsync( + self.item3, + { $set: { xxx: true } }, + { + returnServerResultPromise: true, + } + ) + .then( + expect(function(res) { + test.equal(1, res); + }) + ); + }, + async function(test, expect) { + var self = this; + test.equal( + await restrictedCollectionWithTransform.findOneAsync(self.item1), + { + _id: self.item1, + foo: 'foo', + bar: 'bar', + baz: 'baz', + } + ); + await restrictedCollectionWithTransform + .removeAsync(self.item1, { + returnServerResultPromise: true, + }) + .then( + expect(function(res) { + test.isTrue(res); + }) + ); + await restrictedCollectionWithTransform + .removeAsync(self.item2, { + returnServerResultPromise: true, + }) + .catch( + expect(function(e) { + test.isTrue(e); + }) + ); + }, + ]); + })(); + + testAsyncMulti('collection - insecure, ' + idGeneration, [ + async function(test, expect) { + await insecureCollection.callClearMethod(); + test.equal(await insecureCollection.find().countAsync(), 0); + }, + async function(test, expect) { + let idThen; + var id = await insecureCollection + .insertAsync( + { foo: 'bar' }, + { + returnServerResultPromise: true, + } + ) + .then(async function(res) { + idThen = res; + test.equal(await insecureCollection.find(res).countAsync(), 1); + test.equal((await insecureCollection.findOneAsync(res)).foo, 'bar'); + return res; + }); + test.equal(idThen, id); + test.equal(await insecureCollection.find(id).countAsync(), 1); + test.equal((await insecureCollection.findOneAsync(id)).foo, 'bar'); + }, + ]); + + testAsyncMulti('collection - locked down, ' + idGeneration, [ + async function(test, expect) { + await lockedDownCollection.callClearMethod(); + test.equal(await lockedDownCollection.find().countAsync(), 0); + }, + async function(test, expect) { + await lockedDownCollection + .insertAsync( + { foo: 'bar' }, + { + returnServerResultPromise: true, + } + ) + .catch(async function(err, res) { + test.equal(err.error, 403); + test.equal(await lockedDownCollection.find().countAsync(), 0); + }); + }, + ]); + + (function() { + var collection = restrictedCollectionForUpdateOptionsTest; + var id1, id2; + testAsyncMulti('collection - update options, ' + idGeneration, [ + // init + async function(test, expect) { + await collection.callClearMethod().then(async function() { + test.equal(await collection.find().countAsync(), 0); + }); + }, + // put a few objects + async function(test, expect) { + var doc = { canInsert: true, canUpdate: true }; + id1 = await collection.insertAsync(doc); + id2 = await collection.insertAsync(doc); + await collection.insertAsync(doc); + await collection + .insertAsync(doc, { returnServerResultPromise: true }) + .then(async function(res) { + test.equal(await collection.find().countAsync(), 4); + }); + }, + // update by id + async function(test, expect) { + await collection + .updateAsync( + id1, + { $set: { updated: true } }, + { returnServerResultPromise: true } + ) + .then(async function(res) { + test.equal(res, 1); + test.equal( + await collection.find({ updated: true }).countAsync(), + 1 + ); + }); + }, + // update by id in an object + async function(test, expect) { + await collection + .updateAsync( + { _id: id2 }, + { $set: { updated: true } }, + { returnServerResultPromise: true } + ) + .then(async function(res) { + test.equal(res, 1); + test.equal( + await collection.find({ updated: true }).countAsync(), + 2 + ); + }); + }, + // update with replacement operator not allowed, and has nice error. + async function(test, expect) { + await collection + .updateAsync( + { _id: id2 }, + { _id: id2, updated: true }, + { returnServerResultPromise: true } + ) + .catch(async function(err) { + test.equal(err.error, 403); + test.matches(err.reason, /In a restricted/); + // unchanged + test.equal( + await collection.find({ updated: true }).countAsync(), + 2 + ); + }); + }, + // upsert not allowed, and has nice error. + async function(test, expect) { + await collection + .updateAsync( + { _id: id2 }, + { $set: { upserted: true } }, + { upsert: true, returnServerResultPromise: true } + ) + .catch(async function(err) { + test.equal(err.error, 403); + test.matches(err.reason, /in a restricted/); + test.equal( + await collection.find({ upserted: true }).countAsync(), + 0 + ); + }); + }, + // update with rename operator not allowed, and has nice error. + async function(test, expect) { + await collection + .updateAsync( + { _id: id2 }, + { $rename: { updated: 'asdf' } }, + { returnServerResultPromise: true } + ) + .catch(async function(err) { + test.equal(err.error, 403); + test.matches(err.reason, /not allowed/); + // unchanged + test.equal( + await collection.find({ updated: true }).countAsync(), + 2 + ); + }); + }, + // update method with a non-ID selector is not allowed + async function(test, expect) { + // We shouldn't even send the method... + await test.throwsAsync(async function() { + await collection.updateAsync( + { updated: { $exists: false } }, + { $set: { updated: true } } + ); + }); + // ... but if we did, the server would reject it too. + await Meteor.callAsync( + '/' + collection._name + '/updateAsync', + { updated: { $exists: false } }, + { $set: { updated: true } } + ).catch(async function(err, res) { + test.equal(err.error, 403); + // unchanged + test.equal( + await collection.find({ updated: true }).countAsync(), + 2 + ); + }); + }, + // make sure it doesn't think that {_id: 'foo', something: else} is ok. + async function(test, expect) { + await test.throwsAsync(async function() { + await collection.updateAsync( + { _id: id1, updated: { $exists: false } }, + { $set: { updated: true } } + ); + }); + }, + // remove method with a non-ID selector is not allowed + async function(test, expect) { + // We shouldn't even send the method... + await test.throwsAsync(async function() { + await collection.removeAsync({ updated: true }); + }); + // ... but if we did, the server would reject it too. + await Meteor.callAsync( + '/' + collection._name + '/removeAsync', + { + updated: true, + } + ).catch(async function(err) { + test.equal(err.error, 403); + // unchanged + test.equal( + await collection.find({ updated: true }).countAsync(), + 2 + ); + }); + }, + ]); + })(); + + + [restrictedCollectionDefaultInsecure, restrictedCollectionDefaultSecure].forEach( + function(collection) { + var canUpdateId, canRemoveId; + + testAsyncMulti('collection - ' + collection.unnoncedName, [ + // init + async function(test, expect) { + await collection.callClearMethod().then(async function() { + test.equal(await collection.find().countAsync(), 0); + }); + }, + + // insert with no allows passing. request is denied. + async function(test, expect) { + await collection + .insertAsync({}, { returnServerResultPromise: true }) + .catch(async function(err, res) { + test.equal(err.error, 403); + test.equal(await collection.find().countAsync(), 0); + }); + }, + // insert with one allow and one deny. denied. + async function(test, expect) { + await collection + .insertAsync( + { canInsert: true, cantInsert: true }, + { returnServerResultPromise: true } + ) + .catch(async function(err, res) { + test.equal(err.error, 403); + test.equal(await collection.find().countAsync(), 0); + }); + }, + // insert with one allow and other deny. denied. + async function(test, expect) { + await collection + .insertAsync( + { canInsert: true, _id: Random.id() }, + { returnServerResultPromise: true } + ) + .catch(async function(err) { + test.equal(err.error, 403); + test.equal(await collection.find().countAsync(), 0); + }); + }, + // insert one allow passes. allowed. + async function(test, expect) { + await collection + .insertAsync( + { canInsert: true }, + { returnServerResultPromise: true } + ) + .then(async function(err, res) { + test.equal(await collection.find().countAsync(), 1); + }); + }, + // insert other allow passes. allowed. + // includes canUpdate for later. + async function(test, expect) { + canUpdateId = await collection + .insertAsync( + { canInsert2: true, canUpdate: true }, + { returnServerResultPromise: true } + ) + .then(async function(res) { + test.equal(await collection.find().countAsync(), 2); + return res; + }); + }, + // yet a third insert executes. this one has canRemove and + // cantRemove set for later. + async function(test, expect) { + canRemoveId = await collection + .insertAsync( + { canInsert: true, canRemove: true, cantRemove: true }, + { returnServerResultPromise: true } + ) + .then(async function(res) { + test.equal(await collection.find().countAsync(), 3); + return res; + }); + }, + + // can't update with a non-operator mutation + async function(test, expect) { + await collection + .updateAsync( + canUpdateId, + { newObject: 1 }, + { returnServerResultPromise: true } + ) + .catch(async function(err, res) { + test.equal(err.error, 403); + test.equal(await collection.find().countAsync(), 3); + }); + }, + + // updating dotted fields works as if we are changing their + // top part + async function(test, expect) { + await collection + .updateAsync( + canUpdateId, + { $set: { 'dotted.field': 1 } }, + { returnServerResultPromise: true } + ) + .then(async function(res) { + test.equal(res, 1); + test.equal( + (await collection.findOneAsync(canUpdateId)).dotted.field, + 1 + ); + }); + }, + async function(test, expect) { + await collection + .updateAsync( + canUpdateId, + { $set: { 'verySecret.field': 1 } }, + { returnServerResultPromise: true } + ) + .catch(async function(err, res) { + test.equal(err.error, 403); + test.equal( + await collection + .find({ verySecret: { $exists: true } }) + .countAsync(), + 0 + ); + }); + }, + + // update doesn't do anything if no docs match + async function(test, expect) { + await collection + .updateAsync( + "doesn't exist", + { $set: { updated: true } }, + { returnServerResultPromise: true } + ) + .then(async function(res) { + test.equal(res, 0); + // nothing has changed + test.equal(await collection.find().countAsync(), 3); + test.equal( + await collection.find({ updated: true }).countAsync(), + 0 + ); + }); + }, + // update fails when access is denied trying to set `verySecret` + async function(test, expect) { + await collection + .updateAsync( + canUpdateId, + { $set: { verySecret: true } }, + { returnServerResultPromise: true } + ) + .catch(async function(err) { + test.equal(err.error, 403); + // nothing has changed + test.equal(await collection.find().countAsync(), 3); + test.equal( + await collection.find({ updated: true }).countAsync(), + 0 + ); + }); + }, + // update fails when trying to set two fields, one of which is + // `verySecret` + async function(test, expect) { + await collection + .updateAsync( + canUpdateId, + { $set: { updated: true, verySecret: true } }, + { returnServerResultPromise: true } + ) + .catch(async function(err, res) { + test.equal(err.error, 403); + // nothing has changed + test.equal(await collection.find().countAsync(), 3); + test.equal( + await collection.find({ updated: true }).countAsync(), + 0 + ); + }); + }, + // update fails when trying to modify docs that don't + // have `canUpdate` set + async function(test, expect) { + await collection + .updateAsync( + canRemoveId, + { $set: { updated: true } }, + { returnServerResultPromise: true } + ) + .catch(async function(err, res) { + test.equal(err.error, 403); + // nothing has changed + test.equal(await collection.find().countAsync(), 3); + test.equal( + await collection.find({ updated: true }).countAsync(), + 0 + ); + }); + }, + // update executes when it should + async function(test, expect) { + await collection + .updateAsync( + canUpdateId, + { $set: { updated: true } }, + { returnServerResultPromise: true } + ) + .then(async function(res) { + test.equal(res, 1); + test.equal( + await collection.find({ updated: true }).countAsync(), + 1 + ); + }); + }, + + // remove fails when trying to modify a doc with no `canRemove` set + async function(test, expect) { + await collection + .removeAsync(canUpdateId, { returnServerResultPromise: true }) + .catch(async function(err, res) { + test.equal(err.error, 403); + // nothing has changed + test.equal(await collection.find().countAsync(), 3); + }); + }, + // remove fails when trying to modify an doc with `cantRemove` + // set + async function(test, expect) { + await collection + .removeAsync(canRemoveId, { returnServerResultPromise: true }) + .catch(async function(err, res) { + test.equal(err.error, 403); + // nothing has changed + test.equal(await collection.find().countAsync(), 3); + }); + }, + + // update the doc to remove cantRemove. + async function(test, expect) { + await collection + .updateAsync( + canRemoveId, + { $set: { cantRemove: false, canUpdate2: true } }, + { returnServerResultPromise: true } + ) + .then(async function(res) { + test.equal(res, 1); + test.equal( + await collection.find({ cantRemove: true }).countAsync(), + 0 + ); + }); + }, + + // now remove can remove it. + async function(test, expect) { + await collection + .removeAsync(canRemoveId, { returnServerResultPromise: true }) + .then(async function(res) { + test.equal(res, 1); + // successfully removed + test.equal(await collection.find().countAsync(), 2); + }); + }, + + // try to remove a doc that doesn't exist. see we remove no docs. + async function(test, expect) { + await collection + .removeAsync('some-random-id-that-never-matches', { + returnServerResultPromise: true, + }) + .then(async function(res) { + test.equal(res, 0); + // nothing removed + test.equal(await collection.find().countAsync(), 2); + }); + }, + + // methods can still bypass restrictions + async function(test, expect) { + await collection.callClearMethod().then(async function(err, res) { + // successfully removed + test.equal(await collection.find().countAsync(), 0); + }); + }, + ]); + } + ); + testAsyncMulti( + 'collection - allow/deny transform must return object, ' + idGeneration, + [ + async function(test, expect) { + await restrictedCollectionForInvalidTransformTest + .insertAsync({}, { returnServerResultPromise: true }) + .catch(function(err) { + test.isTrue(err); + }); + }, + ] + ); + testAsyncMulti( + 'collection - restricted collection allows client-side id, ' + + idGeneration, + [ + async function(test, expect) { + var self = this; + self.id = Random.id(); + await restrictedCollectionForClientIdTest + .insertAsync({ _id: self.id }, { returnServerResultPromise: true }) + .then(async function(res) { + test.equal(res, self.id); + test.equal( + await restrictedCollectionForClientIdTest.findOneAsync(self.id), + { + _id: self.id, + } + ); + }); + }, + ] + ); + }); // end idGeneration loop +} // end if isClient + + + +// A few simple server-only tests which don't need to coordinate collections +// with the client.. +if (Meteor.isServer) { + Tinytest.add("collection - allow and deny validate options", function (test) { + var collection = new Mongo.Collection(null); + + test.throws(function () { + collection.allow({invalidOption: true}); + }); + test.throws(function () { + collection.deny({invalidOption: true}); + }); + + ['insert', 'update', 'remove', 'fetch'].forEach(function (key) { + var options = {}; + options[key] = true; + test.throws(function () { + collection.allow(options); + }); + test.throws(function () { + collection.deny(options); + }); + }); + + ['insert', 'update', 'remove'].forEach(function (key) { + var options = {}; + options[key] = false; + test.throws(function () { + collection.allow(options); + }); + test.throws(function () { + collection.deny(options); + }); + }); + + ['insert', 'update', 'remove'].forEach(function (key) { + var options = {}; + options[key] = undefined; + test.throws(function () { + collection.allow(options); + }); + test.throws(function () { + collection.deny(options); + }); + }); + + ['insert', 'update', 'remove'].forEach(function (key) { + var options = {}; + options[key] = ['an array']; // this should be a function, not an array + test.throws(function () { + collection.allow(options); + }); + test.throws(function () { + collection.deny(options); + }); + }); + + test.throws(function () { + collection.allow({fetch: function () {}}); // this should be an array + }); + }); + + Tinytest.add("collection - calling allow restricts", function (test) { + var collection = new Mongo.Collection(null); + test.equal(collection._restricted, false); + collection.allow({ + insert: function() {} + }); + test.equal(collection._restricted, true); + }); + + Tinytest.add("collection - global insecure", function (test) { + // note: This test alters the global insecure status, by sneakily hacking + // the global Package object! + var insecurePackage = Package.insecure; + + Package.insecure = {}; + var collection = new Mongo.Collection(null); + test.equal(collection._isInsecure(), true); + + Package.insecure = undefined; + test.equal(collection._isInsecure(), false); + + delete Package.insecure; + test.equal(collection._isInsecure(), false); + + collection._insecure = true; + test.equal(collection._isInsecure(), true); + + if (insecurePackage) + Package.insecure = insecurePackage; + else + delete Package.insecure; + }); +} + var AllowAsyncValidateCollection; Tinytest.addAsync( From 0f840eafcee35f8b79bcbeae9800fba4f0fe0ea7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nacho=20Codo=C3=B1er?= Date: Mon, 2 Dec 2024 15:17:30 +0100 Subject: [PATCH 04/10] fix allow/deny failing regression and ensure sync configs accepts async validation --- packages/allow-deny/allow-deny.js | 178 +--------------------------- packages/mongo/tests/allow_tests.js | 13 +- 2 files changed, 10 insertions(+), 181 deletions(-) diff --git a/packages/allow-deny/allow-deny.js b/packages/allow-deny/allow-deny.js index d6e58f1f8b..3d52acd4c2 100644 --- a/packages/allow-deny/allow-deny.js +++ b/packages/allow-deny/allow-deny.js @@ -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 @@ -408,8 +378,9 @@ CollectionPrototype._validatedUpdateAsync = async function( }); } - - const doc = await self._collection.findOneAsync(selector, findOptions) || {}; + const doc = await self._collection.findOneAsync(selector, findOptions); + if (!doc) // none satisfied! + return 0; // call user validators. // Any deny returns true means denied. @@ -436,9 +407,6 @@ CollectionPrototype._validatedUpdateAsync = async function( throw new Meteor.Error(403, "Access denied"); } - if (!doc) // none satisfied! - return 0; - options._forbidReplace = true; // Back when we supported arbitrary client-provided selectors, we actually @@ -450,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 @@ -570,7 +442,9 @@ CollectionPrototype._validatedRemoveAsync = async function(userId, selector) { }); } - const doc = await self._collection.findOneAsync(selector, findOptions) || {}; + const doc = await self._collection.findOneAsync(selector, findOptions); + if (!doc) + return 0; // call user validators. // Any deny returns true means denied. @@ -588,9 +462,6 @@ CollectionPrototype._validatedRemoveAsync = async function(userId, selector) { throw new Meteor.Error(403, "Access denied"); } - if (!doc) - return 0; - // 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 @@ -599,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 diff --git a/packages/mongo/tests/allow_tests.js b/packages/mongo/tests/allow_tests.js index 41abcd32a3..84442b8003 100644 --- a/packages/mongo/tests/allow_tests.js +++ b/packages/mongo/tests/allow_tests.js @@ -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); @@ -1328,14 +1329,6 @@ function configAllSyncAllowDeny(collection, configType = 'allow', enabled) { async function runAllSyncExpect(test, collection, expected) { let id; /* sync tests */ - const syncCallback = (error) => { - if (error) { - test.isTrue(!expected); - return; - } - test.isTrue(expected); - }; - await new Promise((resolve) => { id = collection.insert({ num: 2 }, (error, result) => { if (error) { @@ -1349,6 +1342,7 @@ async function runAllSyncExpect(test, collection, expected) { }); await new Promise((resolve) => { + id = collection.insert({ force: true }); collection.update(id, { $set: { num: 22 } }, (error, result) => { if (error) { test.isTrue(!expected); @@ -1361,6 +1355,7 @@ async function runAllSyncExpect(test, collection, expected) { }); await new Promise((resolve) => { + id = collection.insert({ force: true }); collection.remove(id, (error, result) => { if (error) { test.isTrue(!expected); @@ -1384,7 +1379,7 @@ testAsyncMulti("collection - sync definitions on allow/deny rules", [ await AllowDenySyncRulesCollections.allowed.removeAsync(); } - configAllAsyncAllowDeny(AllowDenySyncRulesCollections.allowed, 'allow', true); + configAllSyncAllowDeny(AllowDenySyncRulesCollections.allowed, 'allow', true); if (Meteor.isClient) { await runAllSyncExpect(test, AllowDenySyncRulesCollections.allowed, true); } From e98956a8db71f9c6b57cf936dfa5eadb6720501a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nacho=20Codo=C3=B1er?= Date: Mon, 2 Dec 2024 16:33:29 +0100 Subject: [PATCH 05/10] clean --- packages/allow-deny/allow-deny.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/allow-deny/allow-deny.js b/packages/allow-deny/allow-deny.js index 3d52acd4c2..528bae567b 100644 --- a/packages/allow-deny/allow-deny.js +++ b/packages/allow-deny/allow-deny.js @@ -549,9 +549,7 @@ function addValidator(collection, allowOrDeny, options) { Object.keys(options).forEach((key) => { if (!validKeysRegEx.test(key)) throw new Error(allowOrDeny + ": Invalid key: " + key); - }); - Object.keys(options).forEach((key) => { // TODO deprecated async config on future versions const isAsyncKey = key.includes('Async'); if (isAsyncKey) { From c40a9fb61b61dbdbcc0fbaa1c496a67741f36334 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nacho=20Codo=C3=B1er?= Date: Mon, 2 Dec 2024 16:44:51 +0100 Subject: [PATCH 06/10] update docs after changes on allow/deny --- v3-docs/docs/api/collections.md | 54 +++++++++------------------------ 1 file changed, 14 insertions(+), 40 deletions(-) diff --git a/v3-docs/docs/api/collections.md b/v3-docs/docs/api/collections.md index 1575ad7d5f..a595c4485d 100644 --- a/v3-docs/docs/api/collections.md +++ b/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` }); From 40c67dd80e004950ebfe2cd2266dbe53c7ed379e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nacho=20Codo=C3=B1er?= Date: Mon, 2 Dec 2024 17:14:15 +0100 Subject: [PATCH 07/10] add missing tests --- packages/mongo/package.js | 16 ++++++++-------- packages/mongo/tests/allow_tests.js | 28 +++++++++++++++++++++++++++- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/packages/mongo/package.js b/packages/mongo/package.js index 16d28e6e72..521b2e8277 100644 --- a/packages/mongo/package.js +++ b/packages/mongo/package.js @@ -124,13 +124,13 @@ Package.onTest(function (api) { ]); // XXX test order dependency: the allow_tests "partial allow" test // fails if it is run before mongo_livedata_tests. - api.addFiles("tests/mongo_livedata_tests.js", ["client", "server"]); - api.addFiles("tests/upsert_compatibility_test.js", "server"); + // api.addFiles("tests/mongo_livedata_tests.js", ["client", "server"]); + // api.addFiles("tests/upsert_compatibility_test.js", "server"); api.addFiles("tests/allow_tests.js", ["client", "server"]); - api.addFiles("tests/collection_tests.js", ["client", "server"]); - api.addFiles("tests/collection_async_tests.js", ["client", "server"]); - api.addFiles("tests/observe_changes_tests.js", ["client", "server"]); - api.addFiles("tests/oplog_tests.js", "server"); - api.addFiles("tests/oplog_v2_converter_tests.js", "server"); - api.addFiles("tests/doc_fetcher_tests.js", "server"); + // api.addFiles("tests/collection_tests.js", ["client", "server"]); + // api.addFiles("tests/collection_async_tests.js", ["client", "server"]); + // api.addFiles("tests/observe_changes_tests.js", ["client", "server"]); + // api.addFiles("tests/oplog_tests.js", "server"); + // api.addFiles("tests/oplog_v2_converter_tests.js", "server"); + // api.addFiles("tests/doc_fetcher_tests.js", "server"); }); diff --git a/packages/mongo/tests/allow_tests.js b/packages/mongo/tests/allow_tests.js index 84442b8003..844e40c2b7 100644 --- a/packages/mongo/tests/allow_tests.js +++ b/packages/mongo/tests/allow_tests.js @@ -1314,7 +1314,7 @@ testAsyncMulti("collection - async definitions on allow/deny rules", [ function configAllSyncAllowDeny(collection, configType = 'allow', enabled) { collection[configType]({ insert(selector, doc) { - if (doc.force) return true; + if (doc.force) return configType === 'allow'; return enabled; }, update() { @@ -1411,4 +1411,30 @@ testAsyncMulti("collection - sync definitions on allow/deny rules", [ await runAllSyncExpect(test, AllowDenySyncRulesCollections.denied, false); } }, + async function (test) { + AllowDenySyncRulesCollections.noRules = + AllowDenySyncRulesCollections.noRules || + new Mongo.Collection(`allowdeny-sync-rules-noRules`); + if (Meteor.isServer) { + await AllowDenySyncRulesCollections.noRules.removeAsync(); + } + + if (Meteor.isClient) { + await runAllSyncExpect(test, AllowDenySyncRulesCollections.noRules, false); + } + }, + async function (test) { + AllowDenySyncRulesCollections.allowThenDeny = + AllowDenySyncRulesCollections.allowThenDeny || + new Mongo.Collection(`allowdeny-sync-rules-allowThenDeny`); + if (Meteor.isServer) { + await AllowDenySyncRulesCollections.allowThenDeny.removeAsync(); + } + + configAllSyncAllowDeny(AllowDenySyncRulesCollections.allowThenDeny, 'allow', true); + configAllSyncAllowDeny(AllowDenySyncRulesCollections.allowThenDeny, 'deny', true); + if (Meteor.isClient) { + await runAllSyncExpect(test, AllowDenySyncRulesCollections.allowThenDeny, false); + } + }, ]); From c7d88f8cb39956b080f3f8fa945c36de09f390fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nacho=20Codo=C3=B1er?= Date: Mon, 2 Dec 2024 17:33:25 +0100 Subject: [PATCH 08/10] refactor tests for sync and async allow/deny configurations --- packages/mongo/tests/allow_tests.js | 335 +++++++++++++--------------- 1 file changed, 150 insertions(+), 185 deletions(-) diff --git a/packages/mongo/tests/allow_tests.js b/packages/mongo/tests/allow_tests.js index 844e40c2b7..d2647060b9 100644 --- a/packages/mongo/tests/allow_tests.js +++ b/packages/mongo/tests/allow_tests.js @@ -1225,216 +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(); - } - - 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(); - } - - configAllAsyncAllowDeny(AllowDenyAsyncRulesCollections.denied, 'deny', true); - if (Meteor.isClient) { - await runAllAsyncExpect(test, AllowDenyAsyncRulesCollections.denied, false); - } - }, -]); - -function configAllSyncAllowDeny(collection, configType = 'allow', enabled) { - collection[configType]({ - insert(selector, doc) { - if (doc.force) return configType === 'allow'; - return enabled; + if (Meteor.isClient) { + await runAllExpect(test, collections.noRules, false, isAsync); + } }, - update() { - return enabled; + 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(); + } + + configAllAllowDeny(collections.allowed, 'allow', true, isAsync); + if (Meteor.isClient) { + await runAllExpect(test, collections.allowed, true, isAsync); + } }, - remove() { - return enabled; + 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(); + } + + 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); + } + }, + ]; } -async function runAllSyncExpect(test, collection, expected) { - let id; - /* sync tests */ - await new Promise((resolve) => { - id = collection.insert({ num: 2 }, (error, result) => { - if (error) { - test.isTrue(!expected); - resolve(); - return; - } - test.isTrue(expected && result != null); - resolve(); - }); - }); +testAsyncMulti("collection - async definitions on allow/deny rules", createAllowDenyRulesTest(AllowDenyRulesCollections, true)); - await new Promise((resolve) => { - id = collection.insert({ force: true }); - collection.update(id, { $set: { num: 22 } }, (error, result) => { - if (error) { - test.isTrue(!expected); - resolve(); - return; - } - test.isTrue(expected && result != null); - resolve(); - }); - }); - - await new Promise((resolve) => { - id = collection.insert({ force: true }); - collection.remove(id, (error, result) => { - if (error) { - test.isTrue(!expected); - resolve(); - return; - } - test.isTrue(expected && result != null); - resolve(); - }); - }); -} - -var AllowDenySyncRulesCollections = {}; - -testAsyncMulti("collection - sync definitions on allow/deny rules", [ - async function (test) { - AllowDenySyncRulesCollections.allowed = - AllowDenySyncRulesCollections.allowed || - new Mongo.Collection(`allowdeny-sync-rules-allowed`); - if (Meteor.isServer) { - await AllowDenySyncRulesCollections.allowed.removeAsync(); - } - - configAllSyncAllowDeny(AllowDenySyncRulesCollections.allowed, 'allow', true); - if (Meteor.isClient) { - await runAllSyncExpect(test, AllowDenySyncRulesCollections.allowed, true); - } - }, - async function (test) { - AllowDenySyncRulesCollections.notAllowed = - AllowDenySyncRulesCollections.notAllowed || - new Mongo.Collection(`allowdeny-sync-rules-notAllowed`); - if (Meteor.isServer) { - await AllowDenySyncRulesCollections.notAllowed.removeAsync(); - await AllowDenySyncRulesCollections.notAllowed.insertAsync({ num: 2 }); - } - - configAllSyncAllowDeny(AllowDenySyncRulesCollections.notAllowed, 'allow', false); - if (Meteor.isClient) { - await runAllSyncExpect(test, AllowDenySyncRulesCollections.notAllowed, false); - } - }, - async function (test) { - AllowDenySyncRulesCollections.denied = - AllowDenySyncRulesCollections.denied || - new Mongo.Collection(`allowdeny-sync-rules-denied`); - if (Meteor.isServer) { - await AllowDenySyncRulesCollections.denied.removeAsync(); - } - - configAllSyncAllowDeny(AllowDenySyncRulesCollections.denied, 'deny', true); - if (Meteor.isClient) { - await runAllSyncExpect(test, AllowDenySyncRulesCollections.denied, false); - } - }, - async function (test) { - AllowDenySyncRulesCollections.noRules = - AllowDenySyncRulesCollections.noRules || - new Mongo.Collection(`allowdeny-sync-rules-noRules`); - if (Meteor.isServer) { - await AllowDenySyncRulesCollections.noRules.removeAsync(); - } - - if (Meteor.isClient) { - await runAllSyncExpect(test, AllowDenySyncRulesCollections.noRules, false); - } - }, - async function (test) { - AllowDenySyncRulesCollections.allowThenDeny = - AllowDenySyncRulesCollections.allowThenDeny || - new Mongo.Collection(`allowdeny-sync-rules-allowThenDeny`); - if (Meteor.isServer) { - await AllowDenySyncRulesCollections.allowThenDeny.removeAsync(); - } - - configAllSyncAllowDeny(AllowDenySyncRulesCollections.allowThenDeny, 'allow', true); - configAllSyncAllowDeny(AllowDenySyncRulesCollections.allowThenDeny, 'deny', true); - if (Meteor.isClient) { - await runAllSyncExpect(test, AllowDenySyncRulesCollections.allowThenDeny, false); - } - }, -]); +testAsyncMulti("collection - sync definitions on allow/deny rules", createAllowDenyRulesTest(AllowDenyRulesCollections, false)); From 0e3c16aa6a1127d37d6c2abb44a8d60393684665 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nacho=20Codo=C3=B1er?= Date: Mon, 2 Dec 2024 17:33:58 +0100 Subject: [PATCH 09/10] reneable tests --- packages/mongo/package.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/mongo/package.js b/packages/mongo/package.js index 521b2e8277..16d28e6e72 100644 --- a/packages/mongo/package.js +++ b/packages/mongo/package.js @@ -124,13 +124,13 @@ Package.onTest(function (api) { ]); // XXX test order dependency: the allow_tests "partial allow" test // fails if it is run before mongo_livedata_tests. - // api.addFiles("tests/mongo_livedata_tests.js", ["client", "server"]); - // api.addFiles("tests/upsert_compatibility_test.js", "server"); + api.addFiles("tests/mongo_livedata_tests.js", ["client", "server"]); + api.addFiles("tests/upsert_compatibility_test.js", "server"); api.addFiles("tests/allow_tests.js", ["client", "server"]); - // api.addFiles("tests/collection_tests.js", ["client", "server"]); - // api.addFiles("tests/collection_async_tests.js", ["client", "server"]); - // api.addFiles("tests/observe_changes_tests.js", ["client", "server"]); - // api.addFiles("tests/oplog_tests.js", "server"); - // api.addFiles("tests/oplog_v2_converter_tests.js", "server"); - // api.addFiles("tests/doc_fetcher_tests.js", "server"); + api.addFiles("tests/collection_tests.js", ["client", "server"]); + api.addFiles("tests/collection_async_tests.js", ["client", "server"]); + api.addFiles("tests/observe_changes_tests.js", ["client", "server"]); + api.addFiles("tests/oplog_tests.js", "server"); + api.addFiles("tests/oplog_v2_converter_tests.js", "server"); + api.addFiles("tests/doc_fetcher_tests.js", "server"); }); From 24b452a81a33619d4dfe45f43a1c0140d84210fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nacho=20Codo=C3=B1er?= Date: Wed, 4 Dec 2024 17:28:07 +0100 Subject: [PATCH 10/10] fix a scenario on allow/deny rules tests due to insecure mode --- packages/mongo/tests/allow_tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mongo/tests/allow_tests.js b/packages/mongo/tests/allow_tests.js index d2647060b9..7fad697128 100644 --- a/packages/mongo/tests/allow_tests.js +++ b/packages/mongo/tests/allow_tests.js @@ -1327,7 +1327,7 @@ function createAllowDenyRulesTest(collections, isAsync = true) { } if (Meteor.isClient) { - await runAllExpect(test, collections.noRules, false, isAsync); + await runAllExpect(test, collections.noRules, collections.noRules._isInsecure(), isAsync); } }, async function (test) {