Code cleanup

Make it easier to add method/app-level scopes in the future

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 || '<default>');
-      debug('Scope required:', accessScope || '<default>');
+      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 = [];

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());

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);

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: []});
 

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);