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/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..9c7617de33 --- /dev/null +++ b/.changeset/chilled-cats-marry.md @@ -0,0 +1,7 @@ +--- +'@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/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); 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..e22aec8129 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -29,9 +29,11 @@ import { } from '@backstage/plugin-auth-backend'; import { AuthorizeResult, - AuthorizeResponse, - AuthorizeRequest, + AuthorizeDecision, + AuthorizeQuery, Identified, + AuthorizeRequest, + AuthorizeResponse, } 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 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), +}); /** * Options required when constructing a new {@link express#Router} using @@ -77,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, @@ -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..30a67e8c8b 100644 --- a/plugins/permission-common/api-report.md +++ b/plugins/permission-common/api-report.md @@ -6,25 +6,35 @@ import { Config } from '@backstage/config'; // @public -export type AuthorizeRequest = { +export type AuthorizeDecision = + | { + result: AuthorizeResult.ALLOW | AuthorizeResult.DENY; + } + | { + result: AuthorizeResult.CONDITIONAL; + conditions: PermissionCriteria; + }; + +// @public +export type AuthorizeQuery = { permission: Permission; resourceRef?: string; }; +// @public +export type AuthorizeRequest = { + items: Identified[]; +}; + // @public export type AuthorizeRequestOptions = { token?: string; }; // @public -export type AuthorizeResponse = - | { - result: AuthorizeResult.ALLOW | AuthorizeResult.DENY; - } - | { - result: AuthorizeResult.CONDITIONAL; - conditions: PermissionCriteria; - }; +export type AuthorizeResponse = { + items: Identified[]; +}; // @public export enum AuthorizeResult { @@ -71,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 a432735a5e..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,12 @@ describe('PermissionClient', () => { describe('authorize', () => { const mockAuthorizeHandler = jest.fn((req, res, { json }: RestContext) => { - const responses = req.body.map((a: Identified) => ({ + const responses = req.body.items.map((a: Identified) => ({ id: a.id, result: AuthorizeResult.ALLOW, })); - return res(json(responses)); + return res(json({ items: responses })); }); beforeEach(() => { @@ -71,38 +71,41 @@ 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]; - 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 () => { - 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'); @@ -115,53 +118,59 @@ describe('PermissionClient', () => { }, ); await expect( - client.authorize([mockAuthorizeRequest], { token }), + client.authorize([mockAuthorizeQuery], { token }), ).rejects.toThrowError(/request failed with 401/i); }); 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( - client.authorize([mockAuthorizeRequest], { token }), + client.authorize([mockAuthorizeQuery], { token }), ).rejects.toThrowError(/Unexpected authorization response/i); }); 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( - 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, - outcome: AuthorizeResult.DENY, + result: AuthorizeResult.DENY, })); - return res(json(responses)); + return res(json({ items: responses })); }, ); const disabled = new 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 }), ); @@ -171,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, })); @@ -183,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 62e5ec8172..58f4d44d20 100644 --- a/plugins/permission-common/src/PermissionClient.ts +++ b/plugins/permission-common/src/PermissionClient.ts @@ -21,11 +21,13 @@ import * as uuid from 'uuid'; import { z } from 'zod'; import { AuthorizeResult, - AuthorizeRequest, - AuthorizeResponse, + AuthorizeQuery, + AuthorizeDecision, Identified, PermissionCriteria, PermissionCondition, + AuthorizeResponse, + AuthorizeRequest, } 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. @@ -94,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 identifiedRequests: Identified[] = 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(identifiedRequests), + body: JSON.stringify(request), headers: { ...this.getAuthorizationHeader(options?.token), 'content-type': 'application/json', @@ -126,28 +130,30 @@ export class PermissionClient implements PermissionAuthorizer { throw await ResponseError.fromResponse(response); } - const identifiedResponses = await response.json(); - this.assertValidResponses(identifiedRequests, identifiedResponses); + const responseBody = await response.json(); + this.assertValidResponse(request, responseBody); - const responsesById = identifiedResponses.reduce((acc, r) => { + const responsesById = responseBody.items.reduce((acc, r) => { acc[r.id] = r; return acc; - }, {} as Record>); + }, {} as Record>); - return identifiedRequests.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( - requests: Identified[], + private assertValidResponse( + request: AuthorizeRequest, json: any, - ): asserts json is Identified[] { + ): asserts json is AuthorizeResponse { 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 = request.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..287c7ffe83 100644 --- a/plugins/permission-common/src/types/api.ts +++ b/plugins/permission-common/src/types/api.ts @@ -43,12 +43,20 @@ export enum AuthorizeResult { } /** - * An authorization request for {@link PermissionClient#authorize}. + * An individual authorization request for {@link PermissionClient#authorize}. + * @public + */ +export type AuthorizeQuery = { + permission: Permission; + resourceRef?: string; +}; + +/** + * A batch of authorization requests from {@link PermissionClient#authorize}. * @public */ export type AuthorizeRequest = { - permission: Permission; - resourceRef?: string; + items: Identified[]; }; /** @@ -75,12 +83,20 @@ export type PermissionCriteria = | TQuery; /** - * An authorization response from {@link PermissionClient#authorize}. + * An individual authorization response from {@link PermissionClient#authorize}. * @public */ -export type AuthorizeResponse = +export type AuthorizeDecision = | { result: AuthorizeResult.ALLOW | AuthorizeResult.DENY } | { result: AuthorizeResult.CONDITIONAL; conditions: PermissionCriteria; }; + +/** + * A batch of authorization responses from {@link PermissionClient#authorize}. + * @public + */ +export type AuthorizeResponse = { + items: Identified[]; +}; diff --git a/plugins/permission-common/src/types/index.ts b/plugins/permission-common/src/types/index.ts index 583a3577bd..b4453fcc63 100644 --- a/plugins/permission-common/src/types/index.ts +++ b/plugins/permission-common/src/types/index.ts @@ -16,7 +16,9 @@ export { AuthorizeResult } from './api'; export type { + AuthorizeQuery, AuthorizeRequest, + AuthorizeDecision, AuthorizeResponse, Identified, PermissionCondition, 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 2c29ef8939..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,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 = { 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; }; /**