From 5c04712efc5f0259345d0c13f94d8f5d4e3f015b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 27 Mar 2017 14:58:01 +0200 Subject: [PATCH 1/3] Add support for scoped access tokens Define a new property `AccessToken.scopes` to contain the list of scopes granted to this access token. Define a new remote method metadata `accessScopes` to contain a list of scope name required by this method. Define a special built-in scope name "DEFAULT" that's used when a method/token does not provide any scopes. This allows access tokens to grant access to both the default scope and any additional custom scopes at the same time. Modify the authorization algorithm to ensure that at least one of the scopes required by a remote method is allowed by the scopes granted to the requesting access token. The "DEFAULT" scope preserve backwards compatibility because existing remote methods with no `accessScopes` can be accessed by (existing) access tokens with no `scopes` defined. Impact on existing applications: - Database schema must be updated after upgrading the loopback version - If the application was already using a custom `AccessToken.scopes` property with a type different from an array, then the relevant code must be updated to work with the new type "array of strings". --- common/models/access-token.json | 4 + common/models/acl.js | 12 +++ lib/access-context.js | 31 +++++++ lib/model.js | 1 + test/acl.test.js | 1 + test/authorization-scopes.test.js | 130 ++++++++++++++++++++++++++++++ test/user.test.js | 6 +- 7 files changed, 182 insertions(+), 3 deletions(-) create mode 100644 test/authorization-scopes.test.js diff --git a/common/models/access-token.json b/common/models/access-token.json index 85fdf910e..9e3d4d464 100644 --- a/common/models/access-token.json +++ b/common/models/access-token.json @@ -11,6 +11,10 @@ "default": 1209600, "description": "time to live in seconds (2 weeks by default)" }, + "scopes": { + "type": ["string"], + "description": "Array of scopes granted to this access token." + }, "created": { "type": "Date", "defaultFn": "now" diff --git a/common/models/acl.js b/common/models/acl.js index e849312fb..8fc3ba285 100644 --- a/common/models/acl.js +++ b/common/models/acl.js @@ -256,6 +256,7 @@ module.exports = function(ACL) { model: req.model, property: req.property, accessType: req.accessType, + accessScope: req.accessScope, permission: permission || ACL.DEFAULT, registry: this.registry}); @@ -443,6 +444,7 @@ module.exports = function(ACL) { var modelDefaultPermission = model && model.settings.defaultPermission; var property = context.property; var accessType = context.accessType; + var accessScope = context.accessScope; var modelName = context.modelName; var methodNames = context.methodNames; @@ -458,10 +460,20 @@ module.exports = function(ACL) { model: modelName, property, accessType, + accessScope, permission: ACL.DEFAULT, methodNames, registry: this.registry}); + if (!context.isScopeAllowed()) { + req.permission = ACL.DENY; + debug('--Denied by scope config--'); + debug('Scopes allowed:', context.accessToken.scopes || ''); + debug('Scope required:', accessScope || ''); + context.debug(); + return callback(null, req); + } + var effectiveACLs = []; var staticACLs = self.getStaticACLs(model.modelName, property); diff --git a/lib/access-context.js b/lib/access-context.js index 156620346..cd466f584 100644 --- a/lib/access-context.js +++ b/lib/access-context.js @@ -32,6 +32,8 @@ var debug = require('debug')('loopback:security:access-context'); * @property {String} property The model property/method/relation name * @property {String} method The model method to be invoked * @property {String} accessType The access type: READ, REPLICATE, WRITE, or EXECUTE. + * @property {String[]} accessScopes The access scopes that can authorize + * access to the invoked resource. * @property {AccessToken} accessToken The access token resolved for the request * @property {RemotingContext} remotingContext The request's remoting context * @property {Registry} registry The application or global registry @@ -70,6 +72,8 @@ function AccessContext(context) { } this.accessType = context.accessType || AccessContext.ALL; + this.accessScopes = context.accessScopes; + assert(loopback.AccessToken, 'AccessToken model must be defined before AccessContext model'); this.accessToken = context.accessToken || loopback.AccessToken.ANONYMOUS; @@ -193,6 +197,28 @@ AccessContext.prototype.isAuthenticated = function() { return !!(this.getUserId() || this.getAppId()); }; +/** + * Check if the scope required by the remote method is allowed + * by the scopes granted to the requesting access token. + * @return {boolean} + */ +AccessContext.prototype.isScopeAllowed = function() { + if (!this.accessToken) return false; + + // For backwards compatibility, tokens with no scopes are treated + // as if they have "DEFAULT" scope granted + const tokenScopes = this.accessToken.scopes || ['DEFAULT']; + + // For backwards compatibility, methods with no scopes defined + // are assigned a single "DEFAULT" scope + const resourceScopes = this.accessScopes || ['DEFAULT']; + + // Scope is allowed when at least one of token's scopes + // is found in method's (resource's) scopes. + return Array.isArray(tokenScopes) && Array.isArray(resourceScopes) && + resourceScopes.some(s => tokenScopes.indexOf(s) !== -1); +}; + /*! * Print debug info for access context. */ @@ -213,10 +239,12 @@ AccessContext.prototype.debug = function() { debug('property %s', this.property); debug('method %s', this.method); debug('accessType %s', this.accessType); + debug('accessScopes %s', this.accessScopes || ['DEFAULT']); if (this.accessToken) { debug('accessToken:'); debug(' id %j', this.accessToken.id); debug(' ttl %j', this.accessToken.ttl); + debug(' scopes', this.accessToken.scopes || ['DEFAULT']); } debug('getUserId() %s', this.getUserId()); debug('isAuthenticated() %s', this.isAuthenticated()); @@ -271,6 +299,7 @@ Principal.prototype.equals = function(p) { * or an AccessRequest instance/object. * @param {String} property The property/method/relation name * @param {String} accessType The access type + * @property {String} accessScope The access scope required by the invoked method. * @param {String} permission The requested permission * @param {String[]} methodNames The names of involved methods * @param {Registry} registry The application or global registry @@ -286,6 +315,7 @@ function AccessRequest(model, property, accessType, permission, methodNames, reg this.model = obj.model || AccessContext.ALL; this.property = obj.property || AccessContext.ALL; this.accessType = obj.accessType || AccessContext.ALL; + this.accessScope = obj.accessScope; this.permission = obj.permission || AccessContext.DEFAULT; this.methodNames = obj.methodNames || []; this.registry = obj.registry; @@ -370,6 +400,7 @@ AccessRequest.prototype.debug = function() { debug(' model %s', this.model); debug(' property %s', this.property); debug(' accessType %s', this.accessType); + debug(' accessScopes %s', this.accessScopes); debug(' permission %s', this.permission); debug(' isWildcard() %s', this.isWildcard()); debug(' isAllowed() %s', this.isAllowed()); diff --git a/lib/model.js b/lib/model.js index f7633b4df..16864a944 100644 --- a/lib/model.js +++ b/lib/model.js @@ -347,6 +347,7 @@ module.exports = function(registry) { sharedMethod: sharedMethod, modelId: modelId, accessType: this._getAccessTypeForMethod(sharedMethod), + accessScopes: sharedMethod.accessScopes, remotingContext: ctx, }, function(err, accessRequest) { if (err) return callback(err); diff --git a/test/acl.test.js b/test/acl.test.js index 15cd87065..0595eba40 100644 --- a/test/acl.test.js +++ b/test/acl.test.js @@ -165,6 +165,7 @@ describe('security ACLs', function() { assert.deepEqual(perm, {model: 'account', property: 'find', accessType: 'WRITE', + accessScope: undefined, permission: 'ALLOW', methodNames: []}); diff --git a/test/authorization-scopes.test.js b/test/authorization-scopes.test.js new file mode 100644 index 000000000..c5657811b --- /dev/null +++ b/test/authorization-scopes.test.js @@ -0,0 +1,130 @@ +'use strict'; + +const loopback = require('../'); +const supertest = require('supertest'); +const strongErrorHandler = require('strong-error-handler'); + +describe('Authorization scopes', () => { + const CUSTOM_SCOPE = 'read:custom'; + + let app, request, User, testUser, regularToken, scopedToken; + beforeEach(givenAppAndRequest); + beforeEach(givenRemoteMethodWithCustomScope); + beforeEach(givenUser); + beforeEach(givenDefaultToken); + beforeEach(givenScopedToken); + + it('denies regular token to invoke custom-scoped method', () => { + logServerErrorsOtherThan(401); + return request.get('/users/scoped') + .set('Authorization', regularToken.id) + .expect(401); + }); + + it('allows regular tokens to invoke default-scoped method', () => { + logAllServerErrors(); + return request.get('/users/' + testUser.id) + .set('Authorization', regularToken.id) + .expect(200); + }); + + it('allows scoped token to invoke custom-scoped method', () => { + logAllServerErrors(); + return request.get('/users/scoped') + .set('Authorization', scopedToken.id) + .expect(204); + }); + + it('denies scoped token to invoke default-scoped method', () => { + logServerErrorsOtherThan(401); + return request.get('/users/' + testUser.id) + .set('Authorization', scopedToken.id) + .expect(401); + }); + + describe('token granted both default and custom scope', () => { + beforeEach('given token with default and custom scope', + () => givenScopedToken(['DEFAULT', CUSTOM_SCOPE])); + beforeEach(logAllServerErrors); + + it('allows invocation of default-scoped method', () => { + return request.get('/users/' + testUser.id) + .set('Authorization', scopedToken.id) + .expect(200); + }); + + it('allows invocation of custom-scoped method', () => { + return request.get('/users/scoped') + .set('Authorization', scopedToken.id) + .expect(204); + }); + }); + + it('allows invocation when at least one method scope is matched', () => { + givenRemoteMethodWithCustomScope(['read', 'write']); + givenScopedToken(['read', 'execute']).then(() => { + return request.get('/users/scoped') + .set('Authorization', scopedToken.id) + .expect(204); + }); + }); + + function givenAppAndRequest() { + app = loopback({localRegistry: true, loadBuiltinModels: true}); + app.set('remoting', {rest: {handleErrors: false}}); + app.dataSource('db', {connector: 'memory'}); + app.enableAuth({dataSource: 'db'}); + request = supertest(app); + + app.use(loopback.rest()); + + User = app.models.User; + } + + function givenRemoteMethodWithCustomScope() { + const accessScopes = arguments[0] || [CUSTOM_SCOPE]; + User.scoped = function(cb) { cb(); }; + User.remoteMethod('scoped', { + accessScopes, + http: {verb: 'GET', path: '/scoped'}, + }); + User.settings.acls.push({ + principalType: 'ROLE', + principalId: '$authenticated', + permission: 'ALLOW', + property: 'scoped', + accessType: 'EXECUTE', + }); + } + + function givenUser() { + return User.create({email: 'test@example.com', password: 'pass'}) + .then(u => testUser = u); + } + + function givenDefaultToken() { + return testUser.createAccessToken(60) + .then(t => regularToken = t); + } + + function givenScopedToken() { + const scopes = arguments[0] || [CUSTOM_SCOPE]; + return testUser.accessTokens.create({ttl: 60, scopes}) + .then(t => scopedToken = t); + } + + function logAllServerErrors() { + logServerErrorsOtherThan(-1); + } + + function logServerErrorsOtherThan(statusCode) { + app.use((err, req, res, next) => { + if ((err.statusCode || 500) !== statusCode) { + console.log('Unhandled error for request %s %s: %s', + req.method, req.url, err.stack || err); + } + res.statusCode = err.statusCode || 500; + res.json(err); + }); + } +}); diff --git a/test/user.test.js b/test/user.test.js index 1004a348b..e7da06ec1 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -624,18 +624,18 @@ describe('User', function() { // Override createAccessToken User.prototype.createAccessToken = function(ttl, options, cb) { // Reduce the ttl by half for testing purpose - this.accessTokens.create({ttl: ttl / 2, scopes: options.scope}, cb); + this.accessTokens.create({ttl: ttl / 2, scopes: [options.scope]}, cb); }; User.login(validCredentialsWithTTLAndScope, function(err, accessToken) { assertGoodToken(accessToken, validCredentialsUser); assert.equal(accessToken.ttl, 1800); - assert.equal(accessToken.scopes, 'all'); + assert.deepEqual(accessToken.scopes, ['all']); User.findById(accessToken.userId, function(err, user) { user.createAccessToken(120, {scope: 'default'}, function(err, accessToken) { assertGoodToken(accessToken, validCredentialsUser); assert.equal(accessToken.ttl, 60); - assert.equal(accessToken.scopes, 'default'); + assert.deepEqual(accessToken.scopes, ['default']); // Restore create access token User.prototype.createAccessToken = createToken; From fab857dd5fd1c16a40d5b4dee9847cf9c12841d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Thu, 6 Apr 2017 13:37:17 +0200 Subject: [PATCH 2/3] Code cleanup Make it easier to add method/app-level scopes in the future --- common/models/acl.js | 10 ++++----- lib/access-context.js | 37 +++++++++++++++++++++---------- lib/model.js | 1 - test/acl.test.js | 1 - test/authorization-scopes.test.js | 2 +- 5 files changed, 30 insertions(+), 21 deletions(-) diff --git a/common/models/acl.js b/common/models/acl.js index 8fc3ba285..10013d1d4 100644 --- a/common/models/acl.js +++ b/common/models/acl.js @@ -256,7 +256,6 @@ module.exports = function(ACL) { model: req.model, property: req.property, accessType: req.accessType, - accessScope: req.accessScope, permission: permission || ACL.DEFAULT, registry: this.registry}); @@ -444,7 +443,6 @@ module.exports = function(ACL) { var modelDefaultPermission = model && model.settings.defaultPermission; var property = context.property; var accessType = context.accessType; - var accessScope = context.accessScope; var modelName = context.modelName; var methodNames = context.methodNames; @@ -460,7 +458,6 @@ module.exports = function(ACL) { model: modelName, property, accessType, - accessScope, permission: ACL.DEFAULT, methodNames, registry: this.registry}); @@ -468,10 +465,11 @@ module.exports = function(ACL) { if (!context.isScopeAllowed()) { req.permission = ACL.DENY; debug('--Denied by scope config--'); - debug('Scopes allowed:', context.accessToken.scopes || ''); - debug('Scope required:', accessScope || ''); + debug('Scopes allowed:', context.accessToken.scopes || 'DEFAULT'); + debug('Scope required:', context.getScopes() || 'DEFAULT'); context.debug(); - return callback(null, req); + callback(null, req); + return callback.promise; } var effectiveACLs = []; diff --git a/lib/access-context.js b/lib/access-context.js index cd466f584..1e752edf9 100644 --- a/lib/access-context.js +++ b/lib/access-context.js @@ -8,6 +8,8 @@ var assert = require('assert'); var loopback = require('./loopback'); var debug = require('debug')('loopback:security:access-context'); +const DEFAULT_SCOPES = ['DEFAULT']; + /** * Access context represents the context for a request to access protected * resources @@ -32,8 +34,6 @@ var debug = require('debug')('loopback:security:access-context'); * @property {String} property The model property/method/relation name * @property {String} method The model method to be invoked * @property {String} accessType The access type: READ, REPLICATE, WRITE, or EXECUTE. - * @property {String[]} accessScopes The access scopes that can authorize - * access to the invoked resource. * @property {AccessToken} accessToken The access token resolved for the request * @property {RemotingContext} remotingContext The request's remoting context * @property {Registry} registry The application or global registry @@ -72,7 +72,6 @@ function AccessContext(context) { } this.accessType = context.accessType || AccessContext.ALL; - this.accessScopes = context.accessScopes; assert(loopback.AccessToken, 'AccessToken model must be defined before AccessContext model'); @@ -197,6 +196,25 @@ AccessContext.prototype.isAuthenticated = function() { return !!(this.getUserId() || this.getAppId()); }; +/** + * Get the list of scopes required by the current access context. + */ +AccessContext.prototype.getScopes = function() { + if (!this.sharedMethod) + return DEFAULT_SCOPES; + + // For backwards compatibility, methods with no scopes defined + // are assigned a single "DEFAULT" scope + const methodLevel = this.sharedMethod.accessScopes || DEFAULT_SCOPES; + + // TODO add model-level and app-level scopes + + debug('--Context scopes of %s()--', this.sharedMethod.stringName); + debug(' method-level: %j', methodLevel); + + return methodLevel; +}; + /** * Check if the scope required by the remote method is allowed * by the scopes granted to the requesting access token. @@ -207,11 +225,9 @@ AccessContext.prototype.isScopeAllowed = function() { // For backwards compatibility, tokens with no scopes are treated // as if they have "DEFAULT" scope granted - const tokenScopes = this.accessToken.scopes || ['DEFAULT']; + const tokenScopes = this.accessToken.scopes || DEFAULT_SCOPES; - // For backwards compatibility, methods with no scopes defined - // are assigned a single "DEFAULT" scope - const resourceScopes = this.accessScopes || ['DEFAULT']; + const resourceScopes = this.getScopes(); // Scope is allowed when at least one of token's scopes // is found in method's (resource's) scopes. @@ -239,12 +255,12 @@ AccessContext.prototype.debug = function() { debug('property %s', this.property); debug('method %s', this.method); debug('accessType %s', this.accessType); - debug('accessScopes %s', this.accessScopes || ['DEFAULT']); + debug('accessScopes %j', this.getScopes()); if (this.accessToken) { debug('accessToken:'); debug(' id %j', this.accessToken.id); debug(' ttl %j', this.accessToken.ttl); - debug(' scopes', this.accessToken.scopes || ['DEFAULT']); + debug(' scopes %j', this.accessToken.scopes || DEFAULT_SCOPES); } debug('getUserId() %s', this.getUserId()); debug('isAuthenticated() %s', this.isAuthenticated()); @@ -299,7 +315,6 @@ Principal.prototype.equals = function(p) { * or an AccessRequest instance/object. * @param {String} property The property/method/relation name * @param {String} accessType The access type - * @property {String} accessScope The access scope required by the invoked method. * @param {String} permission The requested permission * @param {String[]} methodNames The names of involved methods * @param {Registry} registry The application or global registry @@ -315,7 +330,6 @@ function AccessRequest(model, property, accessType, permission, methodNames, reg this.model = obj.model || AccessContext.ALL; this.property = obj.property || AccessContext.ALL; this.accessType = obj.accessType || AccessContext.ALL; - this.accessScope = obj.accessScope; this.permission = obj.permission || AccessContext.DEFAULT; this.methodNames = obj.methodNames || []; this.registry = obj.registry; @@ -400,7 +414,6 @@ AccessRequest.prototype.debug = function() { debug(' model %s', this.model); debug(' property %s', this.property); debug(' accessType %s', this.accessType); - debug(' accessScopes %s', this.accessScopes); debug(' permission %s', this.permission); debug(' isWildcard() %s', this.isWildcard()); debug(' isAllowed() %s', this.isAllowed()); diff --git a/lib/model.js b/lib/model.js index 16864a944..f7633b4df 100644 --- a/lib/model.js +++ b/lib/model.js @@ -347,7 +347,6 @@ module.exports = function(registry) { sharedMethod: sharedMethod, modelId: modelId, accessType: this._getAccessTypeForMethod(sharedMethod), - accessScopes: sharedMethod.accessScopes, remotingContext: ctx, }, function(err, accessRequest) { if (err) return callback(err); diff --git a/test/acl.test.js b/test/acl.test.js index 0595eba40..15cd87065 100644 --- a/test/acl.test.js +++ b/test/acl.test.js @@ -165,7 +165,6 @@ describe('security ACLs', function() { assert.deepEqual(perm, {model: 'account', property: 'find', accessType: 'WRITE', - accessScope: undefined, permission: 'ALLOW', methodNames: []}); diff --git a/test/authorization-scopes.test.js b/test/authorization-scopes.test.js index c5657811b..797cbd48f 100644 --- a/test/authorization-scopes.test.js +++ b/test/authorization-scopes.test.js @@ -62,7 +62,7 @@ describe('Authorization scopes', () => { it('allows invocation when at least one method scope is matched', () => { givenRemoteMethodWithCustomScope(['read', 'write']); - givenScopedToken(['read', 'execute']).then(() => { + return givenScopedToken(['read', 'execute']).then(() => { return request.get('/users/scoped') .set('Authorization', scopedToken.id) .expect(204); From a035db9624add61c099141d0b6ee071cd0c71c7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Wed, 29 Mar 2017 14:23:26 +0200 Subject: [PATCH 3/3] Support scopes defined via model settings --- lib/access-context.js | 22 ++++++++++++++--- test/authorization-scopes.test.js | 39 +++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/lib/access-context.js b/lib/access-context.js index 1e752edf9..ce5e2381a 100644 --- a/lib/access-context.js +++ b/lib/access-context.js @@ -207,12 +207,28 @@ AccessContext.prototype.getScopes = function() { // are assigned a single "DEFAULT" scope const methodLevel = this.sharedMethod.accessScopes || DEFAULT_SCOPES; - // TODO add model-level and app-level scopes + // TODO app-level scopes + const modelSettings = this.sharedMethod.sharedClass.ctor.settings || {}; + const modelScopes = modelSettings.accessScopes || {}; + + const allMethodNames = [this.sharedMethod.name] + .concat(this.sharedMethod.aliases) + .map(n => (this.sharedMethod.isStatic ? '' : 'prototype.') + n); debug('--Context scopes of %s()--', this.sharedMethod.stringName); debug(' method-level: %j', methodLevel); - - return methodLevel; + debug(' model-level:'); + let modelLevel = []; + allMethodNames.forEach(alias => { + const scopes = modelScopes[alias]; + if (!scopes) return; + modelLevel = modelLevel.concat(scopes); + debug(' - %s: %j', alias, scopes); + }); + if (!modelLevel.length) + debug(' (empty)'); + + return methodLevel.concat(modelLevel); }; /** diff --git a/test/authorization-scopes.test.js b/test/authorization-scopes.test.js index 797cbd48f..f3036d10f 100644 --- a/test/authorization-scopes.test.js +++ b/test/authorization-scopes.test.js @@ -69,6 +69,45 @@ describe('Authorization scopes', () => { }); }); + describe('scope config defined at model-level', () => { + beforeEach(logAllServerErrors); + + it('hounours scope defined for method name', () => { + User.settings.accessScopes = { + findById: ['read'], + }; + + return givenScopedToken(['read']).then(() => { + return request.get('/users/' + testUser.id) + .set('Authorization', scopedToken.id) + .expect(200); + }); + }); + + it('honours scope defined for method alias', () => { + User.settings.accessScopes = { + 'prototype.updateAttributes': ['write'], + }; + + return givenScopedToken(['write']).then(() => { + return request.patch('/users/' + testUser.id) + .send({username: 'test-user'}) + .set('Authorization', scopedToken.id) + .expect(200); + }); + }); + + it('adds model-level scopes to method-level scopes', () => { + User.settings.accessScopes = { + findById: ['read'], + }; + + return request.get('/users/' + testUser.id) + .set('Authorization', regularToken.id) + .expect(200); + }); + }); + function givenAppAndRequest() { app = loopback({localRegistry: true, loadBuiltinModels: true}); app.set('remoting', {rest: {handleErrors: false}});