diff --git a/dashboard/src/js/client.js b/dashboard/src/js/client.js index b2dc4d568..1cc0d8b26 100644 --- a/dashboard/src/js/client.js +++ b/dashboard/src/js/client.js @@ -1948,12 +1948,13 @@ angular.module('Application').service('Client', ['$http', '$interval', '$timeout }); }; - Client.prototype.addOidcClient = function (id, name, secret, loginRedirectUri, logoutRedirectUri, callback) { + Client.prototype.addOidcClient = function (id, name, secret, loginRedirectUri, logoutRedirectUri, tokenSignatureAlgorithm, callback) { var data = { id: id, name: name, secret: secret, - loginRedirectUri: loginRedirectUri + loginRedirectUri: loginRedirectUri, + tokenSignatureAlgorithm: tokenSignatureAlgorithm }; if (logoutRedirectUri) data.logoutRedirectUri = logoutRedirectUri; @@ -1966,11 +1967,12 @@ angular.module('Application').service('Client', ['$http', '$interval', '$timeout }); }; - Client.prototype.updateOidcClient = function (id, name, secret, loginRedirectUri, logoutRedirectUri, callback) { + Client.prototype.updateOidcClient = function (id, name, secret, loginRedirectUri, logoutRedirectUri, tokenSignatureAlgorithm, callback) { var data = { secret: secret, name: name, - loginRedirectUri: loginRedirectUri + loginRedirectUri: loginRedirectUri, + tokenSignatureAlgorithm, tokenSignatureAlgorithm }; if (logoutRedirectUri) data.logoutRedirectUri = logoutRedirectUri; diff --git a/dashboard/src/views/oidc.html b/dashboard/src/views/oidc.html index b1756cf0f..68f5d6bd2 100644 --- a/dashboard/src/views/oidc.html +++ b/dashboard/src/views/oidc.html @@ -34,6 +34,15 @@ +
+ +
+ +
+
@@ -72,6 +81,15 @@ +
+ +
+ +
+
@@ -159,7 +177,7 @@ Name Client ID - Client Secret + Signing Algorithm {{ 'main.actions' | tr }} @@ -175,7 +193,7 @@ {{ client.id }} - {{ client.secret }} + {{ client.tokenSignatureAlgorithm }} diff --git a/dashboard/src/views/oidc.js b/dashboard/src/views/oidc.js index 35d8c0d27..69d6087e0 100644 --- a/dashboard/src/views/oidc.js +++ b/dashboard/src/views/oidc.js @@ -26,6 +26,7 @@ angular.module('Application').controller('OidcController', ['$scope', '$location secret: '', loginRedirectUri: '', logoutRedirectUri: '', + tokenSignatureAlgorithm: '', show: function () { $scope.clientAdd.id = ''; @@ -33,6 +34,7 @@ angular.module('Application').controller('OidcController', ['$scope', '$location $scope.clientAdd.name = ''; $scope.clientAdd.loginRedirectUri = ''; $scope.clientAdd.logoutRedirectUri = ''; + $scope.clientAdd.tokenSignatureAlgorithm = 'RS256'; $scope.clientAdd.busy = false; $scope.clientAdd.error = null; $scope.clientAddForm.$setPristine(); @@ -44,7 +46,7 @@ angular.module('Application').controller('OidcController', ['$scope', '$location $scope.clientAdd.busy = true; $scope.clientAdd.error = {}; - Client.addOidcClient($scope.clientAdd.id, $scope.clientAdd.name, $scope.clientAdd.secret, $scope.clientAdd.loginRedirectUri, $scope.clientAdd.logoutRedirectUri, function (error) { + Client.addOidcClient($scope.clientAdd.id, $scope.clientAdd.name, $scope.clientAdd.secret, $scope.clientAdd.loginRedirectUri, $scope.clientAdd.logoutRedirectUri, $scope.clientAdd.tokenSignatureAlgorithm, function (error) { if (error) { if (error.statusCode === 409) { $scope.clientAdd.error.id = 'Client ID already exists'; @@ -74,6 +76,7 @@ angular.module('Application').controller('OidcController', ['$scope', '$location secret: '', loginRedirectUri: '', logoutRedirectUri: '', + tokenSignatureAlgorithm: '', show: function (client) { $scope.clientEdit.id = client.id; @@ -81,6 +84,7 @@ angular.module('Application').controller('OidcController', ['$scope', '$location $scope.clientEdit.secret = client.secret; $scope.clientEdit.loginRedirectUri = client.loginRedirectUri; $scope.clientEdit.logoutRedirectUri = client.logoutRedirectUri; + $scope.clientEdit.tokenSignatureAlgorithm = client.tokenSignatureAlgorithm; $scope.clientEdit.busy = false; $scope.clientEdit.error = null; $scope.clientEditForm.$setPristine(); @@ -92,7 +96,7 @@ angular.module('Application').controller('OidcController', ['$scope', '$location $scope.clientEdit.busy = true; $scope.clientEdit.error = {}; - Client.updateOidcClient($scope.clientEdit.id, $scope.clientEdit.name, $scope.clientEdit.secret, $scope.clientEdit.loginRedirectUri, $scope.clientEdit.logoutRedirectUri, function (error) { + Client.updateOidcClient($scope.clientEdit.id, $scope.clientEdit.name, $scope.clientEdit.secret, $scope.clientEdit.loginRedirectUri, $scope.clientEdit.logoutRedirectUri, $scope.clientEdit.tokenSignatureAlgorithm, function (error) { if (error) { console.error('Unable to edit openid client.', error); diff --git a/migrations/20230318141011-apps-add-enableTurn.js b/migrations/20230318141011-apps-add-enableTurn.js index 7b2b10df0..15ec2ea2c 100644 --- a/migrations/20230318141011-apps-add-enableTurn.js +++ b/migrations/20230318141011-apps-add-enableTurn.js @@ -4,7 +4,7 @@ exports.up = async function(db) { await db.runSql('ALTER TABLE apps ADD COLUMN enableTurn BOOLEAN DEFAULT 1'); }; -exports.down = async function(db, callback) { +exports.down = async function(db) { await db.runSql('ALTER TABLE apps DROP COLUMN enableTurn'); }; diff --git a/migrations/20230329084726-apps-add-enableRedis.js b/migrations/20230329084726-apps-add-enableRedis.js index 2ad72f815..72d087573 100644 --- a/migrations/20230329084726-apps-add-enableRedis.js +++ b/migrations/20230329084726-apps-add-enableRedis.js @@ -4,7 +4,7 @@ exports.up = async function(db) { await db.runSql('ALTER TABLE apps ADD COLUMN enableRedis BOOLEAN DEFAULT 1'); }; -exports.down = async function(db, callback) { +exports.down = async function(db) { await db.runSql('ALTER TABLE apps DROP COLUMN enableRedis'); }; diff --git a/migrations/20230404140451-oidc-clients-add-tokenSignatureAlgorithm.js b/migrations/20230404140451-oidc-clients-add-tokenSignatureAlgorithm.js new file mode 100644 index 000000000..0091a76dd --- /dev/null +++ b/migrations/20230404140451-oidc-clients-add-tokenSignatureAlgorithm.js @@ -0,0 +1,9 @@ +'use strict'; + +exports.up = async function(db) { + await db.runSql('ALTER TABLE oidcClients ADD COLUMN tokenSignatureAlgorithm VARCHAR(128) DEFAULT ""'); +}; + +exports.down = async function(db) { + await db.runSql('ALTER TABLE oidcClients DROP COLUMN tokenSignatureAlgorithm'); +}; diff --git a/migrations/schema.sql b/migrations/schema.sql index e8afb8cc1..f16a9762c 100644 --- a/migrations/schema.sql +++ b/migrations/schema.sql @@ -320,4 +320,5 @@ CREATE TABLE IF NOT EXISTS oidcClients( name VARCHAR(128) DEFAULT "", loginRedirectUri VARCHAR(256) DEFAULT "", logoutRedirectUri VARCHAR(256) DEFAULT "", + tokenSignatureAlgorithm VARCHAR(128) DEFAULT "", PRIMARY KEY(id)); diff --git a/src/oidc.js b/src/oidc.js index 4f9216a6f..61f396dc3 100644 --- a/src/oidc.js +++ b/src/oidc.js @@ -34,15 +34,24 @@ const assert = require('assert'), util = require('util'); const OIDC_CLIENTS_TABLE_NAME = 'oidcClients'; -const OIDC_CLIENTS_FIELDS = [ 'id', 'secret', 'name', 'appId', 'loginRedirectUri', 'logoutRedirectUri' ]; +const OIDC_CLIENTS_FIELDS = [ 'id', 'secret', 'name', 'appId', 'loginRedirectUri', 'logoutRedirectUri', 'tokenSignatureAlgorithm' ]; const ROUTE_PREFIX = '/openid'; +const DEFAULT_TOKEN_SIGNATURE_ALGORITHM='RS256'; let gHttpServer = null; // ----------------------------- // Database model // ----------------------------- +function postProcess(result) { + assert.strictEqual(typeof result, 'object'); + + result.tokenSignatureAlgorithm = result.tokenSignatureAlgorithm || DEFAULT_TOKEN_SIGNATURE_ALGORITHM; + + return result; +} + async function clientsAdd(id, data) { assert.strictEqual(typeof id, 'string'); assert.strictEqual(typeof data.secret, 'string'); @@ -50,11 +59,12 @@ async function clientsAdd(id, data) { assert.strictEqual(typeof data.logoutRedirectUri, 'string'); assert.strictEqual(typeof data.name, 'string'); assert.strictEqual(typeof data.appId, 'string'); + assert(data.tokenSignatureAlgorithm === 'RS256' || data.tokenSignatureAlgorithm === 'EdDSA'); - debug(`clientsAdd: id:${id} secret:${data.secret} name:${data.name} appId:${data.appId} loginRedirectUri:${data.loginRedirectUri} logoutRedirectUri:${data.logoutRedirectUri}`); + debug(`clientsAdd: id:${id} secret:${data.secret} name:${data.name} appId:${data.appId} loginRedirectUri:${data.loginRedirectUri} logoutRedirectUri:${data.logoutRedirectUri} tokenSignatureAlgorithm:${data.tokenSignatureAlgorithm}`); - const query = `INSERT INTO ${OIDC_CLIENTS_TABLE_NAME} (id, secret, name, appId, loginRedirectUri, logoutRedirectUri) VALUES (?, ?, ?, ?, ?, ?)`; - const args = [ id, data.secret, data.name, data.appId, data.loginRedirectUri, data.logoutRedirectUri ]; + const query = `INSERT INTO ${OIDC_CLIENTS_TABLE_NAME} (id, secret, name, appId, loginRedirectUri, logoutRedirectUri, tokenSignatureAlgorithm) VALUES (?, ?, ?, ?, ?, ?, ?)`; + const args = [ id, data.secret, data.name, data.appId, data.loginRedirectUri, data.logoutRedirectUri, data.tokenSignatureAlgorithm ]; const [error] = await safe(database.query(query, args)); if (error && error.code === 'ER_DUP_ENTRY') throw new BoxError(BoxError.ALREADY_EXISTS, 'client already exists'); @@ -69,7 +79,7 @@ async function clientsGet(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]; + return postProcess(result[0]); } async function clientsUpdate(id, data) { @@ -79,10 +89,11 @@ async function clientsUpdate(id, data) { assert.strictEqual(typeof data.logoutRedirectUri, 'string'); assert.strictEqual(typeof data.name, 'string'); assert.strictEqual(typeof data.appId, 'string'); + assert(data.tokenSignatureAlgorithm === 'RS256' || data.tokenSignatureAlgorithm === 'EdDSA'); - debug(`clientsUpdate: id:${id} secret:${data.secret} name:${data.name} appId:${data.appId} loginRedirectUri:${data.loginRedirectUri} logoutRedirectUri:${data.logoutRedirectUri}`); + debug(`clientsUpdate: id:${id} secret:${data.secret} name:${data.name} appId:${data.appId} loginRedirectUri:${data.loginRedirectUri} logoutRedirectUri:${data.logoutRedirectUri} tokenSignatureAlgorithm:${data.tokenSignatureAlgorithm}`); - const result = await database.query(`UPDATE ${OIDC_CLIENTS_TABLE_NAME} SET secret=?, name=?, appId=?, loginRedirectUri=?, logoutRedirectUri=? WHERE id = ?`, [ data.secret, data.name, data.appId, data.loginRedirectUri, data.logoutRedirectUri, id]); + const result = await database.query(`UPDATE ${OIDC_CLIENTS_TABLE_NAME} SET secret=?, name=?, appId=?, loginRedirectUri=?, logoutRedirectUri=?, tokenSignatureAlgorithm=? WHERE id = ?`, [ data.secret, data.name, data.appId, data.loginRedirectUri, data.logoutRedirectUri, data.tokenSignatureAlgorithm, id]); if (result.affectedRows !== 1) throw new BoxError(BoxError.NOT_FOUND, 'client not found'); } @@ -95,6 +106,9 @@ async function clientsDel(id) { async function clientsList() { const results = await database.query(`SELECT * FROM ${OIDC_CLIENTS_TABLE_NAME} ORDER BY id ASC`, []); + + results.forEach(postProcess); + return results; } @@ -236,7 +250,7 @@ class CloudronAdapter { client_id: id, client_secret: client.secret, redirect_uris: [ client.loginRedirectUri ], - id_token_signed_response_alg: 'EdDSA' + id_token_signed_response_alg: client.tokenSignatureAlgorithm || 'RS256' }; if (client.logoutRedirectUri) tmp.post_logout_redirect_uris = [ client.logoutRedirectUri ]; @@ -617,13 +631,13 @@ async function start() { let keyRs256 = await blobs.getString(blobs.OIDC_KEY_RS256); if (!keyRs256) { - debug('Generating new OIDC EdDSA key'); + debug('Generating new OIDC RS256 key'); const { privateKey } = await jose.generateKeyPair('RS256'); keyRs256 = await jose.exportJWK(privateKey); await blobs.setString(blobs.OIDC_KEY_RS256, JSON.stringify(keyRs256)); jwksKeys.push(keyRs256); } else { - debug('Using existing OIDC EdDSA key'); + debug('Using existing OIDC RS256 key'); jwksKeys.push(JSON.parse(keyRs256)); } diff --git a/src/routes/oidc.js b/src/routes/oidc.js index 870f4548a..51556ae48 100644 --- a/src/routes/oidc.js +++ b/src/routes/oidc.js @@ -26,12 +26,14 @@ async function add(req, res, next) { if (typeof req.body.name !== 'string' || !req.body.name) return next(new HttpError(400, 'name must be non-empty string')); if (typeof req.body.secret !== 'string' || !req.body.secret) return next(new HttpError(400, 'secret must be non-empty string')); if (typeof req.body.loginRedirectUri !== 'string' || !req.body.loginRedirectUri) return next(new HttpError(400, 'loginRedirectUri must be non-empty string')); + if (req.body.tokenSignatureAlgorithm !== 'EdDSA' && req.body.tokenSignatureAlgorithm !== 'RS256') return next(new HttpError(400, 'tokenSignatureAlgorithm must be either EdDSA or RS256')); if ('logoutRedirectUri' in req.body && (typeof req.body.logoutRedirectUri !== 'string' || !req.body.logoutRedirectUri)) return next(new HttpError(400, 'logoutRedirectUri must be non-empty string if provided')); const data = { secret: req.body.secret, name: req.body.name, appId: '', // always empty for custom clients + tokenSignatureAlgorithm: req.body.tokenSignatureAlgorithm, loginRedirectUri: req.body.loginRedirectUri, logoutRedirectUri: req.body.logoutRedirectUri || '' }; @@ -59,12 +61,14 @@ async function update(req, res, next) { if (typeof req.body.name !== 'string' || !req.body.name) return next(new HttpError(400, 'name must be non-empty string')); if (typeof req.body.secret !== 'string' || !req.body.secret) return next(new HttpError(400, 'secret must be non-empty string')); if (typeof req.body.loginRedirectUri !== 'string' || !req.body.loginRedirectUri) return next(new HttpError(400, 'loginRedirectUri must be non-empty string')); + if (req.body.tokenSignatureAlgorithm !== 'EdDSA' && req.body.tokenSignatureAlgorithm !== 'RS256') return next(new HttpError(400, 'tokenSignatureAlgorithm must be either EdDSA or RS256')); if ('logoutRedirectUri' in req.body && (typeof req.body.logoutRedirectUri !== 'string' || !req.body.logoutRedirectUri)) return next(new HttpError(400, 'logoutRedirectUri must be non-empty string if provided')); const data = { secret: req.body.secret, name: req.body.name, appId: '', // always empty for custom clients + tokenSignatureAlgorithm: req.body.tokenSignatureAlgorithm, loginRedirectUri: req.body.loginRedirectUri, logoutRedirectUri: req.body.logoutRedirectUri || '' }; diff --git a/src/routes/test/oidc-test.js b/src/routes/test/oidc-test.js index 6de89743e..bdb80ad97 100644 --- a/src/routes/test/oidc-test.js +++ b/src/routes/test/oidc-test.js @@ -14,6 +14,7 @@ const CLIENT_0 = { id: 'client0', name: 'test client 0', secret: 'secret0', + tokenSignatureAlgorithm: 'RS256', loginRedirectUri: 'http://foo.bar' }; @@ -21,6 +22,7 @@ const CLIENT_1 = { id: 'client1', name: 'test client 1', secret: 'secret1', + tokenSignatureAlgorithm: 'EdDSA', loginRedirectUri: 'https://cloudron.io/login', logoutRedirectUri: 'https://cloudron.io/logout' }; @@ -89,6 +91,7 @@ describe('OpenID connect clients API', function () { expect(response.body.secret).to.equal(CLIENT_1.secret); expect(response.body.loginRedirectUri).to.equal(CLIENT_1.loginRedirectUri); expect(response.body.logoutRedirectUri).to.equal(CLIENT_1.logoutRedirectUri); + expect(response.body.tokenSignatureAlgorithm).to.equal(CLIENT_1.tokenSignatureAlgorithm); }); it('cannot update non-existent client', async function () { @@ -147,7 +150,7 @@ describe('OpenID connect clients API', function () { it('can update client without logoutRedirectUri', async function () { const response = await superagent.post(`${serverUrl}/api/v1/oidc/clients/${CLIENT_0.id}`) .query({ access_token: owner.token }) - .send({ secret: 'newsecret', name: 'new name', loginRedirectUri: CLIENT_0.loginRedirectUri }) + .send({ secret: 'newsecret', name: 'new name', loginRedirectUri: CLIENT_0.loginRedirectUri, tokenSignatureAlgorithm: CLIENT_0.tokenSignatureAlgorithm }) .ok(() => true); expect(response.statusCode).to.equal(201); @@ -163,7 +166,7 @@ describe('OpenID connect clients API', function () { it('can update client with logoutRedirectUri', async function () { const response = await superagent.post(`${serverUrl}/api/v1/oidc/clients/${CLIENT_0.id}`) .query({ access_token: owner.token }) - .send({ secret: 'newsecret', name: CLIENT_1.name, loginRedirectUri: CLIENT_0.loginRedirectUri, logoutRedirectUri: CLIENT_1.logoutRedirectUri }) + .send({ secret: 'newsecret', name: CLIENT_1.name, loginRedirectUri: CLIENT_0.loginRedirectUri, logoutRedirectUri: CLIENT_1.logoutRedirectUri, tokenSignatureAlgorithm: CLIENT_1.tokenSignatureAlgorithm }) .ok(() => true); expect(response.statusCode).to.equal(201); @@ -175,6 +178,7 @@ describe('OpenID connect clients API', function () { expect(response2.body.secret).to.equal('newsecret'); expect(response2.body.loginRedirectUri).to.equal(CLIENT_0.loginRedirectUri); expect(response2.body.logoutRedirectUri).to.equal(CLIENT_1.logoutRedirectUri); + expect(response2.body.tokenSignatureAlgorithm).to.equal(CLIENT_1.tokenSignatureAlgorithm); }); it('cannot remove without token', async function () {