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
26 changes: 17 additions & 9 deletions keyring/backends/Windows.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand All @@ -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:
Expand Down
152 changes: 152 additions & 0 deletions tests/backends/test_Windows.py
Original file line number Diff line number Diff line change
@@ -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


Expand Down Expand Up @@ -69,3 +71,153 @@ 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')

responses = {"service": cred_service, "user2@service": cred_compound}

with mock.patch.object(kr, '_read_credential', side_effect=responses.get):
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'