diff --git a/docs/html/topics/authentication.md b/docs/html/topics/authentication.md index a2649071762..a0eb8d56e17 100644 --- a/docs/html/topics/authentication.md +++ b/docs/html/topics/authentication.md @@ -2,8 +2,18 @@ ## Basic HTTP authentication -pip supports basic HTTP-based authentication credentials. This is done by -providing the username (and optionally password) in the URL: +pip supports basic HTTP-based authentication credentials. You can provide the +credentials directly in the URLs or using {pypi}`keyring` or a `.netrc` file. When +needed pip will search for credentials in the following order: + +1. package URL from requirement (if any) +2. index URLs +3. keyring (if available) +4. `.netrc` file (if present) + +## URL credentials support + +You can provide the username (and optionally password) directly in the URL: ``` https://username:password@pypi.company.com/simple @@ -34,6 +44,15 @@ https://user:he%2F%2Fo@pypi.company.com/simple [reserved-chars]: https://en.wikipedia.org/wiki/Percent-encoding#Percent-encoding_reserved_characters + +### Securely storing password in keyring + +It is recommended to avoid storing password in clear text. For this purpose, you can +use {pypi}`keyring` to store the password securely while still mentioning the username +to use in your URL. pip will then extract the username from the URL and, as it did not +find a password, it will search for a corresponding one in your keyring. See [Keyring +support](#keyring-support) bellow. + ## netrc support pip supports loading credentials from a user's `.netrc` file. If no credentials @@ -63,14 +82,45 @@ man pages][netrc-docs]. [netrc-docs]: https://www.gnu.org/software/inetutils/manual/html_node/The-_002enetrc-file.html [netrc-std-lib]: https://docs.python.org/3/library/netrc.html -## Keyring Support +## Keyring support + +pip supports loading credentials stored in your keyring using the {pypi}`keyring` +library, which can be enabled by passing `--keyring-provider` with a value of `auto`, +`import`, or `subprocess`. The default value `auto` respects `--no-input` and does not +query keyring at all if the option is used; otherwise it tries the `import`, +`subprocess`, and `disabled` providers (in this order) and uses the first one that +works. + +You can explicitly disable keyring support by passing `--keyring-provider=disabled`. + +When looking for credentials, pip will first try to find a record in your keyring for +the corresponding URL and if none are found it will try with just the server hostname. + +### Storing credentials + +In interactive mode, when the keyring is available and the server requires the +user to authenticate, pip will prompt you for the credentials and then save +them in the keyring. In this case the credentials will be saved based on the server +hostname. + +You can also manually store your credentials in your keyring, either for an index URL +(note that the URL _must_ end with a `/`): + +``` +keyring set https://pypi.company.com/dev/simple/ user.name@company.com +``` + +Or for a server hostname: -pip supports loading credentials stored in your keyring using the -{pypi}`keyring` library, which can be enabled py passing `--keyring-provider` -with a value of `auto`, `disabled`, `import`, or `subprocess`. The default -value `auto` respects `--no-input` and does not query keyring at all if the option -is used; otherwise it tries the `import`, `subprocess`, and `disabled` -providers (in this order) and uses the first one that works. +``` +keyring set pypi.company.com user.name@company.com +``` + +In both cases, `keyring` will prompt you for the password to store. + +Note: For server requiring a token instead of a username and password, you can still +store it as the username with an empty password in keyring but due to the limitation of +the `subprocess` provider, this only make sense when using the `import` provider. ### Configuring pip's keyring usage @@ -103,6 +153,10 @@ $ export PIP_KEYRING_PROVIDER=disabled Setting `keyring-provider` to `import` makes pip communicate with `keyring` via its Python interface. +This is slightly faster than the `subprocess` provider and it makes it possible to use +URLs without any username as it can find it in your keyring. The downside is that you +have to install it in every virtualenv. + ```bash # install keyring from PyPI $ pip install keyring --index-url https://pypi.org/simple @@ -115,9 +169,10 @@ $ pip install your-package --keyring-provider import --index-url https://pypi.co Setting `keyring-provider` to `subprocess` makes pip look for and use the `keyring` command found on `PATH`. -For this use case, a username *must* be included in the URL, since it is -required by `keyring`'s command line interface. See the example below or the -basic HTTP authentication section at the top of this page. +The advantage is that you only need to install it once (and it can even be installed +system-wide for all users). +The disadvantage is that, a username *must* be included in the URL, since it is required +by `keyring`'s command line interface. See the example below. ```bash # Install keyring from PyPI using pipx, which we assume is installed properly @@ -163,25 +218,6 @@ from the subprocess in which they run Pip. You won't know whether the keyring backend is waiting the user input or not in such situations. ``` -pip is conservative and does not query keyring at all when `--no-input` is used -because the keyring might require user interaction such as prompting the user -on the console. You can force keyring usage by passing `--force-keyring` or one -of the following: - -```bash -# possibly with --user, --global or --site -$ pip config set global.force-keyring true -# or -$ export PIP_FORCE_KEYRING=1 -``` - -```{warning} -Be careful when doing this since it could cause tools such as pipx and Pipenv -to appear to hang. They show their own progress indicator while hiding output -from the subprocess in which they run Pip. You won't know whether the keyring -backend is waiting the user input or not in such situations. -``` - Note that `keyring` (the Python package) needs to be installed separately from pip. This can create a bootstrapping issue if you need the credentials stored in the keyring to download and install keyring. diff --git a/news/11721.bugfix.rst b/news/11721.bugfix.rst new file mode 100644 index 00000000000..0dbe55cc14a --- /dev/null +++ b/news/11721.bugfix.rst @@ -0,0 +1,2 @@ +Wait until the index server tells us that authentication is required before trying +to make sense of the userinfo section of the URLs and potentially query ``keyring``. diff --git a/news/11721.doc.rst b/news/11721.doc.rst new file mode 100644 index 00000000000..743278c169c --- /dev/null +++ b/news/11721.doc.rst @@ -0,0 +1,2 @@ +- Removed mention of un-implemented ``--force-keyring`` option. +- Detailed the pros and cons of using the ``import`` and ``subprocess`` keyring providers. diff --git a/src/pip/_internal/cli/cmdoptions.py b/src/pip/_internal/cli/cmdoptions.py index d05e502f908..6889918397f 100644 --- a/src/pip/_internal/cli/cmdoptions.py +++ b/src/pip/_internal/cli/cmdoptions.py @@ -260,8 +260,8 @@ class PipOption(Option): default="auto", help=( "Enable the credential lookup via the keyring library if user input is allowed." - " Specify which mechanism to use [disabled, import, subprocess]." - " (default: disabled)" + " Specify which mechanism to use [auto, disabled, import, subprocess]." + " (default: %default)" ), ) diff --git a/src/pip/_internal/network/auth.py b/src/pip/_internal/network/auth.py index 94a82fa6618..f014131562d 100644 --- a/src/pip/_internal/network/auth.py +++ b/src/pip/_internal/network/auth.py @@ -14,7 +14,7 @@ from functools import lru_cache from os.path import commonprefix from pathlib import Path -from typing import Any, Dict, List, NamedTuple, Optional, Tuple +from typing import Any, List, NamedTuple, Optional, Tuple from pip._vendor.requests.auth import AuthBase, HTTPBasicAuth from pip._vendor.requests.models import Request, Response @@ -230,7 +230,14 @@ def __init__( self.prompting = prompting self.index_urls = index_urls self.keyring_provider = keyring_provider # type: ignore[assignment] - self.passwords: Dict[str, AuthInfo] = {} + # In order to avoid prompting the user or its keyring repeatedly + # we cache the credentials required per URL "prefix" and not just by + # netloc. A single server might host multiple independent indexes with + # independent credentials for them so we must be more specific than the + # netloc, thus the URL "prefix". + # The list is kept sorted by decreasing prefix length in order to guarantee + # maximal specificity. + self._required_credentials: list[Tuple[str, AuthInfo]] = [] # When the user is prompted to enter credentials and keyring is # available, we will offer to save them. If the user accepts, # this value is set to the credentials they entered. After the @@ -279,8 +286,8 @@ def _get_keyring_auth( get_keyring_provider.cache_clear() return None - def _get_index_url(self, url: str) -> Optional[str]: - """Return the original index URL matching the requested URL. + def _get_index_url(self, url: str) -> Optional[Tuple[str, str]]: + """Return the common prefix and original index URL matching the requested URL. Cached or dynamically generated credentials may work against the original index URL rather than just the netloc. @@ -302,9 +309,10 @@ def _get_index_url(self, url: str) -> Optional[str]: for index in self.index_urls: index = index.rstrip("/") + "/" - parsed_index = urllib.parse.urlsplit(remove_auth_from_url(index)) + index_without_auth = remove_auth_from_url(index) + parsed_index = urllib.parse.urlsplit(index_without_auth) if parsed_url == parsed_index: - return index + return index_without_auth, index if parsed_url.netloc != parsed_index.netloc: continue @@ -325,119 +333,130 @@ def _get_index_url(self, url: str) -> Optional[str]: ).rfind("/"), ) - return urllib.parse.urlunsplit(candidates[0]) + index = urllib.parse.urlunsplit(candidates[0]) + common_path = ( + commonprefix([parsed_url.path, candidates[0].path]).rsplit("/", 1)[0] + "/" + ) + matching_prefix = urllib.parse.urlunsplit(parsed_url._replace(path=common_path)) + return matching_prefix, index def _get_new_credentials( self, original_url: str, - *, - allow_netrc: bool = True, - allow_keyring: bool = False, - ) -> AuthInfo: - """Find and return credentials for the specified URL.""" + ) -> Tuple[str, AuthInfo]: + """Find credentials for the specified URL and a key for caching them""" # Split the credentials and netloc from the url. - url, netloc, url_user_password = split_auth_netloc_from_url( + url_without_auth, netloc, (username, password) = split_auth_netloc_from_url( original_url, ) + cache_key = url_without_auth # Start with the credentials embedded in the url - username, password = url_user_password if username is not None and password is not None: - logger.debug("Found credentials in url for %s", netloc) - return url_user_password + logger.debug("Found credentials in url for %s", cache_key) + return cache_key, (username, password) # Find a matching index url for this request - index_url = self._get_index_url(url) - if index_url: + index_info = self._get_index_url(url_without_auth) + if index_info: + cache_key, index_url = index_info # Split the credentials from the url. - index_info = split_auth_netloc_from_url(index_url) - if index_info: - index_url, _, index_url_user_password = index_info - logger.debug("Found index url %s", index_url) - - # If an index URL was found, try its embedded credentials - if index_url and index_url_user_password[0] is not None: - username, password = index_url_user_password - if username is not None and password is not None: - logger.debug("Found credentials in index url for %s", netloc) - return index_url_user_password - - # Get creds from netrc if we still don't have them - if allow_netrc: - netrc_auth = get_netrc_auth(original_url) - if netrc_auth: - logger.debug("Found credentials in netrc for %s", netloc) - return netrc_auth - - # If we don't have a password and keyring is available, use it. - if allow_keyring: - # The index url is more specific than the netloc, so try it first - # fmt: off - kr_auth = ( - self._get_keyring_auth(index_url, username) or - self._get_keyring_auth(netloc, username) - ) - # fmt: on - if kr_auth: - logger.debug("Found credentials in keyring for %s", netloc) - return kr_auth - - return username, password + ( + url_without_auth, + _, + (index_username, password), + ) = split_auth_netloc_from_url(index_url) + logger.debug("Found index url %s", url_without_auth) + + # If an index URL was found, try its embedded credentials as long as the + # original and index urls agree on the username. + if ( + index_username is not None + and password is not None + and username in (index_username, None) + ): + logger.debug("Found credentials in index url for %s", cache_key) + return cache_key, (index_username, password) + + # Use the username from the index only if the original does not specify one + if username is None: + username = index_username - def _get_url_and_credentials( - self, original_url: str - ) -> Tuple[str, Optional[str], Optional[str]]: - """Return the credentials to use for the provided URL. + if self.use_keyring: + # We still don't have a password so let's lookup up the url_without_auth in + # the keyring. Note that this is the original_url (without auth) if we did + # not find a matching index otherwise it's the index url (without auth). + kr_auth = self._get_keyring_auth(url_without_auth, username) + if kr_auth is None: + # Still no password so let's try with the netloc only. That also means + # we'll have to cache credentials for a minimal url, i.e.: + # scheme://netloc/. + scheme, netloc, *_ = urllib.parse.urlsplit(cache_key) + cache_key = urllib.parse.urlunsplit((scheme, netloc, "/", None, None)) + kr_auth = self._get_keyring_auth(netloc, username) + if kr_auth and ( # we found some credentials and + username is None # the URL does not constrain the username + or kr_auth[0] == username # or credentials are for the same user + ): + logger.debug("Found credentials in keyring for %s", cache_key) + return cache_key, kr_auth + + # As a last resort, try with netrc. This is as specific as the netloc so we + # have to cache credentials for a minimal url, i.e.: scheme://netloc/. + scheme, netloc, *_ = urllib.parse.urlsplit(cache_key) + cache_key = urllib.parse.urlunsplit((scheme, netloc, "/", None, None)) + netrc_auth = get_netrc_auth(original_url) + if netrc_auth: + logger.debug("Found credentials in netrc for %s", cache_key) + return cache_key, netrc_auth + + # Either username and password are None because neither the original_url nor any + # matching index had credentials and we did not find any entry in keyring or + # netrc. + # Or username is not None and password is None in case the original_url or any + # matching index_url has a userinfo without a `:` and we did not find + # any entry in keyring or netrc to complement the username with a password. + # If both a username and password where found (even if being empty strings) we + # would have exited earlier. + if username is None and password is None: + logger.debug("No credentials found for %s", cache_key) + else: + logger.debug( + "No password found for %s. Will assume username is a token.", cache_key + ) + return cache_key, (username, password) - If allowed, netrc and keyring may be used to obtain the - correct credentials. + def _get_required_credentials(self, url_without_auth: str) -> AuthInfo: + """Return the credentials that were found to be required for the given URL. - Returns (url_without_credentials, username, password). Note - that even if the original URL contains credentials, this - function may return a different username and password. + The URL must not contain any auth. """ - url, netloc, _ = split_auth_netloc_from_url(original_url) - - # Try to get credentials from original url - username, password = self._get_new_credentials(original_url) - - # If credentials not found, use any stored credentials for this netloc. - # Do this if either the username or the password is missing. - # This accounts for the situation in which the user has specified - # the username in the index url, but the password comes from keyring. - if (username is None or password is None) and netloc in self.passwords: - un, pw = self.passwords[netloc] - # It is possible that the cached credentials are for a different username, - # in which case the cache should be ignored. - if username is None or username == un: - username, password = un, pw - - if username is not None or password is not None: - # Convert the username and password if they're None, so that - # this netloc will show up as "cached" in the conditional above. - # Further, HTTPBasicAuth doesn't accept None, so it makes sense to - # cache the value that is going to be used. - username = username or "" - password = password or "" - - # Store any acquired credentials. - self.passwords[netloc] = (username, password) - - assert ( - # Credentials were found - (username is not None and password is not None) - # Credentials were not found - or (username is None and password is None) - ), f"Could not load credentials from url: {original_url}" - - return url, username, password + # Check if a similar URL is known to require credentials + for url_prefix, credentials in self._required_credentials: + if url_without_auth.startswith(url_prefix): + logger.debug("Found required credentials for %s", url_prefix) + return credentials + + # We're not aware of any required credentials for a similar URL. + # We might find out it is the case only after the server answered with + # a 401 Unauthorized response. + return None, None + + def _cache_required_credentials( + self, url_prefix: str, credentials: AuthInfo + ) -> None: + self._required_credentials = sorted( + self._required_credentials + [(url_prefix, credentials)], + reverse=True, + key=lambda v: len(v[0]), + ) def __call__(self, req: Request) -> Request: - # Get credentials for this request - url, username, password = self._get_url_and_credentials(req.url) - # Set the url of the request to the url without any credentials - req.url = url + req.url = remove_auth_from_url(req.url) + + # Get credentials for this request if they're required + username, password = self._get_required_credentials(req.url) if username is not None and password is not None: # Send the basic auth with this request @@ -478,15 +497,7 @@ def handle_401(self, resp: Response, **kwargs: Any) -> Response: if resp.status_code != 401: return resp - username, password = None, None - - # Query the keyring for credentials: - if self.use_keyring: - username, password = self._get_new_credentials( - resp.url, - allow_netrc=False, - allow_keyring=True, - ) + cache_key, (username, password) = self._get_new_credentials(resp.url) # We are not able to prompt the user so simply return the response if not self.prompting and not username and not password: @@ -498,14 +509,23 @@ def handle_401(self, resp: Response, **kwargs: Any) -> Response: save = False if not username and not password: username, password, save = self._prompt_for_password(parsed.netloc) + if username: + # As we prompted the user for credentials for the netloc and + # not a more specific URL, we will cache them for the least + # specific URL, i.e.: scheme://netloc/ + scheme, netloc, *_ = parsed + cache_key = urllib.parse.urlunsplit((scheme, netloc, "/", None, None)) # Store the new username and password to use for future requests self._credentials_to_save = None - if username is not None and password is not None: - self.passwords[parsed.netloc] = (username, password) - + if username is not None: + self._cache_required_credentials(cache_key, (username, password)) # Prompt to save the password to keyring - if save and self._should_save_password_to_keyring(): + if ( + save + and password is not None + and self._should_save_password_to_keyring() + ): self._credentials_to_save = Credentials( url=parsed.netloc, username=username, diff --git a/tests/unit/test_network_auth.py b/tests/unit/test_network_auth.py index 5c12d870156..beb3b71f5fd 100644 --- a/tests/unit/test_network_auth.py +++ b/tests/unit/test_network_auth.py @@ -19,86 +19,46 @@ def reset_keyring() -> Iterable[None]: pip._internal.network.auth.get_keyring_provider.cache_clear() -@pytest.mark.parametrize( - ["input_url", "url", "username", "password"], - [ - ( - "http://user%40email.com:password@example.com/path", - "http://example.com/path", - "user@email.com", - "password", - ), - ( - "http://username:password@example.com/path", - "http://example.com/path", - "username", - "password", - ), - ( - "http://token@example.com/path", - "http://example.com/path", - "token", - "", - ), - ( - "http://example.com/path", - "http://example.com/path", - None, - None, - ), - ], -) -def test_get_credentials_parses_correctly( - input_url: str, url: str, username: Optional[str], password: Optional[str] -) -> None: +def test_cache_required_credentials_maintains_url_prefix_specificity() -> None: auth = MultiDomainBasicAuth() - get = auth._get_url_and_credentials - - # Check URL parsing - assert get(input_url) == (url, username, password) - assert ( - # There are no credentials in the URL - (username is None and password is None) - or - # Credentials were found and "cached" appropriately - auth.passwords["example.com"] == (username, password) - ) + save = auth._cache_required_credentials + expected = [ + ("http://longest.example.com/prefix/", ("user1", "pass1")), + ("https://example.com/other/prefix/", ("user2", "pass2")), + ("http://shorter.example.com/", ("user3", "pass3")), + ("https://short.example.com/", ("user4", "pass4")), + ("http://example.com/short/", ("user5", "pass5")), + ] + # Save credentials in any wrong order + wrongly_ordered = [expected[3], expected[1], expected[2], expected[0], expected[4]] + for url_prefix, credentials in wrongly_ordered: + save(url_prefix, credentials) -def test_get_credentials_not_to_uses_cached_credentials() -> None: - auth = MultiDomainBasicAuth() - auth.passwords["example.com"] = ("user", "pass") + assert auth._required_credentials == expected - got = auth._get_url_and_credentials("http://foo:bar@example.com/path") - expected = ("http://example.com/path", "foo", "bar") - assert got == expected - -def test_get_credentials_not_to_uses_cached_credentials_only_username() -> None: - auth = MultiDomainBasicAuth() - auth.passwords["example.com"] = ("user", "pass") - - got = auth._get_url_and_credentials("http://foo@example.com/path") - expected = ("http://example.com/path", "foo", "") - assert got == expected - - -def test_get_credentials_uses_cached_credentials() -> None: +@pytest.mark.parametrize( + ["input_url", "username", "password"], + [ + ("http://unknown.example.com/path", None, None), + ("http://required.example.com/path/subpath", "username", "password"), + ("http://required.example.com/different/subpath", "token", None), + ], +) +def test_get_required_credentials( + input_url: str, username: Optional[str], password: Optional[str] +) -> None: auth = MultiDomainBasicAuth() - auth.passwords["example.com"] = ("user", "pass") + for url_prefix, credentials in ( + ("http://required.example.com/path/", ("username", "password")), + ("http://required.example.com/different/", ("token", None)), + ): + auth._cache_required_credentials(url_prefix, credentials) - got = auth._get_url_and_credentials("http://example.com/path") - expected = ("http://example.com/path", "user", "pass") - assert got == expected + get = auth._get_required_credentials - -def test_get_credentials_uses_cached_credentials_only_username() -> None: - auth = MultiDomainBasicAuth() - auth.passwords["example.com"] = ("user", "pass") - - got = auth._get_url_and_credentials("http://user@example.com/path") - expected = ("http://example.com/path", "user", "pass") - assert got == expected + assert get(input_url) == (username, password) def test_get_index_url_credentials() -> None: @@ -108,13 +68,17 @@ def test_get_index_url_credentials() -> None: "http://foo:bar@example.com/path", ] ) - get = functools.partial( - auth._get_new_credentials, allow_netrc=False, allow_keyring=False - ) + get = functools.partial(auth._get_new_credentials) # Check resolution of indexes - assert get("http://example.com/path/path2") == ("foo", "bar") - assert get("http://example.com/path3/path2") == (None, None) + assert get("http://example.com/path/path2") == ( + "http://example.com/path/", + ("foo", "bar"), + ) + assert get("http://example.com/path3/path2") == ( + "http://example.com/", + (None, None), + ) def test_prioritize_longest_path_prefix_match_organization() -> None: @@ -124,16 +88,17 @@ def test_prioritize_longest_path_prefix_match_organization() -> None: "http://bar:foo@example.com/org-name-beta/repo-alias/simple", ] ) - get = functools.partial( - auth._get_new_credentials, allow_netrc=False, allow_keyring=False - ) + get = functools.partial(auth._get_new_credentials) # Inspired by Azure DevOps URL structure, GitLab should look similar - assert get("http://example.com/org-name-alpha/repo-guid/dowbload/") == ( - "foo", - "bar", + assert get("http://example.com/org-name-alpha/repo-guid/download/") == ( + "http://example.com/org-name-alpha/", + ("foo", "bar"), + ) + assert get("http://example.com/org-name-beta/repo-guid/download/") == ( + "http://example.com/org-name-beta/", + ("bar", "foo"), ) - assert get("http://example.com/org-name-beta/repo-guid/dowbload/") == ("bar", "foo") def test_prioritize_longest_path_prefix_match_project() -> None: @@ -143,17 +108,15 @@ def test_prioritize_longest_path_prefix_match_project() -> None: "http://bar:foo@example.com/org-alpha/project-name-beta/repo-alias/simple", ] ) - get = functools.partial( - auth._get_new_credentials, allow_netrc=False, allow_keyring=False - ) + get = functools.partial(auth._get_new_credentials) # Inspired by Azure DevOps URL structure, GitLab should look similar assert get( - "http://example.com/org-alpha/project-name-alpha/repo-guid/dowbload/" - ) == ("foo", "bar") + "http://example.com/org-alpha/project-name-alpha/repo-guid/download/" + ) == ("http://example.com/org-alpha/project-name-alpha/", ("foo", "bar")) assert get( - "http://example.com/org-alpha/project-name-beta/repo-guid/dowbload/" - ) == ("bar", "foo") + "http://example.com/org-alpha/project-name-beta/repo-guid/download/" + ) == ("http://example.com/org-alpha/project-name-beta/", ("bar", "foo")) class KeyringModuleV1: @@ -178,19 +141,28 @@ def set_password(self, system: str, username: str, password: str) -> None: @pytest.mark.parametrize( "url, expect", ( - ("http://example.com/path1", (None, None)), + ("http://example.com/path1", ("http://example.com/", (None, None))), # path1 URLs will be resolved by netloc - ("http://user@example.com/path3", ("user", "user!netloc")), - ("http://user2@example.com/path3", ("user2", "user2!netloc")), + ( + "http://user@example.com/path3", + ("http://example.com/", ("user", "user!netloc")), + ), + ( + "http://user2@example.com/path3", + ("http://example.com/", ("user2", "user2!netloc")), + ), # path2 URLs will be resolved by index URL - ("http://example.com/path2/path3", (None, None)), - ("http://foo@example.com/path2/path3", ("foo", "foo!url")), + ("http://example.com/path2/path3", ("http://example.com/", (None, None))), + ( + "http://foo@example.com/path2/path3", + ("http://example.com/path2/", ("foo", "foo!url")), + ), ), ) def test_keyring_get_password( monkeypatch: pytest.MonkeyPatch, url: str, - expect: Tuple[Optional[str], Optional[str]], + expect: Tuple[str, Tuple[Optional[str], Optional[str]]], ) -> None: keyring = KeyringModuleV1() monkeypatch.setitem(sys.modules, "keyring", keyring) @@ -199,7 +171,7 @@ def test_keyring_get_password( keyring_provider="import", ) - actual = auth._get_new_credentials(url, allow_netrc=False, allow_keyring=True) + actual = auth._get_new_credentials(url) assert actual == expect @@ -247,12 +219,16 @@ def test_keyring_get_password_username_in_index( index_urls=["http://user@example.com/path2", "http://example.com/path4"], keyring_provider="import", ) - get = functools.partial( - auth._get_new_credentials, allow_netrc=False, allow_keyring=True - ) + get = functools.partial(auth._get_new_credentials) - assert get("http://example.com/path2/path3") == ("user", "user!url") - assert get("http://example.com/path4/path1") == (None, None) + assert get("http://example.com/path2/path3") == ( + "http://example.com/path2/", + ("user", "user!url"), + ) + assert get("http://example.com/path4/path1") == ( + "http://example.com/", + (None, None), + ) @pytest.mark.parametrize( @@ -280,7 +256,6 @@ def test_keyring_set_password( keyring = KeyringModuleV1() monkeypatch.setitem(sys.modules, "keyring", keyring) auth = MultiDomainBasicAuth(prompting=True, keyring_provider="import") - monkeypatch.setattr(auth, "_get_url_and_credentials", lambda u: (u, None, None)) monkeypatch.setattr(auth, "_prompt_for_password", lambda *a: creds) if creds[2]: # when _prompt_for_password indicates to save, we should save @@ -346,13 +321,22 @@ def get_credential(self, system: str, username: str) -> Optional[Credential]: @pytest.mark.parametrize( "url, expect", ( - ("http://example.com/path1", ("username", "netloc")), - ("http://example.com/path2/path3", ("username", "url")), - ("http://user2@example.com/path2/path3", ("username", "url")), + ( + "http://example.com/", + ("http://example.com/", ("username", "netloc")), + ), + ( + "http://example.com/path2/path3", + ("http://example.com/path2/", ("username", "url")), + ), + ( + "http://user2@example.com/path2/path3", + ("http://example.com/", ("user2", None)), + ), ), ) def test_keyring_get_credential( - monkeypatch: pytest.MonkeyPatch, url: str, expect: Tuple[str, str] + monkeypatch: pytest.MonkeyPatch, url: str, expect: Tuple[str, Tuple[str, str]] ) -> None: monkeypatch.setitem(sys.modules, "keyring", KeyringModuleV2()) auth = MultiDomainBasicAuth( @@ -360,9 +344,7 @@ def test_keyring_get_credential( keyring_provider="import", ) - assert ( - auth._get_new_credentials(url, allow_netrc=False, allow_keyring=True) == expect - ) + assert auth._get_new_credentials(url) == expect class KeyringModuleBroken: @@ -387,9 +369,7 @@ def test_broken_keyring_disables_keyring(monkeypatch: pytest.MonkeyPatch) -> Non assert keyring_broken._call_count == 0 for i in range(5): url = "http://example.com/path" + str(i) - assert auth._get_new_credentials( - url, allow_netrc=False, allow_keyring=True - ) == (None, None) + assert auth._get_new_credentials(url) == ("http://example.com/", (None, None)) assert keyring_broken._call_count == 1 @@ -443,19 +423,28 @@ def check_returncode(self) -> None: @pytest.mark.parametrize( "url, expect", ( - ("http://example.com/path1", (None, None)), + ("http://example.com/path1", ("http://example.com/", (None, None))), # path1 URLs will be resolved by netloc - ("http://user@example.com/path3", ("user", "user!netloc")), - ("http://user2@example.com/path3", ("user2", "user2!netloc")), + ( + "http://user@example.com/path3/", + ("http://example.com/", ("user", "user!netloc")), + ), + ( + "http://user2@example.com/path3", + ("http://example.com/", ("user2", "user2!netloc")), + ), # path2 URLs will be resolved by index URL - ("http://example.com/path2/path3", (None, None)), - ("http://foo@example.com/path2/path3", ("foo", "foo!url")), + ("http://example.com/path2/path3", ("http://example.com/", (None, None))), + ( + "http://foo@example.com/path2/path3", + ("http://example.com/path2/", ("foo", "foo!url")), + ), ), ) def test_keyring_cli_get_password( monkeypatch: pytest.MonkeyPatch, url: str, - expect: Tuple[Optional[str], Optional[str]], + expect: Tuple[str, Tuple[Optional[str], Optional[str]]], ) -> None: monkeypatch.setattr(pip._internal.network.auth.shutil, "which", lambda x: "keyring") monkeypatch.setattr( @@ -466,7 +455,7 @@ def test_keyring_cli_get_password( keyring_provider="subprocess", ) - actual = auth._get_new_credentials(url, allow_netrc=False, allow_keyring=True) + actual = auth._get_new_credentials(url) assert actual == expect @@ -496,7 +485,6 @@ def test_keyring_cli_set_password( keyring = KeyringSubprocessResult() monkeypatch.setattr(pip._internal.network.auth.subprocess, "run", keyring) auth = MultiDomainBasicAuth(prompting=True, keyring_provider="subprocess") - monkeypatch.setattr(auth, "_get_url_and_credentials", lambda u: (u, None, None)) monkeypatch.setattr(auth, "_prompt_for_password", lambda *a: creds) if creds[2]: # when _prompt_for_password indicates to save, we should save diff --git a/tests/unit/test_options.py b/tests/unit/test_options.py index 22ff7f721d7..d450ac637f8 100644 --- a/tests/unit/test_options.py +++ b/tests/unit/test_options.py @@ -513,6 +513,16 @@ def test_no_input(self) -> None: assert options1.no_input assert options2.no_input + def test_keyring_provider(self) -> None: + # FakeCommand intentionally returns the wrong type. + options1, args1 = cast( + Tuple[Values, List[str]], main(["--keyring-provider", "import", "fake"]) + ) + options2, args2 = cast( + Tuple[Values, List[str]], main(["fake", "--keyring-provider", "import"]) + ) + assert options1.keyring_provider == options2.keyring_provider == "import" + def test_proxy(self) -> None: # FakeCommand intentionally returns the wrong type. options1, args1 = cast(