diff --git a/src/apps.js b/src/apps.js index 0a56281cd..f58021b67 100644 --- a/src/apps.js +++ b/src/apps.js @@ -204,6 +204,13 @@ function postProcess(app) { app.portBindings = result; } +function addSpacesSuffix(location, user) { + if (user.admin || !config.isSpacesEnabled()) return location; + + const spacesSuffix = user.username.replace(/\./g, '-'); + return location === '' ? spacesSuffix : `${location}-${spacesSuffix}`; +} + function validateAccessRestriction(accessRestriction) { assert.strictEqual(typeof accessRestriction, 'object'); @@ -487,8 +494,9 @@ function mailboxNameForLocation(location, manifest) { return (location ? location : manifest.title.toLowerCase().replace(/[^a-zA-Z0-9]/g, '')) + '.app'; } -function install(data, auditSource, callback) { +function install(data, user, auditSource, callback) { assert(data && typeof data === 'object'); + assert(user && typeof user === 'object'); assert.strictEqual(typeof auditSource, 'object'); assert.strictEqual(typeof callback, 'function'); @@ -560,6 +568,9 @@ function install(data, auditSource, callback) { if (error && error.reason === DomainsError.NOT_FOUND) return callback(new AppsError(AppsError.NOT_FOUND, 'No such domain')); if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, 'Could not get domain info:' + error.message)); + location = addSpacesSuffix(location, user); + alternateDomains.forEach(function (ad) { ad.subdomain = addSpacesSuffix(ad.subdomain, user); }); // TODO: validate these + error = domains.validateHostname(location, domainObject); if (error) return callback(new AppsError(AppsError.BAD_FIELD, 'Bad location: ' + error.message)); @@ -628,9 +639,10 @@ function install(data, auditSource, callback) { }); } -function configure(appId, data, auditSource, callback) { +function configure(appId, data, user, auditSource, callback) { assert.strictEqual(typeof appId, 'string'); assert(data && typeof data === 'object'); + assert(user && typeof user === 'object'); assert.strictEqual(typeof auditSource, 'object'); assert.strictEqual(typeof callback, 'function'); @@ -691,12 +703,15 @@ function configure(appId, data, auditSource, callback) { if ('alternateDomains' in data) { // TODO validate all subdomains [{ domain: '', subdomain: ''}] values.alternateDomains = data.alternateDomains; + values.alternateDomains.forEach(function (ad) { ad.subdomain = addSpacesSuffix(ad.subdomain, user); }); // TODO: validate these } domains.get(domain, function (error, domainObject) { if (error && error.reason === DomainsError.NOT_FOUND) return callback(new AppsError(AppsError.NOT_FOUND, 'No such domain')); if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, 'Could not get domain info:' + error.message)); + location = addSpacesSuffix(location, user); + error = domains.validateHostname(location, domainObject); if (error) return callback(new AppsError(AppsError.BAD_FIELD, 'Bad location: ' + error.message)); @@ -919,9 +934,10 @@ function restore(appId, data, auditSource, callback) { }); } -function clone(appId, data, auditSource, callback) { +function clone(appId, data, user, auditSource, callback) { assert.strictEqual(typeof appId, 'string'); assert.strictEqual(typeof data, 'object'); + assert(user && typeof user === 'object'); assert.strictEqual(typeof auditSource, 'object'); assert.strictEqual(typeof callback, 'function'); @@ -960,6 +976,7 @@ function clone(appId, data, auditSource, callback) { if (error && error.reason === DomainsError.NOT_FOUND) return callback(new AppsError(AppsError.EXTERNAL_ERROR, 'No such domain')); if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, 'Could not get domain info:' + error.message)); + location = addSpacesSuffix(location, user); error = domains.validateHostname(location, domainObject); if (error) return callback(new AppsError(AppsError.BAD_FIELD, 'Bad location: ' + error.message)); diff --git a/src/routes/apps.js b/src/routes/apps.js index 811413148..144889498 100644 --- a/src/routes/apps.js +++ b/src/routes/apps.js @@ -30,7 +30,6 @@ exports = module.exports = { var apps = require('../apps.js'), AppsError = apps.AppsError, assert = require('assert'), - config = require('../config.js'), debug = require('debug')('box:routes/apps'), fs = require('fs'), HttpError = require('connect-lastmile').HttpError, @@ -45,14 +44,6 @@ function auditSource(req) { return { ip: ip, username: req.user ? req.user.username : null, userId: req.user ? req.user.id : null }; } -// TODO: move this to model code -function addSpacesSuffix(location, user) { - if (user.admin || !config.isSpacesEnabled()) return location; - - const spacesSuffix = user.username.replace(/\./g, '-'); - return location === '' ? spacesSuffix : `${location}-${spacesSuffix}`; -} - function getApp(req, res, next) { assert.strictEqual(typeof req.params.id, 'string'); @@ -99,7 +90,6 @@ function installApp(req, res, next) { // required if (typeof data.location !== 'string') return next(new HttpError(400, 'location is required')); - data.location = addSpacesSuffix(data.location, req.user); if (typeof data.domain !== 'string') return next(new HttpError(400, 'domain is required')); if (typeof data.accessRestriction !== 'object') return next(new HttpError(400, 'accessRestriction is required')); @@ -130,13 +120,11 @@ function installApp(req, res, next) { if ('alternateDomains' in data) { if (!Array.isArray(data.alternateDomains)) return next(new HttpError(400, 'alternateDomains must be an array')); if (data.alternateDomains.some(function (d) { return (typeof d.domain !== 'string' || typeof d.subdomain !== 'string'); })) return next(new HttpError(400, 'alternateDomains array must contain objects with domain and subdomain strings')); - - data.alternateDomains.forEach(function (ad) { ad.subdomain = addSpacesSuffix(ad.subdomain, req.user); }); } debug('Installing app :%j', data); - apps.install(data, auditSource(req), function (error, app) { + apps.install(data, req.user, auditSource(req), function (error, app) { if (error && error.reason === AppsError.NOT_FOUND) return next(new HttpError(404, error.message)); if (error && error.reason === AppsError.ALREADY_EXISTS) return next(new HttpError(409, error.message)); if (error && error.reason === AppsError.PORT_RESERVED) return next(new HttpError(409, 'Port ' + error.message + ' is reserved.')); @@ -157,11 +145,7 @@ function configureApp(req, res, next) { var data = req.body; - if ('location' in data) { - if (typeof data.location !== 'string') return next(new HttpError(400, 'location must be string')); - data.location = addSpacesSuffix(data.location, req.user); - } - + if ('location' in data && typeof data.location !== 'string') return next(new HttpError(400, 'location must be string')); if ('domain' in data && typeof data.domain !== 'string') return next(new HttpError(400, 'domain must be string')); if ('portBindings' in data && typeof data.portBindings !== 'object') return next(new HttpError(400, 'portBindings must be an object')); if ('accessRestriction' in data && typeof data.accessRestriction !== 'object') return next(new HttpError(400, 'accessRestriction must be an object')); @@ -186,13 +170,11 @@ function configureApp(req, res, next) { if ('alternateDomains' in data) { if (!Array.isArray(data.alternateDomains)) return next(new HttpError(400, 'alternateDomains must be an array')); if (data.alternateDomains.some(function (d) { return (typeof d.domain !== 'string' || typeof d.subdomain !== 'string'); })) return next(new HttpError(400, 'alternateDomains array must contain objects with domain and subdomain strings')); - - data.alternateDomains.forEach(function (ad) { ad.subdomain = addSpacesSuffix(ad.subdomain, req.user); }); } debug('Configuring app id:%s data:%j', req.params.id, data); - apps.configure(req.params.id, data, auditSource(req), function (error) { + apps.configure(req.params.id, data, req.user, auditSource(req), function (error) { if (error && error.reason === AppsError.ALREADY_EXISTS) return next(new HttpError(409, error.message)); if (error && error.reason === AppsError.PORT_RESERVED) return next(new HttpError(409, 'Port ' + error.message + ' is reserved.')); if (error && error.reason === AppsError.PORT_CONFLICT) return next(new HttpError(409, 'Port ' + error.message + ' is already in use.')); @@ -239,11 +221,10 @@ function cloneApp(req, res, next) { if (typeof data.backupId !== 'string') return next(new HttpError(400, 'backupId must be a string')); if (typeof data.location !== 'string') return next(new HttpError(400, 'location is required')); - data.location = addSpacesSuffix(data.location, req.user); if (typeof data.domain !== 'string') return next(new HttpError(400, 'domain is required')); if (('portBindings' in data) && typeof data.portBindings !== 'object') return next(new HttpError(400, 'portBindings must be an object')); - apps.clone(req.params.id, data, auditSource(req), function (error, result) { + apps.clone(req.params.id, data, req.user, auditSource(req), function (error, result) { if (error && error.reason === AppsError.NOT_FOUND) return next(new HttpError(404, 'No such app')); if (error && error.reason === AppsError.PORT_RESERVED) return next(new HttpError(409, 'Port ' + error.message + ' is reserved.')); if (error && error.reason === AppsError.PORT_CONFLICT) return next(new HttpError(409, 'Port ' + error.message + ' is already in use.'));