Refactor the decide to return an approvals set

narval-xyz · Mar 19, 2024 · 3f9c812 · 3f9c812
1 parent 8b11c74
commit 3f9c812
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 70 deletions.
diff --git a/apps/policy-engine/src/open-policy-agent/core/__test__/unit/open-policy-agent.engine.spec.ts b/apps/policy-engine/src/open-policy-agent/core/__test__/unit/open-policy-agent.engine.spec.ts
@@ -120,7 +120,7 @@ describe('OpenPolicyAgentEngine', () => {
 
   describe('decide', () => {
     it('returns forbid when any of the reasons is forbid', () => {
-      const response: Result[] = [
+      const results: Result[] = [
         {
           permit: false,
           reasons: [
@@ -142,13 +142,13 @@ describe('OpenPolicyAgentEngine', () => {
         }
       ]
 
-      const result = engine.decide(response)
+      const result = engine.decide(results)
 
       expect(result.decision).toEqual(Decision.FORBID)
     })
 
     it('returns permit when all of the reasons are permit', () => {
-      const response: Result[] = [
+      const results: Result[] = [
         {
           permit: true,
           reasons: [
@@ -170,13 +170,13 @@ describe('OpenPolicyAgentEngine', () => {
         }
       ]
 
-      const result = engine.decide(response)
+      const result = engine.decide(results)
 
       expect(result.decision).toEqual(Decision.PERMIT)
     })
 
     it('returns confirm when any of the reasons are forbid for a permit type rule where approvals are missing', () => {
-      const response: Result[] = [
+      const results: Result[] = [
         {
           permit: false,
           reasons: [
@@ -198,7 +198,7 @@ describe('OpenPolicyAgentEngine', () => {
         }
       ]
 
-      const result = engine.decide(response)
+      const result = engine.decide(results)
 
       expect(result.decision).toEqual(Decision.CONFIRM)
     })
@@ -232,7 +232,7 @@ describe('OpenPolicyAgentEngine', () => {
         entityIds: ['user-id'],
         countPrincipal: true
       }
-      const response: Result[] = [
+      const results: Result[] = [
         {
           permit: false,
           reasons: [
@@ -258,14 +258,16 @@ describe('OpenPolicyAgentEngine', () => {
           ]
         }
       ]
-      const result = engine.decide(response)
+
+      const result = engine.decide(results)
 
       expect(result).toEqual({
-        originalResponse: response,
         decision: Decision.CONFIRM,
-        totalApprovalsRequired: [missingApproval, missingApproval2, satisfiedApproval, satisfiedApproval2],
-        approvalsMissing: [missingApproval, missingApproval2],
-        approvalsSatisfied: [satisfiedApproval, satisfiedApproval2]
+        approvals: {
+          required: [missingApproval, missingApproval2, satisfiedApproval, satisfiedApproval2],
+          missing: [missingApproval, missingApproval2],
+          satisfied: [satisfiedApproval, satisfiedApproval2]
+        }
       })
     })
   })

diff --git a/apps/policy-engine/src/open-policy-agent/core/open-policy-agent.engine.ts b/apps/policy-engine/src/open-policy-agent/core/open-policy-agent.engine.ts
@@ -1,5 +1,6 @@
 import {
   Action,
+  ApprovalRequirement,
   Decision,
   Engine,
   Entities,
@@ -9,6 +10,7 @@ import {
 } from '@narval/policy-engine-shared'
 import { HttpStatus } from '@nestjs/common'
 import { loadPolicy } from '@open-policy-agent/opa-wasm'
+import { compact } from 'lodash/fp'
 import { resolve } from 'path'
 import { v4 } from 'uuid'
 import { z } from 'zod'
@@ -109,15 +111,8 @@ export class OpenPolicyAgentEngine implements Engine<OpenPolicyAgentEngine> {
 
     return {
       decision: decision.decision,
-      request: request.request,
-      approvals: undefined
-      //approvals: decision.totalApprovalsRequired?.length
-      //  ? {
-      //      required: decision.totalApprovalsRequired,
-      //      satisfied: decision.approvalsSatisfied,
-      //      missing: decision.approvalsMissing
-      //    }
-      //  : undefined
+      approvals: decision.approvals,
+      request: request.request
     }
   }
 
@@ -151,39 +146,44 @@ export class OpenPolicyAgentEngine implements Engine<OpenPolicyAgentEngine> {
     })
   }
 
-  decide(results: Result[]) {
+  /**
+   * Computes OPA's query results into a final decision. This step is also
+   * known as "post-evaluation".
+   */
+  decide(results: Result[]): {
+    decision: Decision
+    approvals?: {
+      required: ApprovalRequirement[]
+      missing: ApprovalRequirement[]
+      satisfied: ApprovalRequirement[]
+    }
+  } {
     const implicitForbid = results.some(this.isImplictForbid)
     const anyExplicitForbid = results.some(this.isExplictForbid)
     const allPermit = results.every(this.isPermit)
-    const anyPermitWithMissingApprovals = results.some(this.isPermitMissingApproval)
+    const permitsMissingApproval = results.some(this.isPermitMissingApproval)
 
     if (implicitForbid || anyExplicitForbid) {
-      return {
-        originalResponse: results,
-        decision: Decision.FORBID,
-        approvalsMissing: [],
-        approvalsSatisfied: []
-      }
+      return { decision: Decision.FORBID }
     }
 
-    // Collect all the approvalsMissing & approvalsSatisfied using functional
-    // map/flat operators
-    const approvalsSatisfied = results
-      .flatMap((result) => result.reasons?.flatMap((reason) => reason.approvalsSatisfied))
-      .filter((v) => !!v)
-    const approvalsMissing = results
-      .flatMap((result) => result.reasons?.flatMap((reason) => reason.approvalsMissing))
-      .filter((v) => !!v)
-    const totalApprovalsRequired = approvalsMissing.concat(approvalsSatisfied)
+    const approvalsSatisfied = compact(
+      results.flatMap((result) => result.reasons?.flatMap((reason) => reason.approvalsSatisfied)).filter((v) => !!v)
+    )
+    const approvalsMissing = compact(
+      results.flatMap((result) => result.reasons?.flatMap((reason) => reason.approvalsMissing)).filter((v) => !!v)
+    )
+    const approvalsRequired = compact(approvalsMissing.concat(approvalsSatisfied))
 
-    const decision = allPermit && !anyPermitWithMissingApprovals ? Decision.PERMIT : Decision.CONFIRM
+    const decision = allPermit && !permitsMissingApproval ? Decision.PERMIT : Decision.CONFIRM
 
     return {
-      originalResponse: results,
       decision,
-      totalApprovalsRequired,
-      approvalsMissing,
-      approvalsSatisfied
+      approvals: {
+        missing: approvalsMissing,
+        required: approvalsRequired,
+        satisfied: approvalsSatisfied
+      }
     }
   }
 

diff --git a/apps/policy-engine/src/open-policy-agent/script/evaluation.script.ts b/apps/policy-engine/src/open-policy-agent/script/evaluation.script.ts
diff --git a/packages/policy-engine-shared/src/lib/type/domain.type.ts b/packages/policy-engine-shared/src/lib/type/domain.type.ts
@@ -158,7 +158,6 @@ export type EvaluationResponse = {
   accessToken?: AccessToken
   transactionRequestIntent?: unknown
 }
-// DOMAIN
 
 export type Hex = `0x${string}` // DOMAIN