From bfc888bf496a0f47d30d3eedc1d2bb3923103449 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Wed, 16 Nov 2022 19:19:18 +0000 Subject: [PATCH 1/3] feat: add authorization --- server/permissions/shield.ts | 161 +++++++++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+) create mode 100644 server/permissions/shield.ts diff --git a/server/permissions/shield.ts b/server/permissions/shield.ts new file mode 100644 index 000000000..c87cbbe67 --- /dev/null +++ b/server/permissions/shield.ts @@ -0,0 +1,161 @@ +import { GraphQLResolveInfo } from 'graphql' +import { IRuleResult, ShieldRule } from 'graphql-shield/typings/types' + +import { deny } from 'graphql-shield' +import { Context } from '../context' + +class TestShield { + @shield, Context, { id: string }>({ + before: deny, + after: () => true, + }) + resolverComplete( + _root: Record, + _args: { id: string }, + _context: Context, + _info: GraphQLResolveInfo + ): boolean { + return true + } + + @shield({ + before: deny, + after: (result, parent, args, context, info) => true, + }) + resolverComplete3( + _root: Record, + _args: { id: string }, + _context: Context, + _info: GraphQLResolveInfo + ): boolean { + return true + } + + @shield({ + before: deny, + after: ( + _result: boolean, + _root: Record, + _args: { id: string }, + _context: Context, + _info: GraphQLResolveInfo + ) => true, + }) + resolverComplete2( + _root: Record, + _args: { id: string; other: string }, + _context: Context, + _info: GraphQLResolveInfo + ): boolean { + return true + } + + @shield({ + before: deny, + after: ( + _result: { id: string }, + _root: Record, + _args: { id: string }, + _context: Context, + _info: GraphQLResolveInfo + ) => true, + }) + resolverCompleteReturn( + _root: Record, + _args: { id: string; other: string }, + _context: Context, + _info: GraphQLResolveInfo + ): Promise<{ id: string; context: string }> { + return true + } +} + +type test = ( + _root: Record, + _args: { id: string }, + _context: Context, + _info: GraphQLResolveInfo +) => boolean + +const obj = {} as test + +function testf( + arg: ResolverFn +) { + return null +} + +testf(obj) + +type Promisable = T extends Promise ? T : Promise | T + +export type ResolverFn = ( + parent: TParent, + args: TArgs, + context: TContext, + info: GraphQLResolveInfo +) => TResult + +type ResultFromResolver = R extends ResolverFn + ? ReturnType + : never +type ParentFromResolver = R extends ResolverFn + ? T + : never +type ContextFromResolver = R extends ResolverFn + ? T + : never +type ArgsFromResolver = R extends ResolverFn + ? T + : never + +// GraphQL Shield doesn't support output rules yet, https://github.com/dimatill/graphql-shield/issues/1210 +type AfterRule = ( + result: TResult, + parent: TParent, + args: TArgs, + context: TContext, + info: GraphQLResolveInfo +) => Promise | IRuleResult + +export function shield({ + before, + after, +}: { + before: ShieldRule + after: AfterRule +}): < + Result extends TResult, + Parent extends TParent, + Context extends TContext, + Args extends TArgs +>( + _target: object, + _key: string | symbol, + descriptor: TypedPropertyDescriptor> +) => void { + return function (_target, _key, descriptor) { + const original = descriptor.value + if (!original) { + throw new Error('No original function') + } + + // @ts-expect-error: We don't really change the type of the function, we just wrap it. + descriptor.value = async function (parent, args, context, info) { + // @ts-expect-error: Graphql-shield does not provide proper types for rules. + // https://github.com/dimatill/graphql-shield/issues/1479 + const result = await before.resolve(parent, args, context, info, {}) + if (result === true) { + const value = await original.apply(this, [parent, args, context, info]) + const resultAfter = await after(value, parent, args, context, info) + if (resultAfter === true) { + return value + } else { + return resultAfter + } + } else { + return result + } + } + } +} From 25f154e92c67fe6a55dce3a0e737dd8b9a396ece Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Sun, 27 Nov 2022 12:57:24 +0000 Subject: [PATCH 2/3] add a few tests --- package.json | 1 + server/permissions/shield.spec.ts | 96 ++++++++++++++++++++++++ server/permissions/shield.ts | 117 +++--------------------------- yarn.lock | 77 +++++++++++++++++++- 4 files changed, 184 insertions(+), 107 deletions(-) create mode 100644 server/permissions/shield.spec.ts diff --git a/package.json b/package.json index 9b472d51c..72b2a6cc7 100644 --- a/package.json +++ b/package.json @@ -72,6 +72,7 @@ "graphql": "^16.6.0", "graphql-passport": "^0.6.4", "graphql-scalars": "^1.20.1", + "graphql-shield": "^7.6.5", "lodash": "^4.17.21", "nodemailer": "^6.8.0", "passport": "^0.6.0", diff --git a/server/permissions/shield.spec.ts b/server/permissions/shield.spec.ts new file mode 100644 index 000000000..01bfd2f95 --- /dev/null +++ b/server/permissions/shield.spec.ts @@ -0,0 +1,96 @@ +import { GraphQLResolveInfo } from 'graphql' +import { Context } from '../context' +import { shield } from './shield' + +// eslint-disable-next-line @typescript-eslint/no-unused-vars, unused-imports/no-unused-vars +class TypeTest { + @shield({ + after: () => true, + }) + noArgsAfterRule( + _root: Record, + _args: { id: string }, + _context: Context, + _info: GraphQLResolveInfo + ): boolean { + return true + } + + @shield({ + after: ( + _result: boolean, + _root: Record, + _args: { id: string }, + _context: Context, + _info: GraphQLResolveInfo + ) => true, + }) + afterRuleWithSameTypeAsResolver( + _root: Record, + _args: { id: string }, + _context: Context, + _info: GraphQLResolveInfo + ): boolean { + return true + } + + @shield({ + after: ( + _result: boolean, + _root: Record, + _args: { id: string }, + _context: Context, + _info: GraphQLResolveInfo + ) => true, + }) + afterRuleAcceptingWiderTypeAsArg( + _root: Record, + _args: { id: string; other: string }, + _context: Context, + _info: GraphQLResolveInfo + ): boolean { + return true + } + + // @ts-expect-error: rule cannot accept more specialized type than resolver + @shield({ + after: ( + _result: { id: string }, + _root: Record, + _args: { id: string; other: string }, + _context: Context, + _info: GraphQLResolveInfo + ) => true, + }) + afterRuleAcceptingInvalidTypeAsArg( + _root: Record, + _args: { id: string }, + _context: Context, + _info: GraphQLResolveInfo + ): boolean { + return true + } +} + +class TestResolver { + @shield({ + after: (result) => result === 'verified', + }) + verify(_root: Record, { status }: { status: string }) { + return status + } +} + +describe('after rule', () => { + it('do not modify resolver result if rule returns true', () => { + const resolver = new TestResolver() + const result = resolver.verify({} as never, { status: 'verified' }) + expect(result).toEqual('verified') + }) + it('throw error if rule returns false', () => { + const resolver = new TestResolver() + expect( + resolver.verify({} as never, { status: 'not verified' }) + ).toThrowError() + }) +}) diff --git a/server/permissions/shield.ts b/server/permissions/shield.ts index c87cbbe67..ed5998031 100644 --- a/server/permissions/shield.ts +++ b/server/permissions/shield.ts @@ -1,94 +1,6 @@ import { GraphQLResolveInfo } from 'graphql' import { IRuleResult, ShieldRule } from 'graphql-shield/typings/types' -import { deny } from 'graphql-shield' -import { Context } from '../context' - -class TestShield { - @shield, Context, { id: string }>({ - before: deny, - after: () => true, - }) - resolverComplete( - _root: Record, - _args: { id: string }, - _context: Context, - _info: GraphQLResolveInfo - ): boolean { - return true - } - - @shield({ - before: deny, - after: (result, parent, args, context, info) => true, - }) - resolverComplete3( - _root: Record, - _args: { id: string }, - _context: Context, - _info: GraphQLResolveInfo - ): boolean { - return true - } - - @shield({ - before: deny, - after: ( - _result: boolean, - _root: Record, - _args: { id: string }, - _context: Context, - _info: GraphQLResolveInfo - ) => true, - }) - resolverComplete2( - _root: Record, - _args: { id: string; other: string }, - _context: Context, - _info: GraphQLResolveInfo - ): boolean { - return true - } - - @shield({ - before: deny, - after: ( - _result: { id: string }, - _root: Record, - _args: { id: string }, - _context: Context, - _info: GraphQLResolveInfo - ) => true, - }) - resolverCompleteReturn( - _root: Record, - _args: { id: string; other: string }, - _context: Context, - _info: GraphQLResolveInfo - ): Promise<{ id: string; context: string }> { - return true - } -} - -type test = ( - _root: Record, - _args: { id: string }, - _context: Context, - _info: GraphQLResolveInfo -) => boolean - -const obj = {} as test - -function testf( - arg: ResolverFn -) { - return null -} - -testf(obj) - -type Promisable = T extends Promise ? T : Promise | T - export type ResolverFn = ( parent: TParent, args: TArgs, @@ -96,19 +8,6 @@ export type ResolverFn = ( info: GraphQLResolveInfo ) => TResult -type ResultFromResolver = R extends ResolverFn - ? ReturnType - : never -type ParentFromResolver = R extends ResolverFn - ? T - : never -type ContextFromResolver = R extends ResolverFn - ? T - : never -type ArgsFromResolver = R extends ResolverFn - ? T - : never - // GraphQL Shield doesn't support output rules yet, https://github.com/dimatill/graphql-shield/issues/1210 type AfterRule = ( result: TResult, @@ -122,8 +21,9 @@ export function shield({ before, after, }: { - before: ShieldRule - after: AfterRule + before?: ShieldRule + // TODO: Bind the return type of the resolver to the type of the rule + after?: AfterRule }): < Result extends TResult, Parent extends TParent, @@ -144,10 +44,15 @@ export function shield({ descriptor.value = async function (parent, args, context, info) { // @ts-expect-error: Graphql-shield does not provide proper types for rules. // https://github.com/dimatill/graphql-shield/issues/1479 - const result = await before.resolve(parent, args, context, info, {}) + const result = before + ? await before.resolve(parent, args, context, info, {}) + : true if (result === true) { - const value = await original.apply(this, [parent, args, context, info]) - const resultAfter = await after(value, parent, args, context, info) + let value = original.apply(this, [parent, args, context, info]) + value = value instanceof Promise ? await value : value + const resultAfter = after + ? await after(value, parent, args, context, info) + : true if (resultAfter === true) { return value } else { diff --git a/yarn.lock b/yarn.lock index 1bccc0ded..898d4c015 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2192,6 +2192,15 @@ __metadata: languageName: node linkType: hard +"@babel/runtime@npm:^7.15.4": + version: 7.20.1 + resolution: "@babel/runtime@npm:7.20.1" + dependencies: + regenerator-runtime: ^0.13.10 + checksum: 00567a333d3357925742a6f5e39394dcc0af6e6029103fe188158bf7ae8b0b3ee3c6c0f68fccc217f0a6cfa455f6be252298baf56b3f5ff37b34313b170cd9f6 + languageName: node + linkType: hard + "@babel/standalone@npm:^7.19.0": version: 7.19.5 resolution: "@babel/standalone@npm:7.19.5" @@ -6162,7 +6171,7 @@ __metadata: languageName: node linkType: hard -"@types/lodash@npm:^4.14.190": +"@types/lodash@npm:^4.14.175, @types/lodash@npm:^4.14.190": version: 4.14.190 resolution: "@types/lodash@npm:4.14.190" checksum: 353a55a1222a57224ead8bd3379fabb2e3e3f6685f599c48b1b66f099d8e9e015e6c6352c6e2275954004c3bb510377804c63117233c6f187700b6ab78adfa02 @@ -6558,6 +6567,13 @@ __metadata: languageName: node linkType: hard +"@types/yup@npm:0.29.13": + version: 0.29.13 + resolution: "@types/yup@npm:0.29.13" + checksum: 042eb51becd38559d0d7c86a40e0118d84d2b071c926999ec7721cbef759673be290d69b35ca7985db1bbb6d97544a2db20bb7cacbdaf827391bdd199e3789f9 + languageName: node + linkType: hard + "@typescript-eslint/eslint-plugin@npm:^5.42.1": version: 5.44.0 resolution: "@typescript-eslint/eslint-plugin@npm:5.44.0" @@ -14167,6 +14183,21 @@ __metadata: languageName: node linkType: hard +"graphql-shield@npm:^7.6.5": + version: 7.6.5 + resolution: "graphql-shield@npm:7.6.5" + dependencies: + "@types/yup": 0.29.13 + object-hash: ^3.0.0 + tslib: ^2.4.0 + yup: ^0.32.0 + peerDependencies: + graphql: ^0.11.0 || ^0.12.0 || ^0.13.0 || ^14.0.0 || ^15.0.0 || ^16.0.0 + graphql-middleware: ^2.0.0 || ^3.0.0 || ^4.0.0 || ^6.0.0 + checksum: fb8ffd32838699a18f9560304be1eb1c042cc32423ff2b4b15225249f7687eff179ba61363c2c8f215a926533ebb09b823c83b56fdc18fc6a4e7e156bb74d45c + languageName: node + linkType: hard + "graphql-tag@npm:^2.11.0, graphql-tag@npm:^2.12.6": version: 2.12.6 resolution: "graphql-tag@npm:2.12.6" @@ -16094,6 +16125,7 @@ __metadata: graphql-codegen-typescript-validation-schema: ^0.7.0 graphql-passport: ^0.6.4 graphql-scalars: ^1.20.1 + graphql-shield: ^7.6.5 lodash: ^4.17.21 mount-vue-component: ^0.10.2 naive-ui: ^2.34.2 @@ -18463,6 +18495,13 @@ __metadata: languageName: node linkType: hard +"nanoclone@npm:^0.2.1": + version: 0.2.1 + resolution: "nanoclone@npm:0.2.1" + checksum: 96b2954e22f70561f41e20d69856266c65583c2a441dae108f1dc71b716785d2c8038dac5f1d5e92b117aed3825f526b53139e2e5d6e6db8a77cfa35b3b8bf40 + languageName: node + linkType: hard + "nanoid@npm:^3.3.1, nanoid@npm:^3.3.4": version: 3.3.4 resolution: "nanoid@npm:3.3.4" @@ -20854,6 +20893,13 @@ __metadata: languageName: node linkType: hard +"property-expr@npm:^2.0.4": + version: 2.0.5 + resolution: "property-expr@npm:2.0.5" + checksum: 4ebe82ce45aaf1527e96e2ab84d75d25217167ec3ff6378cf83a84fb4abc746e7c65768a79d275881602ae82f168f9a6dfaa7f5e331d0fcc83d692770bcce5f1 + languageName: node + linkType: hard + "property-information@npm:^5.0.0, property-information@npm:^5.3.0": version: 5.6.0 resolution: "property-information@npm:5.6.0" @@ -21554,6 +21600,13 @@ __metadata: languageName: node linkType: hard +"regenerator-runtime@npm:^0.13.10": + version: 0.13.11 + resolution: "regenerator-runtime@npm:0.13.11" + checksum: 27481628d22a1c4e3ff551096a683b424242a216fee44685467307f14d58020af1e19660bf2e26064de946bad7eff28950eae9f8209d55723e2d9351e632bbb4 + languageName: node + linkType: hard + "regenerator-runtime@npm:^0.13.4, regenerator-runtime@npm:^0.13.7": version: 0.13.10 resolution: "regenerator-runtime@npm:0.13.10" @@ -24009,6 +24062,13 @@ __metadata: languageName: node linkType: hard +"toposort@npm:^2.0.2": + version: 2.0.2 + resolution: "toposort@npm:2.0.2" + checksum: d64c74b570391c9432873f48e231b439ee56bc49f7cb9780b505cfdf5cb832f808d0bae072515d93834dd6bceca5bb34448b5b4b408335e4d4716eaf68195dcb + languageName: node + linkType: hard + "tr46@npm:~0.0.3": version: 0.0.3 resolution: "tr46@npm:0.0.3" @@ -26462,6 +26522,21 @@ __metadata: languageName: node linkType: hard +"yup@npm:^0.32.0": + version: 0.32.11 + resolution: "yup@npm:0.32.11" + dependencies: + "@babel/runtime": ^7.15.4 + "@types/lodash": ^4.14.175 + lodash: ^4.17.21 + lodash-es: ^4.17.21 + nanoclone: ^0.2.1 + property-expr: ^2.0.4 + toposort: ^2.0.2 + checksum: 43a16786b47cc910fed4891cebdd89df6d6e31702e9462e8f969c73eac88551ce750732608012201ea6b93802c8847cb0aa27b5d57370640f4ecf30f9f97d4b0 + languageName: node + linkType: hard + "zen-observable-ts@npm:^1.1.0, zen-observable-ts@npm:^1.2.5": version: 1.2.5 resolution: "zen-observable-ts@npm:1.2.5" From 8c6abecf7609204c5072b7f95abd6a1f3c750ef2 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Mon, 28 Nov 2022 11:11:25 +0000 Subject: [PATCH 3/3] fix promise issue --- server/permissions/shield.ts | 64 +++++++++++++++++++++++++----------- 1 file changed, 45 insertions(+), 19 deletions(-) diff --git a/server/permissions/shield.ts b/server/permissions/shield.ts index ed5998031..850aee11d 100644 --- a/server/permissions/shield.ts +++ b/server/permissions/shield.ts @@ -17,6 +17,23 @@ type AfterRule = ( info: GraphQLResolveInfo ) => Promise | IRuleResult +function then( + value: PromiseOrValue, + onValue: (t: T, sync?: true) => PromiseOrValue, + onError: (error: any) => PromiseOrValue = (e) => { + throw e + } +): PromiseOrValue { + if (value instanceof Promise) { + return value.then(onValue).catch(onError) + } + try { + return onValue(value, true) + } catch (e) { + return onError(e) + } +} + export function shield({ before, after, @@ -41,26 +58,35 @@ export function shield({ } // @ts-expect-error: We don't really change the type of the function, we just wrap it. - descriptor.value = async function (parent, args, context, info) { - // @ts-expect-error: Graphql-shield does not provide proper types for rules. - // https://github.com/dimatill/graphql-shield/issues/1479 - const result = before - ? await before.resolve(parent, args, context, info, {}) - : true - if (result === true) { - let value = original.apply(this, [parent, args, context, info]) - value = value instanceof Promise ? await value : value - const resultAfter = after - ? await after(value, parent, args, context, info) - : true - if (resultAfter === true) { - return value - } else { - return resultAfter + descriptor.value = function (parent, args, context, info) { + return then( + before + ? // @ts-expect-error: Graphql-shield does not provide proper types for rules. + // https://github.com/dimatill/graphql-shield/issues/1479 + before.resolve(parent, args, context, info, {}) + : true, + (result) => { + if (result === true) { + return then( + original.apply(this, [parent, args, context, info]), + (value) => { + return then( + after ? after(value, parent, args, context, info) : true, + (resultAfter) => { + if (resultAfter === true) { + return value + } else { + return resultAfter + } + } + ) + } + ) + } else { + return result + } } - } else { - return result - } + ) } } }