diff --git a/.changeset/thick-pugs-rush.md b/.changeset/thick-pugs-rush.md new file mode 100644 index 0000000000..7f0171eb83 --- /dev/null +++ b/.changeset/thick-pugs-rush.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend': patch +--- + +Added a persistent session store through the database diff --git a/plugins/auth-backend/migrations/20230428155633_sessions.js b/plugins/auth-backend/migrations/20230428155633_sessions.js new file mode 100644 index 0000000000..49462ac503 --- /dev/null +++ b/plugins/auth-backend/migrations/20230428155633_sessions.js @@ -0,0 +1,52 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// @ts-check + +/** + * @param {import('knex').Knex} knex + */ +exports.up = async function up(knex) { + // See https://github.com/gx0r/connect-session-knex + // Modeled loosely after https://github.com/gx0r/connect-session-knex/blob/4e0e36a9afbb13c3000a89f5e341f2d2d4339a02/lib/index.js#L114 + // For simplicity we always make the session a string + // Do NOT change around this table or column names; the connect-session-knex library makes assumptions about them + await knex.schema.createTable('sessions', table => { + table.comment('Session data'); + table.string('sid').primary().notNullable().comment('ID of the session'); + table + .text('sess', 'longtext') + .notNullable() + .comment('Session data, JSON serialized'); + table + .timestamp('expired') + .notNullable() + .comment('The point in time when the session expires'); + table.index('sid', 'sessions_sid_idx'); + table.index('expired', 'sessions_expired_idx'); + }); +}; + +/** + * @param {import('knex').Knex} knex + */ +exports.down = async function down(knex) { + await knex.schema.alterTable('sessions', table => { + table.dropIndex([], 'sessions_sid_idx'); + table.dropIndex([], 'sessions_expired_idx'); + }); + await knex.schema.dropTable('sessions'); +}; diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index 61c3fe9a8d..aa57e06708 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -44,6 +44,7 @@ "@types/express": "^4.17.6", "@types/passport": "^1.0.3", "compression": "^1.7.4", + "connect-session-knex": "^3.0.1", "cookie-parser": "^1.4.5", "cors": "^2.8.5", "express": "^4.17.1", diff --git a/plugins/auth-backend/src/database/AuthDatabase.ts b/plugins/auth-backend/src/database/AuthDatabase.ts new file mode 100644 index 0000000000..9c112e0aac --- /dev/null +++ b/plugins/auth-backend/src/database/AuthDatabase.ts @@ -0,0 +1,76 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + DatabaseManager, + PluginDatabaseManager, + resolvePackagePath, +} from '@backstage/backend-common'; +import { ConfigReader } from '@backstage/config'; +import { Knex } from 'knex'; + +const migrationsDir = resolvePackagePath( + '@backstage/plugin-auth-backend', + 'migrations', +); + +/** + * Ensures that a database connection is established exactly once and only when + * asked for, and runs migrations. + */ +export class AuthDatabase { + readonly #database: PluginDatabaseManager; + #promise: Promise | undefined; + + static create(database: PluginDatabaseManager): AuthDatabase { + return new AuthDatabase(database); + } + + static forTesting(): AuthDatabase { + const config = new ConfigReader({ + backend: { + database: { + client: 'better-sqlite3', + connection: ':memory:', + useNullAsDefault: true, + }, + }, + }); + const database = DatabaseManager.fromConfig(config).forPlugin('auth'); + return new AuthDatabase(database); + } + + static async runMigrations(knex: Knex): Promise { + await knex.migrate.latest({ + directory: migrationsDir, + }); + } + + private constructor(database: PluginDatabaseManager) { + this.#database = database; + } + + get(): Promise { + this.#promise ??= this.#database.getClient().then(async client => { + if (!this.#database.migrations?.skip) { + await AuthDatabase.runMigrations(client); + } + return client; + }); + + return this.#promise; + } +} diff --git a/plugins/auth-backend/src/identity/DatabaseKeyStore.test.ts b/plugins/auth-backend/src/identity/DatabaseKeyStore.test.ts index da67a2f88f..913dfcef22 100644 --- a/plugins/auth-backend/src/identity/DatabaseKeyStore.test.ts +++ b/plugins/auth-backend/src/identity/DatabaseKeyStore.test.ts @@ -14,33 +14,10 @@ * limitations under the License. */ -import Knex, { Knex as KnexType } from 'knex'; -import { DatabaseKeyStore } from './DatabaseKeyStore'; import { DateTime } from 'luxon'; - -function createDatabaseManager( - client: KnexType, - skipMigrations: boolean = false, -) { - return { - getClient: async () => client, - migrations: { - skip: skipMigrations, - }, - }; -} - -function createDB() { - const knex = Knex({ - client: 'better-sqlite3', - connection: ':memory:', - useNullAsDefault: true, - }); - knex.client.pool.on('createSuccess', (_eventId: any, resource: any) => { - resource.run('PRAGMA foreign_keys = ON', () => {}); - }); - return knex; -} +import { AuthDatabase } from '../database/AuthDatabase'; +import { DatabaseKeyStore } from './DatabaseKeyStore'; +import { TestDatabases } from '@backstage/backend-test-utils'; const keyBase = { use: 'sig', @@ -48,95 +25,107 @@ const keyBase = { alg: 'Base64', } as const; +jest.setTimeout(60_000); + describe('DatabaseKeyStore', () => { - it('should store a key', async () => { - const client = createDB(); - const store = await DatabaseKeyStore.create({ - database: createDatabaseManager(client), - }); - - 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( - DateTime.fromJSDate(items[0].createdAt).diffNow('seconds').seconds, - ), - ).toBeLessThan(10); + const databases = TestDatabases.create({ + ids: ['POSTGRES_13', 'POSTGRES_9', 'SQLITE_3'], }); - it('should remove stored keys', async () => { - const client = createDB(); - const store = await DatabaseKeyStore.create({ - database: createDatabaseManager(client), - }); + it.each(databases.eachSupportedId())( + 'should store a key, %p', + async databaseId => { + const knex = await databases.init(databaseId); + await AuthDatabase.runMigrations(knex); - const key1 = { kid: '1', ...keyBase }; - const key2 = { kid: '2', ...keyBase }; - const key3 = { kid: '3', ...keyBase }; + const store = new DatabaseKeyStore(knex); - await store.addKey(key1); - await store.addKey(key2); - await store.addKey(key3); + const key = { + kid: '123', + ...keyBase, + }; - await expect(store.listKeys()).resolves.toEqual({ - items: [ - { key: key1, createdAt: expect.anything() }, - { key: key2, createdAt: expect.anything() }, - { key: key3, createdAt: expect.anything() }, - ], - }); + await expect(store.listKeys()).resolves.toEqual({ items: [] }); + await store.addKey(key); - store.removeKeys(['1']); + const { items } = await store.listKeys(); + expect(items).toEqual([{ createdAt: expect.anything(), key }]); + expect( + Math.abs( + DateTime.fromJSDate(items[0].createdAt).diffNow('seconds').seconds, + ), + ).toBeLessThan(10); + }, + ); - await expect(store.listKeys()).resolves.toEqual({ - items: [ - { key: key2, createdAt: expect.anything() }, - { key: key3, createdAt: expect.anything() }, - ], - }); + it.each(databases.eachSupportedId())( + 'should remove stored keys, %p', + async databaseId => { + const knex = await databases.init(databaseId); + await AuthDatabase.runMigrations(knex); - store.removeKeys(['1', '2']); + const store = new DatabaseKeyStore(knex); - await expect(store.listKeys()).resolves.toEqual({ - items: [{ key: key3, createdAt: expect.anything() }], - }); + const key1 = { kid: '1', ...keyBase }; + const key2 = { kid: '2', ...keyBase }; + const key3 = { kid: '3', ...keyBase }; - store.removeKeys([]); + await store.addKey(key1); + await store.addKey(key2); + await store.addKey(key3); - await expect(store.listKeys()).resolves.toEqual({ - items: [{ key: key3, createdAt: expect.anything() }], - }); + await expect(store.listKeys()).resolves.toEqual({ + items: [ + { key: key1, createdAt: expect.anything() }, + { key: key2, createdAt: expect.anything() }, + { key: key3, createdAt: expect.anything() }, + ], + }); - store.removeKeys(['3', '4']); + await store.removeKeys(['1']); - await expect(store.listKeys()).resolves.toEqual({ - items: [], - }); + await expect(store.listKeys()).resolves.toEqual({ + items: [ + { key: key2, createdAt: expect.anything() }, + { key: key3, createdAt: expect.anything() }, + ], + }); - await store.addKey(key1); - await store.addKey(key2); - await store.addKey(key3); + await store.removeKeys(['1', '2']); - await expect(store.listKeys()).resolves.toEqual({ - items: [ - { key: key1, createdAt: expect.anything() }, - { key: key2, createdAt: expect.anything() }, - { key: key3, createdAt: expect.anything() }, - ], - }); + await expect(store.listKeys()).resolves.toEqual({ + items: [{ key: key3, createdAt: expect.anything() }], + }); - store.removeKeys(['1', '2', '3']); + await store.removeKeys([]); - await expect(store.listKeys()).resolves.toEqual({ - items: [], - }); - }); + await expect(store.listKeys()).resolves.toEqual({ + items: [{ key: key3, createdAt: expect.anything() }], + }); + + await 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() }, + ], + }); + + await store.removeKeys(['1', '2', '3']); + + await expect(store.listKeys()).resolves.toEqual({ + items: [], + }); + }, + ); }); diff --git a/plugins/auth-backend/src/identity/DatabaseKeyStore.ts b/plugins/auth-backend/src/identity/DatabaseKeyStore.ts index 8039837469..0559b698e9 100644 --- a/plugins/auth-backend/src/identity/DatabaseKeyStore.ts +++ b/plugins/auth-backend/src/identity/DatabaseKeyStore.ts @@ -14,19 +14,10 @@ * limitations under the License. */ -import { - PluginDatabaseManager, - resolvePackagePath, -} from '@backstage/backend-common'; import { Knex } from 'knex'; import { DateTime } from 'luxon'; import { AnyJWK, KeyStore, StoredKey } from './types'; -const migrationsDir = resolvePackagePath( - '@backstage/plugin-auth-backend', - 'migrations', -); - const TABLE = 'signing_keys'; type Row = { @@ -35,10 +26,6 @@ type Row = { key: string; }; -type Options = { - database: PluginDatabaseManager; -}; - const parseDate = (date: string | Date) => { const parsedDate = typeof date === 'string' @@ -55,24 +42,7 @@ const parseDate = (date: string | Date) => { }; export class DatabaseKeyStore implements KeyStore { - static async create(options: Options): Promise { - const { database } = options; - const client = await database.getClient(); - - if (!database.migrations?.skip) { - await client.migrate.latest({ - directory: migrationsDir, - }); - } - - return new DatabaseKeyStore(client); - } - - private readonly client: Knex; - - private constructor(client: Knex) { - this.client = client; - } + constructor(private readonly client: Knex) {} async addKey(key: AnyJWK): Promise { await this.client(TABLE).insert({ diff --git a/plugins/auth-backend/src/identity/KeyStores.test.ts b/plugins/auth-backend/src/identity/KeyStores.test.ts index 375db4698b..d186db6c46 100644 --- a/plugins/auth-backend/src/identity/KeyStores.test.ts +++ b/plugins/auth-backend/src/identity/KeyStores.test.ts @@ -14,13 +14,12 @@ * limitations under the License. */ -import { DatabaseManager } from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; - -import { MemoryKeyStore } from './MemoryKeyStore'; +import { AuthDatabase } from '../database/AuthDatabase'; import { DatabaseKeyStore } from './DatabaseKeyStore'; import { FirestoreKeyStore } from './FirestoreKeyStore'; import { KeyStores } from './KeyStores'; +import { MemoryKeyStore } from './MemoryKeyStore'; describe('KeyStores', () => { const defaultConfigOptions = { @@ -46,18 +45,9 @@ describe('KeyStores', () => { }); it('can handle without auth config', async () => { - const config = new ConfigReader({ - backend: { - database: { - client: 'better-sqlite3', - connection: ':memory:', - }, - }, + const keyStore = await KeyStores.fromConfig(new ConfigReader({}), { + database: AuthDatabase.forTesting(), }); - const database = - DatabaseManager.fromConfig(config).forPlugin('auth-backend'); - const keyStore = await KeyStores.fromConfig(config, { database }); - expect(keyStore).toBeInstanceOf(DatabaseKeyStore); }); diff --git a/plugins/auth-backend/src/identity/KeyStores.ts b/plugins/auth-backend/src/identity/KeyStores.ts index e29b4b6fc4..d000a96879 100644 --- a/plugins/auth-backend/src/identity/KeyStores.ts +++ b/plugins/auth-backend/src/identity/KeyStores.ts @@ -14,20 +14,20 @@ * limitations under the License. */ -import { Logger } from 'winston'; import { pickBy } from 'lodash'; +import { Logger } from 'winston'; -import { PluginDatabaseManager } from '@backstage/backend-common'; import { Config } from '@backstage/config'; +import { AuthDatabase } from '../database/AuthDatabase'; import { DatabaseKeyStore } from './DatabaseKeyStore'; -import { MemoryKeyStore } from './MemoryKeyStore'; import { FirestoreKeyStore } from './FirestoreKeyStore'; +import { MemoryKeyStore } from './MemoryKeyStore'; import { KeyStore } from './types'; type Options = { logger?: Logger; - database?: PluginDatabaseManager; + database: AuthDatabase; }; export class KeyStores { @@ -52,8 +52,7 @@ export class KeyStores { if (!database) { throw new Error('This KeyStore provider requires a database'); } - - return await DatabaseKeyStore.create({ database }); + return new DatabaseKeyStore(await database.get()); } if (provider === 'memory') { diff --git a/plugins/auth-backend/src/migrations.test.ts b/plugins/auth-backend/src/migrations.test.ts new file mode 100644 index 0000000000..75df0de3e2 --- /dev/null +++ b/plugins/auth-backend/src/migrations.test.ts @@ -0,0 +1,78 @@ +/* + * Copyright 2022 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Knex } from 'knex'; +import { TestDatabases } from '@backstage/backend-test-utils'; +import fs from 'fs'; + +const migrationsDir = `${__dirname}/../migrations`; +const migrationsFiles = fs.readdirSync(migrationsDir).sort(); + +async function migrateUpOnce(knex: Knex): Promise { + await knex.migrate.up({ directory: migrationsDir }); +} + +async function migrateDownOnce(knex: Knex): Promise { + await knex.migrate.down({ directory: migrationsDir }); +} + +async function migrateUntilBefore(knex: Knex, target: string): Promise { + const index = migrationsFiles.indexOf(target); + if (index === -1) { + throw new Error(`Migration ${target} not found`); + } + for (let i = 0; i < index; i++) { + await migrateUpOnce(knex); + } +} + +jest.setTimeout(60_000); + +describe('migrations', () => { + const databases = TestDatabases.create({ + ids: ['POSTGRES_13', 'POSTGRES_9', 'SQLITE_3'], + }); + + it.each(databases.eachSupportedId())( + '20230428155633_sessions.js, %p', + async databaseId => { + const knex = await databases.init(databaseId); + + await migrateUntilBefore(knex, '20230428155633_sessions.js'); + await migrateUpOnce(knex); + + // Ensure that large cookies are supported + const data = `{"cookie":"${'a'.repeat(100_000)}"}`; + await knex + .insert({ sid: 'abc', expired: knex.fn.now(), sess: data }) + .into('sessions'); + await knex + .insert({ sid: 'def', expired: knex.fn.now(), sess: data }) + .into('sessions'); + + await expect(knex('sessions').orderBy('sid', 'asc')).resolves.toEqual([ + { sid: 'abc', expired: expect.anything(), sess: data }, + { sid: 'def', expired: expect.anything(), sess: data }, + ]); + + await migrateDownOnce(knex); + + await expect(knex('sessions')).rejects.toThrow(); + + await knex.destroy(); + }, + ); +}); diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 8013777415..2996ff68be 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -32,9 +32,11 @@ import { CatalogApi, CatalogClient } from '@backstage/catalog-client'; import { Config } from '@backstage/config'; import { createOidcRouter, TokenFactory, KeyStores } from '../identity'; import session from 'express-session'; +import connectSessionKnex from 'connect-session-knex'; import passport from 'passport'; import { Minimatch } from 'minimatch'; import { CatalogAuthResolverContext } from '../lib/resolvers'; +import { AuthDatabase } from '../database/AuthDatabase'; /** @public */ export type ProviderFactories = { [s: string]: AuthProviderFactory }; @@ -70,7 +72,11 @@ export async function createRouter( const appUrl = config.getString('app.baseUrl'); const authUrl = await discovery.getExternalBaseUrl('auth'); - const keyStore = await KeyStores.fromConfig(config, { logger, database }); + const authDb = AuthDatabase.create(database); + const keyStore = await KeyStores.fromConfig(config, { + logger, + database: authDb, + }); const keyDurationSeconds = 3600; const tokenIssuer = new TokenFactory({ @@ -84,14 +90,18 @@ export async function createRouter( const secret = config.getOptionalString('auth.session.secret'); if (secret) { router.use(cookieParser(secret)); - // TODO: Configure the server-side session storage. The default MemoryStore is not designed for production const enforceCookieSSL = authUrl.startsWith('https'); + const KnexSessionStore = connectSessionKnex(session); router.use( session({ secret, saveUninitialized: false, resave: false, cookie: { secure: enforceCookieSSL ? 'auto' : false }, + store: new KnexSessionStore({ + createtable: false, + knex: await authDb.get(), + }), }), ); router.use(passport.initialize()); diff --git a/yarn.lock b/yarn.lock index d9be5d6064..f0648b5a13 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4796,6 +4796,7 @@ __metadata: "@types/passport-strategy": ^0.2.35 "@types/xml2js": ^0.4.7 compression: ^1.7.4 + connect-session-knex: ^3.0.1 cookie-parser: ^1.4.5 cors: ^2.8.5 express: ^4.17.1 @@ -20544,6 +20545,16 @@ __metadata: languageName: node linkType: hard +"connect-session-knex@npm:^3.0.1": + version: 3.0.1 + resolution: "connect-session-knex@npm:3.0.1" + dependencies: + bluebird: ^3.7.2 + knex: ^2.3.0 + checksum: f5a80c3c34d30e7cd4e79a6aae09d112825986d51ddc3a4563ef95ade425178239d4e3e05420fb3b6204b2353b0dfd754d318b288989b539583f6c0e52758b7a + languageName: node + linkType: hard + "consola@npm:^2.15.0": version: 2.15.3 resolution: "consola@npm:2.15.3" @@ -28982,7 +28993,7 @@ __metadata: languageName: node linkType: hard -"knex@npm:^2.0.0, knex@npm:^2.4.2": +"knex@npm:^2.0.0, knex@npm:^2.3.0, knex@npm:^2.4.2": version: 2.4.2 resolution: "knex@npm:2.4.2" dependencies: