diff --git a/src/apps.js b/src/apps.js index 3d31c1365..ce7cda918 100644 --- a/src/apps.js +++ b/src/apps.js @@ -226,12 +226,14 @@ function getIconUrlSync(app) { return fs.existsSync(iconPath) ? '/api/v1/apps/' + app.id + '/icon' : null; } -function hasAccessTo(app, user) { +function hasAccessTo(app, user, callback) { assert.strictEqual(typeof app, 'object'); assert.strictEqual(typeof user, 'object'); + assert.strictEqual(typeof callback, 'function'); - if (app.accessRestriction === null) return true; - return app.accessRestriction.users.some(function (e) { return e === user.id; }); + if (app.accessRestriction === null) return callback(null, true); + + callback(null, app.accessRestriction.users.some(function (e) { return e === user.id; })); } function get(appId, callback) { diff --git a/src/routes/oauth2.js b/src/routes/oauth2.js index 77d7291ce..30226b29a 100644 --- a/src/routes/oauth2.js +++ b/src/routes/oauth2.js @@ -380,9 +380,12 @@ var authorization = [ appdb.get(req.oauth2.client.appId, function (error, appObject) { if (error) return sendErrorPageOrRedirect(req, res, 'Invalid request. Unknown app for this client_id.'); - if (!apps.hasAccessTo(appObject, req.oauth2.user)) return sendErrorPageOrRedirect(req, res, 'No access to this app.'); + apps.hasAccessTo(appObject, req.oauth2.user, function (error, access) { + if (error) return sendError(req, res, 'Internal error'); + if (!access) return sendErrorPageOrRedirect(req, res, 'No access to this app.'); - next(); + next(); + }); }); }, gServer.decision({ loadTransaction: false }) diff --git a/src/simpleauth.js b/src/simpleauth.js index 878bea123..0ba50a23c 100644 --- a/src/simpleauth.js +++ b/src/simpleauth.js @@ -45,17 +45,20 @@ function loginLogic(clientId, username, password, callback) { apps.get(clientObject.appId, function (error, appObject) { if (error) return callback(error); - if (!apps.hasAccessTo(appObject, userObject)) return callback(new AppsError(AppsError.ACCESS_DENIED)); - - var accessToken = tokendb.generateToken(); - var expires = Date.now() + 24 * 60 * 60 * 1000; // 1 day - - tokendb.add(accessToken, tokendb.PREFIX_USER + userObject.id, clientId, expires, clientObject.scope, function (error) { + apps.hasAccessTo(appObject, userObject, function (error, access) { if (error) return callback(error); + if (!access) return callback(new AppsError(AppsError.ACCESS_DENIED)); - debug('login: new access token for client %s and user %s: %s', clientId, username, accessToken); + var accessToken = tokendb.generateToken(); + var expires = Date.now() + 24 * 60 * 60 * 1000; // 1 day - callback(null, { accessToken: accessToken, user: userObject }); + tokendb.add(accessToken, tokendb.PREFIX_USER + userObject.id, clientId, expires, clientObject.scope, function (error) { + if (error) return callback(error); + + debug('login: new access token for client %s and user %s: %s', clientId, username, accessToken); + + callback(null, { accessToken: accessToken, user: userObject }); + }); }); }); }); diff --git a/src/test/apps-test.js b/src/test/apps-test.js index 29aefc218..22c12c761 100644 --- a/src/test/apps-test.js +++ b/src/test/apps-test.js @@ -182,24 +182,44 @@ describe('Apps', function () { }); describe('hasAccessTo', function () { - it('returns true for unrestricted access', function () { - expect(apps.hasAccessTo({ accessRestriction: null }, { id: 'someuser' })).to.equal(true); + it('returns true for unrestricted access', function (done) { + apps.hasAccessTo({ accessRestriction: null }, { id: 'someuser' }, function (error, access) { + expect(error).to.be(null); + expect(access).to.be(true); + done(); + }); }); - it('returns true for allowed user', function () { - expect(apps.hasAccessTo({ accessRestriction: { users: [ 'someuser' ] } }, { id: 'someuser' })).to.equal(true); + it('returns true for allowed user', function (done) { + apps.hasAccessTo({ accessRestriction: { users: [ 'someuser' ] } }, { id: 'someuser' }, function (error, access) { + expect(error).to.be(null); + expect(access).to.be(true); + done(); + }); }); - it('returns true for allowed user with multiple allowed', function () { - expect(apps.hasAccessTo({ accessRestriction: { users: [ 'foo', 'someuser', 'anotheruser' ] } }, { id: 'someuser' })).to.equal(true); + it('returns true for allowed user with multiple allowed', function (done) { + apps.hasAccessTo({ accessRestriction: { users: [ 'foo', 'someuser', 'anotheruser' ] } }, { id: 'someuser' }, function (error, access) { + expect(error).to.be(null); + expect(access).to.be(true); + done(); + }); }); - it('returns false for not allowed user', function () { - expect(apps.hasAccessTo({ accessRestriction: { users: [ 'foo' ] } }, { id: 'someuser' })).to.equal(false); + it('returns false for not allowed user', function (done) { + apps.hasAccessTo({ accessRestriction: { users: [ 'foo' ] } }, { id: 'someuser' }, function (error, access) { + expect(error).to.be(null); + expect(access).to.be(false); + done(); + }); }); - it('returns false for not allowed user with multiple allowed', function () { - expect(apps.hasAccessTo({ accessRestriction: { users: [ 'foo', 'anotheruser' ] } }, { id: 'someuser' })).to.equal(false); + it('returns false for not allowed user with multiple allowed', function (done) { + apps.hasAccessTo({ accessRestriction: { users: [ 'foo', 'anotheruser' ] } }, { id: 'someuser' }, function (error, access) { + expect(error).to.be(null); + expect(access).to.be(false); + done(); + }); }); }); });