From 97be4a96ed7707d84f53ededd0a3af2044e1607f Mon Sep 17 00:00:00 2001
From: Harry Hogg <hhogg@spotify.com>
Date: Mon, 30 Jan 2023 14:27:21 +0000
Subject: [PATCH] Refactored createIsAuthorized to take a decision

Signed-off-by: Harry Hogg <hhogg@spotify.com>
Co-authored-by: Vincenzo Scamporlino <vincenzos@spotify.com>
---
 .../createPermissionIntegrationRouter.ts      | 18 +++--
 .../scaffolder-backend/src/service/router.ts  | 79 ++++---------------
 2 files changed, 26 insertions(+), 71 deletions(-)

diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts
index a4f14e6cbc..5b0c2c25f1 100644
--- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts
+++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts
@@ -27,6 +27,7 @@ import {
   Permission,
   PermissionCondition,
   PermissionCriteria,
+  PolicyDecision,
 } from '@backstage/plugin-permission-common';
 import { PermissionRule } from '../types';
 import {
@@ -167,18 +168,21 @@ const applyConditions = <TResourceType extends string, TResource>(
  *
  * @public
  */
-export const createIsAuthorized = <
-  TResourceType extends string,
-  TResource,
-  TQuery,
->(
+export const createIsAuthorized = <TResource, TQuery>(
   rules: PermissionRule<TResource, TQuery, string>[],
 ) => {
   const getRule = createGetRule(rules);
+
   return (
-    criteria: PermissionCriteria<PermissionCondition<TResourceType>>,
+    decision: PolicyDecision,
     resource: TResource | undefined,
-  ) => applyConditions(criteria, resource, getRule);
+  ): boolean => {
+    if (decision.result === AuthorizeResult.CONDITIONAL) {
+      return applyConditions(decision.conditions, resource, getRule);
+    }
+
+    return decision.result === AuthorizeResult.ALLOW;
+  };
 };
 
 /**
diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts
index 6442adbc4b..74b3d7a095 100644
--- a/plugins/scaffolder-backend/src/service/router.ts
+++ b/plugins/scaffolder-backend/src/service/router.ts
@@ -578,81 +578,32 @@ export async function createRouter(
       );
     }
 
-    const [parameterDecision, propertyDecision, stepDecision] =
+    const [parameterDecision, stepDecision] =
       await permissionApi.authorizeConditional(
         [
           { permission: templateParameterReadPermission },
-          { permission: templatePropertyReadPermission },
           { permission: templateStepReadPermission },
         ],
         { token },
       );
 
-    // authorize parameters
-    if (parameterDecision.result === AuthorizeResult.DENY) {
-      template.spec.parameters = [];
-    } else if (parameterDecision.result === AuthorizeResult.CONDITIONAL) {
-      if (Array.isArray(template.spec.parameters)) {
-        template.spec.parameters = template.spec.parameters.filter(step =>
-          isAuthorized(parameterDecision.conditions, step),
-        );
-      } else {
-        if (
-          template.spec.parameters &&
-          !isAuthorized(parameterDecision.conditions, template.spec.parameters)
-        ) {
-          template.spec.parameters = undefined;
-        }
-      }
-    }
-
-    // authorize properties
-    if (propertyDecision.result === AuthorizeResult.DENY) {
-      if (Array.isArray(template.spec.parameters)) {
-        template.spec.parameters.forEach(parameter => {
-          parameter.properties = {};
-        });
-      } else {
-        if (template.spec.parameters) {
-          template.spec.parameters.properties = {};
-        }
-      }
-    } else if (propertyDecision.result === AuthorizeResult.CONDITIONAL) {
-      if (Array.isArray(template.spec.parameters)) {
-        template.spec.parameters.forEach(parameter => {
-          parameter.properties = Object.entries(
-            parameter.properties || {},
-          ).reduce<Record<string, TemplateProperty>>((acc, [key, value]) => {
-            if (isAuthorized(propertyDecision.conditions, value)) {
-              acc[key] = value;
-            }
-            return acc;
-          }, {});
-        });
-      } else {
-        // TODO extract this to a generic method and use it in the above if block
-        if (template.spec.parameters) {
-          template.spec.parameters.properties = Object.entries(
-            template.spec.parameters.properties || {},
-          ).reduce<Record<string, TemplateProperty>>((acc, [key, value]) => {
-            if (isAuthorized(propertyDecision.conditions, value)) {
-              acc[key] = value;
-            }
-            return acc;
-          }, {});
-        }
-      }
-    }
-
-    // authorize steps
-    if (stepDecision.result === AuthorizeResult.DENY) {
-      template.spec.steps = [];
-    } else if (stepDecision.result === AuthorizeResult.CONDITIONAL) {
-      template.spec.steps = template.spec.steps.filter(step =>
-        isAuthorized(stepDecision.conditions, step),
+    // Authorize parameters
+    if (Array.isArray(template.spec.parameters)) {
+      template.spec.parameters = template.spec.parameters.filter(step =>
+        isAuthorized(parameterDecision, step),
       );
+    } else if (
+      template.spec.parameters &&
+      !isAuthorized(parameterDecision, template.spec.parameters)
+    ) {
+      template.spec.parameters = undefined;
     }
 
+    // Authorize steps
+    template.spec.steps = template.spec.steps.filter(step =>
+      isAuthorized(stepDecision, step),
+    );
+
     return template;
   }