From e6f5d737070054fc03dde6e25bdec367acbd1e28 Mon Sep 17 00:00:00 2001 From: Varun Chawla Date: Fri, 13 Feb 2026 00:53:17 -0800 Subject: [PATCH 1/4] Fix case-sensitive username comparison in WinVaultKeyring The WinVaultKeyring backend was comparing usernames with exact case-sensitive equality, causing inconsistent behavior on Windows where usernames are case-insensitive. For example, storing a password for 'USER' and then retrieving it with 'user' would fail to find the credential. Introduce a _username_match helper for case-insensitive comparison and apply it in _resolve_credential, set_password, and delete_password. Also skip redundant compound-name creation in set_password when the same user (regardless of case) updates their password for a service. Fixes #736 --- keyring/backends/Windows.py | 26 ++++-- tests/backends/test_Windows.py | 159 +++++++++++++++++++++++++++++++++ 2 files changed, 176 insertions(+), 9 deletions(-) diff --git a/keyring/backends/Windows.py b/keyring/backends/Windows.py index 110075b2..27e85f5e 100644 --- a/keyring/backends/Windows.py +++ b/keyring/backends/Windows.py @@ -62,6 +62,13 @@ def value(self): return decoded_cred_utf8 +def _username_match(a: str | None, b: str | None) -> bool: + """Compare usernames case-insensitively to match Windows behavior.""" + if a is None or b is None: + return a is b + return a.lower() == b.lower() + + class WinVaultKeyring(KeyringBackend): """ WinVaultKeyring stores encrypted passwords using the Windows Credential @@ -104,7 +111,7 @@ def _resolve_credential( ) -> DecodingCredential | None: # first attempt to get the password under the service name res = self._read_credential(service) - if not res or username and res['UserName'] != username: + if not res or username and not _username_match(res['UserName'], username): # It wasn't found so attempt to get it with the compound name res = self._read_credential(self._compound_name(username, service)) return res @@ -123,14 +130,15 @@ def _read_credential(self, target): def set_password(self, service, username, password): existing_pw = self._read_credential(service) if existing_pw: - # resave the existing password using a compound target existing_username = existing_pw['UserName'] - target = self._compound_name(existing_username, service) - self._set_password( - target, - existing_username, - existing_pw.value, - ) + if not _username_match(existing_username, username): + # resave the existing password using a compound target + target = self._compound_name(existing_username, service) + self._set_password( + target, + existing_username, + existing_pw.value, + ) self._set_password(service, username, str(password)) def _set_password(self, target, username, password): @@ -149,7 +157,7 @@ def delete_password(self, service, username): deleted = False for target in service, compound: existing_pw = self._read_credential(target) - if existing_pw and existing_pw['UserName'] == username: + if existing_pw and _username_match(existing_pw['UserName'], username): deleted = True self._delete_password(target) if not deleted: diff --git a/tests/backends/test_Windows.py b/tests/backends/test_Windows.py index b7b02e9f..378fb052 100644 --- a/tests/backends/test_Windows.py +++ b/tests/backends/test_Windows.py @@ -1,8 +1,10 @@ import sys +from unittest import mock import pytest import keyring.backends.Windows +from keyring.backends.Windows import _username_match from keyring.testing.backend import UNICODE_CHARS, BackendBasicTests @@ -69,3 +71,160 @@ def test_winvault_always_viable(): The WinVault backend should always be viable on Windows. """ assert keyring.backends.Windows.WinVaultKeyring.viable + + +class TestUsernameMatch: + """Tests for case-insensitive username comparison.""" + + def test_same_case(self): + assert _username_match('user', 'user') + + def test_different_case(self): + assert _username_match('USER', 'user') + assert _username_match('user', 'USER') + assert _username_match('User', 'uSER') + + def test_not_matching(self): + assert not _username_match('user1', 'user2') + + def test_none_values(self): + assert _username_match(None, None) + assert not _username_match('user', None) + assert not _username_match(None, 'user') + + def test_empty_string(self): + assert _username_match('', '') + assert not _username_match('', 'user') + + +class TestWinVaultCaseInsensitive: + """ + Test that WinVaultKeyring handles usernames case-insensitively. + + Uses mocking since win32cred is not available on non-Windows platforms. + See https://github.com/jaraco/keyring/issues/736 + """ + + def _make_credential(self, username, password): + return keyring.backends.Windows.DecodingCredential( + { + 'UserName': username, + 'CredentialBlob': password.encode('utf-16'), + } + ) + + def _make_keyring(self): + kr = keyring.backends.Windows.WinVaultKeyring.__new__( + keyring.backends.Windows.WinVaultKeyring + ) + return kr + + def test_get_password_different_case(self): + """get_password should find a credential regardless of username case.""" + kr = self._make_keyring() + cred = self._make_credential('USER', 'secret') + + with mock.patch.object(kr, '_read_credential', return_value=cred): + result = kr.get_password('service', 'user') + assert result == 'secret' + + def test_get_password_exact_case(self): + """get_password should work with matching case.""" + kr = self._make_keyring() + cred = self._make_credential('user', 'secret') + + with mock.patch.object(kr, '_read_credential', return_value=cred): + result = kr.get_password('service', 'user') + assert result == 'secret' + + def test_resolve_credential_case_insensitive(self): + """_resolve_credential should match usernames case-insensitively.""" + kr = self._make_keyring() + cred = self._make_credential('Admin', 'pass') + + with mock.patch.object(kr, '_read_credential', return_value=cred): + result = kr._resolve_credential('service', 'admin') + assert result is cred + + def test_resolve_credential_falls_through_on_mismatch(self): + """_resolve_credential should try compound name for different users.""" + kr = self._make_keyring() + cred_service = self._make_credential('user1', 'pass1') + cred_compound = self._make_credential('user2', 'pass2') + + def fake_read(target): + if target == 'service': + return cred_service + if target == 'user2@service': + return cred_compound + return None + + with mock.patch.object(kr, '_read_credential', side_effect=fake_read): + result = kr._resolve_credential('service', 'user2') + assert result is cred_compound + + def test_set_password_same_user_different_case(self): + """ + set_password should not create compound entry when the same user + (different case) updates their password. + """ + kr = self._make_keyring() + existing = self._make_credential('USER', 'old_pass') + + with ( + mock.patch.object(kr, '_read_credential', return_value=existing), + mock.patch.object(kr, '_set_password') as mock_set, + ): + kr.set_password('service', 'user', 'new_pass') + + # Should only call _set_password once (update in place), + # not twice (which would mean it moved to compound) + mock_set.assert_called_once_with('service', 'user', 'new_pass') + + def test_set_password_different_user_creates_compound(self): + """ + set_password should move existing credential to compound name + when a different user sets a password for the same service. + """ + kr = self._make_keyring() + existing = self._make_credential('user1', 'pass1') + + with ( + mock.patch.object(kr, '_read_credential', return_value=existing), + mock.patch.object(kr, '_set_password') as mock_set, + ): + kr.set_password('service', 'user2', 'pass2') + + assert mock_set.call_count == 2 + mock_set.assert_any_call('user1@service', 'user1', 'pass1') + mock_set.assert_any_call('service', 'user2', 'pass2') + + def test_delete_password_case_insensitive(self): + """delete_password should match usernames case-insensitively.""" + kr = self._make_keyring() + cred = self._make_credential('USER', 'secret') + + def fake_read(target): + if target == 'service': + return cred + return None + + with ( + mock.patch.object(kr, '_read_credential', side_effect=fake_read), + mock.patch.object(kr, '_delete_password') as mock_del, + ): + kr.delete_password('service', 'user') + + mock_del.assert_called_once_with('service') + + def test_get_credential_case_insensitive(self): + """get_credential should find credentials case-insensitively.""" + kr = self._make_keyring() + cred = self._make_credential('Admin', 'secret') + + with mock.patch.object(kr, '_read_credential', return_value=cred): + result = kr.get_credential('service', 'admin') + + assert result is not None + assert result.username == 'Admin' + assert result.password == 'secret' From 866d7c51259b64fdf657d7fc1d4fa928a4e4c7a5 Mon Sep 17 00:00:00 2001 From: Varun Chawla Date: Fri, 13 Feb 2026 22:54:39 -0800 Subject: [PATCH 2/4] Fix ruff format for test_Windows.py --- tests/backends/test_Windows.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/backends/test_Windows.py b/tests/backends/test_Windows.py index 378fb052..c349fdb1 100644 --- a/tests/backends/test_Windows.py +++ b/tests/backends/test_Windows.py @@ -106,12 +106,10 @@ class TestWinVaultCaseInsensitive: """ def _make_credential(self, username, password): - return keyring.backends.Windows.DecodingCredential( - { - 'UserName': username, - 'CredentialBlob': password.encode('utf-16'), - } - ) + return keyring.backends.Windows.DecodingCredential({ + 'UserName': username, + 'CredentialBlob': password.encode('utf-16'), + }) def _make_keyring(self): kr = keyring.backends.Windows.WinVaultKeyring.__new__( From 2ece204f3f391ba7f8b002e63fc859fc58bac9f8 Mon Sep 17 00:00:00 2001 From: Varun Chawla Date: Sat, 14 Feb 2026 23:07:16 -0800 Subject: [PATCH 3/4] Fix diffcov: eliminate uncovered line in test_Windows.py Replace the fake_read function with a dict lookup so there's no unreachable fallback return statement at line 158. --- tests/backends/test_Windows.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/backends/test_Windows.py b/tests/backends/test_Windows.py index c349fdb1..bffd1dd3 100644 --- a/tests/backends/test_Windows.py +++ b/tests/backends/test_Windows.py @@ -150,14 +150,9 @@ def test_resolve_credential_falls_through_on_mismatch(self): cred_service = self._make_credential('user1', 'pass1') cred_compound = self._make_credential('user2', 'pass2') - def fake_read(target): - if target == 'service': - return cred_service - if target == 'user2@service': - return cred_compound - return None + responses = {"service": cred_service, "user2@service": cred_compound} - with mock.patch.object(kr, '_read_credential', side_effect=fake_read): + with mock.patch.object(kr, '_read_credential', side_effect=responses.get): result = kr._resolve_credential('service', 'user2') assert result is cred_compound From 38a580294f5e36d4f881511fcf2c8bfb31f71c9e Mon Sep 17 00:00:00 2001 From: Varun Chawla Date: Mon, 23 Feb 2026 23:41:00 -0800 Subject: [PATCH 4/4] Retrigger CI to pick up updated mypy constraint for PyPy