diff --git a/migrations/20230316134353-oidc-clients-add-table.js b/migrations/20230316134353-oidc-clients-add-table.js new file mode 100644 index 000000000..e82c613d8 --- /dev/null +++ b/migrations/20230316134353-oidc-clients-add-table.js @@ -0,0 +1,15 @@ +'use strict'; + +exports.up = async function (db) { + var cmd = 'CREATE TABLE oidcClients(' + + 'id VARCHAR(128) NOT NULL UNIQUE,' + + 'secret VARCHAR(128) NOT NULL,' + + 'redirectUri VARCHAR(256) NOT NULL,' + + 'PRIMARY KEY (id))'; + + await db.runSql(cmd); +}; + +exports.down = async function (db) { + await db.runSql('DROP TABLE oidcClients'); +}; diff --git a/migrations/schema.sql b/migrations/schema.sql index 4f0736abe..6e15af10e 100644 --- a/migrations/schema.sql +++ b/migrations/schema.sql @@ -313,4 +313,8 @@ CREATE TABLE IF NOT EXISTS appLinks( PRIMARY KEY(id)); -CHARACTER SET utf8 COLLATE utf8_bin; +CREATE TABLE IF NOT EXISTS oidcClients( + id VARCHAR(128) NOT NULL UNIQUE, + secret VARCHAR(128) DEFAULT "", + redirectUri VARCHAR(256) DEFAULT "", + PRIMARY KEY(id)); diff --git a/src/oidc.js b/src/oidc.js index 06d5dddcb..9e99e5586 100644 --- a/src/oidc.js +++ b/src/oidc.js @@ -2,7 +2,13 @@ exports = module.exports = { getProvider, - upsertClient, + clients: { + add: clientsAdd, + get: clientsGet, + del: clientsDel, + update: clientsUpdate, + list: clientsList + }, routes: { renderInteractionPage, interactionLogin, @@ -12,21 +18,66 @@ exports = module.exports = { }; const assert = require('assert'), + BoxError = require('./boxerror.js'), + database = require('./database.js'), debug = require('debug')('box:oidc'), fs = require('fs'), middleware = require('./middleware'), path = require('path'), paths = require('./paths.js'), - BoxError = require('./boxerror.js'), HttpError = require('connect-lastmile').HttpError, HttpSuccess = require('connect-lastmile').HttpSuccess, users = require('./users.js'), safe = require('safetydance'), settings = require('./settings.js'); -// currently non-persistent client store instead of .json file -// FIXME this has to go to our db -const gClients = {}; +const OIDC_CLIENTS_TABLE_NAME = 'oidcClients'; +const OIDC_CLIENTS_FIELDS = [ 'id', 'secret', 'redirectUri' ]; + +async function clientsAdd(id, secret, redirectUri) { + assert.strictEqual(typeof id, 'string'); + assert.strictEqual(typeof secret, 'string'); + assert.strictEqual(typeof redirectUri, 'string'); + + const query = 'INSERT INTO oidcClients (id, secret, redirectUri) VALUES (?, ?, ?)'; + const args = [ id, secret, redirectUri ]; + + const [error] = await safe(database.query(query, args)); + if (error && error.code === 'ER_DUP_ENTRY') throw new BoxError(BoxError.ALREADY_EXISTS, 'client already exists'); + if (error) throw error; +} + +async function clientsGet(id) { + assert.strictEqual(typeof id, 'string'); + + debug(`clientsGet: id:${id}`); + + const result = await database.query(`SELECT ${OIDC_CLIENTS_FIELDS} FROM ${OIDC_CLIENTS_TABLE_NAME} WHERE id = ?`, [ id ]); + if (result.length === 0) return null; + + return result[0]; +} + +async function clientsUpdate(id, secret, redirectUri) { + assert.strictEqual(typeof id, 'string'); + assert.strictEqual(typeof secret, 'string'); + assert.strictEqual(typeof redirectUri, 'string'); + + const result = await database.query(`UPDATE ${OIDC_CLIENTS_TABLE_NAME} SET secret=?, redirectUri=? WHERE id = ?`, [ secret, redirectUri, id]); + if (result.affectedRows !== 1) throw new BoxError(BoxError.NOT_FOUND, 'client not found'); +} + +async function clientsDel(id) { + assert.strictEqual(typeof id, 'string'); + + const result = await database.query(`DELETE FROM ${OIDC_CLIENTS_TABLE_NAME} WHERE id = ?`, [ id ]); + if (result.affectedRows !== 1) throw new BoxError(BoxError.NOT_FOUND, 'client not found'); +} + +async function clientsList() { + const results = await database.query(`SELECT * FROM ${OIDC_CLIENTS_TABLE_NAME}`, []); + return results; +} class CloudronAdapter { @@ -44,8 +95,8 @@ class CloudronAdapter { constructor(name) { this.name = name; - if (name === 'Client') { - this.store = gClients; + if (this.name === 'Client') { + this.store = null; this.fileStorePath = null; } else { this.fileStorePath = path.join(paths.OIDC_STORE_DIR, `${name}.json`); @@ -78,8 +129,12 @@ class CloudronAdapter { async upsert(id, payload, expiresIn) { debug(`[${this.name}] upsert id:${id} expiresIn:${expiresIn}`, payload); - this.store[id] = { id, expiresIn, payload, consumed: false }; - if (this.fileStorePath) fs.writeFileSync(this.fileStorePath, JSON.stringify(this.store), 'utf8'); + if (this.name === 'Client') { + console.log('WARNING!! this should not happen as it is stored in our db'); + } else { + this.store[id] = { id, expiresIn, payload, consumed: false }; + if (this.fileStorePath) fs.writeFileSync(this.fileStorePath, JSON.stringify(this.store), 'utf8'); + } } /** @@ -95,11 +150,24 @@ class CloudronAdapter { async find(id) { debug(`[${this.name}] find id:${id}`); - if (!this.store[id]) return false; + if (this.name === 'Client') { + const [error, client] = await safe(clientsGet(id)); + if (error) return null; - debug(`[${this.name}] find id:${id}`, this.store[id]); + debug(`[${this.name}] find id:${id}`, client); - return this.store[id].payload; + return { + client_id: id, + client_secret: client.secret, + redirect_uris: [ client.redirectUri ], + }; + } else { + if (!this.store[id]) return false; + + debug(`[${this.name}] find id:${id}`, this.store[id]); + + return this.store[id].payload; + } } /** @@ -130,11 +198,15 @@ class CloudronAdapter { async findByUid(uid) { debug(`[${this.name}] findByUid uid:${uid}`); - for (let d in this.store) { - if (this.store[d].payload.uid === uid) return this.store[d].payload; - } + if (this.name === 'Client') { + console.log('WARNING!! this should not happen as it is stored in our db'); + } else { + for (let d in this.store) { + if (this.store[d].payload.uid === uid) return this.store[d].payload; + } - return false; + return false; + } } /** @@ -151,9 +223,13 @@ class CloudronAdapter { async consume(id) { debug(`[${this.name}] consume id:${id}`); - if (this.store[id]) this.store[id].consumed = true; + if (this.name === 'Client') { + console.log('WARNING!! this should not happen as it is stored in our db'); + } else { + if (this.store[id]) this.store[id].consumed = true; - if (this.fileStorePath) fs.writeFileSync(this.fileStorePath, JSON.stringify(this.store), 'utf8'); + if (this.fileStorePath) fs.writeFileSync(this.fileStorePath, JSON.stringify(this.store), 'utf8'); + } } /** @@ -169,9 +245,13 @@ class CloudronAdapter { async destroy(id) { debug(`[${this.name}] destroy id:${id}`); - delete this.store[id]; + if (this.name === 'Client') { + console.log('WARNING!! this should not happen as it is stored in our db'); + } else { + delete this.store[id]; - if (this.fileStorePath) fs.writeFileSync(this.fileStorePath, JSON.stringify(this.store), 'utf8'); + if (this.fileStorePath) fs.writeFileSync(this.fileStorePath, JSON.stringify(this.store), 'utf8'); + } } /** @@ -187,32 +267,19 @@ class CloudronAdapter { async revokeByGrantId(grantId) { debug(`[${this.name}] revokeByGrantId grantId:${grantId}`); - for (let d in this.store) { - if (this.store[d].grantId === grantId) { - delete this.store[d]; - return; + if (this.name === 'Client') { + console.log('WARNING!! this should not happen as it is stored in our db'); + } else { + for (let d in this.store) { + if (this.store[d].grantId === grantId) { + delete this.store[d]; + return; + } } } } } -async function upsertClient(provider, clientId, clientSecret, redirectUri) { - assert.strictEqual(typeof provider, 'object'); - assert.strictEqual(typeof clientId, 'string'); - assert.strictEqual(typeof clientSecret, 'string'); - assert.strictEqual(typeof redirectUri, 'string'); - - debug(`upsertClient clientId:${clientId} clientSecret:${clientSecret} redirectUri:${redirectUri}`); - - const payload = { - client_id: clientId, - client_secret: clientSecret, - redirect_uris: [ redirectUri ], - } - - gClients[clientId] = { id: clientId, expiresIn: 0, payload, consumed: false }; -} - function renderInteractionPage(routePrefix, provider) { assert.strictEqual(typeof routePrefix, 'string'); assert.strictEqual(typeof provider, 'object'); diff --git a/src/server.js b/src/server.js index abedff0a1..cdd84bd0b 100644 --- a/src/server.js +++ b/src/server.js @@ -388,10 +388,6 @@ async function initializeExpressSync() { app.use(oidcPrefix, oidcProvider.callback()); - // FIXME this should come from the database - await oidc.upsertClient(oidcProvider, 'foo', 'bar', 'https://openidconnect.net/callback'); - await oidc.upsertClient(oidcProvider, 'oidcdebugger', 'bar', 'https://oidcdebugger.com/debug'); - // disable server socket "idle" timeout. we use the timeout middleware to handle timeouts on a route level // we rely on nginx for timeouts on the TCP level (see client_header_timeout) httpServer.setTimeout(0); @@ -438,6 +434,8 @@ async function start() { await util.promisify(gHttpServer.listen.bind(gHttpServer))(constants.PORT, '127.0.0.1'); await safe(eventlog.add(eventlog.ACTION_START, { userId: null, username: 'boot' }, { version: constants.VERSION })); // can fail if db down + + try { await oidc.clients.add('foo', 'bar', 'https://openidconnect.net/callback'); } catch (e) { console.log('Client already exists', e); } } async function stop() {