diff --git a/CHANGES b/CHANGES index d9ba0e1..68e9e8e 100644 --- a/CHANGES +++ b/CHANGES @@ -13,6 +13,11 @@ Version [unreleased] text files (npm 8) and whose workspaces are missing (npm >= 9); the default `--npm=latest` no longer ends in a 404 there `#310 `_ +- Quoted values in the config file, as the README shows them, are read + without the quotes, and `user:password@` in the `--mirror` URL is sent to + the mirror with HTTP Basic authentication instead of being taken for a part + of the host; the password stays out of error messages and redirects + `#321 `_ Version 1.11.0 -------------- diff --git a/README.rst b/README.rst index eb43d3b..1c496ee 100644 --- a/README.rst +++ b/README.rst @@ -153,6 +153,11 @@ or a version range, an ``index.json`` next to them:: $ nodeenv --node=22.14.0 --mirror=file:///srv/node-mirror env-22 +A mirror that asks for a login takes it in the URL, with special characters +percent-encoded:: + + $ nodeenv --mirror=https://user:p%2Fss@artifactory.example.com/nodejs env + Install the highest node.js release matching a version range:: $ nodeenv --node=22 env-22 @@ -349,7 +354,9 @@ Installation options ``--mirror=URL`` Set mirror server of nodejs.org to download from. A ``file://`` URL - points nodeenv at a local directory instead of a server. + points nodeenv at a local directory instead of a server. The + ``user:password@`` part of the URL goes to the mirror with HTTP Basic + authentication, and to no other host. ``-c, --clean-src`` Remove "src" directory after installation. This is the default. diff --git a/nodeenv.py b/nodeenv.py index 7ee2a0b..9007cd7 100644 --- a/nodeenv.py +++ b/nodeenv.py @@ -10,6 +10,7 @@ :license: BSD, see LICENSE for more details. """ +import base64 import contextlib import io import json @@ -37,6 +38,8 @@ from ConfigParser import SafeConfigParser as ConfigParser # pyright: ignore[reportMissingImports] # noqa: E501 # noinspection PyCompatibility import urllib2 # pyright: ignore[reportMissingImports] + from urlparse import urlsplit, urlunsplit # pyright: ignore[reportMissingImports] # noqa: E501 + from urllib import unquote # pyright: ignore[reportAttributeAccessIssue] iteritems = operator.methodcaller('iteritems') import httplib # pyright: ignore[reportMissingImports] IncompleteRead = httplib.IncompleteRead @@ -44,6 +47,7 @@ from configparser import ConfigParser # noinspection PyUnresolvedReferences import urllib.request as urllib2 + from urllib.parse import unquote, urlsplit, urlunsplit iteritems = operator.methodcaller('items') import http IncompleteRead = http.client.IncompleteRead @@ -62,6 +66,9 @@ # SSL context backed by the certifi bundle, built once by main() # when --with-certifi is given and certifi is importable certifi_context = None +# Authorization header for src_base_url, built by main() from the +# user:password@ part of --mirror, which is cut off src_base_url +src_auth = None # --------------------------------------------------------- # Utils @@ -137,6 +144,9 @@ def _load(cls, configfiles, verbose=False): val = ini_file.getboolean(section, attr) else: val = ini_file.get(section, attr) + # ConfigParser keeps the quotes the README shows, #321 + if len(val) > 1 and val[0] == val[-1] and val[0] in '\'"': + val = val[1:-1] if verbose: print('CONFIG {0}: {1} = {2}'.format( @@ -947,10 +957,30 @@ def _urlopen(req): return urllib2.urlopen(req) +def split_url_auth(url): + """ + Cut user:password@ off the URL, urllib takes it for a part of the host, + and return it as a Basic Authorization header. + https://github.com/ekalinin/nodeenv/issues/321 + """ + parts = urlsplit(url) + if parts.username is None: + return url, None + credentials = '%s:%s' % (unquote(parts.username), + unquote(parts.password or '')) + auth = base64.b64encode(credentials.encode('utf-8')).decode('ascii') + netloc = parts.netloc.rpartition('@')[2] + return urlunsplit(parts._replace(netloc=netloc)), 'Basic ' + auth + + def urlopen(url): home_url = "https://github.com/ekalinin/nodeenv/" headers = {'User-Agent': 'nodeenv/%s (%s)' % (nodeenv_version, home_url)} req = urllib2.Request(url, None, headers) + # the mirror's password is neither for other hosts, like the npm + # registry, nor for the ones the mirror redirects to + if src_auth and url.startswith(src_base_url): + req.add_unredirected_header('Authorization', src_auth) try: return _urlopen(req) except urllib2.HTTPError: @@ -1574,6 +1604,7 @@ def main(): exit(1) global src_base_url + global src_auth global ignore_ssl_certs global certifi_context @@ -1594,6 +1625,7 @@ def main(): src_domain = 'nodejs.org' if src_base_url is None: src_base_url = 'https://%s/download/release' % src_domain + src_base_url, src_auth = split_url_auth(src_base_url) # Decide on the system node before any version resolution, so that # a found system node never triggers a request for index.json diff --git a/tests/nodeenv_test.py b/tests/nodeenv_test.py index c6be29f..2ec7056 100644 --- a/tests/nodeenv_test.py +++ b/tests/nodeenv_test.py @@ -6,16 +6,20 @@ from pipes import quote as _quote else: from shlex import quote as _quote +import contextlib +import http.server import io import json import os.path import pathlib import shutil +import socket import subprocess import sys import platform import ssl import tarfile +import threading import zipfile try: @@ -453,6 +457,140 @@ def test_mirror_option_local_directory(tmpdir): mock_logger.assert_called_with('99.0.0') +def test_config_strips_quotes(tmpdir): + """Values quoted as in the README are read without the quotes, see #321""" + rc = tmpdir.join('nodeenvrc') + rc.write('[nodeenv]\n' + "node = '22.14.0'\n" + 'mirror = "https://example.com/mirror"\n') + try: + nodeenv.Config._load([str(rc)]) + assert nodeenv.Config.node == '22.14.0' + assert nodeenv.Config.mirror == 'https://example.com/mirror' + finally: + nodeenv.Config.node = nodeenv.Config._default['node'] + nodeenv.Config.mirror = nodeenv.Config._default['mirror'] + + +@pytest.fixture +def auth_mirror(monkeypatch): + """ + A local mirror that serves index.json only to an authorized request. + /moved/ redirects to /cdn/, which needs no authorization, like + a presigned URL of a cloud storage. + Yields its port and the paths with the Authorization headers it has seen. + """ + for name in PROXY_VARS: + monkeypatch.delenv(name, raising=False) + seen = [] + + class Handler(http.server.BaseHTTPRequestHandler): + def do_GET(self): + auth = self.headers.get('Authorization') + seen.append((self.path, auth)) + if self.path.startswith('/moved/'): + self.send_response(302) + self.send_header('Location', '/cdn/' + self.path[7:]) + self.end_headers() + return + if auth is None and not self.path.startswith('/cdn/'): + self.send_response(401) + self.send_header('WWW-Authenticate', 'Basic realm="mirror"') + self.end_headers() + return + body = (b'[{"version": "v99.0.0", "date": "2026-01-01",' + b' "lts": false, "files": ["linux-x64",' + b' "linux-x64-musl", "linux-riscv64"]}]') + self.send_response(200) + self.send_header('Content-Length', str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *args): + pass + + server = http.server.HTTPServer(('127.0.0.1', 0), Handler) + thread = threading.Thread(target=server.serve_forever) + thread.start() + try: + yield server.server_address[1], seen + finally: + server.shutdown() + server.server_close() + thread.join() + + +@pytest.mark.parametrize('userinfo, header', [ + ('user:secret', 'Basic dXNlcjpzZWNyZXQ='), + # me@corp.com:p/ss, a URL has room for these characters only encoded + ('me%40corp.com:p%2Fss', 'Basic bWVAY29ycC5jb206cC9zcw=='), +]) +def test_mirror_credentials_are_sent_as_basic_auth(auth_mirror, + userinfo, header): + """user:password@ in --mirror authenticates to the mirror, see #321""" + port, seen = auth_mirror + mirror = 'http://%s@127.0.0.1:%d' % (userinfo, port) + argv = [__file__, '--list', '--mirror=' + mirror] + with mock.patch.object(sys, 'argv', argv), \ + mock.patch.object(nodeenv.logger, 'info') as mock_logger: + nodeenv.src_base_url = None + nodeenv.main() + mock_logger.assert_called_with('99.0.0') + + assert set(seen) == {('/index.json', header)} + + +def test_mirror_credentials_do_not_follow_redirects(auth_mirror): + """A redirect may lead to another host, the password stays behind""" + port, seen = auth_mirror + mirror = 'http://user:secret@127.0.0.1:%d/moved' % port + argv = [__file__, '--list', '--mirror=' + mirror] + with mock.patch.object(sys, 'argv', argv), \ + mock.patch.object(nodeenv.logger, 'info') as mock_logger: + nodeenv.src_base_url = None + nodeenv.main() + mock_logger.assert_called_with('99.0.0') + + assert set(seen) == {('/moved/index.json', 'Basic dXNlcjpzZWNyZXQ='), + ('/cdn/index.json', None)} + + +def test_mirror_credentials_stay_out_of_error_messages(monkeypatch): + """A failed download must not print the mirror's password""" + for name in PROXY_VARS: + monkeypatch.delenv(name, raising=False) + # a port that was free a moment ago refuses the connection + with contextlib.closing(socket.socket()) as sock: + sock.bind(('127.0.0.1', 0)) + port = sock.getsockname()[1] + mirror = 'http://user:secret@127.0.0.1:%d' % port + argv = [__file__, '--list', '--mirror=' + mirror] + with mock.patch.object(sys, 'argv', argv), \ + mock.patch.object(nodeenv.logger, 'error') as m_error: + nodeenv.src_base_url = None + with pytest.raises(SystemExit): + nodeenv.main() + + errors = _logged_errors(m_error) + assert 'http://127.0.0.1:%d/index.json' % port in errors + assert 'secret' not in errors + + +def test_mirror_credentials_stay_with_the_mirror(): + """The npm registry must not get the mirror's password""" + with mock.patch.object(nodeenv, 'src_base_url', + 'https://mirror.example.com/node'), \ + mock.patch.object(nodeenv, 'src_auth', + 'Basic dXNlcjpzZWNyZXQ='), \ + mock.patch.object(nodeenv.urllib2, 'urlopen') as m_urlopen: + nodeenv.urlopen('https://mirror.example.com/node/index.json') + nodeenv.urlopen('https://registry.npmjs.org/npm/latest') + + to_mirror, to_registry = [c[0][0] for c in m_urlopen.call_args_list] + assert to_mirror.has_header('Authorization') + assert not to_registry.has_header('Authorization') + + @pytest.mark.usefixtures('mock_index_json', 'mock_host_platform') def test_get_latest_node_version(): assert nodeenv.get_last_stable_node_version() == '13.5.0'