fix: ensure bearer_token respects token_from

[TamerShlash]

The Problem

The bearer_token authenticator always returns 401 when providing any token_from option. The reason is because the token is indeed detected, and therefore the authenticator is deemed responsible, but the token is not actually passed forward to the session store in the Authorization header as the instructions here and here say should be done.

The only exception when it does work is when using the default Authorization: bearer <session-token> option (i.e, not specifying token_from at all), because the Authorization header is forwarded anyways as you can see here:

oathkeeper/pipeline/authn/authenticator_bearer_token.go

Line 124 in acb2584

c.ForwardHTTPHeaders = append(c.ForwardHTTPHeaders, []string{header.Authorization}...)

This PR fixes that by making sure to set Authorization: bearer <token> for the request going to the sessions store if a token is detected.

Is It Breaking?

No, it should not be.

Related issue(s)

#1144

Checklist

• I have read the contributing guidelines.
• I have referenced an issue containing the design document if my change
  introduces a new feature.
• I am following the
  contributing code guidelines.
• I have read the security policy.
• I confirm that this pull request does not address a security
  vulnerability. If this pull request addresses a security vulnerability, I
  confirm that I got the approval (please contact
  [email protected]) from the maintainers to push
  the changes.
• I have added tests that prove my fix is effective or that my feature
  works.
• I have added or changed the documentation.

[TamerShlash]

Hello @aeneasr 👋 can you take a look at this?

[zepatrik]

Thanks for chiming in here! However, I'm not quite sure if this is the correct way to solve the problem.
The basic problem is that the bearer authenticator handles token_from in an unexpected way: the token is forwarded, but with the same method as defined in token_from.
Your fix adds the token to the forwarded request in the authorization header, however the original header is also forwarded. This fixes where the session store expects a Authorization: bearer <token>, however it does not solve other cases.
What do you think of adding a new config option to the authenticator, basically forward_token_as that is the inverse of token_from. With this approach we can ensure that we don't break anyone relying on the current behavior, but also don't forward the token twice.

I would not want to forward the token twice, as the token store might not correctly strip the token from logs etc. when it receives "just another extra header".

WDYT?

[TamerShlash]

Hi @zepatrik! thank you for taking a look at this.

I think the main misunderstanding here is that in my mind a session store was equal to Ory Kratos, which expects the session token exclusively in the Authorization header.
What I'm understanding from you now is that this PR might break solutions where the sesison store was something other than Ory Kratos, and those solutions have been able to expect the token in other headers and thus forwarding worked fine for them.

If that's what you mean, that seems totally reasonable, and the fix you suggested with forward_token_as seems like a pretty reasonable fix.

My only take on this is that Oathkeeper authenticators configuration has already been quite confusing to understand to begin with (maybe just for us), so I think we need to properly document how this new forward_token_as option works especially with Ory Kratos as a session store since I'm assuming it's a major use case.

How do you want to go forward with this? I'm happy to update this PR with that suggestion if but can't tell how soon it will be since it's a bit busy for me this period and this is my first time working with Go ever 😄 Otherwise of course it's great if you want to take care of it.