From 0ac08da7ed9d5e4cef52d055be5259fc87ff23ae Mon Sep 17 00:00:00 2001
From: ian <licitdev@gmail.com>
Date: Fri, 18 Nov 2022 00:04:56 +0800
Subject: [PATCH] Fix legacy permissions for M2O fields in GraphQL (#16430)

* Remove relation for legacy permission without allowed field

* Remove deprecated formatError and improve error handling

* Add unit test

Co-authored-by: Rijk van Zanten <rijkvanzanten@me.com>
---
 api/src/services/graphql/index.ts             |  4 +--
 .../graphql/utils/process-error.test.ts       | 34 +++++++++++++++++++
 .../services/graphql/utils/process-error.ts   | 25 ++++++++++++++
 api/src/utils/reduce-schema.ts                |  4 ++-
 4 files changed, 64 insertions(+), 3 deletions(-)
 create mode 100644 api/src/services/graphql/utils/process-error.test.ts
 create mode 100644 api/src/services/graphql/utils/process-error.ts

diff --git a/api/src/services/graphql/index.ts b/api/src/services/graphql/index.ts
index bba8ffec55..515301d45d 100644
--- a/api/src/services/graphql/index.ts
+++ b/api/src/services/graphql/index.ts
@@ -6,7 +6,6 @@ import {
 	execute,
 	ExecutionResult,
 	FieldNode,
-	formatError,
 	FormattedExecutionResult,
 	FragmentDefinitionNode,
 	GraphQLBoolean,
@@ -41,6 +40,7 @@ import {
 	toInputObjectType,
 	ObjectTypeComposerFieldConfigDefinition,
 } from 'graphql-compose';
+import processError from './utils/process-error';
 import { Knex } from 'knex';
 import { flatten, get, mapKeys, merge, omit, pick, set, transform, uniq } from 'lodash';
 import ms from 'ms';
@@ -159,7 +159,7 @@ export class GraphQLService {
 
 		const formattedResult: FormattedExecutionResult = {
 			...result,
-			errors: result.errors?.map(formatError),
+			errors: result.errors?.map((error) => processError(this.accountability, error)),
 		};
 
 		return formattedResult;
diff --git a/api/src/services/graphql/utils/process-error.test.ts b/api/src/services/graphql/utils/process-error.test.ts
new file mode 100644
index 0000000000..02aeaf9be0
--- /dev/null
+++ b/api/src/services/graphql/utils/process-error.test.ts
@@ -0,0 +1,34 @@
+import { GraphQLError } from 'graphql';
+import { describe, expect, test } from 'vitest';
+import processError from './process-error';
+
+describe('GraphQL processError util', () => {
+	const sampleError = new GraphQLError('An error message', { path: ['test_collection'] });
+	const redactedError = {
+		message: 'An unexpected error occurred.',
+		extensions: {
+			code: 'INTERNAL_SERVER_ERROR',
+		},
+	};
+
+	test('returns redacted error when unauthenticated', () => {
+		expect(processError(null, sampleError)).toEqual(expect.objectContaining(redactedError));
+	});
+
+	test('returns redacted error when authenticated but not an admin', () => {
+		expect(processError({ role: 'd674e22b-f405-48ba-9958-9a7bd16a1aa9' }, sampleError)).toEqual(
+			expect.objectContaining(redactedError)
+		);
+	});
+
+	test('returns original error when authenticated and is an admin', () => {
+		expect(processError({ role: 'd674e22b-f405-48ba-9958-9a7bd16a1aa9', admin: true }, sampleError)).toEqual(
+			expect.objectContaining({
+				...sampleError,
+				extensions: {
+					code: 'INTERNAL_SERVER_ERROR',
+				},
+			})
+		);
+	});
+});
diff --git a/api/src/services/graphql/utils/process-error.ts b/api/src/services/graphql/utils/process-error.ts
new file mode 100644
index 0000000000..1e2ab8889c
--- /dev/null
+++ b/api/src/services/graphql/utils/process-error.ts
@@ -0,0 +1,25 @@
+import logger from '../../../logger';
+import { GraphQLError, GraphQLFormattedError } from 'graphql';
+import { Accountability } from '@directus/shared/types';
+
+const processError = (accountability: Accountability | null, error: Readonly<GraphQLError>): GraphQLFormattedError => {
+	logger.error(error);
+
+	if (accountability?.admin === true) {
+		return {
+			...error,
+			extensions: {
+				code: 'INTERNAL_SERVER_ERROR',
+			},
+		};
+	} else {
+		return {
+			message: 'An unexpected error occurred.',
+			extensions: {
+				code: 'INTERNAL_SERVER_ERROR',
+			},
+		};
+	}
+};
+
+export default processError;
diff --git a/api/src/utils/reduce-schema.ts b/api/src/utils/reduce-schema.ts
index 7d77214887..c86fd3f892 100644
--- a/api/src/utils/reduce-schema.ts
+++ b/api/src/utils/reduce-schema.ts
@@ -85,7 +85,9 @@ export function reduceSchema(
 
 		if (
 			relation.related_collection &&
-			Object.keys(allowedFieldsInCollection).includes(relation.related_collection) === false
+			(Object.keys(allowedFieldsInCollection).includes(relation.related_collection) === false ||
+				// Ignore legacy permissions with an empty fields array
+				allowedFieldsInCollection[relation.related_collection].length === 0)
 		) {
 			collectionsAllowed = false;
 		}