diff --git a/src/apps.js b/src/apps.js index f3a5cd2f2..eab53272c 100644 --- a/src/apps.js +++ b/src/apps.js @@ -188,7 +188,8 @@ function validateAccessRestriction(accessRestriction) { if (accessRestriction === null) return null; - if (!accessRestriction.users) return new Error('Users property required'); + if (!accessRestriction.users || !Array.isArray(accessRestriction.users)) return new Error('users array property required'); + if (accessRestriction.users.length === 0) return new Error('users array cannot be empty'); if (!accessRestriction.users.every(function (e) { return typeof e === 'string'; })) return new Error('All users have to be strings'); return null; diff --git a/src/routes/test/apps-test.js b/src/routes/test/apps-test.js index 47b179e75..e934d2641 100644 --- a/src/routes/test/apps-test.js +++ b/src/routes/test/apps-test.js @@ -221,7 +221,7 @@ describe('App API', function () { it('app install fails - invalid location', function (done) { request.post(SERVER_URL + '/api/v1/apps/install') .query({ access_token: token }) - .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: '!awesome', accessRestriction: '', oauthProxy: false }) + .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: '!awesome', accessRestriction: null, oauthProxy: false }) .end(function (err, res) { expect(res.statusCode).to.equal(400); expect(res.body.message).to.eql('Hostname can only contain alphanumerics and hyphen'); @@ -232,7 +232,7 @@ describe('App API', function () { it('app install fails - invalid location type', function (done) { request.post(SERVER_URL + '/api/v1/apps/install') .query({ access_token: token }) - .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: 42, accessRestriction: '', oauthProxy: false }) + .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: 42, accessRestriction: null, oauthProxy: false }) .end(function (err, res) { expect(res.statusCode).to.equal(400); expect(res.body.message).to.eql('location is required'); @@ -243,7 +243,7 @@ describe('App API', function () { it('app install fails - reserved admin location', function (done) { request.post(SERVER_URL + '/api/v1/apps/install') .query({ access_token: token }) - .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: constants.ADMIN_LOCATION, accessRestriction: '', oauthProxy: false }) + .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: constants.ADMIN_LOCATION, accessRestriction: null, oauthProxy: false }) .end(function (err, res) { expect(res.statusCode).to.equal(400); expect(res.body.message).to.eql(constants.ADMIN_LOCATION + ' is reserved'); @@ -254,7 +254,7 @@ describe('App API', function () { it('app install fails - reserved api location', function (done) { request.post(SERVER_URL + '/api/v1/apps/install') .query({ access_token: token }) - .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: constants.API_LOCATION, accessRestriction: '', oauthProxy: true }) + .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: constants.API_LOCATION, accessRestriction: null, oauthProxy: true }) .end(function (err, res) { expect(res.statusCode).to.equal(400); expect(res.body.message).to.eql(constants.API_LOCATION + ' is reserved'); @@ -265,7 +265,7 @@ describe('App API', function () { it('app install fails - portBindings must be object', function (done) { request.post(SERVER_URL + '/api/v1/apps/install') .query({ access_token: token }) - .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: 23, accessRestriction: '', oauthProxy: false }) + .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: 23, accessRestriction: null, oauthProxy: false }) .end(function (err, res) { expect(res.statusCode).to.equal(400); expect(res.body.message).to.eql('portBindings must be an object'); @@ -284,10 +284,21 @@ describe('App API', function () { }); }); + it('app install fails - accessRestriction type is wrong', function (done) { + request.post(SERVER_URL + '/api/v1/apps/install') + .query({ access_token: token }) + .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: {}, accessRestriction: '', oauthProxy: false }) + .end(function (err, res) { + expect(res.statusCode).to.equal(400); + expect(res.body.message).to.eql('accessRestriction is required'); + done(err); + }); + }); + it('app install fails - oauthProxy is required', function (done) { request.post(SERVER_URL + '/api/v1/apps/install') .query({ access_token: token }) - .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: {}, accessRestriction: '' }) + .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: {}, accessRestriction: null }) .end(function (err, res) { expect(res.statusCode).to.equal(400); expect(res.body.message).to.eql('oauthProxy must be a boolean'); @@ -298,7 +309,7 @@ describe('App API', function () { it('app install fails for non admin', function (done) { request.post(SERVER_URL + '/api/v1/apps/install') .query({ access_token: token_1 }) - .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: null, accessRestriction: '', oauthProxy: false }) + .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: null, accessRestriction: null, oauthProxy: false }) .end(function (err, res) { expect(res.statusCode).to.equal(403); done(err); @@ -310,7 +321,7 @@ describe('App API', function () { request.post(SERVER_URL + '/api/v1/apps/install') .query({ access_token: token }) - .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: null, accessRestriction: '', oauthProxy: false }) + .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: null, accessRestriction: null, oauthProxy: false }) .end(function (err, res) { expect(res.statusCode).to.equal(402); expect(fake.isDone()).to.be.ok(); @@ -323,7 +334,7 @@ describe('App API', function () { request.post(SERVER_URL + '/api/v1/apps/install') .query({ access_token: token }) - .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: null, accessRestriction: '', oauthProxy: false }) + .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: null, accessRestriction: null, oauthProxy: false }) .end(function (err, res) { expect(res.statusCode).to.equal(202); expect(res.body.id).to.be.a('string'); @@ -338,7 +349,7 @@ describe('App API', function () { request.post(SERVER_URL + '/api/v1/apps/install') .query({ access_token: token }) - .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: null, accessRestriction: '', oauthProxy: false }) + .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: null, accessRestriction: null, oauthProxy: false }) .end(function (err, res) { expect(res.statusCode).to.equal(409); expect(fake.isDone()).to.be.ok(); @@ -462,7 +473,7 @@ describe('App API', function () { request.post(SERVER_URL + '/api/v1/apps/install') .query({ access_token: token }) - .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION_2, portBindings: null, accessRestriction: '', oauthProxy: false }) + .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION_2, portBindings: null, accessRestriction: null, oauthProxy: false }) .end(function (err, res) { expect(res.statusCode).to.equal(202); expect(res.body.id).to.be.a('string'); @@ -491,7 +502,7 @@ describe('App API', function () { request.post(SERVER_URL + '/api/v1/apps/install') .query({ access_token: token }) - .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, location: APP_LOCATION+APP_LOCATION, portBindings: null, accessRestriction: '', oauthProxy: false }) + .send({ appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, location: APP_LOCATION+APP_LOCATION, portBindings: null, accessRestriction: null, oauthProxy: false }) .end(function (err, res) { expect(res.statusCode).to.equal(202); expect(res.body.id).to.be.a('string'); @@ -602,7 +613,7 @@ describe('App installation', function () { request.post(SERVER_URL + '/api/v1/apps/install') .query({ access_token: token }) - .send({ appId: APP_ID, appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: null, accessRestriction: '', oauthProxy: false }) + .send({ appId: APP_ID, appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: null, accessRestriction: null, oauthProxy: false }) .end(function (err, res) { expect(res.statusCode).to.equal(202); expect(fake.isDone()).to.be.ok(); @@ -1057,7 +1068,7 @@ describe('App installation - port bindings', function () { request.post(SERVER_URL + '/api/v1/apps/install') .query({ access_token: token }) - .send({ appId: APP_ID, appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: { ECHO_SERVER_PORT: 7171 }, accessRestriction: '', oauthProxy: false }) + .send({ appId: APP_ID, appStoreId: APP_STORE_ID, manifest: APP_MANIFEST, password: PASSWORD, location: APP_LOCATION, portBindings: { ECHO_SERVER_PORT: 7171 }, accessRestriction: null, oauthProxy: false }) .end(function (err, res) { expect(res.statusCode).to.equal(202); expect(fake.isDone()).to.be.ok(); @@ -1218,7 +1229,7 @@ describe('App installation - port bindings', function () { it('cannot reconfigure app with missing location', function (done) { request.post(SERVER_URL + '/api/v1/apps/' + APP_ID + '/configure') .query({ access_token: token }) - .send({ appId: APP_ID, password: PASSWORD, portBindings: { ECHO_SERVER_PORT: 7172 }, accessRestriction: '', oauthProxy: true }) + .send({ appId: APP_ID, password: PASSWORD, portBindings: { ECHO_SERVER_PORT: 7172 }, accessRestriction: null, oauthProxy: true }) .end(function (err, res) { expect(res.statusCode).to.equal(400); done(); @@ -1238,7 +1249,7 @@ describe('App installation - port bindings', function () { it('cannot reconfigure app with missing oauthProxy', function (done) { request.post(SERVER_URL + '/api/v1/apps/' + APP_ID + '/configure') .query({ access_token: token }) - .send({ appId: APP_ID, password: PASSWORD, location: APP_LOCATION_NEW, portBindings: { ECHO_SERVER_PORT: 7172 }, accessRestriction: '' }) + .send({ appId: APP_ID, password: PASSWORD, location: APP_LOCATION_NEW, portBindings: { ECHO_SERVER_PORT: 7172 }, accessRestriction: null }) .end(function (err, res) { expect(res.statusCode).to.equal(400); done(); @@ -1248,7 +1259,7 @@ describe('App installation - port bindings', function () { it('non admin cannot reconfigure app', function (done) { request.post(SERVER_URL + '/api/v1/apps/' + APP_ID + '/configure') .query({ access_token: token_1 }) - .send({ appId: APP_ID, password: PASSWORD, location: APP_LOCATION_NEW, portBindings: { ECHO_SERVER_PORT: 7172 }, accessRestriction: '', oauthProxy: true }) + .send({ appId: APP_ID, password: PASSWORD, location: APP_LOCATION_NEW, portBindings: { ECHO_SERVER_PORT: 7172 }, accessRestriction: null, oauthProxy: true }) .end(function (err, res) { expect(res.statusCode).to.equal(403); done(); @@ -1258,7 +1269,7 @@ describe('App installation - port bindings', function () { it('can reconfigure app', function (done) { request.post(SERVER_URL + '/api/v1/apps/' + APP_ID + '/configure') .query({ access_token: token }) - .send({ appId: APP_ID, password: PASSWORD, location: APP_LOCATION_NEW, portBindings: { ECHO_SERVER_PORT: 7172 }, accessRestriction: '', oauthProxy: true }) + .send({ appId: APP_ID, password: PASSWORD, location: APP_LOCATION_NEW, portBindings: { ECHO_SERVER_PORT: 7172 }, accessRestriction: null, oauthProxy: true }) .end(function (err, res) { expect(res.statusCode).to.equal(202); checkConfigureStatus(0, done); diff --git a/src/routes/test/backups-test.js b/src/routes/test/backups-test.js index 4d62db3e8..704fb0cd5 100644 --- a/src/routes/test/backups-test.js +++ b/src/routes/test/backups-test.js @@ -51,7 +51,7 @@ function setup(done) { function addApp(callback) { var manifest = { version: '0.0.1', manifestVersion: 1, dockerImage: 'foo', healthCheckPath: '/', httpPort: 3, title: 'ok', addons: { } }; - appdb.add('appid', 'appStoreId', manifest, 'location', [ ] /* portBindings */, '' /* accessRestriction */, false /* oauthProxy */, callback); + appdb.add('appid', 'appStoreId', manifest, 'location', [ ] /* portBindings */, null /* accessRestriction */, false /* oauthProxy */, callback); } ], done); } diff --git a/src/routes/test/oauth2-test.js b/src/routes/test/oauth2-test.js index 77cdbec0a..54d4137c2 100644 --- a/src/routes/test/oauth2-test.js +++ b/src/routes/test/oauth2-test.js @@ -154,7 +154,7 @@ describe('OAuth2', function () { manifest: { version: '0.1.0' }, location: 'test', portBindings: {}, - accessRestriction: '', + accessRestriction: null, oauthProxy: true }; @@ -164,7 +164,7 @@ describe('OAuth2', function () { manifest: { version: '0.1.0' }, location: 'test1', portBindings: {}, - accessRestriction: 'user-foobar', + accessRestriction: { users: [ 'foobar' ] }, oauthProxy: true }; @@ -174,7 +174,7 @@ describe('OAuth2', function () { manifest: { version: '0.1.0' }, location: 'test2', portBindings: {}, - accessRestriction: 'user-' + USER_0.id, + accessRestriction: { users: [ USER_0.id ] }, oauthProxy: true }; @@ -296,7 +296,7 @@ describe('OAuth2', function () { // update the global objects to reflect the new user id USER_0.id = userObject.id; - APP_2.accessRestriction = 'user-foobar,user-' + userObject.id; + APP_2.accessRestriction = { users: [ 'foobar', userObject.id ] }; appdb.update(APP_2.id, APP_2, callback); }); diff --git a/src/routes/test/settings-test.js b/src/routes/test/settings-test.js index 82b0273b3..0cfd94aa9 100644 --- a/src/routes/test/settings-test.js +++ b/src/routes/test/settings-test.js @@ -54,7 +54,7 @@ function setup(done) { function addApp(callback) { var manifest = { version: '0.0.1', manifestVersion: 1, dockerImage: 'foo', healthCheckPath: '/', httpPort: 3, title: 'ok' }; - appdb.add('appid', 'appStoreId', manifest, 'location', [ ] /* portBindings */, '' /* accessRestriction */, false /* oauthProxy */, callback); + appdb.add('appid', 'appStoreId', manifest, 'location', [ ] /* portBindings */, null /* accessRestriction */, false /* oauthProxy */, callback); } ], done); } diff --git a/src/routes/test/simpleauth-test.js b/src/routes/test/simpleauth-test.js index 38da65910..4ca7957fa 100644 --- a/src/routes/test/simpleauth-test.js +++ b/src/routes/test/simpleauth-test.js @@ -29,7 +29,7 @@ describe('SimpleAuth API', function () { manifest: { version: '0.1.0' }, location: 'test0', portBindings: {}, - accessRestriction: 'user-foobar,user-someone', + accessRestriction: { users: [ 'foobar', 'someone'] }, oauthProxy: true }; @@ -39,7 +39,7 @@ describe('SimpleAuth API', function () { manifest: { version: '0.1.0' }, location: 'test1', portBindings: {}, - accessRestriction: 'user-foobar,user-' + USERNAME + ',user-someone', + accessRestriction: { users: [ 'foobar', USERNAME, 'someone' ] }, oauthProxy: true }; @@ -49,7 +49,7 @@ describe('SimpleAuth API', function () { manifest: { version: '0.1.0' }, location: 'test2', portBindings: {}, - accessRestriction: '', + accessRestriction: null, oauthProxy: true }; diff --git a/src/test/apps-test.js b/src/test/apps-test.js index 1ae0ab399..29aefc218 100644 --- a/src/test/apps-test.js +++ b/src/test/apps-test.js @@ -36,7 +36,7 @@ describe('Apps', function () { containerId: null, portBindings: { PORT: 5678 }, healthy: null, - accessRestriction: '', + accessRestriction: null, oauthProxy: false }; @@ -160,59 +160,46 @@ describe('Apps', function () { }); describe('validateAccessRestriction', function () { - it('allows empty input', function () { - expect(apps._validateAccessRestriction('')).to.eql(null); + it('allows null input', function () { + expect(apps._validateAccessRestriction(null)).to.eql(null); + }); + + it('does not allow wrong user type', function () { + expect(apps._validateAccessRestriction({ users: {} })).to.be.an(Error); + }); + + it('does not allow no user input', function () { + expect(apps._validateAccessRestriction({ users: [] })).to.be.an(Error); }); it('allows single user input', function () { - expect(apps._validateAccessRestriction('user-someuserid')).to.eql(null); - }); - - it('does not allow single user input with no prefix', function () { - expect(apps._validateAccessRestriction('someuserid')).to.be.an(Error); - }); - - it('does not allow single user input with unkown prefix', function () { - expect(apps._validateAccessRestriction('foobar-someuserid')).to.be.an(Error); + expect(apps._validateAccessRestriction({ users: [ 'someuserid' ] })).to.eql(null); }); it('allows multi user input', function () { - expect(apps._validateAccessRestriction('user-someuserid,user-someuserid1,user-someuserid2,user-someuserid3')).to.eql(null); - }); - - it('allows multi user input with whitespace', function () { - expect(apps._validateAccessRestriction('user-someuserid ,user-someuserid1 ,user-someuserid2 , user-someuserid3')).to.eql(null); - }); - - it('does not allow multi user input with no prefix', function () { - expect(apps._validateAccessRestriction('user-someuserid,someuserid1,user-someuserid2,user-someuserid3')).to.be.an(Error); - }); - - it('does not allow multi user input with unkown prefix', function () { - expect(apps._validateAccessRestriction('user-someuserid,user-someuserid1,user-someuserid2,foo-someuserid3')).to.be.an(Error); + expect(apps._validateAccessRestriction({ users: [ 'someuserid', 'someuserid1', 'someuserid2', 'someuserid3' ] })).to.eql(null); }); }); describe('hasAccessTo', function () { it('returns true for unrestricted access', function () { - expect(apps.hasAccessTo({ accessRestriction: '' }, { id: 'someuser' })).to.equal(true); + expect(apps.hasAccessTo({ accessRestriction: null }, { id: 'someuser' })).to.equal(true); }); it('returns true for allowed user', function () { - expect(apps.hasAccessTo({ accessRestriction: 'user-someuser' }, { id: 'someuser' })).to.equal(true); + expect(apps.hasAccessTo({ accessRestriction: { users: [ 'someuser' ] } }, { id: 'someuser' })).to.equal(true); }); it('returns true for allowed user with multiple allowed', function () { - expect(apps.hasAccessTo({ accessRestriction: 'user-foo,user-someuser, user-anotheruser' }, { id: 'someuser' })).to.equal(true); + expect(apps.hasAccessTo({ accessRestriction: { users: [ 'foo', 'someuser', 'anotheruser' ] } }, { id: 'someuser' })).to.equal(true); }); it('returns false for not allowed user', function () { - expect(apps.hasAccessTo({ accessRestriction: 'user-foo' }, { id: 'someuser' })).to.equal(false); + expect(apps.hasAccessTo({ accessRestriction: { users: [ 'foo' ] } }, { id: 'someuser' })).to.equal(false); }); it('returns false for not allowed user with multiple allowed', function () { - expect(apps.hasAccessTo({ accessRestriction: 'user-foo, user-anotheruser' }, { id: 'someuser' })).to.equal(false); + expect(apps.hasAccessTo({ accessRestriction: { users: [ 'foo', 'anotheruser' ] } }, { id: 'someuser' })).to.equal(false); }); }); }); - diff --git a/src/test/apptask-test.js b/src/test/apptask-test.js index a40ca4b13..f50381f34 100644 --- a/src/test/apptask-test.js +++ b/src/test/apptask-test.js @@ -57,7 +57,7 @@ var APP = { containerId: null, httpPort: 4567, portBindings: null, - accessRestriction: '', + accessRestriction: null, oauthProxy: false, dnsRecordId: 'someDnsRecordId' }; diff --git a/src/test/database-test.js b/src/test/database-test.js index c4c0e7f28..59817efda 100644 --- a/src/test/database-test.js +++ b/src/test/database-test.js @@ -478,7 +478,7 @@ describe('database', function () { containerId: null, portBindings: { port: 5678 }, health: null, - accessRestriction: '', + accessRestriction: null, oauthProxy: false, lastBackupId: null, lastBackupConfig: null, @@ -497,7 +497,7 @@ describe('database', function () { containerId: null, portBindings: { }, health: null, - accessRestriction: '', + accessRestriction: { users: [ 'foobar' ] }, oauthProxy: true, lastBackupId: null, lastBackupConfig: null,