senecajs / seneca-web

Http route mapping for Seneca microservices.
MIT License
76 stars 44 forks source link

Pass middleware per route #127

Closed kasongoyo closed 6 years ago

kasongoyo commented 6 years ago

I appreciate the great work that has been done by the team behind seneca well i have little suggestion if possible to put a way for implementers to be to pass middleware per route. Forexample I prefer my own auth middleware over passport but I don't see a way I can pass my middleware per route

tswaters commented 6 years ago

One thing to note is that if routes pass over a microservice boundary, any functions will be stripped (functions can't be transported via json). That said, you could proxy role:web,route:*, use prior actions to add in middleware before it hits seneca-web, so I think the case is valid.

I'd like to see how this would be implemented in the various adapters though. For the connect-style frameworks it's easy enough to prepend middleware, but I don't have a lot of experience with HAPI and I'm not sure off hand how this would look... Thoughts?

kasongoyo commented 6 years ago

Can you elaborate how that can be achieved by proxying role:web,rote:*? I agree that we can't pass object within route for the reason you have explained but may be we can pass key value object called may be middlewareMap with key as middleware name and value as middleware instance during init of this plugin and only provide keys within the route to direct that the specific route need particular middleware and all those information will be passed down to adapters and adapters can decide which middleware to add to which route. I can add that functionality in seneca web express adapter and it will start with express atleast.

tswaters commented 6 years ago

An example of using prior to proxy calls might look something like this:

seneca = Seneca()
seneca.use(SenecaWeb, {...etc...})
seneca.ready(() => {
  seneca.add('role:web,routes:*', function (msg, cb) {

    msg.routes[0].map.authRoute.middleware = [
      (req, res, next) => {...etc...}
    ]

    this.prior(msg, cb)

  })
})

Assuming the message comes in looking something like this:

{
  "routes": [
    {
      "pin": "cmd:*",
      "map": {
        "authRoute": {"GET": true}
      }
    }
  ]
}

It would modify the authRoute pin to include middleware before it gets sent to seneca-web (via the this.prior call). That of course is pretty bare-bones - and that could turn into a bit of a maintenance nightmare keeping the web microservice up-to-date with what needs to have special middleware.

Another option might be to add a more specific pin like role:web,auth:true,routes:* and modify all the map keys to include special middleware prior to sending the message role:web,routes:*, e.g.:

seneca.add('role:web,auth:true,route:*', (msg, cb) => {

  const routes = msg.routes
  // todo: modify all incoming routes to include middleware
  seneca.act('role:web', {routes})

})

Finally, if you don't care about ALL routes having the same middleware, you can attach it to whatever was provided via the context parameter.... e.g.,

seneca.use(SenecaWeb, {
  ...etc...
  context: new Router()
})

seneca.ready(() => {

  const app = express()
  const router = seneca.export('web/context')()
  router.use((req, res, next) => {...etc...})
  app.use('/api', router)

})
kasongoyo commented 6 years ago

I like the first approach, I think to anticipate maintenance problems, lets pass collection of middleware instead of specific middleware. Something like this:

seneca = Seneca()
seneca.use(SenecaWeb, {...etc ...})
seneca.ready(() => {
    seneca
        .add('role:web,routes:*', function (msg, cb) {

            msg.middleware = [
                {'middleware1': (req, res, next) => {...etc...}},
                {'middleware2': (req, res, next) => {...etc...}},
            ]
            this.prior(msg, cb)
        })
})

Incoming Message

{
  "routes": [
    {
      pin: 'role:api,cmd:*',
      map: {
        ping: {
          GET: 'true',
          middleware: ['middleware1', 'middleware2']
        }
      }
    }
  ]
}

Then in the mapRoute function of seneca-web we modify like this;

function mapRoutes (msg, done) {
  var seneca = this
  var adapter = msg.adapter || locals.adapter
  var context = msg.context || locals.context
  var options = msg.options || locals.options
  var routes = Mapper(msg.routes)
  var auth = msg.auth || locals.auth
  var middleware = msg.middleware || locals.middleware

  // Call the adaptor with the mapped routes, context to apply them to and
  // instance of seneca and the provided consumer callback.
  adapter.call(seneca, options, context, auth, routes, middleware, done)
}

Then adapter implementators will inspect route to find if there is middleware key then extract middleware from middleware array and apply it in the context. That way will enable middleware to be applied per specific route and not to all routes.

tswaters commented 6 years ago

Terribly sorry for not responding earlier. My main concern with this is it changes the API contract between seneca-web and the adapters. This will require a major bump and a bunch of staging to make sure we publish new major versions of all the adapters at the same time.

Also, I liked the other way a bit more - providing functions.... but, at the same time, I can see value in providing named keys as well. How about both? We could check if the entry is a string - if so, pull in the middleware -- if it's a function, assume that it is middleware and use that. Of course that concern is more in the adapters and how they deal with this new information.

In terms of seneca-web itself

Also, we're going to need docs that describe this pretty well I think, another page under ./docs/ called using middleware that describes both methods - providing functions in a prior or proxied call. Or, providing keyed middleware as part of initialization options and using those keys from microservices.

I'll respond also in the PR with specific things that need to happen.