-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Basic auth fixed #428
base: master
Are you sure you want to change the base?
Basic auth fixed #428
Conversation
1. BasicConnect handler was preventing other handlers from running 2. Basic was trying to authenticate connection that was already authenticated by BasicConnect
@elazarl Could you merge this PR? |
I'm very careful about adding yet another field to the context. Can we do it in some other way? |
Actually I did't think another way :) But this PR working correctly for me. |
Sorry, I had a problem after i implemented it. I am getting 407 (proxy auth required) for this code. Is there something I did wrong? `addr := flag.String("addr", ":8080", "proxy listen address")
|
Any update on this? |
@thalesfsp as I said, I'm reluctant to add another field to the Ctx without a really good reason. I reviewed the code and asked isn't using the User pointer possible, but didn't receive response yet. |
@elazarl Given the importance of the MR, could you do the change you are suggesting? Best regards! |
这个啥时候能修复呢 |
@guanyufen123 just fork and merge the change. This is what happens when the community needs and demands things but authors are reluctant to listen. |
@thalesfsp can you please explain me again, why using the I'm very open to listen, but was not able to spurr discussion. |
Issue: When using AlwaysMitm ext/auth/basic.go would attempt to authenticate requests twice resulting in the 407 unauthorized message.
This fixes two outstanding issues with the basic auth behavior of this library.
Resolves: #309, #416
Below is a simple test case to validate the behavior before/after the patch: