From 27d0e0ea3ea75ad7400e9d1d42bcbbbe088aaa01 Mon Sep 17 00:00:00 2001 From: Jamie Beverley Date: Thu, 3 Oct 2024 09:19:54 -0400 Subject: [PATCH 1/8] fix: passing a username not associated to a credential to get_credential for WinVaultKeyring returns None --- keyring/backends/Windows.py | 7 ++++--- keyring/testing/backend.py | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/keyring/backends/Windows.py b/keyring/backends/Windows.py index f9652487..56168479 100644 --- a/keyring/backends/Windows.py +++ b/keyring/backends/Windows.py @@ -164,7 +164,8 @@ def get_credential(self, service, 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 + cred = self._get_password(service) + res = cred if username is None or username == cred["UserName"] else None + if not res: + return None return SimpleCredential(res['UserName'], res.value) diff --git a/keyring/testing/backend.py b/keyring/testing/backend.py index 45b0b2aa..d2f3d2a2 100644 --- a/keyring/testing/backend.py +++ b/keyring/testing/backend.py @@ -181,3 +181,21 @@ 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 + # Most recent entered + assert keyring.get_credential(service, None).password == password_2 + # Missing/wrong username should not return a cred + assert keyring.get_credential(service, "nobody!") is None From ecf05b877597f19e8f10bafb91cf10504155b600 Mon Sep 17 00:00:00 2001 From: Jamie Beverley Date: Thu, 3 Oct 2024 21:10:42 -0400 Subject: [PATCH 2/8] remove faulty test assertion (presumably a result of some async dbus thing on linux?) --- keyring/testing/backend.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/keyring/testing/backend.py b/keyring/testing/backend.py index d2f3d2a2..74c1e476 100644 --- a/keyring/testing/backend.py +++ b/keyring/testing/backend.py @@ -195,7 +195,6 @@ def test_wrong_username_returns_none(self): assert keyring.get_credential(service, "user1").password == password_1 assert keyring.get_credential(service, "user2").password == password_2 - # Most recent entered - assert keyring.get_credential(service, None).password == password_2 + # Missing/wrong username should not return a cred assert keyring.get_credential(service, "nobody!") is None From 916c0390067b774b062fe01ac829bcb1ff693626 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sat, 26 Oct 2024 09:59:06 -0400 Subject: [PATCH 3/8] Add news fragment. --- newsfragments/698.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/698.bugfix.rst diff --git a/newsfragments/698.bugfix.rst b/newsfragments/698.bugfix.rst new file mode 100644 index 00000000..a07ac6da --- /dev/null +++ b/newsfragments/698.bugfix.rst @@ -0,0 +1 @@ +In get_credential, now returns None when the indicated username is not found. \ No newline at end of file From 562c69fdf66a9fba6d7aeb12b778f8a4fc1580b8 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sat, 26 Oct 2024 10:00:06 -0400 Subject: [PATCH 4/8] =?UTF-8?q?=F0=9F=91=B9=20Feed=20the=20hobgoblins=20(d?= =?UTF-8?q?elint).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- keyring/testing/backend.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keyring/testing/backend.py b/keyring/testing/backend.py index 74c1e476..89a414bf 100644 --- a/keyring/testing/backend.py +++ b/keyring/testing/backend.py @@ -195,6 +195,6 @@ def test_wrong_username_returns_none(self): 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 From 2f3f4eccdf3ab178235128aa9409e2895d21ecd8 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sat, 26 Oct 2024 10:22:59 -0400 Subject: [PATCH 5/8] Updated comment to match new behavior. --- keyring/backends/Windows.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keyring/backends/Windows.py b/keyring/backends/Windows.py index 56168479..33bed8b1 100644 --- a/keyring/backends/Windows.py +++ b/keyring/backends/Windows.py @@ -162,7 +162,7 @@ def get_credential(self, service, username): # 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 + # get a credential matching service and username if provided if not res: cred = self._get_password(service) res = cred if username is None or username == cred["UserName"] else None From f33fcfb6421a532db2ab87a486fb074859631ef5 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sat, 26 Oct 2024 10:24:54 -0400 Subject: [PATCH 6/8] Restore the indentation on the check, keeping the logic in the only place it's reachable (and limiting the diff). --- keyring/backends/Windows.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/keyring/backends/Windows.py b/keyring/backends/Windows.py index 33bed8b1..ae4a71b2 100644 --- a/keyring/backends/Windows.py +++ b/keyring/backends/Windows.py @@ -166,6 +166,6 @@ def get_credential(self, service, username): if not res: cred = self._get_password(service) res = cred if username is None or username == cred["UserName"] else None - if not res: - return None + if not res: + return None return SimpleCredential(res['UserName'], res.value) From 5e4b99eefb5e905836dc2259d44e0c90b11263ba Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sat, 26 Oct 2024 11:18:51 -0400 Subject: [PATCH 7/8] Unified the Windows.get_password and .get_credential logic through a new _resolve_credential method. --- keyring/backends/Windows.py | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/keyring/backends/Windows.py b/keyring/backends/Windows.py index ae4a71b2..2c81207f 100644 --- a/keyring/backends/Windows.py +++ b/keyring/backends/Windows.py @@ -94,16 +94,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 +119,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 +146,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,14 +162,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 a credential matching service and username if provided - if not res: - cred = self._get_password(service) - res = cred if username is None or username == cred["UserName"] else None - 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) From 103c53550cf3e9904666c43138d32e827befe0ac Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Sat, 26 Oct 2024 11:23:02 -0400 Subject: [PATCH 8/8] =?UTF-8?q?=F0=9F=A7=8E=E2=80=8D=E2=99=80=EF=B8=8F=20G?= =?UTF-8?q?enuflect=20to=20the=20types.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- keyring/backends/Windows.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/keyring/backends/Windows.py b/keyring/backends/Windows.py index 2c81207f..110075b2 100644 --- a/keyring/backends/Windows.py +++ b/keyring/backends/Windows.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import logging from jaraco.context import ExceptionTrap