fix(auth): Mutation should always throw error (#40)

ktutnik · Apr 9, 2023 · 0869a38 · 0869a38
1 parent 9d52b52
commit 0869a38
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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()
         })
     })
 })