Skip to content

Commit

Permalink
refactor: adds comments to rulesToQuery and Rule matching and adds mo…
Browse files Browse the repository at this point in the history 
…re tests
  • Loading branch information
@stalniy
stalniy committed Jul 26, 2024
1 parent 14bf935 commit 11a84ca
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 38 deletions.
83 changes: 55 additions & 28 deletions packages/casl-ability/spec/rulesToQuery.spec.js
View file
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -39,40 +33,48 @@ 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 })
cannot('read', 'Post')
})
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 })
Expand All @@ -81,45 +83,45 @@ 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' }
]
})
})

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' })
cannot('read', 'Post', { status: 'published', createdBy: 'me' })
})
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' } }
]
})
})

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' })
Expand All @@ -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' }
Expand All @@ -140,14 +142,39 @@ 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 })
can('read', 'Post')
})
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 }]
})
})
})
6 changes: 4 additions & 2 deletions packages/casl-ability/src/Rule.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,15 @@ export class Rule<A extends Abilities, C> {
}

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);
}
}
28 changes: 20 additions & 8 deletions packages/casl-ability/src/extra/rulesToQuery.ts
View file
Original file line number Diff line number Diff line change
@@ -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<T extends AnyAbility, R = object> = (rule: RuleOf<T>) => R;
Expand All @@ -15,27 +15,39 @@ export function rulesToQuery<T extends AnyAbility, R = object>(
subjectType: ExtractSubjectType<Parameters<T['rulesFor']>[1]>,
convert: RuleToQueryConverter<T, R>
): AbilityQuery<R> | null {
const query: AbilityQuery<R> = {};
const $and: Generics<T>['conditions'][] = [];
const $or: Generics<T>['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<AnyAbility>): Condition {
Expand Down

0 comments on commit 11a84ca

Please sign in to comment.