Improve recursive filtering (#4493)

* Use lowercase chars only

* Fix join alias mapping

* Pass keys as array in delete

* Cleanup delete controller

* Don't catch unexpected errors

api/src/controllers/items.ts

@@ -213,7 +213,9 @@ router.delete(
 			accountability: req.accountability,
 			schema: req.schema,
 		});
+
 		await service.delete(req.body as PrimaryKey[]);
+
 		return next();
 	}),
 	respond
@@ -229,7 +231,9 @@ router.delete(
 			accountability: req.accountability,
 			schema: req.schema,
 		});
+
 		const pk = req.params.pk.includes(',') ? req.params.pk.split(',') : req.params.pk;
+
 		await service.delete(pk as any);
 		return next();
 	}),

api/src/services/authorization.ts

@@ -334,21 +334,13 @@ export class AuthorizationService {
 			schema: this.schema,
 		});
 
-		try {
-			const query: Query = {
-				fields: ['*'],
-			};
+		const query: Query = {
+			fields: ['*'],
+		};
 
-			const result = await itemsService.readByKey(pk as any, query, action);
+		const result = await itemsService.readByKey(pk as any, query, action);
 
-			if (!result) throw '';
-			if (Array.isArray(pk) && pk.length > 1 && result.length !== pk.length) throw '';
-		} catch {
-			throw new ForbiddenException(`You're not allowed to ${action} item "${pk}" in collection "${collection}".`, {
-				collection,
-				item: pk,
-				action,
-			});
-		}
+		if (!result) throw new ForbiddenException();
+		if (Array.isArray(pk) && pk.length > 1 && result.length !== pk.length) throw new ForbiddenException();
 	}
 }

api/src/services/items.ts

@@ -217,6 +217,7 @@ export class ItemsService<Item extends AnyItem = AnyItem> implements AbstractSer
 		action: PermissionsAction = 'read'
 	): Promise<null | Partial<Item> | Partial<Item>[]> {
 		query = clone(query);
+
 		const primaryKeyField = this.schema.tables[this.collection].primary;
 		const keys = toArray(key);
 
@@ -474,7 +475,7 @@ export class ItemsService<Item extends AnyItem = AnyItem> implements AbstractSer
 				schema: this.schema,
 			});
 
-			await authorizationService.checkAccess('delete', this.collection, key);
+			await authorizationService.checkAccess('delete', this.collection, keys);
 		}
 
 		await emitter.emitAsync(`${this.eventScope}.delete.before`, {

api/src/utils/apply-query.ts

@@ -1,11 +1,13 @@
 import { Knex } from 'knex';
 import { Query, Filter, Relation, SchemaOverview } from '../types';
-import { clone, isPlainObject } from 'lodash';
+import { clone, isPlainObject, get, set } from 'lodash';
 import { systemRelationRows } from '../database/system-data/relations';
-import { nanoid } from 'nanoid';
+import { customAlphabet } from 'nanoid';
 import getLocalType from './get-local-type';
 import validate from 'uuid-validate';
 
+const generateAlias = customAlphabet('abcdefghijklmnopqrstuvwxyz', 5);
+
 export default function applyQuery(
 	collection: string,
 	dbQuery: Knex.QueryBuilder,
@@ -100,8 +102,9 @@ export function applyFilter(
 
 				const isM2O = relation.many_collection === parentCollection && relation.many_field === pathParts[0];
 
-				const alias = nanoid(8);
-				aliasMap[pathParts.join('+')] = alias;
+				const alias = generateAlias();
+
+				set(aliasMap, parentAlias ? [parentAlias, ...pathParts] : pathParts, alias);
 
 				if (isM2O) {
 					dbQuery.leftJoin(
@@ -275,13 +278,13 @@ export function applyFilter(
 		function getWhereColumn(path: string[], collection: string) {
 			path = clone(path);
 
-			let columnName = '';
+			return followRelation(path);
 
-			followRelation(path);
-
-			return columnName;
-
-			function followRelation(pathParts: string[], parentCollection: string = collection) {
+			function followRelation(
+				pathParts: string[],
+				parentCollection: string = collection,
+				parentAlias?: string
+			): string | void {
 				const relation = relations.find((relation) => {
 					return (
 						(relation.many_collection === parentCollection && relation.many_field === pathParts[0]) ||
@@ -292,18 +295,18 @@ export function applyFilter(
 				if (!relation) return;
 
 				const isM2O = relation.many_collection === parentCollection && relation.many_field === pathParts[0];
-				const alias = aliasMap[pathParts.join('+')];
+				const alias = get(aliasMap, parentAlias ? [parentAlias, ...pathParts] : pathParts);
 
-				pathParts.shift();
+				const remainingParts = pathParts.slice(1);
 
 				const parent = isM2O ? relation.one_collection! : relation.many_collection;
 
-				if (pathParts.length === 1) {
-					columnName = `${alias || parent}.${pathParts[0]}`;
+				if (remainingParts.length === 1) {
+					return `${alias || parent}.${remainingParts[0]}`;
 				}
 
-				if (pathParts.length) {
-					followRelation(pathParts, parent);
+				if (remainingParts.length) {
+					return followRelation(remainingParts, parent, alias);
 				}
 			}
 		}