From 9fe88c4fab903b293a43f0bfdc87469dc05fa359 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Wed, 28 Sep 2022 15:08:43 +0100 Subject: [PATCH] Added parameter validation using the param schemas Signed-off-by: Harry Hogg --- .../createConditionTransformer.test.ts | 4 +-- .../integration/createConditionTransformer.ts | 10 +++++++- .../createPermissionIntegrationRouter.test.ts | 25 +++++++++++-------- .../createPermissionIntegrationRouter.ts | 9 ++++++- 4 files changed, 33 insertions(+), 15 deletions(-) diff --git a/plugins/permission-node/src/integration/createConditionTransformer.test.ts b/plugins/permission-node/src/integration/createConditionTransformer.test.ts index 421bbcb2f4..e7e1e89d06 100644 --- a/plugins/permission-node/src/integration/createConditionTransformer.test.ts +++ b/plugins/permission-node/src/integration/createConditionTransformer.test.ts @@ -27,7 +27,7 @@ const transformConditions = createConditionTransformer([ name: 'test-rule-1', description: 'Test rule 1', resourceType: 'test-resource', - schema: z.tuple([]), + schema: z.tuple([z.string(), z.number()]), apply: jest.fn(), toQuery: jest.fn( (firstParam: string, secondParam: number) => @@ -38,7 +38,7 @@ const transformConditions = createConditionTransformer([ name: 'test-rule-2', description: 'Test rule 2', resourceType: 'test-resource', - schema: z.tuple([]), + schema: z.tuple([z.object({})]), apply: jest.fn(), toQuery: jest.fn( (firstParam: object) => `test-rule-2:${JSON.stringify(firstParam)}`, diff --git a/plugins/permission-node/src/integration/createConditionTransformer.ts b/plugins/permission-node/src/integration/createConditionTransformer.ts index 112bb30d01..b40063ec13 100644 --- a/plugins/permission-node/src/integration/createConditionTransformer.ts +++ b/plugins/permission-node/src/integration/createConditionTransformer.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import { InputError } from '@backstage/errors'; import { AllOfCriteria, AnyOfCriteria, @@ -45,7 +46,14 @@ const mapConditions = ( }; } - return getRule(criteria.rule).toQuery(...criteria.params); + const rule = getRule(criteria.rule); + const result = rule.schema.safeParse(criteria.params); + + if (rule.schema && !result.success) { + throw new InputError(`Parameters to rule are invalid`, result.error); + } + + return rule.toQuery(...criteria.params); }; /** diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts index 829b1b3614..5649480a9c 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts @@ -40,7 +40,10 @@ const testRule1 = createPermissionRule({ name: 'test-rule-1', description: 'Test rule 1', resourceType: 'test-resource', - schema: z.tuple([z.string(), z.number()]), + schema: z.tuple([ + z.string().describe('firstParam'), + z.number().describe('secondParam'), + ]), apply: (_resource: any, _firstParam: string, _secondParam: number) => true, toQuery: (_firstParam: string, _secondParam: number) => ({}), }); @@ -49,7 +52,7 @@ const testRule2 = createPermissionRule({ name: 'test-rule-2', description: 'Test rule 2', resourceType: 'test-resource', - schema: z.tuple([z.object({})]), + schema: z.tuple([z.object({}).describe('firstParam')]), apply: (_resource: any, _firstParam: object) => false, toQuery: (_firstParam: object) => ({}), }); @@ -250,7 +253,7 @@ describe('createPermissionIntegrationRouter', () => { conditions: { rule: 'test-rule-1', resourceType: 'test-resource', - params: [], + params: ['a', 1], }, }, { @@ -260,7 +263,7 @@ describe('createPermissionIntegrationRouter', () => { conditions: { rule: 'test-rule-2', resourceType: 'test-resource', - params: [], + params: [{}], }, }, { @@ -271,7 +274,7 @@ describe('createPermissionIntegrationRouter', () => { not: { rule: 'test-rule-1', resourceType: 'test-resource', - params: [], + params: ['a', 1], }, }, }, @@ -283,7 +286,7 @@ describe('createPermissionIntegrationRouter', () => { not: { rule: 'test-rule-2', resourceType: 'test-resource', - params: [], + params: [{}], }, }, }, @@ -296,12 +299,12 @@ describe('createPermissionIntegrationRouter', () => { { rule: 'test-rule-1', resourceType: 'test-resource', - params: [], + params: ['a', 1], }, { rule: 'test-rule-2', resourceType: 'test-resource', - params: [], + params: [{}], }, ], }, @@ -430,7 +433,7 @@ describe('createPermissionIntegrationRouter', () => { conditions: { rule: 'test-rule-1', resourceType: 'test-resource', - params: [], + params: ['a', 1], }, }, { @@ -440,7 +443,7 @@ describe('createPermissionIntegrationRouter', () => { conditions: { rule: 'test-rule-1', resourceType: 'test-resource', - params: [], + params: ['a', 1], }, }, { @@ -450,7 +453,7 @@ describe('createPermissionIntegrationRouter', () => { conditions: { rule: 'test-rule-1', resourceType: 'test-resource', - params: [], + params: ['a', 1], }, }, ], diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index 28abc2f48d..6c4df1763f 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -124,7 +124,14 @@ const applyConditions = ( return !applyConditions(criteria.not, resource, getRule); } - return getRule(criteria.rule).apply(resource, ...criteria.params); + const rule = getRule(criteria.rule); + const result = rule.schema.safeParse(criteria.params); + + if (rule.schema && !result.success) { + throw new InputError(`Parameters to rule are invalid`, result.error); + } + + return rule.apply(resource, ...criteria.params); }; /**