Handling inverted and direct rules with same condition when interpreting SQL conditions

[mdwheele]

Hello!

First, thanks so much for this library! We've gotten a lot of use out of CASL so far. Up to this point, we've yet to convert resource collections of our REST API to be scoped based on the CASL ability instance for a given user (we're reliant on legacy code to do this). Making this leap is one of the last steps we need to take to deprecate our legacy permissions model implementation.

We use knex and scope queries on REST resource collections before applying user-provided filters onto the query. We would instead like to use the interpreter in @ucast/sql to process the Ability instance AST; producing a raw where constraint we can apply to our query builder.

For additive permissions, everything seems to work as expected. However, we need to be able to support negative (inverted) permissions as well. The ability instance handles this great when querying permissions on individual resources. However, something is lost in translation in our processing of the underlying AST when producing SQL and after tinkering for a while and doing some research, we're left thinking this HAS to be a common problem others have solved.

I've created a simple example that demonstrates what we're trying to achieve:

const { subject, createMongoAbility, AbilityBuilder } = require('@casl/ability')
const { rulesToAST } = require('@casl/ability/extra')
const { allInterpreters, createSqlInterpreter, mysql } = require('@ucast/sql')

const { can, cannot, build } = new AbilityBuilder(createMongoAbility)

cannot('manage', 'Post', { id: 1 })
can('manage', 'Post', { id: 1 })

const ability = build()

console.log(
  `We can update a post with id === 1:`,
  ability.can('update', subject('Post', { id: 1 }))
)

const condition = rulesToAST(ability, 'update', 'Post')

console.log(`Our condition AST is...`)
console.log(JSON.stringify(condition, null, 2))

const interpret = createSqlInterpreter(allInterpreters)
const [sql, replacements] = interpret(condition, {
  ...mysql,
  paramPlaceholder: () => '?',
})

console.log(
  `The matching SQL constraint would be:`,
  sql,
  replacements
)

Output when ordering is cannot before can

We can update a post with id === 1: true
Our condition AST is...
{
  "operator": "and",
  "value": [
    {
      "operator": "not",
      "value": [
        {
          "operator": "eq",
          "value": 1,
          "field": "id"
        }
      ]
    },
    {
      "operator": "eq",
      "value": 1,
      "field": "id"
    }
  ]
}

The matching SQL constraint would be: (not (`id` = ?) and `id` = ?) [ 1, 1 ]

Output when ordering is can before cannot

We can update a post with id === 1: false
Our condition AST is...
{
  "operator": "and",
  "value": [
    {
      "operator": "not",
      "value": [
        {
          "operator": "eq",
          "value": 1,
          "field": "id"
        }
      ]
    },
    {
      "operator": "eq",
      "value": 1,
      "field": "id"
    }
  ]
}

The matching SQL constraint would be: (not (`id` = ?) and `id` = ?) [ 1, 1 ]

A few important notes:

1. If the order of rules is changed, the runtime ability.can check behaves as expected. I will lose access if the cannot comes last and I retain access if can is last.
2. Regardless of the order, the AST is the same; which makes sense! It should theoretically be the same. However, I'm lost on how to translate this to SQL in that case such that the ordering of the rules produces different SQL.

We've also come across https://gist.github.com/ygrishajev/9ef01444fdb5c386c43b6611400c0fc6 which uses rulesToQuery to return AST nodes and constructs a compound condition manually. However, it seems to hit this same issue when there are inverted and non-inverted rules applied to a resource where you get a lossy translation to the database; differing from results you see calling ability.can.

[stalniy]

I also always had hard time to translate such intersection to DB query and as a result this edge case has never been solved on casl side.

The recommendation says to get rid of inverted rules as much as possible. If you do not use them then you don’t have to solve this edge case.

[stalniy]

But it works better than it sounds from my message above. Because the issue appears only if we use the same property in can and cannot rules. So the edge case is actually smaller

you can safely combine can and cannot rules for db queries if they do not use the same attributes.

But if they use the same attributes the logic should be changed and more complex SQL query must be generated by interpreter

[stalniy]

There is a way to make it work in SQL via CASE/END statement but very likely db won't be able to use indexes on that condition. Which may be a good tradeoff.

Obviously this can be optimized to normal WHERE condition almost in every case but it requires complex analysis of the conditions.

For example this:

cannot('manage', 'Post', { id: { $gte 5 } })
can('manage', 'Post', { id: 10 })
can('manage', 'Post', { id: 1 })
cannot('manage', 'Post', { id: 1, private: true })

is translated to:

CASE
  WHEN id = 1 AND private = true THEN FALSE
  WHEN id = 1 THEN TRUE
  WHEN id = 10 THEN TRUE
  WHEN id >= 5 THEN FALSE
END

But it can be rewritten to

(id = 1 AND private = false) OR id = 10 OR id < 5

and the actual casl rules also can be rewritten to get this better result:

can('manage', 'Post', { id: { $lt 5 } })
can('manage', 'Post', { id: 10 })
can('manage', 'Post', { id: 1, private: false })

That's why the general recommendation to use direct rules still remains valid and will produce more optimized SQL query eventually

[stalniy]

The same is possible in MongoDB, either with $where (for < v4.2) and with $expr + $switch (>=v4.2). At least it will work in consistent way between db and runtime and the rest can be optimized

[mdwheele]

Thanks so much for the in-depth response @stalniy!

We're going to stick with this and invest some more time here on possible solutions to consistency between runtime and mapping to MariaDB. Knowing this is an edge-case motivates spending a bit more time trying to figure something out that could work.

Toward your recommendation to not rely on inverted rules: we can certainly model "negative permissions" and map that to what the direct rules should be when hydrating users' Ability instances. I want to spend a bit more time working on a successful runtime-to-database mapping that includes conflicting direct/inverted rules, but I do fear that what we'll end up with is much less comprehensible than simple direct rules.

Thanks again for the advice. We'll add any further insight we glean to this issue.