diff --git a/src/accesscontrol.js b/src/accesscontrol.js index b8c90a99b..ec3678936 100644 --- a/src/accesscontrol.js +++ b/src/accesscontrol.js @@ -69,13 +69,11 @@ function validateScope(scope) { } // tests if all requiredScopes are attached to the request -function hasScopes(authInfo, requiredScopes) { - assert.strictEqual(typeof authInfo, 'object'); +function hasScopes(authorizedScope, requiredScopes) { + assert.strictEqual(typeof authorizedScope, 'string'); assert(Array.isArray(requiredScopes), 'Expecting array'); - if (!authInfo || !authInfo.scope) return new Error('No scope found'); - - var scopes = authInfo.scope.split(','); + var scopes = authorizedScope.split(','); if (scopes.indexOf(exports.SCOPE_ANY) !== -1) return null; diff --git a/src/routes/accesscontrol.js b/src/routes/accesscontrol.js index 49b6a31a5..fa1e52341 100644 --- a/src/routes/accesscontrol.js +++ b/src/routes/accesscontrol.js @@ -97,11 +97,11 @@ function accessTokenAuth(accessToken, callback) { assert.strictEqual(typeof callback, 'function'); tokendb.get(accessToken, function (error, token) { - if (error && error.reason === DatabaseError.NOT_FOUND) return callback(null, false); - if (error) return callback(error); + if (error && error.reason === DatabaseError.NOT_FOUND) return callback(null, null /* user */, 'Invalid Token'); // will end up as a 401 + if (error) return callback(error); // this triggers 'internal error' in passport users.get(token.identifier, function (error, user) { - if (error && error.reason === UsersError.NOT_FOUND) return callback(null, false); + if (error && error.reason === UsersError.NOT_FOUND) return callback(null, null /* user */, 'Invalid Token'); // will end up as a 401 if (error) return callback(error); // scopes here can define what capabilities that token carries @@ -110,7 +110,7 @@ function accessTokenAuth(accessToken, callback) { var scope = accesscontrol.intersectScope(userScope, token.scope); // these clients do not require password checks unlike UI const skipPasswordVerification = token.clientId === 'cid-sdk' || token.clientId === 'cid-cli'; - var info = { scope: scope, skipPasswordVerification: skipPasswordVerification }; + var info = { authorizedScope: scope, skipPasswordVerification: skipPasswordVerification }; callback(null, user, info); }); @@ -135,7 +135,9 @@ function scope(requiredScope) { passport.authenticate(['bearer'], { session: false }), function (req, res, next) { - var error = accesscontrol.hasScopes(req.authInfo || null, requiredScopes); + assert(req.authInfo && typeof req.authInfo === 'object'); + + var error = accesscontrol.hasScopes(req.authInfo.authorizedScope, requiredScopes); if (error) return next(new HttpError(403, error.message)); next(); @@ -153,9 +155,8 @@ function websocketAuth(requiredScopes, req, res, next) { if (!user) return next(new HttpError(401, 'Unauthorized')); req.user = user; - req.authInfo = info; - var e = accesscontrol.hasScopes(req.authInfo, requiredScopes); + var e = accesscontrol.hasScopes(info.authorizedScope, requiredScopes); if (e) return next(new HttpError(403, e.message)); next(); diff --git a/src/routes/profile.js b/src/routes/profile.js index 6384eb5ce..542872907 100644 --- a/src/routes/profile.js +++ b/src/routes/profile.js @@ -9,8 +9,7 @@ exports = module.exports = { disableTwoFactorAuthentication: disableTwoFactorAuthentication }; -var accesscontrol = require('../accesscontrol.js'), - assert = require('assert'), +var assert = require('assert'), HttpError = require('connect-lastmile').HttpError, HttpSuccess = require('connect-lastmile').HttpSuccess, users = require('../users.js'), @@ -31,7 +30,7 @@ function get(req, res, next) { email: req.user.email, fallbackEmail: req.user.fallbackEmail, admin: req.user.admin, - scope: accesscontrol.canonicalScope(req.authInfo.scope), // this returns the token scope and not the user's scope + tokenScope: req.authInfo.authorizedScope, displayName: req.user.displayName, twoFactorAuthenticationEnabled: req.user.twoFactorAuthenticationEnabled })); diff --git a/src/routes/test/accesscontrol-test.js b/src/routes/test/accesscontrol-test.js index a12f335c9..824a603fb 100644 --- a/src/routes/test/accesscontrol-test.js +++ b/src/routes/test/accesscontrol-test.js @@ -25,29 +25,9 @@ describe('scopes middleware', function () { passport.authenticate = passportAuthenticateSave; }); - it('fails due to missing authInfo', function (done) { + it('fails due to empty scope in request', function (done) { var mw = accesscontrol.scope('admin')[1]; - var req = {}; - - mw(req, null, function (error) { - expect(error).to.be.a(HttpError); - done(); - }); - }); - - it('fails due to missing scope property in authInfo', function (done) { - var mw = accesscontrol.scope('admin')[1]; - var req = { authInfo: {} }; - - mw(req, null, function (error) { - expect(error).to.be.a(HttpError); - done(); - }); - }); - - it('fails due to missing scope in request', function (done) { - var mw = accesscontrol.scope('admin')[1]; - var req = { authInfo: { scope: '' } }; + var req = { authInfo: { authorizedScope: '' } }; mw(req, null, function (error) { expect(error).to.be.a(HttpError); @@ -57,7 +37,7 @@ describe('scopes middleware', function () { it('fails due to wrong scope in request', function (done) { var mw = accesscontrol.scope('admin')[1]; - var req = { authInfo: { scope: 'foobar,something' } }; + var req = { authInfo: { authorizedScope: 'foobar,something' } }; mw(req, null, function (error) { expect(error).to.be.a(HttpError); @@ -67,7 +47,7 @@ describe('scopes middleware', function () { it('fails due to wrong scope in request', function (done) { var mw = accesscontrol.scope('admin,users')[1]; - var req = { authInfo: { scope: 'foobar,admin' } }; + var req = { authInfo: { authorizedScope: 'foobar,admin' } }; mw(req, null, function (error) { expect(error).to.be.a(HttpError); @@ -77,7 +57,7 @@ describe('scopes middleware', function () { it('succeeds with one requested scope and one provided scope', function (done) { var mw = accesscontrol.scope('admin')[1]; - var req = { authInfo: { scope: 'admin' } }; + var req = { authInfo: { authorizedScope: 'admin' } }; mw(req, null, function (error) { expect(error).to.not.be.ok(); @@ -87,7 +67,7 @@ describe('scopes middleware', function () { it('succeeds with one requested scope and two provided scopes', function (done) { var mw = accesscontrol.scope('admin')[1]; - var req = { authInfo: { scope: 'foobar,admin' } }; + var req = { authInfo: { authorizedScope: 'foobar,admin' } }; mw(req, null, function (error) { expect(error).to.not.be.ok(); @@ -97,7 +77,7 @@ describe('scopes middleware', function () { it('succeeds with two requested scope and two provided scopes', function (done) { var mw = accesscontrol.scope('admin,foobar')[1]; - var req = { authInfo: { scope: 'foobar,admin' } }; + var req = { authInfo: { authorizedScope: 'foobar,admin' } }; mw(req, null, function (error) { expect(error).to.not.be.ok(); @@ -107,7 +87,7 @@ describe('scopes middleware', function () { it('succeeds with two requested scope and provided wildcard scope', function (done) { var mw = accesscontrol.scope('admin,foobar')[1]; - var req = { authInfo: { scope: '*' } }; + var req = { authInfo: { authorizedScope: '*' } }; mw(req, null, function (error) { expect(error).to.not.be.ok(); diff --git a/src/routes/test/profile-test.js b/src/routes/test/profile-test.js index f508b5e8e..1cffaf84b 100644 --- a/src/routes/test/profile-test.js +++ b/src/routes/test/profile-test.js @@ -106,6 +106,7 @@ describe('Profile API', function () { expect(result.body.displayName).to.be.a('string'); expect(result.body.password).to.not.be.ok(); expect(result.body.salt).to.not.be.ok(); + expect(result.body.tokenScope).to.be('apps,clients,cloudron,domains,mail,profile,settings,users'); user_0 = result.body; @@ -144,6 +145,7 @@ describe('Profile API', function () { expect(result.body.displayName).to.be.a('string'); expect(result.body.password).to.not.be.ok(); expect(result.body.salt).to.not.be.ok(); + expect(result.body.tokenScope).to.be('apps,clients,cloudron,domains,mail,profile,settings,users'); done(); }); }); diff --git a/src/test/accesscontrol-test.js b/src/test/accesscontrol-test.js index 9d981c025..9b822721b 100644 --- a/src/test/accesscontrol-test.js +++ b/src/test/accesscontrol-test.js @@ -63,23 +63,23 @@ describe('access control', function () { describe('hasScopes', function () { it('succeeds if it contains the scope', function () { - expect(accesscontrol.hasScopes({ scope: 'apps' }, [ 'apps' ])).to.be(null); - expect(accesscontrol.hasScopes({ scope: 'apps,mail' }, [ 'mail' ])).to.be(null); - expect(accesscontrol.hasScopes({ scope: 'clients,*,apps,mail' }, [ 'mail' ])).to.be(null); + expect(accesscontrol.hasScopes('apps', [ 'apps' ])).to.be(null); + expect(accesscontrol.hasScopes('apps,mail', [ 'mail' ])).to.be(null); + expect(accesscontrol.hasScopes('clients,*,apps,mail', [ 'mail' ])).to.be(null); // subscope - expect(accesscontrol.hasScopes({ scope: 'apps' }, [ 'apps:read' ])).to.be(null); - expect(accesscontrol.hasScopes({ scope: 'apps:read' }, [ 'apps:read' ])).to.be(null); - expect(accesscontrol.hasScopes({ scope: 'apps,mail' }, [ 'apps:*' ])).to.be(null); - expect(accesscontrol.hasScopes({ scope: '*' }, [ 'apps:read' ])).to.be(null); + expect(accesscontrol.hasScopes('apps', [ 'apps:read' ])).to.be(null); + expect(accesscontrol.hasScopes('apps:read', [ 'apps:read' ])).to.be(null); + expect(accesscontrol.hasScopes('apps,mail', [ 'apps:*' ])).to.be(null); + expect(accesscontrol.hasScopes('*', [ 'apps:read' ])).to.be(null); }); it('fails if it does not contain the scope', function () { - expect(accesscontrol.hasScopes({ scope: 'apps' }, [ 'mail' ])).to.be.an(Error); - expect(accesscontrol.hasScopes({ scope: 'apps,mail' }, [ 'clients' ])).to.be.an(Error); + expect(accesscontrol.hasScopes('apps', [ 'mail' ])).to.be.an(Error); + expect(accesscontrol.hasScopes('apps,mail', [ 'clients' ])).to.be.an(Error); // subscope - expect(accesscontrol.hasScopes({ scope: 'apps:write' }, [ 'apps:read' ])).to.be.an(Error); + expect(accesscontrol.hasScopes('apps:write', [ 'apps:read' ])).to.be.an(Error); }); }); diff --git a/src/users.js b/src/users.js index 9d7b1b8a0..3a20771d0 100644 --- a/src/users.js +++ b/src/users.js @@ -329,7 +329,6 @@ function get(userId, callback) { result.groupIds = groupIds; result.admin = groupIds.indexOf(constants.ADMIN_GROUP_ID) !== -1; - result.scope = result.admin ? '*' : 'profile'; return callback(null, result); });