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
14 changes: 14 additions & 0 deletions commit_msg.txt
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't commit this file.

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
Suppress PasswordDeleteError in ChainerBackend.delete_password

When deleting a password through ChainerBackend, if the
highest-priority backend raises PasswordDeleteError (because it
doesn't hold that particular password), the chainer now catches
the error and continues to the next backend instead of
propagating immediately.

This matches the existing pattern for NotImplementedError and
fixes the case where a password exists in a lower-priority
backend but cannot be deleted because a higher-priority backend
fails first.

Fixes #697
3 changes: 2 additions & 1 deletion keyring/backends/chainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from .. import backend
from ..compat import properties
from ..errors import PasswordDeleteError
from . import fail


Expand Down Expand Up @@ -61,7 +62,7 @@ def delete_password(self, service, username):
for keyring in self.backends:
try:
return keyring.delete_password(service, username)
except NotImplementedError:
except (NotImplementedError, PasswordDeleteError):
pass

def get_credential(self, service, username):
Expand Down
17 changes: 17 additions & 0 deletions tests/backends/test_chainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import keyring.backends.chainer
from keyring import backend
from keyring.errors import PasswordDeleteError


@pytest.fixture
Expand All @@ -16,6 +17,9 @@ def get_password(self, system, user):
def set_password(self, system, user, password):
pass

def delete_password(self, system, user):
pass

class Keyring2(backend.KeyringBackend):
priority = 2

Expand All @@ -25,6 +29,9 @@ def get_password(self, system, user):
def set_password(self, system, user, password):
raise NotImplementedError()

def delete_password(self, system, user):
raise PasswordDeleteError("Password not found")

return Keyring1(), Keyring2()

monkeypatch.setattr('keyring.backend.get_all_keyring', get_two)
Expand All @@ -45,3 +52,13 @@ def test_chainer_defers_to_fail(self, monkeypatch):
assert keyring.backend.by_priority(
keyring.backends.chainer.ChainerBackend
) < keyring.backend.by_priority(keyring.backends.fail.Keyring)

def test_chainer_delete_skips_missing(self, two_keyrings):
"""
When a higher-priority backend raises PasswordDeleteError,
the chainer should fall through to the next backend.
"""
chainer = keyring.backends.chainer.ChainerBackend()
# Should not raise even though Keyring2 (higher priority)
# raises PasswordDeleteError
chainer.delete_password('alpha', 'bravo')
Loading