Skip to content

Commit

Permalink
fix: 🐛 consider rules without fields in select queries
Browse files Browse the repository at this point in the history
  • Loading branch information
dennemark committed Jul 26, 2024
1 parent 140ae9c commit 2a222ae
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 11 deletions.
65 changes: 61 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
[Prisma client extension](https://www.prisma.io/docs/orm/prisma-client/client-extensions) that utilizes [CASL](https://casl.js.org/) to enforce authorization logic on most queries.

> [!CAUTION]
>
> WIP - some abstractions might change in the future and lead to different interpretation of CASL rules.
>
> Please be very careful using this library in production! Test your endpoints on your own and raise an issue if some case is not supported by this library!
Supports mainly/only CRUD actions `create`, `read`, `update` and `delete`, which allows us to generate and transform `include`, `select` and `where` queries to enforce nested filtering.
Expand Down Expand Up @@ -75,17 +78,64 @@ Check out tests for some other examples.

### Limitations and Constraints

- On nested `connect`, `disconnect`, `upsert` or `connectOrCreate` mutation queries the client assumes an `update` action for insertion or connection.
#### Nested mutations use `update` connection

- A limitation is the necessary use of `create`, `read`, `update` and `delete` as actions for nested queries. Since this allows us to deal with nested creations or updates. However there is an option to specify a custom `caslAction` for the highest query. It has no typing and is not tested yet.
On nested `connect`, `disconnect`, `upsert` or `connectOrCreate` mutation queries the client assumes an `update` action for insertion or connection.


#### CRUD actions

A limitation is the necessary use of `create`, `read`, `update` and `delete` as actions for nested queries. Since this allows us to deal with nested creations or updates. However there is an option to specify a custom `caslAction` for the highest query. It has no typing and is not tested yet.

```ts
client.user.findUnique({ where: { id: 0 }, caslAction: 'customAction' })
```

- When using prisma probably no one will use columns named `data`, `create`, `update` or `where`. However, if this should be the case, then this library most probably won't work.
#### Avoid columns with prisma naming

- Currently the following case is not supported. It is still possible to read `email`. Waithing for reply [here](https://github.com/stalniy/casl/discussions/948).
When using prisma probably no one will use columns named `data`, `create`, `update` or `where`. However, if this should be the case, then this library most probably won't work.

#### Limit fields via conditions
The main use case is allowing more fields for some users:
```ts
can('read', 'User', 'email')
can('read', 'User', ['email', 'id'], {
id: 0
})
client.user.findMany()
// will return all users with email
// however
client.user.findMany({ id: 0 })
// will show email and id!
```
If fields should only be permitted on certain conditions, they will only be accessible, if these conditions apply. The reason is, that we do not know if it contradicts another rule.
See this example:

```ts
can('read', 'User')
can('read', 'User', 'email', {
id: 0
})
can('read', 'User', 'id', {
id: 1
})
client.user.findMany()
// will return all users and more than the email property
// since we cannot check if id is 0 or 1

// however if our query matches the condition its rule will be used
client.user.findMany({ id: 0 })
// will restrict access to email prop
```
This makes DX a bit inconvenient, since we have to use our condition within our query.


#### Nested fields and wildcards are not supported

`can('read', 'User', ['nested.field', 'field.*'])` won't work. Although a wildcard could be useful in the future.

#### Conditionally filter fields with cannot
Currently the following case is not supported. It is still possible to read `email`. Waithing for reply [here](https://github.com/stalniy/casl/discussions/948).

```ts
can('read', 'User', {
Expand All @@ -95,3 +145,10 @@ Check out tests for some other examples.
id: 0
})
```
In real world application, rather consider this:
```ts
cannot('read', 'User', 'email')
can('read', 'User', {
id: userId
})
```
41 changes: 34 additions & 7 deletions src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,33 +82,60 @@ export const propertyFieldsByModel = Object.fromEntries(Prisma.dmmf.datamodel.mo
return [model.name, propertyFields]
}))


/**
* goes through all permitted fields of a model
*
* - if there area only rules with fields, permittedFields are at least empty
*
* - if a rule has fields And conditions, it's fields are only permitted
* if the query overlaps with the conditions
*
* - if permittedFields are empty, but there is a rule without fields,
* the result is undefined. allowing us to query for all fields
*
* @param abilities
* @param args
* @param action
* @param model
* @returns
*/
export function getPermittedFields(
abilities: PureAbility<AbilityTuple, PrismaQuery>,
args: any,
action: string,
model: string
){
let hasPermittedFields = false
let hasNoRuleWithoutFields: boolean | undefined = undefined
const omittedFieldsSet = new Set()
const permittedFields = permittedFieldsOf(abilities, action, model, {
fieldsFrom: rule => {
if(hasNoRuleWithoutFields === undefined){
// make assumption true on first call of fieldsFrom
hasNoRuleWithoutFields = true
}
if (rule.fields) {
if(rule.inverted){
rule.fields.forEach((field)=>omittedFieldsSet.add(field))
}
else{
} else {
hasPermittedFields = true
}
if (rule.conditions) {
if (isSubset(rule.conditions, args.where)) {
return rule.fields
} else {
// console.warn(`${model} fields ${JSON.stringify(rule.fields)} can only be read with following conditions: ${JSON.stringify(rule.conditions)}`)
}
}
// else if(process.env.NODE_ENV === 'development' || process.env.NODE_ENV === 'test') {
// const queriedFields = args.select ? Object.keys(args.select) : args.include ? Object.keys(args.include) : []

// if(queriedFields.findIndex((field)=> rule.fields?.includes(field)) > -1){
// console.warn(`${model} fields ${JSON.stringify(rule.fields)} can only be read with following conditions: ${JSON.stringify(rule.conditions)}`)
// }
// }
} else {
return rule.fields
}
}else{
hasNoRuleWithoutFields = false
}
return []
}
Expand All @@ -120,7 +147,7 @@ export function getPermittedFields(
permittedFields.push(...Object.keys(propertyFieldsByModel[model]).filter((field)=> !omittedFieldsSet.has(field)))
hasPermittedFields = true
}
return hasPermittedFields ? permittedFields : undefined
return hasPermittedFields && permittedFields.length > 0 ? permittedFields : hasNoRuleWithoutFields ? [] : undefined
}


Expand Down
28 changes: 28 additions & 0 deletions test/applyCaslToQuery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,35 @@ describe('apply casl to query', () => {
}
})
})
it('ignores conditional rule, if not part of query', ()=>{
const { can, build } = abilityBuilder()
can('read', 'all' as any)
can('read', 'User', ['email', 'id'], {
id: 0
})
const abilities = build()
const result = applyCaslToQuery('findUnique', {}, abilities, 'User')
expect(result).toEqual({})
})

it('applies filter props and ignores weaker can rule', ()=>{
const { can, build } = abilityBuilder()
can('read', 'User', {
id: 0
})
can('read', 'User', ['email', 'id'])
const abilities = build()
const result = applyCaslToQuery('findUnique', {}, abilities, 'User')
expect(result).toEqual({ select: { email: true, id: true }})
})
it('allows to see more props on a condition', ()=>{
const { can, build } = abilityBuilder()
can('read', 'User', 'email')
can('read', 'User', ['email', 'id'], {id:0})
const abilities = build()
const result = applyCaslToQuery('findUnique', { where: { id: 0 } }, abilities, 'User')
expect(result).toEqual({ where: { id: 0 }, select: { email: true, id: true }})
})
Object.entries(caslOperationDict).map(([operation, settings]) => {
it(`${operation} applies ${settings.dataQuery ? 'data' : 'no data'} ${settings.whereQuery ? 'where' : 'no where'} and ${settings.includeSelectQuery ? 'include/select' : 'no include/select'} query`, () => {
const { can, build } = abilityBuilder()
Expand Down
29 changes: 29 additions & 0 deletions test/applyIncludeSelectQuery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,35 @@ describe('apply include select query', () => {
}
})
}*/)
it('applies select method and does not allow reading of field', () => {
const { can, cannot, build } = abilityBuilder()
can('read', 'User')
cannot('read', 'User', 'email', {
id: 0
})
const args = applyIncludeSelectQuery(build(), {
where: {
author: {
id: 0
}
},
select: {
author: true
}
}, 'Post')
expect(args).toEqual({
select: {
author: {
select: undefined
}
},
where: {
author: {
id: 0
}
}
})
})
it('applies select method on array', () => {
const { can, cannot, build } = abilityBuilder()
can('read', 'Post', 'id', {
Expand Down

0 comments on commit 2a222ae

Please sign in to comment.