diff --git a/docs/tutorials/backend-to-backend-auth.md b/docs/tutorials/backend-to-backend-auth.md index 6163d4f592..cd73a95c89 100644 --- a/docs/tutorials/backend-to-backend-auth.md +++ b/docs/tutorials/backend-to-backend-auth.md @@ -29,7 +29,10 @@ backend-to-backend authentication to work in production. Requests originating from a backend plugin can be authenticated by decorating them with a backend token. Backend tokens can be generated using a `TokenManager`, which can be passed to plugin backends via the -`PluginEnvironment`. +`PluginEnvironment`. The `TokenManager` provided in new Backstage instances +generated by `create-app` is a stub, which returns empty tokens and accepts any +input string as valid. To enable backend-to-backend authentication, you'll need +to instantiate a new one using the secret from your config instead: ```diff // packages/backend/src/index.ts @@ -43,6 +46,7 @@ function makeCreateEnv(config: Config) { const cacheManager = CacheManager.fromConfig(config); const databaseManager = DatabaseManager.fromConfig(config); +- const tokenManager = ServerTokenManager.noop(); + const tokenManager = ServerTokenManager.fromConfig(config, { logger: root }); ``` diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index cd44601d8c..b929251890 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -501,6 +501,8 @@ export class ServerTokenManager implements TokenManager { getToken(): Promise<{ token: string; }>; + // (undocumented) + static noop(): TokenManager; } // @public (undocumented) diff --git a/packages/backend-common/src/tokens/ServerTokenManager.test.ts b/packages/backend-common/src/tokens/ServerTokenManager.test.ts index fb82eada29..1799291544 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.test.ts +++ b/packages/backend-common/src/tokens/ServerTokenManager.test.ts @@ -18,6 +18,7 @@ import { ConfigReader } from '@backstage/config'; import { ServerTokenManager } from './ServerTokenManager'; import { Logger } from 'winston'; import { JWK } from 'jose'; +import { TokenManager } from './types'; const emptyConfig = new ConfigReader({}); const configWithSecret = new ConfigReader({ @@ -43,6 +44,11 @@ describe('ServerTokenManager', () => { }); expect((await tokenManager.getToken()).token).toBeDefined(); }); + + it('should return a token string if using a noop TokenManager', async () => { + const tokenManager = ServerTokenManager.noop(); + expect((await tokenManager.getToken()).token).toBeDefined(); + }); }); describe('authenticate', () => { @@ -126,6 +132,22 @@ describe('ServerTokenManager', () => { ); }); + it('should throw for server tokens created using a noop TokenManager', async () => { + const noopTokenManager = ServerTokenManager.noop(); + const tokenManager = ServerTokenManager.fromConfig( + new ConfigReader({ + backend: { auth: { keys: [{ secret: 'a1b2c3' }] } }, + }), + { logger }, + ); + + const { token } = await noopTokenManager.getToken(); + + await expect(tokenManager.authenticate(token)).rejects.toThrowError( + /invalid server token/i, + ); + }); + it('should throw for server tokens created by a different generated secret', async () => { (process.env as any).NODE_ENV = 'development'; const tokenManager1 = ServerTokenManager.fromConfig( @@ -146,7 +168,7 @@ describe('ServerTokenManager', () => { }); }); - describe('default', () => { + describe('fromConfig', () => { describe('NODE_ENV === production', () => { it('should throw if backend auth configuration is missing', () => { expect(() => @@ -215,4 +237,38 @@ describe('ServerTokenManager', () => { }); }); }); + + describe('ServerTokenManager.noop', () => { + let noopTokenManager: TokenManager; + + beforeEach(() => { + noopTokenManager = ServerTokenManager.noop(); + }); + + it('should accept tokens it generates', async () => { + const { token } = await noopTokenManager.getToken(); + + await expect(noopTokenManager.authenticate(token)).resolves.not.toThrow(); + }); + + it('should accept tokens generated by other noop token managers', async () => { + const noopTokenManager2 = ServerTokenManager.noop(); + await expect( + noopTokenManager.authenticate( + ( + await noopTokenManager2.getToken() + ).token, + ), + ).resolves.not.toThrow(); + }); + + it('should accept signed tokens', async () => { + const tokenManager = ServerTokenManager.fromConfig(configWithSecret, { + logger, + }); + await expect( + noopTokenManager.authenticate((await tokenManager.getToken()).token), + ).resolves.not.toThrow(); + }); + }); }); diff --git a/packages/backend-common/src/tokens/ServerTokenManager.ts b/packages/backend-common/src/tokens/ServerTokenManager.ts index 0b7ea49dce..7b50ff4300 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.ts +++ b/packages/backend-common/src/tokens/ServerTokenManager.ts @@ -20,6 +20,14 @@ import { AuthenticationError } from '@backstage/errors'; import { TokenManager } from './types'; import { Logger } from 'winston'; +class NoopTokenManager implements TokenManager { + async getToken() { + return { token: '' }; + } + + async authenticate() {} +} + /** * Creates and validates tokens for use during backend-to-backend * authentication. @@ -30,6 +38,10 @@ export class ServerTokenManager implements TokenManager { private readonly verificationKeys: JWKS.KeyStore; private readonly signingKey: JWK.Key; + static noop(): TokenManager { + return new NoopTokenManager(); + } + static fromConfig(config: Config, options: { logger: Logger }) { const { logger } = options; diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 96def8d03f..c7d9d9b5ee 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -63,7 +63,7 @@ function makeCreateEnv(config: Config) { const root = getRootLogger(); const reader = UrlReaders.default({ logger: root, config }); const discovery = SingleHostDiscovery.fromConfig(config); - const tokenManager = ServerTokenManager.fromConfig(config, { logger: root }); + const tokenManager = ServerTokenManager.noop(); const permissions = new ServerPermissionClient({ discoveryApi: discovery, configApi: config, diff --git a/packages/create-app/templates/default-app/packages/backend/src/index.ts b/packages/create-app/templates/default-app/packages/backend/src/index.ts index 7a2d2a7a34..3f12122a3f 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/index.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/index.ts @@ -38,7 +38,7 @@ function makeCreateEnv(config: Config) { const cacheManager = CacheManager.fromConfig(config); const databaseManager = DatabaseManager.fromConfig(config); - const tokenManager = ServerTokenManager.fromConfig(config, { logger: root }); + const tokenManager = ServerTokenManager.noop(); return (plugin: string): PluginEnvironment => { const logger = root.child({ type: 'plugin', plugin }); diff --git a/plugins/permission-common/api-report.md b/plugins/permission-common/api-report.md index 043b27554c..cc2a433504 100644 --- a/plugins/permission-common/api-report.md +++ b/plugins/permission-common/api-report.md @@ -74,6 +74,10 @@ export class PermissionClient { requests: AuthorizeRequest[], options?: AuthorizeRequestOptions, ): Promise; + // (undocumented) + protected readonly enabled: boolean; + // (undocumented) + protected shouldBypass(_options?: AuthorizeRequestOptions): Promise; } // @public diff --git a/plugins/permission-common/src/PermissionClient.ts b/plugins/permission-common/src/PermissionClient.ts index f98d40a3c4..18e618d5f0 100644 --- a/plugins/permission-common/src/PermissionClient.ts +++ b/plugins/permission-common/src/PermissionClient.ts @@ -72,7 +72,7 @@ export type AuthorizeRequestOptions = { * @public */ export class PermissionClient { - private readonly enabled: boolean; + protected readonly enabled: boolean; private readonly discoveryApi: DiscoveryApi; constructor(options: { discoveryApi: DiscoveryApi; configApi: Config }) { @@ -106,7 +106,7 @@ export class PermissionClient { // but no resourceRef. That way clients who aren't prepared to handle filtering according // to conditions can be guaranteed that they won't unexpectedly get a CONDITIONAL response. - if (!this.enabled) { + if (await this.shouldBypass(options)) { return requests.map(_ => ({ result: AuthorizeResult.ALLOW })); } @@ -141,6 +141,12 @@ export class PermissionClient { return identifiedRequests.map(request => responsesById[request.id]); } + protected async shouldBypass( + _options?: AuthorizeRequestOptions, + ): Promise { + return !this.enabled; + } + private getAuthorizationHeader(token?: string): Record { return token ? { Authorization: `Bearer ${token}` } : {}; } diff --git a/plugins/permission-node/api-report.md b/plugins/permission-node/api-report.md index 367bcd2386..90181925c4 100644 --- a/plugins/permission-node/api-report.md +++ b/plugins/permission-node/api-report.md @@ -5,7 +5,6 @@ ```ts import { AuthorizeRequest } from '@backstage/plugin-permission-common'; import { AuthorizeRequestOptions } from '@backstage/plugin-permission-common'; -import { AuthorizeResponse } from '@backstage/plugin-permission-common'; import { AuthorizeResult } from '@backstage/plugin-permission-common'; import { BackstageIdentityResponse } from '@backstage/plugin-auth-backend'; import { Config } from '@backstage/config'; @@ -14,7 +13,7 @@ import { PermissionClient } from '@backstage/plugin-permission-common'; import { PermissionCondition } from '@backstage/plugin-permission-common'; import { PermissionCriteria } from '@backstage/plugin-permission-common'; import { Router } from 'express'; -import { ServerTokenManager } from '@backstage/backend-common'; +import { TokenManager } from '@backstage/backend-common'; // @public export type ApplyConditionsRequest = { @@ -131,12 +130,9 @@ export class ServerPermissionClient extends PermissionClient { constructor(options: { discoveryApi: DiscoveryApi; configApi: Config; - serverTokenManager: ServerTokenManager; + serverTokenManager: TokenManager; }); // (undocumented) - authorize( - requests: AuthorizeRequest[], - options?: AuthorizeRequestOptions, - ): Promise; + shouldBypass(options?: AuthorizeRequestOptions): Promise; } ``` diff --git a/plugins/permission-node/package.json b/plugins/permission-node/package.json index 7166ae5c31..92755e2012 100644 --- a/plugins/permission-node/package.json +++ b/plugins/permission-node/package.json @@ -40,6 +40,7 @@ "devDependencies": { "@backstage/cli": "^0.10.1", "@types/supertest": "^2.0.8", + "msw": "^0.35.0", "supertest": "^6.1.3" }, "files": [ diff --git a/plugins/permission-node/src/ServerPermissionClient.test.ts b/plugins/permission-node/src/ServerPermissionClient.test.ts new file mode 100644 index 0000000000..8efd6414a7 --- /dev/null +++ b/plugins/permission-node/src/ServerPermissionClient.test.ts @@ -0,0 +1,118 @@ +/* + * 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 { ServerPermissionClient } from '.'; +import { + DiscoveryApi, + Permission, + Identified, + AuthorizeRequest, + AuthorizeResult, +} from '@backstage/plugin-permission-common'; +import { ConfigReader } from '@backstage/config'; +import { getVoidLogger, ServerTokenManager } from '@backstage/backend-common'; +import { setupServer } from 'msw/node'; +import { RestContext, rest } from 'msw'; + +const server = setupServer(); +const mockAuthorizeHandler = jest.fn((req, res, { json }: RestContext) => { + const responses = req.body.map((r: Identified) => ({ + id: r.id, + result: AuthorizeResult.ALLOW, + })); + + return res(json(responses)); +}); +const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; +const discoveryApi: DiscoveryApi = { + async getBaseUrl() { + return mockBaseUrl; + }, +}; +const testPermission: Permission = { + name: 'test.permission', + attributes: {}, + resourceType: 'test-resource', +}; +const config = new ConfigReader({ + permission: { enabled: true }, + backend: { auth: { keys: [{ secret: 'a-secret-key' }] } }, +}); +const logger = getVoidLogger(); + +describe('ServerPermissionClient', () => { + beforeAll(() => server.listen({ onUnhandledRequest: 'error' })); + afterAll(() => server.close()); + beforeEach(() => { + server.use(rest.post(`${mockBaseUrl}/authorize`, mockAuthorizeHandler)); + }); + afterEach(() => server.resetHandlers()); + + it('should bypass authorization if permissions are disabled', async () => { + const client = new ServerPermissionClient({ + discoveryApi, + configApi: new ConfigReader({}), + serverTokenManager: ServerTokenManager.noop(), + }); + + await client.authorize([{ permission: testPermission }]); + + expect(mockAuthorizeHandler).not.toHaveBeenCalled(); + }); + + it('should bypass authorization if permissions are enabled and request has valid server token', async () => { + const tokenManager = ServerTokenManager.fromConfig(config, { logger }); + const client = new ServerPermissionClient({ + discoveryApi, + configApi: config, + serverTokenManager: tokenManager, + }); + + await client.authorize([{ permission: testPermission }], { + token: (await tokenManager.getToken()).token, + }); + + expect(mockAuthorizeHandler).not.toHaveBeenCalled(); + }); + + it('should authorize normally if permissions are enabled and request does not have valid server token', async () => { + const tokenManager = ServerTokenManager.fromConfig(config, { logger }); + const client = new ServerPermissionClient({ + discoveryApi, + configApi: config, + serverTokenManager: tokenManager, + }); + + await client.authorize([{ permission: testPermission }], { + token: 'a-user-token', + }); + + expect(mockAuthorizeHandler).toHaveBeenCalled(); + }); + + it('should error if permissions are enabled but a no-op token manager is configured', async () => { + expect( + () => + new ServerPermissionClient({ + discoveryApi, + configApi: config, + serverTokenManager: ServerTokenManager.noop(), + }), + ).toThrowError( + 'You must configure at least one key in backend.auth.keys if permissions are enabled.', + ); + }); +}); diff --git a/plugins/permission-node/src/ServerPermissionClient.ts b/plugins/permission-node/src/ServerPermissionClient.ts index 0ef464c639..37d75c8ebd 100644 --- a/plugins/permission-node/src/ServerPermissionClient.ts +++ b/plugins/permission-node/src/ServerPermissionClient.ts @@ -14,13 +14,10 @@ * limitations under the License. */ -import { ServerTokenManager } from '@backstage/backend-common'; +import { TokenManager, ServerTokenManager } from '@backstage/backend-common'; import { Config } from '@backstage/config'; import { - AuthorizeRequest, AuthorizeRequestOptions, - AuthorizeResponse, - AuthorizeResult, DiscoveryApi, PermissionClient, } from '@backstage/plugin-permission-common'; @@ -31,26 +28,36 @@ import { * @public */ export class ServerPermissionClient extends PermissionClient { - private readonly serverTokenManager: ServerTokenManager; + private readonly serverTokenManager: TokenManager; constructor(options: { discoveryApi: DiscoveryApi; configApi: Config; - serverTokenManager: ServerTokenManager; + serverTokenManager: TokenManager; }) { const { discoveryApi, configApi, serverTokenManager } = options; super({ discoveryApi, configApi }); + + if (this.enabled && !(serverTokenManager instanceof ServerTokenManager)) { + throw new Error( + 'You must configure at least one key in backend.auth.keys if permissions are enabled.', + ); + } this.serverTokenManager = serverTokenManager; } - async authorize( - requests: AuthorizeRequest[], - options?: AuthorizeRequestOptions, - ): Promise { - if (await this.isValidServerToken(options?.token)) { - return requests.map(_ => ({ result: AuthorizeResult.ALLOW })); + async shouldBypass(options?: AuthorizeRequestOptions): Promise { + // Call super first in order to check if permissions are enabled before + // validating the server token. That way when permissions are disabled, the + // noop token manager can be used without fouling up the logic inside the + // ServerPermissionClient, because the code path won't be reached. + if (await super.shouldBypass(options)) { + return true; } - return super.authorize(requests, options); + if (await this.isValidServerToken(options?.token)) { + return true; + } + return false; } private async isValidServerToken(