From f028746410a472a63f45c9270e997f450bd23893 Mon Sep 17 00:00:00 2001 From: Tim Hansen Date: Wed, 17 Nov 2021 14:58:09 -0700 Subject: [PATCH 01/22] Add permission-backend plugin Signed-off-by: Tim Hansen --- plugins/permission-backend/.eslintrc.js | 3 + plugins/permission-backend/README.md | 6 + plugins/permission-backend/api-report.md | 29 +++ plugins/permission-backend/package.json | 44 +++++ plugins/permission-backend/src/index.ts | 21 +++ plugins/permission-backend/src/run.ts | 33 ++++ .../permission-backend/src/service/index.ts | 17 ++ .../src/service/router.test.ts | 103 ++++++++++ .../permission-backend/src/service/router.ts | 177 ++++++++++++++++++ .../src/service/standaloneServer.ts | 59 ++++++ plugins/permission-backend/src/setupTests.ts | 17 ++ 11 files changed, 509 insertions(+) create mode 100644 plugins/permission-backend/.eslintrc.js create mode 100644 plugins/permission-backend/README.md create mode 100644 plugins/permission-backend/api-report.md create mode 100644 plugins/permission-backend/package.json create mode 100644 plugins/permission-backend/src/index.ts create mode 100644 plugins/permission-backend/src/run.ts create mode 100644 plugins/permission-backend/src/service/index.ts create mode 100644 plugins/permission-backend/src/service/router.test.ts create mode 100644 plugins/permission-backend/src/service/router.ts create mode 100644 plugins/permission-backend/src/service/standaloneServer.ts create mode 100644 plugins/permission-backend/src/setupTests.ts diff --git a/plugins/permission-backend/.eslintrc.js b/plugins/permission-backend/.eslintrc.js new file mode 100644 index 0000000000..16a033dbc6 --- /dev/null +++ b/plugins/permission-backend/.eslintrc.js @@ -0,0 +1,3 @@ +module.exports = { + extends: [require.resolve('@backstage/cli/config/eslint.backend')], +}; diff --git a/plugins/permission-backend/README.md b/plugins/permission-backend/README.md new file mode 100644 index 0000000000..59fe1730ba --- /dev/null +++ b/plugins/permission-backend/README.md @@ -0,0 +1,6 @@ +# @backstage/plugin-permission-backend + +> NOTE: THIS PACKAGE IS EXPERIMENTAL, HERE BE DRAGONS + +Backend for Backstage authorization and permissions. For more information, see +the [authorization PRFC](https://github.com/backstage/backstage/pull/7761). diff --git a/plugins/permission-backend/api-report.md b/plugins/permission-backend/api-report.md new file mode 100644 index 0000000000..5b7801d0ab --- /dev/null +++ b/plugins/permission-backend/api-report.md @@ -0,0 +1,29 @@ +## API Report File for "@backstage/plugin-permission-backend" + +> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). + +```ts +import { Config } from '@backstage/config'; +import express from 'express'; +import { Logger as Logger_2 } from 'winston'; +import { PermissionPolicy } from '@backstage/plugin-permission-node'; + +// Warning: (ae-missing-release-tag) "createRouter" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public (undocumented) +export function createRouter(options: RouterOptions): Promise; + +// Warning: (ae-missing-release-tag) "RouterOptions" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public (undocumented) +export interface RouterOptions { + // (undocumented) + config: Config; + // (undocumented) + logger: Logger_2; + // (undocumented) + policy: PermissionPolicy; +} + +// (No @packageDocumentation comment for this package) +``` diff --git a/plugins/permission-backend/package.json b/plugins/permission-backend/package.json new file mode 100644 index 0000000000..f6aac0b3ab --- /dev/null +++ b/plugins/permission-backend/package.json @@ -0,0 +1,44 @@ +{ + "name": "@backstage/plugin-permission-backend", + "version": "0.1.0", + "main": "src/index.ts", + "types": "src/index.ts", + "license": "Apache-2.0", + "publishConfig": { + "access": "public", + "main": "dist/index.cjs.js", + "types": "dist/index.d.ts" + }, + "scripts": { + "start": "backstage-cli backend:dev", + "build": "backstage-cli backend:build", + "lint": "backstage-cli lint", + "test": "backstage-cli test", + "prepack": "backstage-cli prepack", + "postpack": "backstage-cli postpack", + "clean": "backstage-cli clean" + }, + "dependencies": { + "@backstage/backend-common": "^0.9.9", + "@backstage/config": "^0.1.11", + "@backstage/errors": "^0.1.4", + "@backstage/plugin-auth-backend": "^0.4.7", + "@backstage/plugin-permission-common": "^0.1.0", + "@backstage/plugin-permission-node": "^0.1.0", + "@types/express": "*", + "cross-fetch": "^3.0.6", + "express": "^4.17.1", + "express-promise-router": "^4.1.0", + "winston": "^3.2.1", + "yn": "^4.0.0" + }, + "devDependencies": { + "@backstage/cli": "^0.8.2", + "@types/supertest": "^2.0.8", + "supertest": "^4.0.2", + "msw": "^0.35.0" + }, + "files": [ + "dist" + ] +} diff --git a/plugins/permission-backend/src/index.ts b/plugins/permission-backend/src/index.ts new file mode 100644 index 0000000000..877b9070c3 --- /dev/null +++ b/plugins/permission-backend/src/index.ts @@ -0,0 +1,21 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Backend for Backstage authorization and permissions. + * @packageDocumentation + */ +export * from './service'; diff --git a/plugins/permission-backend/src/run.ts b/plugins/permission-backend/src/run.ts new file mode 100644 index 0000000000..addfdfd6d7 --- /dev/null +++ b/plugins/permission-backend/src/run.ts @@ -0,0 +1,33 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { getRootLogger } from '@backstage/backend-common'; +import yn from 'yn'; +import { startStandaloneServer } from './service/standaloneServer'; + +const port = process.env.PLUGIN_PORT ? Number(process.env.PLUGIN_PORT) : 7000; +const enableCors = yn(process.env.PLUGIN_CORS, { default: false }); +const logger = getRootLogger(); + +startStandaloneServer({ port, enableCors, logger }).catch(err => { + logger.error(err); + process.exit(1); +}); + +process.on('SIGINT', () => { + logger.info('CTRL+C pressed; exiting.'); + process.exit(0); +}); diff --git a/plugins/permission-backend/src/service/index.ts b/plugins/permission-backend/src/service/index.ts new file mode 100644 index 0000000000..b26567988c --- /dev/null +++ b/plugins/permission-backend/src/service/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export * from './router'; diff --git a/plugins/permission-backend/src/service/router.test.ts b/plugins/permission-backend/src/service/router.test.ts new file mode 100644 index 0000000000..f0fc889e8b --- /dev/null +++ b/plugins/permission-backend/src/service/router.test.ts @@ -0,0 +1,103 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import express from 'express'; +import request from 'supertest'; +import { getVoidLogger } from '@backstage/backend-common'; +import { ConfigReader } from '@backstage/config'; +import { IdentityClient } from '@backstage/plugin-auth-backend'; +import { + AuthorizeResult, + Permission, +} from '@backstage/plugin-permission-common'; +import { PermissionPolicy } from '@backstage/plugin-permission-node'; + +import { createRouter } from './router'; + +const identityApi: Partial = { + authenticate: jest.fn().mockImplementation(_ => ({ id: 'test-user' })), +}; + +const policy: PermissionPolicy = { + handle: jest.fn().mockImplementation((_req, identity) => { + if (identity) { + return { result: AuthorizeResult.ALLOW }; + } + return { result: AuthorizeResult.DENY }; + }), +}; + +const permission: Permission = { + name: 'test.permission', + attributes: { action: 'read' }, +}; + +describe('createRouter', () => { + let app: express.Express; + + beforeAll(async () => { + const router = await createRouter({ + logger: getVoidLogger(), + config: new ConfigReader({ + backend: { + baseUrl: 'http://localhost', + listen: { port: 7007 }, + }, + }), + policy, + identity: identityApi as IdentityClient, + }); + app = express().use(router); + }); + + describe('GET /health', () => { + it('returns ok', async () => { + const response = await request(app).get('/health'); + + expect(response.status).toEqual(200); + expect(response.body).toEqual({ status: 'ok' }); + }); + }); + + describe('POST /authorize', () => { + it('calls the permission policy', async () => { + const response = await request(app) + .post('/authorize') + .send([{ id: 123, permission }]); + + expect(response.status).toEqual(200); + expect(policy.handle).toHaveBeenCalledWith({ permission }, undefined); + expect(response.body).toEqual([ + { id: 123, result: AuthorizeResult.DENY }, + ]); + }); + + it('resolves identity from the Authorization header', async () => { + const token = 'token'; + const response = await request(app) + .post('/authorize') + .auth(token, { type: 'bearer' }) + .send([{ id: 123, permission }]); + + expect(response.status).toEqual(200); + expect(identityApi.authenticate).toHaveBeenCalledWith('token'); + expect(policy.handle).toHaveBeenCalledWith( + { permission }, + { id: 'test-user' }, + ); + }); + }); +}); diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts new file mode 100644 index 0000000000..2019d13d2a --- /dev/null +++ b/plugins/permission-backend/src/service/router.ts @@ -0,0 +1,177 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + errorHandler, + SingleHostDiscovery, + PluginEndpointDiscovery, +} from '@backstage/backend-common'; +import fetch from 'cross-fetch'; +import express, { Request, Response } from 'express'; +import Router from 'express-promise-router'; +import { Logger } from 'winston'; +import { + BackstageIdentity, + IdentityClient, +} from '@backstage/plugin-auth-backend'; +import { Config } from '@backstage/config'; +import { ConflictError, ResponseError } from '@backstage/errors'; +import { + AuthorizeResult, + AuthorizeResponse, + AuthorizeRequest, + Identified, + PermissionCondition, + PermissionCriteria, +} from '@backstage/plugin-permission-common'; +import { + ApplyConditionsRequest, + PermissionPolicy, +} from '@backstage/plugin-permission-node'; + +export interface RouterOptions { + logger: Logger; + config: Config; + policy: PermissionPolicy; + identity?: IdentityClient; +} + +// TODO(permission-backend): probably move this to a separate client +const applyConditions = async ( + resourceRef: string, + conditions: { + pluginId: string; + resourceType: string; + conditions: PermissionCriteria; + }, + discoveryApi: PluginEndpointDiscovery, + authHeader?: string, +): Promise<{ result: AuthorizeResult.ALLOW | AuthorizeResult.DENY }> => { + const endpoint = `${await discoveryApi.getBaseUrl( + conditions.pluginId, + )}/permissions/apply-conditions`; + + const request: ApplyConditionsRequest = { + resourceRef, + resourceType: conditions.resourceType, + conditions: conditions.conditions, + }; + + const response = await fetch(endpoint, { + method: 'POST', + body: JSON.stringify(request), + headers: { + ...(authHeader ? { authorization: authHeader } : {}), + 'content-type': 'application/json', + }, + }); + + if (!response.ok) { + throw await ResponseError.fromResponse(response); + } + + // TODO(permission-backend): validate response + return await response.json(); +}; + +const handleRequest = async ( + { id, resourceRef, ...request }: Identified, + user: BackstageIdentity | undefined, + policy: PermissionPolicy, + discoveryApi: PluginEndpointDiscovery, + authHeader?: string, +): Promise> => { + const response = await policy.handle(request, user); + + if (response.result === AuthorizeResult.CONDITIONAL) { + // Sanity check that any resource provided matches the one expected by the permission + if (request.permission.resourceType !== response.conditions.resourceType) { + throw new ConflictError( + `Invalid resource conditions returned from permission policy for permission ${request.permission.name}`, + ); + } + + if (resourceRef) { + return { + id, + ...(await applyConditions( + resourceRef, + response.conditions, + discoveryApi, + authHeader, + )), + }; + } + + return { + id, + result: AuthorizeResult.CONDITIONAL, + conditions: response.conditions.conditions, + }; + } + + return { id, ...response }; +}; + +export async function createRouter( + options: RouterOptions, +): Promise { + const { config, policy } = options; + const discovery = SingleHostDiscovery.fromConfig(config); + const identity = + options.identity ?? + new IdentityClient({ + discovery, + issuer: await discovery.getExternalBaseUrl('auth'), + }); + + const router = Router(); + router.use(express.json()); + + router.get('/health', (_, response) => { + response.send({ status: 'ok' }); + }); + + router.post( + '/authorize', + async ( + req: Request[]>, + res: Response[]>, + ) => { + const token = IdentityClient.getBearerToken(req.header('authorization')); + const user = token ? await identity.authenticate(token) : undefined; + + const body: Identified[] = req.body; + + res.json( + await Promise.all( + body.map(request => + handleRequest( + request, + user, + policy, + discovery, + req.header('authorization'), + ), + ), + ), + ); + }, + ); + + router.use(errorHandler()); + return router; +} diff --git a/plugins/permission-backend/src/service/standaloneServer.ts b/plugins/permission-backend/src/service/standaloneServer.ts new file mode 100644 index 0000000000..6218d22d59 --- /dev/null +++ b/plugins/permission-backend/src/service/standaloneServer.ts @@ -0,0 +1,59 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + createServiceBuilder, + loadBackendConfig, +} from '@backstage/backend-common'; +import { Server } from 'http'; +import { Logger } from 'winston'; +import { createRouter } from './router'; +import { AuthorizeResult } from '@backstage/plugin-permission-common'; + +export interface ServerOptions { + port: number; + enableCors: boolean; + logger: Logger; +} + +export async function startStandaloneServer( + options: ServerOptions, +): Promise { + const logger = options.logger.child({ service: 'permission-backend' }); + const config = await loadBackendConfig({ logger, argv: process.argv }); + logger.debug('Starting application server...'); + const router = await createRouter({ + logger, + config, + policy: { + handle: () => Promise.resolve({ result: AuthorizeResult.ALLOW }), + }, + }); + + let service = createServiceBuilder(module) + .setPort(options.port) + .addRouter('/permission', router); + if (options.enableCors) { + service = service.enableCors({ origin: 'http://localhost:3000' }); + } + + return await service.start().catch(err => { + logger.error(err); + process.exit(1); + }); +} + +module.hot?.accept(); diff --git a/plugins/permission-backend/src/setupTests.ts b/plugins/permission-backend/src/setupTests.ts new file mode 100644 index 0000000000..a330613afb --- /dev/null +++ b/plugins/permission-backend/src/setupTests.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export {}; From 806778d58e2d79d272ccf847a0a3d08cc19daa25 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 22 Nov 2021 11:25:48 +0000 Subject: [PATCH 02/22] permission-backend: fixup api-report Signed-off-by: Mike Lewis --- plugins/permission-backend/api-report.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/permission-backend/api-report.md b/plugins/permission-backend/api-report.md index 5b7801d0ab..66e0dabd3b 100644 --- a/plugins/permission-backend/api-report.md +++ b/plugins/permission-backend/api-report.md @@ -5,6 +5,7 @@ ```ts import { Config } from '@backstage/config'; import express from 'express'; +import { IdentityClient } from '@backstage/plugin-auth-backend'; import { Logger as Logger_2 } from 'winston'; import { PermissionPolicy } from '@backstage/plugin-permission-node'; @@ -20,10 +21,10 @@ export interface RouterOptions { // (undocumented) config: Config; // (undocumented) + identity?: IdentityClient; + // (undocumented) logger: Logger_2; // (undocumented) policy: PermissionPolicy; } - -// (No @packageDocumentation comment for this package) ``` From be1f89d3835b4b072df9a734c34f101a791a7572 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 22 Nov 2021 11:22:24 +0000 Subject: [PATCH 03/22] permission-backend: extract AllowAllPermissionPolicy from standaloneServer in permission-backend Signed-off-by: Mike Lewis --- .../service/AllowAllPermissionPolicy.test.ts | 35 +++++++++++++++++++ .../src/service/AllowAllPermissionPolicy.ts | 34 ++++++++++++++++++ .../src/service/standaloneServer.ts | 6 ++-- 3 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 plugins/permission-backend/src/service/AllowAllPermissionPolicy.test.ts create mode 100644 plugins/permission-backend/src/service/AllowAllPermissionPolicy.ts diff --git a/plugins/permission-backend/src/service/AllowAllPermissionPolicy.test.ts b/plugins/permission-backend/src/service/AllowAllPermissionPolicy.test.ts new file mode 100644 index 0000000000..5ebebd04e5 --- /dev/null +++ b/plugins/permission-backend/src/service/AllowAllPermissionPolicy.test.ts @@ -0,0 +1,35 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + AuthorizeResult, + Permission, +} from '@backstage/plugin-permission-common'; +import { AllowAllPermissionPolicy } from './AllowAllPermissionPolicy'; + +describe('AllowAllPermissionPolicy', () => { + const policy = new AllowAllPermissionPolicy(); + + it('returns ALLOW when no identity is passed', async () => { + await expect( + policy.handle({ + permission: { + name: 'any.permission', + } as Permission, + }), + ).resolves.toEqual({ result: AuthorizeResult.ALLOW }); + }); +}); diff --git a/plugins/permission-backend/src/service/AllowAllPermissionPolicy.ts b/plugins/permission-backend/src/service/AllowAllPermissionPolicy.ts new file mode 100644 index 0000000000..bb39d4a334 --- /dev/null +++ b/plugins/permission-backend/src/service/AllowAllPermissionPolicy.ts @@ -0,0 +1,34 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { BackstageIdentity } from '@backstage/plugin-auth-backend'; +import { AuthorizeResult } from '@backstage/plugin-permission-common'; +import { + PermissionPolicy, + PolicyAuthorizeRequest, + PolicyResult, +} from '@backstage/plugin-permission-node'; + +export class AllowAllPermissionPolicy implements PermissionPolicy { + async handle( + _request: PolicyAuthorizeRequest, + _user?: BackstageIdentity, + ): Promise { + return { + result: AuthorizeResult.ALLOW, + }; + } +} diff --git a/plugins/permission-backend/src/service/standaloneServer.ts b/plugins/permission-backend/src/service/standaloneServer.ts index 6218d22d59..87c62c6234 100644 --- a/plugins/permission-backend/src/service/standaloneServer.ts +++ b/plugins/permission-backend/src/service/standaloneServer.ts @@ -20,8 +20,8 @@ import { } from '@backstage/backend-common'; import { Server } from 'http'; import { Logger } from 'winston'; +import { AllowAllPermissionPolicy } from './AllowAllPermissionPolicy'; import { createRouter } from './router'; -import { AuthorizeResult } from '@backstage/plugin-permission-common'; export interface ServerOptions { port: number; @@ -38,9 +38,7 @@ export async function startStandaloneServer( const router = await createRouter({ logger, config, - policy: { - handle: () => Promise.resolve({ result: AuthorizeResult.ALLOW }), - }, + policy: new AllowAllPermissionPolicy(), }); let service = createServiceBuilder(module) From 4ca6802b97d3e4dee8582c3eadbb219d3d426fec Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 22 Nov 2021 11:27:23 +0000 Subject: [PATCH 04/22] permission-backend: bump @backstage/cli Signed-off-by: Mike Lewis --- plugins/permission-backend/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/permission-backend/package.json b/plugins/permission-backend/package.json index f6aac0b3ab..6e711ee4b2 100644 --- a/plugins/permission-backend/package.json +++ b/plugins/permission-backend/package.json @@ -33,7 +33,7 @@ "yn": "^4.0.0" }, "devDependencies": { - "@backstage/cli": "^0.8.2", + "@backstage/cli": "^0.9.0", "@types/supertest": "^2.0.8", "supertest": "^4.0.2", "msw": "^0.35.0" From 671bf72ed6690e852538ff302fced84310d256f2 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 22 Nov 2021 12:02:49 +0000 Subject: [PATCH 05/22] permission-backend: fix initial test suite Signed-off-by: Mike Lewis --- plugins/permission-backend/api-report.md | 3 -- .../src/service/router.test.ts | 34 ++++++++++++++----- .../permission-backend/src/service/router.ts | 11 +++--- 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/plugins/permission-backend/api-report.md b/plugins/permission-backend/api-report.md index 66e0dabd3b..1450382881 100644 --- a/plugins/permission-backend/api-report.md +++ b/plugins/permission-backend/api-report.md @@ -5,7 +5,6 @@ ```ts import { Config } from '@backstage/config'; import express from 'express'; -import { IdentityClient } from '@backstage/plugin-auth-backend'; import { Logger as Logger_2 } from 'winston'; import { PermissionPolicy } from '@backstage/plugin-permission-node'; @@ -21,8 +20,6 @@ export interface RouterOptions { // (undocumented) config: Config; // (undocumented) - identity?: IdentityClient; - // (undocumented) logger: Logger_2; // (undocumented) policy: PermissionPolicy; diff --git a/plugins/permission-backend/src/service/router.test.ts b/plugins/permission-backend/src/service/router.test.ts index f0fc889e8b..73b816f752 100644 --- a/plugins/permission-backend/src/service/router.test.ts +++ b/plugins/permission-backend/src/service/router.test.ts @@ -18,7 +18,6 @@ import express from 'express'; import request from 'supertest'; import { getVoidLogger } from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; -import { IdentityClient } from '@backstage/plugin-auth-backend'; import { AuthorizeResult, Permission, @@ -27,9 +26,26 @@ import { PermissionPolicy } from '@backstage/plugin-permission-node'; import { createRouter } from './router'; -const identityApi: Partial = { - authenticate: jest.fn().mockImplementation(_ => ({ id: 'test-user' })), -}; +jest.mock('@backstage/plugin-auth-backend', () => { + class MockIdentityClient { + authenticate = jest.fn(token => + Promise.resolve( + token + ? { + id: 'test-user', + token, + } + : undefined, + ), + ); + + static getBearerToken = jest.fn(authHeader => + authHeader ? `` : undefined, + ); + } + + return { IdentityClient: MockIdentityClient }; +}); const policy: PermissionPolicy = { handle: jest.fn().mockImplementation((_req, identity) => { @@ -58,8 +74,8 @@ describe('createRouter', () => { }, }), policy, - identity: identityApi as IdentityClient, }); + app = express().use(router); }); @@ -86,18 +102,20 @@ describe('createRouter', () => { }); it('resolves identity from the Authorization header', async () => { - const token = 'token'; + const token = 'test-token'; const response = await request(app) .post('/authorize') .auth(token, { type: 'bearer' }) .send([{ id: 123, permission }]); expect(response.status).toEqual(200); - expect(identityApi.authenticate).toHaveBeenCalledWith('token'); expect(policy.handle).toHaveBeenCalledWith( { permission }, - { id: 'test-user' }, + { id: 'test-user', token: '' }, ); + expect(response.body).toEqual([ + { id: 123, result: AuthorizeResult.ALLOW }, + ]); }); }); }); diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index 2019d13d2a..e52f3d14ee 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -46,7 +46,6 @@ export interface RouterOptions { logger: Logger; config: Config; policy: PermissionPolicy; - identity?: IdentityClient; } // TODO(permission-backend): probably move this to a separate client @@ -131,12 +130,10 @@ export async function createRouter( ): Promise { const { config, policy } = options; const discovery = SingleHostDiscovery.fromConfig(config); - const identity = - options.identity ?? - new IdentityClient({ - discovery, - issuer: await discovery.getExternalBaseUrl('auth'), - }); + const identity = new IdentityClient({ + discovery, + issuer: await discovery.getExternalBaseUrl('auth'), + }); const router = Router(); router.use(express.json()); From e9b6b7bf89a430265a890d351b19d43c67b01bdd Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 22 Nov 2021 13:19:48 +0000 Subject: [PATCH 06/22] permission-backend: extract client for calling other plugins to apply conditions Also adds response validation Signed-off-by: Mike Lewis --- plugins/permission-backend/package.json | 3 +- .../PermissionIntegrationClient.test.ts | 173 ++++++++++++++++++ .../service/PermissionIntegrationClient.ts | 76 ++++++++ .../permission-backend/src/service/router.ts | 60 +----- 4 files changed, 261 insertions(+), 51 deletions(-) create mode 100644 plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts create mode 100644 plugins/permission-backend/src/service/PermissionIntegrationClient.ts diff --git a/plugins/permission-backend/package.json b/plugins/permission-backend/package.json index 6e711ee4b2..dae7fdb5af 100644 --- a/plugins/permission-backend/package.json +++ b/plugins/permission-backend/package.json @@ -30,7 +30,8 @@ "express": "^4.17.1", "express-promise-router": "^4.1.0", "winston": "^3.2.1", - "yn": "^4.0.0" + "yn": "^4.0.0", + "zod": "^3.11.6" }, "devDependencies": { "@backstage/cli": "^0.9.0", diff --git a/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts b/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts new file mode 100644 index 0000000000..d062bccb33 --- /dev/null +++ b/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts @@ -0,0 +1,173 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { RestContext, rest } from 'msw'; +import { setupServer } from 'msw/node'; +import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { AuthorizeResult } from '@backstage/plugin-permission-common'; +import { PermissionIntegrationClient } from './PermissionIntegrationClient'; + +const server = setupServer(); + +const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; +const discovery: PluginEndpointDiscovery = { + async getBaseUrl() { + return mockBaseUrl; + }, + async getExternalBaseUrl() { + throw new Error('Not implemented.'); + }, +}; + +const client: PermissionIntegrationClient = new PermissionIntegrationClient({ + discovery, +}); + +const mockConditions = { + not: { + allOf: [ + { rule: 'RULE_1', params: [] }, + { rule: 'RULE_2', params: ['abc'] }, + ], + }, +}; + +describe('PermissionIntegrationClient', () => { + beforeAll(() => server.listen({ onUnhandledRequest: 'error' })); + afterAll(() => server.close()); + afterEach(() => server.resetHandlers()); + + describe('applyConditions', () => { + const mockApplyConditionsHandler = jest.fn( + (_req, res, { json }: RestContext) => { + return res(json({ result: AuthorizeResult.ALLOW })); + }, + ); + + beforeEach(() => { + server.use( + rest.post( + `${mockBaseUrl}/permissions/apply-conditions`, + mockApplyConditionsHandler, + ), + ); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should make a POST request to the correct endpoint', async () => { + await client.applyConditions('testResource1', { + pluginId: 'test-plugin', + resourceType: 'test-resource', + conditions: mockConditions, + }); + + expect(mockApplyConditionsHandler).toHaveBeenCalled(); + }); + + it('should include a request body', async () => { + await client.applyConditions('testResource1', { + pluginId: 'test-plugin', + resourceType: 'test-resource', + conditions: mockConditions, + }); + + expect(mockApplyConditionsHandler).toHaveBeenCalledWith( + expect.objectContaining({ + body: { + resourceRef: 'testResource1', + resourceType: 'test-resource', + conditions: mockConditions, + }, + }), + expect.anything(), + expect.anything(), + ); + }); + + it('should return the response from the fetch request', async () => { + const response = await client.applyConditions('testResource1', { + pluginId: 'test-plugin', + resourceType: 'test-resource', + conditions: mockConditions, + }); + + expect(response).toEqual( + expect.objectContaining({ result: AuthorizeResult.ALLOW }), + ); + }); + + it('should not include authorization headers if no token is supplied', async () => { + await client.applyConditions('testResource1', { + pluginId: 'test-plugin', + resourceType: 'test-resource', + conditions: mockConditions, + }); + + const request = mockApplyConditionsHandler.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.applyConditions( + 'testResource1', + { + pluginId: 'test-plugin', + resourceType: 'test-resource', + conditions: mockConditions, + }, + 'Bearer fake-token', + ); + + const request = mockApplyConditionsHandler.mock.calls[0][0]; + expect(request.headers.get('authorization')).toEqual('Bearer fake-token'); + }); + + it('should forward response errors', async () => { + mockApplyConditionsHandler.mockImplementationOnce( + (_req, res, { status }: RestContext) => { + return res(status(401)); + }, + ); + + await expect( + client.applyConditions('testResource1', { + pluginId: 'test-plugin', + resourceType: 'test-resource', + conditions: mockConditions, + }), + ).rejects.toThrowError(/request failed with 401/i); + }); + + it('should reject invalid responses', async () => { + mockApplyConditionsHandler.mockImplementationOnce( + (_req, res, { json }: RestContext) => { + return res(json({ outcome: AuthorizeResult.ALLOW })); + }, + ); + + await expect( + client.applyConditions('testResource1', { + pluginId: 'test-plugin', + resourceType: 'test-resource', + conditions: mockConditions, + }), + ).rejects.toThrowError(/invalid input/i); + }); + }); +}); diff --git a/plugins/permission-backend/src/service/PermissionIntegrationClient.ts b/plugins/permission-backend/src/service/PermissionIntegrationClient.ts new file mode 100644 index 0000000000..dd0aad1215 --- /dev/null +++ b/plugins/permission-backend/src/service/PermissionIntegrationClient.ts @@ -0,0 +1,76 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import fetch from 'cross-fetch'; +import { z } from 'zod'; +import { ResponseError } from '@backstage/errors'; +import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { + AuthorizeResult, + PermissionCondition, + PermissionCriteria, +} from '@backstage/plugin-permission-common'; +import { + ApplyConditionsRequest, + ApplyConditionsResponse, +} from '@backstage/plugin-permission-node'; + +const responseSchema = z.object({ + result: z.literal(AuthorizeResult.ALLOW).or(z.literal(AuthorizeResult.DENY)), +}); + +export class PermissionIntegrationClient { + private readonly discovery: PluginEndpointDiscovery; + + constructor(options: { discovery: PluginEndpointDiscovery }) { + this.discovery = options.discovery; + } + + async applyConditions( + resourceRef: string, + conditions: { + pluginId: string; + resourceType: string; + conditions: PermissionCriteria; + }, + authHeader?: string, + ): Promise { + const endpoint = `${await this.discovery.getBaseUrl( + conditions.pluginId, + )}/permissions/apply-conditions`; + + const request: ApplyConditionsRequest = { + resourceRef, + resourceType: conditions.resourceType, + conditions: conditions.conditions, + }; + + const response = await fetch(endpoint, { + method: 'POST', + body: JSON.stringify(request), + headers: { + ...(authHeader ? { authorization: authHeader } : {}), + 'content-type': 'application/json', + }, + }); + + if (!response.ok) { + throw await ResponseError.fromResponse(response); + } + + return responseSchema.parse(await response.json()); + } +} diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index e52f3d14ee..9114b8fee2 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -19,7 +19,6 @@ import { SingleHostDiscovery, PluginEndpointDiscovery, } from '@backstage/backend-common'; -import fetch from 'cross-fetch'; import express, { Request, Response } from 'express'; import Router from 'express-promise-router'; import { Logger } from 'winston'; @@ -28,19 +27,15 @@ import { IdentityClient, } from '@backstage/plugin-auth-backend'; import { Config } from '@backstage/config'; -import { ConflictError, ResponseError } from '@backstage/errors'; +import { ConflictError } from '@backstage/errors'; import { AuthorizeResult, AuthorizeResponse, AuthorizeRequest, Identified, - PermissionCondition, - PermissionCriteria, } from '@backstage/plugin-permission-common'; -import { - ApplyConditionsRequest, - PermissionPolicy, -} from '@backstage/plugin-permission-node'; +import { PermissionPolicy } from '@backstage/plugin-permission-node'; +import { PermissionIntegrationClient } from './PermissionIntegrationClient'; export interface RouterOptions { logger: Logger; @@ -48,49 +43,11 @@ export interface RouterOptions { policy: PermissionPolicy; } -// TODO(permission-backend): probably move this to a separate client -const applyConditions = async ( - resourceRef: string, - conditions: { - pluginId: string; - resourceType: string; - conditions: PermissionCriteria; - }, - discoveryApi: PluginEndpointDiscovery, - authHeader?: string, -): Promise<{ result: AuthorizeResult.ALLOW | AuthorizeResult.DENY }> => { - const endpoint = `${await discoveryApi.getBaseUrl( - conditions.pluginId, - )}/permissions/apply-conditions`; - - const request: ApplyConditionsRequest = { - resourceRef, - resourceType: conditions.resourceType, - conditions: conditions.conditions, - }; - - const response = await fetch(endpoint, { - method: 'POST', - body: JSON.stringify(request), - headers: { - ...(authHeader ? { authorization: authHeader } : {}), - 'content-type': 'application/json', - }, - }); - - if (!response.ok) { - throw await ResponseError.fromResponse(response); - } - - // TODO(permission-backend): validate response - return await response.json(); -}; - const handleRequest = async ( { id, resourceRef, ...request }: Identified, user: BackstageIdentity | undefined, policy: PermissionPolicy, - discoveryApi: PluginEndpointDiscovery, + permissionIntegrationClient: PermissionIntegrationClient, authHeader?: string, ): Promise> => { const response = await policy.handle(request, user); @@ -106,10 +63,9 @@ const handleRequest = async ( if (resourceRef) { return { id, - ...(await applyConditions( + ...(await permissionIntegrationClient.applyConditions( resourceRef, response.conditions, - discoveryApi, authHeader, )), }; @@ -135,6 +91,10 @@ export async function createRouter( issuer: await discovery.getExternalBaseUrl('auth'), }); + const permissionIntegrationClient = new PermissionIntegrationClient({ + discovery, + }); + const router = Router(); router.use(express.json()); @@ -160,7 +120,7 @@ export async function createRouter( request, user, policy, - discovery, + permissionIntegrationClient, req.header('authorization'), ), ), From 0ce2bc41295ea473e16cc8a66e5940b767f0e5cb Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 22 Nov 2021 13:23:14 +0000 Subject: [PATCH 07/22] permission-backend: remove manual construction of service discovery We shouldn't be constructing a service discovery implementation inside the router, since it removes the ability to provide a different discovery mechanism. Signed-off-by: Mike Lewis --- plugins/permission-backend/api-report.md | 4 ++-- plugins/permission-backend/src/service/router.test.ts | 11 ++++------- plugins/permission-backend/src/service/router.ts | 8 +++----- .../src/service/standaloneServer.ts | 4 +++- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/plugins/permission-backend/api-report.md b/plugins/permission-backend/api-report.md index 1450382881..4fb0928b0b 100644 --- a/plugins/permission-backend/api-report.md +++ b/plugins/permission-backend/api-report.md @@ -3,10 +3,10 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts -import { Config } from '@backstage/config'; import express from 'express'; import { Logger as Logger_2 } from 'winston'; import { PermissionPolicy } from '@backstage/plugin-permission-node'; +import { PluginEndpointDiscovery } from '@backstage/backend-common'; // Warning: (ae-missing-release-tag) "createRouter" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // @@ -18,7 +18,7 @@ export function createRouter(options: RouterOptions): Promise; // @public (undocumented) export interface RouterOptions { // (undocumented) - config: Config; + discovery: PluginEndpointDiscovery; // (undocumented) logger: Logger_2; // (undocumented) diff --git a/plugins/permission-backend/src/service/router.test.ts b/plugins/permission-backend/src/service/router.test.ts index 73b816f752..13261125c8 100644 --- a/plugins/permission-backend/src/service/router.test.ts +++ b/plugins/permission-backend/src/service/router.test.ts @@ -17,7 +17,6 @@ import express from 'express'; import request from 'supertest'; import { getVoidLogger } from '@backstage/backend-common'; -import { ConfigReader } from '@backstage/config'; import { AuthorizeResult, Permission, @@ -67,12 +66,10 @@ describe('createRouter', () => { beforeAll(async () => { const router = await createRouter({ logger: getVoidLogger(), - config: new ConfigReader({ - backend: { - baseUrl: 'http://localhost', - listen: { port: 7007 }, - }, - }), + discovery: { + getBaseUrl: jest.fn(), + getExternalBaseUrl: jest.fn(), + }, policy, }); diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index 9114b8fee2..744982e2e3 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -16,7 +16,6 @@ import { errorHandler, - SingleHostDiscovery, PluginEndpointDiscovery, } from '@backstage/backend-common'; import express, { Request, Response } from 'express'; @@ -26,7 +25,6 @@ import { BackstageIdentity, IdentityClient, } from '@backstage/plugin-auth-backend'; -import { Config } from '@backstage/config'; import { ConflictError } from '@backstage/errors'; import { AuthorizeResult, @@ -39,7 +37,7 @@ import { PermissionIntegrationClient } from './PermissionIntegrationClient'; export interface RouterOptions { logger: Logger; - config: Config; + discovery: PluginEndpointDiscovery; policy: PermissionPolicy; } @@ -84,8 +82,8 @@ const handleRequest = async ( export async function createRouter( options: RouterOptions, ): Promise { - const { config, policy } = options; - const discovery = SingleHostDiscovery.fromConfig(config); + const { policy, discovery } = options; + const identity = new IdentityClient({ discovery, issuer: await discovery.getExternalBaseUrl('auth'), diff --git a/plugins/permission-backend/src/service/standaloneServer.ts b/plugins/permission-backend/src/service/standaloneServer.ts index 87c62c6234..a2478ac57b 100644 --- a/plugins/permission-backend/src/service/standaloneServer.ts +++ b/plugins/permission-backend/src/service/standaloneServer.ts @@ -17,6 +17,7 @@ import { createServiceBuilder, loadBackendConfig, + SingleHostDiscovery, } from '@backstage/backend-common'; import { Server } from 'http'; import { Logger } from 'winston'; @@ -34,10 +35,11 @@ export async function startStandaloneServer( ): Promise { const logger = options.logger.child({ service: 'permission-backend' }); const config = await loadBackendConfig({ logger, argv: process.argv }); + const discovery = SingleHostDiscovery.fromConfig(config); logger.debug('Starting application server...'); const router = await createRouter({ logger, - config, + discovery, policy: new AllowAllPermissionPolicy(), }); From 6de1e719d4bffad8e56fff03a12bfe5609575e87 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 22 Nov 2021 15:13:27 +0000 Subject: [PATCH 08/22] permission-backend: api docs Signed-off-by: Mike Lewis --- plugins/permission-backend/api-report.md | 8 ++------ plugins/permission-backend/src/service/router.ts | 12 ++++++++++++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/plugins/permission-backend/api-report.md b/plugins/permission-backend/api-report.md index 4fb0928b0b..7f82913e18 100644 --- a/plugins/permission-backend/api-report.md +++ b/plugins/permission-backend/api-report.md @@ -8,14 +8,10 @@ import { Logger as Logger_2 } from 'winston'; import { PermissionPolicy } from '@backstage/plugin-permission-node'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; -// Warning: (ae-missing-release-tag) "createRouter" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// -// @public (undocumented) +// @public export function createRouter(options: RouterOptions): Promise; -// Warning: (ae-missing-release-tag) "RouterOptions" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// -// @public (undocumented) +// @public export interface RouterOptions { // (undocumented) discovery: PluginEndpointDiscovery; diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index 744982e2e3..9110295f77 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -35,6 +35,12 @@ import { import { PermissionPolicy } from '@backstage/plugin-permission-node'; import { PermissionIntegrationClient } from './PermissionIntegrationClient'; +/** + * Options required when constructing a new {@link express#Router} using + * {@link createRouter}. + * + * @public + */ export interface RouterOptions { logger: Logger; discovery: PluginEndpointDiscovery; @@ -79,6 +85,12 @@ const handleRequest = async ( return { id, ...response }; }; +/** + * Creates a new {@link express#Router} which provides the backend API + * for the permission system. + * + * @public + */ export async function createRouter( options: RouterOptions, ): Promise { From cd77cbd48f487374fbfad985ecdd920b65b35e40 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 22 Nov 2021 15:20:59 +0000 Subject: [PATCH 09/22] permission-backend: clean up PermissionIntegrationClient applyConditions signature Signed-off-by: Mike Lewis --- .../PermissionIntegrationClient.test.ts | 20 ++++++++++++------- .../service/PermissionIntegrationClient.ts | 15 +++++++++----- .../permission-backend/src/service/router.ts | 6 ++++-- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts b/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts index d062bccb33..325ee7c21c 100644 --- a/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts +++ b/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts @@ -71,8 +71,9 @@ describe('PermissionIntegrationClient', () => { }); it('should make a POST request to the correct endpoint', async () => { - await client.applyConditions('testResource1', { + await client.applyConditions({ pluginId: 'test-plugin', + resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, }); @@ -81,8 +82,9 @@ describe('PermissionIntegrationClient', () => { }); it('should include a request body', async () => { - await client.applyConditions('testResource1', { + await client.applyConditions({ pluginId: 'test-plugin', + resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, }); @@ -101,8 +103,9 @@ describe('PermissionIntegrationClient', () => { }); it('should return the response from the fetch request', async () => { - const response = await client.applyConditions('testResource1', { + const response = await client.applyConditions({ pluginId: 'test-plugin', + resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, }); @@ -113,8 +116,9 @@ describe('PermissionIntegrationClient', () => { }); it('should not include authorization headers if no token is supplied', async () => { - await client.applyConditions('testResource1', { + await client.applyConditions({ pluginId: 'test-plugin', + resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, }); @@ -125,9 +129,9 @@ describe('PermissionIntegrationClient', () => { it('should include correctly-constructed authorization header if token is supplied', async () => { await client.applyConditions( - 'testResource1', { pluginId: 'test-plugin', + resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, }, @@ -146,8 +150,9 @@ describe('PermissionIntegrationClient', () => { ); await expect( - client.applyConditions('testResource1', { + client.applyConditions({ pluginId: 'test-plugin', + resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, }), @@ -162,8 +167,9 @@ describe('PermissionIntegrationClient', () => { ); await expect( - client.applyConditions('testResource1', { + client.applyConditions({ pluginId: 'test-plugin', + resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, }), diff --git a/plugins/permission-backend/src/service/PermissionIntegrationClient.ts b/plugins/permission-backend/src/service/PermissionIntegrationClient.ts index dd0aad1215..0076edd05f 100644 --- a/plugins/permission-backend/src/service/PermissionIntegrationClient.ts +++ b/plugins/permission-backend/src/service/PermissionIntegrationClient.ts @@ -40,8 +40,13 @@ export class PermissionIntegrationClient { } async applyConditions( - resourceRef: string, - conditions: { + { + pluginId, + resourceRef, + resourceType, + conditions, + }: { + resourceRef: string; pluginId: string; resourceType: string; conditions: PermissionCriteria; @@ -49,13 +54,13 @@ export class PermissionIntegrationClient { authHeader?: string, ): Promise { const endpoint = `${await this.discovery.getBaseUrl( - conditions.pluginId, + pluginId, )}/permissions/apply-conditions`; const request: ApplyConditionsRequest = { resourceRef, - resourceType: conditions.resourceType, - conditions: conditions.conditions, + resourceType, + conditions, }; const response = await fetch(endpoint, { diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index 9110295f77..e5a12c5987 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -68,8 +68,10 @@ const handleRequest = async ( return { id, ...(await permissionIntegrationClient.applyConditions( - resourceRef, - response.conditions, + { + resourceRef, + ...response.conditions, + }, authHeader, )), }; From 7f4a77a884a70ece7862015c073819580e1d584b Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 22 Nov 2021 15:35:46 +0000 Subject: [PATCH 10/22] permission-backend: add todo Signed-off-by: Mike Lewis --- plugins/permission-backend/src/service/router.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index e5a12c5987..05b28f2ea1 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -80,6 +80,9 @@ const handleRequest = async ( return { id, result: AuthorizeResult.CONDITIONAL, + // TODO(mtlewis): this .conditions.conditions situation is a bit awkward. I think it's + // worth exploring a bit of reorganization of the ConditionalPolicyResult type so that + // the naming of property chains like this makes a bit more sense. conditions: response.conditions.conditions, }; } From 2335e0a01f6fce9aaff9733c8bf9ef86124f24f0 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 22 Nov 2021 15:38:36 +0000 Subject: [PATCH 11/22] permission-backend: update default port for standalone server Signed-off-by: Mike Lewis --- plugins/permission-backend/src/run.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/permission-backend/src/run.ts b/plugins/permission-backend/src/run.ts index addfdfd6d7..53d4e4334a 100644 --- a/plugins/permission-backend/src/run.ts +++ b/plugins/permission-backend/src/run.ts @@ -18,7 +18,7 @@ import { getRootLogger } from '@backstage/backend-common'; import yn from 'yn'; import { startStandaloneServer } from './service/standaloneServer'; -const port = process.env.PLUGIN_PORT ? Number(process.env.PLUGIN_PORT) : 7000; +const port = process.env.PLUGIN_PORT ? Number(process.env.PLUGIN_PORT) : 7007; const enableCors = yn(process.env.PLUGIN_CORS, { default: false }); const logger = getRootLogger(); From dbd89112a581053599927864ae3fd1d04db18b5c Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 22 Nov 2021 17:36:21 +0000 Subject: [PATCH 12/22] permission-backend: add permission-node integration tests Signed-off-by: Mike Lewis --- .../PermissionIntegrationClient.test.ts | 175 ++++++++++++++---- 1 file changed, 144 insertions(+), 31 deletions(-) diff --git a/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts b/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts index 325ee7c21c..8e3e85b5af 100644 --- a/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts +++ b/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts @@ -14,50 +14,54 @@ * limitations under the License. */ +import { AddressInfo } from 'net'; +import { Server } from 'http'; +import express, { Router } from 'express'; import { RestContext, rest } from 'msw'; -import { setupServer } from 'msw/node'; +import { setupServer, SetupServerApi } from 'msw/node'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; import { AuthorizeResult } from '@backstage/plugin-permission-common'; +import { createPermissionIntegrationRouter } from '@backstage/plugin-permission-node'; import { PermissionIntegrationClient } from './PermissionIntegrationClient'; -const server = setupServer(); - -const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; -const discovery: PluginEndpointDiscovery = { - async getBaseUrl() { - return mockBaseUrl; - }, - async getExternalBaseUrl() { - throw new Error('Not implemented.'); - }, -}; - -const client: PermissionIntegrationClient = new PermissionIntegrationClient({ - discovery, -}); - -const mockConditions = { - not: { - allOf: [ - { rule: 'RULE_1', params: [] }, - { rule: 'RULE_2', params: ['abc'] }, - ], - }, -}; - describe('PermissionIntegrationClient', () => { - beforeAll(() => server.listen({ onUnhandledRequest: 'error' })); - afterAll(() => server.close()); - afterEach(() => server.resetHandlers()); - describe('applyConditions', () => { + let server: SetupServerApi; + + const mockConditions = { + not: { + allOf: [ + { rule: 'RULE_1', params: [] }, + { rule: 'RULE_2', params: ['abc'] }, + ], + }, + }; + const mockApplyConditionsHandler = jest.fn( (_req, res, { json }: RestContext) => { return res(json({ result: AuthorizeResult.ALLOW })); }, ); - beforeEach(() => { + const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; + const discovery: PluginEndpointDiscovery = { + async getBaseUrl() { + return mockBaseUrl; + }, + async getExternalBaseUrl() { + throw new Error('Not implemented.'); + }, + }; + + const client: PermissionIntegrationClient = new PermissionIntegrationClient( + { + discovery, + }, + ); + + beforeAll(() => { + server = setupServer(); + server.listen({ onUnhandledRequest: 'error' }); server.use( rest.post( `${mockBaseUrl}/permissions/apply-conditions`, @@ -66,6 +70,8 @@ describe('PermissionIntegrationClient', () => { ); }); + afterAll(() => server.close()); + afterEach(() => { jest.clearAllMocks(); }); @@ -176,4 +182,111 @@ describe('PermissionIntegrationClient', () => { ).rejects.toThrowError(/invalid input/i); }); }); + + describe('integration with @backstage/plugin-permission-node', () => { + let server: Server; + let client: PermissionIntegrationClient; + + beforeAll(async () => { + const router = Router(); + + router.use( + createPermissionIntegrationRouter({ + resourceType: 'test-resource', + getResource: async resourceRef => ({ id: resourceRef }), + rules: [ + { + name: 'RULE_1', + description: 'Test rule 1', + apply: (_resource: any, input: 'yes' | 'no') => input === 'yes', + toQuery: () => { + throw new Error('Not implemented'); + }, + }, + { + name: 'RULE_2', + description: 'Test rule 2', + apply: (_resource: any, input: 'yes' | 'no') => input === 'yes', + toQuery: () => { + throw new Error('Not implemented'); + }, + }, + ], + }), + ); + + const app = express(); + + app.use('/test-plugin', router); + + await new Promise(resolve => { + server = app.listen(resolve); + }); + + const discovery: PluginEndpointDiscovery = { + async getBaseUrl(pluginId: string) { + const listenPort = (server.address()! as AddressInfo).port; + + return `http://0.0.0.0:${listenPort}/${pluginId}`; + }, + async getExternalBaseUrl() { + throw new Error('Not implemented.'); + }, + }; + + client = new PermissionIntegrationClient({ + discovery, + }); + }); + + afterAll( + async () => + new Promise((resolve, reject) => + server.close(err => (err ? reject(err) : resolve())), + ), + ); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('works for simple conditions', async () => { + await expect( + client.applyConditions({ + pluginId: 'test-plugin', + resourceRef: 'testResource1', + resourceType: 'test-resource', + conditions: { rule: 'RULE_1', params: ['no'] }, + }), + ).resolves.toEqual({ result: AuthorizeResult.DENY }); + }); + + it('works for complex criteria', async () => { + await expect( + client.applyConditions({ + pluginId: 'test-plugin', + resourceRef: 'testResource1', + resourceType: 'test-resource', + conditions: { + allOf: [ + { + allOf: [ + { rule: 'RULE_1', params: ['yes'] }, + { not: { rule: 'RULE_2', params: ['no'] } }, + ], + }, + { + not: { + allOf: [ + { rule: 'RULE_1', params: ['no'] }, + { rule: 'RULE_2', params: ['yes'] }, + ], + }, + }, + ], + }, + }), + ).resolves.toEqual({ result: AuthorizeResult.ALLOW }); + }); + }); }); From f5c32111291dc3fa8941a6ac4e00297f95e2cd12 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Tue, 23 Nov 2021 13:51:16 +0000 Subject: [PATCH 13/22] permission-backend: 500 when policy returns incorrect resourceType Signed-off-by: Mike Lewis --- plugins/permission-backend/src/service/router.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index 05b28f2ea1..13f6f7d3d4 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -25,7 +25,6 @@ import { BackstageIdentity, IdentityClient, } from '@backstage/plugin-auth-backend'; -import { ConflictError } from '@backstage/errors'; import { AuthorizeResult, AuthorizeResponse, @@ -59,7 +58,7 @@ const handleRequest = async ( if (response.result === AuthorizeResult.CONDITIONAL) { // Sanity check that any resource provided matches the one expected by the permission if (request.permission.resourceType !== response.conditions.resourceType) { - throw new ConflictError( + throw new Error( `Invalid resource conditions returned from permission policy for permission ${request.permission.name}`, ); } From dfd3b25c34383c123c1f02a8fa188300d836afdd Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 24 Nov 2021 14:32:31 +0000 Subject: [PATCH 14/22] permission-backend: set version to 0.0.0 before initial release Signed-off-by: Mike Lewis --- plugins/permission-backend/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/permission-backend/package.json b/plugins/permission-backend/package.json index dae7fdb5af..01255c00c6 100644 --- a/plugins/permission-backend/package.json +++ b/plugins/permission-backend/package.json @@ -1,6 +1,6 @@ { "name": "@backstage/plugin-permission-backend", - "version": "0.1.0", + "version": "0.0.0", "main": "src/index.ts", "types": "src/index.ts", "license": "Apache-2.0", From 8f835847061941a6aeff4ef8bf2bafb79170e371 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 24 Nov 2021 14:35:10 +0000 Subject: [PATCH 15/22] permission-backend: remove standaloneServer, run.ts and AllowAllPermissionPolicy The run.ts / standaloneServer.ts stuff is a hangover and expected to be removed or heavily adjusted in the future, and since it's intended to run from the command line, there's no neat way to pass in a permission policy. As such it's not going to be useful in its current form, so we're removing it for now. Signed-off-by: Mike Lewis --- plugins/permission-backend/src/run.ts | 33 ----------- .../service/AllowAllPermissionPolicy.test.ts | 35 ----------- .../src/service/AllowAllPermissionPolicy.ts | 34 ----------- .../src/service/standaloneServer.ts | 59 ------------------- 4 files changed, 161 deletions(-) delete mode 100644 plugins/permission-backend/src/run.ts delete mode 100644 plugins/permission-backend/src/service/AllowAllPermissionPolicy.test.ts delete mode 100644 plugins/permission-backend/src/service/AllowAllPermissionPolicy.ts delete mode 100644 plugins/permission-backend/src/service/standaloneServer.ts diff --git a/plugins/permission-backend/src/run.ts b/plugins/permission-backend/src/run.ts deleted file mode 100644 index 53d4e4334a..0000000000 --- a/plugins/permission-backend/src/run.ts +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright 2021 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { getRootLogger } from '@backstage/backend-common'; -import yn from 'yn'; -import { startStandaloneServer } from './service/standaloneServer'; - -const port = process.env.PLUGIN_PORT ? Number(process.env.PLUGIN_PORT) : 7007; -const enableCors = yn(process.env.PLUGIN_CORS, { default: false }); -const logger = getRootLogger(); - -startStandaloneServer({ port, enableCors, logger }).catch(err => { - logger.error(err); - process.exit(1); -}); - -process.on('SIGINT', () => { - logger.info('CTRL+C pressed; exiting.'); - process.exit(0); -}); diff --git a/plugins/permission-backend/src/service/AllowAllPermissionPolicy.test.ts b/plugins/permission-backend/src/service/AllowAllPermissionPolicy.test.ts deleted file mode 100644 index 5ebebd04e5..0000000000 --- a/plugins/permission-backend/src/service/AllowAllPermissionPolicy.test.ts +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright 2021 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { - AuthorizeResult, - Permission, -} from '@backstage/plugin-permission-common'; -import { AllowAllPermissionPolicy } from './AllowAllPermissionPolicy'; - -describe('AllowAllPermissionPolicy', () => { - const policy = new AllowAllPermissionPolicy(); - - it('returns ALLOW when no identity is passed', async () => { - await expect( - policy.handle({ - permission: { - name: 'any.permission', - } as Permission, - }), - ).resolves.toEqual({ result: AuthorizeResult.ALLOW }); - }); -}); diff --git a/plugins/permission-backend/src/service/AllowAllPermissionPolicy.ts b/plugins/permission-backend/src/service/AllowAllPermissionPolicy.ts deleted file mode 100644 index bb39d4a334..0000000000 --- a/plugins/permission-backend/src/service/AllowAllPermissionPolicy.ts +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright 2021 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { BackstageIdentity } from '@backstage/plugin-auth-backend'; -import { AuthorizeResult } from '@backstage/plugin-permission-common'; -import { - PermissionPolicy, - PolicyAuthorizeRequest, - PolicyResult, -} from '@backstage/plugin-permission-node'; - -export class AllowAllPermissionPolicy implements PermissionPolicy { - async handle( - _request: PolicyAuthorizeRequest, - _user?: BackstageIdentity, - ): Promise { - return { - result: AuthorizeResult.ALLOW, - }; - } -} diff --git a/plugins/permission-backend/src/service/standaloneServer.ts b/plugins/permission-backend/src/service/standaloneServer.ts deleted file mode 100644 index a2478ac57b..0000000000 --- a/plugins/permission-backend/src/service/standaloneServer.ts +++ /dev/null @@ -1,59 +0,0 @@ -/* - * Copyright 2021 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { - createServiceBuilder, - loadBackendConfig, - SingleHostDiscovery, -} from '@backstage/backend-common'; -import { Server } from 'http'; -import { Logger } from 'winston'; -import { AllowAllPermissionPolicy } from './AllowAllPermissionPolicy'; -import { createRouter } from './router'; - -export interface ServerOptions { - port: number; - enableCors: boolean; - logger: Logger; -} - -export async function startStandaloneServer( - options: ServerOptions, -): Promise { - const logger = options.logger.child({ service: 'permission-backend' }); - const config = await loadBackendConfig({ logger, argv: process.argv }); - const discovery = SingleHostDiscovery.fromConfig(config); - logger.debug('Starting application server...'); - const router = await createRouter({ - logger, - discovery, - policy: new AllowAllPermissionPolicy(), - }); - - let service = createServiceBuilder(module) - .setPort(options.port) - .addRouter('/permission', router); - if (options.enableCors) { - service = service.enableCors({ origin: 'http://localhost:3000' }); - } - - return await service.start().catch(err => { - logger.error(err); - process.exit(1); - }); -} - -module.hot?.accept(); From b5e43875d5afe3e7feb21a1d1ae1556a975386bc Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 24 Nov 2021 14:47:51 +0000 Subject: [PATCH 16/22] permission-backend: enumerate exports in service index Signed-off-by: Mike Lewis --- plugins/permission-backend/src/service/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/permission-backend/src/service/index.ts b/plugins/permission-backend/src/service/index.ts index b26567988c..3ce8cdbcc4 100644 --- a/plugins/permission-backend/src/service/index.ts +++ b/plugins/permission-backend/src/service/index.ts @@ -14,4 +14,5 @@ * limitations under the License. */ -export * from './router'; +export { createRouter } from './router'; +export type { RouterOptions } from './router'; From 1ff5d10a6251aeb21d163494cae0b5e958054f6a Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 24 Nov 2021 14:53:20 +0000 Subject: [PATCH 17/22] permission-backend: update dependency on permission-node to 0.0.0 to match unreleased version Signed-off-by: Mike Lewis --- plugins/permission-backend/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/permission-backend/package.json b/plugins/permission-backend/package.json index 01255c00c6..c4c9aa11ad 100644 --- a/plugins/permission-backend/package.json +++ b/plugins/permission-backend/package.json @@ -24,7 +24,7 @@ "@backstage/errors": "^0.1.4", "@backstage/plugin-auth-backend": "^0.4.7", "@backstage/plugin-permission-common": "^0.1.0", - "@backstage/plugin-permission-node": "^0.1.0", + "@backstage/plugin-permission-node": "^0.0.0", "@types/express": "*", "cross-fetch": "^3.0.6", "express": "^4.17.1", From 12b0eebfb565990390ca41a864ca288c0b6e5570 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 24 Nov 2021 15:18:50 +0000 Subject: [PATCH 18/22] permission-backend: pass in identity client Signed-off-by: Mike Lewis --- plugins/permission-backend/api-report.md | 3 ++ .../src/service/router.test.ts | 36 ++++++++----------- .../permission-backend/src/service/router.ts | 8 ++--- 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/plugins/permission-backend/api-report.md b/plugins/permission-backend/api-report.md index 7f82913e18..4857c8b044 100644 --- a/plugins/permission-backend/api-report.md +++ b/plugins/permission-backend/api-report.md @@ -4,6 +4,7 @@ ```ts import express from 'express'; +import { IdentityClient } from '@backstage/plugin-auth-backend'; import { Logger as Logger_2 } from 'winston'; import { PermissionPolicy } from '@backstage/plugin-permission-node'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; @@ -16,6 +17,8 @@ export interface RouterOptions { // (undocumented) discovery: PluginEndpointDiscovery; // (undocumented) + identity: IdentityClient; + // (undocumented) logger: Logger_2; // (undocumented) policy: PermissionPolicy; diff --git a/plugins/permission-backend/src/service/router.test.ts b/plugins/permission-backend/src/service/router.test.ts index 13261125c8..29b5245006 100644 --- a/plugins/permission-backend/src/service/router.test.ts +++ b/plugins/permission-backend/src/service/router.test.ts @@ -17,6 +17,7 @@ import express from 'express'; import request from 'supertest'; import { getVoidLogger } from '@backstage/backend-common'; +import { IdentityClient } from '@backstage/plugin-auth-backend'; import { AuthorizeResult, Permission, @@ -25,27 +26,6 @@ import { PermissionPolicy } from '@backstage/plugin-permission-node'; import { createRouter } from './router'; -jest.mock('@backstage/plugin-auth-backend', () => { - class MockIdentityClient { - authenticate = jest.fn(token => - Promise.resolve( - token - ? { - id: 'test-user', - token, - } - : undefined, - ), - ); - - static getBearerToken = jest.fn(authHeader => - authHeader ? `` : undefined, - ); - } - - return { IdentityClient: MockIdentityClient }; -}); - const policy: PermissionPolicy = { handle: jest.fn().mockImplementation((_req, identity) => { if (identity) { @@ -70,6 +50,18 @@ describe('createRouter', () => { getBaseUrl: jest.fn(), getExternalBaseUrl: jest.fn(), }, + identity: { + authenticate: jest.fn(token => { + if (!token) { + throw new Error('No token supplied!'); + } + + return Promise.resolve({ + id: 'test-user', + token, + }); + }), + } as unknown as IdentityClient, policy, }); @@ -108,7 +100,7 @@ describe('createRouter', () => { expect(response.status).toEqual(200); expect(policy.handle).toHaveBeenCalledWith( { permission }, - { id: 'test-user', token: '' }, + { id: 'test-user', token: 'test-token' }, ); expect(response.body).toEqual([ { id: 123, result: AuthorizeResult.ALLOW }, diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index 13f6f7d3d4..0fd72297c5 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -44,6 +44,7 @@ export interface RouterOptions { logger: Logger; discovery: PluginEndpointDiscovery; policy: PermissionPolicy; + identity: IdentityClient; } const handleRequest = async ( @@ -98,12 +99,7 @@ const handleRequest = async ( export async function createRouter( options: RouterOptions, ): Promise { - const { policy, discovery } = options; - - const identity = new IdentityClient({ - discovery, - issuer: await discovery.getExternalBaseUrl('auth'), - }); + const { policy, discovery, identity } = options; const permissionIntegrationClient = new PermissionIntegrationClient({ discovery, From fafb482d730751ba07806ec8830c46bb2e872444 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 24 Nov 2021 15:27:42 +0000 Subject: [PATCH 19/22] permission-backend: add request validation Signed-off-by: Mike Lewis --- .../permission-backend/src/service/router.ts | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index 0fd72297c5..8caba23b50 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -14,13 +14,14 @@ * limitations under the License. */ +import { z } from 'zod'; +import express, { Request, Response } from 'express'; +import Router from 'express-promise-router'; +import { Logger } from 'winston'; import { errorHandler, PluginEndpointDiscovery, } from '@backstage/backend-common'; -import express, { Request, Response } from 'express'; -import Router from 'express-promise-router'; -import { Logger } from 'winston'; import { BackstageIdentity, IdentityClient, @@ -34,6 +35,27 @@ import { import { PermissionPolicy } from '@backstage/plugin-permission-node'; import { PermissionIntegrationClient } from './PermissionIntegrationClient'; +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(), + }), + }), + }), +); + /** * Options required when constructing a new {@link express#Router} using * {@link createRouter}. @@ -121,7 +143,7 @@ export async function createRouter( const token = IdentityClient.getBearerToken(req.header('authorization')); const user = token ? await identity.authenticate(token) : undefined; - const body: Identified[] = req.body; + const body = requestSchema.parse(req.body); res.json( await Promise.all( From 2bacc8be347311f2ffe231b42e349a8ccf8410cb Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 24 Nov 2021 16:46:19 +0000 Subject: [PATCH 20/22] permission-backend: expand router test suite Signed-off-by: Mike Lewis --- .../src/service/router.test.ts | 222 ++++++++++++++++-- 1 file changed, 205 insertions(+), 17 deletions(-) diff --git a/plugins/permission-backend/src/service/router.test.ts b/plugins/permission-backend/src/service/router.test.ts index 29b5245006..78e7121da6 100644 --- a/plugins/permission-backend/src/service/router.test.ts +++ b/plugins/permission-backend/src/service/router.test.ts @@ -18,15 +18,22 @@ import express from 'express'; import request from 'supertest'; import { getVoidLogger } from '@backstage/backend-common'; import { IdentityClient } from '@backstage/plugin-auth-backend'; -import { - AuthorizeResult, - Permission, -} from '@backstage/plugin-permission-common'; -import { PermissionPolicy } from '@backstage/plugin-permission-node'; +import { AuthorizeResult } from '@backstage/plugin-permission-common'; +import { ApplyConditionsResponse } from '@backstage/plugin-permission-node'; +import { PermissionIntegrationClient } from './PermissionIntegrationClient'; import { createRouter } from './router'; -const policy: PermissionPolicy = { +const mockApplyConditions: jest.MockedFunction< + InstanceType['applyConditions'] +> = jest.fn(); +jest.mock('./PermissionIntegrationClient', () => ({ + PermissionIntegrationClient: jest.fn(() => ({ + applyConditions: mockApplyConditions, + })), +})); + +const policy = { handle: jest.fn().mockImplementation((_req, identity) => { if (identity) { return { result: AuthorizeResult.ALLOW }; @@ -35,11 +42,6 @@ const policy: PermissionPolicy = { }), }; -const permission: Permission = { - name: 'test.permission', - attributes: { action: 'read' }, -}; - describe('createRouter', () => { let app: express.Express; @@ -81,12 +83,47 @@ describe('createRouter', () => { it('calls the permission policy', async () => { const response = await request(app) .post('/authorize') - .send([{ id: 123, permission }]); + .send([ + { + id: '123', + permission: { + name: 'test.permission1', + attributes: {}, + }, + }, + { + id: '234', + permission: { + name: 'test.permission2', + attributes: {}, + }, + }, + ]); expect(response.status).toEqual(200); - expect(policy.handle).toHaveBeenCalledWith({ permission }, undefined); + + expect(policy.handle).toHaveBeenCalledWith( + { + permission: { + name: 'test.permission1', + attributes: {}, + }, + }, + undefined, + ); + expect(policy.handle).toHaveBeenCalledWith( + { + permission: { + name: 'test.permission2', + attributes: {}, + }, + }, + undefined, + ); + expect(response.body).toEqual([ - { id: 123, result: AuthorizeResult.DENY }, + { id: '123', result: AuthorizeResult.DENY }, + { id: '234', result: AuthorizeResult.DENY }, ]); }); @@ -95,16 +132,167 @@ describe('createRouter', () => { const response = await request(app) .post('/authorize') .auth(token, { type: 'bearer' }) - .send([{ id: 123, permission }]); + .send([ + { + id: '123', + permission: { + name: 'test.permission', + attributes: {}, + }, + }, + ]); expect(response.status).toEqual(200); expect(policy.handle).toHaveBeenCalledWith( - { permission }, + { + permission: { + name: 'test.permission', + attributes: {}, + }, + }, { id: 'test-user', token: 'test-token' }, ); expect(response.body).toEqual([ - { id: 123, result: AuthorizeResult.ALLOW }, + { id: '123', result: AuthorizeResult.ALLOW }, ]); }); + + describe('conditional policy result', () => { + beforeEach(() => { + policy.handle.mockReturnValueOnce({ + result: AuthorizeResult.CONDITIONAL, + conditions: { + pluginId: 'test-plugin', + resourceType: 'test-resource-1', + conditions: { + anyOf: [{ rule: 'test-rule', params: ['abc'] }], + }, + }, + }); + }); + + it('returns conditions if no resourceRef is supplied', async () => { + const response = await request(app) + .post('/authorize') + .send([ + { + 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, + conditions: { anyOf: [{ rule: 'test-rule', params: ['abc'] }] }, + }, + ]); + }); + + it.each([ + AuthorizeResult.ALLOW, + AuthorizeResult.DENY, + ])( + 'applies conditions and returns %s if resourceRef is supplied', + async result => { + mockApplyConditions.mockResolvedValueOnce({ + result, + }); + + 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: {}, + }, + }, + ]); + + expect(mockApplyConditions).toHaveBeenCalledWith( + { + pluginId: 'test-plugin', + resourceType: 'test-resource-1', + resourceRef: 'test/resource', + conditions: { anyOf: [{ rule: 'test-rule', params: ['abc'] }] }, + }, + 'Bearer test-token', + ); + + expect(response.status).toEqual(200); + expect(response.body).toEqual([ + { + id: '123', + result, + }, + ]); + }, + ); + }); + + it.each([ + undefined, + '', + {}, + [{ permission: { name: 'test.permission', attributes: {} } }], + [{ id: '123' }], + [{ id: '123', permission: { name: 'test.permission' } }], + [{ id: '123', permission: { attributes: { invalid: 'attribute' } } }], + ])('returns a 500 error for invalid request %#', async requestBody => { + const response = await request(app).post('/authorize').send(requestBody); + + expect(response.status).toEqual(500); + expect(response.body).toEqual( + expect.objectContaining({ + error: expect.objectContaining({ + message: expect.stringMatching(/invalid/i), + }), + }), + ); + }); + + it('returns a 500 error if the policy returns a different resourceType', async () => { + policy.handle.mockReturnValueOnce({ + result: AuthorizeResult.CONDITIONAL, + conditions: { + pluginId: 'test-plugin', + resourceType: 'test-resource-2', + conditions: {}, + }, + }); + + const response = await request(app) + .post('/authorize') + .send([ + { + id: '123', + permission: { + name: 'test.permission', + resourceType: 'test-resource-1', + attributes: {}, + }, + }, + ]); + + expect(response.status).toEqual(500); + expect(response.body).toEqual( + expect.objectContaining({ + error: expect.objectContaining({ + message: expect.stringMatching(/invalid resource conditions/i), + }), + }), + ); + }); }); }); From 7a8312f126855bf2f779d6819024265662c84bd4 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Thu, 25 Nov 2021 09:31:45 +0000 Subject: [PATCH 21/22] permission-backend: add changeset Signed-off-by: Mike Lewis --- .changeset/olive-pumpkins-burn.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/olive-pumpkins-burn.md diff --git a/.changeset/olive-pumpkins-burn.md b/.changeset/olive-pumpkins-burn.md new file mode 100644 index 0000000000..3cd9c46267 --- /dev/null +++ b/.changeset/olive-pumpkins-burn.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-permission-backend': minor +--- + +New package containing the backend for authorization and permissions. For more information, see the [authorization PRFC](https://github.com/backstage/backstage/pull/7761). From 2e5a67b77d63cda79a2c1bfd929061d13d8144ad Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Thu, 25 Nov 2021 09:50:24 +0000 Subject: [PATCH 22/22] permission-backend: replace cross-fetch with node-fetch Signed-off-by: Mike Lewis --- plugins/permission-backend/package.json | 3 +-- .../src/service/PermissionIntegrationClient.test.ts | 2 +- .../src/service/PermissionIntegrationClient.ts | 7 ++++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/permission-backend/package.json b/plugins/permission-backend/package.json index c4c9aa11ad..f30c73f8eb 100644 --- a/plugins/permission-backend/package.json +++ b/plugins/permission-backend/package.json @@ -21,14 +21,13 @@ "dependencies": { "@backstage/backend-common": "^0.9.9", "@backstage/config": "^0.1.11", - "@backstage/errors": "^0.1.4", "@backstage/plugin-auth-backend": "^0.4.7", "@backstage/plugin-permission-common": "^0.1.0", "@backstage/plugin-permission-node": "^0.0.0", "@types/express": "*", - "cross-fetch": "^3.0.6", "express": "^4.17.1", "express-promise-router": "^4.1.0", + "node-fetch": "^2.6.1", "winston": "^3.2.1", "yn": "^4.0.0", "zod": "^3.11.6" diff --git a/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts b/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts index 8e3e85b5af..6a35488117 100644 --- a/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts +++ b/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts @@ -162,7 +162,7 @@ describe('PermissionIntegrationClient', () => { resourceType: 'test-resource', conditions: mockConditions, }), - ).rejects.toThrowError(/request failed with 401/i); + ).rejects.toThrowError(/401/i); }); it('should reject invalid responses', async () => { diff --git a/plugins/permission-backend/src/service/PermissionIntegrationClient.ts b/plugins/permission-backend/src/service/PermissionIntegrationClient.ts index 0076edd05f..0d6017cfd3 100644 --- a/plugins/permission-backend/src/service/PermissionIntegrationClient.ts +++ b/plugins/permission-backend/src/service/PermissionIntegrationClient.ts @@ -14,9 +14,8 @@ * limitations under the License. */ -import fetch from 'cross-fetch'; +import fetch from 'node-fetch'; import { z } from 'zod'; -import { ResponseError } from '@backstage/errors'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; import { AuthorizeResult, @@ -73,7 +72,9 @@ export class PermissionIntegrationClient { }); if (!response.ok) { - throw await ResponseError.fromResponse(response); + throw new Error( + `Unexpected response from plugin upstream when applying conditions. Expected 200 but got ${response.status} - ${response.statusText}`, + ); } return responseSchema.parse(await response.json());