Skip to content

Commit

Permalink
fix: 🐛 correct accessible check of updated models
Browse files Browse the repository at this point in the history
  • Loading branch information
dennemark committed Oct 11, 2024
1 parent 48e5bef commit 53ed04b
Show file tree
Hide file tree
Showing 7 changed files with 210 additions and 33 deletions.
12 changes: 6 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,12 @@ console.log(result) // [{ email: "-" }]
Here are some performance metrics for the above query for the small test sqlite db:
- plain prisma query: **0.4236**
- casl prisma query: **3.09900**
- create abilities: **0.21132**
- enrich query with casl: **0.09345**
- prisma query: **2.71646**
- filtering query results: **0.07777**
- plain prisma query: **0.56225**
- casl prisma query: **3.33416**
- create abilities: **0.18987**
- enrich query with casl: **0.09284**
- prisma query: **2.97764**
- filtering query results: **0.07380**
#### Nested fields and wildcards are not supported / tested
Expand Down
16 changes: 8 additions & 8 deletions src/applyDataQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export function applyDataQuery(
model: string,
creationTree?: CreationTree
) {
const tree = creationTree ? creationTree : { action: action, model: model, children: {} } as CreationTree
const tree = creationTree ? creationTree : { action: action, model: model, children: {}, mutation: [] } as CreationTree

const permittedFields = getPermittedFields(abilities, action, model)

Expand All @@ -57,15 +57,15 @@ export function applyDataQuery(
return field in propertyFieldsByModel[model]
})
}))

tree.mutation.push({ fields: [...argFields], where: argsEntry.where })
const nestedAbilities = createPrismaAbility(abilities.rules.filter((rule) => {
if (rule.fields && rule.subject === model) {
if (rule.inverted) {
const hasNoForbiddenFields = argFields.isDisjointFrom(new Set(rule.fields))
if (!rule.conditions && !hasNoForbiddenFields) {
throw new Error(`It's not allowed to "${action}" "${rule.fields.toString()}" on "${model}"`)
}
return hasNoForbiddenFields
return argFields.isDisjointFrom(new Set(rule.fields))
// if (!rule.conditions && !hasNoForbiddenFields) {
// throw new Error(`It's not allowed to "${action}" "${rule.fields.toString()}" on "${model}"`)
// }
// return hasNoForbiddenFields
} else {
return argFields.isSubsetOf(new Set(Array.isArray(rule.fields) ? rule.fields : [rule.fields]))
}
Expand Down Expand Up @@ -118,7 +118,7 @@ export function applyDataQuery(
const mutationAction = caslNestedOperationDict[nestedAction]
const isConnection = nestedAction === 'connect' || nestedAction === 'disconnect'

tree.children[field] = { action: mutationAction, model: relationModel.type as Prisma.ModelName, children: {} }
tree.children[field] = { action: mutationAction, model: relationModel.type as Prisma.ModelName, children: {}, mutation: [] }
if (nestedAction !== 'disconnect' && nestedArgs !== true) {
const dataQuery = applyDataQuery(abilities, nestedArgs, mutationAction, relationModel.type, tree.children[field])
mutation[field][nestedAction] = dataQuery.args
Expand Down
16 changes: 15 additions & 1 deletion src/convertCreationTreeToSelect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,21 @@ import { PrismaQuery } from '@casl/prisma';
import { Prisma } from '@prisma/client';
import { getRuleRelationsQuery } from './getRuleRelationsQuery';

export type CreationTree = { action: string, model: Prisma.ModelName, children: Record<string, CreationTree> }
export type CreationTree = {
action: string,
model: Prisma.ModelName,
children: Record<string, CreationTree>,
/**
* mutation query for creation / update
* with fields that are modified on mutation
* and the where query
*
* we use the where query to see which entries have been modified
* and the check accessibleBy per field
* to see if mutations are forbidden
*/
mutation: { fields: string[], where: any }[]
}

export function convertCreationTreeToSelect(abilities: PureAbility<AbilityTuple, PrismaQuery>, relationQuery: CreationTree): Record<string, any> | true | null {
// Recursively filter children
Expand Down
26 changes: 25 additions & 1 deletion src/filterQueryResults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { AbilityTuple, PureAbility } from "@casl/ability";
import { PrismaQuery } from "@casl/prisma";
import { Prisma } from "@prisma/client";
import { CreationTree } from "./convertCreationTreeToSelect";
import { getPermittedFields, getSubject, relationFieldsByModel } from "./helpers";
import { getPermittedFields, getSubject, isSubset, relationFieldsByModel } from "./helpers";
import { storePermissions } from "./storePermissions";

export function filterQueryResults(result: any, mask: any, creationTree: CreationTree | undefined, abilities: PureAbility<AbilityTuple, PrismaQuery>, model: string, permissionField?: string) {
Expand All @@ -16,6 +16,7 @@ export function filterQueryResults(result: any, mask: any, creationTree: Creatio

const filterPermittedFields = (entry: any) => {
if (!entry) { return null }
/** if we have created a model, we check if it is allowed and otherwise throw an error */
if (creationTree?.action === 'create') {
try {
if (!abilities.can('create', getSubject(model, entry))) {
Expand All @@ -26,7 +27,30 @@ export function filterQueryResults(result: any, mask: any, creationTree: Creatio
}
}

/**
* if we have updated a model, we have to check, the current entry
* has been updated by seeing if it overlaps with the where query
* and if it does, we check if all fields were allowed to be updated
* otherwise we throw an error
* */
if (creationTree?.action === 'update' && creationTree.mutation.length > 0) {
creationTree.mutation.forEach(({ fields, where }) => {
fields.forEach((field) => {
if (isSubset(where, entry)) {
try {
if (!abilities.can('update', getSubject(model, entry), field)) {
throw new Error(field)
}
} catch (e) {
throw new Error(`It's not allowed to update ${field} on ${model} ` + e)
}
}
})
})
}

const permittedFields = getPermittedFields(abilities, 'read', model, entry)

let hasKeys = false
Object.keys(entry).filter((field) => field !== permissionField).forEach((field) => {
const relationField = relationFieldsByModel[model][field]
Expand Down
24 changes: 13 additions & 11 deletions test/applyDataQuery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('apply data query', () => {
can('update', 'User')
const result = applyDataQuery(build(), { data: { authorId: 0 }, where: { id: 0 } }, 'update', 'Post')
expect(result.args).toEqual({ data: { author: { connect: { id: 0, } } }, where: { id: 0, } })
expect(result.creationTree).toEqual({ children: { "author": { children: {}, model: 'User', action: "update" } }, model: 'Post', action: "update" })
expect(result.creationTree).toEqual({ children: { author: { mutation: [], children: {}, model: 'User', action: "update" } }, model: 'Post', action: "update", mutation: [{ fields: ['authorId'], where: { id: 0 } }], })
})
it('throws error if update on connection is not allowed', () => {
const { can, cannot, build } = abilityBuilder()
Expand All @@ -32,7 +32,7 @@ describe('apply data query', () => {
data: { id: 0 },
where: { id: 1, AND: [{ OR: [{ id: 0 }] }] }
})
expect(result.creationTree).toEqual({ children: {}, model: 'User', action: mutation })
expect(result.creationTree).toEqual({ children: {}, model: 'User', action: mutation, mutation: [{ fields: ['id'], where: { id: 1 } }], })
})

it('throws error if mutation of property is omitted', () => {
Expand Down Expand Up @@ -72,9 +72,11 @@ describe('apply data query', () => {
creator: {
children: {},
action: 'create',
model: 'User'
model: 'User',
mutation: [{ fields: ['email'], where: { id: 1 } }],
}
}
},
mutation: [{ fields: [], where: { id: 0 } }],
})
})
it('throws error if creation is not allowed', () => {
Expand All @@ -100,7 +102,7 @@ describe('apply data query', () => {
})
const result = applyDataQuery(build(), { data: { id: 1, posts: { connect: { id: 0 } } }, where: { id: 0 } }, 'update', 'User')
expect(result.args).toEqual({ data: { id: 1, posts: { connect: { id: 0, AND: [{ OR: [{ id: 1 }] }] } } }, where: { id: 0, AND: [{ OR: [{ id: 0 }] }] } })
expect(result.creationTree).toEqual({ children: { posts: { children: {}, action: 'update', model: 'Post', } }, model: 'User', action: "update" })
expect(result.creationTree).toEqual({ children: { posts: { children: {}, action: 'update', model: 'Post', mutation: [] } }, model: 'User', action: "update", mutation: [{ fields: ['id'], where: { id: 0 } }], })
})
it('adds where and connection clause in nested array connection update', () => {
const { can, build } = abilityBuilder()
Expand All @@ -113,7 +115,7 @@ describe('apply data query', () => {

const result = applyDataQuery(build(), { data: { id: 1, posts: { connect: [{ id: 0 }] } }, where: { id: 0 } }, 'update', 'User')
expect(result.args).toEqual({ data: { id: 1, posts: { connect: [{ id: 0, AND: [{ OR: [{ id: 1 }] }] }] } }, where: { id: 0, AND: [{ OR: [{ id: 0 }] }] } })
expect(result.creationTree).toEqual({ children: { posts: { children: {}, model: 'Post', action: "update" } }, model: 'User', action: "update" })
expect(result.creationTree).toEqual({ children: { posts: { children: {}, model: 'Post', action: "update", mutation: [] } }, model: 'User', action: "update", mutation: [{ fields: ['id'], where: { id: 0 } }] })
})
it('throws error if data in nested connection is not allowed', () => {
const { can, cannot, build } = abilityBuilder()
Expand All @@ -136,7 +138,7 @@ describe('apply data query', () => {
})
const result = applyDataQuery(build(), { data: { id: 1, posts: { disconnect: true } }, where: { id: 0 } }, 'update', 'User')
expect(result.args).toEqual({ data: { id: 1, posts: { disconnect: true } }, where: { id: 0, AND: [{ OR: [{ id: 0 }] }] } })
expect(result.creationTree).toEqual({ children: { posts: { children: {}, action: 'update', model: 'Post', } }, model: 'User', action: "update" })
expect(result.creationTree).toEqual({ children: { posts: { children: {}, action: 'update', model: 'Post', mutation: [] } }, model: 'User', action: "update", mutation: [{ fields: ['id'], where: { id: 0 } }] })
})
})
describe('connectOrCreate', () => {
Expand Down Expand Up @@ -169,7 +171,7 @@ describe('apply data query', () => {
}, 'update', 'User')

expect(result.args).toEqual({ data: { id: 1, posts: { connectOrCreate: { create: { text: '' }, where: { id: 1, AND: [{ OR: [{ id: 2 }] }] } } } }, where: { id: 0, AND: [{ OR: [{ id: 0 }] }] } })
expect(result.creationTree).toEqual({ action: 'update', model: 'User', children: { posts: { model: 'Post', action: 'create', children: {} } } })
expect(result.creationTree).toEqual({ action: 'update', model: 'User', children: { posts: { model: 'Post', action: 'create', children: {}, mutation: [{ fields: ['text'], where: { id: 1 } }], } }, mutation: [{ fields: ['id'], where: { id: 0 } }], })
})

it('adds where and connection clause in nested array connection update', () => {
Expand Down Expand Up @@ -197,7 +199,7 @@ describe('apply data query', () => {
}
}, 'update', 'User')
expect(result.args).toEqual({ data: { id: 1, posts: { connectOrCreate: [{ create: { text: '' }, where: { id: 0, AND: [{ OR: [{ id: 2 }] }] } }] } }, where: { id: 0, AND: [{ OR: [{ id: 0 }] }] } })
expect(result.creationTree).toEqual({ action: 'update', model: 'User', children: { posts: { model: 'Post', action: 'create', children: {} } } })
expect(result.creationTree).toEqual({ action: 'update', model: 'User', children: { posts: { model: 'Post', action: 'create', children: {}, mutation: [{ fields: ['text'], where: { id: 0 } }] } }, mutation: [{ fields: ['id'], where: { id: 0 } }] })
})
it('throws error if data in nested connection is not allowed', () => {
const { can, cannot, build } = abilityBuilder()
Expand Down Expand Up @@ -269,7 +271,7 @@ describe('apply data query', () => {

const result = applyDataQuery(build(), { data: { id: 1, posts: { update: { data: { thread: { update: { id: 0 } } }, where: { id: 0 } } } }, where: { id: 0 } }, 'update', 'User')
expect(result.args).toEqual({ data: { id: 1, posts: { update: { data: { thread: { update: { id: 0 } } }, where: { id: 0, } } } }, where: { id: 0, } })
expect(result.creationTree).toEqual({ children: { posts: { children: { thread: { children: {}, model: 'Thread', action: "update" } }, model: 'Post', action: "update" } }, model: 'User', action: "update" })
expect(result.creationTree).toEqual({ children: { posts: { children: { thread: { children: {}, model: 'Thread', action: "update", mutation: [] } }, model: 'Post', action: "update", mutation: [{ fields: [], where: { id: 0 } }] } }, model: 'User', action: "update", mutation: [{ fields: ['id'], where: { id: 0 } }] })
})
it('throws error if data in nested nested update is not allowed', () => {
const { can, cannot, build } = abilityBuilder()
Expand Down Expand Up @@ -300,7 +302,7 @@ describe('apply data query', () => {
}, 'create', 'User')
expect(result.args)
.toEqual({ data: { id: 0, posts: { createMany: { data: { text: '' } } } } })
expect(result.creationTree).toEqual({ action: 'create', model: 'User', children: { posts: { model: 'Post', action: 'create', children: {} } } })
expect(result.creationTree).toEqual({ action: 'create', model: 'User', children: { posts: { model: 'Post', action: 'create', children: {}, mutation: [] } }, mutation: [] })
})
it('throws error if data in nested create is not allowed', () => {
const { can, cannot, build } = abilityBuilder()
Expand Down
18 changes: 13 additions & 5 deletions test/convertCreationTreeToSelect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,24 @@ describe('convert creation tree to select query', () => {
action: 'create',
model: 'Thread',
children: {},
mutation: []
},
tag: {
action: 'update',
model: 'Thread',
children: {},
mutation: []
},
},
},
}
},
mutation: []

},
},
mutation: []
}
},
mutation: []

})
).toEqual({ post: { select: { author: { select: { id: true } }, topic: { select: { category: { select: {} } } } } } })
})
Expand All @@ -53,9 +59,11 @@ describe('convert creation tree to select query', () => {
posts: {
action: 'create',
model: 'Post',
children: { thread: { model: 'Thread', action: 'update', children: {} } }
children: { thread: { model: 'Thread', action: 'update', children: {}, mutation: [] } },
mutation: []
}
}
},
mutation: []
})).toEqual({ posts: { select: {} } })
})
})
Loading

0 comments on commit 53ed04b

Please sign in to comment.