Remove username/password from log message

Merge pull request #5339 from jzafran/remove-password-from-stdout

Author: pradyunsg

news/5249.bugfix

@@ -0,0 +1 @@
+Remove username/password from log message when using index with basic auth

src/pip/_internal/index.py

@@ -33,7 +33,7 @@
 from pip._internal.utils.logging import indent_log
 from pip._internal.utils.misc import (
     ARCHIVE_EXTENSIONS, SUPPORTED_EXTENSIONS, cached_property, normalize_path,
-    splitext,
+    remove_auth_from_url, splitext,
 )
 from pip._internal.utils.packaging import check_requires_python
 from pip._internal.wheel import Wheel, wheel_ext
@@ -197,7 +197,8 @@ def get_formatted_locations(self):
         lines = []
         if self.index_urls and self.index_urls != [PyPI.simple_url]:
             lines.append(
-                "Looking in indexes: {}".format(", ".join(self.index_urls))
+                "Looking in indexes: {}".format(", ".join(
+                    remove_auth_from_url(url) for url in self.index_urls))
             )
         if self.find_links:
             lines.append(

src/pip/_internal/utils/misc.py

@@ -24,6 +24,7 @@
 from pip._vendor.retrying import retry  # type: ignore
 from pip._vendor.six import PY2
 from pip._vendor.six.moves import input
+from pip._vendor.six.moves.urllib import parse as urllib_parse
 
 from pip._internal.compat import console_to_str, expanduser, stdlib_pkgs
 from pip._internal.exceptions import InstallationError
@@ -47,7 +48,7 @@
            'unzip_file', 'untar_file', 'unpack_file', 'call_subprocess',
            'captured_stdout', 'ensure_dir',
            'ARCHIVE_EXTENSIONS', 'SUPPORTED_EXTENSIONS',
-           'get_installed_version']
+           'get_installed_version', 'remove_auth_from_url']
 
 
 logger = std_logging.getLogger(__name__)
@@ -849,3 +850,21 @@ def enum(*sequential, **named):
     reverse = {value: key for key, value in enums.items()}
     enums['reverse_mapping'] = reverse
     return type('Enum', (), enums)
+
+
+def remove_auth_from_url(url):
+    # Return a copy of url with 'username:password@' removed.
+    # username/pass params are passed to subversion through flags
+    # and are not recognized in the url.
+
+    # parsed url
+    purl = urllib_parse.urlsplit(url)
+    stripped_netloc = \
+        purl.netloc.split('@')[-1]
+
+    # stripped url
+    url_pieces = (
+        purl.scheme, stripped_netloc, purl.path, purl.query, purl.fragment
+    )
+    surl = urllib_parse.urlunsplit(url_pieces)
+    return surl

src/pip/_internal/vcs/subversion.py

@@ -8,7 +8,7 @@
 
 from pip._internal.index import Link
 from pip._internal.utils.logging import indent_log
-from pip._internal.utils.misc import display_path, rmtree
+from pip._internal.utils.misc import display_path, remove_auth_from_url, rmtree
 from pip._internal.vcs import VersionControl, vcs
 
 _svn_xml_url_re = re.compile('url="([^"]+)"')
@@ -63,7 +63,7 @@ def export(self, location):
         """Export the svn repository at the url to the destination location"""
         url, rev = self.get_url_rev()
         rev_options = get_rev_options(self, url, rev)
-        url = self.remove_auth_from_url(url)
+        url = remove_auth_from_url(url)
         logger.info('Exporting svn repository %s to %s', url, location)
         with indent_log():
             if os.path.exists(location):
@@ -84,7 +84,7 @@ def update(self, dest, rev_options):
     def obtain(self, dest):
         url, rev = self.get_url_rev()
         rev_options = get_rev_options(self, url, rev)
-        url = self.remove_auth_from_url(url)
+        url = remove_auth_from_url(url)
         if self.check_destination(dest, url, rev_options):
             rev_display = rev_options.to_display()
             logger.info(
@@ -221,24 +221,6 @@ def is_commit_id_equal(self, dest, name):
         """Always assume the versions don't match"""
         return False
 
-    @staticmethod
-    def remove_auth_from_url(url):
-        # Return a copy of url with 'username:password@' removed.
-        # username/pass params are passed to subversion through flags
-        # and are not recognized in the url.
-
-        # parsed url
-        purl = urllib_parse.urlsplit(url)
-        stripped_netloc = \
-            purl.netloc.split('@')[-1]
-
-        # stripped url
-        url_pieces = (
-            purl.scheme, stripped_netloc, purl.path, purl.query, purl.fragment
-        )
-        surl = urllib_parse.urlunsplit(url_pieces)
-        return surl
-
 
 def get_rev_options(vcs, url, rev):
     """

tests/unit/test_index.py

@@ -139,3 +139,18 @@ def test_secure_origin(location, trusted, expected):
     logger = MockLogger()
     finder._validate_secure_origin(logger, location)
     assert logger.called == expected
+
+
+def test_get_formatted_locations_basic_auth():
+    """
+    Test that basic authentication credentials defined in URL
+    is not included in formatted output.
+    """
+    index_urls = [
+        'https://pypi.org/simple',
+        'https://user:[email protected]',
+    ]
+    finder = PackageFinder([], index_urls, session=[])
+
+    result = finder.get_formatted_locations()
+    assert 'user' not in result and 'pass' not in result

tests/unit/test_utils.py

@@ -24,7 +24,8 @@
 from pip._internal.utils.hashes import Hashes, MissingHashes
 from pip._internal.utils.misc import (
     call_subprocess, egg_link_path, ensure_dir, get_installed_distributions,
-    get_prog, normalize_path, rmtree, untar_file, unzip_file,
+    get_prog, normalize_path, remove_auth_from_url, rmtree, untar_file,
+    unzip_file,
 )
 from pip._internal.utils.packaging import check_dist_requires_python
 from pip._internal.utils.temp_dir import TempDirectory
@@ -624,3 +625,24 @@ def test_call_subprocess_works_okay_when_just_given_nothing():
 def test_call_subprocess_closes_stdin():
     with pytest.raises(InstallationError):
         call_subprocess([sys.executable, '-c', 'input()'])
+
+
+@pytest.mark.parametrize('auth_url, expected_url', [
+    ('https://user:[email protected]/project/tags/v0.2',
+     'https://domain.tld/project/tags/v0.2'),
+    ('https://domain.tld/project/tags/v0.2',
+     'https://domain.tld/project/tags/v0.2',),
+    ('https://user:[email protected]/svn/project/trunk@8181',
+     'https://domain.tld/svn/project/trunk@8181'),
+    ('https://domain.tld/project/trunk@8181',
+     'https://domain.tld/project/trunk@8181',),
+    ('git+https://pypi.org/something',
+     'git+https://pypi.org/something'),
+    ('git+https://user:[email protected]/something',
+     'git+https://pypi.org/something'),
+    ('git+ssh://[email protected]/something',
+     'git+ssh://pypi.org/something'),
+])
+def test_remove_auth_from_url(auth_url, expected_url):
+    url = remove_auth_from_url(auth_url)
+    assert url == expected_url

tests/unit/test_vcs.py

@@ -173,32 +173,6 @@ def test_bazaar_simple_urls():
     )
 
 
-def test_subversion_remove_auth_from_url():
-    # Check that the url is doctored appropriately to remove auth elements
-    #    from the url
-    svn_auth_url = 'https://user:[email protected]/svn/project/tags/v0.2'
-    expected_url = 'https://svnrepo.org/svn/project/tags/v0.2'
-    url = Subversion.remove_auth_from_url(svn_auth_url)
-    assert url == expected_url
-
-    # Check that this doesn't impact urls without authentication'
-    svn_noauth_url = 'https://svnrepo.org/svn/project/tags/v0.2'
-    expected_url = svn_noauth_url
-    url = Subversion.remove_auth_from_url(svn_noauth_url)
-    assert url == expected_url
-
-    # Check that links to specific revisions are handled properly
-    svn_rev_url = 'https://user:[email protected]/svn/project/trunk@8181'
-    expected_url = 'https://svnrepo.org/svn/project/trunk@8181'
-    url = Subversion.remove_auth_from_url(svn_rev_url)
-    assert url == expected_url
-
-    svn_rev_url = 'https://svnrepo.org/svn/project/trunk@8181'
-    expected_url = 'https://svnrepo.org/svn/project/trunk@8181'
-    url = Subversion.remove_auth_from_url(svn_rev_url)
-    assert url == expected_url
-
-
 def test_get_git_version():
     git_version = Git().get_git_version()
     assert git_version >= parse_version('1.0.0')