From 0a305d7f7059aa40a1b94cf50d4d1055089055d3 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 18 Jun 2020 12:56:54 +0200 Subject: [PATCH 01/20] auth-backend: initial identity provider implementation --- .../migrations/20200619125845_init.js | 49 +++++++++++ plugins/auth-backend/package.json | 4 +- .../src/identity/DatabaseKeyStore.ts | 76 +++++++++++++++++ .../auth-backend/src/identity/TokenFactory.ts | 85 +++++++++++++++++++ plugins/auth-backend/src/identity/index.ts | 20 +++++ plugins/auth-backend/src/identity/router.ts | 73 ++++++++++++++++ plugins/auth-backend/src/identity/types.ts | 36 ++++++++ yarn.lock | 12 +++ 8 files changed, 354 insertions(+), 1 deletion(-) create mode 100644 plugins/auth-backend/migrations/20200619125845_init.js create mode 100644 plugins/auth-backend/src/identity/DatabaseKeyStore.ts create mode 100644 plugins/auth-backend/src/identity/TokenFactory.ts create mode 100644 plugins/auth-backend/src/identity/index.ts create mode 100644 plugins/auth-backend/src/identity/router.ts create mode 100644 plugins/auth-backend/src/identity/types.ts diff --git a/plugins/auth-backend/migrations/20200619125845_init.js b/plugins/auth-backend/migrations/20200619125845_init.js new file mode 100644 index 0000000000..295e4eea40 --- /dev/null +++ b/plugins/auth-backend/migrations/20200619125845_init.js @@ -0,0 +1,49 @@ +/* + * 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. + */ + +// @ts-check + +/** + * @param {import('knex')} knex + */ +exports.up = async function up(knex) { + return knex.schema.createTable('signing_keys', table => { + table.comment( + 'Signing keys that are currently in use or have recently been used to issue tokens', + ); + table + .string('kid') + .primary() + .notNullable() + .comment('ID of the signing key'); + table + .timestamp('created_at') + .notNullable() + .defaultTo(knex.fn.now()) + .comment('The creation time of the key'); + table + .string('key') + .notNullable() + .comment('The serialized public part of the signing key'); + }); +}; + +/** + * @param {import('knex')} knex + */ +exports.down = async function down(knex) { + return knex.schema.dropTable('auth_keystore'); +}; diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index d22a5ef6d6..bee3b3c8b5 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -32,7 +32,9 @@ "express-promise-router": "^3.0.3", "fs-extra": "^9.0.0", "helmet": "^3.22.0", + "jose": "^1.27.1", "jwt-decode": "2.2.0", + "knex": "^0.21.1", "morgan": "^1.10.0", "passport": "^0.4.1", "passport-github2": "^0.1.12", @@ -46,10 +48,10 @@ "@types/body-parser": "^1.19.0", "@types/cookie-parser": "^1.4.2", "@types/jwt-decode": "2.2.1", + "@types/passport": "^1.0.3", "@types/passport-github2": "^1.2.4", "@types/passport-google-oauth20": "^2.0.3", "@types/passport-saml": "^1.1.2", - "@types/passport": "^1.0.3", "jest-fetch-mock": "^3.0.3" }, "files": [ diff --git a/plugins/auth-backend/src/identity/DatabaseKeyStore.ts b/plugins/auth-backend/src/identity/DatabaseKeyStore.ts new file mode 100644 index 0000000000..d3ea588c14 --- /dev/null +++ b/plugins/auth-backend/src/identity/DatabaseKeyStore.ts @@ -0,0 +1,76 @@ +/* + * 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 Knex from 'knex'; +import path from 'path'; +import { Logger } from 'winston'; +import { PublicKey } from './types'; + +const migrationsDir = path.resolve( + require.resolve('@backstage/plugin-auth-backend/package.json'), + '../migrations', +); + +const TABLE = 'signing_keys'; + +type Row = { + created_at: Date; + kid: string; + key: string; +}; + +type Options = { + logger: Logger; + database: Knex; +}; + +export class DatabaseKeyStore { + static async create(options: Options): Promise { + const { logger, database } = options; + + await database.migrate.latest({ + directory: migrationsDir, + }); + + return new DatabaseKeyStore({ logger, database }); + } + + private readonly logger: Logger; + private readonly database: Knex; + + private constructor(options: Options) { + const { logger, database } = options; + + this.database = database; + this.logger = logger.child({ service: 'key-store' }); + } + + async addPublicKey(key: PublicKey): Promise { + this.logger.info(`Storing public key ${key.kid}`); + + await this.database(TABLE).insert({ + kid: key.kid, + key: JSON.stringify(key), + }); + console.log(`DEBUG: stored key`); + } + + async listPublicKeys(): Promise { + const rows = await this.database(TABLE).select(); + console.log('DEBUG: rows =', rows); + return rows.map(row => JSON.parse(row.key)); + } +} diff --git a/plugins/auth-backend/src/identity/TokenFactory.ts b/plugins/auth-backend/src/identity/TokenFactory.ts new file mode 100644 index 0000000000..3df962bf22 --- /dev/null +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -0,0 +1,85 @@ +/* + * 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 { TokenIssuer, TokenParams, KeyStore, PublicKey } from './types'; +import { JSONWebKey, JWK, JWS } from 'jose'; +import { Logger } from 'winston'; + +type Options = { + issuer: string; + logger: Logger; + keyStore: KeyStore; +}; + +export class TokenFactory implements TokenIssuer { + private readonly issuer: string; + private readonly logger: Logger; + private readonly keyStore: KeyStore; + + private privateKey?: Promise; + + constructor(options: Options) { + const { issuer, logger, keyStore } = options; + this.issuer = issuer; + this.keyStore = keyStore; + this.logger = logger.child({ service: 'issuer' }); + } + + async issueToken(claims: TokenParams): Promise { + const key = await this.getKey(); + + const iss = this.issuer; + const sub = claims.sub; + const aud = 'backstage'; + const iat = (Date.now() / 1000) | 0; + const exp = iat + 3600; + + this.logger.info(`Issuing token for ${sub}`); + + return JWS.sign({ iss, sub, aud, iat, exp }, key, { + alg: key.alg, + kid: key.kid, + }); + } + + private async getKey(): Promise { + if (this.privateKey) { + return this.privateKey; + } + + this.privateKey = (async () => { + const dateStr = new Date().toISOString(); + const randStr = Math.random().toString(36).slice(2, 6); + const kid = `key-${dateStr}-${randStr}`; + + const key = await JWK.generate('EC', 'P-384', { use: 'sig', kid }); + + await this.keyStore.addPublicKey( + (key.toJWK(false) as unknown) as PublicKey, + ); + + return key as JSONWebKey; + })(); + + try { + await this.privateKey; + } catch (error) { + this.logger.error(`Failed to generate signing key, ${error}`); + } + + return this.privateKey; + } +} diff --git a/plugins/auth-backend/src/identity/index.ts b/plugins/auth-backend/src/identity/index.ts new file mode 100644 index 0000000000..19eec94556 --- /dev/null +++ b/plugins/auth-backend/src/identity/index.ts @@ -0,0 +1,20 @@ +/* + * 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. + */ + +export { createOidcRouter } from './router'; +export { TokenFactory } from './TokenFactory'; +export { DatabaseKeyStore } from './DatabaseKeyStore'; +export type { KeyStore, TokenIssuer, TokenParams } from './types'; diff --git a/plugins/auth-backend/src/identity/router.ts b/plugins/auth-backend/src/identity/router.ts new file mode 100644 index 0000000000..a52e632ec2 --- /dev/null +++ b/plugins/auth-backend/src/identity/router.ts @@ -0,0 +1,73 @@ +/* + * 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 Router from 'express-promise-router'; +import { Logger } from 'winston'; +import { KeyStore } from './types'; + +export type Options = { + logger: Logger; + baseUrl: string; + keyStore: KeyStore; +}; + +export function createOidcRouter(options: Options) { + const { + logger: baseLogger, + keyStore, + baseUrl = 'http://localhost:7000/auth', + } = options; + const logger = baseLogger.child({ router: 'identity' }); + + const router = Router(); + + const config = { + issuer: baseUrl, + token_endpoint: `${baseUrl}/v1/token`, + userinfo_endpoint: `${baseUrl}/v1/userinfo`, + jwks_uri: `${baseUrl}/v1/certs`, + response_types_supported: ['id_token'], + subject_types_supported: ['public'], + id_token_signing_alg_values_supported: ['RS256'], + scopes_supported: ['openid'], + token_endpoint_auth_methods_supported: [], + claims_supported: ['sub'], + grant_types_supported: [], + }; + + router.get('/.well-known/openid-configuration', (_req, res) => { + logger.info('request configuration'); + res.json(config); + }); + + router.get('/v1/token', (_req, res) => { + logger.info('request token'); + res.status(501).send('Not Implemented'); + }); + + router.get('/v1/userinfo', (_req, res) => { + logger.info('request userinfo'); + res.status(501).send('Not Implemented'); + }); + + router.get('/v1/certs', async (_req, res) => { + logger.info('request certs'); + const keys = await keyStore.listPublicKeys(); + res.json(keys); + }); + + return router; +} diff --git a/plugins/auth-backend/src/identity/types.ts b/plugins/auth-backend/src/identity/types.ts new file mode 100644 index 0000000000..59221e8d81 --- /dev/null +++ b/plugins/auth-backend/src/identity/types.ts @@ -0,0 +1,36 @@ +/* + * 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. + */ + +export interface PublicKey extends Record { + use: 'sig' | 'enc'; + alg: string; + kid: string; + kty: string; +} + +export type KeyStore = { + addPublicKey(key: PublicKey): Promise; + + listPublicKeys(): Promise; +}; + +export type TokenParams = { + sub: string; +}; + +export type TokenIssuer = { + issueToken(claims: TokenParams): Promise; +}; diff --git a/yarn.lock b/yarn.lock index c18da809e9..c924d383c7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2411,6 +2411,11 @@ resolved "https://registry.npmjs.org/@open-draft/until/-/until-1.0.3.tgz#db9cc719191a62e7d9200f6e7bab21c5b848adca" integrity sha512-Aq58f5HiWdyDlFffbbSjAlv596h/cOnt2DO1w3DOC7OJ5EHs0hd/nycJfiu9RJbT6Yk6F1knnRRXNSpxoIVZ9Q== +"@panva/asn1.js@^1.0.0": + version "1.0.0" + resolved "https://registry.npmjs.org/@panva/asn1.js/-/asn1.js-1.0.0.tgz#dd55ae7b8129e02049f009408b97c61ccf9032f6" + integrity sha512-UdkG3mLEqXgnlKsWanWcgb6dOjUzJ+XC5f+aWw30qrtjxeNUSfKX1cd5FBzOaXQumoe9nIqeZUvrRJS03HCCtw== + "@reach/router@^1.2.1": version "1.3.3" resolved "https://registry.npmjs.org/@reach/router/-/router-1.3.3.tgz#58162860dce6c9449d49be86b0561b5ef46d80db" @@ -11697,6 +11702,13 @@ jest@^26.0.1: import-local "^3.0.2" jest-cli "^26.0.1" +jose@^1.27.1: + version "1.27.1" + resolved "https://registry.npmjs.org/jose/-/jose-1.27.1.tgz#a1de2ecb5b3ae1ae28f0d9d0cc536349ada27ec8" + integrity sha512-VyHM6IJPw0TTGqHVNlPWg16/ASDPAmcChcLqSb3WNBvwWFoWPeFqlmAUCm8/oIG1GjZwAlUDuRKFfycowarcVA== + dependencies: + "@panva/asn1.js" "^1.0.0" + js-cookie@^2.2.1: version "2.2.1" resolved "https://registry.npmjs.org/js-cookie/-/js-cookie-2.2.1.tgz#69e106dc5d5806894562902aa5baec3744e9b2b8" From 90cc2ca615eeb9768223090acb1619def6628e0a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 19 Jun 2020 14:08:54 +0200 Subject: [PATCH 02/20] auth-backend: expire signing keys --- .../auth-backend/src/identity/TokenFactory.ts | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/plugins/auth-backend/src/identity/TokenFactory.ts b/plugins/auth-backend/src/identity/TokenFactory.ts index 3df962bf22..16798442f3 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -18,6 +18,8 @@ import { TokenIssuer, TokenParams, KeyStore, PublicKey } from './types'; import { JSONWebKey, JWK, JWS } from 'jose'; import { Logger } from 'winston'; +const KEY_DURATION_MS = 3600 * 1000; + type Options = { issuer: string; logger: Logger; @@ -29,7 +31,8 @@ export class TokenFactory implements TokenIssuer { private readonly logger: Logger; private readonly keyStore: KeyStore; - private privateKey?: Promise; + private keyExpiry?: number; + private privateKeyPromise?: Promise; constructor(options: Options) { const { issuer, logger, keyStore } = options; @@ -56,11 +59,16 @@ export class TokenFactory implements TokenIssuer { } private async getKey(): Promise { - if (this.privateKey) { - return this.privateKey; + if (this.privateKeyPromise) { + if (this.keyExpiry && Date.now() < this.keyExpiry) { + return this.privateKeyPromise; + } + this.logger.info(`Signing key has expired, generating new key`); + delete this.privateKeyPromise; } - this.privateKey = (async () => { + this.keyExpiry = Date.now() + KEY_DURATION_MS; + const promise = (async () => { const dateStr = new Date().toISOString(); const randStr = Math.random().toString(36).slice(2, 6); const kid = `key-${dateStr}-${randStr}`; @@ -74,12 +82,18 @@ export class TokenFactory implements TokenIssuer { return key as JSONWebKey; })(); + this.privateKeyPromise = promise; + try { - await this.privateKey; + // If we fail to generate a new key, we need to clear the state so that + // the next caller will try to generate another key. + await promise; } catch (error) { - this.logger.error(`Failed to generate signing key, ${error}`); + this.logger.error(`Failed to generate new signing key, ${error}`); + delete this.keyExpiry; + delete this.privateKeyPromise; } - return this.privateKey; + return promise; } } From adc574ee338d24b5e47cdd534db991aa91fa319c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 19 Jun 2020 15:24:29 +0200 Subject: [PATCH 03/20] auth-backend: make DatabaseKeyStore only return valid keys and remove expired ones --- .../migrations/20200619125845_init.js | 2 +- plugins/auth-backend/package.json | 1 + .../src/identity/DatabaseKeyStore.ts | 59 ++++++++++++++++++- 3 files changed, 58 insertions(+), 4 deletions(-) diff --git a/plugins/auth-backend/migrations/20200619125845_init.js b/plugins/auth-backend/migrations/20200619125845_init.js index 295e4eea40..6d06fd1125 100644 --- a/plugins/auth-backend/migrations/20200619125845_init.js +++ b/plugins/auth-backend/migrations/20200619125845_init.js @@ -30,7 +30,7 @@ exports.up = async function up(knex) { .notNullable() .comment('ID of the signing key'); table - .timestamp('created_at') + .timestamp('created_at', { useTz: false, precision: 0 }) .notNullable() .defaultTo(knex.fn.now()) .comment('The creation time of the key'); diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index bee3b3c8b5..7da7b75968 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -35,6 +35,7 @@ "jose": "^1.27.1", "jwt-decode": "2.2.0", "knex": "^0.21.1", + "moment": "^2.26.0", "morgan": "^1.10.0", "passport": "^0.4.1", "passport-github2": "^0.1.12", diff --git a/plugins/auth-backend/src/identity/DatabaseKeyStore.ts b/plugins/auth-backend/src/identity/DatabaseKeyStore.ts index d3ea588c14..016761fe99 100644 --- a/plugins/auth-backend/src/identity/DatabaseKeyStore.ts +++ b/plugins/auth-backend/src/identity/DatabaseKeyStore.ts @@ -16,6 +16,7 @@ import Knex from 'knex'; import path from 'path'; +import { utc } from 'moment'; import { Logger } from 'winston'; import { PublicKey } from './types'; @@ -24,6 +25,8 @@ const migrationsDir = path.resolve( '../migrations', ); +const KEY_DURATION_MS = 3600 * 1000; + const TABLE = 'signing_keys'; type Row = { @@ -51,6 +54,8 @@ export class DatabaseKeyStore { private readonly logger: Logger; private readonly database: Knex; + private removingExpiredRows: boolean = false; + private constructor(options: Options) { const { logger, database } = options; @@ -65,12 +70,60 @@ export class DatabaseKeyStore { kid: key.kid, key: JSON.stringify(key), }); - console.log(`DEBUG: stored key`); } async listPublicKeys(): Promise { const rows = await this.database(TABLE).select(); - console.log('DEBUG: rows =', rows); - return rows.map(row => JSON.parse(row.key)); + + const [validRows, expiredRows] = this.splitExpiredRows(rows); + if (expiredRows.length > 0) { + // We don't await this, just let it run in the background + this.removeExpiredRows(expiredRows); + } + + return validRows.map(row => JSON.parse(row.key)); + } + + private splitExpiredRows(rows: Row[]) { + const validRows = []; + const expiredRows = []; + + for (const row of rows) { + const createdAt = utc(row.created_at); + const expireAt = createdAt.add(3 * KEY_DURATION_MS, 'ms'); + const isExpired = expireAt.isBefore(); + + if (isExpired) { + expiredRows.push(row); + } else { + validRows.push(row); + } + } + + return [validRows, expiredRows]; + } + + private async removeExpiredRows(rows: Row[]) { + if (this.removingExpiredRows) { + return; + } + + try { + this.removingExpiredRows = true; + + const kids = rows.map(row => row.kid); + this.logger.info(`Removing expired signing keys, '${kids.join(', ')}'`); + + const result = await this.database(TABLE).delete().whereIn('kid', kids); + if (result !== kids.length) { + this.logger.warn( + `Wanted to remove ${kids.length} expired signing, but removed ${result} instead`, + ); + } + } catch (error) { + this.logger.error(`Failed to remove expired signing keys, ${error}`); + } finally { + this.removingExpiredRows = false; + } } } From ca9a2ddf1acea57d8a7531932d3c25266254be06 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 19 Jun 2020 18:10:35 +0200 Subject: [PATCH 04/20] auth-backend: hook up oidc router and token issuer/db --- packages/backend/src/plugins/auth.ts | 3 ++- .../auth-backend/src/providers/factories.ts | 4 +++- plugins/auth-backend/src/providers/types.ts | 2 ++ plugins/auth-backend/src/service/router.ts | 20 ++++++++++++++++++- 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/plugins/auth.ts b/packages/backend/src/plugins/auth.ts index a9c687cbc4..b24dd6ca6b 100644 --- a/packages/backend/src/plugins/auth.ts +++ b/packages/backend/src/plugins/auth.ts @@ -19,7 +19,8 @@ import { PluginEnvironment } from '../types'; export default async function createPlugin({ logger, + database, config, }: PluginEnvironment) { - return await createRouter({ logger, config }); + return await createRouter({ logger, config, database }); } diff --git a/plugins/auth-backend/src/providers/factories.ts b/plugins/auth-backend/src/providers/factories.ts index b5e59b713b..5096b0698a 100644 --- a/plugins/auth-backend/src/providers/factories.ts +++ b/plugins/auth-backend/src/providers/factories.ts @@ -20,6 +20,7 @@ import { createGoogleProvider } from './google'; import { createSamlProvider } from './saml'; import { AuthProviderFactory, AuthProviderConfig } from './types'; import { Logger } from 'winston'; +import { TokenIssuer } from '../identity'; const factories: { [providerId: string]: AuthProviderFactory } = { google: createGoogleProvider, @@ -32,13 +33,14 @@ export const createAuthProviderRouter = ( globalConfig: AuthProviderConfig, providerConfig: any, // TODO: make this a config reader object of sorts logger: Logger, + issuer: TokenIssuer, ) => { const factory = factories[providerId]; if (!factory) { throw Error(`No auth provider available for '${providerId}'`); } - const provider = factory(globalConfig, providerConfig, logger); + const provider = factory(globalConfig, providerConfig, logger, issuer); const router = Router(); router.get('/start', provider.start.bind(provider)); diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index 7c16436021..3660751c1c 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -16,6 +16,7 @@ import express from 'express'; import { Logger } from 'winston'; +import { TokenIssuer } from '../identity'; export type OAuthProviderOptions = { /** @@ -180,6 +181,7 @@ export type AuthProviderFactory = ( globalConfig: AuthProviderConfig, providerConfig: EnvironmentProviderConfig, logger: Logger, + issuer: TokenIssuer, ) => AuthProviderRouteHandlers; export type AuthInfoBase = { diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 2e7b4623ff..12ddbf8504 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -18,12 +18,15 @@ import express from 'express'; import Router from 'express-promise-router'; import cookieParser from 'cookie-parser'; import bodyParser from 'body-parser'; +import Knex from 'knex'; import { Logger } from 'winston'; import { createAuthProviderRouter } from '../providers'; import { Config } from '@backstage/config'; +import { DatabaseKeyStore, TokenFactory, createOidcRouter } from '../identity'; export interface RouterOptions { logger: Logger; + database: Knex; config: Config; } @@ -33,6 +36,18 @@ export async function createRouter( const router = Router(); const logger = options.logger.child({ plugin: 'auth' }); + const baseUrl = `${options.config.getString('backend.baseUrl')}/auth`; + + const keyStore = await DatabaseKeyStore.create({ + database: options.database, + logger, + }); + const issuer = new TokenFactory({ + issuer: baseUrl, + keyStore, + logger, + }); + router.use(cookieParser()); router.use(bodyParser.urlencoded({ extended: false })); router.use(bodyParser.json()); @@ -79,7 +94,6 @@ export async function createRouter( const providerConfigs = config.auth.providers; for (const [providerId, providerConfig] of Object.entries(providerConfigs)) { - const baseUrl = `${options.config.getString('backend.baseUrl')}/auth`; logger.info(`Configuring provider, ${providerId}`); try { const providerRouter = createAuthProviderRouter( @@ -87,11 +101,15 @@ export async function createRouter( { baseUrl }, providerConfig, logger, + issuer, ); router.use(`/${providerId}`, providerRouter); } catch (e) { logger.error(e.message); } } + + router.use(createOidcRouter({ logger, keyStore, baseUrl })); + return router; } From b0bf34cfc91f16d20a65fcc8bde6ef7424541591 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 19 Jun 2020 18:14:59 +0200 Subject: [PATCH 05/20] auth-backend: use well-known route for jwks --- plugins/auth-backend/src/identity/router.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/auth-backend/src/identity/router.ts b/plugins/auth-backend/src/identity/router.ts index a52e632ec2..e9dd1e0f80 100644 --- a/plugins/auth-backend/src/identity/router.ts +++ b/plugins/auth-backend/src/identity/router.ts @@ -53,6 +53,12 @@ export function createOidcRouter(options: Options) { res.json(config); }); + router.get('/.well-known/jwks.json', async (_req, res) => { + logger.info('request certs'); + const keys = await keyStore.listPublicKeys(); + res.json(keys); + }); + router.get('/v1/token', (_req, res) => { logger.info('request token'); res.status(501).send('Not Implemented'); @@ -63,11 +69,5 @@ export function createOidcRouter(options: Options) { res.status(501).send('Not Implemented'); }); - router.get('/v1/certs', async (_req, res) => { - logger.info('request certs'); - const keys = await keyStore.listPublicKeys(); - res.json(keys); - }); - return router; } From af0bfccbb59766f6f0fd074a04cc87831f59e758 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 20 Jun 2020 00:24:03 +0200 Subject: [PATCH 06/20] auth-backend: generate kid with uuid and use P-256 --- plugins/auth-backend/package.json | 1 + plugins/auth-backend/src/identity/TokenFactory.ts | 11 ++++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index 7da7b75968..bf6e120a4e 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -41,6 +41,7 @@ "passport-github2": "^0.1.12", "passport-google-oauth20": "^2.0.0", "passport-saml": "^1.3.3", + "uuid": "^8.0.0", "winston": "^3.2.1", "yn": "^4.0.0" }, diff --git a/plugins/auth-backend/src/identity/TokenFactory.ts b/plugins/auth-backend/src/identity/TokenFactory.ts index 16798442f3..3899e2e608 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -17,6 +17,7 @@ import { TokenIssuer, TokenParams, KeyStore, PublicKey } from './types'; import { JSONWebKey, JWK, JWS } from 'jose'; import { Logger } from 'winston'; +import { v4 as uuid } from 'uuid'; const KEY_DURATION_MS = 3600 * 1000; @@ -69,11 +70,11 @@ export class TokenFactory implements TokenIssuer { this.keyExpiry = Date.now() + KEY_DURATION_MS; const promise = (async () => { - const dateStr = new Date().toISOString(); - const randStr = Math.random().toString(36).slice(2, 6); - const kid = `key-${dateStr}-${randStr}`; - - const key = await JWK.generate('EC', 'P-384', { use: 'sig', kid }); + const key = await JWK.generate('EC', 'P-256', { + use: 'sig', + kid: uuid(), + alg: 'ES256', + }); await this.keyStore.addPublicKey( (key.toJWK(false) as unknown) as PublicKey, From 55faf713c34fb724f25a41783fdee4368f2ae090 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 20 Jun 2020 00:24:41 +0200 Subject: [PATCH 07/20] auth-backend: fix jwks response not wrapping keys in object --- plugins/auth-backend/src/identity/router.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/auth-backend/src/identity/router.ts b/plugins/auth-backend/src/identity/router.ts index e9dd1e0f80..9155bed9e8 100644 --- a/plugins/auth-backend/src/identity/router.ts +++ b/plugins/auth-backend/src/identity/router.ts @@ -56,7 +56,7 @@ export function createOidcRouter(options: Options) { router.get('/.well-known/jwks.json', async (_req, res) => { logger.info('request certs'); const keys = await keyStore.listPublicKeys(); - res.json(keys); + res.json({ keys }); }); router.get('/v1/token', (_req, res) => { From c72056eb29221f50cf45cc649e777cc4b40bdeaf Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 20 Jun 2020 00:35:01 +0200 Subject: [PATCH 08/20] auth-backend: make key duration configurable --- .../auth-backend/src/identity/DatabaseKeyStore.ts | 14 ++++++++------ plugins/auth-backend/src/identity/TokenFactory.ts | 14 +++++++++----- plugins/auth-backend/src/service/router.ts | 3 +++ 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/plugins/auth-backend/src/identity/DatabaseKeyStore.ts b/plugins/auth-backend/src/identity/DatabaseKeyStore.ts index 016761fe99..b8247f066f 100644 --- a/plugins/auth-backend/src/identity/DatabaseKeyStore.ts +++ b/plugins/auth-backend/src/identity/DatabaseKeyStore.ts @@ -25,8 +25,6 @@ const migrationsDir = path.resolve( '../migrations', ); -const KEY_DURATION_MS = 3600 * 1000; - const TABLE = 'signing_keys'; type Row = { @@ -38,28 +36,32 @@ type Row = { type Options = { logger: Logger; database: Knex; + /** Expiration time of signing keys in seconds */ + keyDuration: number; }; export class DatabaseKeyStore { static async create(options: Options): Promise { - const { logger, database } = options; + const { database } = options; await database.migrate.latest({ directory: migrationsDir, }); - return new DatabaseKeyStore({ logger, database }); + return new DatabaseKeyStore(options); } private readonly logger: Logger; private readonly database: Knex; + private readonly keyDuration: number; private removingExpiredRows: boolean = false; private constructor(options: Options) { - const { logger, database } = options; + const { logger, database, keyDuration } = options; this.database = database; + this.keyDuration = keyDuration; this.logger = logger.child({ service: 'key-store' }); } @@ -90,7 +92,7 @@ export class DatabaseKeyStore { for (const row of rows) { const createdAt = utc(row.created_at); - const expireAt = createdAt.add(3 * KEY_DURATION_MS, 'ms'); + const expireAt = createdAt.add(3 * this.keyDuration, 'seconds'); const isExpired = expireAt.isBefore(); if (isExpired) { diff --git a/plugins/auth-backend/src/identity/TokenFactory.ts b/plugins/auth-backend/src/identity/TokenFactory.ts index 3899e2e608..60f827b51f 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -19,26 +19,30 @@ import { JSONWebKey, JWK, JWS } from 'jose'; import { Logger } from 'winston'; import { v4 as uuid } from 'uuid'; -const KEY_DURATION_MS = 3600 * 1000; - type Options = { - issuer: string; logger: Logger; + /** Value of the issuer claim in issued tokens */ + issuer: string; + /** Key store used for storing signing keys */ keyStore: KeyStore; + /** Expiration time of signing keys in seconds */ + keyDuration: number; }; export class TokenFactory implements TokenIssuer { private readonly issuer: string; private readonly logger: Logger; private readonly keyStore: KeyStore; + private readonly keyDuration: number; private keyExpiry?: number; private privateKeyPromise?: Promise; constructor(options: Options) { - const { issuer, logger, keyStore } = options; + const { issuer, logger, keyStore, keyDuration } = options; this.issuer = issuer; this.keyStore = keyStore; + this.keyDuration = keyDuration; this.logger = logger.child({ service: 'issuer' }); } @@ -68,7 +72,7 @@ export class TokenFactory implements TokenIssuer { delete this.privateKeyPromise; } - this.keyExpiry = Date.now() + KEY_DURATION_MS; + this.keyExpiry = Date.now() + this.keyDuration * 1000; const promise = (async () => { const key = await JWK.generate('EC', 'P-256', { use: 'sig', diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 12ddbf8504..d57225daf6 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -37,15 +37,18 @@ export async function createRouter( const logger = options.logger.child({ plugin: 'auth' }); const baseUrl = `${options.config.getString('backend.baseUrl')}/auth`; + const keyDuration = 3600; const keyStore = await DatabaseKeyStore.create({ database: options.database, logger, + keyDuration, }); const issuer = new TokenFactory({ issuer: baseUrl, keyStore, logger, + keyDuration, }); router.use(cookieParser()); From f2746a6eeced8ed38940a2f0cf434dd557caf003 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 20 Jun 2020 00:59:29 +0200 Subject: [PATCH 09/20] auth-backend: move logger setup out from constructors and use non-reserved fields --- plugins/auth-backend/src/identity/TokenFactory.ts | 9 ++++----- plugins/auth-backend/src/identity/router.ts | 7 +------ plugins/auth-backend/src/service/router.ts | 12 +++++++++--- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/plugins/auth-backend/src/identity/TokenFactory.ts b/plugins/auth-backend/src/identity/TokenFactory.ts index 60f827b51f..f121f2522d 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -39,11 +39,10 @@ export class TokenFactory implements TokenIssuer { private privateKeyPromise?: Promise; constructor(options: Options) { - const { issuer, logger, keyStore, keyDuration } = options; - this.issuer = issuer; - this.keyStore = keyStore; - this.keyDuration = keyDuration; - this.logger = logger.child({ service: 'issuer' }); + this.issuer = options.issuer; + this.logger = options.logger; + this.keyStore = options.keyStore; + this.keyDuration = options.keyDuration; } async issueToken(claims: TokenParams): Promise { diff --git a/plugins/auth-backend/src/identity/router.ts b/plugins/auth-backend/src/identity/router.ts index 9155bed9e8..7ba0835ecd 100644 --- a/plugins/auth-backend/src/identity/router.ts +++ b/plugins/auth-backend/src/identity/router.ts @@ -25,12 +25,7 @@ export type Options = { }; export function createOidcRouter(options: Options) { - const { - logger: baseLogger, - keyStore, - baseUrl = 'http://localhost:7000/auth', - } = options; - const logger = baseLogger.child({ router: 'identity' }); + const { logger, keyStore, baseUrl } = options; const router = Router(); diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index d57225daf6..7809bc7ef8 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -41,13 +41,13 @@ export async function createRouter( const keyStore = await DatabaseKeyStore.create({ database: options.database, - logger, + logger: logger.child({ component: 'db-key-store' }), keyDuration, }); const issuer = new TokenFactory({ issuer: baseUrl, keyStore, - logger, + logger: logger.child({ component: 'token-factory' }), keyDuration, }); @@ -112,7 +112,13 @@ export async function createRouter( } } - router.use(createOidcRouter({ logger, keyStore, baseUrl })); + router.use( + createOidcRouter({ + logger: logger.child({ component: 'oidc-router' }), + keyStore, + baseUrl, + }), + ); return router; } From 5bd70f9aa5626133e4dcec0cace3bb4c5e09ce1d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 20 Jun 2020 12:06:17 +0200 Subject: [PATCH 10/20] auth-backend: issue user id tokens when signing in with google or github --- plugins/auth-backend/src/lib/OAuthProvider.test.ts | 3 +++ plugins/auth-backend/src/lib/OAuthProvider.ts | 11 +++++++++++ plugins/auth-backend/src/providers/github/provider.ts | 3 +++ plugins/auth-backend/src/providers/google/provider.ts | 3 +++ 4 files changed, 20 insertions(+) diff --git a/plugins/auth-backend/src/lib/OAuthProvider.test.ts b/plugins/auth-backend/src/lib/OAuthProvider.test.ts index bb261d1380..80c69fdffd 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.test.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.test.ts @@ -180,6 +180,9 @@ describe('OAuthProvider', () => { disableRefresh: true, baseUrl: 'http://localhost:7000/auth', appOrigin: 'http://localhost:3000', + tokenIssuer: { + issueToken: async () => 'my-id-token', + }, }; it('sets the correct headers in start', async () => { diff --git a/plugins/auth-backend/src/lib/OAuthProvider.ts b/plugins/auth-backend/src/lib/OAuthProvider.ts index 62b565b60d..d8d11d52a3 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.ts @@ -23,6 +23,7 @@ import { OAuthProviderHandlers, } from '../providers/types'; import { InputError } from '@backstage/backend-common'; +import { TokenIssuer } from '../identity'; export const THOUSAND_DAYS_MS = 1000 * 24 * 60 * 60 * 1000; export const TEN_MINUTES_MS = 600 * 1000; @@ -33,6 +34,7 @@ export type Options = { disableRefresh?: boolean; baseUrl: string; appOrigin: string; + tokenIssuer: TokenIssuer; }; export const verifyNonce = (req: express.Request, providerId: string) => { @@ -142,6 +144,10 @@ export class OAuthProvider implements AuthProviderRouteHandlers { this.setRefreshTokenCookie(res, refreshToken); } + user.userIdToken = await this.options.tokenIssuer.issueToken({ + sub: user.profile.email, + }); + // post message back to popup if successful return postMessageResponse(res, this.options.appOrigin, { type: 'auth-result', @@ -198,6 +204,11 @@ export class OAuthProvider implements AuthProviderRouteHandlers { refreshToken, scope, ); + + refreshInfo.userIdToken = await this.options.tokenIssuer.issueToken({ + sub: refreshInfo.profile?.email, + }); + return res.send(refreshInfo); } catch (error) { return res.status(401).send(`${error.message}`); diff --git a/plugins/auth-backend/src/providers/github/provider.ts b/plugins/auth-backend/src/providers/github/provider.ts index 0bcd80cf2c..b5f9d7e46b 100644 --- a/plugins/auth-backend/src/providers/github/provider.ts +++ b/plugins/auth-backend/src/providers/github/provider.ts @@ -36,6 +36,7 @@ import { EnvironmentHandler, } from '../../lib/EnvironmentHandler'; import { Logger } from 'winston'; +import { TokenIssuer } from '../../identity'; export class GithubAuthProvider implements OAuthProviderHandlers { private readonly _strategy: GithubStrategy; @@ -69,6 +70,7 @@ export function createGithubProvider( { baseUrl }: AuthProviderConfig, providerConfig: EnvironmentProviderConfig, logger: Logger, + tokenIssuer: TokenIssuer, ) { const envProviders: EnvironmentHandlers = {}; @@ -101,6 +103,7 @@ export function createGithubProvider( secure, baseUrl, appOrigin, + tokenIssuer, }); } return new EnvironmentHandler(envProviders); diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index 9675d4eef2..f41fbfe787 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -41,6 +41,7 @@ import { EnvironmentHandlers, } from '../../lib/EnvironmentHandler'; import { Logger } from 'winston'; +import { TokenIssuer } from '../../identity'; export class GoogleAuthProvider implements OAuthProviderHandlers { private readonly _strategy: GoogleStrategy; @@ -116,6 +117,7 @@ export function createGoogleProvider( { baseUrl }: AuthProviderConfig, providerConfig: EnvironmentProviderConfig, logger: Logger, + tokenIssuer: TokenIssuer, ) { const envProviders: EnvironmentHandlers = {}; @@ -148,6 +150,7 @@ export function createGoogleProvider( secure, baseUrl, appOrigin, + tokenIssuer, }); } return new EnvironmentHandler(envProviders); From 625f50725b201c45b16bc2c164a489e1f0511b65 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 20 Jun 2020 14:18:22 +0200 Subject: [PATCH 11/20] auth-backend: update standalone setup to use service builder and HMR --- plugins/auth-backend/src/run.ts | 7 ++- .../src/service/standaloneApplication.ts | 54 ------------------- .../src/service/standaloneServer.ts | 42 +++++++++------ 3 files changed, 28 insertions(+), 75 deletions(-) delete mode 100644 plugins/auth-backend/src/service/standaloneApplication.ts diff --git a/plugins/auth-backend/src/run.ts b/plugins/auth-backend/src/run.ts index a2c2601258..679f8ded2e 100644 --- a/plugins/auth-backend/src/run.ts +++ b/plugins/auth-backend/src/run.ts @@ -14,15 +14,12 @@ * limitations under the License. */ -import yn from 'yn'; import { getRootLogger } from '@backstage/backend-common'; import { startStandaloneServer } from './service/standaloneServer'; -const port = process.env.PLUGIN_PORT ? Number(process.env.PLUGIN_PORT) : 3003; -const enableCors = yn(process.env.PLUGIN_CORS, { default: false }); const logger = getRootLogger(); -startStandaloneServer({ port, enableCors, logger }).catch(err => { +startStandaloneServer({ logger }).catch(err => { logger.error(err); process.exit(1); }); @@ -31,3 +28,5 @@ process.on('SIGINT', () => { logger.info('CTRL+C pressed; exiting.'); process.exit(0); }); + +module.hot?.accept(); diff --git a/plugins/auth-backend/src/service/standaloneApplication.ts b/plugins/auth-backend/src/service/standaloneApplication.ts deleted file mode 100644 index cd81e32ccb..0000000000 --- a/plugins/auth-backend/src/service/standaloneApplication.ts +++ /dev/null @@ -1,54 +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 { - errorHandler, - notFoundHandler, - requestLoggingHandler, -} from '@backstage/backend-common'; -import { Config } from '@backstage/config'; -import compression from 'compression'; -import cors from 'cors'; -import express from 'express'; -import helmet from 'helmet'; -import { Logger } from 'winston'; -import { createRouter } from './router'; - -export interface ApplicationOptions { - enableCors: boolean; - logger: Logger; - config: Config; -} - -export async function createStandaloneApplication( - options: ApplicationOptions, -): Promise { - const { enableCors, logger, config } = options; - const app = express(); - - app.use(helmet()); - if (enableCors) { - app.use(cors()); - } - app.use(compression()); - app.use(express.json()); - app.use(requestLoggingHandler()); - app.use('/', await createRouter({ logger, config })); - app.use(notFoundHandler()); - app.use(errorHandler()); - - return app; -} diff --git a/plugins/auth-backend/src/service/standaloneServer.ts b/plugins/auth-backend/src/service/standaloneServer.ts index c03fe44b40..d2372a7e91 100644 --- a/plugins/auth-backend/src/service/standaloneServer.ts +++ b/plugins/auth-backend/src/service/standaloneServer.ts @@ -14,15 +14,15 @@ * limitations under the License. */ +import Knex from 'knex'; import { Server } from 'http'; import { Logger } from 'winston'; -import { createStandaloneApplication } from './standaloneApplication'; import { ConfigReader } from '@backstage/config'; import { loadConfig } from '@backstage/config-loader'; +import { createRouter } from './router'; +import { createServiceBuilder, useHotMemoize } from '@backstage/backend-common'; export interface ServerOptions { - port: number; - enableCors: boolean; logger: Logger; } @@ -32,23 +32,31 @@ export async function startStandaloneServer( const logger = options.logger.child({ service: 'auth-backend' }); const config = ConfigReader.fromConfigs(await loadConfig()); - logger.debug('Creating application...'); - const app = await createStandaloneApplication({ - enableCors: options.enableCors, - logger, - config, + const database = useHotMemoize(module, () => { + const knex = Knex({ + client: 'sqlite3', + connection: ':memory:', + useNullAsDefault: true, + }); + knex.client.pool.on('createSuccess', (_eventId: any, resource: any) => { + resource.run('PRAGMA foreign_keys = ON', () => {}); + }); + return knex; }); logger.debug('Starting application server...'); - return await new Promise((resolve, reject) => { - const server = app.listen(options.port, (err?: Error) => { - if (err) { - reject(err); - return; - } + const router = await createRouter({ + logger, + config, + database, + }); - logger.info(`Listening on port ${options.port}`); - resolve(server); - }); + const service = createServiceBuilder(module) + .enableCors({ origin: 'http://localhost:3000', credentials: true }) + .addRouter('/auth', router); + + return await service.start().catch(err => { + logger.error(err); + process.exit(1); }); } From 48b6f9533c7f8309cbfed95a37d6308cfca75059 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 20 Jun 2020 14:33:34 +0200 Subject: [PATCH 12/20] auth-backend: refactor identity APIs for forwards compatibility --- .../auth-backend/migrations/20200619125845_init.js | 5 +---- .../auth-backend/src/identity/DatabaseKeyStore.ts | 12 +++++++----- plugins/auth-backend/src/identity/TokenFactory.ts | 12 ++++++------ plugins/auth-backend/src/identity/router.ts | 2 +- plugins/auth-backend/src/identity/types.ts | 14 ++++++++------ plugins/auth-backend/src/lib/OAuthProvider.ts | 4 ++-- 6 files changed, 25 insertions(+), 24 deletions(-) diff --git a/plugins/auth-backend/migrations/20200619125845_init.js b/plugins/auth-backend/migrations/20200619125845_init.js index 6d06fd1125..e31731b037 100644 --- a/plugins/auth-backend/migrations/20200619125845_init.js +++ b/plugins/auth-backend/migrations/20200619125845_init.js @@ -34,10 +34,7 @@ exports.up = async function up(knex) { .notNullable() .defaultTo(knex.fn.now()) .comment('The creation time of the key'); - table - .string('key') - .notNullable() - .comment('The serialized public part of the signing key'); + table.string('key').notNullable().comment('The serialized signing key'); }); }; diff --git a/plugins/auth-backend/src/identity/DatabaseKeyStore.ts b/plugins/auth-backend/src/identity/DatabaseKeyStore.ts index b8247f066f..9d288b3624 100644 --- a/plugins/auth-backend/src/identity/DatabaseKeyStore.ts +++ b/plugins/auth-backend/src/identity/DatabaseKeyStore.ts @@ -18,7 +18,7 @@ import Knex from 'knex'; import path from 'path'; import { utc } from 'moment'; import { Logger } from 'winston'; -import { PublicKey } from './types'; +import { AnyJWK, KeyStore } from './types'; const migrationsDir = path.resolve( require.resolve('@backstage/plugin-auth-backend/package.json'), @@ -40,7 +40,7 @@ type Options = { keyDuration: number; }; -export class DatabaseKeyStore { +export class DatabaseKeyStore implements KeyStore { static async create(options: Options): Promise { const { database } = options; @@ -65,7 +65,7 @@ export class DatabaseKeyStore { this.logger = logger.child({ service: 'key-store' }); } - async addPublicKey(key: PublicKey): Promise { + async storeKey({ key }: { key: AnyJWK }): Promise { this.logger.info(`Storing public key ${key.kid}`); await this.database(TABLE).insert({ @@ -74,7 +74,7 @@ export class DatabaseKeyStore { }); } - async listPublicKeys(): Promise { + async listKeys(): Promise<{ keys: AnyJWK[] }> { const rows = await this.database(TABLE).select(); const [validRows, expiredRows] = this.splitExpiredRows(rows); @@ -83,7 +83,9 @@ export class DatabaseKeyStore { this.removeExpiredRows(expiredRows); } - return validRows.map(row => JSON.parse(row.key)); + return { + keys: validRows.map(row => JSON.parse(row.key)), + }; } private splitExpiredRows(rows: Row[]) { diff --git a/plugins/auth-backend/src/identity/TokenFactory.ts b/plugins/auth-backend/src/identity/TokenFactory.ts index f121f2522d..4d7ba0c7e6 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { TokenIssuer, TokenParams, KeyStore, PublicKey } from './types'; +import { TokenIssuer, TokenParams, KeyStore, AnyJWK } from './types'; import { JSONWebKey, JWK, JWS } from 'jose'; import { Logger } from 'winston'; import { v4 as uuid } from 'uuid'; @@ -45,11 +45,11 @@ export class TokenFactory implements TokenIssuer { this.keyDuration = options.keyDuration; } - async issueToken(claims: TokenParams): Promise { + async issueToken(params: TokenParams): Promise { const key = await this.getKey(); const iss = this.issuer; - const sub = claims.sub; + const sub = params.claims.sub; const aud = 'backstage'; const iat = (Date.now() / 1000) | 0; const exp = iat + 3600; @@ -79,9 +79,9 @@ export class TokenFactory implements TokenIssuer { alg: 'ES256', }); - await this.keyStore.addPublicKey( - (key.toJWK(false) as unknown) as PublicKey, - ); + await this.keyStore.storeKey({ + key: (key.toJWK(false) as unknown) as AnyJWK, + }); return key as JSONWebKey; })(); diff --git a/plugins/auth-backend/src/identity/router.ts b/plugins/auth-backend/src/identity/router.ts index 7ba0835ecd..c18fdb7bc8 100644 --- a/plugins/auth-backend/src/identity/router.ts +++ b/plugins/auth-backend/src/identity/router.ts @@ -50,7 +50,7 @@ export function createOidcRouter(options: Options) { router.get('/.well-known/jwks.json', async (_req, res) => { logger.info('request certs'); - const keys = await keyStore.listPublicKeys(); + const { keys } = await keyStore.listKeys(); res.json({ keys }); }); diff --git a/plugins/auth-backend/src/identity/types.ts b/plugins/auth-backend/src/identity/types.ts index 59221e8d81..c19f8e1ddf 100644 --- a/plugins/auth-backend/src/identity/types.ts +++ b/plugins/auth-backend/src/identity/types.ts @@ -14,23 +14,25 @@ * limitations under the License. */ -export interface PublicKey extends Record { - use: 'sig' | 'enc'; +export interface AnyJWK extends Record { + use: 'sig'; alg: string; kid: string; kty: string; } export type KeyStore = { - addPublicKey(key: PublicKey): Promise; + storeKey(params: { key: AnyJWK }): Promise; - listPublicKeys(): Promise; + listKeys(): Promise<{ keys: AnyJWK[] }>; }; export type TokenParams = { - sub: string; + claims: { + sub: string; + }; }; export type TokenIssuer = { - issueToken(claims: TokenParams): Promise; + issueToken(params: TokenParams): Promise; }; diff --git a/plugins/auth-backend/src/lib/OAuthProvider.ts b/plugins/auth-backend/src/lib/OAuthProvider.ts index d8d11d52a3..38065c3380 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.ts @@ -145,7 +145,7 @@ export class OAuthProvider implements AuthProviderRouteHandlers { } user.userIdToken = await this.options.tokenIssuer.issueToken({ - sub: user.profile.email, + claims: { sub: user.profile.email }, }); // post message back to popup if successful @@ -206,7 +206,7 @@ export class OAuthProvider implements AuthProviderRouteHandlers { ); refreshInfo.userIdToken = await this.options.tokenIssuer.issueToken({ - sub: refreshInfo.profile?.email, + claims: { sub: refreshInfo.profile?.email }, }); return res.send(refreshInfo); From a1e975b8ac688e88dd3ffbfba625f955ccd82e26 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 20 Jun 2020 14:45:11 +0200 Subject: [PATCH 13/20] auth-backend: document identity types --- plugins/auth-backend/src/identity/types.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/plugins/auth-backend/src/identity/types.ts b/plugins/auth-backend/src/identity/types.ts index c19f8e1ddf..1ceb9dd2cb 100644 --- a/plugins/auth-backend/src/identity/types.ts +++ b/plugins/auth-backend/src/identity/types.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +/** Represents any form of serializable JWK */ export interface AnyJWK extends Record { use: 'sig'; alg: string; @@ -21,18 +22,34 @@ export interface AnyJWK extends Record { kty: string; } +/** A KeyStore can store and keep track of recently used JWKs */ export type KeyStore = { + /** + * Store a new key to be used for signing. Before the key has been successfully + * stored it should not be used for signing. + */ storeKey(params: { key: AnyJWK }): Promise; + /** + * List all keys that are currently being used to sign tokens, or have been used + * in the past within the token expiration time, including a grace period. + */ listKeys(): Promise<{ keys: AnyJWK[] }>; }; +/** Parameters used to issue new ID Tokens */ export type TokenParams = { + /** The claims that will be embedded within the token */ claims: { + /** The token subject, i.e. User ID */ sub: string; }; }; +/** A TokenIssuer is able to issue verifiable ID Tokens on demand */ export type TokenIssuer = { + /** + * Issues a new ID Token + */ issueToken(params: TokenParams): Promise; }; From a4bbcaa00223c165bc86d31f0fd734a8106db73b Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 20 Jun 2020 16:13:37 +0200 Subject: [PATCH 14/20] auth-backend: refactor identity to remove logic from storage layer --- .../src/identity/DatabaseKeyStore.ts | 77 ++++--------------- .../auth-backend/src/identity/TokenFactory.ts | 36 ++++++++- plugins/auth-backend/src/identity/router.ts | 8 +- plugins/auth-backend/src/identity/types.ts | 53 +++++++++---- .../src/lib/OAuthProvider.test.ts | 1 + plugins/auth-backend/src/service/router.ts | 9 +-- 6 files changed, 94 insertions(+), 90 deletions(-) diff --git a/plugins/auth-backend/src/identity/DatabaseKeyStore.ts b/plugins/auth-backend/src/identity/DatabaseKeyStore.ts index 9d288b3624..18e947a378 100644 --- a/plugins/auth-backend/src/identity/DatabaseKeyStore.ts +++ b/plugins/auth-backend/src/identity/DatabaseKeyStore.ts @@ -18,7 +18,7 @@ import Knex from 'knex'; import path from 'path'; import { utc } from 'moment'; import { Logger } from 'winston'; -import { AnyJWK, KeyStore } from './types'; +import { AnyJWK, KeyStore, StoredKey } from './types'; const migrationsDir = path.resolve( require.resolve('@backstage/plugin-auth-backend/package.json'), @@ -36,8 +36,6 @@ type Row = { type Options = { logger: Logger; database: Knex; - /** Expiration time of signing keys in seconds */ - keyDuration: number; }; export class DatabaseKeyStore implements KeyStore { @@ -53,81 +51,36 @@ export class DatabaseKeyStore implements KeyStore { private readonly logger: Logger; private readonly database: Knex; - private readonly keyDuration: number; - - private removingExpiredRows: boolean = false; private constructor(options: Options) { - const { logger, database, keyDuration } = options; - - this.database = database; - this.keyDuration = keyDuration; - this.logger = logger.child({ service: 'key-store' }); + this.database = options.database; + this.logger = options.logger; } - async storeKey({ key }: { key: AnyJWK }): Promise { - this.logger.info(`Storing public key ${key.kid}`); - + async addKey(key: AnyJWK): Promise { await this.database(TABLE).insert({ kid: key.kid, key: JSON.stringify(key), }); } - async listKeys(): Promise<{ keys: AnyJWK[] }> { + async listKeys(): Promise<{ items: StoredKey[] }> { const rows = await this.database(TABLE).select(); - const [validRows, expiredRows] = this.splitExpiredRows(rows); - if (expiredRows.length > 0) { - // We don't await this, just let it run in the background - this.removeExpiredRows(expiredRows); - } - return { - keys: validRows.map(row => JSON.parse(row.key)), + items: rows.map(row => ({ + key: JSON.parse(row.key), + createdAt: utc(row.created_at), + })), }; } - private splitExpiredRows(rows: Row[]) { - const validRows = []; - const expiredRows = []; - - for (const row of rows) { - const createdAt = utc(row.created_at); - const expireAt = createdAt.add(3 * this.keyDuration, 'seconds'); - const isExpired = expireAt.isBefore(); - - if (isExpired) { - expiredRows.push(row); - } else { - validRows.push(row); - } - } - - return [validRows, expiredRows]; - } - - private async removeExpiredRows(rows: Row[]) { - if (this.removingExpiredRows) { - return; - } - - try { - this.removingExpiredRows = true; - - const kids = rows.map(row => row.kid); - this.logger.info(`Removing expired signing keys, '${kids.join(', ')}'`); - - const result = await this.database(TABLE).delete().whereIn('kid', kids); - if (result !== kids.length) { - this.logger.warn( - `Wanted to remove ${kids.length} expired signing, but removed ${result} instead`, - ); - } - } catch (error) { - this.logger.error(`Failed to remove expired signing keys, ${error}`); - } finally { - this.removingExpiredRows = false; + async removeKeys(kids: string[]): Promise { + const result = await this.database(TABLE).delete().whereIn('kid', kids); + if (result !== kids.length) { + this.logger.warn( + `Wanted to remove ${kids.length} keys, but only removed ${result}`, + ); } } } diff --git a/plugins/auth-backend/src/identity/TokenFactory.ts b/plugins/auth-backend/src/identity/TokenFactory.ts index 4d7ba0c7e6..42ce93721b 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -62,6 +62,37 @@ export class TokenFactory implements TokenIssuer { }); } + async listPublicKeys(): Promise<{ keys: AnyJWK[] }> { + const { items: keys } = await this.keyStore.listKeys(); + + const validKeys = []; + const expiredKeys = []; + + for (const key of keys) { + // Allow for a grace period of another full key duration before we remove the keys from the database + const expireAt = key.createdAt.add(3 * this.keyDuration, 'seconds'); + if (expireAt.isBefore()) { + expiredKeys.push(key); + } else { + validKeys.push(key); + } + } + + if (expiredKeys.length > 0) { + const kids = expiredKeys.map(({ key }) => key.kid); + + this.logger.info(`Removing expired signing keys, '${kids.join("', '")}'`); + + // We don't await this, just let it run in the background + this.keyStore.removeKeys(kids).catch(error => { + this.logger.error(`Failed to remove expired keys, ${error}`); + }); + } + + // NOTE: we're currently only storing public keys, but if we start storing private keys we'd have to convert here + return { keys: validKeys.map(({ key }) => key) }; + } + private async getKey(): Promise { if (this.privateKeyPromise) { if (this.keyExpiry && Date.now() < this.keyExpiry) { @@ -79,9 +110,8 @@ export class TokenFactory implements TokenIssuer { alg: 'ES256', }); - await this.keyStore.storeKey({ - key: (key.toJWK(false) as unknown) as AnyJWK, - }); + this.logger.info(`Created new signing key ${key.kid}`); + await this.keyStore.addKey((key.toJWK(false) as unknown) as AnyJWK); return key as JSONWebKey; })(); diff --git a/plugins/auth-backend/src/identity/router.ts b/plugins/auth-backend/src/identity/router.ts index c18fdb7bc8..7e35cce65b 100644 --- a/plugins/auth-backend/src/identity/router.ts +++ b/plugins/auth-backend/src/identity/router.ts @@ -16,16 +16,16 @@ import Router from 'express-promise-router'; import { Logger } from 'winston'; -import { KeyStore } from './types'; +import { TokenIssuer } from './types'; export type Options = { logger: Logger; baseUrl: string; - keyStore: KeyStore; + tokenIssuer: TokenIssuer; }; export function createOidcRouter(options: Options) { - const { logger, keyStore, baseUrl } = options; + const { logger, tokenIssuer, baseUrl } = options; const router = Router(); @@ -50,7 +50,7 @@ export function createOidcRouter(options: Options) { router.get('/.well-known/jwks.json', async (_req, res) => { logger.info('request certs'); - const { keys } = await keyStore.listKeys(); + const { keys } = await tokenIssuer.listPublicKeys(); res.json({ keys }); }); diff --git a/plugins/auth-backend/src/identity/types.ts b/plugins/auth-backend/src/identity/types.ts index 1ceb9dd2cb..827aba51ab 100644 --- a/plugins/auth-backend/src/identity/types.ts +++ b/plugins/auth-backend/src/identity/types.ts @@ -22,21 +22,6 @@ export interface AnyJWK extends Record { kty: string; } -/** A KeyStore can store and keep track of recently used JWKs */ -export type KeyStore = { - /** - * Store a new key to be used for signing. Before the key has been successfully - * stored it should not be used for signing. - */ - storeKey(params: { key: AnyJWK }): Promise; - - /** - * List all keys that are currently being used to sign tokens, or have been used - * in the past within the token expiration time, including a grace period. - */ - listKeys(): Promise<{ keys: AnyJWK[] }>; -}; - /** Parameters used to issue new ID Tokens */ export type TokenParams = { /** The claims that will be embedded within the token */ @@ -46,10 +31,46 @@ export type TokenParams = { }; }; -/** A TokenIssuer is able to issue verifiable ID Tokens on demand */ +/** + * A TokenIssuer is able to issue verifiable ID Tokens on demand. + */ export type TokenIssuer = { /** * Issues a new ID Token */ issueToken(params: TokenParams): Promise; + + /** + * List all public keys that are currently being used to sign tokens, or have been used + * in the past within the token expiration time, including a grace period. + */ + listPublicKeys(): Promise<{ keys: AnyJWK[] }>; +}; + +/** + * A JWK stored by a KeyStore + */ +export type StoredKey = { + key: AnyJWK; + createdAt: moment.Moment; +}; + +/** + * A KeyStore stores JWKs for later and shared use. + */ +export type KeyStore = { + /** + * Store a new key to be used for signing. + */ + addKey(key: AnyJWK): Promise; + + /** + * Remove all keys with the provided kids. + */ + removeKeys(kids: string[]): Promise; + + /** + * List all stored keys. + */ + listKeys(): Promise<{ items: StoredKey[] }>; }; diff --git a/plugins/auth-backend/src/lib/OAuthProvider.test.ts b/plugins/auth-backend/src/lib/OAuthProvider.test.ts index 80c69fdffd..48952e6dad 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.test.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.test.ts @@ -182,6 +182,7 @@ describe('OAuthProvider', () => { appOrigin: 'http://localhost:3000', tokenIssuer: { issueToken: async () => 'my-id-token', + listPublicKeys: async () => ({ keys: [] }), }, }; diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 7809bc7ef8..505b870b50 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -42,13 +42,12 @@ export async function createRouter( const keyStore = await DatabaseKeyStore.create({ database: options.database, logger: logger.child({ component: 'db-key-store' }), - keyDuration, }); - const issuer = new TokenFactory({ + const tokenIssuer = new TokenFactory({ issuer: baseUrl, keyStore, - logger: logger.child({ component: 'token-factory' }), keyDuration, + logger: logger.child({ component: 'token-factory' }), }); router.use(cookieParser()); @@ -104,7 +103,7 @@ export async function createRouter( { baseUrl }, providerConfig, logger, - issuer, + tokenIssuer, ); router.use(`/${providerId}`, providerRouter); } catch (e) { @@ -115,7 +114,7 @@ export async function createRouter( router.use( createOidcRouter({ logger: logger.child({ component: 'oidc-router' }), - keyStore, + tokenIssuer, baseUrl, }), ); From 5d85c188b21595a17ac105adec9cc09ce7f8d7b5 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 20 Jun 2020 16:36:54 +0200 Subject: [PATCH 15/20] auth-backend: added tests for DatabaseKeyStore --- .../src/identity/DatabaseKeyStore.test.ts | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 plugins/auth-backend/src/identity/DatabaseKeyStore.test.ts diff --git a/plugins/auth-backend/src/identity/DatabaseKeyStore.test.ts b/plugins/auth-backend/src/identity/DatabaseKeyStore.test.ts new file mode 100644 index 0000000000..3badd90c2f --- /dev/null +++ b/plugins/auth-backend/src/identity/DatabaseKeyStore.test.ts @@ -0,0 +1,125 @@ +/* + * 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 Knex from 'knex'; +import moment from 'moment'; +import { DatabaseKeyStore } from './DatabaseKeyStore'; +import { getVoidLogger } from '@backstage/backend-common'; + +function createDB() { + const knex = Knex({ + client: 'sqlite3', + connection: ':memory:', + useNullAsDefault: true, + }); + knex.client.pool.on('createSuccess', (_eventId: any, resource: any) => { + resource.run('PRAGMA foreign_keys = ON', () => {}); + }); + return knex; +} + +const keyBase = { + use: 'sig', + kty: 'plain', + alg: 'Base64', +} as const; + +const logger = getVoidLogger(); + +describe('DatabaseKeyStore', () => { + it('should store a key', async () => { + const database = createDB(); + const store = await DatabaseKeyStore.create({ database, logger }); + + const key = { + kid: '123', + ...keyBase, + }; + + await expect(store.listKeys()).resolves.toEqual({ items: [] }); + await store.addKey(key); + + const { items } = await store.listKeys(); + expect(items).toEqual([{ createdAt: expect.anything(), key }]); + expect(Math.abs(items[0].createdAt.diff(moment(), 's'))).toBeLessThan(10); + }); + + it('should remove stored keys', async () => { + const database = createDB(); + const store = await DatabaseKeyStore.create({ database, logger }); + + const key1 = { kid: '1', ...keyBase }; + const key2 = { kid: '2', ...keyBase }; + const key3 = { kid: '3', ...keyBase }; + + await store.addKey(key1); + await store.addKey(key2); + await store.addKey(key3); + + await expect(store.listKeys()).resolves.toEqual({ + items: [ + { key: key1, createdAt: expect.anything() }, + { key: key2, createdAt: expect.anything() }, + { key: key3, createdAt: expect.anything() }, + ], + }); + + store.removeKeys(['1']); + + await expect(store.listKeys()).resolves.toEqual({ + items: [ + { key: key2, createdAt: expect.anything() }, + { key: key3, createdAt: expect.anything() }, + ], + }); + + store.removeKeys(['1', '2']); + + await expect(store.listKeys()).resolves.toEqual({ + items: [{ key: key3, createdAt: expect.anything() }], + }); + + store.removeKeys([]); + + await expect(store.listKeys()).resolves.toEqual({ + items: [{ key: key3, createdAt: expect.anything() }], + }); + + store.removeKeys(['3', '4']); + + await expect(store.listKeys()).resolves.toEqual({ + items: [], + }); + + await store.addKey(key1); + await store.addKey(key2); + await store.addKey(key3); + + await expect(store.listKeys()).resolves.toEqual({ + items: [ + { key: key1, createdAt: expect.anything() }, + { key: key2, createdAt: expect.anything() }, + { key: key3, createdAt: expect.anything() }, + ], + }); + + store.removeKeys(['1', '2', '3']); + + await expect(store.listKeys()).resolves.toEqual({ + items: [], + }); + }); +}); From 8e05f91c2955adcd394076fbdfe5d40b87d01593 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 20 Jun 2020 17:07:27 +0200 Subject: [PATCH 16/20] auth-backend: added tests for TokenFactory and fix keyDuration being ignored --- .../src/identity/TokenFactory.test.ts | 133 ++++++++++++++++++ .../auth-backend/src/identity/TokenFactory.ts | 8 +- 2 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 plugins/auth-backend/src/identity/TokenFactory.test.ts diff --git a/plugins/auth-backend/src/identity/TokenFactory.test.ts b/plugins/auth-backend/src/identity/TokenFactory.test.ts new file mode 100644 index 0000000000..a7e7760dcf --- /dev/null +++ b/plugins/auth-backend/src/identity/TokenFactory.test.ts @@ -0,0 +1,133 @@ +/* + * 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 { 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 }; + }; + return header.kid; +} + +describe('TokenFactory', () => { + it('should issue valid tokens signed by a listed key', async () => { + const keyDuration = 5; + const factory = new TokenFactory({ + issuer: 'my-issuer', + keyStore: new MemoryKeyStore(), + keyDuration, + logger, + }); + + await expect(factory.listPublicKeys()).resolves.toEqual({ keys: [] }); + const token = await factory.issueToken({ claims: { sub: 'foo' } }); + + const { keys } = await factory.listPublicKeys(); + const keyStore = JWKS.asKeyStore({ + keys: keys.map(key => key as JSONWebKey), + }); + + const payload = JWT.verify(token, keyStore) as object & { + iat: number; + exp: number; + }; + expect(payload).toEqual({ + iss: 'my-issuer', + aud: 'backstage', + sub: 'foo', + iat: expect.any(Number), + exp: expect.any(Number), + }); + expect(payload.exp).toBe(payload.iat + keyDuration * 1000); + }); + + it('should generate new signing keys when the current one expires', async () => { + const fixedTime = Date.now(); + jest.spyOn(Date, 'now').mockImplementation(() => fixedTime); + + const factory = new TokenFactory({ + issuer: 'my-issuer', + keyStore: new MemoryKeyStore(), + keyDuration: 5, + logger, + }); + + const token1 = await factory.issueToken({ claims: { sub: 'foo' } }); + const token2 = await factory.issueToken({ claims: { sub: 'foo' } }); + expect(jwtKid(token1)).toBe(jwtKid(token2)); + + await expect(factory.listPublicKeys()).resolves.toEqual({ + keys: [ + expect.objectContaining({ + kid: jwtKid(token1), + }), + ], + }); + + jest.spyOn(Date, 'now').mockImplementation(() => fixedTime + 60000); + + await expect(factory.listPublicKeys()).resolves.toEqual({ + keys: [], + }); + + const token3 = await factory.issueToken({ claims: { sub: 'foo' } }); + expect(jwtKid(token3)).not.toBe(jwtKid(token2)); + + await expect(factory.listPublicKeys()).resolves.toEqual({ + keys: [ + expect.objectContaining({ + kid: jwtKid(token3), + }), + ], + }); + }); +}); diff --git a/plugins/auth-backend/src/identity/TokenFactory.ts b/plugins/auth-backend/src/identity/TokenFactory.ts index 42ce93721b..e596363729 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -19,6 +19,8 @@ import { JSONWebKey, JWK, JWS } from 'jose'; import { Logger } from 'winston'; import { v4 as uuid } from 'uuid'; +const MS_IN_S = 1000; + type Options = { logger: Logger; /** Value of the issuer claim in issued tokens */ @@ -51,8 +53,8 @@ export class TokenFactory implements TokenIssuer { const iss = this.issuer; const sub = params.claims.sub; const aud = 'backstage'; - const iat = (Date.now() / 1000) | 0; - const exp = iat + 3600; + const iat = (Date.now() / MS_IN_S) | 0; + const exp = iat + this.keyDuration * MS_IN_S; this.logger.info(`Issuing token for ${sub}`); @@ -102,7 +104,7 @@ export class TokenFactory implements TokenIssuer { delete this.privateKeyPromise; } - this.keyExpiry = Date.now() + this.keyDuration * 1000; + this.keyExpiry = Date.now() + this.keyDuration * MS_IN_S; const promise = (async () => { const key = await JWK.generate('EC', 'P-256', { use: 'sig', From 20cb44ff2c1c660b862ed22c782a91d3dd2e8150 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 20 Jun 2020 17:27:20 +0200 Subject: [PATCH 17/20] auth-backend: more docs for TokenFactory --- .../auth-backend/src/identity/TokenFactory.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/plugins/auth-backend/src/identity/TokenFactory.ts b/plugins/auth-backend/src/identity/TokenFactory.ts index e596363729..db59354839 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -31,6 +31,20 @@ type Options = { keyDuration: number; }; +/** + * A token issuer that is able to issue tokens in a distributed system + * backed by a single database. Tokens are issued using lazily generated + * signing keys, where each running instance of the auth service uses its own + * signing key. + * + * The public parts of the keys are all stored in the shared key storage, + * and any of the instances of the auth service will return the full list + * of public keys that are currently in storage. + * + * Signing keys are automatically rotated at the same interval as the token + * duration. Expired keys are kept in storage until there are no valid tokens + * in circulation that could have been signed by that key. + */ export class TokenFactory implements TokenIssuer { private readonly issuer: string; private readonly logger: Logger; @@ -64,6 +78,9 @@ export class TokenFactory implements TokenIssuer { }); } + // This will be called by other services that want to verify ID tokens. + // It is important that it returns a list of all public keys that could + // have been used to sign tokens that have not yet expired. async listPublicKeys(): Promise<{ keys: AnyJWK[] }> { const { items: keys } = await this.keyStore.listKeys(); @@ -80,6 +97,7 @@ export class TokenFactory implements TokenIssuer { } } + // Lazily prune expired keys. This may cause duplicate removals if we have concurrent callers, but w/e if (expiredKeys.length > 0) { const kids = expiredKeys.map(({ key }) => key.kid); @@ -96,6 +114,7 @@ export class TokenFactory implements TokenIssuer { } private async getKey(): Promise { + // Make sure that we only generate one key at a time if (this.privateKeyPromise) { if (this.keyExpiry && Date.now() < this.keyExpiry) { return this.privateKeyPromise; @@ -106,15 +125,23 @@ export class TokenFactory implements TokenIssuer { this.keyExpiry = Date.now() + this.keyDuration * MS_IN_S; const promise = (async () => { + // This generates a new signing key to be used to sign tokens until the next key rotation const key = await JWK.generate('EC', 'P-256', { use: 'sig', kid: uuid(), alg: 'ES256', }); + // We're not allowed to use the key until it has been successfully stored + // TODO: some token verification implementations aggressively cache the list of keys, and + // don't attempt to fetch new ones even if they encounter an unknown kid. Therefore we + // may want to keep using the existing key for some period of time until we switch to + // the new one. This also needs to be implemented cross-service though, meaning new services + // that boot up need to be able to grab an existing key to use for signing. this.logger.info(`Created new signing key ${key.kid}`); await this.keyStore.addKey((key.toJWK(false) as unknown) as AnyJWK); + // At this point we are allowed to start using the new key return key as JSONWebKey; })(); From 031b1d8b3902dbfce8be4321c2804ddd58c33e65 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 20 Jun 2020 21:14:26 +0200 Subject: [PATCH 18/20] auth-backend: remove logging from DatabaseKeyStore --- .../src/identity/DatabaseKeyStore.test.ts | 7 ++----- plugins/auth-backend/src/identity/DatabaseKeyStore.ts | 11 +---------- plugins/auth-backend/src/service/router.ts | 1 - 3 files changed, 3 insertions(+), 16 deletions(-) diff --git a/plugins/auth-backend/src/identity/DatabaseKeyStore.test.ts b/plugins/auth-backend/src/identity/DatabaseKeyStore.test.ts index 3badd90c2f..b22ab0ecda 100644 --- a/plugins/auth-backend/src/identity/DatabaseKeyStore.test.ts +++ b/plugins/auth-backend/src/identity/DatabaseKeyStore.test.ts @@ -17,7 +17,6 @@ import Knex from 'knex'; import moment from 'moment'; import { DatabaseKeyStore } from './DatabaseKeyStore'; -import { getVoidLogger } from '@backstage/backend-common'; function createDB() { const knex = Knex({ @@ -37,12 +36,10 @@ const keyBase = { alg: 'Base64', } as const; -const logger = getVoidLogger(); - describe('DatabaseKeyStore', () => { it('should store a key', async () => { const database = createDB(); - const store = await DatabaseKeyStore.create({ database, logger }); + const store = await DatabaseKeyStore.create({ database }); const key = { kid: '123', @@ -59,7 +56,7 @@ describe('DatabaseKeyStore', () => { it('should remove stored keys', async () => { const database = createDB(); - const store = await DatabaseKeyStore.create({ database, logger }); + const store = await DatabaseKeyStore.create({ database }); const key1 = { kid: '1', ...keyBase }; const key2 = { kid: '2', ...keyBase }; diff --git a/plugins/auth-backend/src/identity/DatabaseKeyStore.ts b/plugins/auth-backend/src/identity/DatabaseKeyStore.ts index 18e947a378..c175cd6e12 100644 --- a/plugins/auth-backend/src/identity/DatabaseKeyStore.ts +++ b/plugins/auth-backend/src/identity/DatabaseKeyStore.ts @@ -17,7 +17,6 @@ import Knex from 'knex'; import path from 'path'; import { utc } from 'moment'; -import { Logger } from 'winston'; import { AnyJWK, KeyStore, StoredKey } from './types'; const migrationsDir = path.resolve( @@ -34,7 +33,6 @@ type Row = { }; type Options = { - logger: Logger; database: Knex; }; @@ -49,12 +47,10 @@ export class DatabaseKeyStore implements KeyStore { return new DatabaseKeyStore(options); } - private readonly logger: Logger; private readonly database: Knex; private constructor(options: Options) { this.database = options.database; - this.logger = options.logger; } async addKey(key: AnyJWK): Promise { @@ -76,11 +72,6 @@ export class DatabaseKeyStore implements KeyStore { } async removeKeys(kids: string[]): Promise { - const result = await this.database(TABLE).delete().whereIn('kid', kids); - if (result !== kids.length) { - this.logger.warn( - `Wanted to remove ${kids.length} keys, but only removed ${result}`, - ); - } + await this.database(TABLE).delete().whereIn('kid', kids); } } diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 505b870b50..3787458f9b 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -41,7 +41,6 @@ export async function createRouter( const keyStore = await DatabaseKeyStore.create({ database: options.database, - logger: logger.child({ component: 'db-key-store' }), }); const tokenIssuer = new TokenFactory({ issuer: baseUrl, From 3562d836fe06f35477ba00593b40a014d2e1116d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 21 Jun 2020 15:47:13 +0200 Subject: [PATCH 19/20] auth-backend: some review feedback of oidc bits --- plugins/auth-backend/src/identity/TokenFactory.ts | 2 +- plugins/auth-backend/src/identity/router.ts | 8 +------- plugins/auth-backend/src/service/router.ts | 1 - 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/plugins/auth-backend/src/identity/TokenFactory.ts b/plugins/auth-backend/src/identity/TokenFactory.ts index db59354839..a9a8ad5a30 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -67,7 +67,7 @@ export class TokenFactory implements TokenIssuer { const iss = this.issuer; const sub = params.claims.sub; const aud = 'backstage'; - const iat = (Date.now() / MS_IN_S) | 0; + const iat = Math.floor(Date.now() / MS_IN_S); const exp = iat + this.keyDuration * MS_IN_S; this.logger.info(`Issuing token for ${sub}`); diff --git a/plugins/auth-backend/src/identity/router.ts b/plugins/auth-backend/src/identity/router.ts index 7e35cce65b..de05e8a06c 100644 --- a/plugins/auth-backend/src/identity/router.ts +++ b/plugins/auth-backend/src/identity/router.ts @@ -15,17 +15,15 @@ */ import Router from 'express-promise-router'; -import { Logger } from 'winston'; import { TokenIssuer } from './types'; export type Options = { - logger: Logger; baseUrl: string; tokenIssuer: TokenIssuer; }; export function createOidcRouter(options: Options) { - const { logger, tokenIssuer, baseUrl } = options; + const { baseUrl, tokenIssuer } = options; const router = Router(); @@ -44,23 +42,19 @@ export function createOidcRouter(options: Options) { }; router.get('/.well-known/openid-configuration', (_req, res) => { - logger.info('request configuration'); res.json(config); }); router.get('/.well-known/jwks.json', async (_req, res) => { - logger.info('request certs'); const { keys } = await tokenIssuer.listPublicKeys(); res.json({ keys }); }); router.get('/v1/token', (_req, res) => { - logger.info('request token'); res.status(501).send('Not Implemented'); }); router.get('/v1/userinfo', (_req, res) => { - logger.info('request userinfo'); res.status(501).send('Not Implemented'); }); diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 3787458f9b..dcd650c858 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -112,7 +112,6 @@ export async function createRouter( router.use( createOidcRouter({ - logger: logger.child({ component: 'oidc-router' }), tokenIssuer, baseUrl, }), From 180be84ba2cd8eb33af83c761fd4d72345776f7c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 21 Jun 2020 16:07:57 +0200 Subject: [PATCH 20/20] auth-backend: clean up naming and types of TokenFactory time handling --- .../src/identity/TokenFactory.test.ts | 8 ++++---- .../auth-backend/src/identity/TokenFactory.ts | 17 +++++++++-------- plugins/auth-backend/src/service/router.ts | 4 ++-- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/plugins/auth-backend/src/identity/TokenFactory.test.ts b/plugins/auth-backend/src/identity/TokenFactory.test.ts index a7e7760dcf..4303401ee6 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.test.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.test.ts @@ -60,11 +60,11 @@ function jwtKid(jwt: string): string { describe('TokenFactory', () => { it('should issue valid tokens signed by a listed key', async () => { - const keyDuration = 5; + const keyDurationSeconds = 5; const factory = new TokenFactory({ issuer: 'my-issuer', keyStore: new MemoryKeyStore(), - keyDuration, + keyDurationSeconds, logger, }); @@ -87,7 +87,7 @@ describe('TokenFactory', () => { iat: expect.any(Number), exp: expect.any(Number), }); - expect(payload.exp).toBe(payload.iat + keyDuration * 1000); + expect(payload.exp).toBe(payload.iat + keyDurationSeconds * 1000); }); it('should generate new signing keys when the current one expires', async () => { @@ -97,7 +97,7 @@ describe('TokenFactory', () => { const factory = new TokenFactory({ issuer: 'my-issuer', keyStore: new MemoryKeyStore(), - keyDuration: 5, + keyDurationSeconds: 5, logger, }); diff --git a/plugins/auth-backend/src/identity/TokenFactory.ts b/plugins/auth-backend/src/identity/TokenFactory.ts index a9a8ad5a30..c4c259ed22 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import moment from 'moment'; import { TokenIssuer, TokenParams, KeyStore, AnyJWK } from './types'; import { JSONWebKey, JWK, JWS } from 'jose'; import { Logger } from 'winston'; @@ -28,7 +29,7 @@ type Options = { /** Key store used for storing signing keys */ keyStore: KeyStore; /** Expiration time of signing keys in seconds */ - keyDuration: number; + keyDurationSeconds: number; }; /** @@ -49,16 +50,16 @@ export class TokenFactory implements TokenIssuer { private readonly issuer: string; private readonly logger: Logger; private readonly keyStore: KeyStore; - private readonly keyDuration: number; + private readonly keyDurationSeconds: number; - private keyExpiry?: number; + private keyExpiry?: moment.Moment; private privateKeyPromise?: Promise; constructor(options: Options) { this.issuer = options.issuer; this.logger = options.logger; this.keyStore = options.keyStore; - this.keyDuration = options.keyDuration; + this.keyDurationSeconds = options.keyDurationSeconds; } async issueToken(params: TokenParams): Promise { @@ -68,7 +69,7 @@ export class TokenFactory implements TokenIssuer { const sub = params.claims.sub; const aud = 'backstage'; const iat = Math.floor(Date.now() / MS_IN_S); - const exp = iat + this.keyDuration * MS_IN_S; + const exp = iat + this.keyDurationSeconds * MS_IN_S; this.logger.info(`Issuing token for ${sub}`); @@ -89,7 +90,7 @@ export class TokenFactory implements TokenIssuer { for (const key of keys) { // Allow for a grace period of another full key duration before we remove the keys from the database - const expireAt = key.createdAt.add(3 * this.keyDuration, 'seconds'); + const expireAt = key.createdAt.add(3 * this.keyDurationSeconds, 's'); if (expireAt.isBefore()) { expiredKeys.push(key); } else { @@ -116,14 +117,14 @@ export class TokenFactory implements TokenIssuer { private async getKey(): Promise { // Make sure that we only generate one key at a time if (this.privateKeyPromise) { - if (this.keyExpiry && Date.now() < this.keyExpiry) { + if (this.keyExpiry?.isAfter()) { return this.privateKeyPromise; } this.logger.info(`Signing key has expired, generating new key`); delete this.privateKeyPromise; } - this.keyExpiry = Date.now() + this.keyDuration * MS_IN_S; + this.keyExpiry = moment().add(this.keyDurationSeconds, 'seconds'); const promise = (async () => { // This generates a new signing key to be used to sign tokens until the next key rotation const key = await JWK.generate('EC', 'P-256', { diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index dcd650c858..68b2d31032 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -37,7 +37,7 @@ export async function createRouter( const logger = options.logger.child({ plugin: 'auth' }); const baseUrl = `${options.config.getString('backend.baseUrl')}/auth`; - const keyDuration = 3600; + const keyDurationSeconds = 3600; const keyStore = await DatabaseKeyStore.create({ database: options.database, @@ -45,7 +45,7 @@ export async function createRouter( const tokenIssuer = new TokenFactory({ issuer: baseUrl, keyStore, - keyDuration, + keyDurationSeconds, logger: logger.child({ component: 'token-factory' }), });