diff --git a/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts b/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts index 4be1b55379..9981496658 100644 --- a/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts +++ b/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts @@ -20,7 +20,11 @@ import express, { Router, RequestHandler } from 'express'; import { RestContext, rest } from 'msw'; import { setupServer, SetupServerApi } from 'msw/node'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; -import { AuthorizeResult } from '@backstage/plugin-permission-common'; +import { + AuthorizeResult, + PermissionCondition, + PermissionCriteria, +} from '@backstage/plugin-permission-common'; import { createPermissionIntegrationRouter } from '@backstage/plugin-permission-node'; import { PermissionIntegrationClient } from './PermissionIntegrationClient'; @@ -28,7 +32,7 @@ describe('PermissionIntegrationClient', () => { describe('applyConditions', () => { let server: SetupServerApi; - const mockConditions = { + const mockConditions: PermissionCriteria = { not: { allOf: [ { rule: 'RULE_1', params: [] }, diff --git a/plugins/permission-common/src/PermissionClient.ts b/plugins/permission-common/src/PermissionClient.ts index 58f4d44d20..45224afc91 100644 --- a/plugins/permission-common/src/PermissionClient.ts +++ b/plugins/permission-common/src/PermissionClient.ts @@ -43,8 +43,8 @@ const permissionCriteriaSchema: z.ZodSchema< rule: z.string(), params: z.array(z.unknown()), }) - .or(z.object({ anyOf: z.array(permissionCriteriaSchema) })) - .or(z.object({ allOf: z.array(permissionCriteriaSchema) })) + .or(z.object({ anyOf: z.array(permissionCriteriaSchema).nonempty() })) + .or(z.object({ allOf: z.array(permissionCriteriaSchema).nonempty() })) .or(z.object({ not: permissionCriteriaSchema })), ); diff --git a/plugins/permission-common/src/types/api.ts b/plugins/permission-common/src/types/api.ts index 287c7ffe83..cf1cdd403e 100644 --- a/plugins/permission-common/src/types/api.ts +++ b/plugins/permission-common/src/types/api.ts @@ -72,14 +72,33 @@ export type PermissionCondition = { params: TParams; }; +/** + * Utility type to represent an array with 1 or more elements. + * + * @private + */ +export type NonEmptyArray = [T, ...T[]]; + +export type AllOfCriteria = { + allOf: NonEmptyArray>; +}; + +export type AnyOfCriteria = { + anyOf: NonEmptyArray>; +}; + +export type NotCriteria = { + not: PermissionCriteria; +}; + /** * Composes several {@link PermissionCondition}s as criteria with a nested AND/OR structure. * @public */ export type PermissionCriteria = - | { allOf: PermissionCriteria[] } - | { anyOf: PermissionCriteria[] } - | { not: PermissionCriteria } + | AllOfCriteria + | AnyOfCriteria + | NotCriteria | TQuery; /** diff --git a/plugins/permission-common/src/types/index.ts b/plugins/permission-common/src/types/index.ts index b4453fcc63..fa98de73f1 100644 --- a/plugins/permission-common/src/types/index.ts +++ b/plugins/permission-common/src/types/index.ts @@ -23,6 +23,9 @@ export type { Identified, PermissionCondition, PermissionCriteria, + AllOfCriteria, + AnyOfCriteria, + NotCriteria, } from './api'; export type { DiscoveryApi } from './discovery'; export type { diff --git a/plugins/permission-node/src/integration/createConditionTransformer.ts b/plugins/permission-node/src/integration/createConditionTransformer.ts index 0e736b832c..94e7a3f76e 100644 --- a/plugins/permission-node/src/integration/createConditionTransformer.ts +++ b/plugins/permission-node/src/integration/createConditionTransformer.ts @@ -25,17 +25,31 @@ import { isOrCriteria, } from './util'; +const nonEmpty = (list: T[]): [T, ...T[]] => { + if (list.length === 0) { + throw new Error( + 'Invalid conditions provided. Expected at least one item for `allOf`/`anyOf` criteria but received none.', + ); + } + + return list as [T, ...T[]]; +}; + const mapConditions = ( criteria: PermissionCriteria, getRule: (name: string) => PermissionRule, ): PermissionCriteria => { if (isAndCriteria(criteria)) { return { - allOf: criteria.allOf.map(child => mapConditions(child, getRule)), + allOf: nonEmpty( + criteria.allOf.map(child => mapConditions(child, getRule)), + ), }; } else if (isOrCriteria(criteria)) { return { - anyOf: criteria.anyOf.map(child => mapConditions(child, getRule)), + anyOf: nonEmpty( + criteria.anyOf.map(child => mapConditions(child, getRule)), + ), }; } else if (isNotCriteria(criteria)) { return { diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts index 065c30a978..6408de46a1 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts @@ -247,7 +247,8 @@ describe('createPermissionIntegrationRouter', () => { resourceRef: 'default:test/resource-1', resourceType: 'test-incorrect-resource-1', conditions: { - anyOf: [], + rule: 'test-rule-1', + params: [{}], }, }, { @@ -255,7 +256,8 @@ describe('createPermissionIntegrationRouter', () => { resourceRef: 'default:test/resource-2', resourceType: 'test-resource', conditions: { - anyOf: [], + rule: 'test-rule-1', + params: [{}], }, }, { @@ -263,7 +265,8 @@ describe('createPermissionIntegrationRouter', () => { resourceRef: 'default:test/resource-3', resourceType: 'test-incorrect-resource-2', conditions: { - anyOf: [], + rule: 'test-rule-1', + params: [{}], }, }, ], diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index 1b01660fb0..20c5cee20a 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -38,8 +38,8 @@ const permissionCriteriaSchema: z.ZodSchema< PermissionCriteria > = z.lazy(() => z.union([ - z.object({ anyOf: z.array(permissionCriteriaSchema) }), - z.object({ allOf: z.array(permissionCriteriaSchema) }), + z.object({ anyOf: z.array(permissionCriteriaSchema).nonempty() }), + z.object({ allOf: z.array(permissionCriteriaSchema).nonempty() }), z.object({ not: permissionCriteriaSchema }), z.object({ rule: z.string(), diff --git a/plugins/permission-node/src/integration/util.ts b/plugins/permission-node/src/integration/util.ts index d9457cfefc..3878e18895 100644 --- a/plugins/permission-node/src/integration/util.ts +++ b/plugins/permission-node/src/integration/util.ts @@ -14,22 +14,27 @@ * limitations under the License. */ -import { PermissionCriteria } from '@backstage/plugin-permission-common'; +import { + AllOfCriteria, + AnyOfCriteria, + NotCriteria, + PermissionCriteria, +} from '@backstage/plugin-permission-common'; import { PermissionRule } from '../types'; export const isAndCriteria = ( filter: PermissionCriteria, -): filter is { allOf: PermissionCriteria[] } => +): filter is AllOfCriteria => Object.prototype.hasOwnProperty.call(filter, 'allOf'); export const isOrCriteria = ( filter: PermissionCriteria, -): filter is { anyOf: PermissionCriteria[] } => +): filter is AnyOfCriteria => Object.prototype.hasOwnProperty.call(filter, 'anyOf'); export const isNotCriteria = ( filter: PermissionCriteria, -): filter is { not: PermissionCriteria } => +): filter is NotCriteria => Object.prototype.hasOwnProperty.call(filter, 'not'); export const createGetRule = (