Merge pull request #21984 from backstage/vinzscam/fix-scaffolder-property-conditions

Scaffolder action conditions validation fix

.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.

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();
+    });
   });
 });
 

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);