From 200018a02294fcddedc7ee4eacf2ac9d57d6e0f8 Mon Sep 17 00:00:00 2001 From: Girish Ramakrishnan Date: Wed, 18 Aug 2021 15:40:28 -0700 Subject: [PATCH] settings: async'ify * directory config * unstable app config --- src/appstore.js | 29 ++++++++++---------- src/routes/profile.js | 11 ++++---- src/routes/settings.js | 37 ++++++++++++------------- src/settings.js | 58 ++++++++++++--------------------------- src/test/settings-test.js | 22 +++++---------- src/users.js | 7 ++--- 6 files changed, 64 insertions(+), 100 deletions(-) diff --git a/src/appstore.js b/src/appstore.js index 578fe8a5f..9e12980a3 100644 --- a/src/appstore.js +++ b/src/appstore.js @@ -464,27 +464,26 @@ function createTicket(info, auditSource, callback) { function getApps(callback) { assert.strictEqual(typeof callback, 'function'); - getCloudronToken(function (error, token) { + getCloudronToken(async function (error, token) { if (error) return callback(error); - settings.getUnstableAppsConfig(function (error, unstable) { - if (error) return callback(error); + const [settingsError, unstable] = await settings.getUnstableAppsConfig(); + if (settingsError) return callback(settingsError); - const url = `${settings.apiServerOrigin()}/api/v1/apps`; - superagent.get(url).query({ accessToken: token, boxVersion: constants.VERSION, unstable: unstable }).timeout(30 * 1000).end(function (error, result) { - if (error && !error.response) return callback(new BoxError(BoxError.NETWORK_ERROR, error.message)); - if (result.statusCode === 403 || result.statusCode === 401) return callback(new BoxError(BoxError.INVALID_CREDENTIALS)); - if (result.statusCode === 422) return callback(new BoxError(BoxError.LICENSE_ERROR, result.body.message)); - if (result.statusCode !== 200) return callback(new BoxError(BoxError.EXTERNAL_ERROR, util.format('App listing failed. %s %j', result.status, result.body))); - if (!result.body.apps) return callback(new BoxError(BoxError.EXTERNAL_ERROR, util.format('Bad response: %s %s', result.statusCode, result.text))); + const url = `${settings.apiServerOrigin()}/api/v1/apps`; + superagent.get(url).query({ accessToken: token, boxVersion: constants.VERSION, unstable: unstable }).timeout(30 * 1000).end(function (error, result) { + if (error && !error.response) return callback(new BoxError(BoxError.NETWORK_ERROR, error.message)); + if (result.statusCode === 403 || result.statusCode === 401) return callback(new BoxError(BoxError.INVALID_CREDENTIALS)); + if (result.statusCode === 422) return callback(new BoxError(BoxError.LICENSE_ERROR, result.body.message)); + if (result.statusCode !== 200) return callback(new BoxError(BoxError.EXTERNAL_ERROR, util.format('App listing failed. %s %j', result.status, result.body))); + if (!result.body.apps) return callback(new BoxError(BoxError.EXTERNAL_ERROR, util.format('Bad response: %s %s', result.statusCode, result.text))); - settings.getAppstoreListingConfig(function (error, listingConfig) { - if (error) return callback(error); + settings.getAppstoreListingConfig(function (error, listingConfig) { + if (error) return callback(error); - const filteredApps = result.body.apps.filter(app => isAppAllowed(app.id, listingConfig)); + const filteredApps = result.body.apps.filter(app => isAppAllowed(app.id, listingConfig)); - callback(null, filteredApps); - }); + callback(null, filteredApps); }); }); }); diff --git a/src/routes/profile.js b/src/routes/profile.js index 96b0c1171..0962fb63c 100644 --- a/src/routes/profile.js +++ b/src/routes/profile.js @@ -23,16 +23,15 @@ const assert = require('assert'), users = require('../users.js'), _ = require('underscore'); -function authorize(req, res, next) { +async function authorize(req, res, next) { assert.strictEqual(typeof req.user, 'object'); - settings.getDirectoryConfig(function (error, directoryConfig) { - if (error) return next(BoxError.toHttpError(error)); + const [error, directoryConfig] = await settings.getDirectoryConfig(); + if (error) return next(BoxError.toHttpError(error)); - if (directoryConfig.lockUserProfiles) return next(new HttpError(403, 'admin has disallowed users from editing profiles')); + if (directoryConfig.lockUserProfiles) return next(new HttpError(403, 'admin has disallowed users from editing profiles')); - next(); - }); + next(); } async function get(req, res, next) { diff --git a/src/routes/settings.js b/src/routes/settings.js index 833ac8939..4360c9890 100644 --- a/src/routes/settings.js +++ b/src/routes/settings.js @@ -15,6 +15,7 @@ const assert = require('assert'), externalLdap = require('../externalldap.js'), HttpError = require('connect-lastmile').HttpError, HttpSuccess = require('connect-lastmile').HttpSuccess, + safe = require('safetydance'), settings = require('../settings.js'); function getAutoupdatePattern(req, res, next) { @@ -165,24 +166,22 @@ function setDynamicDnsConfig(req, res, next) { }); } -function getUnstableAppsConfig(req, res, next) { - settings.getUnstableAppsConfig(function (error, enabled) { - if (error) return next(BoxError.toHttpError(error)); +async function getUnstableAppsConfig(req, res, next) { + const [error, enabled] = await safe(settings.getUnstableAppsConfig()); + if (error) return next(BoxError.toHttpError(error)); - next(new HttpSuccess(200, { enabled: enabled })); - }); + next(new HttpSuccess(200, { enabled })); } -function setUnstableAppsConfig(req, res, next) { +async function setUnstableAppsConfig(req, res, next) { assert.strictEqual(typeof req.body, 'object'); if (typeof req.body.enabled !== 'boolean') return next(new HttpError(400, 'enabled boolean is required')); - settings.setUnstableAppsConfig(req.body.enabled, function (error) { - if (error) return next(BoxError.toHttpError(error)); + const [error] = await safe(settings.setUnstableAppsConfig(req.body.enabled)); + if (error) return next(BoxError.toHttpError(error)); - next(new HttpSuccess(200, {})); - }); + next(new HttpSuccess(200, {})); } function getRegistryConfig(req, res, next) { @@ -211,25 +210,23 @@ function setRegistryConfig(req, res, next) { }); } -function getDirectoryConfig(req, res, next) { - settings.getDirectoryConfig(function (error, directoryConfig) { - if (error) return next(BoxError.toHttpError(error)); +async function getDirectoryConfig(req, res, next) { + const [error, directoryConfig] = await settings.getDirectoryConfig(); + if (error) return next(BoxError.toHttpError(error)); - next(new HttpSuccess(200, directoryConfig)); - }); + next(new HttpSuccess(200, directoryConfig)); } -function setDirectoryConfig(req, res, next) { +async function setDirectoryConfig(req, res, next) { assert.strictEqual(typeof req.body, 'object'); if (typeof req.body.lockUserProfiles !== 'boolean') return next(new HttpError(400, 'lockUserProfiles is required')); if (typeof req.body.mandatory2FA !== 'boolean') return next(new HttpError(400, 'mandatory2FA is required')); - settings.setDirectoryConfig(req.body, function (error) { - if (error) return next(BoxError.toHttpError(error)); + const [error] = await safe(settings.setDirectoryConfig(req.body)); + if (error) return next(BoxError.toHttpError(error)); - next(new HttpSuccess(200, {})); - }); + next(new HttpSuccess(200, {})); } function getSysinfoConfig(req, res, next) { diff --git a/src/settings.js b/src/settings.js index e0d081038..4751d475c 100644 --- a/src/settings.js +++ b/src/settings.js @@ -118,7 +118,8 @@ exports = module.exports = { CLOUDRON_AVATAR_KEY: 'cloudron_avatar', // testing - _setApiServerOrigin: setApiServerOrigin + _setApiServerOrigin: setApiServerOrigin, + _clear: clear }; const assert = require('assert'), @@ -222,7 +223,7 @@ async function get(key) { assert.strictEqual(typeof key, 'string'); const result = await database.query(`SELECT ${SETTINGS_FIELDS} FROM settings WHERE name = ?`, [ key ]); - if (result.length === 0) return gDefaults[key]; + if (result.length === 0) return null; // can't return the default value here because we might need to massage/json parse the result return result[0].value; } @@ -381,29 +382,17 @@ function setDynamicDnsConfig(enabled, callback) { }); } -function getUnstableAppsConfig(callback) { - assert.strictEqual(typeof callback, 'function'); - - settingsdb.get(exports.UNSTABLE_APPS_KEY, function (error, enabled) { - if (error && error.reason === BoxError.NOT_FOUND) return callback(null, gDefaults[exports.UNSTABLE_APPS_KEY]); - if (error) return callback(error); - - callback(null, !!enabled); // settingsdb holds string values only - }); +async function getUnstableAppsConfig() { + const result = await get(exports.UNSTABLE_APPS_KEY); + if (result === null) gDefaults[exports.UNSTABLE_APPS_KEY]; + return !!result; // db holds string values only } -function setUnstableAppsConfig(enabled, callback) { +async function setUnstableAppsConfig(enabled) { assert.strictEqual(typeof enabled, 'boolean'); - assert.strictEqual(typeof callback, 'function'); - // settingsdb takes string values only - settingsdb.set(exports.UNSTABLE_APPS_KEY, enabled ? 'enabled' : '', function (error) { - if (error) return callback(error); - - notifyChange(exports.UNSTABLE_APPS_KEY, enabled); - - return callback(null); - }); + await set(exports.UNSTABLE_APPS_KEY, enabled ? 'enabled' : ''); // db holds string values only + notifyChange(exports.UNSTABLE_APPS_KEY, enabled); } function getBackupConfig(callback) { @@ -646,30 +635,19 @@ function setSysinfoConfig(sysinfoConfig, callback) { }); } -function getDirectoryConfig(callback) { - assert.strictEqual(typeof callback, 'function'); - - settingsdb.get(exports.DIRECTORY_CONFIG_KEY, function (error, value) { - if (error && error.reason === BoxError.NOT_FOUND) return callback(null, gDefaults[exports.DIRECTORY_CONFIG_KEY]); - if (error) return callback(error); - - callback(null, JSON.parse(value)); - }); +async function getDirectoryConfig() { + const value = await get(exports.DIRECTORY_CONFIG_KEY); + if (value === null) return gDefaults[exports.DIRECTORY_CONFIG_KEY]; + return JSON.parse(value); } -function setDirectoryConfig(directoryConfig, callback) { +async function setDirectoryConfig(directoryConfig) { assert.strictEqual(typeof directoryConfig, 'object'); - assert.strictEqual(typeof callback, 'function'); - if (isDemo()) return callback(new BoxError(BoxError.BAD_FIELD, 'Not allowed in demo mode')); + if (isDemo()) throw new BoxError(BoxError.BAD_FIELD, 'Not allowed in demo mode'); - settingsdb.set(exports.DIRECTORY_CONFIG_KEY, JSON.stringify(directoryConfig), function (error) { - if (error) return callback(error); - - notifyChange(exports.DIRECTORY_CONFIG_KEY, directoryConfig); - - callback(null); - }); + await set(exports.DIRECTORY_CONFIG_KEY, JSON.stringify(directoryConfig)); + notifyChange(exports.DIRECTORY_CONFIG_KEY, directoryConfig); } function getAppstoreListingConfig(callback) { diff --git a/src/test/settings-test.js b/src/test/settings-test.js index 94f2897e6..a477d4ea2 100644 --- a/src/test/settings-test.js +++ b/src/test/settings-test.js @@ -84,24 +84,16 @@ describe('Settings', function () { }); }); - it('can get default unstable apps setting', function (done) { - settings.getUnstableAppsConfig(function (error, enabled) { - expect(error).to.be(null); - expect(enabled).to.be(true); - done(); - }); + it('can get default unstable apps setting', async function () { + const enabled = await settings.getUnstableAppsConfig(); + expect(enabled).to.be(true); }); - it('can set unstable apps setting', function (done) { - settings.setUnstableAppsConfig(false, function (error) { - expect(error).to.be(null); + it('can set unstable apps setting', async function () { + await settings.setUnstableAppsConfig(false); - settings.getUnstableAppsConfig(function (error, enabled) { - expect(error).to.be(null); - expect(enabled).to.be(false); - done(); - }); - }); + const enabled = await settings.getUnstableAppsConfig(); + expect(enabled).to.be(false); }); it('can get all values', async function () { diff --git a/src/users.js b/src/users.js index 0310bb0f2..573897801 100644 --- a/src/users.js +++ b/src/users.js @@ -92,7 +92,6 @@ const CRYPTO_DIGEST = 'sha1'; // used to be the default in node 4.1.1 cannot cha const pbkdf2Async = util.promisify(crypto.pbkdf2); const randomBytesAsync = util.promisify(crypto.randomBytes); -const getDirectoryConfigAsync = util.promisify(settings.getDirectoryConfig); function postProcess(result) { assert.strictEqual(typeof result, 'object'); @@ -660,7 +659,7 @@ async function createInvite(user, auditSource) { const resetToken = hat(256), resetTokenCreationTime = new Date(); - const directoryConfig = await getDirectoryConfigAsync(); + const directoryConfig = await settings.getDirectoryConfig(); await update(user, { resetToken, resetTokenCreationTime }, auditSource); @@ -676,7 +675,7 @@ async function sendInvite(user, options) { if (user.source) throw new BoxError(BoxError.CONFLICT, 'User is from an external directory'); if (!user.resetToken) throw new BoxError(BoxError.CONFLICT, 'Must generate resetToken to send invitation'); - const directoryConfig = await getDirectoryConfigAsync(); + const directoryConfig = await settings.getDirectoryConfig(); mailer.sendInvite(user, options.invitor || null, inviteLink(user, directoryConfig)); } @@ -686,7 +685,7 @@ async function setupAccount(user, data, auditSource) { assert.strictEqual(typeof data, 'object'); assert(auditSource && typeof auditSource === 'object'); - const directoryConfig = await getDirectoryConfigAsync(); + const directoryConfig = await settings.getDirectoryConfig(); if (directoryConfig.lockUserProfiles) return; await update(user, _.pick(data, 'username', 'displayName'), auditSource);