outmoded / discuss

The "mailing list"
99 stars 9 forks source link

Pre handlers - What are you using them for? #66

Closed adrianblynch closed 5 years ago

adrianblynch commented 9 years ago

My colleagues and I are having a conversation about how we use pre handlers.

The API we're working on has permissions on the various entities.

The pre handlers do things like check you have the right level of access on an entity for the action you're trying to do. So something like:

GET /plans/{id} Pre handler: plan.hasAccessLevel('viewer')

Or:

PUT /plans/{id} > {name: 'New name'} Pre handler: plan.hasAccessLevel('editor')

The problem as I see it is, the logic in hasAccessLevel is reliant on the request. Mostly the {id} parameter. This feels wrong to me.

I feel like pre handlers should work on very generic things rather than business logic which belongs in the route handler or the libraries we're creating away from Hapi.

So the question is, what are you guys using your pre handlers for?

hueniverse commented 9 years ago

I added pre support to make handlers tiny. I think about them as just an array of handlers and the handler itself as the last one in the chain. Beyond that, do what works for you.

adrianblynch commented 9 years ago

Thanks @hueniverse

Can I ask if there's a reason for pre and handler being separate, namely does anything happen to the request maybe, between the two?

I like the idea of an array of handlers, run in series or parallel. Would there be anything wrong with baking that into the handler and dropping the pre?

Hearing why you added it has helped in our design now. So thanks for that!

hueniverse commented 9 years ago

At one point I wanted to combine the two into a single handlers array but because pre allows parallel execution, it was hard to do cleanly with the requirement that at the end, there has to be one finalizer.

adrianblynch commented 9 years ago

I thought that might be the case. handler is then the final, definitive place to handle the request.

Having to make sure you have a serial final function would cause confusion:

{
    handlers: [
        [
            para1,
            para2,
        ]
        serial1,
        [
            para3,
            para4
        ]
        finalSerial
    ]
}

We're still working out how best to use pre but cheers for the explanations.

Anyone else using pre handlers, please add how you're using them as it'll be interesting to see.

ericeslinger commented 9 years ago

I use pre-handlers (and if this is an anti-pattern, please let me know, it's just what came naturally) to decompose stuff that happens in the run up to the handler itself. For something like a "view this RESTful item" I do:

pre:
  figure out what item to load and put it in a common place on request
  take the item from request, call item.authorize(level, user)
handler:
  send item as JSON

I do similar things for PUT, POST, and DELETE, but the key idea is basically: all the model objects have an authorize(level, user) method that returns true or false, all my REST requests need to figure out where the model is coming from and attach it to the request object, and then authorize the action, and then optionally create a new object (or update an existing object) from incoming data, then carry out the actual action on the backend and return a HTTP status code.

The pre-methods let me abstract out those common patterns and write really small actual handlers. If I didn't have something like that, though, I'd just put some boilerplate like

loadItem(request.params.id, itemType)
.then(function(item){request.item = item; return authorizeItem(request.auth.credentials.user, level);})
.then( ...etc
.catch(function(err){... err handling 
srlm-io commented 9 years ago

@ericeslinger Interesting use. I like the way that you've broken it down. It makes sense to me, and I think I'll adopt it for the project that I'm working on now. Can you share more on your item.authorize(level, user) step? Checking authentication on an individual resource basis seems to be the biggest part missing from Hapi right now, and without a good public solution.

There was something similar here, using onPreAuth: https://github.com/hapijs/hapi/issues/2194

ericeslinger commented 9 years ago

@srlmproductions IMO, it's not the job of hapijs to handle authorization - this layer is for authentication, but not really authorization. I guess hapi could provide hooks to make sure that objects get authorized rather than having a manual process.

I apologize for the coffeescript here (I'm slowly moving to es6 whenever I touch a file), but here's the generic "show" route on one of my items. I am using coffeescript classes to abstract some stuff (the show route is the same for all items, mostly, all that differs is what kind of item is loaded, but each controller also handles a couple of item-specific routes as well, so this seemed like a reasonable way to handle that for me coming from a classical c++ object background. In the year I've been working with node, I've come to appreciate better ways to approach this, but here's what I have for now.

  method: 'GET'
  path: "{{basePath}}/{itemId}"
  handler: @show
  config:
    pre: [
      {method: TrellisController.inflateItem(@model), assign: 'item'}
      {method: TrellisController.authorizeItem('item', 'grant: view'), assign: 'auth'}
      {method: TrellisController.filterAllReferences, assign: 'filt'}
    ]
    auth: 'token'
    validate:
      params:
        itemId: Joi.number()

And here's my authorizeItem call.

@authorizeItem: (location, level)->
  (req, reply)->
    req.pre[location].authorize(req.auth.credentials.user, level)
    .then (auth)->
      if auth == true
        reply true
      else
        reply Hapi.error.forbidden()

The inflateItem call just hits the database and preloads the item, and then the filterReferences runs an item.authorize call on everything that was sideloaded to filter out related items (such as the list of all communities that this document being shown is shared with) the user doesn't have view rights on (such as private communities in the previous list). I'm in the middle of refactoring the database to make it feasible to have that calculation done at SQL query time, rather than by filtering stuff out afterwards, as you can imagine it represents a pretty significant portion of the time spent executing user requests in my server, and only grows over time.

mark-bradshaw commented 9 years ago

We use prereqs very similarly to what's been described. Authorization seems to be a natural fit in that spot. Having it explicitly listed as part of the config allows for easy composing and makes the route table self-documenting. One thing we wanted to do was to have post-reqs as well, since we had certain logging and cleanup tasks that applied to certain routes. Since hapi doesn't natively support this we created MrHorse (https://github.com/mark-bradshaw/mrhorse). It allows for pre and post actions. Same idea as pre-reqs, just slightly different terminology.

kellyrmilligan commented 8 years ago

@ericeslinger - just curious, where do you recommend putting arbitrary data on request? in the request.response.source object? i'm trying to figure out a way to add data to be used for rendering in separate handlers, like pre, or on every request, for example adding the output of mobile-detect for every request so I can use them in my rendering

ericeslinger commented 8 years ago

@kellyrmilligan I put everything in pre. Basically, I use some conventions so things are in the same place, and re-use code like mobile-detect or authenticate and authorize that'll be on pretty much every route in a common handler. Then that common handler gets re-used across all my controllers and routes. I personally don't do mobile-detect stuff on the server side, because I use hapi just for a json API rather than rendering out templates; the front end is angular which assembles stuff there.

example:

let resources = require('./resources');
function CRUD(model) {
  return {
    show: {
      method: 'GET',
      path: '/{itemId}',
      handler: show,
      config: {
        pre: [
          {method: resources.loadItem(model), assign: 'item'},
          {method: resources.authorizeItem('item', 1), assign: 'auth'},
       ],
        auth: 'token',
        validate: {
          params: {
            itemId: Joi.number()
          },
          query: {
            withRelated: Joi.array()
          }
        }
      }
    },
  //etc
  };
}

This is a bit of metaprogramming, which just curries up the CRUD routes for a given data model type (I just showed the show route here). The item (be it a Document, Profile, or any other data model in the system) is always loaded and stored on req.pre.item, and if I regularly worried about other stuff like mobile-detect, I'd put it in the same pre route. There's repetition, but the currying helps there, since I really only have a few actual route types. The model-specific logic (different model types have different authorization rules, for example) are implemented in the model class with the same method signature.

Here's an example of how it all hangs together:

let Profile = require('../models/profile');
let baseRoutes = require('./common/baseRoutes');

function plugin(server, options, next) {
  let crud = baseRoutes.CRUD(Profile);
  server.route([crud.create, crud.show, crud.update, crud.list, crud.delete, crud.related, crud.add, crud.remove, crud.events]);
  next();
}

Depending on the model type, I may or may not add all the specific CRUD routes (which by this time have feature-creeped to contain other commonly-used stuff in my json api). I can also alter the defaults if there's some specialty thing in the resultant routes for one particular data type (tags, for example, have an itemId which can be non-numeric).

devinivy commented 8 years ago

@kellyrmilligan to put arbitrary request-specific state, you might also look at request.app.

kellyrmilligan commented 8 years ago

@devinivy is request.app for a specific request? thought that was maybe for the app state, not the requeste

nlf commented 8 years ago

request.app is for request specific application state, there's also a server.app for things that aren't tied to a request