Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Authorization header #90

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

willoucom
Copy link

Fixes #89

Since getHeaderLine() can return a coma separated string, i add an explode and a foreach.

Updated tests to reflect the change

@omega3000
Copy link

I agree to this solution.
In my setup I provide a slim API for a frontend Vue application were I wanted to use basic auth to secure the API endpoint and a bearer token for user authentication. So my header looked like this:

Authorization: Basic <base64UserPasswd>, Bearer <jsonWebToken>

The approach from @willoucom made that work!

@tuupola
Copy link
Owner

tuupola commented Mar 28, 2022

AFAIK header which looks like below is invalid syntax.

Authorization: Basic <base64UserPasswd>, Bearer <jsonWebToken>

@nerdlibfront
Copy link

nerdlibfront commented May 12, 2022

We occured the same issue in another scenario.
It seems that the combination of AWS ALB, EKS ingress, an apache pod and HTTP/2 leads to a duplication of the header somewhere down the road, resulting in a Header like:
Authorization: Basic foo,Basic foo

I agree that this is basically not the problem of the library, but it would add some resilience if the lib would just take the first header starting with "Basic".

@willoucom
Copy link
Author

I agree with @tuupola , having 2 different authorization is invalid.
There are multiple discussions on the Internet about this header and it seems that nobody agrees on a solution.

However, I think the problem with some loadbalancers (e.g. amazon/scaleway/azure) duplicating the header can lead to a lot of problems for developers wanting to use this library (which I consider great).
To solve this problem, I can modify my patch to retrieve only the first element of the header and ignore the others, I think this will solve the problem caused by LBs and prevent misuse of the header (i.e. using multiple schemas in the same header).

What do you think about this alternative?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Authentification error
4 participants