Skip to content
Open
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
5 changes: 5 additions & 0 deletions keyring/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,17 @@ def _validate_username_in_set_password(cls):
Wrap ``set_password`` such to validate the passed username.
"""
orig = cls.set_password
if getattr(orig, '_validates_username', False):
# inherited from a base class that was already wrapped; wrapping
# again would validate (and warn) once per class in the MRO.
return

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

wrapper._validates_username = True
cls.set_password = wrapper


Expand Down
1 change: 1 addition & 0 deletions newsfragments/+set-password-wrap.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed ``KeyringBackendMeta`` re-wrapping ``set_password`` once per class in the MRO, which made the empty-username deprecation warning fire more than once per call and misattribute its source location.
61 changes: 61 additions & 0 deletions tests/test_backend_meta.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
"""Tests for keyring.backend.KeyringBackendMeta."""

import warnings

import pytest

from keyring import backend


class BaseKeyring(backend.KeyringBackend):
"""A backend defining ``set_password``, so the metaclass wraps it once."""

priority = 1

def get_password(self, service, username):
return None

def set_password(self, service, username, password):
self.saved = (service, username, password)


class DerivedKeyring(BaseKeyring):
"""A subclass inheriting ``set_password``, so it must not be re-wrapped."""


class TestValidateUsernameInSetPassword:
def test_validates_username_once(self):
"""
The metaclass wraps ``set_password`` on every class it creates, but a
subclass that merely inherits ``set_password`` already has the
wrapped version. Re-wrapping makes the username validation (and its
deprecation warning) run once per class in the MRO.
"""
keyring = DerivedKeyring()
with pytest.deprecated_call() as record:
keyring.set_password('svc', '', 'pw')
assert len(record) == 1
assert keyring.saved == ('svc', '', 'pw')

def test_overridden_set_password_is_still_wrapped(self):
"""
A subclass that supplies its own ``set_password`` is not yet
wrapped, so it must be.
"""

class OverridingKeyring(BaseKeyring):
def set_password(self, service, username, password):
self.saved = (service, username, password)

keyring = OverridingKeyring()
with pytest.deprecated_call() as record:
keyring.set_password('svc', '', 'pw')
assert len(record) == 1
assert keyring.saved == ('svc', '', 'pw')

def test_no_warning_for_nonempty_username(self):
keyring = DerivedKeyring()
with warnings.catch_warnings(record=True) as record:
warnings.simplefilter('always')
keyring.set_password('svc', 'user', 'pw')
assert [w for w in record if w.category is DeprecationWarning] == []