From a2291d7cc5eea60e956198359e57ae10ef93d6d6 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Tue, 5 Jan 2021 11:10:18 +0100 Subject: [PATCH 1/8] Optional identity token authorization of api requests --- .changeset/weak-oranges-rhyme.md | 5 ++ packages/backend/src/index.ts | 28 +++++-- plugins/auth-backend/package.json | 1 + .../src/identity/IdentityClient.test.ts | 84 +++++++++++++++++++ .../src/identity/IdentityClient.ts | 52 ++++++++++++ plugins/auth-backend/src/identity/index.ts | 1 + plugins/auth-backend/src/index.ts | 1 + .../lib/passport/BackstageIdentityStrategy.ts | 72 ++++++++++++++++ .../auth-backend/src/lib/passport/index.ts | 1 + yarn.lock | 2 +- 10 files changed, 238 insertions(+), 9 deletions(-) create mode 100644 .changeset/weak-oranges-rhyme.md create mode 100644 plugins/auth-backend/src/identity/IdentityClient.test.ts create mode 100644 plugins/auth-backend/src/identity/IdentityClient.ts create mode 100644 plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts diff --git a/.changeset/weak-oranges-rhyme.md b/.changeset/weak-oranges-rhyme.md new file mode 100644 index 0000000000..f85baea224 --- /dev/null +++ b/.changeset/weak-oranges-rhyme.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend': minor +--- + +Optional identity token authorization of api requests diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 68cc170901..c467ccefa1 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -34,6 +34,7 @@ import { useHotMemoize, } from '@backstage/backend-common'; import { Config } from '@backstage/config'; +import { BackstageIdentityStrategy } from '@backstage/plugin-auth-backend'; import healthcheck from './plugins/healthcheck'; import auth from './plugins/auth'; import catalog from './plugins/catalog'; @@ -45,6 +46,7 @@ import techdocs from './plugins/techdocs'; import graphql from './plugins/graphql'; import app from './plugins/app'; import { PluginEnvironment } from './types'; +import passport from 'passport'; function makeCreateEnv(config: Config) { const root = getRootLogger(); @@ -80,16 +82,26 @@ async function main() { const graphqlEnv = useHotMemoize(module, () => createEnv('graphql')); const appEnv = useHotMemoize(module, () => createEnv('app')); + passport.use( + new BackstageIdentityStrategy({ + discovery: SingleHostDiscovery.fromConfig(config), + }), + ); + const backstageAuth = passport.authenticate('backstage', { + session: false, + }); + const apiRouter = Router(); - apiRouter.use('/catalog', await catalog(catalogEnv)); - apiRouter.use('/rollbar', await rollbar(rollbarEnv)); - apiRouter.use('/scaffolder', await scaffolder(scaffolderEnv)); + apiRouter.use(passport.initialize()); + apiRouter.use('/catalog', backstageAuth, await catalog(catalogEnv)); + apiRouter.use('/rollbar', backstageAuth, await rollbar(rollbarEnv)); + apiRouter.use('/scaffolder', backstageAuth, await scaffolder(scaffolderEnv)); apiRouter.use('/auth', await auth(authEnv)); - apiRouter.use('/techdocs', await techdocs(techdocsEnv)); - apiRouter.use('/kubernetes', await kubernetes(kubernetesEnv)); - apiRouter.use('/proxy', await proxy(proxyEnv)); - apiRouter.use('/graphql', await graphql(graphqlEnv)); - apiRouter.use(notFoundHandler()); + apiRouter.use('/techdocs', backstageAuth, await techdocs(techdocsEnv)); + apiRouter.use('/kubernetes', backstageAuth, await kubernetes(kubernetesEnv)); + apiRouter.use('/proxy', backstageAuth, await proxy(proxyEnv)); + apiRouter.use('/graphql', backstageAuth, await graphql(graphqlEnv)); + apiRouter.use(backstageAuth, notFoundHandler()); const service = createServiceBuilder(module) .loadConfig(config) diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index 8f88c69487..ff9b6db5cb 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -59,6 +59,7 @@ "passport-okta-oauth": "^0.0.1", "passport-onelogin-oauth": "^0.0.1", "passport-saml": "^2.0.0", + "passport-strategy": "^1.0.0", "uuid": "^8.0.0", "winston": "^3.2.1", "yn": "^4.0.0" diff --git a/plugins/auth-backend/src/identity/IdentityClient.test.ts b/plugins/auth-backend/src/identity/IdentityClient.test.ts new file mode 100644 index 0000000000..d3bb6aa9c5 --- /dev/null +++ b/plugins/auth-backend/src/identity/IdentityClient.test.ts @@ -0,0 +1,84 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { rest } from 'msw'; +import { setupServer } from 'msw/node'; +import { JWKECKey } from 'jose'; +import { IdentityClient } from './IdentityClient'; +import { PluginEndpointDiscovery } from '@backstage/backend-common'; + +const server = setupServer(); +const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; +const discovery: PluginEndpointDiscovery = { + async getBaseUrl(_pluginId) { + return mockBaseUrl; + }, + async getExternalBaseUrl(_pluginId) { + return mockBaseUrl; + }, +}; + +describe('IdentityClient', () => { + let client: IdentityClient; + + beforeAll(() => server.listen({ onUnhandledRequest: 'error' })); + afterAll(() => server.close()); + afterEach(() => server.resetHandlers()); + + beforeEach(() => { + client = new IdentityClient({ discovery }); + }); + + describe('listPublicKeys', () => { + const defaultServiceResponse: { + keys: JWKECKey[]; + } = { + keys: [ + { + crv: 'P-256', + x: 'JWy80Goa-8C3oaeDLnk0ANVPPMfI9T3u_T5T7W2b_ls', + y: 'Ge6jAhCDW1PFBfme2RA5ZsXN0cESiCwW29LMRPX5wkw', + kty: 'EC', + kid: 'fecd6d82-224f-43d6-b174-9a48bc42ae44', + alg: 'ES256', + use: 'sig', + }, + { + crv: 'P-256', + x: 'PYGrR5otsNwGdwQC4Ob6sbkVc80jEwsPMUI25y7eUpY', + y: 'GB_mlRnp6METpCW5yUWHpPprZaPJ5sK6RD5nEo1FYac', + kty: 'EC', + kid: 'c5d8c8a8-ca83-43ef-baca-3fccc198901d', + alg: 'ES256', + use: 'sig', + }, + ], + }; + + beforeEach(() => { + server.use( + rest.get(`${mockBaseUrl}/.well-known/jwks.json`, (_, res, ctx) => { + return res(ctx.json(defaultServiceResponse)); + }), + ); + }); + + it('should use the correct endpoint', async () => { + const response = await client.listPublicKeys(); + expect(response).toEqual(defaultServiceResponse); + }); + }); +}); diff --git a/plugins/auth-backend/src/identity/IdentityClient.ts b/plugins/auth-backend/src/identity/IdentityClient.ts new file mode 100644 index 0000000000..2d1dd40b6c --- /dev/null +++ b/plugins/auth-backend/src/identity/IdentityClient.ts @@ -0,0 +1,52 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { JWKECKey } from 'jose'; +import { PluginEndpointDiscovery } from '@backstage/backend-common'; + +/** + * A identity client to interact with auth-backend. + */ +export class IdentityClient { + private readonly discovery: PluginEndpointDiscovery; + + constructor(options: { discovery: PluginEndpointDiscovery }) { + this.discovery = options.discovery; + } + + /** + * Lists public part of keys used to sign Backstage Identity tokens + */ + async listPublicKeys(): Promise<{ + keys: JWKECKey[]; + }> { + const url = `${await this.discovery.getBaseUrl( + 'auth', + )}/.well-known/jwks.json`; + const response = await fetch(url); + + if (!response.ok) { + const payload = await response.text(); + const message = `Request failed with ${response.status} ${response.statusText}, ${payload}`; + throw new Error(message); + } + + const publicKeys: { keys: JWKECKey[] } = await response.json(); + + return publicKeys; + } +} diff --git a/plugins/auth-backend/src/identity/index.ts b/plugins/auth-backend/src/identity/index.ts index 19eec94556..a76dcea440 100644 --- a/plugins/auth-backend/src/identity/index.ts +++ b/plugins/auth-backend/src/identity/index.ts @@ -15,6 +15,7 @@ */ export { createOidcRouter } from './router'; +export { IdentityClient } from './IdentityClient'; export { TokenFactory } from './TokenFactory'; export { DatabaseKeyStore } from './DatabaseKeyStore'; export type { KeyStore, TokenIssuer, TokenParams } from './types'; diff --git a/plugins/auth-backend/src/index.ts b/plugins/auth-backend/src/index.ts index 3d3f059dcc..892842670d 100644 --- a/plugins/auth-backend/src/index.ts +++ b/plugins/auth-backend/src/index.ts @@ -15,6 +15,7 @@ */ export * from './service/router'; +export { BackstageIdentityStrategy } from './lib/passport'; export * from './providers'; // flow package provides 2 functions diff --git a/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts b/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts new file mode 100644 index 0000000000..8f98583e55 --- /dev/null +++ b/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts @@ -0,0 +1,72 @@ +import { Request } from 'express'; +import { JWK, JWT, JWKS } from 'jose'; +import { BackstageIdentity } from '../../providers'; +import { IdentityClient } from '../../identity'; +import { PluginEndpointDiscovery } from '@backstage/backend-common'; + +const Strategy = require('passport-strategy'); +// Using singleton keyStore instead of membership due to Passport wtf +let keyStore: JWKS.KeyStore; +let keyStoreUpdated: number; + +export class BackstageIdentityStrategy extends Strategy { + private readonly client: IdentityClient; + private readonly discovery: PluginEndpointDiscovery; + + constructor(options: { discovery: PluginEndpointDiscovery }) { + super(); + this.client = new IdentityClient({ discovery: options.discovery }); + this.discovery = options.discovery; + this.name = 'backstage'; + } + + private async refreshKeyStore(rawJwtToken: string) { + const { header, payload } = JWT.decode(rawJwtToken, { + complete: true, + }) as { + header: { kid: string }; + payload: { iat: number }; + }; + // Refresh public keys from identity if needed + if ( + !keyStore || + (!keyStore.get({ kid: header.kid }) && + payload?.iat && + payload.iat > keyStoreUpdated) + ) { + const now = Date.now() / 1000; + const publicKeys = await this.client.listPublicKeys(); + keyStore = new JWKS.KeyStore(publicKeys.keys.map(key => JWK.asKey(key))); + keyStoreUpdated = now; + } + } + + async authenticate(req: Request) { + if ( + !req.headers.authorization || + !req.headers.authorization.startsWith('Bearer ') + ) { + this.fail(new Error('No bearer token found in authorization header')); + } + + try { + const token = req!.headers!.authorization!.substring(7); + const issuer = await this.discovery.getExternalBaseUrl('auth'); + await this.refreshKeyStore(token); + const decoded = JWT.IdToken.verify(token, keyStore, { + algorithms: ['ES256'], + audience: 'backstage', + issuer, + }) as { sub: string }; + // Verified, forward BackstageIdentity to req.user + const user: BackstageIdentity = { + id: decoded.sub, + idToken: token, + }; + this.success(user); + } catch (error) { + // JWT verification failed + this.fail(error); + } + } +} diff --git a/plugins/auth-backend/src/lib/passport/index.ts b/plugins/auth-backend/src/lib/passport/index.ts index c307e212fa..9e8b528ac4 100644 --- a/plugins/auth-backend/src/lib/passport/index.ts +++ b/plugins/auth-backend/src/lib/passport/index.ts @@ -22,3 +22,4 @@ export { makeProfileInfo, } from './PassportStrategyHelper'; export type { PassportDoneCallback } from './PassportStrategyHelper'; +export { BackstageIdentityStrategy } from './BackstageIdentityStrategy'; diff --git a/yarn.lock b/yarn.lock index 6710c7e8e7..36d5838364 100644 --- a/yarn.lock +++ b/yarn.lock @@ -19984,7 +19984,7 @@ passport-saml@^2.0.0: xmlbuilder "^11.0.0" xmldom "0.1.x" -passport-strategy@*, passport-strategy@1.x.x: +passport-strategy@*, passport-strategy@1.x.x, passport-strategy@^1.0.0: version "1.0.0" resolved "https://registry.npmjs.org/passport-strategy/-/passport-strategy-1.0.0.tgz#b5539aa8fc225a3d1ad179476ddf236b440f52e4" integrity sha1-tVOaqPwiWj0a0XlHbd8ja0QPUuQ= From 796d1fefbd5fd25f8374f8cc89c4c9d706c92cc9 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Tue, 5 Jan 2021 11:35:32 +0100 Subject: [PATCH 2/8] Fix linting --- packages/backend/package.json | 1 + .../src/identity/IdentityClient.test.ts | 4 ++-- .../lib/passport/BackstageIdentityStrategy.ts | 16 ++++++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/backend/package.json b/packages/backend/package.json index 243ebf2ce3..3fda0ad728 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -47,6 +47,7 @@ "express": "^4.17.1", "express-promise-router": "^3.0.3", "knex": "^0.21.6", + "passport": "^0.4.1", "pg": "^8.3.0", "pg-connection-string": "^2.3.0", "sqlite3": "^5.0.0", diff --git a/plugins/auth-backend/src/identity/IdentityClient.test.ts b/plugins/auth-backend/src/identity/IdentityClient.test.ts index d3bb6aa9c5..4acaf335a4 100644 --- a/plugins/auth-backend/src/identity/IdentityClient.test.ts +++ b/plugins/auth-backend/src/identity/IdentityClient.test.ts @@ -23,10 +23,10 @@ import { PluginEndpointDiscovery } from '@backstage/backend-common'; const server = setupServer(); const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; const discovery: PluginEndpointDiscovery = { - async getBaseUrl(_pluginId) { + async getBaseUrl() { return mockBaseUrl; }, - async getExternalBaseUrl(_pluginId) { + async getExternalBaseUrl() { return mockBaseUrl; }, }; diff --git a/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts b/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts index 8f98583e55..a0ae4d4aac 100644 --- a/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts +++ b/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts @@ -1,3 +1,19 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { Request } from 'express'; import { JWK, JWT, JWKS } from 'jose'; import { BackstageIdentity } from '../../providers'; From 4323cc66f9514aa0baec08f06c80124715be6ba1 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Fri, 8 Jan 2021 19:30:55 +0100 Subject: [PATCH 3/8] Added todo on settling user format for authorized users --- .../auth-backend/src/lib/passport/BackstageIdentityStrategy.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts b/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts index a0ae4d4aac..0910087127 100644 --- a/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts +++ b/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts @@ -75,6 +75,7 @@ export class BackstageIdentityStrategy extends Strategy { issuer, }) as { sub: string }; // Verified, forward BackstageIdentity to req.user + // TODO: Settle internal user format/properties const user: BackstageIdentity = { id: decoded.sub, idToken: token, From 67e60126bb5e7c03efb241978f740796a5c4d362 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Fri, 8 Jan 2021 19:39:25 +0100 Subject: [PATCH 4/8] TODO: Extract and add tests to refreshKeyStore --- .../auth-backend/src/lib/passport/BackstageIdentityStrategy.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts b/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts index 0910087127..7635e1ec40 100644 --- a/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts +++ b/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts @@ -36,6 +36,7 @@ export class BackstageIdentityStrategy extends Strategy { this.name = 'backstage'; } + // TODO(erilar): Move to its own module (IdentityClient?) and add tests private async refreshKeyStore(rawJwtToken: string) { const { header, payload } = JWT.decode(rawJwtToken, { complete: true, From c2926a6c0c3a5fb944dfdd3277ace8165726a1b1 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Sat, 9 Jan 2021 15:13:28 +0100 Subject: [PATCH 5/8] Refactored away from passport into IdentityClient --- packages/backend/src/index.ts | 44 +++++---- plugins/auth-backend/package.json | 1 - .../src/identity/IdentityClient.test.ts | 2 +- .../src/identity/IdentityClient.ts | 98 ++++++++++++++++++- plugins/auth-backend/src/index.ts | 2 +- .../lib/passport/BackstageIdentityStrategy.ts | 90 ----------------- .../auth-backend/src/lib/passport/index.ts | 1 - yarn.lock | 2 +- 8 files changed, 124 insertions(+), 116 deletions(-) delete mode 100644 plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index c467ccefa1..009b4e13af 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -22,6 +22,7 @@ * Happy hacking! */ +import { Request, Response, NextFunction } from 'express'; import Router from 'express-promise-router'; import { createServiceBuilder, @@ -34,7 +35,7 @@ import { useHotMemoize, } from '@backstage/backend-common'; import { Config } from '@backstage/config'; -import { BackstageIdentityStrategy } from '@backstage/plugin-auth-backend'; +import { IdentityClient } from '@backstage/plugin-auth-backend'; import healthcheck from './plugins/healthcheck'; import auth from './plugins/auth'; import catalog from './plugins/catalog'; @@ -46,7 +47,6 @@ import techdocs from './plugins/techdocs'; import graphql from './plugins/graphql'; import app from './plugins/app'; import { PluginEnvironment } from './types'; -import passport from 'passport'; function makeCreateEnv(config: Config) { const root = getRootLogger(); @@ -82,26 +82,34 @@ async function main() { const graphqlEnv = useHotMemoize(module, () => createEnv('graphql')); const appEnv = useHotMemoize(module, () => createEnv('app')); - passport.use( - new BackstageIdentityStrategy({ - discovery: SingleHostDiscovery.fromConfig(config), - }), - ); - const backstageAuth = passport.authenticate('backstage', { - session: false, + const discovery = SingleHostDiscovery.fromConfig(config); + const identity = new IdentityClient({ + discovery, + issuer: await discovery.getExternalBaseUrl('auth'), }); + const authMiddleware = async ( + req: Request, + res: Response, + next: NextFunction, + ) => { + try { + req.user = await identity.authenticate(req.headers.authorization); + next(); + } catch (error) { + res.status(401).send(`Unauthorized`); + } + }; const apiRouter = Router(); - apiRouter.use(passport.initialize()); - apiRouter.use('/catalog', backstageAuth, await catalog(catalogEnv)); - apiRouter.use('/rollbar', backstageAuth, await rollbar(rollbarEnv)); - apiRouter.use('/scaffolder', backstageAuth, await scaffolder(scaffolderEnv)); + apiRouter.use('/catalog', authMiddleware, await catalog(catalogEnv)); + apiRouter.use('/rollbar', authMiddleware, await rollbar(rollbarEnv)); + apiRouter.use('/scaffolder', authMiddleware, await scaffolder(scaffolderEnv)); apiRouter.use('/auth', await auth(authEnv)); - apiRouter.use('/techdocs', backstageAuth, await techdocs(techdocsEnv)); - apiRouter.use('/kubernetes', backstageAuth, await kubernetes(kubernetesEnv)); - apiRouter.use('/proxy', backstageAuth, await proxy(proxyEnv)); - apiRouter.use('/graphql', backstageAuth, await graphql(graphqlEnv)); - apiRouter.use(backstageAuth, notFoundHandler()); + apiRouter.use('/techdocs', authMiddleware, await techdocs(techdocsEnv)); + apiRouter.use('/kubernetes', authMiddleware, await kubernetes(kubernetesEnv)); + apiRouter.use('/proxy', authMiddleware, await proxy(proxyEnv)); + apiRouter.use('/graphql', authMiddleware, await graphql(graphqlEnv)); + apiRouter.use(authMiddleware, notFoundHandler()); const service = createServiceBuilder(module) .loadConfig(config) diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index ff9b6db5cb..8f88c69487 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -59,7 +59,6 @@ "passport-okta-oauth": "^0.0.1", "passport-onelogin-oauth": "^0.0.1", "passport-saml": "^2.0.0", - "passport-strategy": "^1.0.0", "uuid": "^8.0.0", "winston": "^3.2.1", "yn": "^4.0.0" diff --git a/plugins/auth-backend/src/identity/IdentityClient.test.ts b/plugins/auth-backend/src/identity/IdentityClient.test.ts index 4acaf335a4..7e208fed56 100644 --- a/plugins/auth-backend/src/identity/IdentityClient.test.ts +++ b/plugins/auth-backend/src/identity/IdentityClient.test.ts @@ -39,7 +39,7 @@ describe('IdentityClient', () => { afterEach(() => server.resetHandlers()); beforeEach(() => { - client = new IdentityClient({ discovery }); + client = new IdentityClient({ discovery, issuer: mockBaseUrl }); }); describe('listPublicKeys', () => { diff --git a/plugins/auth-backend/src/identity/IdentityClient.ts b/plugins/auth-backend/src/identity/IdentityClient.ts index 2d1dd40b6c..610d43f7ef 100644 --- a/plugins/auth-backend/src/identity/IdentityClient.ts +++ b/plugins/auth-backend/src/identity/IdentityClient.ts @@ -15,17 +15,97 @@ */ import fetch from 'cross-fetch'; -import { JWKECKey } from 'jose'; +import { JWK, JWT, JWKS, JWKECKey } from 'jose'; +import { BackstageIdentity } from '../providers'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; /** - * A identity client to interact with auth-backend. + * A identity client to interact with auth-backend + * and authenticate backstage identity tokens */ export class IdentityClient { private readonly discovery: PluginEndpointDiscovery; + private readonly issuer: string; + private keyStore: JWKS.KeyStore; + private keyStoreUpdated: number; - constructor(options: { discovery: PluginEndpointDiscovery }) { + constructor(options: { discovery: PluginEndpointDiscovery; issuer: string }) { this.discovery = options.discovery; + this.issuer = options.issuer; + this.keyStore = new JWKS.KeyStore(); + this.keyStoreUpdated = 0; + } + + /** + * Verifies the given backstage identity token + * (provided as a Bearer token in the authorization header). + * Returns a BackstageIdentity (user) matching the token. + * The method throws an error if verification fails. + */ + async authenticate( + authorizationHeader: string | undefined, + ): Promise { + // Extract token from header + const token = IdentityClient.getBearerToken(authorizationHeader); + if (!token) { + throw new Error('No bearer token found in authorization header'); + } + // Get signing key matching token + const key = await this.getKey(token); + if (!key) { + throw new Error('No signing key matching token found'); + } + // Verify token claims and signature + // Note: Claims must match those set by TokenFactory when issuing tokens + // Note: verify throws if verification fails + const decoded = JWT.IdToken.verify(token, key, { + algorithms: ['ES256'], + audience: 'backstage', + issuer: this.issuer, + }) as { sub: string }; + // Verified, return the matching user as BackstageIdentity + // TODO: Settle internal user format/properties + const user: BackstageIdentity = { + id: decoded.sub, + idToken: token, + }; + return user; + } + + /** + * Parses the given authorization header and returns + * the bearer token, or null if no bearer token is given + */ + static getBearerToken( + authorizationHeader: string | undefined, + ): string | null { + if (typeof authorizationHeader !== 'string') { + return null; + } + const matches = authorizationHeader.match(/Bearer\s+(\S+)/i); + return matches && matches[1]; + } + + /** + * Returns the public signing key matching the given jwt token, + * or null if no matching key was found + */ + private async getKey(rawJwtToken: string): Promise { + const { header, payload } = JWT.decode(rawJwtToken, { + complete: true, + }) as { + header: { kid: string }; + payload: { iat: number }; + }; + // Refresh public keys if needed + if ( + !this.keyStore.get({ kid: header.kid }) && + payload?.iat && + payload.iat > this.keyStoreUpdated + ) { + await this.refreshKeyStore(); + } + return this.keyStore.get({ kid: header.kid }); } /** @@ -49,4 +129,16 @@ export class IdentityClient { return publicKeys; } + + /** + * Fetches public keys and caches them locally + */ + private async refreshKeyStore(): Promise { + const now = Date.now() / 1000; + const publicKeys = await this.listPublicKeys(); + this.keyStore = new JWKS.KeyStore( + publicKeys.keys.map(key => JWK.asKey(key)), + ); + this.keyStoreUpdated = now; + } } diff --git a/plugins/auth-backend/src/index.ts b/plugins/auth-backend/src/index.ts index 892842670d..a352362089 100644 --- a/plugins/auth-backend/src/index.ts +++ b/plugins/auth-backend/src/index.ts @@ -15,7 +15,7 @@ */ export * from './service/router'; -export { BackstageIdentityStrategy } from './lib/passport'; +export { IdentityClient } from './identity'; export * from './providers'; // flow package provides 2 functions diff --git a/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts b/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts deleted file mode 100644 index 7635e1ec40..0000000000 --- a/plugins/auth-backend/src/lib/passport/BackstageIdentityStrategy.ts +++ /dev/null @@ -1,90 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * 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 { Request } from 'express'; -import { JWK, JWT, JWKS } from 'jose'; -import { BackstageIdentity } from '../../providers'; -import { IdentityClient } from '../../identity'; -import { PluginEndpointDiscovery } from '@backstage/backend-common'; - -const Strategy = require('passport-strategy'); -// Using singleton keyStore instead of membership due to Passport wtf -let keyStore: JWKS.KeyStore; -let keyStoreUpdated: number; - -export class BackstageIdentityStrategy extends Strategy { - private readonly client: IdentityClient; - private readonly discovery: PluginEndpointDiscovery; - - constructor(options: { discovery: PluginEndpointDiscovery }) { - super(); - this.client = new IdentityClient({ discovery: options.discovery }); - this.discovery = options.discovery; - this.name = 'backstage'; - } - - // TODO(erilar): Move to its own module (IdentityClient?) and add tests - private async refreshKeyStore(rawJwtToken: string) { - const { header, payload } = JWT.decode(rawJwtToken, { - complete: true, - }) as { - header: { kid: string }; - payload: { iat: number }; - }; - // Refresh public keys from identity if needed - if ( - !keyStore || - (!keyStore.get({ kid: header.kid }) && - payload?.iat && - payload.iat > keyStoreUpdated) - ) { - const now = Date.now() / 1000; - const publicKeys = await this.client.listPublicKeys(); - keyStore = new JWKS.KeyStore(publicKeys.keys.map(key => JWK.asKey(key))); - keyStoreUpdated = now; - } - } - - async authenticate(req: Request) { - if ( - !req.headers.authorization || - !req.headers.authorization.startsWith('Bearer ') - ) { - this.fail(new Error('No bearer token found in authorization header')); - } - - try { - const token = req!.headers!.authorization!.substring(7); - const issuer = await this.discovery.getExternalBaseUrl('auth'); - await this.refreshKeyStore(token); - const decoded = JWT.IdToken.verify(token, keyStore, { - algorithms: ['ES256'], - audience: 'backstage', - issuer, - }) as { sub: string }; - // Verified, forward BackstageIdentity to req.user - // TODO: Settle internal user format/properties - const user: BackstageIdentity = { - id: decoded.sub, - idToken: token, - }; - this.success(user); - } catch (error) { - // JWT verification failed - this.fail(error); - } - } -} diff --git a/plugins/auth-backend/src/lib/passport/index.ts b/plugins/auth-backend/src/lib/passport/index.ts index 9e8b528ac4..c307e212fa 100644 --- a/plugins/auth-backend/src/lib/passport/index.ts +++ b/plugins/auth-backend/src/lib/passport/index.ts @@ -22,4 +22,3 @@ export { makeProfileInfo, } from './PassportStrategyHelper'; export type { PassportDoneCallback } from './PassportStrategyHelper'; -export { BackstageIdentityStrategy } from './BackstageIdentityStrategy'; diff --git a/yarn.lock b/yarn.lock index 36d5838364..6710c7e8e7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -19984,7 +19984,7 @@ passport-saml@^2.0.0: xmlbuilder "^11.0.0" xmldom "0.1.x" -passport-strategy@*, passport-strategy@1.x.x, passport-strategy@^1.0.0: +passport-strategy@*, passport-strategy@1.x.x: version "1.0.0" resolved "https://registry.npmjs.org/passport-strategy/-/passport-strategy-1.0.0.tgz#b5539aa8fc225a3d1ad179476ddf236b440f52e4" integrity sha1-tVOaqPwiWj0a0XlHbd8ja0QPUuQ= From acaa350fe59e78ece2a0cfde1345faf391a4f597 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Sun, 10 Jan 2021 02:19:45 +0100 Subject: [PATCH 6/8] Added more IdentityClient tests --- .../src/identity/IdentityClient.test.ts | 219 ++++++++++++++++-- .../src/identity/IdentityClient.ts | 12 +- 2 files changed, 212 insertions(+), 19 deletions(-) diff --git a/plugins/auth-backend/src/identity/IdentityClient.test.ts b/plugins/auth-backend/src/identity/IdentityClient.test.ts index 7e208fed56..b209d1bac8 100644 --- a/plugins/auth-backend/src/identity/IdentityClient.test.ts +++ b/plugins/auth-backend/src/identity/IdentityClient.test.ts @@ -14,11 +14,55 @@ * limitations under the License. */ +import { JWT, JSONWebKey } from 'jose'; +import { utc } from 'moment'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import { JWKECKey } from 'jose'; +import { + getVoidLogger, + PluginEndpointDiscovery, +} from '@backstage/backend-common'; import { IdentityClient } from './IdentityClient'; -import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { TokenFactory } from './TokenFactory'; +import { KeyStore, AnyJWK, StoredKey } from './types'; + +const logger = getVoidLogger(); + +class MemoryKeyStore implements KeyStore { + private readonly keys = new Map< + string, + { createdAt: moment.Moment; key: string } + >(); + + async addKey(key: AnyJWK): Promise { + this.keys.set(key.kid, { + createdAt: utc(), + key: JSON.stringify(key), + }); + } + + async removeKeys(kids: string[]): Promise { + for (const kid of kids) { + this.keys.delete(kid); + } + } + + async listKeys(): Promise<{ items: StoredKey[] }> { + return { + items: Array.from(this.keys).map(([, { createdAt, key: keyStr }]) => ({ + createdAt, + key: JSON.parse(keyStr), + })), + }; + } +} + +function jwtKid(jwt: string): string { + const { header } = JWT.decode(jwt, { complete: true }) as { + header: { kid: string }; + }; + return header.kid; +} const server = setupServer(); const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; @@ -33,6 +77,9 @@ const discovery: PluginEndpointDiscovery = { describe('IdentityClient', () => { let client: IdentityClient; + let factory: TokenFactory; + let keyStore: KeyStore; + const keyDurationSeconds = 5; beforeAll(() => server.listen({ onUnhandledRequest: 'error' })); afterAll(() => server.close()); @@ -40,11 +87,166 @@ describe('IdentityClient', () => { beforeEach(() => { client = new IdentityClient({ discovery, issuer: mockBaseUrl }); + keyStore = new MemoryKeyStore(); + factory = new TokenFactory({ + issuer: mockBaseUrl, + keyStore: keyStore, + keyDurationSeconds, + logger, + }); + }); + + describe('authenticate', () => { + beforeEach(() => { + server.use( + rest.get( + `${mockBaseUrl}/.well-known/jwks.json`, + async (_, res, ctx) => { + const keys = await factory.listPublicKeys(); + return res(ctx.json(keys)); + }, + ), + ); + }); + + it('should use the correct endpoint', async () => { + await factory.issueToken({ claims: { sub: 'foo' } }); + const keys = await factory.listPublicKeys(); + const response = await client.listPublicKeys(); + expect(response).toEqual(keys); + }); + + it('should throw on undefined header', async () => { + return expect(async () => { + await client.authenticate(undefined); + }).rejects.toThrow(); + }); + + it('should accept fresh token', async () => { + const token = await factory.issueToken({ claims: { sub: 'foo' } }); + const response = await client.authenticate(`Bearer ${token}`); + expect(response).toEqual({ id: 'foo', idToken: token }); + }); + + it('should throw on incorrect issuer', async () => { + const hackerFactory = new TokenFactory({ + issuer: 'hacker', + keyStore, + keyDurationSeconds, + logger, + }); + return expect(async () => { + const token = await hackerFactory.issueToken({ + claims: { sub: 'foo' }, + }); + await client.authenticate(`Bearer ${token}`); + }).rejects.toThrow(); + }); + + it('should throw on expired token', async () => { + return expect(async () => { + const fixedTime = Date.now(); + jest + .spyOn(Date, 'now') + .mockImplementation(() => fixedTime - keyDurationSeconds * 1000 * 2); + const token = await factory.issueToken({ + claims: { sub: 'foo' }, + }); + jest.spyOn(Date, 'now').mockImplementation(() => fixedTime); + await client.authenticate(`Bearer ${token}`); + }).rejects.toThrow(); + }); + + it('should throw on incorrect signing key', async () => { + const hackerFactory = new TokenFactory({ + issuer: mockBaseUrl, + keyStore: new MemoryKeyStore(), + keyDurationSeconds, + logger, + }); + return expect(async () => { + const token = await hackerFactory.issueToken({ + claims: { sub: 'foo' }, + }); + await client.authenticate(`Bearer ${token}`); + }).rejects.toThrow(); + }); + + it('should accept token from new key', async () => { + const fixedTime = Date.now(); + jest + .spyOn(Date, 'now') + .mockImplementation(() => fixedTime - keyDurationSeconds * 1000 * 2); + const token1 = await factory.issueToken({ claims: { sub: 'foo1' } }); + try { + // This throws as token has already expired + await client.authenticate(`Bearer ${token1}`); + } catch (_err) { + // Ignore thrown error + } + // Move forward in time where the signing key has been rotated + jest.spyOn(Date, 'now').mockImplementation(() => fixedTime); + const token = await factory.issueToken({ claims: { sub: 'foo' } }); + const response = await client.authenticate(`Bearer ${token}`); + expect(response).toEqual({ id: 'foo', idToken: token }); + }); + + it('should not be fooled by the none algorithm', async () => { + return expect(async () => { + const token = await factory.issueToken({ claims: { sub: 'foo' } }); + const header = btoa( + JSON.stringify({ alg: 'none', kid: jwtKid(token) }), + ); + const payload = btoa( + JSON.stringify({ + iss: mockBaseUrl, + sub: 'foo', + aud: 'backstage', + iat: Date.now() / 1000, + exp: Date.now() / 1000 + 60000, + }), + ); + const fakeToken = `${header}.${payload}.`; + return await client.authenticate(`Bearer ${fakeToken}`); + }).rejects.toThrow(); + }); + }); + + describe('getBearerToken', () => { + it('should return null on undefined input', async () => { + const token = IdentityClient.getBearerToken(undefined); + expect(token).toBeNull(); + }); + + it('should return null on malformed input', async () => { + const token = IdentityClient.getBearerToken('malformed'); + expect(token).toBeNull(); + }); + + it('should return null on unexpected scheme', async () => { + const token = IdentityClient.getBearerToken('Basic token'); + expect(token).toBeNull(); + }); + + it('should return Bearer token', async () => { + const token = IdentityClient.getBearerToken('Bearer token'); + expect(token).toEqual('token'); + }); + + it('should return Bearer token despite extra space', async () => { + const token = IdentityClient.getBearerToken('Bearer \n token '); + expect(token).toEqual('token'); + }); + + it('should return Bearer token despite unconventionial case', async () => { + const token = IdentityClient.getBearerToken('bEARER token'); + expect(token).toEqual('token'); + }); }); describe('listPublicKeys', () => { const defaultServiceResponse: { - keys: JWKECKey[]; + keys: JSONWebKey[]; } = { keys: [ { @@ -52,16 +254,7 @@ describe('IdentityClient', () => { x: 'JWy80Goa-8C3oaeDLnk0ANVPPMfI9T3u_T5T7W2b_ls', y: 'Ge6jAhCDW1PFBfme2RA5ZsXN0cESiCwW29LMRPX5wkw', kty: 'EC', - kid: 'fecd6d82-224f-43d6-b174-9a48bc42ae44', - alg: 'ES256', - use: 'sig', - }, - { - crv: 'P-256', - x: 'PYGrR5otsNwGdwQC4Ob6sbkVc80jEwsPMUI25y7eUpY', - y: 'GB_mlRnp6METpCW5yUWHpPprZaPJ5sK6RD5nEo1FYac', - kty: 'EC', - kid: 'c5d8c8a8-ca83-43ef-baca-3fccc198901d', + kid: 'kid-a', alg: 'ES256', use: 'sig', }, diff --git a/plugins/auth-backend/src/identity/IdentityClient.ts b/plugins/auth-backend/src/identity/IdentityClient.ts index 610d43f7ef..df885f9888 100644 --- a/plugins/auth-backend/src/identity/IdentityClient.ts +++ b/plugins/auth-backend/src/identity/IdentityClient.ts @@ -15,7 +15,7 @@ */ import fetch from 'cross-fetch'; -import { JWK, JWT, JWKS, JWKECKey } from 'jose'; +import { JWK, JWT, JWKS, JSONWebKey } from 'jose'; import { BackstageIdentity } from '../providers'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; @@ -112,7 +112,7 @@ export class IdentityClient { * Lists public part of keys used to sign Backstage Identity tokens */ async listPublicKeys(): Promise<{ - keys: JWKECKey[]; + keys: JSONWebKey[]; }> { const url = `${await this.discovery.getBaseUrl( 'auth', @@ -125,7 +125,7 @@ export class IdentityClient { throw new Error(message); } - const publicKeys: { keys: JWKECKey[] } = await response.json(); + const publicKeys: { keys: JSONWebKey[] } = await response.json(); return publicKeys; } @@ -136,9 +136,9 @@ export class IdentityClient { private async refreshKeyStore(): Promise { const now = Date.now() / 1000; const publicKeys = await this.listPublicKeys(); - this.keyStore = new JWKS.KeyStore( - publicKeys.keys.map(key => JWK.asKey(key)), - ); + this.keyStore = JWKS.asKeyStore({ + keys: publicKeys.keys.map(key => key as JSONWebKey), + }); this.keyStoreUpdated = now; } } From b798e8b9e8bd5a53ef162d2922892d339adc592e Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Wed, 20 Jan 2021 01:06:34 +0100 Subject: [PATCH 7/8] Refactored as per PR comments --- .../tutorials/authenticate-api-requests.md | 52 ++++++++++++++++ packages/backend/package.json | 1 - packages/backend/src/index.ts | 36 +++-------- .../src/identity/IdentityClient.test.ts | 59 +++++-------------- .../src/identity/IdentityClient.ts | 30 +++++----- .../src/identity/MemoryKeyStore.ts | 47 +++++++++++++++ .../src/identity/TokenFactory.test.ts | 32 +--------- 7 files changed, 139 insertions(+), 118 deletions(-) create mode 100644 contrib/docs/tutorials/authenticate-api-requests.md create mode 100644 plugins/auth-backend/src/identity/MemoryKeyStore.ts diff --git a/contrib/docs/tutorials/authenticate-api-requests.md b/contrib/docs/tutorials/authenticate-api-requests.md new file mode 100644 index 0000000000..2b8407abd2 --- /dev/null +++ b/contrib/docs/tutorials/authenticate-api-requests.md @@ -0,0 +1,52 @@ +# Authenticate API requests + +The Backstage backend APIs are by default available without authentication. To avoid evil-doers from accessing or modifying data, one might use a network protection mechanism such as a firewall or an authenticating reverse proxy. For Backstage instances that are available on the Internet one can instead use the experimental IdentityClient as outlined below. + +API requests from frontend plugins include an authorization header with a Backstage identity token acquired when the user logs in. By adding a middleware that verifies said token to be valid and signed by Backstage, non-authenticated requests can be blocked with a 401 Unauthorized response. + +Note that this means Backstage will stop working for guests, as no token is issued for them. + +Caveat: as of writing this, Backstage does not refresh the identity token so eventually users will get a 401 response on API calls (not on loading the web page as only the API calls are authenticated) and have to logout/login again to get a new token. + +```typescript +// packages/backend/src/index.ts from a create-app deployment + +import { Request, Response, NextFunction } from 'express'; +import { IdentityClient } from '@backstage/plugin-auth-backend'; + +// ... + +async function main() { + // ... + + const discovery = SingleHostDiscovery.fromConfig(config); + const identity = new IdentityClient({ + discovery, + issuer: await discovery.getExternalBaseUrl('auth'), + }); + const authMiddleware = async ( + req: Request, + res: Response, + next: NextFunction, + ) => { + try { + const token = IdentityClient.getBearerToken(req.headers.authorization); + req.user = await identity.authenticate(token); + next(); + } catch (error) { + res.status(401).send(`Unauthorized`); + } + }; + + const apiRouter = Router(); + // The auth route must be publically available as it is used during login + apiRouter.use('/auth', await auth(authEnv)); + // Only authenticated requests are allowed to the routes below + apiRouter.use('/catalog', authMiddleware, await catalog(catalogEnv)); + apiRouter.use('/techdocs', authMiddleware, await techdocs(techdocsEnv)); + apiRouter.use('/proxy', authMiddleware, await proxy(proxyEnv)); + apiRouter.use(authMiddleware, notFoundHandler()); + + // ... +} +``` diff --git a/packages/backend/package.json b/packages/backend/package.json index 3fda0ad728..243ebf2ce3 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -47,7 +47,6 @@ "express": "^4.17.1", "express-promise-router": "^3.0.3", "knex": "^0.21.6", - "passport": "^0.4.1", "pg": "^8.3.0", "pg-connection-string": "^2.3.0", "sqlite3": "^5.0.0", diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 009b4e13af..68cc170901 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -22,7 +22,6 @@ * Happy hacking! */ -import { Request, Response, NextFunction } from 'express'; import Router from 'express-promise-router'; import { createServiceBuilder, @@ -35,7 +34,6 @@ import { useHotMemoize, } from '@backstage/backend-common'; import { Config } from '@backstage/config'; -import { IdentityClient } from '@backstage/plugin-auth-backend'; import healthcheck from './plugins/healthcheck'; import auth from './plugins/auth'; import catalog from './plugins/catalog'; @@ -82,34 +80,16 @@ async function main() { const graphqlEnv = useHotMemoize(module, () => createEnv('graphql')); const appEnv = useHotMemoize(module, () => createEnv('app')); - const discovery = SingleHostDiscovery.fromConfig(config); - const identity = new IdentityClient({ - discovery, - issuer: await discovery.getExternalBaseUrl('auth'), - }); - const authMiddleware = async ( - req: Request, - res: Response, - next: NextFunction, - ) => { - try { - req.user = await identity.authenticate(req.headers.authorization); - next(); - } catch (error) { - res.status(401).send(`Unauthorized`); - } - }; - const apiRouter = Router(); - apiRouter.use('/catalog', authMiddleware, await catalog(catalogEnv)); - apiRouter.use('/rollbar', authMiddleware, await rollbar(rollbarEnv)); - apiRouter.use('/scaffolder', authMiddleware, await scaffolder(scaffolderEnv)); + apiRouter.use('/catalog', await catalog(catalogEnv)); + apiRouter.use('/rollbar', await rollbar(rollbarEnv)); + apiRouter.use('/scaffolder', await scaffolder(scaffolderEnv)); apiRouter.use('/auth', await auth(authEnv)); - apiRouter.use('/techdocs', authMiddleware, await techdocs(techdocsEnv)); - apiRouter.use('/kubernetes', authMiddleware, await kubernetes(kubernetesEnv)); - apiRouter.use('/proxy', authMiddleware, await proxy(proxyEnv)); - apiRouter.use('/graphql', authMiddleware, await graphql(graphqlEnv)); - apiRouter.use(authMiddleware, notFoundHandler()); + apiRouter.use('/techdocs', await techdocs(techdocsEnv)); + apiRouter.use('/kubernetes', await kubernetes(kubernetesEnv)); + apiRouter.use('/proxy', await proxy(proxyEnv)); + apiRouter.use('/graphql', await graphql(graphqlEnv)); + apiRouter.use(notFoundHandler()); const service = createServiceBuilder(module) .loadConfig(config) diff --git a/plugins/auth-backend/src/identity/IdentityClient.test.ts b/plugins/auth-backend/src/identity/IdentityClient.test.ts index b209d1bac8..5f317aa0c2 100644 --- a/plugins/auth-backend/src/identity/IdentityClient.test.ts +++ b/plugins/auth-backend/src/identity/IdentityClient.test.ts @@ -15,7 +15,6 @@ */ import { JWT, JSONWebKey } from 'jose'; -import { utc } from 'moment'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import { @@ -23,40 +22,12 @@ import { PluginEndpointDiscovery, } from '@backstage/backend-common'; import { IdentityClient } from './IdentityClient'; +import { MemoryKeyStore } from './MemoryKeyStore'; import { TokenFactory } from './TokenFactory'; -import { KeyStore, AnyJWK, StoredKey } from './types'; +import { KeyStore } from './types'; const logger = getVoidLogger(); -class MemoryKeyStore implements KeyStore { - private readonly keys = new Map< - string, - { createdAt: moment.Moment; key: string } - >(); - - async addKey(key: AnyJWK): Promise { - this.keys.set(key.kid, { - createdAt: utc(), - key: JSON.stringify(key), - }); - } - - async removeKeys(kids: string[]): Promise { - for (const kid of kids) { - this.keys.delete(kid); - } - } - - async listKeys(): Promise<{ items: StoredKey[] }> { - return { - items: Array.from(this.keys).map(([, { createdAt, key: keyStr }]) => ({ - createdAt, - key: JSON.parse(keyStr), - })), - }; - } -} - function jwtKid(jwt: string): string { const { header } = JWT.decode(jwt, { complete: true }) as { header: { kid: string }; @@ -124,7 +95,7 @@ describe('IdentityClient', () => { it('should accept fresh token', async () => { const token = await factory.issueToken({ claims: { sub: 'foo' } }); - const response = await client.authenticate(`Bearer ${token}`); + const response = await client.authenticate(token); expect(response).toEqual({ id: 'foo', idToken: token }); }); @@ -139,7 +110,7 @@ describe('IdentityClient', () => { const token = await hackerFactory.issueToken({ claims: { sub: 'foo' }, }); - await client.authenticate(`Bearer ${token}`); + await client.authenticate(token); }).rejects.toThrow(); }); @@ -153,7 +124,7 @@ describe('IdentityClient', () => { claims: { sub: 'foo' }, }); jest.spyOn(Date, 'now').mockImplementation(() => fixedTime); - await client.authenticate(`Bearer ${token}`); + await client.authenticate(token); }).rejects.toThrow(); }); @@ -168,7 +139,7 @@ describe('IdentityClient', () => { const token = await hackerFactory.issueToken({ claims: { sub: 'foo' }, }); - await client.authenticate(`Bearer ${token}`); + await client.authenticate(token); }).rejects.toThrow(); }); @@ -180,14 +151,14 @@ describe('IdentityClient', () => { const token1 = await factory.issueToken({ claims: { sub: 'foo1' } }); try { // This throws as token has already expired - await client.authenticate(`Bearer ${token1}`); + await client.authenticate(token1); } catch (_err) { // Ignore thrown error } // Move forward in time where the signing key has been rotated jest.spyOn(Date, 'now').mockImplementation(() => fixedTime); const token = await factory.issueToken({ claims: { sub: 'foo' } }); - const response = await client.authenticate(`Bearer ${token}`); + const response = await client.authenticate(token); expect(response).toEqual({ id: 'foo', idToken: token }); }); @@ -207,25 +178,25 @@ describe('IdentityClient', () => { }), ); const fakeToken = `${header}.${payload}.`; - return await client.authenticate(`Bearer ${fakeToken}`); + return await client.authenticate(fakeToken); }).rejects.toThrow(); }); }); describe('getBearerToken', () => { - it('should return null on undefined input', async () => { + it('should return undefined on undefined input', async () => { const token = IdentityClient.getBearerToken(undefined); - expect(token).toBeNull(); + expect(token).toBeUndefined(); }); - it('should return null on malformed input', async () => { + it('should return undefined on malformed input', async () => { const token = IdentityClient.getBearerToken('malformed'); - expect(token).toBeNull(); + expect(token).toBeUndefined(); }); - it('should return null on unexpected scheme', async () => { + it('should return undefined on unexpected scheme', async () => { const token = IdentityClient.getBearerToken('Basic token'); - expect(token).toBeNull(); + expect(token).toBeUndefined(); }); it('should return Bearer token', async () => { diff --git a/plugins/auth-backend/src/identity/IdentityClient.ts b/plugins/auth-backend/src/identity/IdentityClient.ts index df885f9888..970c30dd40 100644 --- a/plugins/auth-backend/src/identity/IdentityClient.ts +++ b/plugins/auth-backend/src/identity/IdentityClient.ts @@ -19,9 +19,13 @@ import { JWK, JWT, JWKS, JSONWebKey } from 'jose'; import { BackstageIdentity } from '../providers'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; +const CLOCK_MARGIN_S = 10; + /** * A identity client to interact with auth-backend * and authenticate backstage identity tokens + * + * @experimental This is not a stable API yet */ export class IdentityClient { private readonly discovery: PluginEndpointDiscovery; @@ -38,17 +42,13 @@ export class IdentityClient { /** * Verifies the given backstage identity token - * (provided as a Bearer token in the authorization header). * Returns a BackstageIdentity (user) matching the token. * The method throws an error if verification fails. */ - async authenticate( - authorizationHeader: string | undefined, - ): Promise { + async authenticate(token: string | undefined): Promise { // Extract token from header - const token = IdentityClient.getBearerToken(authorizationHeader); if (!token) { - throw new Error('No bearer token found in authorization header'); + throw new Error('No token specified'); } // Get signing key matching token const key = await this.getKey(token); @@ -78,12 +78,12 @@ export class IdentityClient { */ static getBearerToken( authorizationHeader: string | undefined, - ): string | null { + ): string | undefined { if (typeof authorizationHeader !== 'string') { - return null; + return undefined; } const matches = authorizationHeader.match(/Bearer\s+(\S+)/i); - return matches && matches[1]; + return matches?.[1]; } /** @@ -97,14 +97,16 @@ export class IdentityClient { header: { kid: string }; payload: { iat: number }; }; + // Refresh public keys if needed - if ( - !this.keyStore.get({ kid: header.kid }) && - payload?.iat && - payload.iat > this.keyStoreUpdated - ) { + // Add a small margin in case clocks are out of sync + const keyStoreHasKey = !!this.keyStore.get({ kid: header.kid }); + const issuedAfterLastRefresh = + payload?.iat && payload.iat > this.keyStoreUpdated - CLOCK_MARGIN_S; + if (!keyStoreHasKey && issuedAfterLastRefresh) { await this.refreshKeyStore(); } + return this.keyStore.get({ kid: header.kid }); } diff --git a/plugins/auth-backend/src/identity/MemoryKeyStore.ts b/plugins/auth-backend/src/identity/MemoryKeyStore.ts new file mode 100644 index 0000000000..b35002b146 --- /dev/null +++ b/plugins/auth-backend/src/identity/MemoryKeyStore.ts @@ -0,0 +1,47 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { utc } from 'moment'; +import { KeyStore, AnyJWK, StoredKey } from './types'; + +export class MemoryKeyStore implements KeyStore { + private readonly keys = new Map< + string, + { createdAt: moment.Moment; key: string } + >(); + + async addKey(key: AnyJWK): Promise { + this.keys.set(key.kid, { + createdAt: utc(), + key: JSON.stringify(key), + }); + } + + async removeKeys(kids: string[]): Promise { + for (const kid of kids) { + this.keys.delete(kid); + } + } + + async listKeys(): Promise<{ items: StoredKey[] }> { + return { + items: Array.from(this.keys).map(([, { createdAt, key: keyStr }]) => ({ + createdAt, + key: JSON.parse(keyStr), + })), + }; + } +} diff --git a/plugins/auth-backend/src/identity/TokenFactory.test.ts b/plugins/auth-backend/src/identity/TokenFactory.test.ts index 8b719799b1..0c7febe616 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.test.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.test.ts @@ -14,43 +14,13 @@ * limitations under the License. */ -import { utc } from 'moment'; +import { MemoryKeyStore } from './MemoryKeyStore'; import { TokenFactory } from './TokenFactory'; import { getVoidLogger } from '@backstage/backend-common'; -import { KeyStore, AnyJWK, StoredKey } from './types'; import { JWKS, JSONWebKey, JWT } from 'jose'; const logger = getVoidLogger(); -class MemoryKeyStore implements KeyStore { - private readonly keys = new Map< - string, - { createdAt: moment.Moment; key: string } - >(); - - async addKey(key: AnyJWK): Promise { - this.keys.set(key.kid, { - createdAt: utc(), - key: JSON.stringify(key), - }); - } - - async removeKeys(kids: string[]): Promise { - for (const kid of kids) { - this.keys.delete(kid); - } - } - - async listKeys(): Promise<{ items: StoredKey[] }> { - return { - items: Array.from(this.keys).map(([, { createdAt, key: keyStr }]) => ({ - createdAt, - key: JSON.parse(keyStr), - })), - }; - } -} - function jwtKid(jwt: string): string { const { header } = JWT.decode(jwt, { complete: true }) as { header: { kid: string }; From 454361c9cbc319851ea03d67300e00f698bfa98e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Jan 2021 12:47:53 +0100 Subject: [PATCH 8/8] Update weak-oranges-rhyme.md --- .changeset/weak-oranges-rhyme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/weak-oranges-rhyme.md b/.changeset/weak-oranges-rhyme.md index f85baea224..a0b68ab498 100644 --- a/.changeset/weak-oranges-rhyme.md +++ b/.changeset/weak-oranges-rhyme.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-auth-backend': minor +'@backstage/plugin-auth-backend': patch --- Optional identity token authorization of api requests