diff --git a/packages/casl-ability/spec/rulesToQuery.spec.js b/packages/casl-ability/spec/rulesToQuery.spec.js index b64394cbc..a34c6e599 100644 --- a/packages/casl-ability/spec/rulesToQuery.spec.js +++ b/packages/casl-ability/spec/rulesToQuery.spec.js @@ -1,6 +1,5 @@ import { defineAbility } from '../src' import { rulesToQuery } from '../src/extra' -import './spec_helper' function toQuery(ability, action, subject) { const convert = rule => rule.inverted ? { $not: rule.conditions } : rule.conditions @@ -12,14 +11,8 @@ describe('rulesToQuery', () => { const ability = defineAbility(can => can('read', 'Post')) const query = toQuery(ability, 'read', 'Post') - expect(Object.keys(query)).to.be.empty - }) - - it('returns `null` if empty `Ability` instance is passed', () => { - const ability = defineAbility(() => {}) - const query = toQuery(ability, 'read', 'Post') - - expect(query).to.be.null + expect(query).toBeInstanceOf(Object) + expect(Object.keys(query)).toHaveLength(0) }) it('returns empty `$or` part if at least one regular rule does not have conditions', () => { @@ -29,7 +22,8 @@ describe('rulesToQuery', () => { }) const query = toQuery(ability, 'read', 'Post') - expect(Object.keys(query)).to.be.empty + expect(query).toBeInstanceOf(Object) + expect(Object.keys(query)).toHaveLength(0) }) it('returns empty `$or` part if rule with conditions defined last', () => { @@ -39,29 +33,37 @@ describe('rulesToQuery', () => { }) const query = toQuery(ability, 'read', 'Post') - expect(Object.keys(query)).to.be.empty + expect(query).toBeInstanceOf(Object) + expect(Object.keys(query)).toHaveLength(0) + }) + + it('returns `null` if empty `Ability` instance is passed', () => { + const ability = defineAbility(() => {}) + const query = toQuery(ability, 'read', 'Post') + + expect(query).toBe(null) }) it('returns `null` if specified only inverted rules', () => { - const ability = defineAbility((can, cannot) => { + const ability = defineAbility((_, cannot) => { cannot('read', 'Post', { private: true }) }) const query = toQuery(ability, 'read', 'Post') - expect(query).to.be.null + expect(query).toBe(null) }) - it('returns `null` if at least one inverted rule does not have conditions', () => { - const ability = defineAbility((can, cannot) => { + it('returns `null` if inverted rule does not have conditions and there are no direct rules', () => { + const ability = defineAbility((_, cannot) => { cannot('read', 'Post', { author: 123 }) cannot('read', 'Post') }) const query = toQuery(ability, 'read', 'Post') - expect(query).to.be.null + expect(query).toBe(null) }) - it('returns `null` if at least one inverted rule does not have conditions even if direct condition exists', () => { + it('returns `null` if at least one inverted rule does not have conditions even if direct rule exists', () => { const ability = defineAbility((can, cannot) => { can('read', 'Post', { public: true }) cannot('read', 'Post', { author: 321 }) @@ -69,10 +71,10 @@ describe('rulesToQuery', () => { }) const query = toQuery(ability, 'read', 'Post') - expect(query).to.be.null + expect(query).toBe(null) }) - it('returns non-`null` if there is at least one regular rule after last inverted one without conditions', () => { + it('returns query if there is at least one regular rule after last inverted one without conditions', () => { const ability = defineAbility((can, cannot) => { can('read', 'Post', { public: true }) cannot('read', 'Post', { author: 321 }) @@ -81,21 +83,21 @@ describe('rulesToQuery', () => { }) const query = toQuery(ability, 'read', 'Post') - expect(query).to.deep.equal({ + expect(query).toEqual({ $or: [ { author: 123 } ] }) }) - it('OR-es conditions for regular rules', () => { + it('OR-s conditions for regular rules', () => { const ability = defineAbility((can) => { can('read', 'Post', { status: 'draft', createdBy: 'someoneelse' }) can('read', 'Post', { status: 'published', createdBy: 'me' }) }) const query = toQuery(ability, 'read', 'Post') - expect(query).to.deep.equal({ + expect(query).toEqual({ $or: [ { status: 'published', createdBy: 'me' }, { status: 'draft', createdBy: 'someoneelse' } @@ -103,7 +105,7 @@ describe('rulesToQuery', () => { }) }) - it('AND-es conditions for inverted rules', () => { + it('AND-s conditions for inverted rules', () => { const ability = defineAbility((can, cannot) => { can('read', 'Post') cannot('read', 'Post', { status: 'draft', createdBy: 'someoneelse' }) @@ -111,7 +113,7 @@ describe('rulesToQuery', () => { }) const query = toQuery(ability, 'read', 'Post') - expect(query).to.deep.equal({ + expect(query).toEqual({ $and: [ { $not: { status: 'published', createdBy: 'me' } }, { $not: { status: 'draft', createdBy: 'someoneelse' } } @@ -119,7 +121,7 @@ describe('rulesToQuery', () => { }) }) - it('OR-es conditions for regular rules and AND-es for inverted ones', () => { + it('OR-s conditions for regular rules and AND-es for inverted ones', () => { const ability = defineAbility((can, cannot) => { can('read', 'Post', { _id: 'mega' }) can('read', 'Post', { state: 'draft' }) @@ -128,7 +130,7 @@ describe('rulesToQuery', () => { }) const query = toQuery(ability, 'read', 'Post') - expect(query).to.deep.equal({ + expect(query).toEqual({ $or: [ { state: 'draft' }, { _id: 'mega' } @@ -140,7 +142,7 @@ describe('rulesToQuery', () => { }) }) - it('returns empty `$and` part if inverted rule with conditions defined before regular rule without conditions', () => { + it('returns empty query if inverted rule with conditions defined before regular rule without conditions', () => { const ability = defineAbility((can, cannot) => { can('read', 'Post', { author: 123 }) cannot('read', 'Post', { private: true }) @@ -148,6 +150,31 @@ describe('rulesToQuery', () => { }) const query = toQuery(ability, 'read', 'Post') - expect(Object.keys(query)).to.be.empty + expect(query).toBeInstanceOf(Object) + expect(Object.keys(query)).toHaveLength(0) + }) + + it('should ignore inverted rules with fields and conditions', () => { + const ability = defineAbility((can, cannot) => { + can('read', 'Post', { author: 123 }) + cannot('read', 'Post', 'description', { private: true }) + }) + const query = toQuery(ability, 'read', 'Post') + + expect(query).toEqual({ + $or: [{ author: 123 }] + }) + }) + + it('should ignore inverted rules with fields and without conditions', () => { + const ability = defineAbility((can, cannot) => { + can('read', 'Post', { author: 123 }) + cannot('read', 'Post', 'description') + }) + const query = toQuery(ability, 'read', 'Post') + + expect(query).toEqual({ + $or: [{ author: 123 }] + }) }) }) diff --git a/packages/casl-ability/src/Rule.ts b/packages/casl-ability/src/Rule.ts index 705275a5c..60ce2106b 100644 --- a/packages/casl-ability/src/Rule.ts +++ b/packages/casl-ability/src/Rule.ts @@ -95,13 +95,15 @@ export class Rule { } if (!field) { + // if there is no field (i.e., checking whether user has access to at least one field on subject) + // we ignore inverted rules because they disallow to do an action on it, so we are continue looking for regular rule return !this.inverted; } - if (this.fields && !this._matchField) { + if (!this._matchField) { this._matchField = this._options.fieldMatcher!(this.fields); } - return this._matchField!(field); + return this._matchField(field); } } diff --git a/packages/casl-ability/src/extra/rulesToQuery.ts b/packages/casl-ability/src/extra/rulesToQuery.ts index e785be4d7..1b9d89143 100644 --- a/packages/casl-ability/src/extra/rulesToQuery.ts +++ b/packages/casl-ability/src/extra/rulesToQuery.ts @@ -1,6 +1,6 @@ import { CompoundCondition, Condition, buildAnd, buildOr } from '@ucast/mongo2js'; import { AnyAbility } from '../PureAbility'; -import { RuleOf } from '../RuleIndex'; +import { Generics, RuleOf } from '../RuleIndex'; import { ExtractSubjectType } from '../types'; export type RuleToQueryConverter = (rule: RuleOf) => R; @@ -15,27 +15,39 @@ export function rulesToQuery( subjectType: ExtractSubjectType[1]>, convert: RuleToQueryConverter ): AbilityQuery | null { - const query: AbilityQuery = {}; + const $and: Generics['conditions'][] = []; + const $or: Generics['conditions'][] = []; const rules = ability.rulesFor(action, subjectType); for (let i = 0; i < rules.length; i++) { const rule = rules[i]; - const op = rule.inverted ? '$and' : '$or'; + const list = rule.inverted ? $and : $or; if (!rule.conditions) { if (rule.inverted) { + // stop if inverted rule without fields and conditions + // Example: + // can('read', 'Post', { id: 2 }) + // cannot('read', "Post") + // can('read', 'Post', { id: 5 }) break; } else { - delete query[op]; - return query; + // if it allows reading all types then remove previous conditions + // Example: + // can('read', 'Post', { id: 1 }) + // can('read', 'Post') + // cannot('read', 'Post', { status: 'draft' }) + return $and.length ? { $and } : {}; } } else { - query[op] = query[op] || []; - query[op]!.push(convert(rule)); + list.push(convert(rule)); } } - return query.$or ? query : null; + // if there are no regular conditions and the where no rule without condition + // then user is not allowed to perform this action on this subject type + if (!$or.length) return null; + return $and.length ? { $or, $and } : { $or }; } function ruleToAST(rule: RuleOf): Condition {