feat(mongoose): throws ForbiddenError instead of returning a hard-c…

…oded value when user has not permissions to do some action

Fixes #404

BREAKING CHANGE: `accessibleBy` eventually throws `ForbiddenError` instead of returning a hard-coded value

  **Before**:

  ```ts
  // ability doesn't allow to read Post
  const ability = defineAbility(can => can('manage', 'Comment'));

  try {
    const items = await Post.accessibleBy(ability, 'read');
    console.log(items); // []
  } catch (error) {
    console.error(error); // no error thrown
  }
  ```

  **After**:

  ```ts
  // ability doesn't allow to read Post
  const ability = defineAbility(can => can('manage', 'Comment'));

  try {
    const items = await Post.accessibleBy(ability, 'read');
    console.log(items); // not reached, because query fails with error
  } catch (error) {
    console.error(error); // ForbiddenError thrown
  }
  ```

.eslintrc

@@ -58,7 +58,8 @@
       },
       "globals": {
         "expect": true,
-        "spy": true
+        "spy": true,
+        "fail": true
       },
       "rules": {
         "@typescript-eslint/semi": ["error", "never"],

packages/casl-mongoose/spec/accessible_records.spec.js

@@ -1,4 +1,4 @@
-import { defineAbility } from '@casl/ability'
+import { defineAbility, ForbiddenError } from '@casl/ability'
 import mongoose from 'mongoose'
 import { accessibleRecordsPlugin, toMongoQuery } from '../src'
 
@@ -84,57 +84,96 @@ describe('Accessible Records Plugin', () => {
         query = Post.find().accessibleBy(ability, 'notAllowedAction')
       })
 
-      it('adds non-existing property check to conditions for other callback based cases', async () => {
-        expect(query.getQuery()).to.have.property('__forbiddenByCasl__').that.equal(1)
-      })
-
-      it('returns empty array for collection request', async () => {
-        const items = await query.exec()
-
-        expect(items).to.be.an('array').that.is.empty
-      })
-
-      it('returns empty array when `find` operation is passed to `exec`', async () => {
-        const items = await query.exec('find')
-
-        expect(items).to.be.an('array').that.is.empty
-      })
-
-      it('returns empty array for collection request when callback is passed to `exec`', async () => {
-        const items = await wrapInPromise(cb => query.exec(cb))
-
-        expect(items).to.be.an('array').that.is.empty
-      })
-
-      it('returns `null` for item request', async () => {
-        const item = await query.findOne().exec()
-
-        expect(item).to.be.null
-      })
-
-      it('returns `null` when `findOne` operation is passed to `exec`', async () => {
-        const item = await query.exec('findOne')
-
-        expect(item).to.be.null
-      })
-
-      it('returns `null` for item request when callback is passed to `exec`', async () => {
-        const item = await wrapInPromise(cb => query.findOne().exec(cb))
-
-        expect(item).to.be.null
-      })
-
-      it('returns `0` for count request', async () => {
-        const count = await query.count()
-
-        expect(count).to.equal(0)
-      })
-
-      it('calls original `exec` for other cases', async () => {
-        Post.Query.prototype.exec = spy(() => Promise.resolve('original `exec` method call'))
-        await query.update({ $set: { state: 'draft' } })
-
-        expect(Post.Query.prototype.exec).to.have.been.called()
+      describe('when exist private `_pre` method', () => {
+        it('throws `ForbiddenError` for collection request', async () => {
+          await query.exec()
+            .then(() => fail('should not execute'))
+            .catch((error) => {
+              expect(error).to.be.instanceOf(ForbiddenError)
+              expect(error.message).to.match(/cannot execute/i)
+            })
+        })
+
+        it('throws `ForbiddenError` when `find` operation is passed to `exec`', async () => {
+          await query.exec('find')
+            .then(() => fail('should not execute'))
+            .catch((error) => {
+              expect(error).to.be.instanceOf(ForbiddenError)
+              expect(error.message).to.match(/cannot execute/i)
+            })
+        })
+
+        it('throws `ForbiddenError` when callback is passed to `exec`', async () => {
+          await wrapInPromise(cb => query.exec(cb))
+            .then(() => fail('should not execute'))
+            .catch((error) => {
+              expect(error).to.be.instanceOf(ForbiddenError)
+              expect(error.message).to.match(/cannot execute/i)
+            })
+        })
+
+        it('throws `ForbiddenError` for item request', async () => {
+          await query.findOne().exec()
+            .then(() => fail('should not execute'))
+            .catch((error) => {
+              expect(error).to.be.instanceOf(ForbiddenError)
+              expect(error.message).to.match(/cannot execute/i)
+            })
+        })
+
+        it('throws `ForbiddenError` when `findOne` operation is passed to `exec`', async () => {
+          await query.exec('findOne')
+            .then(() => fail('should not execute'))
+            .catch((error) => {
+              expect(error).to.be.instanceOf(ForbiddenError)
+              expect(error.message).to.match(/cannot execute/i)
+            })
+        })
+
+        it('throws `ForbiddenError` for item request when callback is passed to `exec`', async () => {
+          await wrapInPromise(cb => query.findOne().exec(cb))
+            .then(() => fail('should not execute'))
+            .catch((error) => {
+              expect(error).to.be.instanceOf(ForbiddenError)
+              expect(error.message).to.match(/cannot execute/i)
+            })
+        })
+
+        it('throws `ForbiddenError` for count request', async () => {
+          await query.count()
+            .then(() => fail('should not execute'))
+            .catch((error) => {
+              expect(error).to.be.instanceOf(ForbiddenError)
+              expect(error.message).to.match(/cannot execute/i)
+            })
+        })
+
+        it('throws `ForbiddenError` for count request', async () => {
+          await query.count()
+            .then(() => fail('should not execute'))
+            .catch((error) => {
+              expect(error).to.be.instanceOf(ForbiddenError)
+              expect(error.message).to.match(/cannot execute/i)
+            })
+        })
+
+        it('throws `ForbiddenError` for `countDocuments` request', async () => {
+          await query.countDocuments()
+            .then(() => fail('should not execute'))
+            .catch((error) => {
+              expect(error).to.be.instanceOf(ForbiddenError)
+              expect(error.message).to.match(/cannot execute/i)
+            })
+        })
+
+        it('throws `ForbiddenError` for `countDocuments` request', async () => {
+          await query.estimatedDocumentCount()
+            .then(() => fail('should not execute'))
+            .catch((error) => {
+              expect(error).to.be.instanceOf(ForbiddenError)
+              expect(error.message).to.match(/cannot execute/i)
+            })
+        })
       })
     })
   })

packages/casl-mongoose/src/accessible_records.ts

@@ -1,33 +1,28 @@
-import { Normalize, AnyMongoAbility, Generics } from '@casl/ability';
+import { Normalize, AnyMongoAbility, Generics, ForbiddenError, getDefaultErrorMessage } from '@casl/ability';
 import { Schema, DocumentQuery, Query, Model, Document } from 'mongoose';
 import { toMongoQuery } from './mongo';
 
-const DENY_CONDITION_NAME = '__forbiddenByCasl__'; // eslint-disable-line
-
-function returnQueryResult(this: any, methodName: string, returnValue: any, ...args: any[]) {
-  const [conditions, , callback] = args;
-
-  if (conditions[DENY_CONDITION_NAME]) {
-    return typeof callback === 'function'
-      ? callback(null, returnValue)
-      : Promise.resolve(returnValue);
-  }
-
-  if (conditions.hasOwnProperty(DENY_CONDITION_NAME)) {
-    delete conditions[DENY_CONDITION_NAME];
-  }
-
-  return this[methodName].apply(this, args);
-}
-
-function emptifyQuery(query: DocumentQuery<Document, Document>) {
-  query.where({ [DENY_CONDITION_NAME]: 1 });
-  const privateQuery: any = query;
-  const collection = Object.create(privateQuery._collection); // eslint-disable-line
-  privateQuery._collection = collection; // eslint-disable-line
-  collection.find = returnQueryResult.bind(collection, 'find', []);
-  collection.findOne = returnQueryResult.bind(collection, 'findOne', null);
-  collection.count = returnQueryResult.bind(collection, 'count', 0);
+function failedQuery(
+  ability: AnyMongoAbility,
+  action: string,
+  modelName: string,
+  query: DocumentQuery<Document, Document>
+) {
+  const error = ForbiddenError.from(ability);
+  error.action = action;
+  error.subjectType = modelName;
+  error.setMessage(getDefaultErrorMessage(error));
+
+  query.where({ __forbiddenByCasl__: 1 }); // eslint-disable-line
+  query.exec = function patchedExecByCasl(...args: any[]) {
+    const cb = typeof args[0] === 'function' ? args[0] : args[1];
+    if (typeof cb === 'function') {
+      process.nextTick(() => cb(error));
+      return;
+    }
+    // eslint-disable-next-line consistent-return
+    return Promise.reject(error);
+  } as typeof query['exec'];
 
   return query;
 }
@@ -52,11 +47,10 @@ function accessibleBy<T extends AnyMongoAbility>(
     throw new TypeError('Cannot detect model name to return accessible records');
   }
 
-  const toQuery = toMongoQuery as (...args: any[]) => ReturnType<typeof toMongoQuery>;
-  const query = toQuery(ability, modelName, action);
+  const query = toMongoQuery(ability, modelName, action);
 
   if (query === null) {
-    return emptifyQuery(this.where());
+    return failedQuery(ability, action || 'read', modelName, this.where());
   }
 
   return this instanceof Query ? this.and([query]) : this.where({ $and: [query] });