From 6d447843fa4657918f4b450ed34a129c8f479e4d Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Mon, 3 Oct 2022 10:19:47 +0100 Subject: [PATCH] Changing over permission rules params API to accept a single object Signed-off-by: Harry Hogg --- .../rules/createPropertyRule.test.ts | 52 +++-- .../permissions/rules/createPropertyRule.ts | 16 +- .../permissions/rules/hasAnnotation.test.ts | 41 ++-- .../src/permissions/rules/hasAnnotation.ts | 16 +- .../src/permissions/rules/hasLabel.test.ts | 16 +- .../src/permissions/rules/hasLabel.ts | 9 +- .../permissions/rules/isEntityKind.test.ts | 18 +- .../src/permissions/rules/isEntityKind.ts | 9 +- .../permissions/rules/isEntityOwner.test.ts | 30 ++- .../src/permissions/rules/isEntityOwner.ts | 10 +- .../src/permissions/rules/util.ts | 5 +- .../service/AuthorizedEntitiesCatalog.test.ts | 8 +- .../src/service/createRouter.test.ts | 8 +- .../PermissionIntegrationClient.test.ts | 41 ++-- .../src/PermissionClient.test.ts | 4 +- .../permission-common/src/PermissionClient.ts | 2 +- plugins/permission-common/src/types/api.ts | 2 +- .../createConditionExports.test.ts | 55 ++++-- .../src/integration/createConditionExports.ts | 2 +- .../createConditionFactory.test.ts | 18 +- .../src/integration/createConditionFactory.ts | 4 +- .../createConditionTransformer.test.ts | 99 +++++++--- .../integration/createConditionTransformer.ts | 2 +- .../createPermissionIntegrationRouter.test.ts | 180 ++++++++++++------ .../createPermissionIntegrationRouter.ts | 4 +- .../src/integration/createPermissionRule.ts | 4 +- .../src/integration/util.test.ts | 4 +- plugins/permission-node/src/types.ts | 12 +- .../DefaultPlaylistPermissionPolicy.test.ts | 32 ++-- .../DefaultPlaylistPermissionPolicy.ts | 10 +- .../playlist-backend/src/permissions/rules.ts | 17 +- 31 files changed, 495 insertions(+), 235 deletions(-) diff --git a/plugins/catalog-backend/src/permissions/rules/createPropertyRule.test.ts b/plugins/catalog-backend/src/permissions/rules/createPropertyRule.test.ts index cb11add9ed..5b34c98ca6 100644 --- a/plugins/catalog-backend/src/permissions/rules/createPropertyRule.test.ts +++ b/plugins/catalog-backend/src/permissions/rules/createPropertyRule.test.ts @@ -41,7 +41,9 @@ describe('createPropertyRule', () => { name: 'test-component', }, }, - 'org.name', + { + key: 'org.name', + }, ), ).toBe(false); }); @@ -57,7 +59,9 @@ describe('createPropertyRule', () => { tags: [], }, }, - 'tags', + { + key: 'tags', + }, ), ).toBe(false); }); @@ -75,7 +79,9 @@ describe('createPropertyRule', () => { }, }, }, - 'org.name', + { + key: 'org.name', + }, ), ).toBe(true); }); @@ -91,7 +97,9 @@ describe('createPropertyRule', () => { tags: ['java'], }, }, - 'tags', + { + key: 'tags', + }, ), ).toBe(true); }); @@ -108,8 +116,10 @@ describe('createPropertyRule', () => { name: 'test-component', }, }, - 'org.name', - 'test-org', + { + key: 'org.name', + value: 'test-org', + }, ), ).toBe(false); }); @@ -127,8 +137,10 @@ describe('createPropertyRule', () => { }, }, }, - 'org.name', - 'test-org', + { + key: 'org.name', + value: 'test-org', + }, ), ).toBe(false); }); @@ -144,8 +156,10 @@ describe('createPropertyRule', () => { tags: ['java'], }, }, - 'tags', - 'python', + { + key: 'tags', + value: 'python', + }, ), ).toBe(false); }); @@ -163,8 +177,10 @@ describe('createPropertyRule', () => { }, }, }, - 'org.name', - 'test-org', + { + key: 'org.name', + value: 'test-org', + }, ), ).toBe(true); }); @@ -180,8 +196,10 @@ describe('createPropertyRule', () => { tags: ['java', 'java11'], }, }, - 'tags', - 'java', + { + key: 'tags', + value: 'java', + }, ), ).toBe(true); }); @@ -190,7 +208,11 @@ describe('createPropertyRule', () => { describe('toQuery', () => { it('returns an appropriate catalog-backend filter', () => { - expect(toQuery('backstage.io/test-component')).toEqual({ + expect( + toQuery({ + key: 'backstage.io/test-component', + }), + ).toEqual({ key: 'metadata.backstage.io/test-component', }); }); diff --git a/plugins/catalog-backend/src/permissions/rules/createPropertyRule.ts b/plugins/catalog-backend/src/permissions/rules/createPropertyRule.ts index 12725c1fcd..da487c112c 100644 --- a/plugins/catalog-backend/src/permissions/rules/createPropertyRule.ts +++ b/plugins/catalog-backend/src/permissions/rules/createPropertyRule.ts @@ -15,7 +15,6 @@ */ import { get } from 'lodash'; -import { Entity } from '@backstage/catalog-model'; import { RESOURCE_TYPE_CATALOG_ENTITY } from '@backstage/plugin-catalog-common'; import { createCatalogPermissionRule } from './util'; import { z } from 'zod'; @@ -25,11 +24,14 @@ export const createPropertyRule = (propertyType: 'metadata' | 'spec') => name: `HAS_${propertyType.toUpperCase()}`, description: `Allow entities which have the specified ${propertyType} subfield.`, resourceType: RESOURCE_TYPE_CATALOG_ENTITY, - schema: z.tuple([ - z.string().describe('Property name'), - z.string().optional().describe('Property value'), - ]), - apply: (resource: Entity, key: string, value?: string) => { + schema: z.object({ + key: z.string().describe(`The key of the ${propertyType} to match on`), + value: z + .string() + .optional() + .describe(`Optional value of the ${propertyType} to match on`), + }), + apply: (resource, { key, value }) => { const foundValue = get(resource[propertyType], key); if (Array.isArray(foundValue)) { @@ -43,7 +45,7 @@ export const createPropertyRule = (propertyType: 'metadata' | 'spec') => } return !!foundValue; }, - toQuery: (key: string, value?: string) => ({ + toQuery: ({ key, value }) => ({ key: `${propertyType}.${key}`, ...(value !== undefined && { values: [value] }), }), diff --git a/plugins/catalog-backend/src/permissions/rules/hasAnnotation.test.ts b/plugins/catalog-backend/src/permissions/rules/hasAnnotation.test.ts index 609114be3d..73fce20a71 100644 --- a/plugins/catalog-backend/src/permissions/rules/hasAnnotation.test.ts +++ b/plugins/catalog-backend/src/permissions/rules/hasAnnotation.test.ts @@ -31,7 +31,9 @@ describe('hasAnnotation permission rule', () => { }, }, }, - 'backstage.io/test-annotation', + { + annotation: 'backstage.io/test-annotation', + }, ), ).toEqual(false); }); @@ -46,7 +48,9 @@ describe('hasAnnotation permission rule', () => { name: 'test-component', }, }, - 'backstage.io/test-annotation', + { + annotation: 'backstage.io/test-annotation', + }, ), ).toEqual(false); expect( @@ -58,8 +62,10 @@ describe('hasAnnotation permission rule', () => { name: 'test-component', }, }, - 'backstage.io/test-annotation', - 'some value', + { + annotation: 'backstage.io/test-annotation', + value: 'some value', + }, ), ).toEqual(false); }); @@ -78,7 +84,9 @@ describe('hasAnnotation permission rule', () => { }, }, }, - 'backstage.io/test-annotation', + { + annotation: 'backstage.io/test-annotation', + }, ), ).toEqual(true); }); @@ -97,8 +105,10 @@ describe('hasAnnotation permission rule', () => { }, }, }, - 'backstage.io/test-annotation', - 'baz', + { + annotation: 'backstage.io/test-annotation', + value: 'baz', + }, ), ).toEqual(false); }); @@ -117,8 +127,10 @@ describe('hasAnnotation permission rule', () => { }, }, }, - 'backstage.io/test-annotation', - 'bar', + { + annotation: 'backstage.io/test-annotation', + value: 'bar', + }, ), ).toEqual(true); }); @@ -126,7 +138,11 @@ describe('hasAnnotation permission rule', () => { describe('toQuery', () => { it('returns an appropriate catalog-backend filter', () => { - expect(hasAnnotation.toQuery('backstage.io/test-annotation')).toEqual({ + expect( + hasAnnotation.toQuery({ + annotation: 'backstage.io/test-annotation', + }), + ).toEqual({ key: 'metadata.annotations.backstage.io/test-annotation', }); }); @@ -134,7 +150,10 @@ describe('hasAnnotation permission rule', () => { it('returns an appropriate catalog-backend filter with values', () => { expect( - hasAnnotation.toQuery('backstage.io/test-annotation', 'foo'), + hasAnnotation.toQuery({ + annotation: 'backstage.io/test-annotation', + value: 'foo', + }), ).toEqual({ key: 'metadata.annotations.backstage.io/test-annotation', values: ['foo'], diff --git a/plugins/catalog-backend/src/permissions/rules/hasAnnotation.ts b/plugins/catalog-backend/src/permissions/rules/hasAnnotation.ts index 4042c34636..9651e9d925 100644 --- a/plugins/catalog-backend/src/permissions/rules/hasAnnotation.ts +++ b/plugins/catalog-backend/src/permissions/rules/hasAnnotation.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { Entity } from '@backstage/catalog-model'; import { RESOURCE_TYPE_CATALOG_ENTITY } from '@backstage/plugin-catalog-common'; import { z } from 'zod'; import { createCatalogPermissionRule } from './util'; @@ -32,16 +31,19 @@ export const hasAnnotation = createCatalogPermissionRule({ description: 'Allow entities which are annotated with the specified annotation', resourceType: RESOURCE_TYPE_CATALOG_ENTITY, - schema: z.tuple([ - z.string().describe('Annotation name'), - z.string().optional().describe('Annotation value'), - ]), - apply: (resource: Entity, annotation: string, value?: string) => + schema: z.object({ + annotation: z.string().describe('The name of the annotation to match on'), + value: z + .string() + .optional() + .describe('Optional value of the annotation to match on'), + }), + apply: (resource, { annotation, value }) => !!resource.metadata.annotations?.hasOwnProperty(annotation) && (value === undefined ? true : resource.metadata.annotations?.[annotation] === value), - toQuery: (annotation: string, value?: string) => + toQuery: ({ annotation, value }) => value === undefined ? { key: `metadata.annotations.${annotation}`, diff --git a/plugins/catalog-backend/src/permissions/rules/hasLabel.test.ts b/plugins/catalog-backend/src/permissions/rules/hasLabel.test.ts index a1b9cb5ad0..8aa43402c5 100644 --- a/plugins/catalog-backend/src/permissions/rules/hasLabel.test.ts +++ b/plugins/catalog-backend/src/permissions/rules/hasLabel.test.ts @@ -31,7 +31,9 @@ describe('hasLabel permission rule', () => { }, }, }, - 'backstage.io/testlabel', + { + label: 'backstage.io/testlabel', + }, ), ).toEqual(false); }); @@ -46,7 +48,9 @@ describe('hasLabel permission rule', () => { name: 'test-component', }, }, - 'backstage.io/testlabel', + { + label: 'backstage.io/testlabel', + }, ), ).toEqual(false); }); @@ -65,7 +69,7 @@ describe('hasLabel permission rule', () => { }, }, }, - 'backstage.io/testlabel', + { label: 'backstage.io/testlabel' }, ), ).toEqual(true); }); @@ -73,7 +77,11 @@ describe('hasLabel permission rule', () => { describe('toQuery', () => { it('returns an appropriate catalog-backend filter', () => { - expect(hasLabel.toQuery('backstage.io/testlabel')).toEqual({ + expect( + hasLabel.toQuery({ + label: 'backstage.io/testlabel', + }), + ).toEqual({ key: 'metadata.labels.backstage.io/testlabel', }); }); diff --git a/plugins/catalog-backend/src/permissions/rules/hasLabel.ts b/plugins/catalog-backend/src/permissions/rules/hasLabel.ts index 3d1a18f8b3..fbad7e42d0 100644 --- a/plugins/catalog-backend/src/permissions/rules/hasLabel.ts +++ b/plugins/catalog-backend/src/permissions/rules/hasLabel.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { Entity } from '@backstage/catalog-model'; import { RESOURCE_TYPE_CATALOG_ENTITY } from '@backstage/plugin-catalog-common'; import { z } from 'zod'; import { createCatalogPermissionRule } from './util'; @@ -28,10 +27,12 @@ export const hasLabel = createCatalogPermissionRule({ name: 'HAS_LABEL', description: 'Allow entities which have the specified label metadata.', resourceType: RESOURCE_TYPE_CATALOG_ENTITY, - schema: z.tuple([z.string().describe('Label name')]), - apply: (resource: Entity, label: string) => + schema: z.object({ + label: z.string().describe('Name of the label'), + }), + apply: (resource, { label }) => !!resource.metadata.labels?.hasOwnProperty(label), - toQuery: (label: string) => ({ + toQuery: ({ label }) => ({ key: `metadata.labels.${label}`, }), }); diff --git a/plugins/catalog-backend/src/permissions/rules/isEntityKind.test.ts b/plugins/catalog-backend/src/permissions/rules/isEntityKind.test.ts index e11e102728..28983f6bec 100644 --- a/plugins/catalog-backend/src/permissions/rules/isEntityKind.test.ts +++ b/plugins/catalog-backend/src/permissions/rules/isEntityKind.test.ts @@ -27,7 +27,11 @@ describe('isEntityKind', () => { name: 'some-component', }, }; - expect(isEntityKind.apply(component, ['b'])).toBe(true); + expect( + isEntityKind.apply(component, { + kinds: ['b'], + }), + ).toBe(true); }); it('returns false when entity is not the correct kind', () => { @@ -38,13 +42,21 @@ describe('isEntityKind', () => { name: 'some-component', }, }; - expect(isEntityKind.apply(component, ['c'])).toBe(false); + expect( + isEntityKind.apply(component, { + kinds: ['c'], + }), + ).toBe(false); }); }); describe('toQuery', () => { it('returns an appropriate catalog-backend filter', () => { - expect(isEntityKind.toQuery(['b'])).toEqual({ + expect( + isEntityKind.toQuery({ + kinds: ['b'], + }), + ).toEqual({ key: 'kind', values: ['b'], }); diff --git a/plugins/catalog-backend/src/permissions/rules/isEntityKind.ts b/plugins/catalog-backend/src/permissions/rules/isEntityKind.ts index 70467558dd..8c16f4c5a1 100644 --- a/plugins/catalog-backend/src/permissions/rules/isEntityKind.ts +++ b/plugins/catalog-backend/src/permissions/rules/isEntityKind.ts @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Entity } from '@backstage/catalog-model'; import { RESOURCE_TYPE_CATALOG_ENTITY } from '@backstage/plugin-catalog-common'; import { z } from 'zod'; import { EntitiesSearchFilter } from '../../catalog/types'; @@ -28,12 +27,14 @@ export const isEntityKind = createCatalogPermissionRule({ name: 'IS_ENTITY_KIND', description: 'Allow entities with the specified kind', resourceType: RESOURCE_TYPE_CATALOG_ENTITY, - schema: z.tuple([z.array(z.string().describe('List of entity kinds'))]), - apply(resource: Entity, kinds: string[]) { + schema: z.object({ + kinds: z.array(z.string()), + }), + apply(resource, { kinds }) { const resourceKind = resource.kind.toLocaleLowerCase('en-US'); return kinds.some(kind => kind.toLocaleLowerCase('en-US') === resourceKind); }, - toQuery(kinds: string[]): EntitiesSearchFilter { + toQuery({ kinds }): EntitiesSearchFilter { return { key: 'kind', values: kinds.map(kind => kind.toLocaleLowerCase('en-US')), diff --git a/plugins/catalog-backend/src/permissions/rules/isEntityOwner.test.ts b/plugins/catalog-backend/src/permissions/rules/isEntityOwner.test.ts index 11304c768c..f09e484c32 100644 --- a/plugins/catalog-backend/src/permissions/rules/isEntityOwner.test.ts +++ b/plugins/catalog-backend/src/permissions/rules/isEntityOwner.test.ts @@ -33,9 +33,11 @@ describe('isEntityOwner', () => { }, ], }; - expect(isEntityOwner.apply(component, ['user:default/spiderman'])).toBe( - true, - ); + expect( + isEntityOwner.apply(component, { + claims: ['user:default/spiderman'], + }), + ).toBe(true); }); it('returns false when entity is not owned by the given user', () => { @@ -52,9 +54,11 @@ describe('isEntityOwner', () => { }, ], }; - expect(isEntityOwner.apply(component, ['user:default/spiderman'])).toBe( - false, - ); + expect( + isEntityOwner.apply(component, { + claims: ['user:default/spiderman'], + }), + ).toBe(false); }); it('returns false when entity does not have an owner', () => { @@ -65,15 +69,21 @@ describe('isEntityOwner', () => { name: 'some-component', }, }; - expect(isEntityOwner.apply(component, ['user:default/spiderman'])).toBe( - false, - ); + expect( + isEntityOwner.apply(component, { + claims: ['user:default/spiderman'], + }), + ).toBe(false); }); }); describe('toQuery', () => { it('returns an appropriate catalog-backend filter', () => { - expect(isEntityOwner.toQuery(['user:default/spiderman'])).toEqual({ + expect( + isEntityOwner.toQuery({ + claims: ['user:default/spiderman'], + }), + ).toEqual({ key: 'relations.ownedBy', values: ['user:default/spiderman'], }); diff --git a/plugins/catalog-backend/src/permissions/rules/isEntityOwner.ts b/plugins/catalog-backend/src/permissions/rules/isEntityOwner.ts index c0cc7c9911..27431f09a7 100644 --- a/plugins/catalog-backend/src/permissions/rules/isEntityOwner.ts +++ b/plugins/catalog-backend/src/permissions/rules/isEntityOwner.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { Entity, RELATION_OWNED_BY } from '@backstage/catalog-model'; +import { RELATION_OWNED_BY } from '@backstage/catalog-model'; import { RESOURCE_TYPE_CATALOG_ENTITY } from '@backstage/plugin-catalog-common'; import { z } from 'zod'; import { createCatalogPermissionRule } from './util'; @@ -29,8 +29,10 @@ export const isEntityOwner = createCatalogPermissionRule({ name: 'IS_ENTITY_OWNER', description: 'Allow entities owned by the current user', resourceType: RESOURCE_TYPE_CATALOG_ENTITY, - schema: z.tuple([z.array(z.string().describe('List of owners'))]), - apply: (resource: Entity, claims: string[]) => { + schema: z.object({ + claims: z.array(z.string()), + }), + apply: (resource, { claims }) => { if (!resource.relations) { return false; } @@ -39,7 +41,7 @@ export const isEntityOwner = createCatalogPermissionRule({ .filter(relation => relation.type === RELATION_OWNED_BY) .some(relation => claims.includes(relation.targetRef)); }, - toQuery: (claims: string[]) => ({ + toQuery: ({ claims }) => ({ key: 'relations.ownedBy', values: claims, }), diff --git a/plugins/catalog-backend/src/permissions/rules/util.ts b/plugins/catalog-backend/src/permissions/rules/util.ts index 36b351595d..37544a14f8 100644 --- a/plugins/catalog-backend/src/permissions/rules/util.ts +++ b/plugins/catalog-backend/src/permissions/rules/util.ts @@ -29,8 +29,9 @@ import { EntitiesSearchFilter } from '../../catalog/types'; * * @alpha */ -export type CatalogPermissionRule = - PermissionRule; +export type CatalogPermissionRule< + TParams extends Record = Record, +> = PermissionRule; /** * Helper function for creating correctly-typed diff --git a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts index b15d1df4bb..66764dc89f 100644 --- a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts @@ -65,7 +65,7 @@ describe('AuthorizedEntitiesCatalog', () => { fakePermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.CONDITIONAL, - conditions: { rule: 'IS_ENTITY_KIND', params: [['b']] }, + conditions: { rule: 'IS_ENTITY_KIND', params: { kinds: ['b'] } }, }, ]); const catalog = createCatalog(isEntityKind); @@ -117,7 +117,7 @@ describe('AuthorizedEntitiesCatalog', () => { fakePermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.CONDITIONAL, - conditions: { rule: 'IS_ENTITY_KIND', params: [['b']] }, + conditions: { rule: 'IS_ENTITY_KIND', params: { kinds: ['b'] } }, }, ]); fakeCatalog.entities.mockResolvedValue({ entities: [] }); @@ -136,7 +136,7 @@ describe('AuthorizedEntitiesCatalog', () => { fakePermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.CONDITIONAL, - conditions: { rule: 'IS_ENTITY_KIND', params: [['b']] }, + conditions: { rule: 'IS_ENTITY_KIND', params: { kinds: ['b'] } }, }, ]); fakeCatalog.entities.mockResolvedValue({ @@ -272,7 +272,7 @@ describe('AuthorizedEntitiesCatalog', () => { fakePermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.CONDITIONAL, - conditions: { rule: 'IS_ENTITY_KIND', params: [['b']] }, + conditions: { rule: 'IS_ENTITY_KIND', params: { kinds: ['b'] } }, }, ]); const catalog = createCatalog(isEntityKind); diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index 163e5e0747..088e6c71e2 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -696,7 +696,9 @@ describe('NextRouter permissioning', () => { name: 'FAKE_RULE', description: 'fake rule', resourceType: RESOURCE_TYPE_CATALOG_ENTITY, - schema: z.tuple([]), + schema: z.object({ + foo: z.string(), + }), apply: () => true, toQuery: () => ({ key: '', values: [] }), }); @@ -760,7 +762,9 @@ describe('NextRouter permissioning', () => { conditions: { rule: 'FAKE_RULE', resourceType: 'catalog-entity', - params: ['user:default/spiderman'], + params: { + foo: 'user:default/spiderman', + }, }, }, ], diff --git a/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts b/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts index 6c05a825ab..8cbeeaebd9 100644 --- a/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts +++ b/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts @@ -39,8 +39,12 @@ describe('PermissionIntegrationClient', () => { const mockConditions: PermissionCriteria = { not: { allOf: [ - { rule: 'RULE_1', resourceType: 'test-resource', params: [] }, - { rule: 'RULE_2', resourceType: 'test-resource', params: ['abc'] }, + { rule: 'RULE_1', resourceType: 'test-resource', params: {} }, + { + rule: 'RULE_2', + resourceType: 'test-resource', + params: { foo: 'abc' }, + }, ], }, }; @@ -280,8 +284,10 @@ describe('PermissionIntegrationClient', () => { name: 'RULE_1', description: 'Test rule 1', resourceType: 'test-resource', - schema: z.tuple([z.enum(['yes', 'no'])]), - apply: (_resource: any, input: 'yes' | 'no') => input === 'yes', + schema: z.object({ + input: z.enum(['yes', 'no']), + }), + apply: (_resource, { input }) => input === 'yes', toQuery: () => { throw new Error('Not implemented'); }, @@ -290,8 +296,11 @@ describe('PermissionIntegrationClient', () => { name: 'RULE_2', description: 'Test rule 2', resourceType: 'test-resource', - schema: z.tuple([z.enum(['yes', 'no'])]), - apply: (_resource: any, input: 'yes' | 'no') => input === 'yes', + + schema: z.object({ + input: z.enum(['yes', 'no']), + }), + apply: (_resource, { input }) => input === 'yes', toQuery: () => { throw new Error('Not implemented'); }, @@ -347,7 +356,9 @@ describe('PermissionIntegrationClient', () => { conditions: { rule: 'RULE_1', resourceType: 'test-resource', - params: ['no'], + params: { + input: 'no', + }, }, }, ]), @@ -368,13 +379,17 @@ describe('PermissionIntegrationClient', () => { { rule: 'RULE_1', resourceType: 'test-resource', - params: ['yes'], + params: { + input: 'yes', + }, }, { not: { rule: 'RULE_2', resourceType: 'test-resource', - params: ['no'], + params: { + input: 'no', + }, }, }, ], @@ -385,12 +400,16 @@ describe('PermissionIntegrationClient', () => { { rule: 'RULE_1', resourceType: 'test-resource', - params: ['no'], + params: { + input: 'no', + }, }, { rule: 'RULE_2', resourceType: 'test-resource', - params: ['yes'], + params: { + input: 'yes', + }, }, ], }, diff --git a/plugins/permission-common/src/PermissionClient.test.ts b/plugins/permission-common/src/PermissionClient.test.ts index 9b2bdcb9ff..bcc6ddd6f6 100644 --- a/plugins/permission-common/src/PermissionClient.test.ts +++ b/plugins/permission-common/src/PermissionClient.test.ts @@ -227,7 +227,7 @@ describe('PermissionClient', () => { conditions: { resourceType: 'test-resource', rule: 'FOO', - params: ['bar'], + params: { foo: 'bar' }, }, }), ); @@ -275,7 +275,7 @@ describe('PermissionClient', () => { conditions: { rule: 'FOO', resourceType: 'test-resource', - params: ['bar'], + params: { foo: 'bar' }, }, }), ); diff --git a/plugins/permission-common/src/PermissionClient.ts b/plugins/permission-common/src/PermissionClient.ts index 9a7bb0bba4..7b429dc46f 100644 --- a/plugins/permission-common/src/PermissionClient.ts +++ b/plugins/permission-common/src/PermissionClient.ts @@ -41,7 +41,7 @@ const permissionCriteriaSchema: z.ZodSchema< .object({ rule: z.string(), resourceType: z.string(), - params: z.array(z.unknown()), + params: z.record(z.unknown()), }) .or(z.object({ anyOf: z.array(permissionCriteriaSchema).nonempty() })) .or(z.object({ allOf: z.array(permissionCriteriaSchema).nonempty() })) diff --git a/plugins/permission-common/src/types/api.ts b/plugins/permission-common/src/types/api.ts index f29a639d18..70b9e902b8 100644 --- a/plugins/permission-common/src/types/api.ts +++ b/plugins/permission-common/src/types/api.ts @@ -101,7 +101,7 @@ export type PolicyDecision = */ export type PermissionCondition< TResourceType extends string = string, - TParams extends unknown[] = unknown[], + TParams extends Record = Record, > = { resourceType: TResourceType; rule: string; diff --git a/plugins/permission-node/src/integration/createConditionExports.test.ts b/plugins/permission-node/src/integration/createConditionExports.test.ts index ed0ab49b5c..a989206fdd 100644 --- a/plugins/permission-node/src/integration/createConditionExports.test.ts +++ b/plugins/permission-node/src/integration/createConditionExports.test.ts @@ -31,25 +31,28 @@ const testIntegration = () => name: 'testRule1', description: 'Test rule 1', resourceType: 'test-resource', - schema: z.tuple([z.string(), z.number()]), - apply: jest.fn( - (_resource: any, _firstParam: string, _secondParam: number) => true, - ), - toQuery: jest.fn((firstParam: string, secondParam: number) => ({ + schema: z.object({ + foo: z.string(), + bar: z.number(), + }), + apply: (_resource: any, _params) => true, + toQuery: params => ({ query: 'testRule1', - params: [firstParam, secondParam], - })), + params, + }), }), testRule2: createPermissionRule({ name: 'testRule2', description: 'Test rule 2', resourceType: 'test-resource', - schema: z.tuple([z.object({})]), - apply: jest.fn((_resource: any, _firstParam: object) => false), - toQuery: jest.fn((firstParam: object) => ({ + schema: z.object({ + foo: z.object({}), + }), + apply: (_resource: any) => false, + toQuery: params => ({ query: 'testRule2', - params: [firstParam], - })), + params, + }), }), }, }); @@ -59,16 +62,26 @@ describe('createConditionExports', () => { it('creates condition factories for the supplied rules', () => { const { conditions } = testIntegration(); - expect(conditions.testRule1('a', 1)).toEqual({ + expect( + conditions.testRule1({ + foo: 'a', + bar: 1, + }), + ).toEqual({ rule: 'testRule1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, }); - expect(conditions.testRule2({ baz: 'quux' })).toEqual({ + expect(conditions.testRule2({ foo: { baz: 'quux' } })).toEqual({ rule: 'testRule2', resourceType: 'test-resource', - params: [{ baz: 'quux' }], + params: { + foo: { baz: 'quux' }, + }, }); }); }); @@ -88,7 +101,10 @@ describe('createConditionExports', () => { { rule: 'testRule1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, }, ], }), @@ -101,7 +117,10 @@ describe('createConditionExports', () => { { rule: 'testRule1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, }, ], }, diff --git a/plugins/permission-node/src/integration/createConditionExports.ts b/plugins/permission-node/src/integration/createConditionExports.ts index ba34c07fed..3921c2dfe3 100644 --- a/plugins/permission-node/src/integration/createConditionExports.ts +++ b/plugins/permission-node/src/integration/createConditionExports.ts @@ -36,7 +36,7 @@ export type Condition = TRule extends PermissionRule< infer TResourceType, infer TParams > - ? (...params: TParams) => PermissionCondition + ? (params: TParams) => PermissionCondition : never; /** diff --git a/plugins/permission-node/src/integration/createConditionFactory.test.ts b/plugins/permission-node/src/integration/createConditionFactory.test.ts index 433d37b3f6..191688f0dd 100644 --- a/plugins/permission-node/src/integration/createConditionFactory.test.ts +++ b/plugins/permission-node/src/integration/createConditionFactory.test.ts @@ -23,9 +23,11 @@ describe('createConditionFactory', () => { name: 'test-rule', description: 'test-description', resourceType: 'test-resource', - schema: z.tuple([]), - apply: jest.fn(), - toQuery: jest.fn(), + schema: z.object({ + foo: z.string(), + }), + apply: (_resource, _params) => true, + toQuery: _params => ({}), }); it('returns a function', () => { @@ -35,10 +37,16 @@ describe('createConditionFactory', () => { describe('return value', () => { it('constructs a condition with the rule name and supplied params', () => { const conditionFactory = createConditionFactory(testRule); - expect(conditionFactory('a', 'b', 1, 2)).toEqual({ + expect( + conditionFactory({ + foo: 'bar', + }), + ).toEqual({ rule: 'test-rule', resourceType: 'test-resource', - params: ['a', 'b', 1, 2], + params: { + foo: 'bar', + }, }); }); }); diff --git a/plugins/permission-node/src/integration/createConditionFactory.ts b/plugins/permission-node/src/integration/createConditionFactory.ts index 15a5b4fd08..5b499ab512 100644 --- a/plugins/permission-node/src/integration/createConditionFactory.ts +++ b/plugins/permission-node/src/integration/createConditionFactory.ts @@ -34,10 +34,10 @@ import { PermissionRule } from '../types'; * @public */ export const createConditionFactory = - ( + >( rule: PermissionRule, ) => - (...params: TParams): PermissionCondition => ({ + (params: TParams): PermissionCondition => ({ rule: rule.name, resourceType: rule.resourceType, params, diff --git a/plugins/permission-node/src/integration/createConditionTransformer.test.ts b/plugins/permission-node/src/integration/createConditionTransformer.test.ts index e7e1e89d06..5021b7766d 100644 --- a/plugins/permission-node/src/integration/createConditionTransformer.test.ts +++ b/plugins/permission-node/src/integration/createConditionTransformer.test.ts @@ -27,22 +27,22 @@ const transformConditions = createConditionTransformer([ name: 'test-rule-1', description: 'Test rule 1', resourceType: 'test-resource', - schema: z.tuple([z.string(), z.number()]), + schema: z.object({ + foo: z.string(), + bar: z.number(), + }), apply: jest.fn(), - toQuery: jest.fn( - (firstParam: string, secondParam: number) => - `test-rule-1:${firstParam}/${secondParam}`, - ), + toQuery: jest.fn(({ foo, bar }) => `test-rule-1:${foo}/${bar}`), }), createPermissionRule({ name: 'test-rule-2', description: 'Test rule 2', resourceType: 'test-resource', - schema: z.tuple([z.object({})]), + schema: z.object({ + foo: z.object({}), + }), apply: jest.fn(), - toQuery: jest.fn( - (firstParam: object) => `test-rule-2:${JSON.stringify(firstParam)}`, - ), + toQuery: jest.fn(({ foo }) => `test-rule-2:${JSON.stringify(foo)}`), }), ]); @@ -55,7 +55,10 @@ describe('createConditionTransformer', () => { conditions: { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['abc', 123], + params: { + foo: 'abc', + bar: 123, + }, }, expectedResult: 'test-rule-1:abc/123', }, @@ -63,7 +66,9 @@ describe('createConditionTransformer', () => { conditions: { rule: 'test-rule-2', resourceType: 'test-resource', - params: [{ foo: 0 }], + params: { + foo: { foo: 0 }, + }, }, expectedResult: 'test-rule-2:{"foo":0}', }, @@ -73,9 +78,18 @@ describe('createConditionTransformer', () => { { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, + }, + { + rule: 'test-rule-2', + resourceType: 'test-resource', + params: { + foo: {}, + }, }, - { rule: 'test-rule-2', resourceType: 'test-resource', params: [{}] }, ], }, expectedResult: { @@ -88,9 +102,18 @@ describe('createConditionTransformer', () => { { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, + }, + { + rule: 'test-rule-2', + resourceType: 'test-resource', + params: { + foo: {}, + }, }, - { rule: 'test-rule-2', resourceType: 'test-resource', params: [{}] }, ], }, expectedResult: { @@ -102,7 +125,9 @@ describe('createConditionTransformer', () => { not: { rule: 'test-rule-2', resourceType: 'test-resource', - params: [{}], + params: { + foo: {}, + }, }, }, expectedResult: { @@ -117,12 +142,17 @@ describe('createConditionTransformer', () => { { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, }, { rule: 'test-rule-2', resourceType: 'test-resource', - params: [{}], + params: { + foo: {}, + }, }, ], }, @@ -132,12 +162,19 @@ describe('createConditionTransformer', () => { { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['b', 2], + params: { + foo: 'b', + bar: 2, + }, }, { rule: 'test-rule-2', resourceType: 'test-resource', - params: [{ c: 3 }], + params: { + foo: { + c: 3, + }, + }, }, ], }, @@ -165,12 +202,19 @@ describe('createConditionTransformer', () => { { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, }, { rule: 'test-rule-2', resourceType: 'test-resource', - params: [{ b: 2 }], + params: { + foo: { + b: 2, + }, + }, }, ], }, @@ -180,13 +224,20 @@ describe('createConditionTransformer', () => { { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['c', 3], + params: { + foo: 'c', + bar: 3, + }, }, { not: { rule: 'test-rule-2', resourceType: 'test-resource', - params: [{ d: 4 }], + params: { + foo: { + d: 4, + }, + }, }, }, ], diff --git a/plugins/permission-node/src/integration/createConditionTransformer.ts b/plugins/permission-node/src/integration/createConditionTransformer.ts index b40063ec13..afbd4a9316 100644 --- a/plugins/permission-node/src/integration/createConditionTransformer.ts +++ b/plugins/permission-node/src/integration/createConditionTransformer.ts @@ -53,7 +53,7 @@ const mapConditions = ( throw new InputError(`Parameters to rule are invalid`, result.error); } - return rule.toQuery(...criteria.params); + 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 7d80459ba3..9c6b345faf 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts @@ -40,21 +40,23 @@ const testRule1 = createPermissionRule({ name: 'test-rule-1', description: 'Test rule 1', resourceType: 'test-resource', - schema: z.tuple([ - z.string().describe('firstParam'), - z.number().describe('secondParam'), - ]), - apply: (_resource: any, _firstParam: string, _secondParam: number) => true, - toQuery: (_firstParam: string, _secondParam: number) => ({}), + schema: z.object({ + foo: z.string(), + bar: z.number().describe('bar'), + }), + apply: (_resource: any, _params) => true, + toQuery: _params => ({}), }); const testRule2 = createPermissionRule({ name: 'test-rule-2', description: 'Test rule 2', resourceType: 'test-resource', - schema: z.tuple([z.object({}).describe('firstParam')]), - apply: (_resource: any, _firstParam: object) => false, - toQuery: (_firstParam: object) => ({}), + schema: z.object({ + foo: z.object({}).describe('foo'), + }), + apply: (_resource: any, _foo) => false, + toQuery: _foo => ({}), }); describe('createPermissionIntegrationRouter', () => { @@ -85,23 +87,35 @@ describe('createPermissionIntegrationRouter', () => { { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['abc', 123], + params: { + foo: 'abc', + bar: 123, + }, }, { anyOf: [ { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, + }, + { + rule: 'test-rule-2', + resourceType: 'test-resource', + params: { foo: {} }, }, - { rule: 'test-rule-2', resourceType: 'test-resource', params: [{}] }, ], }, { not: { rule: 'test-rule-2', resourceType: 'test-resource', - params: [{}], + params: { + foo: {}, + }, }, }, { @@ -111,12 +125,17 @@ describe('createPermissionIntegrationRouter', () => { { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, }, { rule: 'test-rule-2', resourceType: 'test-resource', - params: [{}], + params: { + foo: {}, + }, }, ], }, @@ -126,12 +145,17 @@ describe('createPermissionIntegrationRouter', () => { { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['b', 2], + params: { + foo: 'b', + bar: 2, + }, }, { rule: 'test-rule-2', resourceType: 'test-resource', - params: [{ c: 3 }], + params: { + foo: { c: 3 }, + }, }, ], }, @@ -167,16 +191,25 @@ describe('createPermissionIntegrationRouter', () => { { rule: 'test-rule-2', resourceType: 'test-resource', - params: [{ foo: 0 }], + params: { + foo: { foo: 0 }, + }, }, { allOf: [ { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, + }, + { + rule: 'test-rule-2', + resourceType: 'test-resource', + params: { foo: {} }, }, - { rule: 'test-rule-2', resourceType: 'test-resource', params: [{}] }, ], }, { @@ -186,12 +219,17 @@ describe('createPermissionIntegrationRouter', () => { { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, }, { rule: 'test-rule-2', resourceType: 'test-resource', - params: [{ b: 2 }], + params: { + foo: { b: 2 }, + }, }, ], }, @@ -201,13 +239,18 @@ describe('createPermissionIntegrationRouter', () => { { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['c', 3], + params: { + foo: 'c', + bar: 3, + }, }, { not: { rule: 'test-rule-2', resourceType: 'test-resource', - params: [{ d: 4 }], + params: { + foo: { d: 4 }, + }, }, }, ], @@ -253,7 +296,10 @@ describe('createPermissionIntegrationRouter', () => { conditions: { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, }, }, { @@ -263,7 +309,9 @@ describe('createPermissionIntegrationRouter', () => { conditions: { rule: 'test-rule-2', resourceType: 'test-resource', - params: [{}], + params: { + foo: {}, + }, }, }, { @@ -274,7 +322,10 @@ describe('createPermissionIntegrationRouter', () => { not: { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, }, }, }, @@ -286,7 +337,9 @@ describe('createPermissionIntegrationRouter', () => { not: { rule: 'test-rule-2', resourceType: 'test-resource', - params: [{}], + params: { + foo: {}, + }, }, }, }, @@ -299,12 +352,17 @@ describe('createPermissionIntegrationRouter', () => { { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, }, { rule: 'test-rule-2', resourceType: 'test-resource', - params: [{}], + params: { + foo: {}, + }, }, ], }, @@ -348,7 +406,9 @@ describe('createPermissionIntegrationRouter', () => { conditions: { rule: 'test-rule-1', resourceType: 'test-incorrect-resource-1', - params: [{}], + params: { + foo: {}, + }, }, }, { @@ -358,7 +418,9 @@ describe('createPermissionIntegrationRouter', () => { conditions: { rule: 'test-rule-1', resourceType: 'test-resource', - params: [{}], + params: { + foo: {}, + }, }, }, { @@ -368,7 +430,9 @@ describe('createPermissionIntegrationRouter', () => { conditions: { rule: 'test-rule-1', resourceType: 'test-incorrect-resource-2', - params: [{}], + params: { + foo: {}, + }, }, }, ], @@ -396,7 +460,7 @@ describe('createPermissionIntegrationRouter', () => { conditions: { rule: 'test-rule-1', resourceType: 'test-resource', - params: [], + params: {}, }, }, ], @@ -433,7 +497,10 @@ describe('createPermissionIntegrationRouter', () => { conditions: { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, }, }, { @@ -443,7 +510,10 @@ describe('createPermissionIntegrationRouter', () => { conditions: { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, }, }, { @@ -453,7 +523,10 @@ describe('createPermissionIntegrationRouter', () => { conditions: { rule: 'test-rule-1', resourceType: 'test-resource', - params: ['a', 1], + params: { + foo: 'a', + bar: 1, + }, }, }, ], @@ -532,21 +605,20 @@ describe('createPermissionIntegrationRouter', () => { resourceType: testRule1.resourceType, schema: { $schema: 'http://json-schema.org/draft-07/schema#', - items: [ - { - description: 'firstParam', + additionalProperties: false, + properties: { + foo: { type: 'string', }, - { - description: 'secondParam', + bar: { + description: 'bar', type: 'number', }, - ], - maxItems: 2, - minItems: 2, - type: 'array', + }, + required: ['foo', 'bar'], + type: 'object', }, - parameters: { count: 2 }, + parameters: { count: 1 }, }, { name: testRule2.name, @@ -554,17 +626,17 @@ describe('createPermissionIntegrationRouter', () => { resourceType: testRule2.resourceType, schema: { $schema: 'http://json-schema.org/draft-07/schema#', - items: [ - { + additionalProperties: false, + properties: { + foo: { additionalProperties: false, - description: 'firstParam', + description: 'foo', properties: {}, type: 'object', }, - ], - maxItems: 1, - minItems: 1, - type: 'array', + }, + required: ['foo'], + type: 'object', }, parameters: { count: 1 }, }, diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index 28268b7f98..c5843f6a84 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -46,7 +46,7 @@ const permissionCriteriaSchema: z.ZodSchema< z.object({ rule: z.string(), resourceType: z.string(), - params: z.array(z.unknown()), + params: z.record(z.unknown()), }), ]), ); @@ -132,7 +132,7 @@ const applyConditions = ( throw new InputError(`Parameters to rule are invalid`, result.error); } - return rule.apply(resource, ...criteria.params); + return rule.apply(resource, criteria.params); }; /** diff --git a/plugins/permission-node/src/integration/createPermissionRule.ts b/plugins/permission-node/src/integration/createPermissionRule.ts index 6673fe76de..ef84a5a26f 100644 --- a/plugins/permission-node/src/integration/createPermissionRule.ts +++ b/plugins/permission-node/src/integration/createPermissionRule.ts @@ -25,7 +25,7 @@ export const createPermissionRule = < TResource, TQuery, TResourceType extends string, - TParams extends unknown[], + TParams extends Record, >( rule: PermissionRule, ) => rule; @@ -40,7 +40,7 @@ export const createPermissionRule = < */ export const makeCreatePermissionRule = () => - ( + >( rule: PermissionRule, ) => createPermissionRule(rule); diff --git a/plugins/permission-node/src/integration/util.test.ts b/plugins/permission-node/src/integration/util.test.ts index 1572964a7d..924ab30757 100644 --- a/plugins/permission-node/src/integration/util.test.ts +++ b/plugins/permission-node/src/integration/util.test.ts @@ -31,7 +31,7 @@ describe('permission integration utils', () => { name: 'test-rule-1', description: 'Test rule 1', resourceType: 'test-resource', - schema: z.tuple([]), + schema: z.object({}), apply: jest.fn(), toQuery: jest.fn(), }); @@ -40,7 +40,7 @@ describe('permission integration utils', () => { name: 'test-rule-2', description: 'Test rule 2', resourceType: 'test-resource', - schema: z.tuple([]), + schema: z.object({}), apply: jest.fn(), toQuery: jest.fn(), }); diff --git a/plugins/permission-node/src/types.ts b/plugins/permission-node/src/types.ts index 7beb5bb92d..02ebe07ac4 100644 --- a/plugins/permission-node/src/types.ts +++ b/plugins/permission-node/src/types.ts @@ -37,7 +37,7 @@ export type PermissionRule< TResource, TQuery, TResourceType extends string, - TParams extends unknown[] = unknown[], + TParams extends Record = Record, > = { name: string; description: string; @@ -46,19 +46,23 @@ export type PermissionRule< /** * A ZodSchema that documents the parameters that this rule accepts. */ - schema: z.ZodSchema; + schema: z.ZodObject<{ + [P in keyof TParams]-?: TParams[P] extends undefined + ? z.ZodOptionalType> + : z.ZodType; + }>; /** * Apply this rule to a resource already loaded from a backing data source. The params are * arguments supplied for the rule; for example, a rule could be `isOwner` with entityRefs as the * params. */ - apply(resource: TResource, ...params: TParams): boolean; + apply(resource: TResource, params: TParams): boolean; /** * Translate this rule to criteria suitable for use in querying a backing data store. The criteria * can be used for loading a collection of resources efficiently with conditional criteria already * applied. */ - toQuery(...params: TParams): PermissionCriteria; + toQuery(params: TParams): PermissionCriteria; }; diff --git a/plugins/playlist-backend/src/permissions/DefaultPlaylistPermissionPolicy.test.ts b/plugins/playlist-backend/src/permissions/DefaultPlaylistPermissionPolicy.test.ts index 179b0d7f9b..3d761bfd35 100644 --- a/plugins/playlist-backend/src/permissions/DefaultPlaylistPermissionPolicy.test.ts +++ b/plugins/playlist-backend/src/permissions/DefaultPlaylistPermissionPolicy.test.ts @@ -67,11 +67,10 @@ describe('DefaultPlaylistPermissionPolicy', () => { resourceType: PLAYLIST_LIST_RESOURCE_TYPE, conditions: { anyOf: [ - playlistConditions.isOwner([ - 'user:default/me', - 'group:default/owner', - ]), - playlistConditions.isPublic(), + playlistConditions.isOwner({ + owners: ['user:default/me', 'group:default/owner'], + }), + playlistConditions.isPublic({}), ], }, }); @@ -89,11 +88,10 @@ describe('DefaultPlaylistPermissionPolicy', () => { resourceType: PLAYLIST_LIST_RESOURCE_TYPE, conditions: { anyOf: [ - playlistConditions.isOwner([ - 'user:default/me', - 'group:default/owner', - ]), - playlistConditions.isPublic(), + playlistConditions.isOwner({ + owners: ['user:default/me', 'group:default/owner'], + }), + playlistConditions.isPublic({}), ], }, }); @@ -109,10 +107,9 @@ describe('DefaultPlaylistPermissionPolicy', () => { result: AuthorizeResult.CONDITIONAL, pluginId: 'playlist', resourceType: PLAYLIST_LIST_RESOURCE_TYPE, - conditions: playlistConditions.isOwner([ - 'user:default/me', - 'group:default/owner', - ]), + conditions: playlistConditions.isOwner({ + owners: ['user:default/me', 'group:default/owner'], + }), }); }); @@ -126,10 +123,9 @@ describe('DefaultPlaylistPermissionPolicy', () => { result: AuthorizeResult.CONDITIONAL, pluginId: 'playlist', resourceType: PLAYLIST_LIST_RESOURCE_TYPE, - conditions: playlistConditions.isOwner([ - 'user:default/me', - 'group:default/owner', - ]), + conditions: playlistConditions.isOwner({ + owners: ['user:default/me', 'group:default/owner'], + }), }); }); }); diff --git a/plugins/playlist-backend/src/permissions/DefaultPlaylistPermissionPolicy.ts b/plugins/playlist-backend/src/permissions/DefaultPlaylistPermissionPolicy.ts index 86d103e789..317f5da9c8 100644 --- a/plugins/playlist-backend/src/permissions/DefaultPlaylistPermissionPolicy.ts +++ b/plugins/playlist-backend/src/permissions/DefaultPlaylistPermissionPolicy.ts @@ -68,8 +68,10 @@ export class DefaultPlaylistPermissionPolicy implements PermissionPolicy { ) { return createPlaylistConditionalDecision(request.permission, { anyOf: [ - playlistConditions.isOwner(user?.identity.ownershipEntityRefs ?? []), - playlistConditions.isPublic(), + playlistConditions.isOwner({ + owners: user?.identity.ownershipEntityRefs ?? [], + }), + playlistConditions.isPublic({}), ], }); } @@ -81,7 +83,9 @@ export class DefaultPlaylistPermissionPolicy implements PermissionPolicy { ) { return createPlaylistConditionalDecision( request.permission, - playlistConditions.isOwner(user?.identity.ownershipEntityRefs ?? []), + playlistConditions.isOwner({ + owners: user?.identity.ownershipEntityRefs ?? [], + }), ); } diff --git a/plugins/playlist-backend/src/permissions/rules.ts b/plugins/playlist-backend/src/permissions/rules.ts index de2d3df5e4..897d2e19eb 100644 --- a/plugins/playlist-backend/src/permissions/rules.ts +++ b/plugins/playlist-backend/src/permissions/rules.ts @@ -29,16 +29,19 @@ const createPlaylistPermissionRule = makeCreatePermissionRule< typeof PLAYLIST_LIST_RESOURCE_TYPE >(); -const isOwner = createPlaylistPermissionRule({ +const isOwner = createPlaylistPermissionRule<{ + owners: string[]; +}>({ name: 'IS_OWNER', description: 'Should allow only if the playlist belongs to the user', resourceType: PLAYLIST_LIST_RESOURCE_TYPE, - schema: z.tuple([z.array(z.string().describe('List of owners'))]), - apply: (list: PlaylistMetadata, userOwnershipRefs: string[]) => - userOwnershipRefs.includes(list.owner), - toQuery: (userOwnershipRefs: string[]) => ({ + schema: z.object({ + owners: z.array(z.string().describe('List of owner entity refs')), + }), + apply: (list: PlaylistMetadata, { owners }) => owners.includes(list.owner), + toQuery: ({ owners }) => ({ key: 'owner', - values: userOwnershipRefs, + values: owners, }), }); @@ -46,7 +49,7 @@ const isPublic = createPlaylistPermissionRule({ name: 'IS_PUBLIC', description: 'Should allow only if the playlist is public', resourceType: PLAYLIST_LIST_RESOURCE_TYPE, - schema: z.tuple([]), + schema: z.object({}), apply: (list: PlaylistMetadata) => list.public, toQuery: () => ({ key: 'public', values: [true] }), });