Skip to content

Commit

Permalink
Deprecate setting a password for an empty username.
Browse files Browse the repository at this point in the history
Ref #668
  • Loading branch information
jaraco committed Aug 1, 2024
1 parent 5d117cb commit a13a477
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 2 deletions.
15 changes: 15 additions & 0 deletions keyring/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import operator
import os
import typing
import warnings

from jaraco.functools import once

Expand Down Expand Up @@ -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 ``._validate_username``.
If the backend cannot store passwords, raise
PasswordSetError.
"""
Expand Down
1 change: 1 addition & 0 deletions keyring/backends/SecretService.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}'"
Expand Down
1 change: 1 addition & 0 deletions keyring/backends/Windows.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions keyring/backends/kwallet.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
1 change: 1 addition & 0 deletions keyring/backends/libsecret.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions keyring/backends/macOS/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ''

Expand Down
5 changes: 4 additions & 1 deletion keyring/backends/null.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
3 changes: 2 additions & 1 deletion keyring/testing/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit a13a477

Please sign in to comment.