From a679bbcc4ae4e8078f2dda6d0f5c37febc9f7ebd Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 28 Jul 2022 16:46:29 +0200 Subject: [PATCH 1/2] catalog-backend: fix conditional decisions for properties of type array Signed-off-by: Vincenzo Scamporlino --- .../rules/createPropertyRule.test.ts | 66 +++++++++++++++++++ .../permissions/rules/createPropertyRule.ts | 7 ++ 2 files changed, 73 insertions(+) diff --git a/plugins/catalog-backend/src/permissions/rules/createPropertyRule.test.ts b/plugins/catalog-backend/src/permissions/rules/createPropertyRule.test.ts index 80c86cf250..cb11add9ed 100644 --- a/plugins/catalog-backend/src/permissions/rules/createPropertyRule.test.ts +++ b/plugins/catalog-backend/src/permissions/rules/createPropertyRule.test.ts @@ -46,6 +46,22 @@ describe('createPropertyRule', () => { ).toBe(false); }); + it('returns false when specified key is an empty array', () => { + expect( + apply( + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'test-component', + tags: [], + }, + }, + 'tags', + ), + ).toBe(false); + }); + it('returns true when specified key is present', () => { expect( apply( @@ -63,6 +79,22 @@ describe('createPropertyRule', () => { ), ).toBe(true); }); + + it('returns true when specified key is an array containing more than an element', () => { + expect( + apply( + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'test-component', + tags: ['java'], + }, + }, + 'tags', + ), + ).toBe(true); + }); }); describe('key and value', () => { @@ -101,6 +133,23 @@ describe('createPropertyRule', () => { ).toBe(false); }); + it(`returns false when key is an array and doesn't contain the specified value`, () => { + expect( + apply( + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'test-component', + tags: ['java'], + }, + }, + 'tags', + 'python', + ), + ).toBe(false); + }); + it('returns true when specified key and value is present', () => { expect( apply( @@ -119,6 +168,23 @@ describe('createPropertyRule', () => { ), ).toBe(true); }); + + it(`returns true when key is an array and contains the specified value`, () => { + expect( + apply( + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'test-component', + tags: ['java', 'java11'], + }, + }, + 'tags', + 'java', + ), + ).toBe(true); + }); }); }); diff --git a/plugins/catalog-backend/src/permissions/rules/createPropertyRule.ts b/plugins/catalog-backend/src/permissions/rules/createPropertyRule.ts index 3808eced7b..57de5eb922 100644 --- a/plugins/catalog-backend/src/permissions/rules/createPropertyRule.ts +++ b/plugins/catalog-backend/src/permissions/rules/createPropertyRule.ts @@ -26,6 +26,13 @@ export const createPropertyRule = (propertyType: 'metadata' | 'spec') => resourceType: RESOURCE_TYPE_CATALOG_ENTITY, apply: (resource: Entity, key: string, value?: string) => { const foundValue = get(resource[propertyType], key); + + if (Array.isArray(foundValue)) { + if (value !== undefined) { + return foundValue.includes(value); + } + return foundValue.length > 0; + } if (value !== undefined) { return value === foundValue; } From e3d301853192923dd7e5816b3d47c1655574dc62 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 28 Jul 2022 16:55:36 +0200 Subject: [PATCH 2/2] Add changeset Signed-off-by: Vincenzo Scamporlino --- .changeset/strange-moles-design.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/strange-moles-design.md diff --git a/.changeset/strange-moles-design.md b/.changeset/strange-moles-design.md new file mode 100644 index 0000000000..8515d02e9b --- /dev/null +++ b/.changeset/strange-moles-design.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Fix issue for conditional decisions based on properties stored as arrays, like tags. + +Before this change, having a permission policy returning conditional decisions based on metadata like tags, such like `createCatalogConditionalDecision(permission, catalogConditions.hasMetadata('tags', 'java'),)`, was producing wrong results. The issue occurred when authorizing entities already loaded from the database, for example when authorizing `catalogEntityDeletePermission`.