fix: ensure that rulesToQuery and rulesToAST generate condition that respect rule priority

Author: stalniy

packages/casl-ability/spec/rulesToAST.spec.js

@@ -43,19 +43,30 @@ describe('rulesToAST', () => {
     const ast = rulesToAST(ability, 'read', 'Post')
 
     expect(ast).to.deep.equal({
-      operator: 'and',
+      operator: 'or',
       value: [
         {
-          operator: 'not',
+          operator: 'and',
           value: [
-            { operator: 'eq', field: 'private', value: true }
+            { operator: 'eq', field: 'sharedWith', value: 1 },
+            {
+              operator: 'not',
+              value: [
+                { operator: 'eq', field: 'private', value: true }
+              ]
+            }
           ]
         },
         {
-          operator: 'or',
+          operator: 'and',
           value: [
-            { operator: 'eq', field: 'sharedWith', value: 1 },
             { operator: 'eq', field: 'author', value: 1 },
+            {
+              operator: 'not',
+              value: [
+                { operator: 'eq', field: 'private', value: true }
+              ]
+            }
           ]
         }
       ]

packages/casl-ability/spec/rulesToQuery.spec.js

@@ -132,16 +132,36 @@ describe('rulesToQuery', () => {
 
     expect(query).toEqual({
       $or: [
-        { state: 'draft' },
-        { _id: 'mega' }
-      ],
-      $and: [
-        { $not: { state: 'archived' } },
-        { $not: { private: true } }
+        {
+          $and: [
+            { state: 'draft' },
+            { $not: { state: 'archived' } },
+            { $not: { private: true } }
+          ]
+        },
+        {
+          $and: [
+            { _id: 'mega' },
+            { $not: { state: 'archived' } },
+            { $not: { private: true } }
+          ]
+        }
       ]
     })
   })
 
+  it('ignores inverted rules that are overridden by higher priority regular rules', () => {
+    const ability = defineAbility((can, cannot) => {
+      cannot('manage', 'Post', { id: 1 })
+      can('manage', 'Post', { id: 1 })
+    })
+    const query = toQuery(ability, 'manage', 'Post')
+
+    expect(query).toEqual({
+      $or: [{ id: 1 }]
+    })
+  })
+
   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 })

packages/casl-ability/src/extra/rulesToQuery.ts

@@ -1,7 +1,7 @@
 import { CompoundCondition, Condition, buildAnd, buildOr } from '@ucast/mongo2js';
-import { AnyAbility } from '../PureAbility';
-import { Generics, RuleOf } from '../RuleIndex';
-import { ExtractSubjectType } from '../types';
+import type { AnyAbility } from '../PureAbility';
+import type { RuleOf } from '../RuleIndex';
+import type { ExtractSubjectType } from '../types';
 
 export type RuleToQueryConverter<T extends AnyAbility, R = object> = (rule: RuleOf<T>) => R;
 export interface AbilityQuery<T = object> {
@@ -15,39 +15,15 @@ export function rulesToQuery<T extends AnyAbility, R = object>(
   subjectType: ExtractSubjectType<Parameters<T['rulesFor']>[1]>,
   convert: RuleToQueryConverter<T, R>
 ): AbilityQuery<R> | null {
-  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 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 {
-        // 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 {
-      list.push(convert(rule));
+  return mergeRules<T, R, AbilityQuery<R>>(
+    ability.rulesFor(action, subjectType),
+    convert,
+    {
+      and: (conditions) => ({ $and: conditions }),
+      or: (conditions) => ({ $or: conditions }),
+      empty: () => ({})
     }
-  }
-
-  // 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 {
@@ -63,19 +39,79 @@ export function rulesToAST<T extends AnyAbility>(
   action: Parameters<T['rulesFor']>[0],
   subjectType: ExtractSubjectType<Parameters<T['rulesFor']>[1]>,
 ): Condition | null {
-  const query = rulesToQuery(ability, action, subjectType, ruleToAST) as AbilityQuery<Condition>;
+  return mergeRules<T, Condition, Condition>(
+    ability.rulesFor(action, subjectType),
+    ruleToAST,
+    {
+      and: buildAnd,
+      or: (conditions) => conditions.length === 1 ? conditions[0] : buildOr(conditions),
+      empty: () => buildAnd([])
+    }
+  );
+}
 
-  if (query === null) {
-    return null;
+/**
+ * Converts CASL's sequential, switch-case priority enforcement into flat boolean logic.
+ *
+ * CASL evaluates rules from bottom to top (highest priority). When a record is evaluated:
+ * - If it matches a `cannot` rule, it returns `false`.
+ * - If it matches a `can` rule, it returns `true`.
+ * - Thus, a `can` rule is only reached if it was not intercepted by any higher-priority `cannot` rule.
+ *
+ * This function flattens this logic for database queries by isolating each `can` rule ("OR" branches)
+ * and strictly bounding it by all the preceding `cannot` conditions ("AND NOT" bounds).
+ * Because standard `$or` logic inherently absorbs the overlap of previously matched `can` paths,
+ * we don't mathematically need to subtract higher-priority `can` rules.
+ *
+ * @param rules - The sorted array of CASL rules (highest priority first).
+ * @param convert - The transformer mapping a CASL rule to the target query/AST format.
+ * @param hooks - The logical combination hooks for the target format.
+ */
+function mergeRules<T extends AnyAbility, R, Result>(
+  rules: RuleOf<T>[],
+  convert: (rule: RuleOf<T>) => R,
+  hooks: {
+    and: (conditions: R[]) => Result,
+    or: (conditions: R[]) => Result,
+    empty: () => Result,
   }
+): Result | null {
+  const higherCannots: R[] = [];
+  const orConditions: R[] = [];
+  let hasUnconditionalCan = false;
 
-  if (!query.$and) {
-    return query.$or ? buildOr(query.$or) : buildAnd([]);
+  for (let i = 0; i < rules.length; i++) {
+    const rule = rules[i];
+
+    if (rule.inverted) {
+      if (!rule.conditions) {
+        break; // stop evaluation on unconditional cannot
+      }
+      higherCannots.push(convert(rule));
+    } else {
+      if (!rule.conditions) {
+        hasUnconditionalCan = true;
+        break; // stop evaluation on unconditional can
+      }
+
+      let cond = convert(rule);
+      if (higherCannots.length > 0) {
+        cond = hooks.and([cond, ...higherCannots]) as unknown as R;
+      }
+      orConditions.push(cond);
+    }
   }
 
-  if (query.$or) {
-    query.$and.push(buildOr(query.$or));
+  if (hasUnconditionalCan) {
+    if (higherCannots.length === 0) {
+      return hooks.empty();
+    }
+    if (orConditions.length === 0) {
+      return hooks.and(higherCannots);
+    }
+    orConditions.push(hooks.and(higherCannots) as unknown as R);
   }
 
-  return buildAnd(query.$and);
+  if (orConditions.length === 0) return null;
+  return hooks.or(orConditions);
 }