From b2f1d0559bcec61e1f06edabc09ded12d4403a97 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Mon, 13 Mar 2017 11:44:44 +0000 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Themes=20API=20activation=20permiss?= =?UTF-8?q?ions=20&=20validation=20(#8104)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs #8093 ✨ Add activate theme permission - add permission to activate themes - update tests - also: update tests for invites TODO: change how the active theme setting is updated to reduce extra permissions ✨ Move theme validation to gscan - add a new gscan validation method and use it for upload - update activate endpoint to do validation also using gscan - change to using SettingsModel instead of API so that we don't call validation or permissions on the settings API - remove validation from the settings model - remove the old validation function - add new invalid theme message to translations & remove a bunch of theme validation related unused keys 📖 Planned changes 🚨 Tests for theme activation API endpoint 🐛 Don't allow deleting the active theme 🚫 Prevent activeTheme being set via settings API - We want to control how this happens in future. - We still want to store the information in settings, via the model. - We just don't want to be able to change this info via the settings edit endpoint 🐛 ✨ Fix warnings for uploads & add for activations - warnings for uploads were broken in f8b498d - fix the response + adds tests to cover that warnings are correctly returned - add the same response to activations + more tests - activations now return a single theme object - the theme that was activated + any warnings 🎨 Improve how we generate theme API responses - remove the requirement to pass in the active theme! - move this to a specialist function, away from the list 🎨 Do not load gscan on boot --- core/server/api/settings.js | 9 + core/server/api/themes.js | 90 +++---- .../server/data/schema/fixtures/fixtures.json | 13 +- core/server/models/settings.js | 9 - core/server/themes/index.js | 3 +- core/server/themes/list.js | 13 - core/server/themes/to-json.js | 33 +++ core/server/themes/validate.js | 37 ++- core/server/translations/en.json | 22 +- .../test/functional/routes/api/themes_spec.js | 227 ++++++++++++++++-- .../test/integration/api/api_settings_spec.js | 29 +-- core/test/integration/migration_spec.js | 74 +++--- .../test/unit/migration_fixture_utils_spec.js | 4 +- core/test/unit/migration_spec.js | 2 +- core/test/utils/fixtures/themes/warnings.zip | Bin 0 -> 638 bytes 15 files changed, 406 insertions(+), 159 deletions(-) create mode 100644 core/server/themes/to-json.js create mode 100644 core/test/utils/fixtures/themes/warnings.zip diff --git a/core/server/api/settings.js b/core/server/api/settings.js index 15cafc9beb..a30883a726 100644 --- a/core/server/api/settings.js +++ b/core/server/api/settings.js @@ -103,6 +103,15 @@ canEditAllSettings = function (settingsInfo, options) { )); } + if (setting.key === 'activeTheme') { + return Promise.reject( + new errors.BadRequestError({ + message: i18n.t('errors.api.settings.activeThemeSetViaAPI.error'), + help: i18n.t('errors.api.settings.activeThemeSetViaAPI.help') + }) + ); + } + return checkSettingPermissions(setting); }); diff --git a/core/server/api/themes.js b/core/server/api/themes.js index 4de0db2096..1695f855ee 100644 --- a/core/server/api/themes.js +++ b/core/server/api/themes.js @@ -1,9 +1,6 @@ // # Themes API // RESTful API for Themes -var debug = require('debug')('ghost:api:themes'), - Promise = require('bluebird'), - _ = require('lodash'), - gscan = require('gscan'), +var Promise = require('bluebird'), fs = require('fs-extra'), config = require('../config'), errors = require('../errors'), @@ -13,7 +10,7 @@ var debug = require('debug')('ghost:api:themes'), apiUtils = require('./utils'), utils = require('./../utils'), i18n = require('../i18n'), - settings = require('./settings'), + settingsModel = require('../models/settings').Settings, settingsCache = require('../settings/cache'), themeUtils = require('../themes'), themeList = themeUtils.list, @@ -26,10 +23,7 @@ var debug = require('debug')('ghost:api:themes'), */ themes = { browse: function browse() { - debug('browsing'); - var result = themeList.toAPI(themeList.getAll(), settingsCache.get('activeTheme')); - debug('got result'); - return Promise.resolve({themes: result}); + return Promise.resolve(themeUtils.toJSON()); }, activate: function activate(options) { @@ -37,12 +31,33 @@ themes = { newSettings = [{ key: 'activeTheme', value: themeName - }]; + }], + loadedTheme, + checkedTheme; - return settings.edit({settings: newSettings}, options).then(function () { - var result = themeList.toAPI(themeList.getAll(), themeName); - return Promise.resolve({themes: result}); - }); + return apiUtils + .handlePermissions('themes', 'activate')(options) + .then(function activateTheme() { + loadedTheme = themeList.get(themeName); + + if (!loadedTheme) { + return Promise.reject(new errors.ValidationError({ + message: i18n.t('notices.data.validation.index.themeCannotBeActivated', {themeName: themeName}), + context: 'activeTheme' + })); + } + + return themeUtils.validate.check(loadedTheme); + }) + .then(function haveValidTheme(_checkedTheme) { + checkedTheme = _checkedTheme; + // We use the model, not the API here, as we don't want to trigger permissions + return settingsModel.edit(newSettings, options); + }) + .then(function hasEditedSetting() { + // @TODO actually do things to activate the theme, other than just the setting? + return themeUtils.toJSON(themeName, checkedTheme); + }); }, upload: function upload(options) { @@ -56,7 +71,8 @@ themes = { path: options.path, name: options.originalname, shortName: storageAdapter.getSanitizedFileName(options.originalname.split('.zip')[0]) - }, theme; + }, + checkedTheme; // check if zip name is casper.zip if (zip.name === 'casper.zip') { @@ -64,23 +80,12 @@ themes = { } return apiUtils.handlePermissions('themes', 'add')(options) - .then(function () { - return gscan.checkZip(zip, {keepExtractedDir: true}); + .then(function validateTheme() { + return themeUtils.validate.check(zip, true); }) - .then(function (_theme) { - theme = _theme; - theme = gscan.format(theme); + .then(function checkExists(_checkedTheme) { + checkedTheme = _checkedTheme; - if (!theme.results.error.length) { - return; - } - - throw new errors.ThemeValidationError({ - message: i18n.t('errors.api.themes.invalidTheme'), - errorDetails: theme.results.error - }); - }) - .then(function () { return storageAdapter.exists(utils.url.urlJoin(config.getContentPath('themes'), zip.shortName)); }) .then(function (themeExists) { @@ -94,22 +99,20 @@ themes = { // store extracted theme return storageAdapter.save({ name: zip.shortName, - path: theme.path + path: checkedTheme.path }, config.getContentPath('themes')); }) .then(function () { + // Loads the theme from the filesystem + // Sets the theme on the themeList return themeUtils.loadOne(zip.shortName); }) - .then(function (themeObject) { - themeObject = themeList.toAPI(themeObject, settingsCache.get('activeTheme')); - // gscan theme structure !== ghost theme structure - if (theme.results.warning.length > 0) { - themeObject.warnings = _.cloneDeep(theme.results.warning); - } - - return {themes: themeObject}; + .then(function () { + // @TODO: unify the name across gscan and Ghost! + return themeUtils.toJSON(zip.shortName, checkedTheme); }) .finally(function () { + // @TODO we should probably do this as part of saving the theme // remove zip upload from multer // happens in background Promise.promisify(fs.removeSync)(zip.path) @@ -117,10 +120,11 @@ themes = { logging.error(new errors.GhostError({err: err})); }); + // @TODO we should probably do this as part of saving the theme // remove extracted dir from gscan // happens in background - if (theme) { - Promise.promisify(fs.removeSync)(theme.path) + if (checkedTheme) { + Promise.promisify(fs.removeSync)(checkedTheme.path) .catch(function (err) { logging.error(new errors.GhostError({err: err})); }); @@ -159,6 +163,10 @@ themes = { throw new errors.ValidationError({message: i18n.t('errors.api.themes.destroyCasper')}); } + if (name === settingsCache.get('activeTheme')) { + throw new errors.ValidationError({message: i18n.t('errors.api.themes.destroyActive')}); + } + theme = themeList.get(name); if (!theme) { diff --git a/core/server/data/schema/fixtures/fixtures.json b/core/server/data/schema/fixtures/fixtures.json index bca05404ed..952e62bf65 100644 --- a/core/server/data/schema/fixtures/fixtures.json +++ b/core/server/data/schema/fixtures/fixtures.json @@ -190,6 +190,11 @@ "action_type": "edit", "object_type": "theme" }, + { + "name": "Activate themes", + "action_type": "activate", + "object_type": "theme" + }, { "name": "Upload themes", "action_type": "add", @@ -301,13 +306,13 @@ "object_type": "invite" }, { - "name": "Add invites", - "action_type": "add", + "name": "Edit invites", + "action_type": "edit", "object_type": "invite" }, { - "name": "Edit invites", - "action_type": "edit", + "name": "Add invites", + "action_type": "add", "object_type": "invite" }, { diff --git a/core/server/models/settings.js b/core/server/models/settings.js index c4eece6e1b..0177973b25 100644 --- a/core/server/models/settings.js +++ b/core/server/models/settings.js @@ -7,7 +7,6 @@ var Settings, events = require('../events'), i18n = require('../i18n'), validation = require('../data/validation'), - themes = require('../themes'), internalContext = {context: {internal: true}}, @@ -83,14 +82,6 @@ Settings = ghostBookshelf.Model.extend({ return validation.validateSchema(self.tableName, setting).then(function then() { return validation.validateSettings(getDefaultSettings(), self); - }).then(function () { - var themeName = setting.value || ''; - - if (setting.key !== 'activeTheme') { - return; - } - - return themes.validate.activeTheme(themeName); }); } }, { diff --git a/core/server/themes/index.js b/core/server/themes/index.js index 3a68776a68..4de338d77d 100644 --- a/core/server/themes/index.js +++ b/core/server/themes/index.js @@ -7,5 +7,6 @@ module.exports = { loadAll: themeLoader.loadAllThemes, loadOne: themeLoader.loadOneTheme, list: require('./list'), - validate: require('./validate') + validate: require('./validate'), + toJSON: require('./to-json') }; diff --git a/core/server/themes/list.js b/core/server/themes/list.js index e1df9db17b..287cdea27c 100644 --- a/core/server/themes/list.js +++ b/core/server/themes/list.js @@ -2,7 +2,6 @@ * Store themes after loading them from the file system */ var _ = require('lodash'), - packages = require('../utils/packages'), themeListCache = {}; module.exports = { @@ -33,17 +32,5 @@ module.exports = { }); return themeListCache; - }, - toAPI: function toAPI(themes, active) { - var toFilter; - - if (themes.hasOwnProperty('name')) { - toFilter = {}; - toFilter[themes.name] = themes; - } else { - toFilter = themes; - } - - return packages.filterPackages(toFilter, active); } }; diff --git a/core/server/themes/to-json.js b/core/server/themes/to-json.js new file mode 100644 index 0000000000..12581045a4 --- /dev/null +++ b/core/server/themes/to-json.js @@ -0,0 +1,33 @@ +var _ = require('lodash'), + themeList = require('./list'), + packages = require('../utils/packages'), + settingsCache = require('../settings/cache'); + +/** + * Provides a JSON object which can be returned via the API + * + * @param {string} [name] - the theme to output + * @param {object} [checkedTheme] - a theme result from gscan + * @return {*} + */ +module.exports = function toJSON(name, checkedTheme) { + var themeResult, toFilter; + + if (!name) { + toFilter = themeList.getAll(); + // Default to returning the full list + themeResult = packages.filterPackages(toFilter, settingsCache.get('activeTheme')); + } else { + // If we pass in a gscan result, convert this instead + toFilter = {}; + toFilter[name] = themeList.get(name); + + themeResult = packages.filterPackages(toFilter, settingsCache.get('activeTheme')); + + if (checkedTheme && checkedTheme.results.warning.length > 0) { + themeResult[0].warnings = _.cloneDeep(checkedTheme.results.warning); + } + } + + return {themes: themeResult}; +}; diff --git a/core/server/themes/validate.js b/core/server/themes/validate.js index 5c6886c483..a505f39e31 100644 --- a/core/server/themes/validate.js +++ b/core/server/themes/validate.js @@ -1,21 +1,32 @@ var Promise = require('bluebird'), errors = require('../errors'), i18n = require('../i18n'), - themeList = require('./list'), - validateActiveTheme; + checkTheme; -// @TODO replace this with something PROPER - we should probably attempt to read the theme from the -// File system at this point and validate the theme using gscan rather than just checking if it's in a cache object -validateActiveTheme = function validateActiveTheme(themeName) { - if (!themeList.getAll() || Object.keys(themeList.getAll()).length === 0) { - // We haven't yet loaded all themes, this is probably being called early? - return Promise.resolve(); +checkTheme = function checkTheme(theme, isZip) { + var checkPromise, + // gscan can slow down boot time if we require on boot, for now nest the require. + gscan = require('gscan'); + + if (isZip) { + checkPromise = gscan.checkZip(theme, {keepExtractedDir: true}); + } else { + checkPromise = gscan.check(theme.path); } - // Else, if we have a list, check if the theme is in it - if (!themeList.get(themeName)) { - return Promise.reject(new errors.ValidationError({message: i18n.t('notices.data.validation.index.themeCannotBeActivated', {themeName: themeName}), context: 'activeTheme'})); - } + return checkPromise.then(function resultHandler(checkedTheme) { + checkedTheme = gscan.format(checkedTheme); + + // @TODO improve gscan results + if (!checkedTheme.results.error.length) { + return checkedTheme; + } + + return Promise.reject(new errors.ThemeValidationError({ + message: i18n.t('errors.api.themes.invalidTheme'), + errorDetails: checkedTheme.results.error + })); + }); }; -module.exports.activeTheme = validateActiveTheme; +module.exports.check = checkTheme; diff --git a/core/server/translations/en.json b/core/server/translations/en.json index ca4e3bc892..da77a6eb3e 100644 --- a/core/server/translations/en.json +++ b/core/server/translations/en.json @@ -105,7 +105,8 @@ } }, "themehandler": { - "missingTheme": "The currently active theme \"{theme}\" is missing." + "missingTheme": "The currently active theme \"{theme}\" is missing.", + "invalidTheme": "The currently active theme \"{theme}\" is invalid." } }, "utils": { @@ -114,18 +115,6 @@ "nameOrVersionMissing": "\"name\" or \"version\" is missing from theme package.json file.", "willBeRequired": "This will be required in future. Please see {url}", "themeFileIsMalformed": "Theme package.json file is malformed" - }, - "validatethemes": { - "themeWithNoPackage": { - "message": "Found a theme with no package.json file", - "context": "Theme name: {name}", - "help": "This will be required in future. Please see {url}" - }, - "malformedPackage": { - "message": "Found a malformed package.json", - "context": "Theme name: {name}", - "help": "Valid package.json will be required in future. Please see {url}" - } } }, "config": { @@ -341,6 +330,10 @@ "settings": { "problemFindingSetting": "Problem finding setting: {key}", "accessCoreSettingFromExtReq": "Attempted to access core setting from external request", + "activeThemeSetViaAPI": { + "error": "Attempted to change activeTheme via settings API", + "help": "Please activate theme via the themes API endpoints instead" + }, "invalidJsonInLabs": "Error: Invalid JSON in settings.labs", "labsColumnCouldNotBeParsed": "The column with key \"labs\" could not be parsed as JSON", "tryUpdatingLabs": "Please try updating a setting on the labs page, or manually editing your DB", @@ -368,7 +361,8 @@ "missingFile": "Please select a theme.", "invalidFile": "Please select a valid zip file.", "overrideCasper": "Please rename your zip, it's not allowed to override the default casper theme.", - "destroyCasper": "Deleting the default casper theme is not allowed." + "destroyCasper": "Deleting the default casper theme is not allowed.", + "destroyActive": "Deleting the active theme is not allowed." }, "images": { "missingFile": "Please select an image.", diff --git a/core/test/functional/routes/api/themes_spec.js b/core/test/functional/routes/api/themes_spec.js index 183c1b66a6..fa2fbed00d 100644 --- a/core/test/functional/routes/api/themes_spec.js +++ b/core/test/functional/routes/api/themes_spec.js @@ -21,8 +21,16 @@ describe('Themes API (Forked)', function () { .attach(fieldName, themePath); }, editor: null - }, forkedGhost, tmpContentPath; + }, + forkedGhost, + tmpContentPath; + /** + * Create a temporary folder that contains: + * - 1 valid theme: casper + * - 1 valid theme that has warnings: test-theme + * - 1 invalid theme: broken-theme + */ function setupThemesFolder() { tmpContentPath = tmp.dirSync({unsafeCleanup: true}); @@ -32,6 +40,22 @@ describe('Themes API (Forked)', function () { join(tmpContentPath.name, 'themes', 'casper', 'package.json'), JSON.stringify({name: 'casper', version: '0.1.2'}) ); + fs.writeFileSync(join(tmpContentPath.name, 'themes', 'casper', 'index.hbs')); + fs.writeFileSync(join(tmpContentPath.name, 'themes', 'casper', 'post.hbs')); + + fs.mkdirSync(join(tmpContentPath.name, 'themes', 'test-theme')); + fs.writeFileSync( + join(tmpContentPath.name, 'themes', 'test-theme', 'package.json'), + JSON.stringify({name: 'test-theme', version: '0.5'}) + ); + fs.writeFileSync(join(tmpContentPath.name, 'themes', 'test-theme', 'index.hbs')); + fs.writeFileSync(join(tmpContentPath.name, 'themes', 'test-theme', 'post.hbs')); + + fs.mkdirSync(join(tmpContentPath.name, 'themes', 'broken-theme')); + fs.writeFileSync( + join(tmpContentPath.name, 'themes', 'broken-theme', 'package.json'), + JSON.stringify({name: 'broken-theme', version: '1.1.2'}) + ); } function teardownThemesFolder() { @@ -97,12 +121,22 @@ describe('Themes API (Forked)', function () { var jsonResponse = res.body; should.exist(jsonResponse.themes); testUtils.API.checkResponse(jsonResponse, 'themes'); - jsonResponse.themes.length.should.eql(1); + jsonResponse.themes.length.should.eql(3); testUtils.API.checkResponse(jsonResponse.themes[0], 'theme'); - jsonResponse.themes[0].name.should.eql('casper'); + jsonResponse.themes[0].name.should.eql('broken-theme'); jsonResponse.themes[0].package.should.be.an.Object().with.properties('name', 'version'); - jsonResponse.themes[0].active.should.be.true(); + jsonResponse.themes[0].active.should.be.false(); + + testUtils.API.checkResponse(jsonResponse.themes[1], 'theme'); + jsonResponse.themes[1].name.should.eql('casper'); + jsonResponse.themes[1].package.should.be.an.Object().with.properties('name', 'version'); + jsonResponse.themes[1].active.should.be.true(); + + testUtils.API.checkResponse(jsonResponse.themes[2], 'theme'); + jsonResponse.themes[2].name.should.eql('test-theme'); + jsonResponse.themes[2].package.should.be.an.Object().with.properties('name', 'version'); + jsonResponse.themes[2].active.should.be.false(); done(); }); @@ -123,7 +157,7 @@ describe('Themes API (Forked)', function () { }); }); - it('upload theme', function (done) { + it('upload new "valid" theme', function (done) { var jsonResponse; scope.uploadTheme({themePath: join(__dirname, '/../../../utils/fixtures/themes/valid.zip')}) @@ -159,9 +193,11 @@ describe('Themes API (Forked)', function () { // ensure tmp theme folder contains two themes now var tmpFolderContents = fs.readdirSync(join(tmpContentPath.name, 'themes')); - tmpFolderContents.should.be.an.Array().with.lengthOf(2); - tmpFolderContents[0].should.eql('casper'); - tmpFolderContents[1].should.eql('valid'); + tmpFolderContents.should.be.an.Array().with.lengthOf(4); + tmpFolderContents[0].should.eql('broken-theme'); + tmpFolderContents[1].should.eql('casper'); + tmpFolderContents[2].should.eql('test-theme'); + tmpFolderContents[3].should.eql('valid'); // Check the Themes API returns the correct result request.get(testUtils.API.getApiQuery('themes/')) @@ -177,7 +213,7 @@ describe('Themes API (Forked)', function () { should.exist(jsonResponse.themes); testUtils.API.checkResponse(jsonResponse, 'themes'); - jsonResponse.themes.length.should.eql(2); + jsonResponse.themes.length.should.eql(4); // Casper should be present and still active casperTheme = _.find(jsonResponse.themes, {name: 'casper'}); @@ -199,7 +235,7 @@ describe('Themes API (Forked)', function () { // NOTE: This test requires the previous upload test // @TODO make this test independent - it('delete theme', function (done) { + it('delete new "valid" theme', function (done) { var jsonResponse; request.del(testUtils.API.getApiQuery('themes/valid')) @@ -216,10 +252,12 @@ describe('Themes API (Forked)', function () { // ensure tmp theme folder contains one theme again now var tmpFolderContents = fs.readdirSync(join(tmpContentPath.name, 'themes')); - tmpFolderContents.should.be.an.Array().with.lengthOf(1); - tmpFolderContents[0].should.eql('casper'); + tmpFolderContents.should.be.an.Array().with.lengthOf(3); + tmpFolderContents[0].should.eql('broken-theme'); + tmpFolderContents[1].should.eql('casper'); + tmpFolderContents[2].should.eql('test-theme'); - // Check the settings API returns the correct result + // Check the themes API returns the correct result after deletion request.get(testUtils.API.getApiQuery('themes/')) .set('Authorization', 'Bearer ' + scope.ownerAccessToken) .expect(200) @@ -233,7 +271,7 @@ describe('Themes API (Forked)', function () { should.exist(jsonResponse.themes); testUtils.API.checkResponse(jsonResponse, 'themes'); - jsonResponse.themes.length.should.eql(1); + jsonResponse.themes.length.should.eql(3); // Casper should be present and still active casperTheme = _.find(jsonResponse.themes, {name: 'casper'}); @@ -249,6 +287,95 @@ describe('Themes API (Forked)', function () { }); }); }); + + it('upload new "warnings" theme that has validation warnings', function (done) { + var jsonResponse; + + scope.uploadTheme({themePath: join(__dirname, '/../../../utils/fixtures/themes/warnings.zip')}) + .end(function (err, res) { + if (err) { + return done(err); + } + + jsonResponse = res.body; + + should.exist(jsonResponse.themes); + testUtils.API.checkResponse(jsonResponse, 'themes'); + jsonResponse.themes.length.should.eql(1); + testUtils.API.checkResponse(jsonResponse.themes[0], 'theme', ['warnings']); + jsonResponse.themes[0].name.should.eql('warnings'); + jsonResponse.themes[0].active.should.be.false(); + jsonResponse.themes[0].warnings.should.be.an.Array(); + + // Delete the theme to clean up after the test + request.del(testUtils.API.getApiQuery('themes/warnings')) + .set('Authorization', 'Bearer ' + scope.ownerAccessToken) + .expect(204) + .end(function (err) { + if (err) { + return done(err); + } + done(); + }); + }); + }); + + it('activate "test-theme" valid theme that has warnings', function (done) { + var jsonResponse, casperTheme, testTheme; + + // First check the browse response to see that casper is the active theme + request.get(testUtils.API.getApiQuery('themes/')) + .set('Authorization', 'Bearer ' + scope.ownerAccessToken) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + jsonResponse = res.body; + + should.exist(jsonResponse.themes); + testUtils.API.checkResponse(jsonResponse, 'themes'); + jsonResponse.themes.length.should.eql(3); + + casperTheme = _.find(jsonResponse.themes, {name: 'casper'}); + should.exist(casperTheme); + testUtils.API.checkResponse(casperTheme, 'theme'); + casperTheme.active.should.be.true(); + + testTheme = _.find(jsonResponse.themes, {name: 'test-theme'}); + should.exist(testTheme); + testUtils.API.checkResponse(testTheme, 'theme'); + testTheme.active.should.be.false(); + + // Finally activate the new theme + request.put(testUtils.API.getApiQuery('themes/test-theme/activate')) + .set('Authorization', 'Bearer ' + scope.ownerAccessToken) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + jsonResponse = res.body; + + should.exist(jsonResponse.themes); + testUtils.API.checkResponse(jsonResponse, 'themes'); + jsonResponse.themes.length.should.eql(1); + + casperTheme = _.find(jsonResponse.themes, {name: 'casper'}); + should.not.exist(casperTheme); + + testTheme = _.find(jsonResponse.themes, {name: 'test-theme'}); + should.exist(testTheme); + testUtils.API.checkResponse(testTheme, 'theme', ['warnings']); + testTheme.active.should.be.true(); + testTheme.warnings.should.be.an.Array(); + + done(); + }); + }); + }); }); describe('error cases', function () { @@ -282,6 +409,40 @@ describe('Themes API (Forked)', function () { }); }); + it('activate "broken-theme" invalid theme', function (done) { + request.put(testUtils.API.getApiQuery('themes/broken-theme/activate')) + .set('Authorization', 'Bearer ' + scope.ownerAccessToken) + .expect(422) + .end(function (err, res) { + if (err) { + return done(err); + } + + res.body.errors.length.should.eql(1); + res.body.errors[0].errorType.should.eql('ThemeValidationError'); + res.body.errors[0].message.should.eql('Theme is not compatible or contains errors.'); + + done(); + }); + }); + + it('activate non-existent theme', function (done) { + request.put(testUtils.API.getApiQuery('themes/not-existent/activate')) + .set('Authorization', 'Bearer ' + scope.ownerAccessToken) + .expect(422) + .end(function (err, res) { + if (err) { + return done(err); + } + + res.body.errors.length.should.eql(1); + res.body.errors[0].errorType.should.eql('ValidationError'); + res.body.errors[0].message.should.eql('not-existent cannot be activated because it is not currently installed.'); + + done(); + }); + }); + it('delete casper', function (done) { request.del(testUtils.API.getApiQuery('themes/casper')) .set('Authorization', 'Bearer ' + scope.ownerAccessToken) @@ -299,7 +460,7 @@ describe('Themes API (Forked)', function () { }); }); - it('delete not existent theme', function (done) { + it('delete non-existent theme', function (done) { request.del(testUtils.API.getApiQuery('themes/not-existent')) .set('Authorization', 'Bearer ' + scope.ownerAccessToken) .expect(404) @@ -316,6 +477,42 @@ describe('Themes API (Forked)', function () { }); }); + it('delete active theme', function (done) { + var jsonResponse, testTheme; + // ensure test-theme is active + request.put(testUtils.API.getApiQuery('themes/test-theme/activate')) + .set('Authorization', 'Bearer ' + scope.ownerAccessToken) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + jsonResponse = res.body; + + testTheme = _.find(jsonResponse.themes, {name: 'test-theme'}); + should.exist(testTheme); + testUtils.API.checkResponse(testTheme, 'theme', ['warnings']); + testTheme.active.should.be.true(); + testTheme.warnings.should.be.an.Array(); + + request.del(testUtils.API.getApiQuery('themes/test-theme')) + .set('Authorization', 'Bearer ' + scope.ownerAccessToken) + .expect(422) + .end(function (err, res) { + if (err) { + return done(err); + } + + res.body.errors.length.should.eql(1); + res.body.errors[0].errorType.should.eql('ValidationError'); + res.body.errors[0].message.should.eql('Deleting the active theme is not allowed.'); + + done(); + }); + }); + }); + it('upload non application/zip', function (done) { scope.uploadTheme({themePath: join(__dirname, '/../../../utils/fixtures/csv/single-column-with-header.csv')}) .end(function (err, res) { diff --git a/core/test/integration/api/api_settings_spec.js b/core/test/integration/api/api_settings_spec.js index c1d5a120b3..d17fec64f1 100644 --- a/core/test/integration/api/api_settings_spec.js +++ b/core/test/integration/api/api_settings_spec.js @@ -151,6 +151,19 @@ describe('Settings API', function () { }); }); + it('cannot edit the active theme setting via API even with internal context', function () { + return callApiWithContext(internalContext, 'edit', 'activeTheme', { + settings: [{key: 'activeTheme', value: 'rasper'}] + }).then(function () { + throw new Error('Allowed to change active theme settting'); + }).catch(function (err) { + should.exist(err); + + err.errorType.should.eql('BadRequestError'); + err.message.should.eql('Attempted to change activeTheme via settings API'); + }); + }); + it('ensures values are stringified before saving to database', function () { return callApiWithContext(defaultContext, 'edit', 'title', []).then(function (response) { should.exist(response); @@ -175,20 +188,4 @@ describe('Settings API', function () { it('set activeTimezone: known timezone', function () { return callApiWithContext(defaultContext, 'edit', {settings: [{key: 'activeTimezone', value: 'Etc/UTC'}]}, {}); }); - - describe('Themes (to be removed from settings)', function () { - beforeEach(testUtils.setup('themes')); - - it('does not allow an active theme which is not installed', function () { - return callApiWithContext(defaultContext, 'edit', 'activeTheme', { - settings: [{key: 'activeTheme', value: 'rasper'}] - }).then(function () { - throw new Error('Allowed to set an active theme which is not installed'); - }).catch(function (err) { - should.exist(err); - - err.errorType.should.eql('ValidationError'); - }); - }); - }); }); diff --git a/core/test/integration/migration_spec.js b/core/test/integration/migration_spec.js index 122126e71a..5fe1427659 100644 --- a/core/test/integration/migration_spec.js +++ b/core/test/integration/migration_spec.js @@ -95,54 +95,68 @@ describe('Database Migration (special functions)', function () { permissions[21].should.be.AssignedToRoles(['Administrator']); permissions[22].name.should.eql('Edit themes'); permissions[22].should.be.AssignedToRoles(['Administrator']); - permissions[23].name.should.eql('Upload themes'); + permissions[23].name.should.eql('Activate themes'); permissions[23].should.be.AssignedToRoles(['Administrator']); - permissions[24].name.should.eql('Download themes'); + permissions[24].name.should.eql('Upload themes'); permissions[24].should.be.AssignedToRoles(['Administrator']); - permissions[25].name.should.eql('Delete themes'); + permissions[25].name.should.eql('Download themes'); permissions[25].should.be.AssignedToRoles(['Administrator']); + permissions[26].name.should.eql('Delete themes'); + permissions[26].should.be.AssignedToRoles(['Administrator']); // Users - permissions[26].name.should.eql('Browse users'); - permissions[26].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); - permissions[27].name.should.eql('Read users'); + permissions[27].name.should.eql('Browse users'); permissions[27].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); - permissions[28].name.should.eql('Edit users'); - permissions[28].should.be.AssignedToRoles(['Administrator', 'Editor']); - permissions[29].name.should.eql('Add users'); + permissions[28].name.should.eql('Read users'); + permissions[28].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); + permissions[29].name.should.eql('Edit users'); permissions[29].should.be.AssignedToRoles(['Administrator', 'Editor']); - permissions[30].name.should.eql('Delete users'); + permissions[30].name.should.eql('Add users'); permissions[30].should.be.AssignedToRoles(['Administrator', 'Editor']); + permissions[31].name.should.eql('Delete users'); + permissions[31].should.be.AssignedToRoles(['Administrator', 'Editor']); // Roles - permissions[31].name.should.eql('Assign a role'); - permissions[31].should.be.AssignedToRoles(['Administrator', 'Editor']); - permissions[32].name.should.eql('Browse roles'); - permissions[32].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); + permissions[32].name.should.eql('Assign a role'); + permissions[32].should.be.AssignedToRoles(['Administrator', 'Editor']); + permissions[33].name.should.eql('Browse roles'); + permissions[33].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); // Clients - permissions[33].name.should.eql('Browse clients'); - permissions[33].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); - permissions[34].name.should.eql('Read clients'); + permissions[34].name.should.eql('Browse clients'); permissions[34].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); - permissions[35].name.should.eql('Edit clients'); + permissions[35].name.should.eql('Read clients'); permissions[35].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); - permissions[36].name.should.eql('Add clients'); + permissions[36].name.should.eql('Edit clients'); permissions[36].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); - permissions[37].name.should.eql('Delete clients'); + permissions[37].name.should.eql('Add clients'); permissions[37].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); + permissions[38].name.should.eql('Delete clients'); + permissions[38].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); // Subscribers - permissions[38].name.should.eql('Browse subscribers'); - permissions[38].should.be.AssignedToRoles(['Administrator']); - permissions[39].name.should.eql('Read subscribers'); + permissions[39].name.should.eql('Browse subscribers'); permissions[39].should.be.AssignedToRoles(['Administrator']); - permissions[40].name.should.eql('Edit subscribers'); + permissions[40].name.should.eql('Read subscribers'); permissions[40].should.be.AssignedToRoles(['Administrator']); - permissions[41].name.should.eql('Add subscribers'); - permissions[41].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); - permissions[42].name.should.eql('Delete subscribers'); - permissions[42].should.be.AssignedToRoles(['Administrator']); + permissions[41].name.should.eql('Edit subscribers'); + permissions[41].should.be.AssignedToRoles(['Administrator']); + permissions[42].name.should.eql('Add subscribers'); + permissions[42].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); + permissions[43].name.should.eql('Delete subscribers'); + permissions[43].should.be.AssignedToRoles(['Administrator']); + + // Invites + permissions[44].name.should.eql('Browse invites'); + permissions[44].should.be.AssignedToRoles(['Administrator', 'Editor']); + permissions[45].name.should.eql('Read invites'); + permissions[45].should.be.AssignedToRoles(['Administrator', 'Editor']); + permissions[46].name.should.eql('Edit invites'); + permissions[46].should.be.AssignedToRoles(['Administrator', 'Editor']); + permissions[47].name.should.eql('Add invites'); + permissions[47].should.be.AssignedToRoles(['Administrator', 'Editor']); + permissions[48].name.should.eql('Delete invites'); + permissions[48].should.be.AssignedToRoles(['Administrator', 'Editor']); }); describe('Populate', function () { @@ -203,11 +217,11 @@ describe('Database Migration (special functions)', function () { result.roles.at(3).get('name').should.eql('Owner'); // Permissions - result.permissions.length.should.eql(48); + result.permissions.length.should.eql(49); result.permissions.toJSON().should.be.CompletePermissions(); done(); - }); + }).catch(done); }); }); }); diff --git a/core/test/unit/migration_fixture_utils_spec.js b/core/test/unit/migration_fixture_utils_spec.js index 90c4841547..45acf6cc7a 100644 --- a/core/test/unit/migration_fixture_utils_spec.js +++ b/core/test/unit/migration_fixture_utils_spec.js @@ -112,7 +112,7 @@ describe('Migration Fixture Utils', function () { postAddStub.calledOnce.should.be.true(); done(); - }); + }).catch(done); }); it('should not call add for main post fixture if it is already found', function (done) { @@ -128,7 +128,7 @@ describe('Migration Fixture Utils', function () { postAddStub.calledOnce.should.be.false(); done(); - }); + }).catch(done); }); }); diff --git a/core/test/unit/migration_spec.js b/core/test/unit/migration_spec.js index 678ee2f35a..660f31f7e7 100644 --- a/core/test/unit/migration_spec.js +++ b/core/test/unit/migration_spec.js @@ -21,7 +21,7 @@ should.equal(true, true); describe('DB version integrity', function () { // Only these variables should need updating var currentSchemaHash = 'ae4ada98be2691b4d6e323eebcdb875f', - currentFixturesHash = 'b9e684a87353c592df9b23948e364c05'; + currentFixturesHash = '46abf9fd0d67fc89fa7845bef7fc7ffd'; // If this test is failing, then it is likely a change has been made that requires a DB version bump, // and the values above will need updating as confirmation diff --git a/core/test/utils/fixtures/themes/warnings.zip b/core/test/utils/fixtures/themes/warnings.zip new file mode 100644 index 0000000000000000000000000000000000000000..4fc1d14999682b837d02ead922f70789465097ec GIT binary patch literal 638 zcmWIWW@h1H0D-eTSzcfUl;C6#U?@*4%FE14FV+u@;9+QfawuXF5PxM9VF-Y0+l!`6 z2w7WZUP@|(UPe+eNPj6%F9^f*i!g8iA-c7q$odNsld}`kQ}wcn^YcJPOh$6T>SsX= zOiTA1xf7zQ&AN0_&y_nNcR!t(#B=x4kwtsX?D>+UD%&w>)0IV!&it{Npyh1Kthz2r z59n}q4v~x6a%MnZ+5j=aBZtsDB8cLU{NfUrKdOO_0AYwf7@0(waRnd`EIb|FI)a!; z!N~y+&H!(O5txCAFk(rgIkFK*LCXOTT8LSQfJOGHC