From d524bf467b0b103006d273a12c3d122a997eaa31 Mon Sep 17 00:00:00 2001 From: Ainhoa Larumbe Date: Tue, 21 Mar 2023 10:14:01 +0000 Subject: [PATCH 01/11] WIP createPermissionIntegrationRouter takes an array of ResourceOptions Signed-off-by: Ainhoa Larumbe --- .../createPermissionIntegrationRouter.ts | 187 +++++++++++++++--- 1 file changed, 165 insertions(+), 22 deletions(-) diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index 850123d55e..adb323c557 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -65,6 +65,27 @@ const applyConditionsRequestSchema = z.object({ ), }); +/* + +BEFORE: +[ + { resourceType: 'A', resourceRef: 'ref1', conditions: ... }, + { resourceType: 'A', resourceRef: 'ref2', conditions: ... } +] + +await getResources(['ref1', 'ref2' ]) + +NOW: +[ + { resourceType: 'A', resourceRef: 'ref1', conditions: ... }, + { resourceType: 'A', resourceRef: 'ref2', conditions: ... } + { resourceType: 'B', resourceRef: 'ref3', conditions: ... } +] + +await getResourcesA(['ref1', 'ref2' ]) +await getResourcesB(['ref3' ]) +*/ + /** * A request to load the referenced resource and apply conditions in order to * finalize a conditional authorization response. @@ -265,6 +286,56 @@ export function createPermissionIntegrationRouter(options: { permissions: Array; }): express.Router; +/** + * + * @public + */ +export function createPermissionIntegrationRouter< + TResourceType1 extends string, + TResource1, + TResourceType2 extends string, + TResource2, +>( + options: [ + CreatePermissionIntegrationRouterResourceOptions< + TResourceType1, + TResource1 + >, + CreatePermissionIntegrationRouterResourceOptions< + TResourceType2, + TResource2 + >, + ], +): express.Router; + +/** + * + * @public + */ +export function createPermissionIntegrationRouter< + TResourceType1 extends string, + TResource1, + TResourceType2 extends string, + TResource2, + TResourceType3 extends string, + TResource3, +>( + options: [ + CreatePermissionIntegrationRouterResourceOptions< + TResourceType1, + TResource1 + >, + CreatePermissionIntegrationRouterResourceOptions< + TResourceType2, + TResource2 + >, + CreatePermissionIntegrationRouterResourceOptions< + TResourceType3, + TResource3 + >, + ], +): express.Router; + /** * @public */ @@ -274,17 +345,33 @@ export function createPermissionIntegrationRouter< >( options: | { permissions: Array } - | CreatePermissionIntegrationRouterResourceOptions< - TResourceType, - TResource + | CreatePermissionIntegrationRouterResourceOptions + | Array< + CreatePermissionIntegrationRouterResourceOptions< + TResourceType, + TResource + > >, ): express.Router { + const allOptions = [options].flat(); + const allRules = allOptions.flatMap( + option => + ( + option as CreatePermissionIntegrationRouterResourceOptions< + TResourceType, + TResource + > + ).rules || [], + ); + const allPermissions = allOptions + .flatMap(option => option.permissions) + .filter((p): p is Permission => !!p); + const router = Router(); router.use(express.json()); - const { permissions = [], rules = [] } = { rules: [], ...options }; router.get('/.well-known/backstage/permissions/metadata', (_, res) => { - const serializedRules: MetadataResponseSerializedRule[] = rules.map( + const serializedRules: MetadataResponseSerializedRule[] = allRules.map( rule => ({ name: rule.name, description: rule.description, @@ -294,7 +381,7 @@ export function createPermissionIntegrationRouter< ); const responseJson: MetadataResponse = { - permissions, + permissions: allPermissions, rules: serializedRules, }; @@ -304,17 +391,40 @@ export function createPermissionIntegrationRouter< router.post( '/.well-known/backstage/permissions/apply-conditions', async (req, res: Response) => { - if ( - !isCreatePermissionIntegrationRouterResourceOptions(options) || - options.getResources === undefined - ) { - throw new NotImplementedError( - `This plugin does not expose any permission rule or can't evaluate conditional decisions`, - ); - } - const { resourceType, getResources } = options; + // if ( + // !isCreatePermissionIntegrationRouterResourceOptions(options) || + // options.getResources === undefined + // ) { + // throw new NotImplementedError( + // `This plugin does not expose any permission rule or can't evaluate conditional decisions`, + // ); + // } - const getRule = createGetRule(rules); + const ruleMapByResourceType: Record< + string, + ReturnType + > = {}; + const getResourcesByResourceType: Record< + string, + CreatePermissionIntegrationRouterResourceOptions< + TResourceType, + TResource + >['getResources'] + > = {}; + + for (const option of allOptions) { + if (isCreatePermissionIntegrationRouterResourceOptions(option)) { + ruleMapByResourceType[option.resourceType] = createGetRule( + option.rules, + ); + + getResourcesByResourceType[option.resourceType] = option.getResources; + } + } + + // const { resourceType, getResources } = options; + + // const getRule = createGetRule(rules); const assertValidResourceTypes = ( requests: ApplyConditionsRequestEntry[], @@ -340,15 +450,45 @@ export function createPermissionIntegrationRouter< assertValidResourceTypes(body.items); - const resourceRefs = Array.from( - new Set(body.items.map(({ resourceRef }) => resourceRef)), - ); + const resourceRefsByResourceType = body.items.reduce< + Record> + >((acc, item) => { + if (!acc[item.resourceType]) { + acc[item.resourceType] = new Set(); + } + acc[item.resourceType].add(item.resourceRef); + return acc; + }, {}); + + const resourcesByResourceType: Record> = {}; + Object.keys(resourceRefsByResourceType).forEach(async resourceType => { + if ( + !getResourcesByResourceType || + !getResourcesByResourceType[resourceType] + ) { + throw new NotImplementedError( + `This plugin does not expose any permission rule or can't evaluate the conditions request for ${resourceType}`, + ); + } + const resourceRefs = Array.from( + resourceRefsByResourceType[resourceType], + ); + const resources = await getResourcesByResourceType[resourceType]( + resourceRefs, + ); + resourceRefs.forEach((resourceRef, index) => { + resourcesByResourceType[resourceType][resourceRef] = resources[index]; + }); + }); + + /* const resourceArray = await getResources(resourceRefs); const resources = resourceRefs.reduce((acc, resourceRef, index) => { acc[resourceRef] = resourceArray[index]; return acc; }, {} as Record); +*/ return res.json({ items: body.items.map(request => ({ @@ -376,9 +516,12 @@ function isCreatePermissionIntegrationRouterResourceOptions< >( options: | { permissions: Array } - | CreatePermissionIntegrationRouterResourceOptions< - TResourceType, - TResource + | CreatePermissionIntegrationRouterResourceOptions + | Array< + CreatePermissionIntegrationRouterResourceOptions< + TResourceType, + TResource + > >, ): options is CreatePermissionIntegrationRouterResourceOptions< TResourceType, From 16c725e9396bed6ef2f511f75371ac6207c19fbb Mon Sep 17 00:00:00 2001 From: Ainhoa Larumbe Date: Wed, 22 Mar 2023 15:26:22 +0000 Subject: [PATCH 02/11] Complete code in router and fix tests Signed-off-by: Ainhoa Larumbe Co-authored-by: Vincenzo Scamporlino --- .../createPermissionIntegrationRouter.test.ts | 122 +++++++++++++++++- .../createPermissionIntegrationRouter.ts | 67 +++------- 2 files changed, 138 insertions(+), 51 deletions(-) diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts index 6e28c5ddc4..759c8a8e81 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts @@ -34,6 +34,11 @@ const testPermission: Permission = createPermission({ attributes: {}, }); +const testPermission2: Permission = createPermission({ + name: 'test.permission2', + attributes: {}, +}); + const mockTestRule1Apply = jest .fn() .mockImplementation((_resource: any, _params) => true); @@ -60,6 +65,17 @@ const testRule2 = createPermissionRule({ toQuery: () => ({}), }); +const mockTestRule3Apply = jest + .fn() + .mockImplementation((_resource: any) => false); +const testRule3 = createPermissionRule({ + name: 'test-rule-3', + description: 'Test rule 3', + resourceType: 'test-resource-2', + apply: mockTestRule2Apply, + toQuery: () => ({}), +}); + const defaultMockedGetResources: CreatePermissionIntegrationRouterResourceOptions< string, { id: string } @@ -69,8 +85,7 @@ const defaultMockedGetResources: CreatePermissionIntegrationRouterResourceOption const createApp = ( mockedGetResources: - | typeof defaultMockedGetResources - | null = defaultMockedGetResources, + | typeof defaultMockedGetResources = defaultMockedGetResources, ) => { const router = mockedGetResources ? createPermissionIntegrationRouter({ @@ -84,6 +99,16 @@ const createApp = ( return express().use(router); }; +const createAppWithResources = ( + resourceOptions: CreatePermissionIntegrationRouterResourceOptions< + string, + any + >, +) => { + const router = createPermissionIntegrationRouter(resourceOptions); + return express().use(router); +}; + describe('createPermissionIntegrationRouter', () => { afterEach(() => { jest.clearAllMocks(); @@ -573,15 +598,35 @@ describe('createPermissionIntegrationRouter', () => { }); it('returns 501 with no getResources implementation', async () => { - const response = await request(createApp(null)) + const response = await request( + createAppWithResources({ + resourceType: 'test-resource', + permissions: [testPermission], + rules: [testRule1, testRule2], + }), + ) .post('/.well-known/backstage/permissions/apply-conditions') .send({ - items: [], + items: [ + { + id: '345', + resourceRef: 'default:test/resource-2', + resourceType: 'test-resource', + conditions: { + rule: 'test-rule-1', + resourceType: 'test-resource', + params: { + foo: 'a', + bar: 1, + }, + }, + }, + ], }); expect(response.status).toEqual(501); expect(response.body.error.message).toEqual( - `This plugin does not expose any permission rule or can't evaluate conditional decisions`, + `This plugin does not expose any permission rule or can't evaluate the conditions request for test-resource`, ); }); }); @@ -630,6 +675,73 @@ describe('createPermissionIntegrationRouter', () => { ], }); }); + it.skip('returns a list of permissions and rules used by a given backend that was created with an array of resource options', async () => { + const mockedResourceOptions = [ + { + resourceType: 'test-resource', + permissions: [testPermission], + rules: [testRule1, testRule2], + }, + { + resourceType: 'test-resource-2', + permissions: [testPermission2], + rules: [testRule3], + }, + ]; + + const response = await request( + createApp({ resourceOptions: mockedResourceOptions }), + ).get('/.well-known/backstage/permissions/metadata'); + + expect(response.status).toEqual(200); + expect(response.body).toEqual({ + permissions: [testPermission, testPermission2], + rules: [ + { + name: testRule1.name, + description: testRule1.description, + resourceType: testRule1.resourceType, + paramsSchema: { + $schema: 'http://json-schema.org/draft-07/schema#', + additionalProperties: false, + properties: { + foo: { + type: 'string', + }, + bar: { + description: 'bar', + type: 'number', + }, + }, + required: ['foo', 'bar'], + type: 'object', + }, + }, + { + name: testRule2.name, + description: testRule2.description, + resourceType: testRule2.resourceType, + paramsSchema: { + $schema: 'http://json-schema.org/draft-07/schema#', + additionalProperties: false, + properties: {}, + type: 'object', + }, + }, + { + name: testRule3.name, + description: testRule3.description, + resourceType: testRule3.resourceType, + paramsSchema: { + $schema: 'http://json-schema.org/draft-07/schema#', + additionalProperties: false, + properties: {}, + type: 'object', + }, + }, + ], + }); + }); }); }); diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index adb323c557..0a66e2d3f2 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -346,12 +346,7 @@ export function createPermissionIntegrationRouter< options: | { permissions: Array } | CreatePermissionIntegrationRouterResourceOptions - | Array< - CreatePermissionIntegrationRouterResourceOptions< - TResourceType, - TResource - > - >, + | Array>, ): express.Router { const allOptions = [options].flat(); const allRules = allOptions.flatMap( @@ -366,6 +361,12 @@ export function createPermissionIntegrationRouter< const allPermissions = allOptions .flatMap(option => option.permissions) .filter((p): p is Permission => !!p); + const allResourceTypes = allOptions.reduce((acc, option) => { + if (isCreatePermissionIntegrationRouterResourceOptions(option)) { + acc.push(option.resourceType); + } + return acc; + }, [] as string[]); const router = Router(); router.use(express.json()); @@ -391,15 +392,6 @@ export function createPermissionIntegrationRouter< router.post( '/.well-known/backstage/permissions/apply-conditions', async (req, res: Response) => { - // if ( - // !isCreatePermissionIntegrationRouterResourceOptions(options) || - // options.getResources === undefined - // ) { - // throw new NotImplementedError( - // `This plugin does not expose any permission rule or can't evaluate conditional decisions`, - // ); - // } - const ruleMapByResourceType: Record< string, ReturnType @@ -422,15 +414,11 @@ export function createPermissionIntegrationRouter< } } - // const { resourceType, getResources } = options; - - // const getRule = createGetRule(rules); - const assertValidResourceTypes = ( requests: ApplyConditionsRequestEntry[], ) => { const invalidResourceTypes = requests - .filter(request => request.resourceType !== resourceType) + .filter(request => !allResourceTypes.includes(request.resourceType)) .map(request => request.resourceType); if (invalidResourceTypes.length) { @@ -461,11 +449,9 @@ export function createPermissionIntegrationRouter< }, {}); const resourcesByResourceType: Record> = {}; - Object.keys(resourceRefsByResourceType).forEach(async resourceType => { - if ( - !getResourcesByResourceType || - !getResourcesByResourceType[resourceType] - ) { + for (const resourceType of Object.keys(resourceRefsByResourceType)) { + const getResources = getResourcesByResourceType[resourceType]; + if (!getResources) { throw new NotImplementedError( `This plugin does not expose any permission rule or can't evaluate the conditions request for ${resourceType}`, ); @@ -473,30 +459,22 @@ export function createPermissionIntegrationRouter< const resourceRefs = Array.from( resourceRefsByResourceType[resourceType], ); - const resources = await getResourcesByResourceType[resourceType]( - resourceRefs, - ); + const resources = await getResources(resourceRefs); resourceRefs.forEach((resourceRef, index) => { + if (!resourcesByResourceType[resourceType]) { + resourcesByResourceType[resourceType] = {}; + } resourcesByResourceType[resourceType][resourceRef] = resources[index]; }); - }); - - /* - const resourceArray = await getResources(resourceRefs); - const resources = resourceRefs.reduce((acc, resourceRef, index) => { - acc[resourceRef] = resourceArray[index]; - - return acc; - }, {} as Record); -*/ + } return res.json({ items: body.items.map(request => ({ id: request.id, result: applyConditions( request.conditions, - resources[request.resourceRef], - getRule, + resourcesByResourceType[request.resourceType][request.resourceRef], + ruleMapByResourceType[request.resourceType], ) ? AuthorizeResult.ALLOW : AuthorizeResult.DENY, @@ -516,12 +494,9 @@ function isCreatePermissionIntegrationRouterResourceOptions< >( options: | { permissions: Array } - | CreatePermissionIntegrationRouterResourceOptions - | Array< - CreatePermissionIntegrationRouterResourceOptions< - TResourceType, - TResource - > + | CreatePermissionIntegrationRouterResourceOptions< + TResourceType, + TResource >, ): options is CreatePermissionIntegrationRouterResourceOptions< TResourceType, From 19eefbd0f48677aaa9195f951a0b5a64a7914d5f Mon Sep 17 00:00:00 2001 From: Ainhoa Larumbe Date: Thu, 23 Mar 2023 10:41:57 +0000 Subject: [PATCH 03/11] Add tests for router with multiple resource types Signed-off-by: Ainhoa Larumbe Co-authored-by: Vincenzo Scamporlino --- .../createPermissionIntegrationRouter.test.ts | 200 +++++++++++++++--- 1 file changed, 174 insertions(+), 26 deletions(-) diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts index 759c8a8e81..a54080af2d 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts @@ -69,23 +69,46 @@ const mockTestRule3Apply = jest .fn() .mockImplementation((_resource: any) => false); const testRule3 = createPermissionRule({ - name: 'test-rule-3', + // simulating a clash of name with test-rule-1 rule of test-resource + name: 'test-rule-1', description: 'Test rule 3', resourceType: 'test-resource-2', - apply: mockTestRule2Apply, + apply: mockTestRule3Apply, toQuery: () => ({}), }); -const defaultMockedGetResources: CreatePermissionIntegrationRouterResourceOptions< +const defaultMockedGetResources1: CreatePermissionIntegrationRouterResourceOptions< string, { id: string } >['getResources'] = jest.fn(async resourceRefs => resourceRefs.map(resourceRef => ({ id: resourceRef })), ); +const defaultMockedGetResources2: CreatePermissionIntegrationRouterResourceOptions< + string, + { id: string } +>['getResources'] = jest.fn(async resourceRefs => + resourceRefs.map(resourceRef => ({ id: resourceRef })), +); + +const mockedResourceOptions = [ + { + resourceType: 'test-resource', + permissions: [testPermission], + getResources: defaultMockedGetResources1, + rules: [testRule1, testRule2], + }, + { + resourceType: 'test-resource-2', + permissions: [testPermission2], + getResources: defaultMockedGetResources2, + rules: [testRule3], + }, +]; + const createApp = ( mockedGetResources: - | typeof defaultMockedGetResources = defaultMockedGetResources, + | typeof defaultMockedGetResources1 = defaultMockedGetResources1, ) => { const router = mockedGetResources ? createPermissionIntegrationRouter({ @@ -100,12 +123,13 @@ const createApp = ( }; const createAppWithResources = ( - resourceOptions: CreatePermissionIntegrationRouterResourceOptions< - string, - any - >, + resourceOptions: + | CreatePermissionIntegrationRouterResourceOptions + | CreatePermissionIntegrationRouterResourceOptions[], ) => { - const router = createPermissionIntegrationRouter(resourceOptions); + const router = createPermissionIntegrationRouter( + resourceOptions as Parameters[0], + ); return express().use(router); }; @@ -185,7 +209,7 @@ describe('createPermissionIntegrationRouter', () => { ], }, ])('returns 200/ALLOW when criteria match (case %#)', async conditions => { - const response = await request(createApp()) + let response = await request(createApp()) .post('/.well-known/backstage/permissions/apply-conditions') .send({ items: [ @@ -207,6 +231,37 @@ describe('createPermissionIntegrationRouter', () => { }, ], }); + + expect(defaultMockedGetResources1).toHaveBeenCalled(); + expect(mockTestRule3Apply).not.toHaveBeenCalled(); + + (defaultMockedGetResources1 as jest.Mock).mockClear(); + + response = await request(createAppWithResources(mockedResourceOptions)) + .post('/.well-known/backstage/permissions/apply-conditions') + .send({ + items: [ + { + id: '123', + resourceRef: 'default:test/resource', + resourceType: 'test-resource', + conditions, + }, + ], + }); + + expect(response.status).toEqual(200); + expect(response.body).toEqual({ + items: [ + { + id: '123', + result: AuthorizeResult.ALLOW, + }, + ], + }); + expect(defaultMockedGetResources1).toHaveBeenCalled(); + expect(defaultMockedGetResources2).not.toHaveBeenCalled(); + expect(mockTestRule3Apply).not.toHaveBeenCalled(); }); it.each([ @@ -388,7 +443,7 @@ describe('createPermissionIntegrationRouter', () => { }); it('calls getResources for all required resources at once', () => { - expect(defaultMockedGetResources).toHaveBeenCalledWith([ + expect(defaultMockedGetResources1).toHaveBeenCalledWith([ 'default:test/resource-1', 'default:test/resource-2', 'default:test/resource-3', @@ -397,6 +452,112 @@ describe('createPermissionIntegrationRouter', () => { }); }); + describe('batched requests with different resource types', () => { + let response: Response; + + beforeEach(async () => { + response = await request(createAppWithResources(mockedResourceOptions)) + .post('/.well-known/backstage/permissions/apply-conditions') + .send({ + items: [ + { + id: '123', + resourceRef: 'default:test/resource-1', + resourceType: 'test-resource', + conditions: { + rule: 'test-rule-1', + resourceType: 'test-resource', + params: { + foo: 'a', + bar: 1, + }, + }, + }, + { + id: '234', + resourceRef: 'default:test/resource-1', + resourceType: 'test-resource', + conditions: { + rule: 'test-rule-2', + resourceType: 'test-resource', + }, + }, + { + id: '345', + resourceRef: 'default:test/resource-2', + resourceType: 'test-resource-2', + conditions: { + not: { + rule: 'test-rule-1', + resourceType: 'test-resource-2', + params: { + foo: 'a', + bar: 1, + }, + }, + }, + }, + { + id: '456', + resourceRef: 'default:test/resource-3', + resourceType: 'test-resource', + conditions: { + not: { + rule: 'test-rule-2', + resourceType: 'test-resource', + }, + }, + }, + { + id: '567', + resourceRef: 'default:test/resource-4', + resourceType: 'test-resource-2', + conditions: { + anyOf: [ + { + rule: 'test-rule-1', + resourceType: 'test-resource-2', + params: { + foo: 'a', + bar: 1, + }, + }, + { + rule: 'test-rule-1', + resourceType: 'test-resource-2', + }, + ], + }, + }, + ], + }); + }); + + it('processes batched requests', () => { + expect(response.status).toEqual(200); + expect(response.body).toEqual({ + items: [ + { id: '123', result: AuthorizeResult.ALLOW }, + { id: '234', result: AuthorizeResult.DENY }, + { id: '345', result: AuthorizeResult.ALLOW }, + { id: '456', result: AuthorizeResult.ALLOW }, + { id: '567', result: AuthorizeResult.DENY }, + ], + }); + }); + + it('calls getResources for all required resources at once', () => { + expect(defaultMockedGetResources1).toHaveBeenCalledWith([ + 'default:test/resource-1', + 'default:test/resource-3', + ]); + expect(defaultMockedGetResources2).toHaveBeenCalledWith([ + 'default:test/resource-2', + 'default:test/resource-4', + ]); + }); + }); + it('returns 400 when called with incorrect resource type', async () => { const response = await request(createApp()) .post('/.well-known/backstage/permissions/apply-conditions') @@ -675,22 +836,9 @@ describe('createPermissionIntegrationRouter', () => { ], }); }); - it.skip('returns a list of permissions and rules used by a given backend that was created with an array of resource options', async () => { - const mockedResourceOptions = [ - { - resourceType: 'test-resource', - permissions: [testPermission], - rules: [testRule1, testRule2], - }, - { - resourceType: 'test-resource-2', - permissions: [testPermission2], - rules: [testRule3], - }, - ]; - + it('returns a list of permissions and rules used by a given backend that was created with an array of resource options', async () => { const response = await request( - createApp({ resourceOptions: mockedResourceOptions }), + createAppWithResources(mockedResourceOptions), ).get('/.well-known/backstage/permissions/metadata'); expect(response.status).toEqual(200); From a788e715cfc94bd657e314cb6121ee3c6eb76e2b Mon Sep 17 00:00:00 2001 From: Ainhoa Larumbe Date: Fri, 24 Mar 2023 10:52:36 +0000 Subject: [PATCH 04/11] add changeset Signed-off-by: Ainhoa Larumbe --- .changeset/slimy-turkeys-return.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/slimy-turkeys-return.md diff --git a/.changeset/slimy-turkeys-return.md b/.changeset/slimy-turkeys-return.md new file mode 100644 index 0000000000..5f8f01479f --- /dev/null +++ b/.changeset/slimy-turkeys-return.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-node': minor +--- + +createPermissionIntegrationRouter now can also take an array of CreatePermissionIntegrationRouterResourceOptions, accepting rules and permissions for multiple resource types. From 82cd54cac821a500ec0a5693c7d0444ba43f02a4 Mon Sep 17 00:00:00 2001 From: Ainhoa Larumbe Date: Fri, 24 Mar 2023 11:00:19 +0000 Subject: [PATCH 05/11] Cleanup comments Signed-off-by: Ainhoa Larumbe --- .../createPermissionIntegrationRouter.ts | 30 ++++++------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index 0a66e2d3f2..2a2342bc8e 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -65,27 +65,6 @@ const applyConditionsRequestSchema = z.object({ ), }); -/* - -BEFORE: -[ - { resourceType: 'A', resourceRef: 'ref1', conditions: ... }, - { resourceType: 'A', resourceRef: 'ref2', conditions: ... } -] - -await getResources(['ref1', 'ref2' ]) - -NOW: -[ - { resourceType: 'A', resourceRef: 'ref1', conditions: ... }, - { resourceType: 'A', resourceRef: 'ref2', conditions: ... } - { resourceType: 'B', resourceRef: 'ref3', conditions: ... } -] - -await getResourcesA(['ref1', 'ref2' ]) -await getResourcesB(['ref3' ]) -*/ - /** * A request to load the referenced resource and apply conditions in order to * finalize a conditional authorization response. @@ -238,6 +217,9 @@ export type CreatePermissionIntegrationRouterResourceOptions< * In case the `permissions` option is provided, the router also * provides a route that exposes permissions and routes of a plugin. * + * In case an array of CreatePermissionIntegrationRouterResourceOptions is + * provided, the routes can handle permissions for multiple resource types. + * * @remarks * * To make this concrete, we can use the Backstage software catalog as an @@ -288,6 +270,9 @@ export function createPermissionIntegrationRouter(options: { /** * + * Create an express Router which provides an authorization route to allow + * integration between the permission backend and other Backstage backend + * plugins. Handles permissions for 2 resource types. * @public */ export function createPermissionIntegrationRouter< @@ -310,6 +295,9 @@ export function createPermissionIntegrationRouter< /** * + * Create an express Router which provides an authorization route to allow + * integration between the permission backend and other Backstage backend + * plugins. Handles permissions for 3 resource types. * @public */ export function createPermissionIntegrationRouter< From 49584cebc4241ecd4a10667dd3369c8f0df01187 Mon Sep 17 00:00:00 2001 From: Ainhoa Larumbe Date: Wed, 5 Apr 2023 16:24:06 +0100 Subject: [PATCH 06/11] do not pass array of options directly Signed-off-by: Ainhoa Larumbe --- .changeset/slimy-turkeys-return.md | 2 +- .../createPermissionIntegrationRouter.test.ts | 33 +++---- .../createPermissionIntegrationRouter.ts | 85 ++++++++++++++----- 3 files changed, 83 insertions(+), 37 deletions(-) diff --git a/.changeset/slimy-turkeys-return.md b/.changeset/slimy-turkeys-return.md index 5f8f01479f..4229da49e3 100644 --- a/.changeset/slimy-turkeys-return.md +++ b/.changeset/slimy-turkeys-return.md @@ -2,4 +2,4 @@ '@backstage/plugin-permission-node': minor --- -createPermissionIntegrationRouter now can also take an array of CreatePermissionIntegrationRouterResourceOptions, accepting rules and permissions for multiple resource types. +`createPermissionIntegrationRouter` now can also take an array of `CreatePermissionIntegrationRouterResourceOptions`, accepting rules and permissions for multiple resource types. diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts index a54080af2d..323a56329d 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts @@ -26,6 +26,7 @@ import { createPermissionIntegrationRouter, CreatePermissionIntegrationRouterResourceOptions, createConditionAuthorizer, + OptionResources, } from './createPermissionIntegrationRouter'; import { createPermissionRule } from './createPermissionRule'; @@ -91,20 +92,22 @@ const defaultMockedGetResources2: CreatePermissionIntegrationRouterResourceOptio resourceRefs.map(resourceRef => ({ id: resourceRef })), ); -const mockedResourceOptions = [ - { - resourceType: 'test-resource', - permissions: [testPermission], - getResources: defaultMockedGetResources1, - rules: [testRule1, testRule2], - }, - { - resourceType: 'test-resource-2', - permissions: [testPermission2], - getResources: defaultMockedGetResources2, - rules: [testRule3], - }, -]; +const mockedResourceOptions = { + resources: [ + { + resourceType: 'test-resource', + permissions: [testPermission], + getResources: defaultMockedGetResources1, + rules: [testRule1, testRule2], + }, + { + resourceType: 'test-resource-2', + permissions: [testPermission2], + getResources: defaultMockedGetResources2, + rules: [testRule3], + }, + ], +}; const createApp = ( mockedGetResources: @@ -125,7 +128,7 @@ const createApp = ( const createAppWithResources = ( resourceOptions: | CreatePermissionIntegrationRouterResourceOptions - | CreatePermissionIntegrationRouterResourceOptions[], + | OptionResources, ) => { const router = createPermissionIntegrationRouter( resourceOptions as Parameters[0], diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index 2a2342bc8e..6eb41aca71 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -207,6 +207,13 @@ export type CreatePermissionIntegrationRouterResourceOptions< ) => Promise>; }; +export type OptionResources = { + resources: + | { permissions: Array } + | CreatePermissionIntegrationRouterResourceOptions + | Array>; +}; + /** * Create an express Router which provides an authorization route to allow * integration between the permission backend and other Backstage backend @@ -252,10 +259,14 @@ export function createPermissionIntegrationRouter< TResourceType extends string, TResource, >( - options: CreatePermissionIntegrationRouterResourceOptions< - TResourceType, - TResource - >, + options: + | CreatePermissionIntegrationRouterResourceOptions + | { + resources: CreatePermissionIntegrationRouterResourceOptions< + TResourceType, + TResource + >; + }, ): express.Router; /** @@ -264,9 +275,11 @@ export function createPermissionIntegrationRouter< * permissions and routes of a plugin. * @public */ -export function createPermissionIntegrationRouter(options: { - permissions: Array; -}): express.Router; +export function createPermissionIntegrationRouter( + options: + | { permissions: Array } + | { resources: { permissions: Array } }, +): express.Router; /** * @@ -280,8 +293,8 @@ export function createPermissionIntegrationRouter< TResource1, TResourceType2 extends string, TResource2, ->( - options: [ +>(options: { + resources: [ CreatePermissionIntegrationRouterResourceOptions< TResourceType1, TResource1 @@ -290,8 +303,8 @@ export function createPermissionIntegrationRouter< TResourceType2, TResource2 >, - ], -): express.Router; + ]; +}): express.Router; /** * @@ -307,8 +320,8 @@ export function createPermissionIntegrationRouter< TResource2, TResourceType3 extends string, TResource3, ->( - options: [ +>(options: { + resources: [ CreatePermissionIntegrationRouterResourceOptions< TResourceType1, TResource1 @@ -321,8 +334,8 @@ export function createPermissionIntegrationRouter< TResourceType3, TResource3 >, - ], -): express.Router; + ]; +}): express.Router; /** * @public @@ -334,9 +347,15 @@ export function createPermissionIntegrationRouter< options: | { permissions: Array } | CreatePermissionIntegrationRouterResourceOptions - | Array>, + | OptionResources, ): express.Router { - const allOptions = [options].flat(); + const optionsWithResources = options as OptionResources< + TResourceType, + TResource + >; + const allOptions = [ + optionsWithResources.resources ? optionsWithResources.resources : options, + ].flat(); const allRules = allOptions.flatMap( option => ( @@ -347,11 +366,29 @@ export function createPermissionIntegrationRouter< ).rules || [], ); const allPermissions = allOptions - .flatMap(option => option.permissions) + .flatMap( + option => (option as { permissions: Array }).permissions, + ) .filter((p): p is Permission => !!p); const allResourceTypes = allOptions.reduce((acc, option) => { - if (isCreatePermissionIntegrationRouterResourceOptions(option)) { - acc.push(option.resourceType); + if ( + isCreatePermissionIntegrationRouterResourceOptions( + option as + | { permissions: Array } + | CreatePermissionIntegrationRouterResourceOptions< + TResourceType, + TResource + >, + ) + ) { + acc.push( + ( + option as CreatePermissionIntegrationRouterResourceOptions< + TResourceType, + TResource + > + ).resourceType, + ); } return acc; }, [] as string[]); @@ -392,7 +429,13 @@ export function createPermissionIntegrationRouter< >['getResources'] > = {}; - for (const option of allOptions) { + for (let option of allOptions) { + option = option as + | { permissions: Array } + | CreatePermissionIntegrationRouterResourceOptions< + TResourceType, + TResource + >; if (isCreatePermissionIntegrationRouterResourceOptions(option)) { ruleMapByResourceType[option.resourceType] = createGetRule( option.rules, From 81ea755347b0251582aca57a4e05bb3431f15c64 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 11 Apr 2023 13:47:54 +0200 Subject: [PATCH 07/11] permission-node: update api-report Signed-off-by: Vincenzo Scamporlino --- plugins/permission-node/api-report.md | 78 +++++++++++++++++-- .../createPermissionIntegrationRouter.ts | 5 ++ 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/plugins/permission-node/api-report.md b/plugins/permission-node/api-report.md index b3d06efdc7..10e4a0eb3c 100644 --- a/plugins/permission-node/api-report.md +++ b/plugins/permission-node/api-report.md @@ -121,15 +121,71 @@ export function createPermissionIntegrationRouter< TResourceType extends string, TResource, >( - options: CreatePermissionIntegrationRouterResourceOptions< - TResourceType, - TResource - >, + options: + | CreatePermissionIntegrationRouterResourceOptions + | { + resources: CreatePermissionIntegrationRouterResourceOptions< + TResourceType, + TResource + >; + }, ): express.Router; // @public -export function createPermissionIntegrationRouter(options: { - permissions: Array; +export function createPermissionIntegrationRouter( + options: + | { + permissions: Array; + } + | { + resources: { + permissions: Array; + }; + }, +): express.Router; + +// @public +export function createPermissionIntegrationRouter< + TResourceType1 extends string, + TResource1, + TResourceType2 extends string, + TResource2, +>(options: { + resources: [ + CreatePermissionIntegrationRouterResourceOptions< + TResourceType1, + TResource1 + >, + CreatePermissionIntegrationRouterResourceOptions< + TResourceType2, + TResource2 + >, + ]; +}): express.Router; + +// @public +export function createPermissionIntegrationRouter< + TResourceType1 extends string, + TResource1, + TResourceType2 extends string, + TResource2, + TResourceType3 extends string, + TResource3, +>(options: { + resources: [ + CreatePermissionIntegrationRouterResourceOptions< + TResourceType1, + TResource1 + >, + CreatePermissionIntegrationRouterResourceOptions< + TResourceType2, + TResource2 + >, + CreatePermissionIntegrationRouterResourceOptions< + TResourceType3, + TResource3 + >, + ]; }): express.Router; // @public @@ -193,6 +249,16 @@ export type MetadataResponseSerializedRule = { paramsSchema?: ReturnType; }; +// @public +export type OptionResources = { + resources: + | { + permissions: Array; + } + | CreatePermissionIntegrationRouterResourceOptions + | Array>; +}; + // @public export interface PermissionPolicy { // (undocumented) diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index 6eb41aca71..d015734d91 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -207,6 +207,11 @@ export type CreatePermissionIntegrationRouterResourceOptions< ) => Promise>; }; +/** + * Options for creating a permission integration router + * + * @public + */ export type OptionResources = { resources: | { permissions: Array } From b947f4230d49e825333bc802fa5a1530f18faa1e Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 12 Apr 2023 11:39:08 +0200 Subject: [PATCH 08/11] permission-node: improve typings Signed-off-by: Vincenzo Scamporlino --- .../createPermissionIntegrationRouter.test.ts | 39 ++-- .../createPermissionIntegrationRouter.ts | 174 +++++++----------- 2 files changed, 84 insertions(+), 129 deletions(-) diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts index 323a56329d..c369c8be95 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts @@ -92,7 +92,7 @@ const defaultMockedGetResources2: CreatePermissionIntegrationRouterResourceOptio resourceRefs.map(resourceRef => ({ id: resourceRef })), ); -const mockedResourceOptions = { +const mockedOptionResources: OptionResources = { resources: [ { resourceType: 'test-resource', @@ -125,17 +125,6 @@ const createApp = ( return express().use(router); }; -const createAppWithResources = ( - resourceOptions: - | CreatePermissionIntegrationRouterResourceOptions - | OptionResources, -) => { - const router = createPermissionIntegrationRouter( - resourceOptions as Parameters[0], - ); - return express().use(router); -}; - describe('createPermissionIntegrationRouter', () => { afterEach(() => { jest.clearAllMocks(); @@ -240,7 +229,11 @@ describe('createPermissionIntegrationRouter', () => { (defaultMockedGetResources1 as jest.Mock).mockClear(); - response = await request(createAppWithResources(mockedResourceOptions)) + const app = express().use( + createPermissionIntegrationRouter(mockedOptionResources), + ); + + response = await request(app) .post('/.well-known/backstage/permissions/apply-conditions') .send({ items: [ @@ -459,7 +452,11 @@ describe('createPermissionIntegrationRouter', () => { let response: Response; beforeEach(async () => { - response = await request(createAppWithResources(mockedResourceOptions)) + const app = express().use( + createPermissionIntegrationRouter(mockedOptionResources), + ); + + response = await request(app) .post('/.well-known/backstage/permissions/apply-conditions') .send({ items: [ @@ -763,11 +760,13 @@ describe('createPermissionIntegrationRouter', () => { it('returns 501 with no getResources implementation', async () => { const response = await request( - createAppWithResources({ - resourceType: 'test-resource', - permissions: [testPermission], - rules: [testRule1, testRule2], - }), + express().use( + createPermissionIntegrationRouter({ + resourceType: 'test-resource', + permissions: [testPermission], + rules: [testRule1, testRule2], + }), + ), ) .post('/.well-known/backstage/permissions/apply-conditions') .send({ @@ -841,7 +840,7 @@ describe('createPermissionIntegrationRouter', () => { }); it('returns a list of permissions and rules used by a given backend that was created with an array of resource options', async () => { const response = await request( - createAppWithResources(mockedResourceOptions), + express().use(createPermissionIntegrationRouter(mockedOptionResources)), ).get('/.well-known/backstage/permissions/metadata'); expect(response.status).toEqual(200); diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index d015734d91..1ac53a4721 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -207,16 +207,46 @@ export type CreatePermissionIntegrationRouterResourceOptions< ) => Promise>; }; -/** - * Options for creating a permission integration router - * - * @public - */ -export type OptionResources = { - resources: - | { permissions: Array } - | CreatePermissionIntegrationRouterResourceOptions - | Array>; +export type OptionResources< + TResourceType1 extends string = string, + TResource1 = any, + TResourceType2 extends string = string, + TResource2 = any, + TResourceType3 extends string = string, + TResource3 = any, +> = { + resources: Readonly< + | [ + CreatePermissionIntegrationRouterResourceOptions< + TResourceType1, + TResource1 + >, + ] + | [ + CreatePermissionIntegrationRouterResourceOptions< + TResourceType1, + TResource1 + >, + CreatePermissionIntegrationRouterResourceOptions< + TResourceType2, + TResource2 + >, + ] + | [ + CreatePermissionIntegrationRouterResourceOptions< + TResourceType1, + TResource1 + >, + CreatePermissionIntegrationRouterResourceOptions< + TResourceType2, + TResource2 + >, + CreatePermissionIntegrationRouterResourceOptions< + TResourceType3, + TResource3 + >, + ] + >; }; /** @@ -229,8 +259,8 @@ export type OptionResources = { * In case the `permissions` option is provided, the router also * provides a route that exposes permissions and routes of a plugin. * - * In case an array of CreatePermissionIntegrationRouterResourceOptions is - * provided, the routes can handle permissions for multiple resource types. + * In case resources is provided, the routes can handle permissions + * for multiple resource types. * * @remarks * @@ -260,64 +290,6 @@ export type OptionResources = { * * @public */ -export function createPermissionIntegrationRouter< - TResourceType extends string, - TResource, ->( - options: - | CreatePermissionIntegrationRouterResourceOptions - | { - resources: CreatePermissionIntegrationRouterResourceOptions< - TResourceType, - TResource - >; - }, -): express.Router; - -/** - * - * Create an express Router which provides a route that exposes - * permissions and routes of a plugin. - * @public - */ -export function createPermissionIntegrationRouter( - options: - | { permissions: Array } - | { resources: { permissions: Array } }, -): express.Router; - -/** - * - * Create an express Router which provides an authorization route to allow - * integration between the permission backend and other Backstage backend - * plugins. Handles permissions for 2 resource types. - * @public - */ -export function createPermissionIntegrationRouter< - TResourceType1 extends string, - TResource1, - TResourceType2 extends string, - TResource2, ->(options: { - resources: [ - CreatePermissionIntegrationRouterResourceOptions< - TResourceType1, - TResource1 - >, - CreatePermissionIntegrationRouterResourceOptions< - TResourceType2, - TResource2 - >, - ]; -}): express.Router; - -/** - * - * Create an express Router which provides an authorization route to allow - * integration between the permission backend and other Backstage backend - * plugins. Handles permissions for 3 resource types. - * @public - */ export function createPermissionIntegrationRouter< TResourceType1 extends string, TResource1, @@ -325,39 +297,23 @@ export function createPermissionIntegrationRouter< TResource2, TResourceType3 extends string, TResource3, ->(options: { - resources: [ - CreatePermissionIntegrationRouterResourceOptions< - TResourceType1, - TResource1 - >, - CreatePermissionIntegrationRouterResourceOptions< - TResourceType2, - TResource2 - >, - CreatePermissionIntegrationRouterResourceOptions< - TResourceType3, - TResource3 - >, - ]; -}): express.Router; - -/** - * @public - */ -export function createPermissionIntegrationRouter< - TResourceType extends string, - TResource, >( options: | { permissions: Array } - | CreatePermissionIntegrationRouterResourceOptions - | OptionResources, + | CreatePermissionIntegrationRouterResourceOptions< + TResourceType1, + TResource1 + > + | OptionResources< + TResourceType1, + TResource1, + TResourceType2, + TResource2, + TResourceType3, + TResource3 + >, ): express.Router { - const optionsWithResources = options as OptionResources< - TResourceType, - TResource - >; + const optionsWithResources = options as OptionResources; const allOptions = [ optionsWithResources.resources ? optionsWithResources.resources : options, ].flat(); @@ -365,8 +321,8 @@ export function createPermissionIntegrationRouter< option => ( option as CreatePermissionIntegrationRouterResourceOptions< - TResourceType, - TResource + TResourceType1, + TResource1 > ).rules || [], ); @@ -381,16 +337,16 @@ export function createPermissionIntegrationRouter< option as | { permissions: Array } | CreatePermissionIntegrationRouterResourceOptions< - TResourceType, - TResource + TResourceType1, + TResource1 >, ) ) { acc.push( ( option as CreatePermissionIntegrationRouterResourceOptions< - TResourceType, - TResource + TResourceType1, + TResource1 > ).resourceType, ); @@ -429,8 +385,8 @@ export function createPermissionIntegrationRouter< const getResourcesByResourceType: Record< string, CreatePermissionIntegrationRouterResourceOptions< - TResourceType, - TResource + TResourceType1, + TResource1 >['getResources'] > = {}; @@ -438,8 +394,8 @@ export function createPermissionIntegrationRouter< option = option as | { permissions: Array } | CreatePermissionIntegrationRouterResourceOptions< - TResourceType, - TResource + TResourceType1, + TResource1 >; if (isCreatePermissionIntegrationRouterResourceOptions(option)) { ruleMapByResourceType[option.resourceType] = createGetRule( From 2c837e99a3d5578b41a634ca866e009c50a79ee9 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 12 Apr 2023 11:45:03 +0200 Subject: [PATCH 09/11] permission-node: add example to changeset Signed-off-by: Vincenzo Scamporlino --- .changeset/slimy-turkeys-return.md | 21 ++- plugins/permission-node/api-report.md | 128 ++++++++---------- .../createPermissionIntegrationRouter.ts | 6 + 3 files changed, 83 insertions(+), 72 deletions(-) diff --git a/.changeset/slimy-turkeys-return.md b/.changeset/slimy-turkeys-return.md index 4229da49e3..db10000509 100644 --- a/.changeset/slimy-turkeys-return.md +++ b/.changeset/slimy-turkeys-return.md @@ -1,5 +1,22 @@ --- -'@backstage/plugin-permission-node': minor +'@backstage/plugin-permission-node': patch --- -`createPermissionIntegrationRouter` now can also take an array of `CreatePermissionIntegrationRouterResourceOptions`, accepting rules and permissions for multiple resource types. +`createPermissionIntegrationRouter` now accepts rules and permissions for multiple resource types. Example: + +```typescript +createPermissionIntegrationRouter({ + resources: [ + { + resourceType: 'resourceType-1', + permissions: permissionsResourceType1, + rules: rulesResourceType1, + }, + { + resourceType: 'resourceType-2', + permissions: permissionsResourceType2, + rules: rulesResourceType2, + }, + ], +}); +``` diff --git a/plugins/permission-node/api-report.md b/plugins/permission-node/api-report.md index 10e4a0eb3c..4aac4616ed 100644 --- a/plugins/permission-node/api-report.md +++ b/plugins/permission-node/api-report.md @@ -116,53 +116,6 @@ export const createConditionTransformer: < permissionRules: [...TRules], ) => ConditionTransformer; -// @public -export function createPermissionIntegrationRouter< - TResourceType extends string, - TResource, ->( - options: - | CreatePermissionIntegrationRouterResourceOptions - | { - resources: CreatePermissionIntegrationRouterResourceOptions< - TResourceType, - TResource - >; - }, -): express.Router; - -// @public -export function createPermissionIntegrationRouter( - options: - | { - permissions: Array; - } - | { - resources: { - permissions: Array; - }; - }, -): express.Router; - -// @public -export function createPermissionIntegrationRouter< - TResourceType1 extends string, - TResource1, - TResourceType2 extends string, - TResource2, ->(options: { - resources: [ - CreatePermissionIntegrationRouterResourceOptions< - TResourceType1, - TResource1 - >, - CreatePermissionIntegrationRouterResourceOptions< - TResourceType2, - TResource2 - >, - ]; -}): express.Router; - // @public export function createPermissionIntegrationRouter< TResourceType1 extends string, @@ -171,22 +124,24 @@ export function createPermissionIntegrationRouter< TResource2, TResourceType3 extends string, TResource3, ->(options: { - resources: [ - CreatePermissionIntegrationRouterResourceOptions< - TResourceType1, - TResource1 - >, - CreatePermissionIntegrationRouterResourceOptions< - TResourceType2, - TResource2 - >, - CreatePermissionIntegrationRouterResourceOptions< - TResourceType3, - TResource3 - >, - ]; -}): express.Router; +>( + options: + | { + permissions: Array; + } + | CreatePermissionIntegrationRouterResourceOptions< + TResourceType1, + TResource1 + > + | OptionResources< + TResourceType1, + TResource1, + TResourceType2, + TResource2, + TResourceType3, + TResource3 + >, +): express.Router; // @public export type CreatePermissionIntegrationRouterResourceOptions< @@ -250,13 +205,46 @@ export type MetadataResponseSerializedRule = { }; // @public -export type OptionResources = { - resources: - | { - permissions: Array; - } - | CreatePermissionIntegrationRouterResourceOptions - | Array>; +export type OptionResources< + TResourceType1 extends string = string, + TResource1 = any, + TResourceType2 extends string = string, + TResource2 = any, + TResourceType3 extends string = string, + TResource3 = any, +> = { + resources: Readonly< + | [ + CreatePermissionIntegrationRouterResourceOptions< + TResourceType1, + TResource1 + >, + ] + | [ + CreatePermissionIntegrationRouterResourceOptions< + TResourceType1, + TResource1 + >, + CreatePermissionIntegrationRouterResourceOptions< + TResourceType2, + TResource2 + >, + ] + | [ + CreatePermissionIntegrationRouterResourceOptions< + TResourceType1, + TResource1 + >, + CreatePermissionIntegrationRouterResourceOptions< + TResourceType2, + TResource2 + >, + CreatePermissionIntegrationRouterResourceOptions< + TResourceType3, + TResource3 + >, + ] + >; }; // @public diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index 1ac53a4721..f417cb554a 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -207,6 +207,12 @@ export type CreatePermissionIntegrationRouterResourceOptions< ) => Promise>; }; +/** + * Options for creating a permission integration router exposing + * permissions and rules from multiple resource types. + * + * @public + */ export type OptionResources< TResourceType1 extends string = string, TResource1 = any, From 4dd6bbe59e6073f89fe188b5fa0b8578016d6686 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 12 Apr 2023 17:13:52 +0200 Subject: [PATCH 10/11] permission-node: add support for extra permissions Signed-off-by: Vincenzo Scamporlino --- .../createPermissionIntegrationRouter.test.ts | 83 ++++++++++++++++++- .../createPermissionIntegrationRouter.ts | 11 +-- 2 files changed, 87 insertions(+), 7 deletions(-) diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts index c369c8be95..9408d154ff 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts @@ -795,7 +795,7 @@ describe('createPermissionIntegrationRouter', () => { }); describe('GET /.well-known/backstage/permissions/metadata', () => { - it('returns a list of permissions and rules used by a given backend', async () => { + it('returns a list of permissions and rules of a single resource type', async () => { const response = await request(createApp()).get( '/.well-known/backstage/permissions/metadata', ); @@ -838,7 +838,8 @@ describe('createPermissionIntegrationRouter', () => { ], }); }); - it('returns a list of permissions and rules used by a given backend that was created with an array of resource options', async () => { + + it('returns a list of permissions and rules from multiple resource types', async () => { const response = await request( express().use(createPermissionIntegrationRouter(mockedOptionResources)), ).get('/.well-known/backstage/permissions/metadata'); @@ -893,6 +894,84 @@ describe('createPermissionIntegrationRouter', () => { }); }); }); + + it('returns a list of basic permissions together with permissions and rules from multiple resource types', async () => { + const aPermission = createPermission({ + name: 'a.permission', + attributes: {}, + }); + + const response = await request( + express().use( + createPermissionIntegrationRouter({ + permissions: [aPermission], + resources: [ + { + resourceType: 'test-resource', + permissions: [testPermission], + getResources: defaultMockedGetResources1, + rules: [testRule1, testRule2], + }, + { + resourceType: 'test-resource-2', + permissions: [testPermission2], + getResources: defaultMockedGetResources2, + rules: [testRule3], + }, + ], + }), + ), + ).get('/.well-known/backstage/permissions/metadata'); + + expect(response.status).toEqual(200); + expect(response.body).toEqual({ + permissions: [aPermission, testPermission, testPermission2], + rules: [ + { + name: testRule1.name, + description: testRule1.description, + resourceType: testRule1.resourceType, + paramsSchema: { + $schema: 'http://json-schema.org/draft-07/schema#', + additionalProperties: false, + properties: { + foo: { + type: 'string', + }, + bar: { + description: 'bar', + type: 'number', + }, + }, + required: ['foo', 'bar'], + type: 'object', + }, + }, + { + name: testRule2.name, + description: testRule2.description, + resourceType: testRule2.resourceType, + paramsSchema: { + $schema: 'http://json-schema.org/draft-07/schema#', + additionalProperties: false, + properties: {}, + type: 'object', + }, + }, + { + name: testRule3.name, + description: testRule3.description, + resourceType: testRule3.resourceType, + paramsSchema: { + $schema: 'http://json-schema.org/draft-07/schema#', + additionalProperties: false, + properties: {}, + type: 'object', + }, + }, + ], + }); + }); }); describe('createConditionAuthorizer', () => { diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index f417cb554a..fd606c7810 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -332,11 +332,12 @@ export function createPermissionIntegrationRouter< > ).rules || [], ); - const allPermissions = allOptions - .flatMap( - option => (option as { permissions: Array }).permissions, - ) - .filter((p): p is Permission => !!p); + const allPermissions = [ + ...((options as { permissions: Permission[] }).permissions || []), + ...(optionsWithResources.resources?.flatMap(o => o.permissions || []) || + []), + ]; + const allResourceTypes = allOptions.reduce((acc, option) => { if ( isCreatePermissionIntegrationRouterResourceOptions( From 7996d8900b22987fefba2ba7915bec4320b73902 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Thu, 13 Apr 2023 21:02:40 +0200 Subject: [PATCH 11/11] permission-node: improve naming Signed-off-by: Vincenzo Scamporlino --- plugins/permission-node/api-report.md | 4 ++-- .../integration/createPermissionIntegrationRouter.test.ts | 4 ++-- .../src/integration/createPermissionIntegrationRouter.ts | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/plugins/permission-node/api-report.md b/plugins/permission-node/api-report.md index 4aac4616ed..2a9a79377b 100644 --- a/plugins/permission-node/api-report.md +++ b/plugins/permission-node/api-report.md @@ -133,7 +133,7 @@ export function createPermissionIntegrationRouter< TResourceType1, TResource1 > - | OptionResources< + | PermissionIntegrationRouterOptions< TResourceType1, TResource1, TResourceType2, @@ -205,7 +205,7 @@ export type MetadataResponseSerializedRule = { }; // @public -export type OptionResources< +export type PermissionIntegrationRouterOptions< TResourceType1 extends string = string, TResource1 = any, TResourceType2 extends string = string, diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts index 9408d154ff..24e1b91065 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts @@ -26,7 +26,7 @@ import { createPermissionIntegrationRouter, CreatePermissionIntegrationRouterResourceOptions, createConditionAuthorizer, - OptionResources, + PermissionIntegrationRouterOptions, } from './createPermissionIntegrationRouter'; import { createPermissionRule } from './createPermissionRule'; @@ -92,7 +92,7 @@ const defaultMockedGetResources2: CreatePermissionIntegrationRouterResourceOptio resourceRefs.map(resourceRef => ({ id: resourceRef })), ); -const mockedOptionResources: OptionResources = { +const mockedOptionResources: PermissionIntegrationRouterOptions = { resources: [ { resourceType: 'test-resource', diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index fd606c7810..b6f6176f4d 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -213,7 +213,7 @@ export type CreatePermissionIntegrationRouterResourceOptions< * * @public */ -export type OptionResources< +export type PermissionIntegrationRouterOptions< TResourceType1 extends string = string, TResource1 = any, TResourceType2 extends string = string, @@ -310,7 +310,7 @@ export function createPermissionIntegrationRouter< TResourceType1, TResource1 > - | OptionResources< + | PermissionIntegrationRouterOptions< TResourceType1, TResource1, TResourceType2, @@ -319,7 +319,7 @@ export function createPermissionIntegrationRouter< TResource3 >, ): express.Router { - const optionsWithResources = options as OptionResources; + const optionsWithResources = options as PermissionIntegrationRouterOptions; const allOptions = [ optionsWithResources.resources ? optionsWithResources.resources : options, ].flat();