Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate empty usernames #687

Merged
merged 6 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions keyring/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@

import abc
import copy
import functools
import logging
import operator
import os
import typing
import warnings

from jaraco.functools import once

Expand All @@ -26,18 +28,38 @@

class KeyringBackendMeta(abc.ABCMeta):
"""
A metaclass that's both an ABCMeta and a type that keeps a registry of
all (non-abstract) types.
Specialized subclass behavior.

Keeps a registry of all (non-abstract) types.

Wraps set_password to validate the username.
"""

def __init__(cls, name, bases, dict):
super().__init__(name, bases, dict)
cls._register()
cls._validate_username_in_set_password()

def _register(cls):
if not hasattr(cls, '_classes'):
cls._classes = set()
classes = cls._classes
if not cls.__abstractmethods__:
classes.add(cls)

def _validate_username_in_set_password(cls):
"""
Wrap ``set_password`` such to validate the passed username.
"""
orig = cls.set_password

@functools.wraps(orig)
def wrapper(self, system, username, *args, **kwargs):
self._validate_username(username)
return orig(self, system, username, *args, **kwargs)

cls.set_password = wrapper


class KeyringBackend(metaclass=KeyringBackendMeta):
"""The abstract base class of the keyring, every backend must implement
Expand Down Expand Up @@ -100,6 +122,18 @@ 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.
Expand Down
6 changes: 6 additions & 0 deletions keyring/testing/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,12 @@ def test_credential(self):
('user2', 'password2'),
)

@pytest.mark.xfail("platform.system() == 'Windows'", reason="#668")
def test_empty_username(self):
with pytest.deprecated_call():
self.set_password('service1', '', 'password1')
assert self.keyring.get_password('service1', '') == 'password1'

def test_set_properties(self, monkeypatch):
env = dict(KEYRING_PROPERTY_FOO_BAR='fizz buzz', OTHER_SETTING='ignore me')
monkeypatch.setattr(os, 'environ', env)
Expand Down
1 change: 1 addition & 0 deletions newsfragments/668.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Deprecated support for empty usernames. Now all backends will reject an empty string as input for the 'username' field when setting a password. Later this deprecation will become a more visible user warning and even later an error. If this warning is triggered in your environment, please consider using a static value (even 'username') or comment in the issue and describe the use-case that demands support for empty usernames.
Loading