From b76825924451180f7891622d8e01a13b10908023 Mon Sep 17 00:00:00 2001 From: MT Lewis Date: Fri, 7 Jan 2022 12:49:18 +0000 Subject: [PATCH 1/3] permission-backend: wrap authorize request and response batches in envelope Signed-off-by: MT Lewis --- .changeset/big-moles-visit.md | 5 + .changeset/chilled-cats-marry.md | 5 + .../src/service/router.test.ts | 489 ++++++++++-------- .../permission-backend/src/service/router.ts | 52 +- plugins/permission-common/api-report.md | 10 + .../src/PermissionClient.test.ts | 49 +- .../permission-common/src/PermissionClient.ts | 62 ++- plugins/permission-common/src/types/api.ts | 20 +- plugins/permission-common/src/types/index.ts | 2 + .../src/ServerPermissionClient.test.ts | 4 +- 10 files changed, 395 insertions(+), 303 deletions(-) create mode 100644 .changeset/big-moles-visit.md create mode 100644 .changeset/chilled-cats-marry.md diff --git a/.changeset/big-moles-visit.md b/.changeset/big-moles-visit.md new file mode 100644 index 0000000000..e1af13bc55 --- /dev/null +++ b/.changeset/big-moles-visit.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-backend': minor +--- + +**BREAKING**: Wrap batched requests and responses to /authorize in an envelope object. The latest version of the PermissionClient in @backstage/permission-common uses the new format - as long as the permission-backend is consumed using this client, no other changes are necessary. diff --git a/.changeset/chilled-cats-marry.md b/.changeset/chilled-cats-marry.md new file mode 100644 index 0000000000..2a91c2bc99 --- /dev/null +++ b/.changeset/chilled-cats-marry.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-common': minor +--- + +**BREAKING**: PermissionClient has been updated to use the new request and response format in the latest version of @backstage/permission-backend. diff --git a/plugins/permission-backend/src/service/router.test.ts b/plugins/permission-backend/src/service/router.test.ts index 971cb6a223..fd09cca828 100644 --- a/plugins/permission-backend/src/service/router.test.ts +++ b/plugins/permission-backend/src/service/router.test.ts @@ -103,22 +103,24 @@ describe('createRouter', () => { it('calls the permission policy', async () => { const response = await request(app) .post('/authorize') - .send([ - { - id: '123', - permission: { - name: 'test.permission1', - attributes: {}, + .send({ + items: [ + { + id: '123', + permission: { + name: 'test.permission1', + attributes: {}, + }, }, - }, - { - id: '234', - permission: { - name: 'test.permission2', - attributes: {}, + { + id: '234', + permission: { + name: 'test.permission2', + attributes: {}, + }, }, - }, - ]); + ], + }); expect(response.status).toEqual(200); @@ -141,10 +143,12 @@ describe('createRouter', () => { undefined, ); - expect(response.body).toEqual([ - { id: '123', result: AuthorizeResult.DENY }, - { id: '234', result: AuthorizeResult.DENY }, - ]); + expect(response.body).toEqual({ + items: [ + { id: '123', result: AuthorizeResult.DENY }, + { id: '234', result: AuthorizeResult.DENY }, + ], + }); }); it('resolves identity from the Authorization header', async () => { @@ -152,15 +156,17 @@ describe('createRouter', () => { const response = await request(app) .post('/authorize') .auth(token, { type: 'bearer' }) - .send([ - { - id: '123', - permission: { - name: 'test.permission', - attributes: {}, + .send({ + items: [ + { + id: '123', + permission: { + name: 'test.permission', + attributes: {}, + }, }, - }, - ]); + ], + }); expect(response.status).toEqual(200); expect(policy.handle).toHaveBeenCalledWith( @@ -172,9 +178,9 @@ describe('createRouter', () => { }, { id: 'test-user', token: 'test-token' }, ); - expect(response.body).toEqual([ - { id: '123', result: AuthorizeResult.ALLOW }, - ]); + expect(response.body).toEqual({ + items: [{ id: '123', result: AuthorizeResult.ALLOW }], + }); }); describe('conditional policy result', () => { @@ -188,27 +194,31 @@ describe('createRouter', () => { const response = await request(app) .post('/authorize') - .send([ - { - id: '123', - permission: { - name: 'test.permission', - resourceType: 'test-resource-1', - attributes: {}, + .send({ + items: [ + { + id: '123', + permission: { + name: 'test.permission', + resourceType: 'test-resource-1', + attributes: {}, + }, }, - }, - ]); + ], + }); expect(response.status).toEqual(200); - expect(response.body).toEqual([ - { - id: '123', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'test-plugin', - resourceType: 'test-resource-1', - conditions: { rule: 'test-rule', params: ['abc'] }, - }, - ]); + expect(response.body).toEqual({ + items: [ + { + id: '123', + result: AuthorizeResult.CONDITIONAL, + pluginId: 'test-plugin', + resourceType: 'test-resource-1', + conditions: { rule: 'test-rule', params: ['abc'] }, + }, + ], + }); }); it('makes separate batched requests to multiple plugin backends', async () => { @@ -241,44 +251,46 @@ describe('createRouter', () => { const response = await request(app) .post('/authorize') .auth('test-token', { type: 'bearer' }) - .send([ - { - id: '123', - permission: { - name: 'test.permission.1', - resourceType: 'test-resource-1', - attributes: {}, + .send({ + items: [ + { + id: '123', + permission: { + name: 'test.permission.1', + resourceType: 'test-resource-1', + attributes: {}, + }, + resourceRef: 'resource:1', }, - resourceRef: 'resource:1', - }, - { - id: '234', - permission: { - name: 'test.permission.2', - resourceType: 'test-resource-2', - attributes: {}, + { + id: '234', + permission: { + name: 'test.permission.2', + resourceType: 'test-resource-2', + attributes: {}, + }, + resourceRef: 'resource:2', }, - resourceRef: 'resource:2', - }, - { - id: '345', - permission: { - name: 'test.permission.3', - resourceType: 'test-resource-1', - attributes: {}, + { + id: '345', + permission: { + name: 'test.permission.3', + resourceType: 'test-resource-1', + attributes: {}, + }, + resourceRef: 'resource:3', }, - resourceRef: 'resource:3', - }, - { - id: '456', - permission: { - name: 'test.permission.4', - resourceType: 'test-resource-2', - attributes: {}, + { + id: '456', + permission: { + name: 'test.permission.4', + resourceType: 'test-resource-2', + attributes: {}, + }, + resourceRef: 'resource:4', }, - resourceRef: 'resource:4', - }, - ]); + ], + }); expect(mockApplyConditions).toHaveBeenCalledWith( 'plugin-1', @@ -319,12 +331,14 @@ describe('createRouter', () => { ); expect(response.status).toEqual(200); - expect(response.body).toEqual([ - { id: '123', result: AuthorizeResult.ALLOW }, - { id: '234', result: AuthorizeResult.ALLOW }, - { id: '345', result: AuthorizeResult.DENY }, - { id: '456', result: AuthorizeResult.DENY }, - ]); + expect(response.body).toEqual({ + items: [ + { id: '123', result: AuthorizeResult.ALLOW }, + { id: '234', result: AuthorizeResult.ALLOW }, + { id: '345', result: AuthorizeResult.DENY }, + { id: '456', result: AuthorizeResult.DENY }, + ], + }); }); it('leaves definitive results unchanged', async () => { @@ -363,60 +377,62 @@ describe('createRouter', () => { const response = await request(app) .post('/authorize') .auth('test-token', { type: 'bearer' }) - .send([ - { - id: '123', - permission: { - name: 'test.permission.1', - resourceType: 'test-resource-1', - attributes: {}, + .send({ + items: [ + { + id: '123', + permission: { + name: 'test.permission.1', + resourceType: 'test-resource-1', + attributes: {}, + }, + resourceRef: 'resource:1', }, - resourceRef: 'resource:1', - }, - { - id: '234', - permission: { - name: 'test.permission.2', - resourceType: 'test-resource-2', - attributes: {}, + { + id: '234', + permission: { + name: 'test.permission.2', + resourceType: 'test-resource-2', + attributes: {}, + }, + resourceRef: 'resource:2', }, - resourceRef: 'resource:2', - }, - { - id: '345', - permission: { - name: 'test.permission.3', - resourceType: 'test-resource-1', - attributes: {}, + { + id: '345', + permission: { + name: 'test.permission.3', + resourceType: 'test-resource-1', + attributes: {}, + }, + resourceRef: 'resource:3', }, - resourceRef: 'resource:3', - }, - { - id: '456', - permission: { - name: 'test.permission.4', - resourceType: 'test-resource-1', - attributes: {}, + { + id: '456', + permission: { + name: 'test.permission.4', + resourceType: 'test-resource-1', + attributes: {}, + }, + resourceRef: 'resource:4', }, - resourceRef: 'resource:4', - }, - { - id: '567', - permission: { - name: 'test.permission.5', - resourceType: 'test-resource-2', - attributes: {}, + { + id: '567', + permission: { + name: 'test.permission.5', + resourceType: 'test-resource-2', + attributes: {}, + }, + resourceRef: 'resource:5', }, - resourceRef: 'resource:5', - }, - { - id: '678', - permission: { - name: 'test.permission.6', - attributes: {}, + { + id: '678', + permission: { + name: 'test.permission.6', + attributes: {}, + }, }, - }, - ]); + ], + }); expect(mockApplyConditions).toHaveBeenCalledWith( 'plugin-1', @@ -457,14 +473,16 @@ describe('createRouter', () => { ); expect(response.status).toEqual(200); - expect(response.body).toEqual([ - { id: '123', result: AuthorizeResult.DENY }, - { id: '234', result: AuthorizeResult.DENY }, - { id: '345', result: AuthorizeResult.ALLOW }, - { id: '456', result: AuthorizeResult.ALLOW }, - { id: '567', result: AuthorizeResult.ALLOW }, - { id: '678', result: AuthorizeResult.DENY }, - ]); + expect(response.body).toEqual({ + items: [ + { id: '123', result: AuthorizeResult.DENY }, + { id: '234', result: AuthorizeResult.DENY }, + { id: '345', result: AuthorizeResult.ALLOW }, + { id: '456', result: AuthorizeResult.ALLOW }, + { id: '567', result: AuthorizeResult.ALLOW }, + { id: '678', result: AuthorizeResult.DENY }, + ], + }); }); it('leaves conditional results without resourceRefs unchanged', async () => { @@ -494,43 +512,45 @@ describe('createRouter', () => { const response = await request(app) .post('/authorize') .auth('test-token', { type: 'bearer' }) - .send([ - { - id: '123', - permission: { - name: 'test.permission.1', - resourceType: 'test-resource-1', - attributes: {}, + .send({ + items: [ + { + id: '123', + permission: { + name: 'test.permission.1', + resourceType: 'test-resource-1', + attributes: {}, + }, + resourceRef: 'resource:1', }, - resourceRef: 'resource:1', - }, - { - id: '234', - permission: { - name: 'test.permission.2', - resourceType: 'test-resource-2', - attributes: {}, + { + id: '234', + permission: { + name: 'test.permission.2', + resourceType: 'test-resource-2', + attributes: {}, + }, + resourceRef: 'resource:2', }, - resourceRef: 'resource:2', - }, - { - id: '345', - permission: { - name: 'test.permission.3', - resourceType: 'test-resource-1', - attributes: {}, + { + id: '345', + permission: { + name: 'test.permission.3', + resourceType: 'test-resource-1', + attributes: {}, + }, + resourceRef: 'resource:3', }, - resourceRef: 'resource:3', - }, - { - id: '456', - permission: { - name: 'test.permission.4', - resourceType: 'test-resource-1', - attributes: {}, + { + id: '456', + permission: { + name: 'test.permission.4', + resourceType: 'test-resource-1', + attributes: {}, + }, }, - }, - ]); + ], + }); expect(mockApplyConditions).toHaveBeenCalledWith( 'plugin-1', @@ -559,18 +579,20 @@ describe('createRouter', () => { ); expect(response.status).toEqual(200); - expect(response.body).toEqual([ - { id: '123', result: AuthorizeResult.ALLOW }, - { id: '234', result: AuthorizeResult.ALLOW }, - { id: '345', result: AuthorizeResult.ALLOW }, - { - id: '456', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', - resourceType: 'test-resource-1', - conditions: { rule: 'test-rule', params: ['abc'] }, - }, - ]); + expect(response.body).toEqual({ + items: [ + { id: '123', result: AuthorizeResult.ALLOW }, + { id: '234', result: AuthorizeResult.ALLOW }, + { id: '345', result: AuthorizeResult.ALLOW }, + { + id: '456', + result: AuthorizeResult.CONDITIONAL, + pluginId: 'plugin-1', + resourceType: 'test-resource-1', + conditions: { rule: 'test-rule', params: ['abc'] }, + }, + ], + }); }); it.each<[ApplyConditionsResponseEntry['result'], string]>([ @@ -600,26 +622,28 @@ describe('createRouter', () => { const response = await request(app) .post('/authorize') .auth('test-token', { type: 'bearer' }) - .send([ - { - id: '123', - resourceRef: 'test/resource', - permission: { - name: 'test.permission', - resourceType: 'test-resource-1', - attributes: {}, + .send({ + items: [ + { + id: '123', + resourceRef: 'test/resource', + permission: { + name: 'test.permission', + resourceType: 'test-resource-1', + attributes: {}, + }, }, - }, - { - id: '234', - resourceRef: 'test/resource', - permission: { - name: 'test.permission', - resourceType: 'test-resource-1', - attributes: {}, + { + id: '234', + resourceRef: 'test/resource', + permission: { + name: 'test.permission', + resourceType: 'test-resource-1', + attributes: {}, + }, }, - }, - ]); + ], + }); expect(mockApplyConditions).toHaveBeenCalledWith( 'test-plugin', @@ -641,16 +665,18 @@ describe('createRouter', () => { ); expect(response.status).toEqual(200); - expect(response.body).toEqual([ - { - id: '123', - result, - }, - { - id: '234', - result, - }, - ]); + expect(response.body).toEqual({ + items: [ + { + id: '123', + result, + }, + { + id: '234', + result, + }, + ], + }); }, ); }); @@ -660,9 +686,14 @@ describe('createRouter', () => { '', {}, [{ permission: { name: 'test.permission', attributes: {} } }], - [{ id: '123' }], - [{ id: '123', permission: { name: 'test.permission' } }], - [{ id: '123', permission: { attributes: { invalid: 'attribute' } } }], + { items: [{ permission: { name: 'test.permission', attributes: {} } }] }, + { items: [{ id: '123' }] }, + { items: [{ id: '123', permission: { name: 'test.permission' } }] }, + { + items: [ + { id: '123', permission: { attributes: { invalid: 'attribute' } } }, + ], + }, ])('returns a 400 error for invalid request %#', async requestBody => { const response = await request(app).post('/authorize').send(requestBody); @@ -686,16 +717,18 @@ describe('createRouter', () => { const response = await request(app) .post('/authorize') - .send([ - { - id: '123', - permission: { - name: 'test.permission', - resourceType: 'test-resource-1', - attributes: {}, + .send({ + items: [ + { + id: '123', + permission: { + name: 'test.permission', + resourceType: 'test-resource-1', + attributes: {}, + }, }, - }, - ]); + ], + }); expect(response.status).toEqual(500); expect(response.body).toEqual( diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index 58438afbb5..c504fdbe3a 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -32,6 +32,8 @@ import { AuthorizeResponse, AuthorizeRequest, Identified, + AuthorizeRequestEnvelope, + AuthorizeResponseEnvelope, } from '@backstage/plugin-permission-common'; import { ApplyConditionsRequestEntry, @@ -42,26 +44,28 @@ import { PermissionIntegrationClient } from './PermissionIntegrationClient'; import { memoize } from 'lodash'; import DataLoader from 'dataloader'; -const requestSchema: z.ZodSchema[]> = z.array( - z.object({ - id: z.string(), - resourceRef: z.string().optional(), - permission: z.object({ - name: z.string(), - resourceType: z.string().optional(), - attributes: z.object({ - action: z - .union([ - z.literal('create'), - z.literal('read'), - z.literal('update'), - z.literal('delete'), - ]) - .optional(), +const requestSchema: z.ZodSchema = z.object({ + items: z.array( + z.object({ + id: z.string(), + resourceRef: z.string().optional(), + permission: z.object({ + name: z.string(), + resourceType: z.string().optional(), + attributes: z.object({ + action: z + .union([ + z.literal('create'), + z.literal('read'), + z.literal('update'), + z.literal('delete'), + ]) + .optional(), + }), }), }), - }), -); + ), +}); /** * Options required when constructing a new {@link express#Router} using @@ -150,8 +154,8 @@ export async function createRouter( router.post( '/authorize', async ( - req: Request[]>, - res: Response[]>, + req: Request, + res: Response, ) => { const token = IdentityClient.getBearerToken(req.header('authorization')); const user = token ? await identity.authenticate(token) : undefined; @@ -164,15 +168,15 @@ export async function createRouter( const body = parseResult.data; - res.json( - await handleRequest( - body, + res.json({ + items: await handleRequest( + body.items, user, policy, permissionIntegrationClient, req.header('authorization'), ), - ); + }); }, ); diff --git a/plugins/permission-common/api-report.md b/plugins/permission-common/api-report.md index 70795d14ac..3c681cc46f 100644 --- a/plugins/permission-common/api-report.md +++ b/plugins/permission-common/api-report.md @@ -11,6 +11,11 @@ export type AuthorizeRequest = { resourceRef?: string; }; +// @public +export type AuthorizeRequestEnvelope = { + items: Identified[]; +}; + // @public export type AuthorizeRequestOptions = { token?: string; @@ -26,6 +31,11 @@ export type AuthorizeResponse = conditions: PermissionCriteria; }; +// @public +export type AuthorizeResponseEnvelope = { + items: Identified[]; +}; + // @public export enum AuthorizeResult { ALLOW = 'ALLOW', diff --git a/plugins/permission-common/src/PermissionClient.test.ts b/plugins/permission-common/src/PermissionClient.test.ts index a432735a5e..0cd6a09af9 100644 --- a/plugins/permission-common/src/PermissionClient.test.ts +++ b/plugins/permission-common/src/PermissionClient.test.ts @@ -54,12 +54,14 @@ describe('PermissionClient', () => { describe('authorize', () => { const mockAuthorizeHandler = jest.fn((req, res, { json }: RestContext) => { - const responses = req.body.map((a: Identified) => ({ - id: a.id, - result: AuthorizeResult.ALLOW, - })); + const responses = req.body.items.map( + (a: Identified) => ({ + id: a.id, + result: AuthorizeResult.ALLOW, + }), + ); - return res(json(responses)); + return res(json({ items: responses })); }); beforeEach(() => { @@ -79,12 +81,15 @@ describe('PermissionClient', () => { await client.authorize([mockAuthorizeRequest]); const request = mockAuthorizeHandler.mock.calls[0][0]; - expect(request.body[0]).toEqual( - expect.objectContaining({ - permission: mockPermission, - resourceRef: 'foo', - }), - ); + + expect(request.body).toEqual({ + items: [ + expect.objectContaining({ + permission: mockPermission, + resourceRef: 'foo', + }), + ], + }); }); it('should return the response from the fetch request', async () => { @@ -122,7 +127,11 @@ describe('PermissionClient', () => { it('should reject responses with missing ids', async () => { mockAuthorizeHandler.mockImplementationOnce( (_req, res, { json }: RestContext) => { - return res(json([{ id: 'wrong-id', result: AuthorizeResult.ALLOW }])); + return res( + json({ + items: [{ id: 'wrong-id', result: AuthorizeResult.ALLOW }], + }), + ); }, ); await expect( @@ -133,12 +142,14 @@ describe('PermissionClient', () => { it('should reject invalid responses', async () => { mockAuthorizeHandler.mockImplementationOnce( (req, res, { json }: RestContext) => { - const responses = req.body.map((a: Identified) => ({ - id: a.id, - outcome: AuthorizeResult.ALLOW, - })); + const responses = req.body.items.map( + (a: Identified) => ({ + id: a.id, + outcome: AuthorizeResult.ALLOW, + }), + ); - return res(json(responses)); + return res(json({ items: responses })); }, ); await expect( @@ -151,10 +162,10 @@ describe('PermissionClient', () => { (req, res, { json }: RestContext) => { const responses = req.body.map((a: Identified) => ({ id: a.id, - outcome: AuthorizeResult.DENY, + result: AuthorizeResult.DENY, })); - return res(json(responses)); + return res(json({ items: responses })); }, ); const disabled = new PermissionClient({ diff --git a/plugins/permission-common/src/PermissionClient.ts b/plugins/permission-common/src/PermissionClient.ts index 62e5ec8172..6e45bdc072 100644 --- a/plugins/permission-common/src/PermissionClient.ts +++ b/plugins/permission-common/src/PermissionClient.ts @@ -26,6 +26,8 @@ import { Identified, PermissionCriteria, PermissionCondition, + AuthorizeResponseEnvelope, + AuthorizeRequestEnvelope, } from './types/api'; import { DiscoveryApi } from './types/discovery'; import { @@ -46,22 +48,24 @@ const permissionCriteriaSchema: z.ZodSchema< .or(z.object({ not: permissionCriteriaSchema })), ); -const responseSchema = z.array( - z - .object({ - id: z.string(), - result: z - .literal(AuthorizeResult.ALLOW) - .or(z.literal(AuthorizeResult.DENY)), - }) - .or( - z.object({ +const responseSchema = z.object({ + items: z.array( + z + .object({ id: z.string(), - result: z.literal(AuthorizeResult.CONDITIONAL), - conditions: permissionCriteriaSchema, - }), - ), -); + result: z + .literal(AuthorizeResult.ALLOW) + .or(z.literal(AuthorizeResult.DENY)), + }) + .or( + z.object({ + id: z.string(), + result: z.literal(AuthorizeResult.CONDITIONAL), + conditions: permissionCriteriaSchema, + }), + ), + ), +}); /** * An isomorphic client for requesting authorization for Backstage permissions. @@ -106,17 +110,17 @@ export class PermissionClient implements PermissionAuthorizer { return requests.map(_ => ({ result: AuthorizeResult.ALLOW })); } - const identifiedRequests: Identified[] = requests.map( - request => ({ + const requestEnvelope: AuthorizeRequestEnvelope = { + items: requests.map(request => ({ id: uuid.v4(), ...request, - }), - ); + })), + }; const permissionApi = await this.discovery.getBaseUrl('permission'); const response = await fetch(`${permissionApi}/authorize`, { method: 'POST', - body: JSON.stringify(identifiedRequests), + body: JSON.stringify(requestEnvelope), headers: { ...this.getAuthorizationHeader(options?.token), 'content-type': 'application/json', @@ -126,15 +130,15 @@ export class PermissionClient implements PermissionAuthorizer { throw await ResponseError.fromResponse(response); } - const identifiedResponses = await response.json(); - this.assertValidResponses(identifiedRequests, identifiedResponses); + const responseEnvelope = await response.json(); + this.assertValidResponses(requestEnvelope, responseEnvelope); - const responsesById = identifiedResponses.reduce((acc, r) => { + const responsesById = responseEnvelope.items.reduce((acc, r) => { acc[r.id] = r; return acc; }, {} as Record>); - return identifiedRequests.map(request => responsesById[request.id]); + return requestEnvelope.items.map(request => responsesById[request.id]); } private getAuthorizationHeader(token?: string): Record { @@ -142,12 +146,14 @@ export class PermissionClient implements PermissionAuthorizer { } private assertValidResponses( - requests: Identified[], + requestEnvelope: AuthorizeRequestEnvelope, json: any, - ): asserts json is Identified[] { + ): asserts json is AuthorizeResponseEnvelope { const authorizedResponses = responseSchema.parse(json); - const responseIds = authorizedResponses.map(r => r.id); - const hasAllRequestIds = requests.every(r => responseIds.includes(r.id)); + const responseIds = authorizedResponses.items.map(r => r.id); + const hasAllRequestIds = requestEnvelope.items.every(r => + responseIds.includes(r.id), + ); if (!hasAllRequestIds) { throw new Error( 'Unexpected authorization response from permission-backend', diff --git a/plugins/permission-common/src/types/api.ts b/plugins/permission-common/src/types/api.ts index 7f4cd0730b..abdae021d0 100644 --- a/plugins/permission-common/src/types/api.ts +++ b/plugins/permission-common/src/types/api.ts @@ -43,7 +43,7 @@ export enum AuthorizeResult { } /** - * An authorization request for {@link PermissionClient#authorize}. + * An individual authorization request for {@link PermissionClient#authorize}. * @public */ export type AuthorizeRequest = { @@ -51,6 +51,14 @@ export type AuthorizeRequest = { resourceRef?: string; }; +/** + * A batch of authorization requests from {@link PermissionClient#authorize}. + * @public + */ +export type AuthorizeRequestEnvelope = { + items: Identified[]; +}; + /** * A condition returned with a CONDITIONAL authorization response. * @@ -75,7 +83,7 @@ export type PermissionCriteria = | TQuery; /** - * An authorization response from {@link PermissionClient#authorize}. + * An individual authorization response from {@link PermissionClient#authorize}. * @public */ export type AuthorizeResponse = @@ -84,3 +92,11 @@ export type AuthorizeResponse = result: AuthorizeResult.CONDITIONAL; conditions: PermissionCriteria; }; + +/** + * A batch of authorization responses from {@link PermissionClient#authorize}. + * @public + */ +export type AuthorizeResponseEnvelope = { + items: Identified[]; +}; diff --git a/plugins/permission-common/src/types/index.ts b/plugins/permission-common/src/types/index.ts index 583a3577bd..9fbe911941 100644 --- a/plugins/permission-common/src/types/index.ts +++ b/plugins/permission-common/src/types/index.ts @@ -17,7 +17,9 @@ export { AuthorizeResult } from './api'; export type { AuthorizeRequest, + AuthorizeRequestEnvelope, AuthorizeResponse, + AuthorizeResponseEnvelope, Identified, PermissionCondition, PermissionCriteria, diff --git a/plugins/permission-node/src/ServerPermissionClient.test.ts b/plugins/permission-node/src/ServerPermissionClient.test.ts index 2c29ef8939..4864aa4606 100644 --- a/plugins/permission-node/src/ServerPermissionClient.test.ts +++ b/plugins/permission-node/src/ServerPermissionClient.test.ts @@ -32,12 +32,12 @@ import { RestContext, rest } from 'msw'; const server = setupServer(); const mockAuthorizeHandler = jest.fn((req, res, { json }: RestContext) => { - const responses = req.body.map((r: Identified) => ({ + const responses = req.body.items.map((r: Identified) => ({ id: r.id, result: AuthorizeResult.ALLOW, })); - return res(json(responses)); + return res(json({ items: responses })); }); const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; const discovery: PluginEndpointDiscovery = { From 0ae4f4cc82dc0303fac95fd06cdb9cad7d56e8ab Mon Sep 17 00:00:00 2001 From: MT Lewis Date: Thu, 13 Jan 2022 13:39:15 +0000 Subject: [PATCH 2/3] permissions: rename authorize request and response types to avoid envelope suffix Signed-off-by: MT Lewis --- .changeset/afraid-gorillas-beg.md | 5 ++ .changeset/chilled-cats-marry.md | 2 + .changeset/pink-actors-poke.md | 6 ++ .../permission-backend/src/service/router.ts | 56 +++++++++---------- plugins/permission-common/api-report.md | 46 +++++++-------- .../src/PermissionClient.test.ts | 40 +++++++------ .../permission-common/src/PermissionClient.ts | 40 ++++++------- plugins/permission-common/src/types/api.ts | 12 ++-- plugins/permission-common/src/types/index.ts | 4 +- .../permission-common/src/types/permission.ts | 6 +- plugins/permission-node/api-report.md | 12 ++-- .../src/ServerPermissionClient.test.ts | 4 +- .../src/ServerPermissionClient.ts | 12 ++-- plugins/permission-node/src/policy/index.ts | 2 +- plugins/permission-node/src/policy/types.ts | 10 ++-- plugins/permission-node/src/types.ts | 2 +- plugins/permission-react/api-report.md | 8 +-- .../src/apis/IdentityPermissionApi.ts | 6 +- .../src/apis/PermissionApi.ts | 6 +- 19 files changed, 145 insertions(+), 134 deletions(-) create mode 100644 .changeset/afraid-gorillas-beg.md create mode 100644 .changeset/pink-actors-poke.md diff --git a/.changeset/afraid-gorillas-beg.md b/.changeset/afraid-gorillas-beg.md new file mode 100644 index 0000000000..fdaf03502a --- /dev/null +++ b/.changeset/afraid-gorillas-beg.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-react': minor +--- + +**BREAKING**: Update to use renamed request and response types from @backstage/plugin-permission-common. diff --git a/.changeset/chilled-cats-marry.md b/.changeset/chilled-cats-marry.md index 2a91c2bc99..9c7617de33 100644 --- a/.changeset/chilled-cats-marry.md +++ b/.changeset/chilled-cats-marry.md @@ -2,4 +2,6 @@ '@backstage/plugin-permission-common': minor --- +**BREAKING**: Authorize API request and response types have been updated. The existing `AuthorizeRequest` and `AuthorizeResponse` types now match the entire request and response objects for the /authorize endpoint, and new types `AuthorizeQuery` and `AuthorizeDecision` have been introduced for individual items in the request and response batches respectively. + **BREAKING**: PermissionClient has been updated to use the new request and response format in the latest version of @backstage/permission-backend. diff --git a/.changeset/pink-actors-poke.md b/.changeset/pink-actors-poke.md new file mode 100644 index 0000000000..e35a925e8c --- /dev/null +++ b/.changeset/pink-actors-poke.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-permission-node': minor +--- + +**BREAKING**: `PolicyAuthorizeRequest` type has been renamed to `PolicyAuthorizeQuery`. +**BREAKING**: Update to use renamed request and response types from @backstage/plugin-permission-common. diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index c504fdbe3a..e22aec8129 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -29,11 +29,11 @@ import { } from '@backstage/plugin-auth-backend'; import { AuthorizeResult, - AuthorizeResponse, - AuthorizeRequest, + AuthorizeDecision, + AuthorizeQuery, Identified, - AuthorizeRequestEnvelope, - AuthorizeResponseEnvelope, + AuthorizeRequest, + AuthorizeResponse, } from '@backstage/plugin-permission-common'; import { ApplyConditionsRequestEntry, @@ -44,27 +44,27 @@ import { PermissionIntegrationClient } from './PermissionIntegrationClient'; import { memoize } from 'lodash'; import DataLoader from 'dataloader'; -const requestSchema: z.ZodSchema = z.object({ - items: z.array( - z.object({ - id: z.string(), - resourceRef: z.string().optional(), - permission: z.object({ - name: z.string(), - resourceType: z.string().optional(), - attributes: z.object({ - action: z - .union([ - z.literal('create'), - z.literal('read'), - z.literal('update'), - z.literal('delete'), - ]) - .optional(), - }), - }), +const querySchema: z.ZodSchema> = z.object({ + id: z.string(), + resourceRef: z.string().optional(), + permission: z.object({ + name: z.string(), + resourceType: z.string().optional(), + attributes: z.object({ + action: z + .union([ + z.literal('create'), + z.literal('read'), + z.literal('update'), + z.literal('delete'), + ]) + .optional(), }), - ), + }), +}); + +const requestSchema: z.ZodSchema = z.object({ + items: z.array(querySchema), }); /** @@ -81,12 +81,12 @@ export interface RouterOptions { } const handleRequest = async ( - requests: Identified[], + requests: Identified[], user: BackstageIdentityResponse | undefined, policy: PermissionPolicy, permissionIntegrationClient: PermissionIntegrationClient, authHeader?: string, -): Promise[]> => { +): Promise[]> => { const applyConditionsLoaderFor = memoize((pluginId: string) => { return new DataLoader< ApplyConditionsRequestEntry, @@ -154,8 +154,8 @@ export async function createRouter( router.post( '/authorize', async ( - req: Request, - res: Response, + req: Request, + res: Response, ) => { const token = IdentityClient.getBearerToken(req.header('authorization')); const user = token ? await identity.authenticate(token) : undefined; diff --git a/plugins/permission-common/api-report.md b/plugins/permission-common/api-report.md index 3c681cc46f..30a67e8c8b 100644 --- a/plugins/permission-common/api-report.md +++ b/plugins/permission-common/api-report.md @@ -6,23 +6,7 @@ import { Config } from '@backstage/config'; // @public -export type AuthorizeRequest = { - permission: Permission; - resourceRef?: string; -}; - -// @public -export type AuthorizeRequestEnvelope = { - items: Identified[]; -}; - -// @public -export type AuthorizeRequestOptions = { - token?: string; -}; - -// @public -export type AuthorizeResponse = +export type AuthorizeDecision = | { result: AuthorizeResult.ALLOW | AuthorizeResult.DENY; } @@ -32,8 +16,24 @@ export type AuthorizeResponse = }; // @public -export type AuthorizeResponseEnvelope = { - items: Identified[]; +export type AuthorizeQuery = { + permission: Permission; + resourceRef?: string; +}; + +// @public +export type AuthorizeRequest = { + items: Identified[]; +}; + +// @public +export type AuthorizeRequestOptions = { + token?: string; +}; + +// @public +export type AuthorizeResponse = { + items: Identified[]; }; // @public @@ -81,18 +81,18 @@ export type PermissionAttributes = { export interface PermissionAuthorizer { // (undocumented) authorize( - requests: AuthorizeRequest[], + queries: AuthorizeQuery[], options?: AuthorizeRequestOptions, - ): Promise; + ): Promise; } // @public export class PermissionClient implements PermissionAuthorizer { constructor(options: { discovery: DiscoveryApi; config: Config }); authorize( - requests: AuthorizeRequest[], + queries: AuthorizeQuery[], options?: AuthorizeRequestOptions, - ): Promise; + ): Promise; } // @public diff --git a/plugins/permission-common/src/PermissionClient.test.ts b/plugins/permission-common/src/PermissionClient.test.ts index 0cd6a09af9..0e57f618b7 100644 --- a/plugins/permission-common/src/PermissionClient.test.ts +++ b/plugins/permission-common/src/PermissionClient.test.ts @@ -18,7 +18,7 @@ import { RestContext, rest } from 'msw'; import { setupServer } from 'msw/node'; import { ConfigReader } from '@backstage/config'; import { PermissionClient } from './PermissionClient'; -import { AuthorizeRequest, AuthorizeResult, Identified } from './types/api'; +import { AuthorizeQuery, AuthorizeResult, Identified } from './types/api'; import { DiscoveryApi } from './types/discovery'; import { Permission } from './types/permission'; @@ -42,7 +42,7 @@ const mockPermission: Permission = { resourceType: 'test-resource', }; -const mockAuthorizeRequest = { +const mockAuthorizeQuery = { permission: mockPermission, resourceRef: 'foo', }; @@ -54,12 +54,10 @@ describe('PermissionClient', () => { describe('authorize', () => { const mockAuthorizeHandler = jest.fn((req, res, { json }: RestContext) => { - const responses = req.body.items.map( - (a: Identified) => ({ - id: a.id, - result: AuthorizeResult.ALLOW, - }), - ); + const responses = req.body.items.map((a: Identified) => ({ + id: a.id, + result: AuthorizeResult.ALLOW, + })); return res(json({ items: responses })); }); @@ -73,12 +71,12 @@ describe('PermissionClient', () => { }); it('should fetch entities from correct endpoint', async () => { - await client.authorize([mockAuthorizeRequest]); + await client.authorize([mockAuthorizeQuery]); expect(mockAuthorizeHandler).toHaveBeenCalled(); }); it('should include a request body', async () => { - await client.authorize([mockAuthorizeRequest]); + await client.authorize([mockAuthorizeQuery]); const request = mockAuthorizeHandler.mock.calls[0][0]; @@ -93,21 +91,21 @@ describe('PermissionClient', () => { }); it('should return the response from the fetch request', async () => { - const response = await client.authorize([mockAuthorizeRequest]); + const response = await client.authorize([mockAuthorizeQuery]); expect(response[0]).toEqual( expect.objectContaining({ result: AuthorizeResult.ALLOW }), ); }); it('should not include authorization headers if no token is supplied', async () => { - await client.authorize([mockAuthorizeRequest]); + await client.authorize([mockAuthorizeQuery]); const request = mockAuthorizeHandler.mock.calls[0][0]; expect(request.headers.has('authorization')).toEqual(false); }); it('should include correctly-constructed authorization header if token is supplied', async () => { - await client.authorize([mockAuthorizeRequest], { token }); + await client.authorize([mockAuthorizeQuery], { token }); const request = mockAuthorizeHandler.mock.calls[0][0]; expect(request.headers.get('authorization')).toEqual('Bearer fake-token'); @@ -120,7 +118,7 @@ describe('PermissionClient', () => { }, ); await expect( - client.authorize([mockAuthorizeRequest], { token }), + client.authorize([mockAuthorizeQuery], { token }), ).rejects.toThrowError(/request failed with 401/i); }); @@ -135,7 +133,7 @@ describe('PermissionClient', () => { }, ); await expect( - client.authorize([mockAuthorizeRequest], { token }), + client.authorize([mockAuthorizeQuery], { token }), ).rejects.toThrowError(/Unexpected authorization response/i); }); @@ -143,7 +141,7 @@ describe('PermissionClient', () => { mockAuthorizeHandler.mockImplementationOnce( (req, res, { json }: RestContext) => { const responses = req.body.items.map( - (a: Identified) => ({ + (a: Identified) => ({ id: a.id, outcome: AuthorizeResult.ALLOW, }), @@ -153,14 +151,14 @@ describe('PermissionClient', () => { }, ); await expect( - client.authorize([mockAuthorizeRequest], { token }), + client.authorize([mockAuthorizeQuery], { token }), ).rejects.toThrowError(/invalid input/i); }); it('should allow all when permission.enabled is false', async () => { mockAuthorizeHandler.mockImplementationOnce( (req, res, { json }: RestContext) => { - const responses = req.body.map((a: Identified) => ({ + const responses = req.body.map((a: Identified) => ({ id: a.id, result: AuthorizeResult.DENY, })); @@ -172,7 +170,7 @@ describe('PermissionClient', () => { discovery, config: new ConfigReader({ permission: { enabled: false } }), }); - const response = await disabled.authorize([mockAuthorizeRequest]); + const response = await disabled.authorize([mockAuthorizeQuery]); expect(response[0]).toEqual( expect.objectContaining({ result: AuthorizeResult.ALLOW }), ); @@ -182,7 +180,7 @@ describe('PermissionClient', () => { it('should allow all when permission.enabled is not configured', async () => { mockAuthorizeHandler.mockImplementationOnce( (req, res, { json }: RestContext) => { - const responses = req.body.map((a: Identified) => ({ + const responses = req.body.map((a: Identified) => ({ id: a.id, outcome: AuthorizeResult.DENY, })); @@ -194,7 +192,7 @@ describe('PermissionClient', () => { discovery, config: new ConfigReader({}), }); - const response = await disabled.authorize([mockAuthorizeRequest]); + const response = await disabled.authorize([mockAuthorizeQuery]); expect(response[0]).toEqual( expect.objectContaining({ result: AuthorizeResult.ALLOW }), ); diff --git a/plugins/permission-common/src/PermissionClient.ts b/plugins/permission-common/src/PermissionClient.ts index 6e45bdc072..58f4d44d20 100644 --- a/plugins/permission-common/src/PermissionClient.ts +++ b/plugins/permission-common/src/PermissionClient.ts @@ -21,13 +21,13 @@ import * as uuid from 'uuid'; import { z } from 'zod'; import { AuthorizeResult, - AuthorizeRequest, - AuthorizeResponse, + AuthorizeQuery, + AuthorizeDecision, Identified, PermissionCriteria, PermissionCondition, - AuthorizeResponseEnvelope, - AuthorizeRequestEnvelope, + AuthorizeResponse, + AuthorizeRequest, } from './types/api'; import { DiscoveryApi } from './types/discovery'; import { @@ -98,29 +98,29 @@ export class PermissionClient implements PermissionAuthorizer { * @public */ async authorize( - requests: AuthorizeRequest[], + queries: AuthorizeQuery[], options?: AuthorizeRequestOptions, - ): Promise { + ): Promise { // TODO(permissions): it would be great to provide some kind of typing guarantee that // conditional responses will only ever be returned for requests containing a resourceType // but no resourceRef. That way clients who aren't prepared to handle filtering according // to conditions can be guaranteed that they won't unexpectedly get a CONDITIONAL response. if (!this.enabled) { - return requests.map(_ => ({ result: AuthorizeResult.ALLOW })); + return queries.map(_ => ({ result: AuthorizeResult.ALLOW })); } - const requestEnvelope: AuthorizeRequestEnvelope = { - items: requests.map(request => ({ + const request: AuthorizeRequest = { + items: queries.map(query => ({ id: uuid.v4(), - ...request, + ...query, })), }; const permissionApi = await this.discovery.getBaseUrl('permission'); const response = await fetch(`${permissionApi}/authorize`, { method: 'POST', - body: JSON.stringify(requestEnvelope), + body: JSON.stringify(request), headers: { ...this.getAuthorizationHeader(options?.token), 'content-type': 'application/json', @@ -130,28 +130,28 @@ export class PermissionClient implements PermissionAuthorizer { throw await ResponseError.fromResponse(response); } - const responseEnvelope = await response.json(); - this.assertValidResponses(requestEnvelope, responseEnvelope); + const responseBody = await response.json(); + this.assertValidResponse(request, responseBody); - const responsesById = responseEnvelope.items.reduce((acc, r) => { + const responsesById = responseBody.items.reduce((acc, r) => { acc[r.id] = r; return acc; - }, {} as Record>); + }, {} as Record>); - return requestEnvelope.items.map(request => responsesById[request.id]); + return request.items.map(query => responsesById[query.id]); } private getAuthorizationHeader(token?: string): Record { return token ? { Authorization: `Bearer ${token}` } : {}; } - private assertValidResponses( - requestEnvelope: AuthorizeRequestEnvelope, + private assertValidResponse( + request: AuthorizeRequest, json: any, - ): asserts json is AuthorizeResponseEnvelope { + ): asserts json is AuthorizeResponse { const authorizedResponses = responseSchema.parse(json); const responseIds = authorizedResponses.items.map(r => r.id); - const hasAllRequestIds = requestEnvelope.items.every(r => + const hasAllRequestIds = request.items.every(r => responseIds.includes(r.id), ); if (!hasAllRequestIds) { diff --git a/plugins/permission-common/src/types/api.ts b/plugins/permission-common/src/types/api.ts index abdae021d0..287c7ffe83 100644 --- a/plugins/permission-common/src/types/api.ts +++ b/plugins/permission-common/src/types/api.ts @@ -46,7 +46,7 @@ export enum AuthorizeResult { * An individual authorization request for {@link PermissionClient#authorize}. * @public */ -export type AuthorizeRequest = { +export type AuthorizeQuery = { permission: Permission; resourceRef?: string; }; @@ -55,8 +55,8 @@ export type AuthorizeRequest = { * A batch of authorization requests from {@link PermissionClient#authorize}. * @public */ -export type AuthorizeRequestEnvelope = { - items: Identified[]; +export type AuthorizeRequest = { + items: Identified[]; }; /** @@ -86,7 +86,7 @@ export type PermissionCriteria = * An individual authorization response from {@link PermissionClient#authorize}. * @public */ -export type AuthorizeResponse = +export type AuthorizeDecision = | { result: AuthorizeResult.ALLOW | AuthorizeResult.DENY } | { result: AuthorizeResult.CONDITIONAL; @@ -97,6 +97,6 @@ export type AuthorizeResponse = * A batch of authorization responses from {@link PermissionClient#authorize}. * @public */ -export type AuthorizeResponseEnvelope = { - items: Identified[]; +export type AuthorizeResponse = { + items: Identified[]; }; diff --git a/plugins/permission-common/src/types/index.ts b/plugins/permission-common/src/types/index.ts index 9fbe911941..b4453fcc63 100644 --- a/plugins/permission-common/src/types/index.ts +++ b/plugins/permission-common/src/types/index.ts @@ -16,10 +16,10 @@ export { AuthorizeResult } from './api'; export type { + AuthorizeQuery, AuthorizeRequest, - AuthorizeRequestEnvelope, + AuthorizeDecision, AuthorizeResponse, - AuthorizeResponseEnvelope, Identified, PermissionCondition, PermissionCriteria, diff --git a/plugins/permission-common/src/types/permission.ts b/plugins/permission-common/src/types/permission.ts index 246f1bec72..805aefa3de 100644 --- a/plugins/permission-common/src/types/permission.ts +++ b/plugins/permission-common/src/types/permission.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { AuthorizeRequest, AuthorizeResponse } from './api'; +import { AuthorizeQuery, AuthorizeDecision } from './api'; /** * The attributes related to a given permission; these should be generic and widely applicable to @@ -48,9 +48,9 @@ export type Permission = { */ export interface PermissionAuthorizer { authorize( - requests: AuthorizeRequest[], + queries: AuthorizeQuery[], options?: AuthorizeRequestOptions, - ): Promise; + ): Promise; } /** diff --git a/plugins/permission-node/api-report.md b/plugins/permission-node/api-report.md index 71685c59b1..43bb548a06 100644 --- a/plugins/permission-node/api-report.md +++ b/plugins/permission-node/api-report.md @@ -3,9 +3,9 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts -import { AuthorizeRequest } from '@backstage/plugin-permission-common'; +import { AuthorizeDecision } from '@backstage/plugin-permission-common'; +import { AuthorizeQuery } from '@backstage/plugin-permission-common'; import { AuthorizeRequestOptions } from '@backstage/plugin-permission-common'; -import { AuthorizeResponse } from '@backstage/plugin-permission-common'; import { AuthorizeResult } from '@backstage/plugin-permission-common'; import { BackstageIdentityResponse } from '@backstage/plugin-auth-backend'; import { Config } from '@backstage/config'; @@ -129,7 +129,7 @@ export const makeCreatePermissionRule: () => < export interface PermissionPolicy { // (undocumented) handle( - request: PolicyAuthorizeRequest, + request: PolicyAuthorizeQuery, user?: BackstageIdentityResponse, ): Promise; } @@ -147,7 +147,7 @@ export type PermissionRule< }; // @public -export type PolicyAuthorizeRequest = Omit; +export type PolicyAuthorizeQuery = Omit; // @public export type PolicyDecision = @@ -158,9 +158,9 @@ export type PolicyDecision = export class ServerPermissionClient implements PermissionAuthorizer { // (undocumented) authorize( - requests: AuthorizeRequest[], + queries: AuthorizeQuery[], options?: AuthorizeRequestOptions, - ): Promise; + ): Promise; // (undocumented) static fromConfig( config: Config, diff --git a/plugins/permission-node/src/ServerPermissionClient.test.ts b/plugins/permission-node/src/ServerPermissionClient.test.ts index 4864aa4606..9fa1eadb5e 100644 --- a/plugins/permission-node/src/ServerPermissionClient.test.ts +++ b/plugins/permission-node/src/ServerPermissionClient.test.ts @@ -18,7 +18,7 @@ import { ServerPermissionClient } from './ServerPermissionClient'; import { Permission, Identified, - AuthorizeRequest, + AuthorizeQuery, AuthorizeResult, } from '@backstage/plugin-permission-common'; import { ConfigReader } from '@backstage/config'; @@ -32,7 +32,7 @@ import { RestContext, rest } from 'msw'; const server = setupServer(); const mockAuthorizeHandler = jest.fn((req, res, { json }: RestContext) => { - const responses = req.body.items.map((r: Identified) => ({ + const responses = req.body.items.map((r: Identified) => ({ id: r.id, result: AuthorizeResult.ALLOW, })); diff --git a/plugins/permission-node/src/ServerPermissionClient.ts b/plugins/permission-node/src/ServerPermissionClient.ts index c85cc7048b..f22779c8e8 100644 --- a/plugins/permission-node/src/ServerPermissionClient.ts +++ b/plugins/permission-node/src/ServerPermissionClient.ts @@ -20,9 +20,9 @@ import { } from '@backstage/backend-common'; import { Config } from '@backstage/config'; import { - AuthorizeRequest, + AuthorizeQuery, AuthorizeRequestOptions, - AuthorizeResponse, + AuthorizeDecision, AuthorizeResult, PermissionClient, PermissionAuthorizer, @@ -78,9 +78,9 @@ export class ServerPermissionClient implements PermissionAuthorizer { } async authorize( - requests: AuthorizeRequest[], + queries: AuthorizeQuery[], options?: AuthorizeRequestOptions, - ): Promise { + ): Promise { // Check if permissions are enabled before validating the server token. That // way when permissions are disabled, the noop token manager can be used // without fouling up the logic inside the ServerPermissionClient, because @@ -89,9 +89,9 @@ export class ServerPermissionClient implements PermissionAuthorizer { !this.permissionEnabled || (await this.isValidServerToken(options?.token)) ) { - return requests.map(_ => ({ result: AuthorizeResult.ALLOW })); + return queries.map(_ => ({ result: AuthorizeResult.ALLOW })); } - return this.permissionClient.authorize(requests, options); + return this.permissionClient.authorize(queries, options); } private async isValidServerToken( diff --git a/plugins/permission-node/src/policy/index.ts b/plugins/permission-node/src/policy/index.ts index 1b05f240d3..1d3fc4a737 100644 --- a/plugins/permission-node/src/policy/index.ts +++ b/plugins/permission-node/src/policy/index.ts @@ -18,6 +18,6 @@ export type { ConditionalPolicyDecision, DefinitivePolicyDecision, PermissionPolicy, - PolicyAuthorizeRequest, + PolicyAuthorizeQuery, PolicyDecision, } from './types'; diff --git a/plugins/permission-node/src/policy/types.ts b/plugins/permission-node/src/policy/types.ts index 4c6a033e11..2e344d6d96 100644 --- a/plugins/permission-node/src/policy/types.ts +++ b/plugins/permission-node/src/policy/types.ts @@ -15,7 +15,7 @@ */ import { - AuthorizeRequest, + AuthorizeQuery, AuthorizeResult, PermissionCondition, PermissionCriteria, @@ -27,13 +27,13 @@ import { BackstageIdentityResponse } from '@backstage/plugin-auth-backend'; * * @remarks * - * This differs from {@link @backstage/permission-common#AuthorizeRequest} in that `resourceRef` + * This differs from {@link @backstage/permission-common#AuthorizeQuery} in that `resourceRef` * should never be provided. This forces policies to be written in a way that's compatible with * filtering collections of resources at data load time. * * @public */ -export type PolicyAuthorizeRequest = Omit; +export type PolicyAuthorizeQuery = Omit; /** * A definitive result to an authorization request, returned by the {@link PermissionPolicy}. @@ -57,7 +57,7 @@ export type DefinitivePolicyDecision = { * conditions hold when evaluated. The conditions will be evaluated by the corresponding plugin * which knows about the referenced permission rules. * - * Similar to {@link @backstage/permission-common#AuthorizeResult}, but with the plugin and resource + * Similar to {@link @backstage/permission-common#AuthorizeDecision}, but with the plugin and resource * identifiers needed to evaluate the returned conditions. * @public */ @@ -95,7 +95,7 @@ export type PolicyDecision = */ export interface PermissionPolicy { handle( - request: PolicyAuthorizeRequest, + request: PolicyAuthorizeQuery, user?: BackstageIdentityResponse, ): Promise; } diff --git a/plugins/permission-node/src/types.ts b/plugins/permission-node/src/types.ts index 678befc99c..d5035f10dc 100644 --- a/plugins/permission-node/src/types.ts +++ b/plugins/permission-node/src/types.ts @@ -18,7 +18,7 @@ import type { PermissionCriteria } from '@backstage/plugin-permission-common'; /** * A conditional rule that can be provided in an - * {@link @backstage/permission-common#AuthorizeResult} response to an authorization request. + * {@link @backstage/permission-common#AuthorizeDecision} response to an authorization request. * * @remarks * diff --git a/plugins/permission-react/api-report.md b/plugins/permission-react/api-report.md index 799b3ad54f..1bcb01fa32 100644 --- a/plugins/permission-react/api-report.md +++ b/plugins/permission-react/api-report.md @@ -4,8 +4,8 @@ ```ts import { ApiRef } from '@backstage/core-plugin-api'; -import { AuthorizeRequest } from '@backstage/plugin-permission-common'; -import { AuthorizeResponse } from '@backstage/plugin-permission-common'; +import { AuthorizeDecision } from '@backstage/plugin-permission-common'; +import { AuthorizeQuery } from '@backstage/plugin-permission-common'; import { ComponentProps } from 'react'; import { Config } from '@backstage/config'; import { DiscoveryApi } from '@backstage/core-plugin-api'; @@ -24,7 +24,7 @@ export type AsyncPermissionResult = { // @public export class IdentityPermissionApi implements PermissionApi { // (undocumented) - authorize(request: AuthorizeRequest): Promise; + authorize(request: AuthorizeQuery): Promise; // (undocumented) static create(options: { config: Config; @@ -35,7 +35,7 @@ export class IdentityPermissionApi implements PermissionApi { // @public export type PermissionApi = { - authorize(request: AuthorizeRequest): Promise; + authorize(request: AuthorizeQuery): Promise; }; // @public diff --git a/plugins/permission-react/src/apis/IdentityPermissionApi.ts b/plugins/permission-react/src/apis/IdentityPermissionApi.ts index 818eca17fa..7d126c56cb 100644 --- a/plugins/permission-react/src/apis/IdentityPermissionApi.ts +++ b/plugins/permission-react/src/apis/IdentityPermissionApi.ts @@ -17,8 +17,8 @@ import { DiscoveryApi, IdentityApi } from '@backstage/core-plugin-api'; import { PermissionApi } from './PermissionApi'; import { - AuthorizeRequest, - AuthorizeResponse, + AuthorizeQuery, + AuthorizeDecision, PermissionClient, } from '@backstage/plugin-permission-common'; import { Config } from '@backstage/config'; @@ -44,7 +44,7 @@ export class IdentityPermissionApi implements PermissionApi { return new IdentityPermissionApi(permissionClient, identity); } - async authorize(request: AuthorizeRequest): Promise { + async authorize(request: AuthorizeQuery): Promise { const response = await this.permissionClient.authorize([request], { token: await this.identityApi.getIdToken(), }); diff --git a/plugins/permission-react/src/apis/PermissionApi.ts b/plugins/permission-react/src/apis/PermissionApi.ts index 5c224ff06d..b17350c38e 100644 --- a/plugins/permission-react/src/apis/PermissionApi.ts +++ b/plugins/permission-react/src/apis/PermissionApi.ts @@ -15,8 +15,8 @@ */ import { - AuthorizeRequest, - AuthorizeResponse, + AuthorizeQuery, + AuthorizeDecision, } from '@backstage/plugin-permission-common'; import { ApiRef, createApiRef } from '@backstage/core-plugin-api'; @@ -27,7 +27,7 @@ import { ApiRef, createApiRef } from '@backstage/core-plugin-api'; * @public */ export type PermissionApi = { - authorize(request: AuthorizeRequest): Promise; + authorize(request: AuthorizeQuery): Promise; }; /** From 12f5c330430ae5d9e8de26856aeee23bf3135f71 Mon Sep 17 00:00:00 2001 From: MT Lewis Date: Thu, 13 Jan 2022 13:30:08 +0000 Subject: [PATCH 3/3] catalog-backend: rename permission-related variable to match type Signed-off-by: MT Lewis --- .../src/service/AuthorizedEntitiesCatalog.ts | 8 ++++---- .../src/service/AuthorizedRefreshService.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts index 5b65070c43..270eb64a40 100644 --- a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts @@ -36,23 +36,23 @@ export class AuthorizedEntitiesCatalog implements EntitiesCatalog { ) {} async entities(request?: EntitiesRequest): Promise { - const authorizeResponse = ( + const authorizeDecision = ( await this.permissionApi.authorize( [{ permission: catalogEntityReadPermission }], { token: request?.authorizationToken }, ) )[0]; - if (authorizeResponse.result === AuthorizeResult.DENY) { + if (authorizeDecision.result === AuthorizeResult.DENY) { return { entities: [], pageInfo: { hasNextPage: false }, }; } - if (authorizeResponse.result === AuthorizeResult.CONDITIONAL) { + if (authorizeDecision.result === AuthorizeResult.CONDITIONAL) { const permissionFilter: EntityFilter = this.transformConditions( - authorizeResponse.conditions, + authorizeDecision.conditions, ); return this.entitiesCatalog.entities({ ...request, diff --git a/plugins/catalog-backend/src/service/AuthorizedRefreshService.ts b/plugins/catalog-backend/src/service/AuthorizedRefreshService.ts index 800bdcf1b9..819451d854 100644 --- a/plugins/catalog-backend/src/service/AuthorizedRefreshService.ts +++ b/plugins/catalog-backend/src/service/AuthorizedRefreshService.ts @@ -28,7 +28,7 @@ export class AuthorizedRefreshService implements RefreshService { ) {} async refresh(options: RefreshOptions) { - const authorizeResponse = ( + const authorizeDecision = ( await this.permissionApi.authorize( [ { @@ -39,7 +39,7 @@ export class AuthorizedRefreshService implements RefreshService { { token: options.authorizationToken }, ) )[0]; - if (authorizeResponse.result !== AuthorizeResult.ALLOW) { + if (authorizeDecision.result !== AuthorizeResult.ALLOW) { throw new NotAllowedError(); } await this.service.refresh(options);