Authorization header use not conforming to HTTP standards
vkrizan opened this issue · 6 comments
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
Hi @vkrizan, thank you for raising this issue. I'll share this with the rest and then we'll get back to you.
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?
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.
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)