Policy directive - accept only a single policy (#146)

* change policy directive to accept only a single policy * Refactored PolicyExecutor API to only expose static methods
Soluto · Jul 1, 2020 · 733e7aa · 733e7aa
1 parent 44c483d
commit 733e7aa
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 66 deletions.
diff --git a/services/src/modules/directives/policy/policy-executor.ts b/services/src/modules/directives/policy/policy-executor.ts
@@ -1,7 +1,7 @@
 import {GraphQLResolveInfo, graphql} from 'graphql';
 import {RequestContext} from '../../context';
 import {Policy, GraphQLArguments, QueryResults} from './types';
-import {Policy as PolicyDefinition, PolicyArgsObject, PolicyAttachments, PolicyQuery} from '../../resource-repository';
+import {Policy as PolicyDefinition, PolicyArgsObject, PolicyAttachments} from '../../resource-repository';
 import {evaluate as evaluateOpa} from './opa';
 import {injectParameters, resolveParameters} from '../../paramInjection';
 
@@ -10,36 +10,53 @@ const typeEvaluators = {
 };
 
 export class PolicyExecutor {
-    private policyDefinitions: PolicyDefinition[];
+    private policyDefinition: PolicyDefinition;
     private policyAttachments: PolicyAttachments;
 
-    constructor(
-        protected policies: Policy[],
+    private constructor(
+        protected policy: Policy,
         protected parent: any,
         protected args: GraphQLArguments,
         protected context: RequestContext,
         protected info: GraphQLResolveInfo
     ) {
-        this.policyDefinitions = context.policies;
+        this.policyDefinition = this.getPolicyDefinition(context.policies, this.policy.namespace, this.policy.name);
         this.policyAttachments = context.policyAttachments;
     }
 
-    async validatePolicies() {
-        await Promise.all(this.policies.map(r => this.validatePolicy(r)));
+    static async evaluatePolicy(
+        policy: Policy,
+        parent: any,
+        args: GraphQLArguments,
+        context: RequestContext,
+        info: GraphQLResolveInfo
+    ): Promise<boolean> {
+        const executor = new PolicyExecutor(policy, parent, args, context, info);
+        return executor.evaluatePolicy();
     }
 
-    async evaluatePolicy(policy: Policy): Promise<boolean> {
-        const policyDefinition = this.getPolicyDefinition(policy.namespace, policy.name);
-
-        const args = policyDefinition.args && this.preparePolicyArgs(policyDefinition.args, policy);
+    static async validatePolicy(
+        policy: Policy,
+        parent: any,
+        args: GraphQLArguments,
+        context: RequestContext,
+        info: GraphQLResolveInfo
+    ): Promise<void> {
+        const executor = new PolicyExecutor(policy, parent, args, context, info);
+        const allow = await executor.evaluatePolicy();
+        if (!allow)
+            throw new Error(`Unauthorized by policy ${executor.policy.name} in namespace ${executor.policy.namespace}`);
+    }
 
-        const query = policyDefinition.query && (await this.evaluatePolicyQuery(policyDefinition.query, args));
+    private async evaluatePolicy(): Promise<boolean> {
+        const args = this.preparePolicyArgs();
+        const query = await this.evaluatePolicyQuery(args);
 
-        const evaluate = typeEvaluators[policyDefinition.type];
-        if (!evaluate) throw new Error(`Unsupported policy type ${policyDefinition.type}`);
+        const evaluate = typeEvaluators[this.policyDefinition.type];
+        if (!evaluate) throw new Error(`Unsupported policy type ${this.policyDefinition.type}`);
 
         const {done, allow} = await evaluate({
-            ...policy,
+            ...this.policy,
             args,
             query,
             policyAttachments: this.policyAttachments,
@@ -48,20 +65,18 @@ export class PolicyExecutor {
         return allow || false;
     }
 
-    async validatePolicy(policy: Policy): Promise<void> {
-        const allow = await this.evaluatePolicy(policy);
-        if (!allow) throw new Error(`Unauthorized by policy ${policy.name} in namespace ${policy.namespace}`);
-    }
+    private preparePolicyArgs(): PolicyArgsObject | undefined {
+        const supportedPolicyArgs = this.policyDefinition.args;
+        if (!supportedPolicyArgs) return;
 
-    private preparePolicyArgs(supportedPolicyArgs: PolicyArgsObject, policy: Policy): PolicyArgsObject {
         return Object.entries(supportedPolicyArgs).reduce<PolicyArgsObject>(
             (policyArgs, [policyArgName, policyArgType]) => {
-                if (policy?.args?.[policyArgName] === undefined)
+                if (this.policy?.args?.[policyArgName] === undefined)
                     throw new Error(
-                        `Missing arg ${policyArgName} for policy ${policy.name} in namespace ${policy.namespace}`
+                        `Missing arg ${policyArgName} for policy ${this.policy.name} in namespace ${this.policy.namespace}`
                     );
 
-                let policyArgValue = policy.args[policyArgName];
+                let policyArgValue = this.policy.args[policyArgName];
                 if (typeof policyArgValue === 'string') {
                     if (policyArgType === 'String') {
                         policyArgValue = injectParameters(
@@ -92,19 +107,19 @@ export class PolicyExecutor {
         );
     }
 
-    private getPolicyDefinition(namespace: string, name: string) {
-        const policyDefinition = this.policyDefinitions.find(({metadata}) => {
+    private getPolicyDefinition(policyDefinitions: PolicyDefinition[], namespace: string, name: string) {
+        const policyDefinition = policyDefinitions.find(({metadata}) => {
             return metadata.namespace === namespace && metadata.name === name;
         });
 
         if (!policyDefinition) throw new Error(`The policy ${name} in namespace ${namespace} was not found`);
         return policyDefinition;
     }
 
-    private async evaluatePolicyQuery(
-        query: PolicyQuery,
-        args: PolicyArgsObject = {}
-    ): Promise<QueryResults | undefined> {
+    private async evaluatePolicyQuery(args: PolicyArgsObject = {}): Promise<QueryResults | undefined> {
+        const query = this.policyDefinition.query;
+        if (!query) return;
+
         let variableValues =
             query.variables &&
             Object.entries(query.variables).reduce<{[key: string]: any}>((policyArgs, [varName, varValue]) => {

diff --git a/services/src/modules/directives/policy/policy-query.ts b/services/src/modules/directives/policy/policy-query.ts
@@ -19,8 +19,7 @@ export class PolicyQueryDirective extends SchemaDirectiveVisitor {
                 args: args,
             };
 
-            const executor = new PolicyExecutor([], parent, args, context, info);
-            const allow = await executor.evaluatePolicy(policy);
+            const allow = await PolicyExecutor.evaluatePolicy(policy, parent, args, context, info);
             return {allow};
         };
     }

diff --git a/services/src/modules/directives/policy/policy.ts b/services/src/modules/directives/policy/policy.ts
@@ -4,16 +4,16 @@ import {SchemaDirectiveVisitor} from 'graphql-tools';
 import {GraphQLField, defaultFieldResolver} from 'graphql';
 import {gql} from 'apollo-server-core';
 import {PolicyExecutor} from './policy-executor';
+import {Policy} from './types';
 
 export class PolicyDirective extends SchemaDirectiveVisitor {
     visitFieldDefinition(field: GraphQLField<any, any>) {
         const originalResolve = field.resolve || defaultFieldResolver;
-        const policies = this.args.policies;
+        const policy = this.args as Policy;
 
         field.resolve = async (parent: any, args: any, context: RequestContext, info: GraphQLResolveInfo) => {
             if (!context.ignorePolicies) {
-                const executor = new PolicyExecutor(policies, parent, args, context, info);
-                await executor.validatePolicies();
+                await PolicyExecutor.validatePolicy(policy, parent, args, context, info);
             }
 
             return originalResolve.call(field, parent, args, context, info);
@@ -22,11 +22,5 @@ export class PolicyDirective extends SchemaDirectiveVisitor {
 }
 
 export const sdl = gql`
-    input PolicyDirectivePolicy {
-        namespace: String!
-        name: String!
-        args: JSONObject
-    }
-
-    directive @policy(policies: [PolicyDirectivePolicy!]!) on FIELD_DEFINITION
+    directive @policy(namespace: String!, name: String!, args: JSONObject) on FIELD_DEFINITION
 `;
diff --git a/services/src/tests/e2e/tests/__snapshots__/authorization_with_queries.spec.ts.snap b/services/src/tests/e2e/tests/__snapshots__/authorization_with_queries.spec.ts.snap
@@ -10,10 +10,8 @@ Object {
         "exception": Object {
           "stacktrace": Array [
             "Error: Unauthorized by policy alwaysDenied in namespace namespace",
-            "    at PolicyExecutor.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:42:19)",
-            "    at async Promise.all (index 0)",
-            "    at async PolicyExecutor.validatePolicies (/service/dist/modules/directives/policy/policy-executor.js:20:9)",
-            "    at async field.resolve (/service/dist/modules/directives/policy/policy.js:14:17)",
+            "    at Function.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:27:19)",
+            "    at async field.resolve (/service/dist/modules/directives/policy/policy.js:13:17)",
           ],
         },
       },
@@ -58,10 +56,8 @@ Object {
         "exception": Object {
           "stacktrace": Array [
             "Error: Unauthorized by policy notClassified in namespace namespace",
-            "    at PolicyExecutor.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:42:19)",
-            "    at async Promise.all (index 0)",
-            "    at async PolicyExecutor.validatePolicies (/service/dist/modules/directives/policy/policy-executor.js:20:9)",
-            "    at async field.resolve (/service/dist/modules/directives/policy/policy.js:14:17)",
+            "    at Function.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:27:19)",
+            "    at async field.resolve (/service/dist/modules/directives/policy/policy.js:13:17)",
           ],
         },
       },
@@ -99,10 +95,8 @@ Object {
         "exception": Object {
           "stacktrace": Array [
             "Error: Unauthorized by policy notClassified in namespace namespace",
-            "    at PolicyExecutor.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:42:19)",
-            "    at async Promise.all (index 0)",
-            "    at async PolicyExecutor.validatePolicies (/service/dist/modules/directives/policy/policy-executor.js:20:9)",
-            "    at async field.resolve (/service/dist/modules/directives/policy/policy.js:14:17)",
+            "    at Function.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:27:19)",
+            "    at async field.resolve (/service/dist/modules/directives/policy/policy.js:13:17)",
           ],
         },
       },

diff --git a/services/src/tests/e2e/tests/authorization_with_queries.spec.ts b/services/src/tests/e2e/tests/authorization_with_queries.spec.ts
@@ -82,14 +82,14 @@ const schema: Schema = {
       name: String
       hireDate: Int
       department: Department!
-      address: String @policy(policies: [{
+      address: String @policy(
         namespace: "namespace",
         name: "notClassified",
         args: {
           departmentId: "{source.department.id}",
           hireDate: "{source.hireDate}"
         }
-      }])
+      )
     }
 
     type Query {
@@ -126,10 +126,7 @@ const schema: Schema = {
       classifiedDepartments: [Department!]! @stub(value: [{
         id: "D1000"
         name: "VIP"
-      }]) @policy(policies: [{
-        namespace: "namespace",
-        name: "alwaysDenied"
-      }])
+      }]) @policy(namespace: "namespace", name: "alwaysDenied")
     }
   `,
 };

diff --git a/services/src/tests/helpers/authzSchema.ts b/services/src/tests/helpers/authzSchema.ts
@@ -39,19 +39,19 @@ export const getSchema = () => ({
     schema: `
     type User {
       firstName: String
-      lastName: String @policy(policies: [
-        { namespace: "ns", name: "onlyAdmin", args: { role: "{source.role}" } }
-      ])
+      lastName: String @policy(namespace: "ns", name: "onlyAdmin", args: { role: "{source.role}" })
       role: String
     }
 
     type ArbitraryData {
-      arbitraryField: String @policy(policies: [
-        { namespace: "ns", name: "jwtName", args: {
+      arbitraryField: String @policy(
+        namespace: "ns",
+        name: "jwtName",
+        args: {
           jwtName: "{jwt.name}",
           allowedName: "Varg"
-        }}
-      ])
+        }
+      )
     }
 
     type Query {