Authorization header use not conforming to HTTP standards

[vkrizan]

Describe the bug

Authorization header use with API tokens is out of HTTP standards of RFC 9110. The header lacks auth-scheme.

Steps to reproduce the bug

No response

Expected behavior

auth-scheme used in Authoriztion header. For API keys this could be Bearer.

Logs, error output, etc.

No response

Screenshots

No response

Additional context

• https://www.rfc-editor.org/rfc/rfc9110#section-11.4
• https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorization

Unleash version

5.4.2

Subscription type

Open source

Hosting type

Self-hosted

SDK information (language and version)

No response

[nunogois]

Hi @vkrizan, thank you for raising this issue. I'll share this with the rest and then we'll get back to you.

[sighphyre]

Hey @vkrizan, our auth protocol looks a lot like RFC 9110 but it isn't, it's a custom protocol we use. This isn't something that's likely to change in the near future since it's pretty deeply baked in at the moment. Is this actually causing problems for you?

[vkrizan]

Thank you for your response.

There are no problems considering the non-standard Authorization header used at the moment. I've raised it, as my assumption about the use of API keys (with the use of "Bearer") against Unleash were incorrect. Documentation about the use is only to be found in the Unleash API spec and on some examples.

I'm a bit concerned that not using the standard can lead to problems when implementing custom authentication methods, or at least create a tech-debt. Our folks have re-implemented a part of the toggle APIs with the Bearer support, with a fallback to using the header value as is: https://github.com/app-sre/unleash/blob/1a232f991ff224b634fd75b9da375b83e65d438f/index.js#L183.

I'd maybe suggest to start migrate to the standard, and have the fallback to header value to not break existing clients.
If you think that this is not such a big concern, feel free to close.

[sighphyre]

Hey @vkrizan, gotta be honest, I was kind of hoping you'd point out a bunch of places that this caused huge problems. It's always felt a little weird to me that we do this but I've never had any real argument to change it and it always felt like a big, fragile change to everything that needs to auth to Unleash.

The code you've linked is actually a pretty neat idea, that would make this safe, I think.

Don't want to close this, I'll bring it up with the team and let's see if we can head in this direction (no promises, it's entirely possible one of the other engineers has an excellent reason for this being in place and I've just never heard it)

[sighphyre]

Hey @vkrizan, just to try and (partially) close this loop. Some of my rather clever colleagues took a stab at this. We've added support for using the header correctly in the linked pull request. There are some limitations in what we can support in the spec, we have a few non standard characters in our token scheme that make this more challenging than we'd like.

Pretty sure that the patch you folks are using will work just fine, but if you'd like to give this a spin so you don't have to maintain that, we've released this behind a flag. You should be able to use this by setting the UNLEASH_EXPERIMENTAL_BEARER_TOKEN_MIDDLEWARE envronment variable to true.

Thanks again for reporting this and nudging us to fix it!

[vkrizan]

Thank you @sighphyre