Repository navigation
Support setting Proxy-Authorization HTTP headers. #3916
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -139,6 +139,11 @@ nvm_download() { | |||||||||||||||||||||
| if [ -n "${NVM_AUTH_HEADER:-}" ]; then | ||||||||||||||||||||||
| sanitized_header="$(nvm_sanitize_auth_header "${NVM_AUTH_HEADER}")" | ||||||||||||||||||||||
| fi | ||||||||||||||||||||||
| local sanitized_proxy_header | ||||||||||||||||||||||
| sanitized_proxy_header='' | ||||||||||||||||||||||
| if [ -n "${NVM_PROXY_AUTH_HEADER:-}" ]; then | ||||||||||||||||||||||
| sanitized_proxy_header="$(nvm_sanitize_auth_header "${NVM_PROXY_AUTH_HEADER}")" | ||||||||||||||||||||||
| fi | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| local NVM_DOWNLOADER | ||||||||||||||||||||||
| NVM_DOWNLOADER='' | ||||||||||||||||||||||
|
|
@@ -184,6 +189,9 @@ nvm_download() { | |||||||||||||||||||||
| if [ -n "${NVM_AUTH_HEADER:-}" ]; then | ||||||||||||||||||||||
| set -- "$@" --header "Authorization: ${sanitized_header}" | ||||||||||||||||||||||
| fi | ||||||||||||||||||||||
| if [ -n "${NVM_PROXY_AUTH_HEADER:-}" ]; then | ||||||||||||||||||||||
| set -- "$@" --header "Proxy-Authorization: ${sanitized_proxy_header}" | ||||||||||||||||||||||
| fi | ||||||||||||||||||||||
|
Comment on lines
+192
to
+194
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
wget has no way to send an arbitrary header to the proxy (only
Suggested change
(I checked this in sh/dash/bash/zsh against the mock proxy: an HTTPS download through it succeeds, the header appears only on the |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| command "${NVM_DOWNLOADER}" "$@" | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| #!/bin/sh | ||
|
|
||
| # Tests nvm_download's NVM_PROXY_AUTH_HEADER support through a proxy that | ||
| # demands its own credentials. A node release is installed (the latest LTS, | ||
| # or the newest release whose binary runs on this system), and the node | ||
| # binary of that installation is used to run the dependency-free mock proxy | ||
| # (test/mocks/mock-proxy.js). The proxy-header checks themselves follow the | ||
| # nvm_download test in "Unit tests", using a local httpbin container as the | ||
| # origin. | ||
|
|
||
| cleanup () { | ||
| unset -f die cleanup | ||
| if [ -n "${mock_proxy_pid:-}" ]; then | ||
| kill "${mock_proxy_pid}" >/dev/null 2>&1 || true | ||
| fi | ||
| rm -f "${mock_proxy_port_file:-}" 2>/dev/null || true | ||
| docker rm -f httpbin >/dev/null 2>&1 || true | ||
| if [ -n "${nvm_alias_bak:-}" ]; then | ||
| rm -rf "${nvm_alias_dir}" 2>/dev/null || true | ||
| mv "${nvm_alias_bak}" "${nvm_alias_dir}" 2>/dev/null || true | ||
| else | ||
| rm -rf "${nvm_alias_dir:-}" 2>/dev/null || true | ||
| fi | ||
| } | ||
| die () { echo "$@" ; cleanup ; exit 1; } | ||
|
|
||
| : nvm.sh | ||
| \. ../../nvm.sh | ||
|
|
||
| set -ex | ||
|
|
||
| nvm deactivate >/dev/null 2>&1 || true | ||
|
|
||
| # install a node release; its node binary is used to run the mock proxy. | ||
| # On Alpine, node -musl binaries are served by the unofficial-builds mirror | ||
| # (the default nodejs.org mirror serves no arm64-musl artifacts), and a | ||
| # -musl binary needs a libstdc++ as new as the Alpine it was built on, so | ||
| # older Alpine releases can only run older node releases: try the candidates | ||
| # newest first and keep the first whose binary runs (the 'musl-binary' CI | ||
| # job locks the same-era pairings). | ||
| if [ -f "/etc/alpine-release" ]; then | ||
| # shellcheck disable=SC2034 # read by nvm.sh (not followed by shellcheck) | ||
| NVM_NODEJS_ORG_MIRROR='https://unofficial-builds.nodejs.org/download/release' | ||
| fi | ||
| NODE_VERSION='' | ||
| for NODE_CANDIDATE in 'lts/*' v20.20.1 v18.20.4 v14.21.3; do | ||
| nvm install -b --no-progress --skip-default-packages "${NODE_CANDIDATE}" || continue | ||
| CURRENT_VERSION="$(nvm current)" | ||
| if [ -n "${CURRENT_VERSION}" ] && [ "${CURRENT_VERSION}" != 'none' ]; then | ||
| NODE_VERSION="${CURRENT_VERSION}" | ||
| break | ||
| fi | ||
| nvm uninstall "${NODE_CANDIDATE}" >/dev/null 2>&1 || true | ||
| done | ||
| if [ -z "${NODE_VERSION}" ]; then | ||
| echo 'skip: no node release with a runnable binary on this system (tried: lts/*, v20.20.1, v18.20.4, v14.21.3)' | ||
| exit 0 | ||
|
Comment on lines
+46
to
+57
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This installs a full Node release from the network inside a Alternatively, a much cheaper test wouldn't need a proxy at all: put an executable |
||
| fi | ||
| NODE_BIN="$(nvm_version_path "${NODE_VERSION}")/bin/node" | ||
| [ -x "${NODE_BIN}" ] || die "the node binary at ${NODE_BIN} is missing or not executable" | ||
| [ "$("${NODE_BIN}" --version)" = "${NODE_VERSION}" ] || die 'the installed node binary does not report the installed version' | ||
|
|
||
| # `nvm install` and `nvm ls-remote` create aliases; keep the checkout clean | ||
| nvm_alias_dir="$(nvm_alias_path)" | ||
| nvm_alias_bak='' | ||
| if [ -d "${nvm_alias_dir}" ]; then | ||
| nvm_alias_bak="${nvm_alias_dir}.test-bak" | ||
| mv "${nvm_alias_dir}" "${nvm_alias_bak}" | ||
| fi | ||
|
Comment on lines
+63
to
+69
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This moves the real |
||
|
|
||
| # start the mock proxy, with the just-installed node, on an ephemeral port | ||
| mock_proxy_port_file="$(mktemp)" | ||
| "${NODE_BIN}" "../mocks/mock-proxy.js" "Bearer proxy-token" "${mock_proxy_port_file}" >/dev/null 2>&1 & | ||
| mock_proxy_pid="$!" | ||
| mock_proxy_port='' | ||
| i=0 | ||
| while [ "${i}" -lt 10 ]; do | ||
| if [ -s "${mock_proxy_port_file}" ]; then | ||
| mock_proxy_port="$(cat "${mock_proxy_port_file}")" | ||
| break | ||
| fi | ||
| i=$((i + 1)) | ||
| sleep 1 | ||
| done | ||
| [ -n "${mock_proxy_port}" ] || die 'the mock proxy did not report a port' | ||
| proxy_url="http://127.0.0.1:${mock_proxy_port}" | ||
|
|
||
| # make sure no_proxy does not bypass the proxy for these tests | ||
| unset no_proxy NO_PROXY | ||
|
|
||
| # the proxy header checks need a local httpbin container (as the nvm_download | ||
| # fast test); retry the pull, and skip (rather than fail) if the image cannot | ||
| # be pulled or run, so a transient Docker registry outage does not fail the | ||
| # suite | ||
| httpbin_pulled=0 | ||
| for i in 1 2 3 4 5; do | ||
| if docker pull kennethreitz/httpbin; then httpbin_pulled=1; break; fi | ||
| echo "docker pull httpbin failed, attempt $i/5" | ||
| sleep $((i * 5)) | ||
| done | ||
| if [ "${httpbin_pulled}" = 1 ] && SHELL=bash docker run -d --name httpbin -p 80:80 kennethreitz/httpbin; then | ||
| sleep 1 # wait for httpbin to start | ||
| # no Authorization and no Proxy-Authorization: expected fail | ||
| http_proxy="${proxy_url}" nvm_download "http://127.0.0.1/get" > /dev/null && die 'nvm_download with no Authorization and no Proxy-Authorization header should fail' | ||
| # Authorization but no Proxy-Authorization: expected fail | ||
| NVM_AUTH_HEADER="Bearer test-token" http_proxy="${proxy_url}" nvm_download "http://127.0.0.1/bearer" > /dev/null && die 'nvm_download with Authorization but no Proxy-Authorization header should fail' | ||
| # no Authorization but Proxy-Authorization: expected fail | ||
| NVM_PROXY_AUTH_HEADER="Bearer proxy-token" http_proxy="${proxy_url}" nvm_download "http://127.0.0.1/bearer" > /dev/null && die 'nvm_download with Proxy-Authorization but no Authorization header should fail' | ||
| # Authorization and Proxy-Authorization: expected success | ||
| NVM_AUTH_HEADER="Bearer test-token" NVM_PROXY_AUTH_HEADER="Bearer proxy-token" http_proxy="${proxy_url}" nvm_download "http://127.0.0.1/bearer" > /dev/null || die 'nvm_download with Authorization and Proxy-Authorization headers should succeed' | ||
|
Comment on lines
+103
to
+110
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of these use a plain |
||
| else | ||
| echo 'skipping proxy header checks: unable to pull or run httpbin' | ||
| fi | ||
|
|
||
| cleanup | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| // A minimal forward proxy for testing nvm's Proxy-Authorization support | ||
| // (see the "nvm_download" fast test). It requires no dependencies beyond | ||
| // Node.js core modules. | ||
| // | ||
| // Usage: node mock-proxy.js <expected-proxy-authorization> <port-file> | ||
| // | ||
| // - Listens on 127.0.0.1 on an OS-assigned free port (port 0) and writes | ||
| // the assigned port to <port-file> once the listener is bound, so the | ||
| // test can discover it. | ||
| // - Answers plain HTTP (absolute-form) requests and CONNECT requests whose | ||
| // Proxy-Authorization header is missing or differs from | ||
| // <expected-proxy-authorization> with "407 Proxy Authentication Required". | ||
| // - Forwards accepted absolute-form requests to the origin they name and | ||
| // streams the origin's response back to the client; the | ||
| // Proxy-Authorization header is not forwarded to the origin. | ||
| // - Tunnels accepted CONNECT requests to the requested host and port. | ||
| 'use strict'; | ||
|
|
||
| const fs = require('fs'); | ||
| const http = require('http'); | ||
| const net = require('net'); | ||
|
|
||
| const [expectedProxyAuth, portFile] = process.argv.slice(2); | ||
| if (!expectedProxyAuth || !portFile) { | ||
| console.error('usage: node mock-proxy.js <expected-proxy-authorization> <port-file>'); | ||
| process.exit(2); | ||
| } | ||
|
|
||
| function proxyAuthOk(req) { | ||
| return req.headers['proxy-authorization'] === expectedProxyAuth; | ||
| } | ||
|
|
||
| function deny(client) { | ||
| const head = 'HTTP/1.1 407 Proxy Authentication Required\r\n' | ||
| + 'Proxy-Authenticate: Basic realm="mock-proxy"\r\n' | ||
| + 'Connection: close\r\n' | ||
| + 'Content-Length: 0\r\n' | ||
| + '\r\n'; | ||
| if (client.writeHead) { | ||
| // A plain HTTP request: res.writeHead already sets the headers. | ||
| client.writeHead(407, { | ||
| 'Proxy-Authenticate': 'Basic realm="mock-proxy"', | ||
| 'Connection': 'close', | ||
| 'Content-Length': '0', | ||
| }); | ||
| client.end(); | ||
| } else { | ||
| // A CONNECT request: write the raw response on the socket. | ||
| client.end(head); | ||
| } | ||
| } | ||
|
|
||
| const server = http.createServer((req, res) => { | ||
| if (!proxyAuthOk(req)) { | ||
| deny(res); | ||
| return; | ||
| } | ||
| // The request line of a proxied plain HTTP request carries the absolute | ||
| // origin URL (e.g. "http://127.0.0.1/get"). | ||
| let target; | ||
| try { | ||
| target = new URL(req.url); | ||
| } catch (err) { | ||
| res.writeHead(400, { 'Content-Type': 'text/plain' }); | ||
| res.end('Bad Request: expected an absolute-form URL'); | ||
| return; | ||
| } | ||
| const headers = Object.assign({}, req.headers, { host: target.host }); | ||
| delete headers['proxy-authorization']; | ||
| delete headers['proxy-connection']; | ||
| const upstream = http.request({ | ||
| protocol: target.protocol, | ||
| hostname: target.hostname, | ||
| port: target.port || (target.protocol === 'https:' ? 443 : 80), | ||
| method: req.method, | ||
| path: target.pathname + target.search, | ||
| headers, | ||
| }, (upstreamRes) => { | ||
| res.writeHead(upstreamRes.statusCode || 502, upstreamRes.headers); | ||
| upstreamRes.pipe(res); | ||
| }); | ||
| upstream.on('error', (err) => { | ||
| res.writeHead(502, { 'Content-Type': 'text/plain' }); | ||
| res.end(`Bad Gateway: ${err.message}`); | ||
| }); | ||
| req.pipe(upstream); | ||
| }); | ||
|
|
||
| server.on('connect', (req, clientSocket, head) => { | ||
| if (!proxyAuthOk(req)) { | ||
| deny(clientSocket); | ||
| return; | ||
| } | ||
| const [host, port] = req.url.split(':'); | ||
| const upstream = net.connect(Number(port) || 443, host, () => { | ||
| clientSocket.write('HTTP/1.1 200 Connection Established\r\n\r\n'); | ||
| if (head && head.length) { | ||
| upstream.write(head); | ||
| } | ||
| upstream.pipe(clientSocket); | ||
| clientSocket.pipe(upstream); | ||
| }); | ||
| upstream.on('error', () => { | ||
| clientSocket.end('HTTP/1.1 502 Bad Gateway\r\nConnection: close\r\nContent-Length: 0\r\n\r\n'); | ||
| }); | ||
| }); | ||
|
|
||
| server.on('clientError', (err, socket) => { | ||
| console.error(`mock proxy: client error: ${err.message}`); | ||
| socket.end('HTTP/1.1 400 Bad Request\r\nConnection: close\r\nContent-Length: 0\r\n\r\n'); | ||
| }); | ||
|
|
||
| server.on('error', (err) => { | ||
| console.error(`mock proxy: ${err.message}`); | ||
| process.exit(1); | ||
| }); | ||
|
|
||
| server.listen(0, '127.0.0.1', () => { | ||
| const port = server.address().port; | ||
| fs.writeFileSync(portFile, `${port}\n`); | ||
| console.log(`mock proxy listening on port ${port}`); | ||
| }); |
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.
This example is Basic auth, which already works with no nvm change:
export https_proxy="http://user:password@proxy.example.com:3128"(and the same forhttp_proxy). I think this section should lead with that, and present$NVM_PROXY_AUTH_HEADERas being for other schemes (eg, Bearer), noting that it requires curl.