From 7ae5dd0e2e74e00dca9d10cd2716f12e2dd2bac0 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Thu, 1 Aug 2024 12:48:17 -0400 Subject: [PATCH] Deprecate setting a password for an empty username. Ref #668 --- keyring/backend.py | 15 +++++++++++++++ keyring/backends/SecretService.py | 1 + keyring/backends/Windows.py | 1 + keyring/backends/kwallet.py | 1 + keyring/backends/libsecret.py | 1 + keyring/backends/macOS/__init__.py | 1 + keyring/backends/null.py | 5 ++++- keyring/testing/backend.py | 3 ++- 8 files changed, 26 insertions(+), 2 deletions(-) diff --git a/keyring/backend.py b/keyring/backend.py index 7680e4b3..cff9d150 100644 --- a/keyring/backend.py +++ b/keyring/backend.py @@ -10,6 +10,7 @@ import operator import os import typing +import warnings from jaraco.functools import once @@ -100,10 +101,24 @@ def get_password(self, service: str, username: str) -> str | None: """Get password of the username for the service""" return None + def _validate_username(self, username: str) -> None: + """ + Ensure the username is not empty. + """ + if not username: + warnings.warn( + "Empty usernames are deprecated. See #668", + DeprecationWarning, + stacklevel=3, + ) + # raise ValueError("Username cannot be empty") + @abc.abstractmethod def set_password(self, service: str, username: str, password: str) -> None: """Set password for the username of the service. + Implementations should call :meth:`_validate_username`. + If the backend cannot store passwords, raise PasswordSetError. """ diff --git a/keyring/backends/SecretService.py b/keyring/backends/SecretService.py index 9b180bb5..b588c6da 100644 --- a/keyring/backends/SecretService.py +++ b/keyring/backends/SecretService.py @@ -84,6 +84,7 @@ def get_password(self, service, username): def set_password(self, service, username, password): """Set password for the username of the service""" + self._validate_username(username) collection = self.get_preferred_collection() attributes = self._query(service, username, application=self.appid) label = f"Password for '{username}' on '{service}'" diff --git a/keyring/backends/Windows.py b/keyring/backends/Windows.py index 035a9e58..251bc8fb 100644 --- a/keyring/backends/Windows.py +++ b/keyring/backends/Windows.py @@ -113,6 +113,7 @@ def _get_password(self, target): return DecodingCredential(res) def set_password(self, service, username, password): + self._validate_username(username) existing_pw = self._get_password(service) if existing_pw: # resave the existing password using a compound target diff --git a/keyring/backends/kwallet.py b/keyring/backends/kwallet.py index 541d25ea..9f90c865 100644 --- a/keyring/backends/kwallet.py +++ b/keyring/backends/kwallet.py @@ -139,6 +139,7 @@ def set_password(self, service, username, password): if not self.connected(service): # the user pressed "cancel" when prompted to unlock their keyring. raise PasswordSetError("Cancelled by user") + self._validate_username(username) self.iface.writePassword(self.handle, service, username, password, self.appid) def delete_password(self, service, username): diff --git a/keyring/backends/libsecret.py b/keyring/backends/libsecret.py index 280f40b0..1aa37425 100644 --- a/keyring/backends/libsecret.py +++ b/keyring/backends/libsecret.py @@ -82,6 +82,7 @@ def get_password(self, service, username): def set_password(self, service, username, password): """Set password for the username of the service""" + self._validate_username(username) attributes = self._query(service, username, application=self.appid) label = f"Password for '{username}' on '{service}'" try: diff --git a/keyring/backends/macOS/__init__.py b/keyring/backends/macOS/__init__.py index c3734e29..109e4664 100644 --- a/keyring/backends/macOS/__init__.py +++ b/keyring/backends/macOS/__init__.py @@ -42,6 +42,7 @@ def priority(cls): @warn_keychain def set_password(self, service, username, password): + self._validate_username(username) if username is None: username = '' diff --git a/keyring/backends/null.py b/keyring/backends/null.py index c7f05571..f4996d68 100644 --- a/keyring/backends/null.py +++ b/keyring/backends/null.py @@ -17,4 +17,7 @@ def priority(cls) -> float: def get_password(self, service, username, password=None): pass - set_password = delete_password = get_password # type: ignore + delete_password = get_password # type: ignore + + def set_password(self, service, username, password): + self._validate_username(username) diff --git a/keyring/testing/backend.py b/keyring/testing/backend.py index 82e0ef2b..45b0b2aa 100644 --- a/keyring/testing/backend.py +++ b/keyring/testing/backend.py @@ -165,7 +165,8 @@ def test_credential(self): @pytest.mark.xfail("platform.system() == 'Windows'", reason="#668") def test_empty_username(self): - self.set_password('service1', '', 'password1') + with pytest.deprecated_call(): + self.set_password('service1', '', 'password1') assert self.keyring.get_password('service1', '') == 'password1' def test_set_properties(self, monkeypatch):