Combo of changes related to policy validation (#323)

Soluto · May 9, 2021 · 38c3364 · 38c3364
1 parent b41b769
commit 38c3364
Show file tree

Hide file tree

Showing 38 changed files with 832 additions and 240 deletions.
diff --git a/docs/arguments_injection.md b/docs/arguments_injection.md
@@ -232,6 +232,10 @@ query {
 }
 ```
 
+### Additional properties
+
+In some cases there are additional properties available for injection. For example, in `@policy` and `@policies` directives when `postValidation` argument is `true`, `result` property has value of the field/object resolver. In `@errorHandler` directive `error` property is available for injection in `catchError` argument and `result` in `throwError` argument.
+
 #### Returning a JSON Object with injection
 
 ```graphql

diff --git a/docs/authorization.md b/docs/authorization.md
@@ -158,6 +158,41 @@ In the example above, the `roles` argument value is received by using the Argume
 
 > Note: See [Arguments Injection](./arguments_injection.md) for more information about configuration of policy arguments.
 
+## Post Resolve
+
+`@policy` directive has optional `postResolve` argument. If the value of the argument is `true` the policy validation will be executed **after** the execution of field resolver. This argument should be used in case the policy logic depends on the result of field resolver execution.
+
+For example if we don't want to allow to query photos marked as private and the policy and the schema should be like this:
+
+```graphql
+type Photo {
+  id: ID!
+  owner: String!
+  private: Boolean!
+}
+
+type Query {
+  photo(id: ID!) @rest(url: ...) @policy(namespace: "ns", name: "public-only", postResolve: true, args: { private: "{ result.private }" })
+}
+```
+
+The `@rest` directive will be executed before the `@policy` one. And its result will be available in the policy argument injection as `result`.
+
+```yaml
+metadata:
+  namespace: ns
+  name: public-only
+type: opa
+code: |
+  default allow = false
+  allow {
+    input.args.private == false
+  }
+args:
+  private:
+    type: Boolean!
+```
+
 ## Directives order
 
 **_Important!_** The [order of directives](https://github.com/graphql/graphql-spec/blob/master/spec/Section%202%20--%20Language.md#directives) does matter! The directives are applied in order they appear in schema. Object type directives are applied before field definition directives.

diff --git a/services/src/modules/apollo-server-plugins/base-policy.ts b/services/src/modules/apollo-server-plugins/base-policy.ts
@@ -29,7 +29,7 @@ export function createBasicPolicyPlugin(): ApolloServerPlugin {
             return policyDefinition.shouldOverrideBasePolicy ?? false;
           });
 
-          if (!shouldOverrideBasePolicy && !context.ignorePolicies) {
+          if (!shouldOverrideBasePolicy) {
             context.policyExecutor.validatePolicySync(basePolicy, source, args, context, info);
           }
         },

diff --git a/services/src/modules/apollo-server/build-federated-schema.ts b/services/src/modules/apollo-server/build-federated-schema.ts
@@ -24,6 +24,7 @@ import { composeAndValidate } from '@apollo/federation';
 import { ApolloError } from 'apollo-server-core';
 import createTypeResolvers from '../implicit-type-resolver';
 import { knownApolloDirectives } from '../config';
+import logger from '../logger';
 interface DirectiveVisitors {
   [directiveName: string]: typeof SchemaDirectiveVisitor;
 }
@@ -75,7 +76,8 @@ export function buildSchemaFromFederatedTypeDefs({
   const compositionResult = composeAndValidate(serviceDefinitions);
   const compositionErrors = compositionResult.errors ?? [];
   if (compositionErrors.length > 0) {
-    throw new ApolloError('Federation validation failed', 'FEDERATION_VALIDATION_FAILURE', {
+    logger.error({ compositionErrors }, 'Schema federation validation failed');
+    throw new ApolloError('Schema federation validation failed', 'FEDERATION_VALIDATION_FAILURE', {
       errors: compositionErrors.map(err => (err instanceof GraphQLError ? err : new GraphQLError(err!.message))),
     });
   }

diff --git a/services/src/modules/arguments-injection/arguments-injection.ts b/services/src/modules/arguments-injection/arguments-injection.ts
@@ -13,11 +13,13 @@ declare module '../context' {
   }
 }
 
-function injectTemplate<T = unknown>(
-  template: string,
-  params: GraphQLFieldResolverParams<unknown, Pick<RequestContext, 'exports' | 'request' | 'resourceGroup'> | undefined>
-): T {
-  const { source, args, context, info } = params;
+type InjectionParams = GraphQLFieldResolverParams<
+  unknown,
+  Pick<RequestContext, 'exports' | 'request' | 'resourceGroup'> | undefined
+> & { [key: string]: unknown };
+
+function injectTemplate<T = unknown>(template: string, params: InjectionParams): T {
+  const { source, args, context, info, ...additionalProps } = params;
   const data = {
     source,
     args,
@@ -28,15 +30,13 @@ function injectTemplate<T = unknown>(
     vars: info?.variableValues,
     info,
     plugins: context?.resourceGroup?.pluginsData,
+    ...additionalProps,
   };
 
   return evaluate<T>(template, data);
 }
 
-export function inject(
-  input: unknown,
-  params: GraphQLFieldResolverParams<unknown, Pick<RequestContext, 'exports' | 'request' | 'resourceGroup'> | undefined>
-): unknown {
+export function inject(input: unknown, params: InjectionParams): unknown {
   if (typeof input === 'string') return injectTemplate(input, params);
 
   if (isObject(input)) {

diff --git a/services/src/modules/directives/__snapshots__/error-handler.spec.ts.snap b/services/src/modules/directives/__snapshots__/error-handler.spec.ts.snap
@@ -6,6 +6,9 @@ Object {
     "bar": Object {
       "baz": "Baz",
     },
+    "bar2": Object {
+      "baz": null,
+    },
     "foo": "Foo",
   },
   "errors": undefined,
@@ -35,22 +38,30 @@ exports[`@errorHandler directive 2) Original resolver DOES throw, no catchError,
   ],
   "data": {
     "foo": null,
-    "bar": {
-      "baz": null
-    }
+    "bar": null,
+    "bar2": null
   }
 }
 `;
 
 exports[`@errorHandler directive 3) Original resolver DOES throw, catchError condition and returnValue are undefined, no throwError 1`] = `
-Object {
-  "data": Object {
-    "bar": Object {
-      "baz": null,
-    },
+{
+  "errors": [
+    {
+      "name": "GraphQLError",
+      "message": "Cannot return null for non-nullable field Bar.baz.",
+      "errorCode": "INTERNAL_SERVER_ERROR",
+      "path": [
+        "bar",
+        "baz"
+      ]
+    }
+  ],
+  "data": {
     "foo": null,
-  },
-  "errors": undefined,
+    "bar": null,
+    "bar2": null
+  }
 }
 `;
 
@@ -60,6 +71,7 @@ Object {
     "bar": Object {
       "baz": "Baz (Caught)",
     },
+    "bar2": null,
     "foo": "Foo (Caught)",
   },
   "errors": undefined,
@@ -72,6 +84,7 @@ Object {
     "bar": Object {
       "baz": "Baz (Caught)",
     },
+    "bar2": null,
     "foo": "Foo (Caught)",
   },
   "errors": undefined,
@@ -101,9 +114,8 @@ exports[`@errorHandler directive 6) Original resolver DOES throw, catchError con
   ],
   "data": {
     "foo": null,
-    "bar": {
-      "baz": null
-    }
+    "bar": null,
+    "bar2": null
   }
 }
 `;
@@ -131,9 +143,8 @@ exports[`@errorHandler directive 7) Original resolver DOES throw, catchError con
   ],
   "data": {
     "foo": null,
-    "bar": {
-      "baz": null
-    }
+    "bar": null,
+    "bar2": null
   }
 }
 `;
@@ -144,6 +155,7 @@ Object {
     "bar": Object {
       "baz": "Baz (Caught)",
     },
+    "bar2": null,
     "foo": "Foo (Caught)",
   },
   "errors": undefined,
@@ -161,6 +173,15 @@ exports[`@errorHandler directive 9) Original resolver DOES NOT throw, no catchEr
         "foo"
       ]
     },
+    {
+      "name": "GraphQLError",
+      "message": "",
+      "errorCode": "INTERNAL_SERVER_ERROR",
+      "path": [
+        "bar2",
+        "baz"
+      ]
+    },
     {
       "name": "GraphQLError",
       "message": "",
@@ -173,7 +194,8 @@ exports[`@errorHandler directive 9) Original resolver DOES NOT throw, no catchEr
   ],
   "data": {
     "foo": null,
-    "bar": {
+    "bar": null,
+    "bar2": {
       "baz": null
     }
   }
@@ -191,6 +213,15 @@ exports[`@errorHandler directive 10) Original resolver DOES NOT throw, no catchE
         "foo"
       ]
     },
+    {
+      "name": "GraphQLError",
+      "message": "Throw me",
+      "errorCode": "INTERNAL_SERVER_ERROR",
+      "path": [
+        "bar2",
+        "baz"
+      ]
+    },
     {
       "name": "GraphQLError",
       "message": "Throw me",
@@ -203,7 +234,8 @@ exports[`@errorHandler directive 10) Original resolver DOES NOT throw, no catchE
   ],
   "data": {
     "foo": null,
-    "bar": {
+    "bar": null,
+    "bar2": {
       "baz": null
     }
   }
@@ -221,6 +253,15 @@ exports[`@errorHandler directive 11) Original resolver DOES NOT throw, no catchE
         "foo"
       ]
     },
+    {
+      "name": "GraphQLError",
+      "message": "Throw me",
+      "errorCode": "INTERNAL_SERVER_ERROR",
+      "path": [
+        "bar2",
+        "baz"
+      ]
+    },
     {
       "name": "GraphQLError",
       "message": "Throw me",
@@ -233,7 +274,8 @@ exports[`@errorHandler directive 11) Original resolver DOES NOT throw, no catchE
   ],
   "data": {
     "foo": null,
-    "bar": {
+    "bar": null,
+    "bar2": {
       "baz": null
     }
   }
@@ -251,6 +293,15 @@ exports[`@errorHandler directive 12) Original resolver DOES NOT throw, no catchE
         "foo"
       ]
     },
+    {
+      "name": "GraphQLError",
+      "message": "Throw me",
+      "errorCode": "INTERNAL_SERVER_ERROR",
+      "path": [
+        "bar2",
+        "baz"
+      ]
+    },
     {
       "name": "GraphQLError",
       "message": "Throw me",
@@ -263,7 +314,8 @@ exports[`@errorHandler directive 12) Original resolver DOES NOT throw, no catchE
   ],
   "data": {
     "foo": null,
-    "bar": {
+    "bar": null,
+    "bar2": {
       "baz": null
     }
   }
@@ -281,6 +333,15 @@ exports[`@errorHandler directive 13) Original resolver DOES NOT throw, no catchE
         "foo"
       ]
     },
+    {
+      "name": "GraphQLError",
+      "message": "{ Throw me }",
+      "errorCode": "INTERNAL_SERVER_ERROR",
+      "path": [
+        "bar2",
+        "baz"
+      ]
+    },
     {
       "name": "GraphQLError",
       "message": "{ Throw me }",
@@ -293,7 +354,8 @@ exports[`@errorHandler directive 13) Original resolver DOES NOT throw, no catchE
   ],
   "data": {
     "foo": null,
-    "bar": {
+    "bar": null,
+    "bar2": {
       "baz": null
     }
   }
@@ -306,6 +368,9 @@ Object {
     "bar": Object {
       "baz": "Baz",
     },
+    "bar2": Object {
+      "baz": null,
+    },
     "foo": "Foo",
   },
   "errors": undefined,
@@ -318,6 +383,9 @@ Object {
     "bar": Object {
       "baz": "Baz",
     },
+    "bar2": Object {
+      "baz": null,
+    },
     "foo": "Foo",
   },
   "errors": undefined,
@@ -347,7 +415,8 @@ exports[`@errorHandler directive 16) Original resolver DOES NOT throw, no catchE
   ],
   "data": {
     "foo": null,
-    "bar": {
+    "bar": null,
+    "bar2": {
       "baz": null
     }
   }
@@ -377,9 +446,8 @@ exports[`@errorHandler directive 17) Original resolver DOES throw, catchError co
   ],
   "data": {
     "foo": null,
-    "bar": {
-      "baz": null
-    }
+    "bar": null,
+    "bar2": null
   }
 }
 `;
@@ -407,9 +475,8 @@ exports[`@errorHandler directive 18) Original resolver DOES throw, catchError co
   ],
   "data": {
     "foo": null,
-    "bar": {
-      "baz": null
-    }
+    "bar": null,
+    "bar2": null
   }
 }
 `;