From 0869a385c1259443dec3d72d1a45947be84b6976 Mon Sep 17 00:00:00 2001 From: Ketut Sandiarsa Date: Sun, 9 Apr 2023 20:16:54 +0800 Subject: [PATCH] fix(auth): Mutation should always throw error (#40) --- README.md | 2 +- packages/auth/readme.md | 2 +- packages/auth/src/index.ts | 6 +++- .../test/__snapshots__/authorize.spec.ts.snap | 34 +++++++++++++++++++ packages/auth/test/authorize.spec.ts | 28 +++++++++++++-- 5 files changed, 67 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index bebb2af..2d99708 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,7 @@ const typeDefs = ` role: String @authorize(policy: "Admin") } - input User { + type User { id: String! name: String! email: String! @authorize(policy: "Authenticated") diff --git a/packages/auth/readme.md b/packages/auth/readme.md index 1f5dd3a..e8c0406 100644 --- a/packages/auth/readme.md +++ b/packages/auth/readme.md @@ -21,7 +21,7 @@ const typeDefs = ` role: String @authorize(policy: "Admin") } - input User { + type User { id: String! name: String! # email only visible by authenticated user diff --git a/packages/auth/src/index.ts b/packages/auth/src/index.ts index 380329c..5f3fb8a 100644 --- a/packages/auth/src/index.ts +++ b/packages/auth/src/index.ts @@ -1,6 +1,6 @@ import { InvocationContext, InvokerHook, createDirectiveInvoker, createDirectiveInvokerPipeline } from "@graphql-directive/core" import { MapperKind, getDirective, mapSchema } from "@graphql-tools/utils" -import { GraphQLError, GraphQLSchema, defaultFieldResolver, isNonNullType } from "graphql" +import { GraphQLError, GraphQLSchema, OperationTypeNode, defaultFieldResolver, isNonNullType } from "graphql" import { Path } from "graphql/jsutils/Path" const errorCode = "GRAPHQL_AUTHORIZATION_FAILED" @@ -48,6 +48,7 @@ const transform = (schema: GraphQLSchema, options: AuthorizeOptions): GraphQLSch return { ...config, resolve: async (parent, args, context, info) => { + const operation = info.operation.operation const path = getPath(info.path) const [inputError, fieldError] = await Promise.all([ pipeline.invoke(args, path, config.args!, [parent, args, context, info]), @@ -60,6 +61,9 @@ const transform = (schema: GraphQLSchema, options: AuthorizeOptions): GraphQLSch if (fieldError.length === 0) { return resolve(parent, args, context, info) } + if (operation === OperationTypeNode.MUTATION) { + throw getError(fieldError) + } if (options.queryResolution === "ThrowError") { throw getError(fieldError) } diff --git a/packages/auth/test/__snapshots__/authorize.spec.ts.snap b/packages/auth/test/__snapshots__/authorize.spec.ts.snap index 2044920..bcd388d 100644 --- a/packages/auth/test/__snapshots__/authorize.spec.ts.snap +++ b/packages/auth/test/__snapshots__/authorize.spec.ts.snap @@ -14,6 +14,31 @@ exports[`Mutation authorization On complex return type (Filter mode) Should auth } `; +exports[`Mutation authorization On complex return type (Filter mode) Should authorize return type with proper role 3`] = ` +{ + "code": "GRAPHQL_AUTHORIZATION_FAILED", + "paths": [ + "test.role", + ], +} +`; + +exports[`Mutation authorization On complex return type (Filter mode) Should throw error when authorize applied on mutation field 1`] = ` +{ + "name": "Wayan Koster", + "role": "admin", +} +`; + +exports[`Mutation authorization On complex return type (Filter mode) Should throw error when authorize applied on mutation field 2`] = ` +{ + "code": "GRAPHQL_AUTHORIZATION_FAILED", + "paths": [ + "test", + ], +} +`; + exports[`Mutation authorization On complex return type (ThrowError mode) Should authorize return type with proper role 1`] = ` { "name": "Wayan Koster", @@ -117,6 +142,15 @@ exports[`Mutation authorization On mutation field Should authorize with proper r } `; +exports[`Mutation authorization On mutation field Should keep throwing error on Filter mode 1`] = ` +{ + "code": "GRAPHQL_AUTHORIZATION_FAILED", + "paths": [ + "test", + ], +} +`; + exports[`Mutation authorization On mutation field Should throw error when specify invalid policy name 1`] = `[GraphQLError: Unknown policy "superadmin" on test]`; exports[`Query authorization Applied on query field argument Should able to apply multiple directives 1`] = ` diff --git a/packages/auth/test/authorize.spec.ts b/packages/auth/test/authorize.spec.ts index 83caba4..d558598 100644 --- a/packages/auth/test/authorize.spec.ts +++ b/packages/auth/test/authorize.spec.ts @@ -92,6 +92,15 @@ describe("Mutation authorization", () => { expect((await test(schema, "user")).errors![0].extensions).toMatchSnapshot() }) + it("Should keep throwing error on Filter mode", async () => { + const schema = createSchema(` + type Mutation { + test(name:String!, role: String): Boolean! @authorize(policy: "admin") + }`, { Mutation: { test: () => true } }, "Filter") + expect((await test(schema, "admin")).data!.test).toBe(true) + expect((await test(schema, "user")).errors![0].extensions).toMatchSnapshot() + }) + it("Should able to apply multiple directives", async () => { const schema = createSchema(` type Mutation { @@ -237,12 +246,27 @@ describe("Mutation authorization", () => { role:String @authorize(policy: "admin") } type Mutation { - test(data:String!): User! + test(data:String!): User! }`, resolver, "Filter") expect((await test(schema, "admin")).data!.test).toMatchSnapshot() const result = await test(schema, "user") expect(result.data!.test).toMatchSnapshot() - expect(result.errors).toBeUndefined() + expect(result.errors![0].extensions).toMatchSnapshot() + }) + + it("Should throw error when authorize applied on mutation field", async () => { + const schema = createSchema(` + type User { + name:String! + role:String @authorize(policy: "admin") + } + type Mutation { + test(data:String!): User! @authorize(policy: "admin") + }`, resolver, "Filter") + expect((await test(schema, "admin")).data!.test).toMatchSnapshot() + const result = await test(schema, "user") + expect(result.data).toBeNull() + expect(result.errors![0].extensions).toMatchSnapshot() }) }) })