Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass middleware per route #127

Closed
kasongoyo opened this issue Nov 16, 2017 · 5 comments
Closed

Pass middleware per route #127

kasongoyo opened this issue Nov 16, 2017 · 5 comments

Comments

@kasongoyo
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

kasongoyo commented Nov 18, 2017

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
Copy link
Member

tswaters commented Nov 22, 2017

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

  • provide capability to send object middleware key via options object, expected to be an object with each value being a middleware function

  • update the mapper to pull in a middleware key from both routeSet and route.map and attach that to the route.

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.

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

No branches or pull requests

2 participants