fix: Security improvements, typing fixes

Fixes potential security hole in no_cache option.

Author: maticzav

README.md

@@ -103,17 +103,25 @@ function getUser(req) {
 
 // Rules
 
-const isAuthenticated = rule()(async (parent, args, ctx, info) => {
-  return ctx.user !== null
-})
+/* Read more about cache options down in the `rules/cache` section. */
 
-const isAdmin = rule()(async (parent, args, ctx, info) => {
-  return ctx.user.role === 'admin'
-})
+const isAuthenticated = rule({ cache: 'contextual' })(
+  async (parent, args, ctx, info) => {
+    return ctx.user !== null
+  },
+)
 
-const isEditor = rule()(async (parent, args, ctx, info) => {
-  return ctx.user.role === 'editor'
-})
+const isAdmin = rule({ cache: 'contextual' })(
+  async (parent, args, ctx, info) => {
+    return ctx.user.role === 'admin'
+  },
+)
+
+const isEditor = rule({ cache: 'contextual' })(
+  async (parent, args, ctx, info) => {
+    return ctx.user.role === 'editor'
+  },
+)
 
 // Permissions
 
@@ -274,25 +282,34 @@ const admin = bool =>
   )
 ```
 
-- Cache is enabled by default across all rules. To prevent `cache` generation, set `{ cache: 'no_cache' }` or `{ cache: false }` when generating a rule.
-- By default, no rule is executed more than once in complete query execution. This accounts for significantly better load times and quick responses.
+- Cache is disabled by default. To enable `cache` generation, set cache option when generating a rule.
 
 ##### Cache
 
 You can choose from three different cache options.
 
 1.  `no_cache` - prevents rules from being cached.
-1.  `contextual` - use when rule only relies on `ctx` parameter.
-1.  `strict` - use when rule relies on `parent` or `args` parameter as well.
+1.  `contextual` - use when rule only relies on `context` parameter (useful for authentication).
+1.  `strict` - use when rule relies on `parent` or `args` parameter as well (field specific modifications).
 
 ```ts
 // Contextual
-const admin = rule({ cache: 'contextual' })(async (parent, args, ctx, info) => {
-  return ctx.user.isAdmin
-})
+const isAdmin = rule({ cache: 'contextual' })(
+  async (parent, args, ctx, info) => {
+    return ctx.user.isAdmin
+  },
+)
 
 // Strict
-const admin = rule({ cache: 'strict' })(async (parent, args, ctx, info) => {
+const canSeeUserSensitiveData = rule({ cache: 'strict' })(
+  async (parent, args, ctx, info) => {
+    /* The id of observed User matches the id of authenticated viewer. */
+    return ctx.viewer.id === parent.id
+  },
+)
+
+// No-cache (defuault)
+const admin = rule({ cache: 'no_cache' })(async (parent, args, ctx, info) => {
   return ctx.user.isAdmin || args.code === 'secret' || parent.id === 'theone'
 })
 ```
@@ -629,7 +646,7 @@ See [#126](https://github.com/maticzav/graphql-shield/issues/126#issuecomment-41
 
 #### A rule is executed only once even though the dataset contains multiple values (and thus should execute the rule multiple times)
 
-This occurs because of caching. When the cache is set to "contextual" only the contextual variable of the rule is expected to be evaluated. Setting the cache to "strict" allows the rule to rely on parent and args parameters as well.
+This occurs because of caching. When the cache is set to `contextual` only the contextual variable of the rule is expected to be evaluated. Setting the cache to `strict` allows the rule to rely on parent and args parameters as well, while setting the cache to `no_cache` won't cache result at all.
 
 ## Contributors
 

examples/advanced/src/permissions/rules.ts

@@ -6,14 +6,16 @@ import { Context, getUserEmail } from '../utils'
 
 // To see the effect with no cache, set { cache: false } in isCustomer rule.
 
-export const isGrocer = rule()(async (parent, args, ctx: Context, info) => {
-  // console.log('SHIELD: IsGrocer?')
+export const isGrocer = rule({ cache: 'contextual' })(
+  async (parent, args, ctx: Context, info) => {
+    // console.log('SHIELD: IsGrocer?')
 
-  const email = getUserEmail(ctx)
-  return ctx.db.exists.Grocer({ email })
-})
+    const email = getUserEmail(ctx)
+    return ctx.db.exists.Grocer({ email })
+  },
+)
 
-export const isCustomer = rule({ cache: true })(
+export const isCustomer = rule({ cache: 'contextual' })(
   async (parent, args, ctx: Context, info) => {
     // console.log('SHIELD: IsCustomer?')
 

examples/basic/index.js

@@ -75,17 +75,23 @@ function getUser(req) {
 
 // Rules
 
-const isAuthenticated = rule()(async (parent, args, ctx, info) => {
-  return ctx.user !== null
-})
+const isAuthenticated = rule({ cache: 'contextual' })(
+  async (parent, args, ctx, info) => {
+    return ctx.user !== null
+  },
+)
 
-const isAdmin = rule()(async (parent, args, ctx, info) => {
-  return ctx.user.role === 'admin'
-})
+const isAdmin = rule({ cache: 'contextual' })(
+  async (parent, args, ctx, info) => {
+    return ctx.user.role === 'admin'
+  },
+)
 
-const isEditor = rule()(async (parent, args, ctx, info) => {
-  return ctx.user.role === 'editor'
-})
+const isEditor = rule({ cache: 'contextual' })(
+  async (parent, args, ctx, info) => {
+    return ctx.user.role === 'editor'
+  },
+)
 
 // Permissions
 

examples/with-graphql-nexus/src/lib/permissions/rules/is-authenticated.js

@@ -1,9 +1,9 @@
-const {rule} = require('graphql-shield');
+const { rule } = require('graphql-shield')
 
-const isAuthenticated = rule()((parent, args, ctx) => {
-	return Boolean(ctx.request.userId);
-});
+const isAuthenticated = rule({ cache: 'contextual' })((parent, args, ctx) => {
+  return Boolean(ctx.request.userId)
+})
 
 module.exports = {
-	isAuthenticated
-};
+  isAuthenticated,
+}

jest.config.js

@@ -17,4 +17,9 @@ module.exports = {
   verbose: true,
   coverageDirectory: './coverage',
   coverageReporters: ['json', 'lcov', 'text', 'clover', 'html'],
+  globals: {
+    'ts-jest': {
+      tsConfig: 'tsconfig.test.json',
+    },
+  },
 }

package.json

@@ -7,18 +7,18 @@
   "author": "Matic Zavadlal <[email protected]>",
   "scripts": {
     "compile": "tsc -d",
-    "coverage": "yarn codecov",
+    "coverage": "codecov",
     "clean": "rimraf dist",
     "docz:dev": "docz dev",
     "docz:build": "docz build",
     "lint": "tslint --project tsconfig.json {src}/**/*.ts && prettier-check --ignore-path .gitignore src/**/*.ts",
     "postinstall": "lightercollective postinstall",
     "posttest": "npm-run-all lint",
     "prepublishOnly": "npm-run-all compile",
-    "prerelease": "npm-run-all test",
-    "pretest": "npm-run-all clean compile",
     "release": "semantic-release",
-    "test": "NODE_ENV=test jest"
+    "test": "npm-run-all --parallel test:*",
+    "test:jest": "NODE_ENV=test jest",
+    "test:types": "tsc --noEmit"
   },
   "dependencies": {
     "@types/yup": "0.26.23",
@@ -38,17 +38,17 @@
     "docz": "0.13.7",
     "docz-theme-default": "0.13.7",
     "graphql": "14.3.1",
-    "graphql-middleware": "3.0.2",
+    "graphql-middleware": "4.0.1",
     "graphql-tools": "4.0.5",
-    "graphql-yoga": "1.18.1",
+    "graphql-yoga": "1.18.3",
     "husky": "3.0.4",
     "jest": "24.9.0",
     "npm-run-all": "4.1.5",
     "prettier": "1.18.2",
     "prettier-check": "2.0.0",
     "pretty-quick": "1.11.1",
     "remark-emoji": "2.0.2",
-    "rimraf": "2.7.1",
+    "rimraf": "3.0.0",
     "semantic-release": "15.13.24",
     "ts-jest": "24.0.2",
     "ts-node": "8.3.0",
@@ -59,7 +59,7 @@
   },
   "peerDependencies": {
     "graphql": "^0.11.0 || ^0.12.0 || ^0.13.0 || ^14.0.0",
-    "graphql-middleware": "^2.0.0 || ^3.0.0"
+    "graphql-middleware": "^2.0.0 || ^3.0.0 || ^4.0.0"
   },
   "resolutions": {
     "graphql-yoga/graphql": "14.3.1"

src/constructors.ts

@@ -70,7 +70,7 @@ export const inputRule = <T>(
   schema?: (yup: typeof Yup) => Yup.Schema<T>,
 ) => {
   if (typeof name === 'string') {
-    return new InputRule(name, schema(Yup))
+    return new InputRule(name, schema!(Yup))
   } else {
     return new InputRule(Math.random().toString(), name(Yup))
   }

src/generator.ts

@@ -25,6 +25,7 @@ import {
   withDefault,
 } from './utils'
 import { ValidationError } from './validation'
+import { IMiddlewareWithOptions } from 'graphql-middleware/dist/types'
 
 /**
  *
@@ -37,9 +38,14 @@ import { ValidationError } from './validation'
 function generateFieldMiddlewareFromRule(
   rule: ShieldRule,
   options: IOptions,
-): IMiddlewareFunction {
+): IMiddlewareFunction<object, object, IShieldContext> {
   async function middleware(
-    resolve: (parent, args, ctx, info) => any,
+    resolve: (
+      parent: object,
+      args: object,
+      ctx: IShieldContext,
+      info: GraphQLResolveInfo,
+    ) => Promise<any>,
     parent: { [key: string]: any },
     args: { [key: string]: any },
     ctx: IShieldContext,
@@ -53,7 +59,6 @@ function generateFieldMiddlewareFromRule(
     if (!ctx._shield) {
       ctx._shield = {
         cache: {},
-        hashFunction: options.hashFunction,
       }
     }
 
@@ -83,17 +88,17 @@ function generateFieldMiddlewareFromRule(
     return {
       fragment: rule.extractFragment(),
       resolve: middleware,
-    }
+    } as IMiddlewareWithOptions<object, object, IShieldContext>
   }
 
   if (isLogicRule(rule)) {
     return {
       fragments: rule.extractFragments(),
       resolve: middleware,
-    }
+    } as IMiddlewareWithOptions<object, object, IShieldContext>
   }
 
-  return middleware
+  return middleware as IMiddlewareFunction<object, object, IShieldContext>
 }
 
 /**