How to configure a custom url for unrecoverable errors like unsupported providers?

[carycodes]

trafficstars

Issue #166 briefly discusses how unrecoverable errors are redirected to the origin with a query string parameter, and not even the values in defaults are loaded. In my case the root endpoint is already busy for legacy reasons, and it's not really acceptable to add oauth error handling as a special case there. For the time being I'm using a small custom middleware in front of grant to pre-verify that the provider is one I support, but that seems like a bit of a hack, and I'll never be sure I've caught all the reasons it might error. How can I redirect the "missing provider" error (and any other unrecoverable errors) to a custom endpoint?

[simov]

Hi @grandopener, thanks for your feedback. I think that's a totally valid use case. Probably a special case for that type of error to check for either the transport or the callback being set inside the defaults would solve this. Generally that's against the overall design about how the configuration works, but I already have 2 special cases for the defaults key so I think it would fit nicely in this case too. I'll think about it and I'll get back to you.

[simov]

I was thinking about possible ways to resolve this, and so here is a little bit of background about why redirecting to the root / path by default was chosen.

Basically this is the safest way to throw an error without making any assumptions about the environment setup. Changing the default now may introduce timeouts and potentially server crashes for clients that are not ready to handle those errors.

That being said the current approach is not ideal either and I'll be thinking about it, but in the meantime here is what you can do.

I don't know why I assumed you are using Express, but if that's not the case let me know. The following example can be applied for Koa as well. Hapi and Fastify have their own built-in req/res lifecycle events, so doing something similar there is definitely possible too.

The general idea is that you can monkey-patch the res object:

var modifyResponseBody = (req, res, next) => {
  var send = res.send
  res.send = (...args) => {
    console.log(args)
    send.apply(res, args)
  }
  next()
}

var modifyRedirect = (req, res, next) => {
  var redirect = res.redirect
  res.redirect = (...args) => {
    console.log(args)
    redirect.apply(res, args)
  }
  next()
}

express()
  .use(modifyResponseBody)
  .use(modifyRedirect)

I know that sounds scary but that sort of thing is being done in many middlewares and in Express as well, so it's not that bad.

On top of that you should be doing it only on Grant routes:

express()
  .use(session({secret: 'grant', saveUninitialized: true, resave: false}))
  .use('/connect/:provider/:callback?', (req, res, next) => {
    var redirect = res.redirect
    res.redirect = (...args) => {
      console.log(args)
      var [path, querystring] = args[0].split('?')
      var query = qs.parse(querystring) // require('qs')
      console.log(path)
      console.log(query)
      if (path === '/' && query.error) {
        // redirect to some other place
        var error = `/error?${qs.stringify(query)}`
        redirect.call(res, error)
        // or respond
        // res.statusCode = 404
        // res.setHeader('content-type', 'application/json')
        // res.end(JSON.stringify(query))
      }
      else {
        // default behavior
        redirect.apply(res, args)
      }
    }
    next()
  })
  .use(grant(require('./config.json')))
  .listen(3000)

The above example redirects to a custom /error path instead of the default root / one. Also below it you can see how you can respond instead of redirecting. The res object is the Node.js one.

Lets keep this issue open. Let me know what you think.

[carycodes]

I am using express; good guess. I've implemented a monkey patch like you suggest and it works well for my situation. (In my case it's most convenient to respond to the client directly in the error path for the redirect override.). I'm using TypeScript so I had to jump through a couple extra hoops to make the signature for redirect work, but nothing too onerous.

I think this is an acceptable solution (although it should probably be documented if it becomes recommended practice). I agree the default behavior shouldn't change at this point. In the long run, if you don't want to add more special cases to the default object, one brainstorm is that it might be reasonable to add a new top-level config object (globals?) that is specifically intended for the things that don't fit conveniently into the current implementation of defaults. Perhaps the other special cases could be moved there as well.

[ivandotv]

special route for an error would be great.