Skip to content
This repository was archived by the owner on Apr 20, 2018. It is now read-only.

multiple hooks / array of hooks #2

Closed
ksmth opened this issue May 31, 2014 · 8 comments
Closed

multiple hooks / array of hooks #2

ksmth opened this issue May 31, 2014 · 8 comments

Comments

@ksmth
Copy link

ksmth commented May 31, 2014

How about arrays as another variant to declare hooks? It might be obvious that I'm coming from a SailsJS background, but I'm quite fond of the way policies work.

UserService.prototype.before = {
  create : function (data, params, callback) {    
    async.applyEach([isAuthenticated, hasPermissions], data, params, callback);
  }
};

Would be equivalent to:

UserService.prototype.before = {
  create : [isAuthenticated, hasPermissions]
};

For reference: async#applyEach

@daffl
Copy link
Member

daffl commented May 31, 2014

On the actual service you can register as many hooks as you want. Although it's a good idea to also be able to supply an array of hooks on the service object (which would actually be fairly easy).

My idea was that you might want to declare cross-cutting concerns not with the service itself but actually in separate modules that are responsible for it (e.g. an authorization module).

Currently you can set up multiple hooks like this:

var app = feathers().use('/users', userService);

// We need to retrieve the wrapped service object from app which has the added hook functionality
var userService = app.lookup('users');

userService.before({
    ...
});

// Somewhere else
userService.before({
    ...
});

The hooks will run in the order they have been registered.

PS: If you want to share, I'd be very interested in a comparison of SailsJS vs. Feathers.

@ksmth
Copy link
Author

ksmth commented May 31, 2014

Sails polices are basically the same as Express middleware. They follow the signature function (req, res, next) {} and each policy is declared in it's own file, where mentioned function is exported. Then you can compose together rulesets for a given controller action in a config file. It may look like this:

// api/policies/userIsAuthorized.js
module.exports = function (req, res, next) {
    if (req.isAuthorized()) {
        return next()
    }
    res.json({ message: 'not authorized' }, 401);
};

// config/policies.js
module.exports.policies = {
    UserController : {
        '*' : ['userIsAuthorized', 'someOtherPolicy']
    }
};

Getting back to your example, the authorization module, I'd be interested in how a module should know which services or service methods should be protected. You're right that it's not really the services' concern to put an authorization hook into place, but neither is it the authorization module's concern, as it should only provide the authorization logic. I guess there are many ways to go about this, the only question is whether there should be one obvious way to do it or if everyone should come up with a solution that fits their own constraints.

I'm currently thinking along the lines of:

var app = feathers.use('/users', userService);
var userService = app.lookup('users');
var auth = require('feathers-auth');

auth.hookup({
  service : userService,
  before  : ['create', 'update', 'patch', 'remove']
});

Or maybe a plugin:

var app  = feathers.use('/users', userService);
var auth = require('feathers-auth');

app.configure(auth({ 
  'users' : {
    before : ['create', 'update', 'patch', 'remove']
  }
}));

@ksmth
Copy link
Author

ksmth commented May 31, 2014

Regarding the Feathers vs Sails comparison, is there anything specific you'd like me to go into detail? Sails' scope is a bit broader (Asset Linker, ORM, CLI generator), so it might make the most sense to just compare the parts that both frameworks have in common.

@daffl
Copy link
Member

daffl commented May 31, 2014

The easiest thing to implement I think would be:

var app = feathers.use('/users', userService);
var userService = app.lookup('users');
var authorize = require('feathers-auth');

userService.before(authorize('create', 'update', 'patch', 'remove'));

So authorize just returns a hook object based on the parameters you passed that adds authorization. Then I'd drop it into an application module that takes care of it like a plugin (there you can add custom application specific rules as well):

// authorization.js
var authorize = require('feathers-auth'); // or your authorization module

module.exports = function(standardAuth) {
  standardAuth = standardAuth || ['create', 'update', 'patch', 'remove'];

  return function() {
    var app = this;
    var oldSetup = app.setup;

    app.setup = function() {
      oldSetup.apply(this, arguments);
      // Add standard authorization
      _.each(this.services, function(service) {
        service.before(authorize.apply(app, standardAuth));
      });

      // To get user information you always have to be authorized
      app.lookup('users').before(authorize('find', 'get'));

      // Other custom rules here
    }
  }
}

// app.js
var authorization = require('./authorization');
var app = feathers();
// ... set up services
app.configure(authorization());

I was just curious about Sails and I know that people love comparison articles (e.g. Sails vs. Feathers vs. Meteor ;). Hoping that there will be a plugin ecosystem around Feathers that provides additional functionality (we already have hooks, associations and some basic service implementations for in-memory services, MongoDB and Mongoose) and maybe a generator that brings it all together.

@daffl daffl modified the milestone: 0.2.0 May 31, 2014
@marshallswain
Copy link
Member

I personally think that this syntax is awesome for registering plugin functionality:

var authorize = require('authorization-plugin');
userService.before(authorize('create', 'update', 'patch', 'remove'));

I really like the function-syntax that is currently in place. I would like to see it extended to work like this, though:

// Bring in a file that has a bunch of available hooks for me to pick and choose from.
var hooks = require('../hook-library.js');

service.before({
  // non-authenticated users can see records without a userID
  index: [hooks.requireAuthAllowPublic],

  // Auth is required.  No exceptions
  create : [hooks.requireAuth, hooks.setUserID, hooks.setCreatedAt]
});

The two syntaxes could be used together, really. If we didn't need to do slightly-modified auth in the example above, but instead wanted to use the authorization plugin on everything:

var authorize = require('authorization-plugin'),
  hooks = require('../hook-library.js');

// Register the authorize plugin on everything. (So it's first in the queue)
userService.before(authorize('list', 'create', 'update', 'patch', 'remove'));

// Register other custom functionality
service.before({
  create : [hooks.setUserID, hooks.setTimestamps]
});

@ekryski
Copy link
Member

ekryski commented Jul 20, 2014

Ya I agree with @marshallswain. You could you both syntaxes as it will allow the most flexibility but I think I prefer this one:

// Bring in a file that has a bunch of available hooks for me to pick and choose from.
var hooks = require('../hook-library.js');

service.before({
  // non-authenticated users can see records without a userID
  index: [hooks.requireAuthAllowPublic],

  // Auth is required.  No exceptions
  create : [hooks.requireAuth, hooks.setUserID, hooks.setCreatedAt]
});

@ekryski
Copy link
Member

ekryski commented Jul 20, 2014

I've closed the issue as you can now register multiple hooks using the array syntax documented in the README. If we want the plugin-style syntax maybe we can re-open the issue or just create a new issue for it that references this one.

@marshallswain
Copy link
Member

Awesome!

Have a great day!

Marshall Thompson

On July 20, 2014 at 1:50:19 PM, Eric Kryski ([email protected]) wrote:

I've closed the issue as you can now register multiple hooks using the array syntax documented in the README. If we want the plugin-style syntax maybe we can re-open the issue or just create a new issue for it that references this one.


Reply to this email directly or view it on GitHub.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants