From 8c8430b52db22fa4e7a051921f134a907e9cbc44 Mon Sep 17 00:00:00 2001
From: Vincenzo Scamporlino <vincenzos@spotify.com>
Date: Fri, 22 Dec 2023 10:02:45 +0100
Subject: [PATCH 1/3] scaffolder-backend: fix permission rule schema

Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>
---
 plugins/scaffolder-backend/src/service/rules.ts | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/plugins/scaffolder-backend/src/service/rules.ts b/plugins/scaffolder-backend/src/service/rules.ts
index 19b34b9f7c..b197810757 100644
--- a/plugins/scaffolder-backend/src/service/rules.ts
+++ b/plugins/scaffolder-backend/src/service/rules.ts
@@ -106,7 +106,9 @@ function buildHasProperty<Schema extends z.ZodType<JsonPrimitive>>({
       key: z
         .string()
         .describe(`Property within the action parameters to match on`),
-      value: valueSchema.describe(`Value of the given property to match on`),
+      value: valueSchema
+        .optional()
+        .describe(`Value of the given property to match on`),
     }) as unknown as z.ZodType<{ key: string; value?: z.infer<Schema> }>,
     apply: (resource, { key, value }) => {
       const foundValue = get(resource.input, key);

From 93be506ca9991cd0afde26af72d293db20bddf63 Mon Sep 17 00:00:00 2001
From: Vincenzo Scamporlino <vincenzos@spotify.com>
Date: Fri, 22 Dec 2023 10:03:59 +0100
Subject: [PATCH 2/3] scaffolder-backend: add extra permission rule tests

Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>
---
 .../src/service/rules.test.ts                 | 81 +++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/plugins/scaffolder-backend/src/service/rules.test.ts b/plugins/scaffolder-backend/src/service/rules.test.ts
index e6032dde15..8e24cacb32 100644
--- a/plugins/scaffolder-backend/src/service/rules.test.ts
+++ b/plugins/scaffolder-backend/src/service/rules.test.ts
@@ -23,6 +23,9 @@ import {
   hasStringProperty,
   hasTag,
 } from './rules';
+import { createConditionAuthorizer } from '@backstage/plugin-permission-node';
+import { RESOURCE_TYPE_SCAFFOLDER_ACTION } from '@backstage/plugin-scaffolder-common/alpha';
+import { AuthorizeResult } from '@backstage/plugin-permission-common';
 
 describe('hasTag', () => {
   describe('apply', () => {
@@ -206,6 +209,84 @@ describe('hasProperty', () => {
         ).toEqual(true);
       },
     );
+
+    it('should throw if params are invalid', () => {
+      const isActionAuthorized = createConditionAuthorizer([hasProperty]);
+
+      expect(() =>
+        isActionAuthorized(
+          {
+            resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION,
+            pluginId: 'scaffolder',
+            result: AuthorizeResult.CONDITIONAL,
+            conditions: {
+              resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION,
+              rule: 'HAS_PROPERTY',
+              params: {
+                key: 1,
+              },
+            },
+          },
+          { action: 'an-action', input: {} },
+        ),
+      ).toThrow();
+      expect(() =>
+        isActionAuthorized(
+          {
+            resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION,
+            pluginId: 'scaffolder',
+            result: AuthorizeResult.CONDITIONAL,
+            conditions: {
+              resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION,
+              rule: 'HAS_PROPERTY',
+              params: {},
+            },
+          },
+          { action: 'an-action', input: {} },
+        ),
+      ).toThrow();
+    });
+
+    it('should not throw if params are valid', () => {
+      const isActionAuthorized = createConditionAuthorizer([hasProperty]);
+
+      expect(() =>
+        isActionAuthorized(
+          {
+            resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION,
+            pluginId: 'scaffolder',
+            result: AuthorizeResult.CONDITIONAL,
+            conditions: {
+              resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION,
+              rule: 'HAS_PROPERTY',
+              params: {
+                key: 'key',
+              },
+            },
+          },
+          { action: 'an-action', input: {} },
+        ),
+      ).not.toThrow();
+
+      expect(() =>
+        isActionAuthorized(
+          {
+            resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION,
+            pluginId: 'scaffolder',
+            result: AuthorizeResult.CONDITIONAL,
+            conditions: {
+              resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION,
+              rule: 'HAS_PROPERTY',
+              params: {
+                key: 'key',
+                value: 'value',
+              },
+            },
+          },
+          { action: 'an-action', input: {} },
+        ),
+      ).not.toThrow();
+    });
   });
 });
 

From e9ab1c4b6d72b7ac264d1d5f1f76a3b6fbe32bc4 Mon Sep 17 00:00:00 2001
From: Vincenzo Scamporlino <vincenzos@spotify.com>
Date: Fri, 22 Dec 2023 10:26:57 +0100
Subject: [PATCH 3/3] scaffolder permission actions changesets

Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>
---
 .changeset/gold-pianos-worry.md | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 .changeset/gold-pianos-worry.md

diff --git a/.changeset/gold-pianos-worry.md b/.changeset/gold-pianos-worry.md
new file mode 100644
index 0000000000..39095dbbd7
--- /dev/null
+++ b/.changeset/gold-pianos-worry.md
@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-scaffolder-backend': patch
+---
+
+Fixed an issue where not passing a `value` to any of the action's permission conditions caused an error.