diff --git a/.changeset/clean-planes-join.md b/.changeset/clean-planes-join.md new file mode 100644 index 0000000000..dd8280f553 --- /dev/null +++ b/.changeset/clean-planes-join.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-node': patch +--- + +Changed the `createPermissionIntegrationRouter` API to allow `getResources`, `resourceType` and `rules` to be optional diff --git a/.changeset/polite-wombats-smash.md b/.changeset/polite-wombats-smash.md new file mode 100644 index 0000000000..98569e1799 --- /dev/null +++ b/.changeset/polite-wombats-smash.md @@ -0,0 +1,5 @@ +--- +'@backstage/errors': patch +--- + +Added `NotImplementedError`, which can be used when the server does not recognize the request method and is incapable of supporting it for any resource. diff --git a/.changeset/what-is-going-on-babe.md b/.changeset/what-is-going-on-babe.md new file mode 100644 index 0000000000..c22d69d1de --- /dev/null +++ b/.changeset/what-is-going-on-babe.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +Add support for `NotImplementedError`, properly returning 501 as status code. diff --git a/packages/backend-app-api/src/http/MiddlewareFactory.ts b/packages/backend-app-api/src/http/MiddlewareFactory.ts index dc9fe9aa5f..833bdba540 100644 --- a/packages/backend-app-api/src/http/MiddlewareFactory.ts +++ b/packages/backend-app-api/src/http/MiddlewareFactory.ts @@ -38,6 +38,7 @@ import { NotModifiedError, serializeError, } from '@backstage/errors'; +import { NotImplementedError } from '@backstage/errors'; /** * Options used to create a {@link MiddlewareFactory}. @@ -257,6 +258,8 @@ function getStatusCode(error: Error): number { return 404; case ConflictError.name: return 409; + case NotImplementedError.name: + return 501; default: break; } diff --git a/packages/errors/api-report.md b/packages/errors/api-report.md index 1a65ae2315..650a826a30 100644 --- a/packages/errors/api-report.md +++ b/packages/errors/api-report.md @@ -84,6 +84,9 @@ export class NotAllowedError extends CustomErrorBase {} // @public export class NotFoundError extends CustomErrorBase {} +// @public +export class NotImplementedError extends CustomErrorBase {} + // @public export class NotModifiedError extends CustomErrorBase {} diff --git a/packages/errors/src/errors/common.ts b/packages/errors/src/errors/common.ts index 80a3d84d82..e1a292d7b5 100644 --- a/packages/errors/src/errors/common.ts +++ b/packages/errors/src/errors/common.ts @@ -74,6 +74,13 @@ export class ConflictError extends CustomErrorBase {} */ export class NotModifiedError extends CustomErrorBase {} +/** + * The server does not support the functionality required to fulfill the request. + * + * @public + */ +export class NotImplementedError extends CustomErrorBase {} + /** * An error that forwards an underlying cause with additional context in the message. * diff --git a/packages/errors/src/errors/index.ts b/packages/errors/src/errors/index.ts index 9e1c2408af..efd3257fbf 100644 --- a/packages/errors/src/errors/index.ts +++ b/packages/errors/src/errors/index.ts @@ -24,6 +24,7 @@ export { NotAllowedError, NotFoundError, NotModifiedError, + NotImplementedError, } from './common'; export { CustomErrorBase } from './CustomErrorBase'; export { ResponseError } from './ResponseError'; diff --git a/plugins/permission-node/api-report.md b/plugins/permission-node/api-report.md index 569fb5c6ea..f89ff0a8d5 100644 --- a/plugins/permission-node/api-report.md +++ b/plugins/permission-node/api-report.md @@ -112,20 +112,33 @@ export const createConditionTransformer: < ) => ConditionTransformer; // @public -export const createPermissionIntegrationRouter: < +export function createPermissionIntegrationRouter< TResourceType extends string, TResource, ->(options: { +>( + options: CreatePermissionIntegrationRouterResourceOptions< + TResourceType, + TResource + >, +): express.Router; + +// @public +export function createPermissionIntegrationRouter(options: { + permissions: Array; +}): express.Router; + +// @public +export type CreatePermissionIntegrationRouterResourceOptions< + TResourceType extends string, + TResource, +> = { resourceType: TResourceType; - permissions?: Permission[] | undefined; - rules: PermissionRule< - TResource, - any, - NoInfer, - PermissionRuleParams - >[]; - getResources: (resourceRefs: string[]) => Promise<(TResource | undefined)[]>; -}) => express.Router; + permissions?: Array; + rules: PermissionRule>[]; + getResources?: ( + resourceRefs: string[], + ) => Promise>; +}; // @public export const createPermissionRule: < diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts index 32d814465f..1a7462665e 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts @@ -19,18 +19,15 @@ import { createPermission, Permission, } from '@backstage/plugin-permission-common'; -import express, { Express, Router } from 'express'; +import express from 'express'; import request, { Response } from 'supertest'; import { z } from 'zod'; -import { createPermissionIntegrationRouter } from './createPermissionIntegrationRouter'; +import { + createPermissionIntegrationRouter, + CreatePermissionIntegrationRouterResourceOptions, +} from './createPermissionIntegrationRouter'; import { createPermissionRule } from './createPermissionRule'; -const mockGetResources: jest.MockedFunction< - Parameters[0]['getResources'] -> = jest.fn(async resourceRefs => - resourceRefs.map(resourceRef => ({ id: resourceRef })), -); - const testPermission: Permission = createPermission({ name: 'test.permission', attributes: {}, @@ -56,29 +53,35 @@ const testRule2 = createPermissionRule({ toQuery: () => ({}), }); +const defaultMockedGetResources: CreatePermissionIntegrationRouterResourceOptions< + string, + { id: string } +>['getResources'] = jest.fn(async resourceRefs => + resourceRefs.map(resourceRef => ({ id: resourceRef })), +); + +const createApp = ( + mockedGetResources: + | typeof defaultMockedGetResources + | null = defaultMockedGetResources, +) => { + const router = mockedGetResources + ? createPermissionIntegrationRouter({ + resourceType: 'test-resource', + permissions: [testPermission], + getResources: mockedGetResources, + rules: [testRule1, testRule2], + }) + : createPermissionIntegrationRouter({ permissions: [testPermission] }); + + return express().use(router); +}; + describe('createPermissionIntegrationRouter', () => { - let app: Express; - let router: Router; - - beforeAll(() => { - router = createPermissionIntegrationRouter({ - resourceType: 'test-resource', - permissions: [testPermission], - getResources: mockGetResources, - rules: [testRule1, testRule2], - }); - - app = express().use(router); - }); - afterEach(() => { jest.clearAllMocks(); }); - it('works', async () => { - expect(router).toBeDefined(); - }); - describe('POST /.well-known/backstage/permissions/apply-conditions', () => { it.each([ { @@ -150,7 +153,7 @@ describe('createPermissionIntegrationRouter', () => { ], }, ])('returns 200/ALLOW when criteria match (case %#)', async conditions => { - const response = await request(app) + const response = await request(createApp()) .post('/.well-known/backstage/permissions/apply-conditions') .send({ items: [ @@ -238,7 +241,7 @@ describe('createPermissionIntegrationRouter', () => { ])( 'returns 200/DENY when criteria do not match (case %#)', async conditions => { - const response = await request(app) + const response = await request(createApp()) .post('/.well-known/backstage/permissions/apply-conditions') .send({ items: [ @@ -262,7 +265,7 @@ describe('createPermissionIntegrationRouter', () => { let response: Response; beforeEach(async () => { - response = await request(app) + response = await request(createApp()) .post('/.well-known/backstage/permissions/apply-conditions') .send({ items: [ @@ -353,7 +356,7 @@ describe('createPermissionIntegrationRouter', () => { }); it('calls getResources for all required resources at once', () => { - expect(mockGetResources).toHaveBeenCalledWith([ + expect(defaultMockedGetResources).toHaveBeenCalledWith([ 'default:test/resource-1', 'default:test/resource-2', 'default:test/resource-3', @@ -363,7 +366,7 @@ describe('createPermissionIntegrationRouter', () => { }); it('returns 400 when called with incorrect resource type', async () => { - const response = await request(app) + const response = await request(createApp()) .post('/.well-known/backstage/permissions/apply-conditions') .send({ items: [ @@ -413,11 +416,14 @@ describe('createPermissionIntegrationRouter', () => { }); it('returns 200/DENY when resource is not found', async () => { - mockGetResources.mockImplementationOnce(async resourceRefs => + const mockedGetResources: CreatePermissionIntegrationRouterResourceOptions< + string, + { id: string } + >['getResources'] = jest.fn(async resourceRefs => resourceRefs.map(() => undefined), ); - const response = await request(app) + const response = await request(createApp(mockedGetResources)) .post('/.well-known/backstage/permissions/apply-conditions') .send({ items: [ @@ -446,7 +452,10 @@ describe('createPermissionIntegrationRouter', () => { }); it('interleaves responses for present and missing resources', async () => { - mockGetResources.mockImplementationOnce(async resourceRefs => + const mockedGetResources: CreatePermissionIntegrationRouterResourceOptions< + string, + { id: string } + >['getResources'] = jest.fn(async resourceRefs => resourceRefs.map(resourceRef => resourceRef === 'default:test/missing-resource' ? undefined @@ -454,7 +463,7 @@ describe('createPermissionIntegrationRouter', () => { ), ); - const response = await request(app) + const response = await request(createApp(mockedGetResources)) .post('/.well-known/backstage/permissions/apply-conditions') .send({ items: [ @@ -548,18 +557,31 @@ describe('createPermissionIntegrationRouter', () => { ], }, ])(`returns 400 for invalid input %#`, async input => { - const response = await request(app) + const response = await request(createApp()) .post('/.well-known/backstage/permissions/apply-conditions') .send(input); expect(response.status).toEqual(400); expect(response.error && response.error.text).toMatch(/invalid/i); }); + + it('returns 501 with no getResources implementation', async () => { + const response = await request(createApp(null)) + .post('/.well-known/backstage/permissions/apply-conditions') + .send({ + items: [], + }); + + expect(response.status).toEqual(501); + expect(response.body.error.message).toEqual( + `This plugin does not expose any permission rule or can't evaluate conditional decisions`, + ); + }); }); describe('GET /.well-known/backstage/permissions/metadata', () => { it('returns a list of permissions and rules used by a given backend', async () => { - const response = await request(app).get( + const response = await request(createApp()).get( '/.well-known/backstage/permissions/metadata', ); diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index 23b59c9c43..087de23198 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -36,6 +36,7 @@ import { isNotCriteria, isOrCriteria, } from './util'; +import { NotImplementedError } from '@backstage/errors'; const permissionCriteriaSchema: z.ZodSchema< PermissionCriteria @@ -159,6 +160,28 @@ const applyConditions = ( return rule.apply(resource, criteria.params ?? {}); }; +/** + * Options for creating a permission integration router specific + * for a particular resource type. + * + * @public + */ +export type CreatePermissionIntegrationRouterResourceOptions< + TResourceType extends string, + TResource, +> = { + resourceType: TResourceType; + permissions?: Array; + // Do not infer value of TResourceType from supplied rules. + // instead only consider the resourceType parameter, and + // consider any rules whose resource type does not match + // to be an error. + rules: PermissionRule>[]; + getResources?: ( + resourceRefs: string[], + ) => Promise>; +}; + /** * Create an express Router which provides an authorization route to allow * integration between the permission backend and other Backstage backend @@ -166,6 +189,9 @@ const applyConditions = ( * their resources should add the router created by this function to their * express app inside their `createRouter` implementation. * + * In case the `permissions` option is provided, the router also + * provides a route that exposes permissions and routes of a plugin. + * * @remarks * * To make this concrete, we can use the Backstage software catalog as an @@ -194,25 +220,44 @@ const applyConditions = ( * * @public */ -export const createPermissionIntegrationRouter = < +export function createPermissionIntegrationRouter< TResourceType extends string, TResource, ->(options: { - resourceType: TResourceType; - permissions?: Array; - // Do not infer value of TResourceType from supplied rules. - // instead only consider the resourceType parameter, and - // consider any rules whose resource type does not match - // to be an error. - rules: PermissionRule>[]; - getResources: ( - resourceRefs: string[], - ) => Promise>; -}): express.Router => { - const { resourceType, permissions, rules, getResources } = options; +>( + options: CreatePermissionIntegrationRouterResourceOptions< + TResourceType, + TResource + >, +): express.Router; + +/** + * + * Create an express Router which provides a route that exposes + * permissions and routes of a plugin. + * @public + */ +export function createPermissionIntegrationRouter(options: { + permissions: Array; +}): express.Router; + +/** + * @public + */ +export function createPermissionIntegrationRouter< + TResourceType extends string, + TResource, +>( + options: + | { permissions: Array } + | CreatePermissionIntegrationRouterResourceOptions< + TResourceType, + TResource + >, +): express.Router { const router = Router(); router.use(express.json()); + const { permissions = [], rules = [] } = { rules: [], ...options }; router.get('/.well-known/backstage/permissions/metadata', (_, res) => { const serializedRules: MetadataResponseSerializedRule[] = rules.map( rule => ({ @@ -231,25 +276,35 @@ export const createPermissionIntegrationRouter = < return res.json(responseJson); }); - const getRule = createGetRule(rules); - - const assertValidResourceTypes = ( - requests: ApplyConditionsRequestEntry[], - ) => { - const invalidResourceTypes = requests - .filter(request => request.resourceType !== resourceType) - .map(request => request.resourceType); - - if (invalidResourceTypes.length) { - throw new InputError( - `Unexpected resource types: ${invalidResourceTypes.join(', ')}.`, - ); - } - }; - router.post( '/.well-known/backstage/permissions/apply-conditions', async (req, res: Response) => { + if ( + !isCreatePermissionIntegrationRouterResourceOptions(options) || + options.getResources === undefined + ) { + throw new NotImplementedError( + `This plugin does not expose any permission rule or can't evaluate conditional decisions`, + ); + } + const { resourceType, getResources } = options; + + const getRule = createGetRule(rules); + + const assertValidResourceTypes = ( + requests: ApplyConditionsRequestEntry[], + ) => { + const invalidResourceTypes = requests + .filter(request => request.resourceType !== resourceType) + .map(request => request.resourceType); + + if (invalidResourceTypes.length) { + throw new InputError( + `Unexpected resource types: ${invalidResourceTypes.join(', ')}.`, + ); + } + }; + const parseResult = applyConditionsRequestSchema.safeParse(req.body); if (!parseResult.success) { @@ -288,4 +343,28 @@ export const createPermissionIntegrationRouter = < router.use(errorHandler()); return router; -}; +} + +function isCreatePermissionIntegrationRouterResourceOptions< + TResourceType extends string, + TResource, +>( + options: + | { permissions: Array } + | CreatePermissionIntegrationRouterResourceOptions< + TResourceType, + TResource + >, +): options is CreatePermissionIntegrationRouterResourceOptions< + TResourceType, + TResource +> { + return ( + ( + options as CreatePermissionIntegrationRouterResourceOptions< + TResourceType, + TResource + > + ).resourceType !== undefined + ); +}