Add dynamic permissions check for update / delete

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 <T = any>(
 	collection: string,
 	pk: number | string,
 	query: Query = {},
-	accountability?: Accountability
+	accountability?: Accountability,
+	operation?: Operation
 ): Promise<T> => {
+	// 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 <T = any>(
 		},
 	};
 
-	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,

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<string, any>,
@@ -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<AST> => {
+export const processAST = async (
+	ast: AST,
+	role: string | null,
+	operation: Operation = 'read'
+): Promise<AST> => {
 	const collectionsRequested = getCollectionsFromAST(ast);
 
 	const permissionsForCollections = await database
 		.select<Permission[]>('*')
 		.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.`);
+	}
+};

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<AST> {
 	/**
 	 * 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