Throw validation error after authorization (#17499)

* Throw validation error after authorization

* Improve tests

* Refactor for deleteMany
Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>

* Refactor other usages in users service that errors before ForbiddenException

* Update tests

* Adjust test for 'deleteByQuery'

---------

Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>
Co-authored-by: Rijk van Zanten <rijkvanzanten@me.com>

api/src/services/items.ts

@@ -121,6 +121,10 @@ export class ItemsService<Item extends AnyItem = AnyItem> implements AbstractSer
 				? await authorizationService.validatePayload('create', this.collection, payloadAfterHooks)
 				: payloadAfterHooks;
 
+			if (opts?.preMutationException) {
+				throw opts.preMutationException;
+			}
+
 			const {
 				payload: payloadWithM2O,
 				revisions: revisionsM2O,
@@ -546,6 +550,10 @@ export class ItemsService<Item extends AnyItem = AnyItem> implements AbstractSer
 			? await authorizationService.validatePayload('update', this.collection, payloadAfterHooks)
 			: payloadAfterHooks;
 
+		if (opts?.preMutationException) {
+			throw opts.preMutationException;
+		}
+
 		await this.knex.transaction(async (trx) => {
 			const payloadService = new PayloadService(this.collection, {
 				accountability: this.accountability,
@@ -792,6 +800,10 @@ export class ItemsService<Item extends AnyItem = AnyItem> implements AbstractSer
 			await authorizationService.checkAccess('delete', this.collection, keys);
 		}
 
+		if (opts?.preMutationException) {
+			throw opts.preMutationException;
+		}
+
 		if (opts?.emitEvents !== false) {
 			await emitter.emitFilter(
 				this.eventScope === 'items' ? ['items.delete', `${this.collection}.items.delete`] : `${this.eventScope}.delete`,

api/src/services/roles.test.ts

@@ -2,12 +2,43 @@ import knex, { Knex } from 'knex';
 import { getTracker, MockClient, Tracker } from 'knex-mock-client';
 import { afterEach, beforeAll, beforeEach, describe, expect, it, MockedFunction, SpyInstance, vi } from 'vitest';
 import { ItemsService, PermissionsService, PresetsService, RolesService, UsersService } from '.';
-import { UnprocessableEntityException } from '../exceptions';
+import { ForbiddenException, UnprocessableEntityException } from '../exceptions';
+import { SchemaOverview } from '@directus/shared/types';
 
 vi.mock('../../src/database/index', () => {
 	return { __esModule: true, default: vi.fn(), getDatabaseClient: vi.fn().mockReturnValue('postgres') };
 });
 
+const testSchema = {
+	collections: {
+		directus_roles: {
+			collection: 'directus_roles',
+			primary: 'id',
+			singleton: false,
+			sortField: null,
+			note: null,
+			accountability: null,
+			fields: {
+				id: {
+					field: 'id',
+					defaultValue: null,
+					nullable: false,
+					generated: true,
+					type: 'uuid',
+					dbType: 'uuid',
+					precision: null,
+					scale: null,
+					special: [],
+					note: null,
+					validation: null,
+					alias: false,
+				},
+			},
+		},
+	},
+	relations: [],
+} as SchemaOverview;
+
 describe('Integration Tests', () => {
 	let db: MockedFunction<Knex>;
 	let tracker: Tracker;
@@ -39,12 +70,9 @@ describe('Integration Tests', () => {
 			beforeEach(() => {
 				service = new RolesService({
 					knex: db,
-					schema: {
-						collections: {},
-						relations: [],
-					},
+					schema: testSchema,
 				});
-				superUpdateOne = vi.spyOn(ItemsService.prototype, 'updateOne').mockResolvedValueOnce(adminRoleId);
+				superUpdateOne = vi.spyOn(ItemsService.prototype, 'updateOne');
 			});
 
 			afterEach(() => {
@@ -96,6 +124,12 @@ describe('Integration Tests', () => {
 						});
 
 						it('having a removed a last user that is the last admin of system', async () => {
+							const service = new RolesService({
+								knex: db,
+								schema: testSchema,
+								accountability: { role: 'test', admin: false },
+							});
+
 							const data: Record<string, any> = {
 								users: [],
 							};
@@ -103,14 +137,24 @@ describe('Integration Tests', () => {
 							tracker.on.select('select "id" from "directus_users" where "role" = ?').responseOnce([{ id: userId1 }]);
 							tracker.on.select('select count(*) as "count" from "directus_users"').responseOnce({ count: 0 });
 
-							expect.assertions(3);
+							const promise = service.updateOne(adminRoleId, data);
+
+							expect.assertions(5); // to ensure both assertions in the catch block are reached
+
 							try {
-								await service.updateOne(adminRoleId, data);
+								await promise;
 							} catch (err: any) {
-								expect(err.message).toBe(`You can't remove the last admin user from the admin role.`);
-								expect(err).toBeInstanceOf(UnprocessableEntityException);
+								expect(err.message).toBe(`You don't have permission to access this.`);
+								expect(err).toBeInstanceOf(ForbiddenException);
 							}
-							expect(superUpdateOne).not.toHaveBeenCalled();
+
+							expect(superUpdateOne).toHaveBeenCalled();
+							expect(superUpdateOne.mock.lastCall![2].preMutationException.message).toBe(
+								`You can't remove the last admin user from the admin role.`
+							);
+							expect(superUpdateOne.mock.lastCall![2].preMutationException).toBeInstanceOf(
+								UnprocessableEntityException
+							);
 						});
 					});
 
@@ -155,6 +199,12 @@ describe('Integration Tests', () => {
 						});
 
 						it('having a removed a last user that is the last admin of system', async () => {
+							const service = new RolesService({
+								knex: db,
+								schema: testSchema,
+								accountability: { role: 'test', admin: false },
+							});
+
 							const data: Record<string, any> = {
 								users: [],
 							};
@@ -162,14 +212,24 @@ describe('Integration Tests', () => {
 							tracker.on.select('select "id" from "directus_users" where "role" = ?').responseOnce([{ id: userId1 }]);
 							tracker.on.select('select count(*) as "count" from "directus_users"').responseOnce({ count: 0 });
 
-							expect.assertions(3);
+							const promise = service.updateOne(adminRoleId, data);
+
+							expect.assertions(5); // to ensure both assertions in the catch block are reached
+
 							try {
-								await service.updateOne(adminRoleId, data);
+								await promise;
 							} catch (err: any) {
-								expect(err.message).toBe(`You can't remove the last admin user from the admin role.`);
-								expect(err).toBeInstanceOf(UnprocessableEntityException);
+								expect(err.message).toBe(`You don't have permission to access this.`);
+								expect(err).toBeInstanceOf(ForbiddenException);
 							}
-							expect(superUpdateOne).not.toHaveBeenCalled();
+
+							expect(superUpdateOne).toHaveBeenCalled();
+							expect(superUpdateOne.mock.lastCall![2].preMutationException.message).toBe(
+								`You can't remove the last admin user from the admin role.`
+							);
+							expect(superUpdateOne.mock.lastCall![2].preMutationException).toBeInstanceOf(
+								UnprocessableEntityException
+							);
 						});
 					});
 
@@ -244,6 +304,12 @@ describe('Integration Tests', () => {
 						});
 
 						it('having a removed a last user that is the last admin of system', async () => {
+							const service = new RolesService({
+								knex: db,
+								schema: testSchema,
+								accountability: { role: 'test', admin: false },
+							});
+
 							const data: Record<string, any> = {
 								users: {
 									create: [],
@@ -255,14 +321,24 @@ describe('Integration Tests', () => {
 							tracker.on.select('select "id" from "directus_users" where "role" = ?').responseOnce([{ id: userId1 }]);
 							tracker.on.select('select count(*) as "count" from "directus_users"').responseOnce({ count: 0 });
 
-							expect.assertions(3);
+							const promise = service.updateOne(adminRoleId, data);
+
+							expect.assertions(5); // to ensure both assertions in the catch block are reached
+
 							try {
-								await service.updateOne(adminRoleId, data);
+								await promise;
 							} catch (err: any) {
-								expect(err.message).toBe(`You can't remove the last admin user from the admin role.`);
-								expect(err).toBeInstanceOf(UnprocessableEntityException);
+								expect(err.message).toBe(`You don't have permission to access this.`);
+								expect(err).toBeInstanceOf(ForbiddenException);
 							}
-							expect(superUpdateOne).not.toHaveBeenCalled();
+
+							expect(superUpdateOne).toHaveBeenCalled();
+							expect(superUpdateOne.mock.lastCall![2].preMutationException.message).toBe(
+								`You can't remove the last admin user from the admin role.`
+							);
+							expect(superUpdateOne.mock.lastCall![2].preMutationException).toBeInstanceOf(
+								UnprocessableEntityException
+							);
 						});
 					});
 				});
@@ -285,6 +361,12 @@ describe('Integration Tests', () => {
 						});
 
 						it('having an added user that is the last admin', async () => {
+							const service = new RolesService({
+								knex: db,
+								schema: testSchema,
+								accountability: { role: 'test', admin: false },
+							});
+
 							const data: Record<string, any> = {
 								users: [userId1, userId2],
 							};
@@ -292,14 +374,24 @@ describe('Integration Tests', () => {
 							tracker.on.select('select "id" from "directus_users" where "role" = ?').responseOnce([{ id: userId1 }]);
 							tracker.on.select('select count(*) as "count" from "directus_users"').responseOnce({ count: 0 });
 
-							expect.assertions(3);
+							const promise = service.updateOne(adminRoleId, data);
+
+							expect.assertions(5); // to ensure both assertions in the catch block are reached
+
 							try {
-								await service.updateOne(adminRoleId, data);
+								await promise;
 							} catch (err: any) {
-								expect(err.message).toBe(`You can't remove the last admin user from the admin role.`);
-								expect(err).toBeInstanceOf(UnprocessableEntityException);
+								expect(err.message).toBe(`You don't have permission to access this.`);
+								expect(err).toBeInstanceOf(ForbiddenException);
 							}
-							expect(superUpdateOne).not.toHaveBeenCalled();
+
+							expect(superUpdateOne).toHaveBeenCalled();
+							expect(superUpdateOne.mock.lastCall![2].preMutationException.message).toBe(
+								`You can't remove the last admin user from the admin role.`
+							);
+							expect(superUpdateOne.mock.lastCall![2].preMutationException).toBeInstanceOf(
+								UnprocessableEntityException
+							);
 						});
 
 						it('having a removed user', async () => {
@@ -331,6 +423,12 @@ describe('Integration Tests', () => {
 						});
 
 						it('having a removed a last user that is the last admin of system', async () => {
+							const service = new RolesService({
+								knex: db,
+								schema: testSchema,
+								accountability: { role: 'test', admin: false },
+							});
+
 							const data: Record<string, any> = {
 								users: [],
 							};
@@ -338,14 +436,24 @@ describe('Integration Tests', () => {
 							tracker.on.select('select "id" from "directus_users" where "role" = ?').responseOnce([{ id: userId1 }]);
 							tracker.on.select('select count(*) as "count" from "directus_users"').responseOnce({ count: 0 });
 
-							expect.assertions(3);
+							const promise = service.updateOne(adminRoleId, data);
+
+							expect.assertions(5); // to ensure both assertions in the catch block are reached
+
 							try {
-								await service.updateOne(adminRoleId, data);
+								await promise;
 							} catch (err: any) {
-								expect(err.message).toBe(`You can't remove the last admin user from the admin role.`);
-								expect(err).toBeInstanceOf(UnprocessableEntityException);
+								expect(err.message).toBe(`You don't have permission to access this.`);
+								expect(err).toBeInstanceOf(ForbiddenException);
 							}
-							expect(superUpdateOne).not.toHaveBeenCalled();
+
+							expect(superUpdateOne).toHaveBeenCalled();
+							expect(superUpdateOne.mock.lastCall![2].preMutationException.message).toBe(
+								`You can't remove the last admin user from the admin role.`
+							);
+							expect(superUpdateOne.mock.lastCall![2].preMutationException).toBeInstanceOf(
+								UnprocessableEntityException
+							);
 						});
 					});
 
@@ -364,6 +472,12 @@ describe('Integration Tests', () => {
 						});
 
 						it('having an added user that is the last admin', async () => {
+							const service = new RolesService({
+								knex: db,
+								schema: testSchema,
+								accountability: { role: 'test', admin: false },
+							});
+
 							const data: Record<string, any> = {
 								users: [{ id: userId1 }, { id: userId2 }],
 							};
@@ -371,14 +485,24 @@ describe('Integration Tests', () => {
 							tracker.on.select('select "id" from "directus_users" where "role" = ?').responseOnce([{ id: userId1 }]);
 							tracker.on.select('select count(*) as "count" from "directus_users"').responseOnce({ count: 0 });
 
-							expect.assertions(3);
+							const promise = service.updateOne(adminRoleId, data);
+
+							expect.assertions(5); // to ensure both assertions in the catch block are reached
+
 							try {
-								await service.updateOne(adminRoleId, data);
+								await promise;
 							} catch (err: any) {
-								expect(err.message).toBe(`You can't remove the last admin user from the admin role.`);
-								expect(err).toBeInstanceOf(UnprocessableEntityException);
+								expect(err.message).toBe(`You don't have permission to access this.`);
+								expect(err).toBeInstanceOf(ForbiddenException);
 							}
-							expect(superUpdateOne).not.toHaveBeenCalled();
+
+							expect(superUpdateOne).toHaveBeenCalled();
+							expect(superUpdateOne.mock.lastCall![2].preMutationException.message).toBe(
+								`You can't remove the last admin user from the admin role.`
+							);
+							expect(superUpdateOne.mock.lastCall![2].preMutationException).toBeInstanceOf(
+								UnprocessableEntityException
+							);
 						});
 
 						it('having a removed user', async () => {
@@ -410,6 +534,12 @@ describe('Integration Tests', () => {
 						});
 
 						it('having a removed a last user that is the last admin of system', async () => {
+							const service = new RolesService({
+								knex: db,
+								schema: testSchema,
+								accountability: { role: 'test', admin: false },
+							});
+
 							const data: Record<string, any> = {
 								users: [],
 							};
@@ -417,14 +547,24 @@ describe('Integration Tests', () => {
 							tracker.on.select('select "id" from "directus_users" where "role" = ?').responseOnce([{ id: userId1 }]);
 							tracker.on.select('select count(*) as "count" from "directus_users"').responseOnce({ count: 0 });
 
-							expect.assertions(3);
+							const promise = service.updateOne(adminRoleId, data);
+
+							expect.assertions(5); // to ensure both assertions in the catch block are reached
+
 							try {
-								await service.updateOne(adminRoleId, data);
+								await promise;
 							} catch (err: any) {
-								expect(err.message).toBe(`You can't remove the last admin user from the admin role.`);
-								expect(err).toBeInstanceOf(UnprocessableEntityException);
+								expect(err.message).toBe(`You don't have permission to access this.`);
+								expect(err).toBeInstanceOf(ForbiddenException);
 							}
-							expect(superUpdateOne).not.toHaveBeenCalled();
+
+							expect(superUpdateOne).toHaveBeenCalled();
+							expect(superUpdateOne.mock.lastCall![2].preMutationException.message).toBe(
+								`You can't remove the last admin user from the admin role.`
+							);
+							expect(superUpdateOne.mock.lastCall![2].preMutationException).toBeInstanceOf(
+								UnprocessableEntityException
+							);
 						});
 					});
 
@@ -464,6 +604,12 @@ describe('Integration Tests', () => {
 						});
 
 						it('having an added user that is the last admin', async () => {
+							const service = new RolesService({
+								knex: db,
+								schema: testSchema,
+								accountability: { role: 'test', admin: false },
+							});
+
 							const data: Record<string, any> = {
 								users: {
 									create: [],
@@ -475,14 +621,24 @@ describe('Integration Tests', () => {
 							tracker.on.select('select "id" from "directus_users" where "role" = ?').responseOnce([{ id: userId1 }]);
 							tracker.on.select('select count(*) as "count" from "directus_users"').responseOnce({ count: 0 });
 
-							expect.assertions(3);
+							const promise = service.updateOne(adminRoleId, data);
+
+							expect.assertions(5); // to ensure both assertions in the catch block are reached
+
 							try {
-								await service.updateOne(adminRoleId, data);
+								await promise;
 							} catch (err: any) {
-								expect(err.message).toBe(`You can't remove the last admin user from the admin role.`);
-								expect(err).toBeInstanceOf(UnprocessableEntityException);
+								expect(err.message).toBe(`You don't have permission to access this.`);
+								expect(err).toBeInstanceOf(ForbiddenException);
 							}
-							expect(superUpdateOne).not.toHaveBeenCalled();
+
+							expect(superUpdateOne).toHaveBeenCalled();
+							expect(superUpdateOne.mock.lastCall![2].preMutationException.message).toBe(
+								`You can't remove the last admin user from the admin role.`
+							);
+							expect(superUpdateOne.mock.lastCall![2].preMutationException).toBeInstanceOf(
+								UnprocessableEntityException
+							);
 						});
 
 						it('having a removed user', async () => {
@@ -522,6 +678,12 @@ describe('Integration Tests', () => {
 						});
 
 						it('having a removed a last user that is the last admin of system', async () => {
+							const service = new RolesService({
+								knex: db,
+								schema: testSchema,
+								accountability: { role: 'test', admin: false },
+							});
+
 							const data: Record<string, any> = {
 								users: {
 									create: [],
@@ -533,14 +695,24 @@ describe('Integration Tests', () => {
 							tracker.on.select('select "id" from "directus_users" where "role" = ?').responseOnce([{ id: userId1 }]);
 							tracker.on.select('select count(*) as "count" from "directus_users"').responseOnce({ count: 0 });
 
-							expect.assertions(3);
+							const promise = service.updateOne(adminRoleId, data);
+
+							expect.assertions(5); // to ensure both assertions in the catch block are reached
+
 							try {
-								await service.updateOne(adminRoleId, data);
+								await promise;
 							} catch (err: any) {
-								expect(err.message).toBe(`You can't remove the last admin user from the admin role.`);
-								expect(err).toBeInstanceOf(UnprocessableEntityException);
+								expect(err.message).toBe(`You don't have permission to access this.`);
+								expect(err).toBeInstanceOf(ForbiddenException);
 							}
-							expect(superUpdateOne).not.toHaveBeenCalled();
+
+							expect(superUpdateOne).toHaveBeenCalled();
+							expect(superUpdateOne.mock.lastCall![2].preMutationException.message).toBe(
+								`You can't remove the last admin user from the admin role.`
+							);
+							expect(superUpdateOne.mock.lastCall![2].preMutationException).toBeInstanceOf(
+								UnprocessableEntityException
+							);
 						});
 					});
 				});

api/src/services/roles.ts

@@ -70,8 +70,12 @@ export class RolesService extends ItemsService {
 	}
 
 	async updateOne(key: PrimaryKey, data: Record<string, any>, opts?: MutationOptions): Promise<PrimaryKey> {
-		if ('users' in data) {
-			await this.checkForOtherAdminUsers(key, data.users);
+		try {
+			if ('users' in data) {
+				await this.checkForOtherAdminUsers(key, data.users);
+			}
+		} catch (err: any) {
+			(opts || (opts = {})).preMutationException = err;
 		}
 
 		return super.updateOne(key, data, opts);
@@ -83,16 +87,24 @@ export class RolesService extends ItemsService {
 		const keys = data.map((item) => item[primaryKeyField]);
 		const setsToNoAdmin = data.some((item) => item.admin_access === false);
 
-		if (setsToNoAdmin) {
-			await this.checkForOtherAdminRoles(keys);
+		try {
+			if (setsToNoAdmin) {
+				await this.checkForOtherAdminRoles(keys);
+			}
+		} catch (err: any) {
+			(opts || (opts = {})).preMutationException = err;
 		}
 
 		return super.updateBatch(data, opts);
 	}
 
 	async updateMany(keys: PrimaryKey[], data: Record<string, any>, opts?: MutationOptions): Promise<PrimaryKey[]> {
-		if ('admin_access' in data && data.admin_access === false) {
-			await this.checkForOtherAdminRoles(keys);
+		try {
+			if ('admin_access' in data && data.admin_access === false) {
+				await this.checkForOtherAdminRoles(keys);
+			}
+		} catch (err: any) {
+			(opts || (opts = {})).preMutationException = err;
 		}
 
 		return super.updateMany(keys, data, opts);
@@ -104,7 +116,13 @@ export class RolesService extends ItemsService {
 	}
 
 	async deleteMany(keys: PrimaryKey[]): Promise<PrimaryKey[]> {
-		await this.checkForOtherAdminRoles(keys);
+		const opts: MutationOptions = {};
+
+		try {
+			await this.checkForOtherAdminRoles(keys);
+		} catch (err: any) {
+			opts.preMutationException = err;
+		}
 
 		await this.knex.transaction(async (trx) => {
 			const itemsService = new ItemsService('directus_roles', {
@@ -133,13 +151,19 @@ export class RolesService extends ItemsService {
 
 			// Delete permissions/presets for this role, suspend all remaining users in role
 
-			await permissionsService.deleteByQuery({
-				filter: { role: { _in: keys } },
-			});
+			await permissionsService.deleteByQuery(
+				{
+					filter: { role: { _in: keys } },
+				},
+				opts
+			);
 
-			await presetsService.deleteByQuery({
-				filter: { role: { _in: keys } },
-			});
+			await presetsService.deleteByQuery(
+				{
+					filter: { role: { _in: keys } },
+				},
+				opts
+			);
 
 			await usersService.updateByQuery(
 				{
@@ -148,10 +172,11 @@ export class RolesService extends ItemsService {
 				{
 					status: 'suspended',
 					role: null,
-				}
+				},
+				opts
 			);
 
-			await itemsService.deleteMany(keys);
+			await itemsService.deleteMany(keys, opts);
 		});
 
 		return keys;

api/src/services/users.test.ts

@@ -3,7 +3,7 @@ import knex, { Knex } from 'knex';
 import { getTracker, MockClient, Tracker } from 'knex-mock-client';
 import { afterEach, beforeAll, beforeEach, describe, expect, it, MockedFunction, SpyInstance, vi } from 'vitest';
 import { ItemsService, UsersService } from '.';
-import { InvalidPayloadException } from '../exceptions';
+import { ForbiddenException, InvalidPayloadException } from '../exceptions';
 import { RecordNotUniqueException } from '../exceptions/database/record-not-unique';
 
 vi.mock('../../src/database/index', () => ({
@@ -231,14 +231,20 @@ describe('Integration Tests', () => {
 
 					const promise = service.updateOne(1, { [field]: 'test' });
 
-					expect.assertions(2); // to ensure both assertions in the catch block are reached
+					expect.assertions(5); // to ensure both assertions in the catch block are reached
 
 					try {
 						await promise;
 					} catch (err: any) {
-						expect(err.message).toBe(`You can't change the "${field}" value manually.`);
-						expect(err).toBeInstanceOf(InvalidPayloadException);
+						expect(err.message).toBe(`You don't have permission to access this.`);
+						expect(err).toBeInstanceOf(ForbiddenException);
 					}
+
+					expect(superUpdateManySpy).toHaveBeenCalled();
+					expect(superUpdateManySpy.mock.lastCall![2].preMutationException.message).toBe(
+						`You can't change the "${field}" value manually.`
+					);
+					expect(superUpdateManySpy.mock.lastCall![2].preMutationException).toBeInstanceOf(InvalidPayloadException);
 				}
 			);
 
@@ -340,14 +346,20 @@ describe('Integration Tests', () => {
 
 					const promise = service.updateMany([1], { [field]: 'test' });
 
-					expect.assertions(2); // to ensure both assertions in the catch block are reached
+					expect.assertions(5); // to ensure both assertions in the catch block are reached
 
 					try {
 						await promise;
 					} catch (err: any) {
-						expect(err.message).toBe(`You can't change the "${field}" value manually.`);
-						expect(err).toBeInstanceOf(InvalidPayloadException);
+						expect(err.message).toBe(`You don't have permission to access this.`);
+						expect(err).toBeInstanceOf(ForbiddenException);
 					}
+
+					expect(superUpdateManySpy).toHaveBeenCalled();
+					expect(superUpdateManySpy.mock.lastCall![2].preMutationException.message).toBe(
+						`You can't change the "${field}" value manually.`
+					);
+					expect(superUpdateManySpy.mock.lastCall![2].preMutationException).toBeInstanceOf(InvalidPayloadException);
 				}
 			);
 
@@ -469,14 +481,20 @@ describe('Integration Tests', () => {
 
 					const promise = service.updateByQuery({}, { [field]: 'test' });
 
-					expect.assertions(2); // to ensure both assertions in the catch block are reached
+					expect.assertions(5); // to ensure both assertions in the catch block are reached
 
 					try {
 						await promise;
 					} catch (err: any) {
-						expect(err.message).toBe(`You can't change the "${field}" value manually.`);
-						expect(err).toBeInstanceOf(InvalidPayloadException);
+						expect(err.message).toBe(`You don't have permission to access this.`);
+						expect(err).toBeInstanceOf(ForbiddenException);
 					}
+
+					expect(superUpdateManySpy).toHaveBeenCalled();
+					expect(superUpdateManySpy.mock.lastCall![2].preMutationException.message).toBe(
+						`You can't change the "${field}" value manually.`
+					);
+					expect(superUpdateManySpy.mock.lastCall![2].preMutationException).toBeInstanceOf(InvalidPayloadException);
 				}
 			);
 
@@ -515,24 +533,72 @@ describe('Integration Tests', () => {
 
 		describe('deleteOne', () => {
 			it('should checkRemainingAdminExistence once', async () => {
-				await service.deleteOne(1);
+				const service = new UsersService({
+					knex: db,
+					schema: testSchema,
+					accountability: { role: 'test', admin: false },
+				});
+
+				const promise = service.deleteOne(1);
+
+				expect.assertions(3); // to ensure both assertions in the catch block are reached
+
+				try {
+					await promise;
+				} catch (err: any) {
+					expect(err.message).toBe(`You don't have permission to access this.`);
+					expect(err).toBeInstanceOf(ForbiddenException);
+				}
+
 				expect(checkRemainingAdminExistenceSpy).toBeCalledTimes(1);
 			});
 		});
 
 		describe('deleteMany', () => {
 			it('should checkRemainingAdminExistence once', async () => {
-				await service.deleteMany([1]);
+				const service = new UsersService({
+					knex: db,
+					schema: testSchema,
+					accountability: { role: 'test', admin: false },
+				});
+
+				const promise = service.deleteMany([1]);
+
+				expect.assertions(3); // to ensure both assertions in the catch block are reached
+
+				try {
+					await promise;
+				} catch (err: any) {
+					expect(err.message).toBe(`You don't have permission to access this.`);
+					expect(err).toBeInstanceOf(ForbiddenException);
+				}
+
 				expect(checkRemainingAdminExistenceSpy).toBeCalledTimes(1);
 			});
 		});
 
 		describe('deleteByQuery', () => {
 			it('should checkRemainingAdminExistence once', async () => {
-				// mock return value for the following empty query
-				vi.spyOn(ItemsService.prototype, 'readByQuery').mockResolvedValue([{ id: 1 }]);
+				const service = new UsersService({
+					knex: db,
+					schema: testSchema,
+					accountability: { role: 'test', admin: false },
+				});
+
+				// mock return value for the following empty query
+				vi.spyOn(ItemsService.prototype, 'readByQuery').mockResolvedValueOnce([{ id: 1 }]);
+
+				const promise = service.deleteByQuery({ filter: { id: { _eq: 1 } } });
+
+				expect.assertions(3); // to ensure both assertions in the catch block are reached
+
+				try {
+					await promise;
+				} catch (err: any) {
+					expect(err.message).toBe(`You don't have permission to access this.`);
+					expect(err).toBeInstanceOf(ForbiddenException);
+				}
 
-				await service.deleteByQuery({});
 				expect(checkRemainingAdminExistenceSpy).toBeCalledTimes(1);
 			});
 		});

api/src/services/users.ts

@@ -155,12 +155,16 @@ export class UsersService extends ItemsService {
 		const emails = data.map((payload) => payload.email).filter((email) => email);
 		const passwords = data.map((payload) => payload.password).filter((password) => password);
 
-		if (emails.length) {
-			await this.checkUniqueEmails(emails);
-		}
+		try {
+			if (emails.length) {
+				await this.checkUniqueEmails(emails);
+			}
 
-		if (passwords.length) {
-			await this.checkPasswordPolicy(passwords);
+			if (passwords.length) {
+				await this.checkPasswordPolicy(passwords);
+			}
+		} catch (err: any) {
+			(opts || (opts = {})).preMutationException = err;
 		}
 
 		return await super.createMany(data, opts);
@@ -207,54 +211,58 @@ export class UsersService extends ItemsService {
 	 * Update many users by primary key
 	 */
 	async updateMany(keys: PrimaryKey[], data: Partial<Item>, opts?: MutationOptions): Promise<PrimaryKey[]> {
-		if (data.role) {
-			// data.role will be an object with id with GraphQL mutations
-			const roleId = data.role?.id ?? data.role;
+		try {
+			if (data.role) {
+				// data.role will be an object with id with GraphQL mutations
+				const roleId = data.role?.id ?? data.role;
 
-			const newRole = await this.knex.select('admin_access').from('directus_roles').where('id', roleId).first();
+				const newRole = await this.knex.select('admin_access').from('directus_roles').where('id', roleId).first();
 
-			if (!newRole?.admin_access) {
-				await this.checkRemainingAdminExistence(keys);
-			}
-		}
-
-		if (data.status !== undefined && data.status !== 'active') {
-			await this.checkRemainingActiveAdmin(keys);
-		}
-
-		if (data.email) {
-			if (keys.length > 1) {
-				throw new RecordNotUniqueException('email', {
-					collection: 'directus_users',
-					field: 'email',
-					invalid: data.email,
-				});
-			}
-			await this.checkUniqueEmails([data.email], keys[0]);
-		}
-
-		if (data.password) {
-			await this.checkPasswordPolicy([data.password]);
-		}
-
-		if (data.tfa_secret !== undefined) {
-			throw new InvalidPayloadException(`You can't change the "tfa_secret" value manually.`);
-		}
-
-		if (data.provider !== undefined) {
-			if (this.accountability && this.accountability.admin !== true) {
-				throw new InvalidPayloadException(`You can't change the "provider" value manually.`);
+				if (!newRole?.admin_access) {
+					await this.checkRemainingAdminExistence(keys);
+				}
 			}
 
-			data.auth_data = null;
-		}
-
-		if (data.external_identifier !== undefined) {
-			if (this.accountability && this.accountability.admin !== true) {
-				throw new InvalidPayloadException(`You can't change the "external_identifier" value manually.`);
+			if (data.status !== undefined && data.status !== 'active') {
+				await this.checkRemainingActiveAdmin(keys);
 			}
 
-			data.auth_data = null;
+			if (data.email) {
+				if (keys.length > 1) {
+					throw new RecordNotUniqueException('email', {
+						collection: 'directus_users',
+						field: 'email',
+						invalid: data.email,
+					});
+				}
+				await this.checkUniqueEmails([data.email], keys[0]);
+			}
+
+			if (data.password) {
+				await this.checkPasswordPolicy([data.password]);
+			}
+
+			if (data.tfa_secret !== undefined) {
+				throw new InvalidPayloadException(`You can't change the "tfa_secret" value manually.`);
+			}
+
+			if (data.provider !== undefined) {
+				if (this.accountability && this.accountability.admin !== true) {
+					throw new InvalidPayloadException(`You can't change the "provider" value manually.`);
+				}
+
+				data.auth_data = null;
+			}
+
+			if (data.external_identifier !== undefined) {
+				if (this.accountability && this.accountability.admin !== true) {
+					throw new InvalidPayloadException(`You can't change the "external_identifier" value manually.`);
+				}
+
+				data.auth_data = null;
+			}
+		} catch (err: any) {
+			(opts || (opts = {})).preMutationException = err;
 		}
 
 		return await super.updateMany(keys, data, opts);
@@ -272,7 +280,11 @@ export class UsersService extends ItemsService {
 	 * Delete multiple users by primary key
 	 */
 	async deleteMany(keys: PrimaryKey[], opts?: MutationOptions): Promise<PrimaryKey[]> {
-		await this.checkRemainingAdminExistence(keys);
+		try {
+			await this.checkRemainingAdminExistence(keys);
+		} catch (err: any) {
+			(opts || (opts = {})).preMutationException = err;
+		}
 
 		await this.knex('directus_notifications').update({ sender: null }).whereIn('sender', keys);
 
@@ -300,8 +312,14 @@ export class UsersService extends ItemsService {
 	}
 
 	async inviteUser(email: string | string[], role: string, url: string | null, subject?: string | null): Promise<void> {
-		if (url && isUrlAllowed(url, env.USER_INVITE_URL_ALLOW_LIST) === false) {
-			throw new InvalidPayloadException(`Url "${url}" can't be used to invite users.`);
+		const opts: MutationOptions = {};
+
+		try {
+			if (url && isUrlAllowed(url, env.USER_INVITE_URL_ALLOW_LIST) === false) {
+				throw new InvalidPayloadException(`Url "${url}" can't be used to invite users.`);
+			}
+		} catch (err: any) {
+			opts.preMutationException = err;
 		}
 
 		const emails = toArray(email);
@@ -318,7 +336,7 @@ export class UsersService extends ItemsService {
 			inviteURL.setQuery('token', token);
 
 			// Create user first to verify uniqueness
-			await this.createOne({ email, role, status: 'invited' });
+			await this.createOne({ email, role, status: 'invited' }, opts);
 
 			await mailService.send({
 				to: email,
@@ -358,10 +376,6 @@ export class UsersService extends ItemsService {
 	}
 
 	async requestPasswordReset(email: string, url: string | null, subject?: string | null): Promise<void> {
-		if (url && isUrlAllowed(url, env.PASSWORD_RESET_URL_ALLOW_LIST) === false) {
-			throw new InvalidPayloadException(`Url "${url}" can't be used to reset passwords.`);
-		}
-
 		const STALL_TIME = 500;
 		const timeStart = performance.now();
 
@@ -376,6 +390,10 @@ export class UsersService extends ItemsService {
 			throw new ForbiddenException();
 		}
 
+		if (url && isUrlAllowed(url, env.PASSWORD_RESET_URL_ALLOW_LIST) === false) {
+			throw new InvalidPayloadException(`Url "${url}" can't be used to reset passwords.`);
+		}
+
 		const mailService = new MailService({
 			schema: this.schema,
 			knex: this.knex,
@@ -413,7 +431,13 @@ export class UsersService extends ItemsService {
 
 		if (scope !== 'password-reset' || !hash) throw new ForbiddenException();
 
-		await this.checkPasswordPolicy([password]);
+		const opts: MutationOptions = {};
+
+		try {
+			await this.checkPasswordPolicy([password]);
+		} catch (err: any) {
+			opts.preMutationException = err;
+		}
 
 		const user = await this.knex.select('id', 'status', 'password').from('directus_users').where({ email }).first();
 
@@ -431,6 +455,6 @@ export class UsersService extends ItemsService {
 			},
 		});
 
-		await service.updateOne(user.id, { password, status: 'active' });
+		await service.updateOne(user.id, { password, status: 'active' }, opts);
 	}
 }

api/src/types/items.ts

@@ -3,6 +3,7 @@
  * expecting an item vs any other generic object.
  */
 
+import { BaseException } from '@directus/shared/exceptions';
 import { EventContext } from '@directus/shared/types';
 
 export type Item = Record<string, any>;
@@ -45,6 +46,11 @@ export type MutationOptions = {
 	 * Can be used to queue up the nested events from item service's create, update and delete
 	 */
 	bypassEmitAction?: (params: ActionEventParams) => void;
+
+	/**
+	 * The validation error to throw right before the mutation takes place
+	 */
+	preMutationException?: BaseException;
 };
 
 export type ActionEventParams = {