Allow to customize "missing provider" error URL?

[endel]

Hi @simov,

I would like to be able to customize the default response location if a provider hasn't been configured yet. Currently, it is hardcoded to "/":

grant/lib/response.js

Line 108 in c220e9e

? `${provider.callback || '/'}?${qs.stringify(output)}`

The reasoning is: I'd like to provide a friendly message when a provider wasn't configured, instead of showing this:

I'd like to send a PR to allow customizing this, would you mind pointing me a preferred direction to allow such customization?

(Thank you for your work again 🙏 grant has been in my toolbelt for years, and it's really great!)

[simov]

Hi @endel,

Thanks for your continued interest in using Grant, I really appreciate that.

The Grant: missing or misconfigured provider error is there only for debugging purposes. The correct response in that case should have been a 404 Not Found instead. But because with 404 you won't know if your server was configured correctly or not, I have decided to tell you that 'Your server is listening, and Grant is accessible, but maybe there is something wrong with your configuration'. And since it is almost impossible to configure Grant incorrectly to get to that error in particular, it almost certainly means that there is no such provider in your configuration.

Here is how a potential implementation may look like for Express:

var Grant = require('../../').express()
var grant = Grant(require('./config.json'))

express()
  // ...
  .use('/connect/:provider', (req, res, next) => {
    Object.keys(grant.config).includes(req.params.provider)
      ? next()
      : res.status(404).send('Not found')
  })
  .use(grant)
  // ...
  .listen(3000)

Similar implementation can be achieved for any of the provided handlers.

If you want to handle that error in some other way, other than a 404, then what would be the difference between implementing this simple middleware, and implementing a dedicated handler just for that error or maybe updating your existing catch all error handler with the logic to handle this error in particular?

In the end whether you want a redirect to some other route or simply responding with a 404 seems like an implementation detail in your own layer. As mentioned here #194 (comment):

Basically this is the safest way to throw an error without making any assumptions about the environment.

Now when thinking about it again I don't know why the author of that thread considered having a middleware in front of Grant being a bit of a hack, so that I had to come up with that really weird implementation instead. There are many other cases where you may want to augment the Grant middleware with a little bit of code, for example recently we had the case of handling alternating domains, but even for builtin features that is required in some cases, like dynamic state overrides and the state transport itself.

Let me know if I am missing something about your setup in particular.

[simov]

I spent a bit more time thinking about this and I wanted to elaborate a little bit more on what are we trying to fix with this proposed feature.

With this proposed feature we are trying to solve the issue of redirecting to the root / path when the transport: 'querystring' is being used (default) and a provider does not have a callback set.

This covers:

1. Not found or otherwise not pre-configured provider (no way to have a callback)
2. Pre-configured provider but without having a callback set

To fix 1. we can add a simple guard middleware before Grant and handle it accordingly:

.use('/connect/:provider', (req, res, next) => {
  Object.keys(grant.config).includes(req.params.provider)
    ? next()
    : res.status(404).send('Not found')
})

To fix 2. we can define a default callback to use for all providers found in our configuration:

{
  "defaults": {
    "callback": "/default"
  }
}

With this setup:

1. Trying to access non pre-configured provider on the /connect/:provider route will return 404 Not Found
2. Trying to access provider without having its callback explicitly set will redirect to the /default route instead, found in the defaults

There is one more case to cover and that is someone accessing the /connect/:provider/callback route directly (outside of the OAuth flow):

.use('/connect/:provider/callback', (req, res, next) => {
  Object.keys(grant.config).includes(req?.session?.grant?.provider)
    ? next()
    : res.status(404).send('Not found')
  })

Note that transport: 'session' is not affected by this issue because having a callback URL in that case is optional, and if missing it will return through the session object instead, and transport: 'state' can only return through the state object.

Here is the full example:

{
  "defaults": {
    "origin": "http://localhost:3000",
    "transport": "querystring",
    "callback": "/default"
  },
  "google": {
    "key": "APP_ID",
    "secret": "APP_SECRET",
    "scope": [
      "openid"
    ]
  }
}

var express = require('express')
var session = require('express-session')
var Grant = require('../../').express()

var grant = Grant(require('./config.json'))

express()
  .use(session({secret: 'grant', saveUninitialized: true, resave: false}))
  .use('/connect/:provider', (req, res, next) => {
    Object.keys(grant.config).includes(req.params.provider)
      ? next()
      : res.status(404).send('Not found')
  })
  .use('/connect/:provider/callback', (req, res, next) => {
    Object.keys(grant.config).includes(req?.session?.grant?.provider)
      ? next()
      : res.status(404).send('Not found')
    })
  .use(grant)
  .get('/default', (req, res) => {
    res.end(JSON.stringify(req.query, null, 2))
  })
  .listen(3000)

Navigating to:

• http://localhost:3000/connect/twitter - returns 404 Not Found
• http://localhost:3000/connect/google/callback - returns 404 Not Found

To sum it up:

1. Using defaults.callback solves the issue of having a provider without an explicit callback
2. Using a simple middleware for the /connect/:provider route is a perfectly valid solution to guard against providers not found in your grant.config
3. Using a simple middleware for the /connect/:provider/callback endpoint solves the problem of someone accessing the callback route outside of the OAuth flow

@endel let me know what you think

[endel]

Thank you SO much for the detailed response - your first suggestion was exactly what I was looking for. The further advice is also immensely helpful 💙