Strip Bearer From Token

[dpbackes]

• Some libraries use the "Bearer" prefix when doing token
  authentication. If we encounter a token with a "Bearer"
  prefix then just strip it off and proceed as normal.

[dpbackes]

The C# library I'm using does this and it'll be easier to deal with it here rather than doing the stripping in all our services. cc @tylerodonnell @eesmith from our discussion earlier today.

[tylerodonnell]

I think we should also strip the lowercase version. Thoughts?

[dpbackes]

don't see any reason not to

[tylerodonnell]

So most of our services are already stripping bearer from the token - but I like having the option of Authentic stripping "Bearer" if it exists.

[flintinatux]

👀

[flintinatux]

See comments below.

One general comment on workflow: please no more squashing and force-pushing. Just make separate commits for your changes. Force-pushing is problematic for others wishing to checkout locally and contribute.

When we merge it, we'll select the Squash and Merge option, which will rollup the whole PR into a single commit for us. It'll keep the commit history nice and pretty.

[flintinatux]

The idiomatic way to do this in js is with the case-insensitive i flag. Also, Ramda has a point-free replace function:

const stripBearer =
  replace(/^Bearer /i, '')

The null-check is best left to the existing enforce function. I'll make a separate comment explaining that.

[flintinatux]

Bumping the version isn't done until after the updates have been merged into #master. After the PR is merged, a new version is created and published with the following flow:

> yarn version  # bumps the version and creates a matching git tag
> git push --tags origin master
> npm publish --access=public

Just move this back to 0.1.2 for now.

[flintinatux]

The need to strip and null-check in multiple places is a 👃 that suggests we organize things a little better with some composition:

// use unauthorized in enforce
const enforce = token =>
  token || Promise.reject(unauthorized('null token not allowed'))

const stripBearer = // <<-- move here to maintain alphebetic order
  replace(/^Bearer /i, '')

const unauthorized = err =>
  Promise.reject(Boom.wrap(err, 401))

const factory = opts => {
  // omiting other bits here...

  // remove stripBearer from verify
  const verify = curryN(2, partialRight(promisify(jwt.verify), [ verifyOpts ]))

  const authentic = token =>
    Promise.resolve(token)
      // remove tapP(enforce) here
      // remove stripBearer
      .then(decode)
      .then(tapP(checkIss))
      .then(getSigningKey)
      .then(chooseKey)
      .then(verify(token))
      .catch(unauthorized)

  // composeP to null-check and strip all in one place
  return composeP(authentic, stripBearer, tapP(enforce))
}

[dpbackes]

@flintinatux updated. Rambda doesn't support catches in composeP so I just did a normal promise chain.

[flintinatux]

@dpbackes, that's why I suggested swapping the new Error in enforce with unauthorized, so that the error would be correctly formatted without catching, and the .catch could be left in the authentic function.

[flintinatux]

Hm, I'm noticing now that it should be Boom.unauthorized in enforce, since the custom unauthorized function wraps a native Error object.

[flintinatux]