-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[Fix] Fully support Authorization headers using the Basic authenticat… #3751
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
base: master
Are you sure you want to change the base?
Conversation
…ion scheme. Usage of the Basic authentication scheme involves setting the NVM_AUTH_HEADER environment variable to the value "Basic <base64 encoded value of string 'username:password'>" Here's an example shell one-liner to set this environment variable. ```sh export NVM_AUTH_HEADER="Basic $(echo 'username:password' | base64)" ``` The above literal username and password values would work since the base64 encoded value would be 'dXNlcm5hbWU6cGFzc3dvcmQK'. However, a username and password combination such as 'test:123?12>' would not work since the base64 encoded value would be 'dGVzdDoxMjM/MTI+Cg=='. The nvm_sanitize_auth_header function before this change would have stripped the valid base64 characters '/', '+', and '='.
ljharb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! We'll need regression tests, though
|
Alright, I added a test. |
…/', and '=' are supported.
…ing it to the upstream proxy.
This is done in case the test servers are used to test forward proxies.
|
Thanks! However, using docker images in CI is a bit of a heavy lift; can you find a way to run the tests without using docker? (also, test files need to |
I saw that the kennethreitz/httpbin container image was already being used for one of the existing tests and just used it to derive a second test to verify the changes in this PR. There is the one test using ferronserver/ferron:2 and I could remove it but that image is a distroless image meaning it's basically just the Or maybe I should ask, what makes using docker images in CI a heavy lift? Is it because it needs root access on the host and/or a daemon to run? If that's the case, perhaps podman can be used instead. Podman can be configured for a system account to run rootless containers without a daemon. Most podman commands can replace usage of docker by simply running I'll fix the test files to have the right chmod permissions. |
…ion scheme.
Usage of the Basic authentication scheme involves setting the NVM_AUTH_HEADER environment variable to the value "Basic <base64 encoded value of string 'username:password'>"
Here's an example shell one-liner to set this environment variable.
The above literal username and password values would work since the base64 encoded value would be 'dXNlcm5hbWU6cGFzc3dvcmQK'. However, a username and password combination such as 'test:123?12>' would not work since the base64 encoded value would be 'dGVzdDoxMjM/MTI+Cg=='. The nvm_sanitize_auth_header function before this change would have stripped the valid base64 characters '/', '+', and '='.