Merge pull request #699 from JamieBeverley/bugfix/wrong_username

Bugfix: WinVaultKeyring.get_credential with non-existent username returns credential of other user (#698)

keyring/backends/Windows.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import logging
 
 from jaraco.context import ExceptionTrap
@@ -94,16 +96,20 @@ def _compound_name(username, service):
         return f'{username}@{service}'
 
     def get_password(self, service, username):
+        res = self._resolve_credential(service, username)
+        return res and res.value
+
+    def _resolve_credential(
+        self, service: str, username: str | None
+    ) -> DecodingCredential | None:
         # first attempt to get the password under the service name
-        res = self._get_password(service)
-        if not res or res['UserName'] != username:
+        res = self._read_credential(service)
+        if not res or username and res['UserName'] != username:
             # It wasn't found so attempt to get it with the compound name
-            res = self._get_password(self._compound_name(username, service))
-        if not res:
-            return None
-        return res.value
+            res = self._read_credential(self._compound_name(username, service))
+        return res
 
-    def _get_password(self, target):
+    def _read_credential(self, target):
         try:
             res = win32cred.CredRead(
                 Type=win32cred.CRED_TYPE_GENERIC, TargetName=target
@@ -115,7 +121,7 @@ def _get_password(self, target):
         return DecodingCredential(res)
 
     def set_password(self, service, username, password):
-        existing_pw = self._get_password(service)
+        existing_pw = self._read_credential(service)
         if existing_pw:
             # resave the existing password using a compound target
             existing_username = existing_pw['UserName']
@@ -142,7 +148,7 @@ def delete_password(self, service, username):
         compound = self._compound_name(username, service)
         deleted = False
         for target in service, compound:
-            existing_pw = self._get_password(target)
+            existing_pw = self._read_credential(target)
             if existing_pw and existing_pw['UserName'] == username:
                 deleted = True
                 self._delete_password(target)
@@ -158,13 +164,5 @@ def _delete_password(self, target):
             raise
 
     def get_credential(self, service, username):
-        res = None
-        # get the credentials associated with the provided username
-        if username:
-            res = self._get_password(self._compound_name(username, service))
-        # get any first password under the service name
-        if not res:
-            res = self._get_password(service)
-            if not res:
-                return None
-        return SimpleCredential(res['UserName'], res.value)
+        res = self._resolve_credential(service, username)
+        return res and SimpleCredential(res['UserName'], res.value)

keyring/testing/backend.py

@@ -181,3 +181,20 @@ def test_new_with_properties(self):
         assert alt.foo == 'bar'
         with pytest.raises(AttributeError):
             self.keyring.foo  # noqa: B018
+
+    def test_wrong_username_returns_none(self):
+        keyring = self.keyring
+        service = 'test_wrong_username_returns_none'
+        cred = keyring.get_credential(service, None)
+        assert cred is None
+
+        password_1 = 'password1'
+        password_2 = 'password2'
+        self.set_password(service, 'user1', password_1)
+        self.set_password(service, 'user2', password_2)
+
+        assert keyring.get_credential(service, "user1").password == password_1
+        assert keyring.get_credential(service, "user2").password == password_2
+
+        # Missing/wrong username should not return a cred
+        assert keyring.get_credential(service, "nobody!") is None

newsfragments/698.bugfix.rst

@@ -0,0 +1 @@
+In get_credential, now returns None when the indicated username is not found.