From 046345c1e82922dc045fb06b795ab4756ea56e47 Mon Sep 17 00:00:00 2001 From: rijkvanzanten Date: Wed, 15 Jul 2020 15:44:25 -0400 Subject: [PATCH] Add dynamic permissions check for update / delete --- src/services/items.ts | 42 ++++++++++++++++++++------------- src/services/permissions.ts | 28 +++++++++++++++++++--- src/utils/get-ast-from-query.ts | 6 ++--- 3 files changed, 54 insertions(+), 22 deletions(-) diff --git a/src/services/items.ts b/src/services/items.ts index 320457de92..5ff3080035 100644 --- a/src/services/items.ts +++ b/src/services/items.ts @@ -2,7 +2,7 @@ import database, { schemaInspector } from '../database'; import { Query } from '../types/query'; import runAST from '../database/run-ast'; import getASTFromQuery from '../utils/get-ast-from-query'; -import { Accountability, AST } from '../types'; +import { Accountability, Operation } from '../types'; import * as PayloadService from './payload'; import * as PermissionsService from './permissions'; @@ -28,15 +28,17 @@ async function saveActivityAndRevision( action_by: accountability.user, }); - await RevisionsService.createRevision({ - activity: activityID, - collection, - item, - delta: payload, - /** @todo make this configurable */ - data: await readItem(collection, item, { fields: ['*'] }), - parent: accountability.parent, - }); + if (action !== ActivityService.Action.DELETE) { + await RevisionsService.createRevision({ + activity: activityID, + collection, + item, + delta: payload, + /** @todo make this configurable */ + data: await readItem(collection, item, { fields: ['*'] }), + parent: accountability.parent, + }); + } } export const createItem = async ( @@ -109,8 +111,13 @@ export const readItem = async ( collection: string, pk: number | string, query: Query = {}, - accountability?: Accountability + accountability?: Accountability, + operation?: Operation ): Promise => { + // We allow overriding the operation, so we can use the item read logic to validate permissions + // for update and delete as well + operation = operation || 'read'; + const primaryKeyField = await schemaInspector.primary(collection); query = { @@ -123,10 +130,10 @@ export const readItem = async ( }, }; - let ast = await getASTFromQuery(collection, query, accountability?.role); + let ast = await getASTFromQuery(collection, query, accountability?.role, operation); if (accountability) { - ast = await PermissionsService.processAST(ast, accountability.role); + ast = await PermissionsService.processAST(ast, accountability.role, operation); } const records = await runAST(ast); @@ -142,16 +149,17 @@ export const updateItem = async ( let payload = data; if (accountability) { + await PermissionsService.checkAccess('update', collection, pk, accountability.role); + payload = await PermissionsService.processValues( 'validate', collection, - accountability?.role, + accountability.role, data ); } - payload = await PayloadService.processValues('update', collection, data); - + payload = await PayloadService.processValues('update', collection, payload); payload = await PayloadService.processM2O(collection, payload); const primaryKeyField = await schemaInspector.primary(collection); @@ -190,6 +198,8 @@ export const deleteItem = async ( const primaryKeyField = await schemaInspector.primary(collection); if (accountability) { + await PermissionsService.checkAccess('delete', collection, pk, accountability.role); + // Don't await this. It can run async in the background saveActivityAndRevision( ActivityService.Action.DELETE, diff --git a/src/services/permissions.ts b/src/services/permissions.ts index b6891ef442..0e96fac0e0 100644 --- a/src/services/permissions.ts +++ b/src/services/permissions.ts @@ -12,7 +12,6 @@ import database from '../database'; import { ForbiddenException, InvalidPayloadException } from '../exceptions'; import { uniq } from 'lodash'; import generateJoi from '../utils/generate-joi'; -import Joi from '@hapi/joi'; export const createPermission = async ( data: Record, @@ -46,13 +45,17 @@ export const deletePermission = async (pk: number, accountability: Accountabilit await ItemsService.deleteItem('directus_permissions', pk, accountability); }; -export const processAST = async (ast: AST, role: string | null): Promise => { +export const processAST = async ( + ast: AST, + role: string | null, + operation: Operation = 'read' +): Promise => { const collectionsRequested = getCollectionsFromAST(ast); const permissionsForCollections = await database .select('*') .from('directus_permissions') - .where({ operation: 'read', role }) + .where({ operation, role }) .whereIn( 'collection', collectionsRequested.map(({ collection }) => collection) @@ -220,3 +223,22 @@ export const processValues = async ( return payload; }; + +export const checkAccess = async ( + operation: Operation, + collection: string, + pk: string | number, + role: string +) => { + try { + const query: Query = { + fields: ['*'], + }; + + const result = await ItemsService.readItem(collection, pk, query, { role }, operation); + + if (!result) throw ''; + } catch { + throw new ForbiddenException(`You're not allowed to ${operation} this item.`); + } +}; diff --git a/src/utils/get-ast-from-query.ts b/src/utils/get-ast-from-query.ts index 644d94f64d..104b1fbea2 100644 --- a/src/utils/get-ast-from-query.ts +++ b/src/utils/get-ast-from-query.ts @@ -2,14 +2,14 @@ * Generate an AST based on a given collection and query */ -import { Relation } from '../types/relation'; -import { AST, NestedCollectionAST, FieldAST, Query } from '../types'; +import { AST, NestedCollectionAST, FieldAST, Query, Relation, Operation } from '../types'; import database from '../database'; export default async function getASTFromQuery( collection: string, query: Query, - role?: string | null + role?: string | null, + operation?: Operation ): Promise { /** * we might not need al this info at all times, but it's easier to fetch it all once, than trying to fetch it for every