feat(auth): allow passwords with empty usernames (#10088)

Co-authored-by: Randy Döring <[email protected]>

Author: abn

docs/repositories.md

@@ -531,7 +531,10 @@ required to set an empty password. This is supported by Poetry.
 poetry config http-basic.foo <TOKEN> ""
 ```
 
-**Note:** Usernames cannot be empty.  Attempting to use an empty username can result in an unpredictable failure.
+**Note:** Empty usernames are discouraged. However, Poetry will honour them if a password is configured without it. This
+is unfortunately commonplace practice, while not best practice, for private indices that use tokens. When a password is
+stored into the system keyring with an empty username, Poetry will use a literal `__poetry_source_empty_username__` as
+the username to circumvent [keyring#687](https://github.com/jaraco/keyring/pull/687).
 {{% /note %}}
 
 ## Certificates

src/poetry/utils/password_manager.py

@@ -27,6 +27,13 @@ class HTTPAuthCredential:
 
 
 class PoetryKeyring:
+    # some private sources expect tokens to be provided as passwords with empty userames
+    # we use a fixed literal to ensure that this can be stored in keyring (jaraco/keyring#687)
+    #
+    # Note: If this is changed, users with passwords stored with empty usernames will have to
+    # re-add the config.
+    _EMPTY_USERNAME_KEY = "__poetry_source_empty_username__"
+
     def __init__(self, namespace: str) -> None:
         self._namespace = namespace
 
@@ -41,6 +48,7 @@ def get_credential(
         for name in names:
             credential = None
             try:
+                # we do default to empty username string here since credentials support empty usernames
                 credential = keyring.get_credential(name, username)
             except KeyringLocked:
                 logger.debug("Keyring %s is locked", name)
@@ -61,7 +69,7 @@ def get_password(self, name: str, username: str) -> str | None:
         name = self.get_entry_name(name)
 
         try:
-            return keyring.get_password(name, username)
+            return keyring.get_password(name, username or self._EMPTY_USERNAME_KEY)
         except (RuntimeError, keyring.errors.KeyringError):
             raise PoetryKeyringError(
                 f"Unable to retrieve the password for {name} from the key ring"
@@ -74,7 +82,7 @@ def set_password(self, name: str, username: str, password: str) -> None:
         name = self.get_entry_name(name)
 
         try:
-            keyring.set_password(name, username, password)
+            keyring.set_password(name, username or self._EMPTY_USERNAME_KEY, password)
         except (RuntimeError, keyring.errors.KeyringError) as e:
             raise PoetryKeyringError(
                 f"Unable to store the password for {name} in the key ring: {e}"
@@ -86,7 +94,7 @@ def delete_password(self, name: str, username: str) -> None:
         name = self.get_entry_name(name)
 
         try:
-            keyring.delete_password(name, username)
+            keyring.delete_password(name, username or self._EMPTY_USERNAME_KEY)
         except (RuntimeError, keyring.errors.KeyringError):
             raise PoetryKeyringError(
                 f"Unable to delete the password for {name} from the key ring"
@@ -196,13 +204,11 @@ def get_http_auth(self, repo_name: str) -> HTTPAuthCredential:
         username = self._config.get(f"http-basic.{repo_name}.username")
         password = self._config.get(f"http-basic.{repo_name}.password")
 
-        if not username:
-            return HTTPAuthCredential()
-
         if password is None and self.use_keyring:
             password = self.keyring.get_password(repo_name, username)
 
-        return HTTPAuthCredential(username=username, password=password)
+        # we use `or None` here to ensure that empty strings are passed as None
+        return HTTPAuthCredential(username=username or None, password=password or None)
 
     def set_http_password(self, repo_name: str, username: str, password: str) -> None:
         auth = {"username": username}

tests/conftest.py

@@ -35,6 +35,7 @@
 from poetry.utils.env import EnvManager
 from poetry.utils.env import SystemEnv
 from poetry.utils.env import VirtualEnv
+from poetry.utils.password_manager import PoetryKeyring
 from tests.helpers import MOCK_DEFAULT_GIT_REVISION
 from tests.helpers import TestLocker
 from tests.helpers import TestRepository
@@ -191,6 +192,11 @@ def get_credential(
         raise KeyringError()
 
 
+@pytest.fixture()
+def poetry_keyring() -> PoetryKeyring:
+    return PoetryKeyring("poetry-repository")
+
+
 @pytest.fixture()
 def dummy_keyring() -> DummyBackend:
     return DummyBackend()

tests/utils/test_authenticator.py

@@ -25,6 +25,7 @@
     from pytest import MonkeyPatch
     from pytest_mock import MockerFixture
 
+    from poetry.utils.password_manager import PoetryKeyring
     from tests.conftest import Config
     from tests.conftest import DummyBackend
 
@@ -153,7 +154,7 @@ def test_authenticator_uses_empty_strings_as_default_password(
     assert request.headers["Authorization"] == f"Basic {basic_auth}"
 
 
-def test_authenticator_ignores_empty_strings_as_default_username(
+def test_authenticator_does_not_ignore_empty_strings_as_default_username(
     config: Config,
     mock_remote: None,
     repo: dict[str, dict[str, str]],
@@ -170,7 +171,8 @@ def test_authenticator_ignores_empty_strings_as_default_username(
     authenticator.request("get", "https://foo.bar/files/foo-0.1.0.tar.gz")
 
     request = http.last_request()
-    assert request.headers["Authorization"] is None
+    basic_auth = base64.b64encode(b":bar").decode()
+    assert request.headers["Authorization"] == f"Basic {basic_auth}"
 
 
 def test_authenticator_falls_back_to_keyring_url(
@@ -207,6 +209,7 @@ def test_authenticator_falls_back_to_keyring_netloc(
     http: type[httpretty.httpretty],
     with_simple_keyring: None,
     dummy_keyring: DummyBackend,
+    poetry_keyring: PoetryKeyring,
 ) -> None:
     config.merge(
         {

tests/utils/test_password_manager.py

@@ -1,6 +1,5 @@
 from __future__ import annotations
 
-import contextlib
 import logging
 import os
 
@@ -42,7 +41,7 @@ def test_set_http_password(
     ("username", "password", "is_valid"),
     [
         ("bar", "baz", True),
-        ("", "baz", False),
+        ("", "baz", True),
         ("bar", "", True),
         ("", "", False),
     ],
@@ -53,10 +52,10 @@ def test_get_http_auth(
     is_valid: bool,
     config: Config,
     with_simple_keyring: None,
-    dummy_keyring: DummyBackend,
+    poetry_keyring: PoetryKeyring,
 ) -> None:
-    with contextlib.nullcontext() if username else pytest.warns(DeprecationWarning):
-        dummy_keyring.set_password("poetry-repository-foo", username, password)
+    poetry_keyring.set_password("foo", username, password)
+
     config.auth_config_source.add_property("http-basic.foo", {"username": username})
     manager = PasswordManager(config)
 
@@ -65,8 +64,8 @@ def test_get_http_auth(
 
     if is_valid:
         assert auth is not None
-        assert auth.username == username
-        assert auth.password == password
+        assert auth.username == (username or None)
+        assert auth.password == (password or None)
     else:
         assert auth.username is auth.password is None
 
@@ -137,7 +136,7 @@ def test_set_http_password_with_unavailable_backend(
     ("username", "password", "is_valid"),
     [
         ("bar", "baz", True),
-        ("", "baz", False),
+        ("", "baz", True),
         ("bar", "", True),
         ("", "", False),
     ],
@@ -159,8 +158,8 @@ def test_get_http_auth_with_unavailable_backend(
 
     if is_valid:
         assert auth is not None
-        assert auth.username == username
-        assert auth.password == password
+        assert auth.username == (username or None)
+        assert auth.password == (password or None)
     else:
         assert auth.username is auth.password is None