WinVaultKeyring only ever returns last password set

[jaraco]

WinVaultKeyring appears to only ever returns last password set. Win32CryptoKeyring works as expected.

#!python
>>> import keyring
>>> keyring.get_keyring()
<keyring.backend.WinVaultKeyring object at 0x00BFC170>
>>> keyring.set_password('service1', 'user1', 'password1')
>>> keyring.set_password('service1', 'user2', 'password2')
>>> keyring.get_password('service1', 'user1')
u'password2'
>>> keyring.set_password('service2', 'user3', 'password3')
>>> keyring.get_password('service1', 'user1')
u'password2'
# Now try Win32CryptoKeyring
>>> keyring.set_keyring(keyring.backend.Win32CryptoKeyring())
>>> keyring.set_password('service3', 'user1', 'password1')
>>> keyring.set_password('service3', 'user2', 'password2')
>>> keyring.get_password('service3', 'user1')
'password1'
>>> keyring.set_password('service4', 'user3', 'password3')
>>> keyring.get_password('service3', 'user1')
'password1'

---

• Bitbucket: https://bitbucket.org/kang/python-keyring-lib/issue/47
• Originally reported by: alangibson
• Originally created at: 2011-07-01T19:38:51.704

[jaraco]

Thanks for this bug report. Your script for reproducing the issue has been incorporated into the test suite as <<changeset 25a620cb2e7e>>.

---

Original comment by: Jason R. Coombs

[jaraco]

(duplicate message removed)

---

Original comment by: Jason R. Coombs

[jaraco]

Someone posted [[http://stackoverflow.com/questions/7806100/mercurial-tortoisehg-keyring-and-using-two-remote-repos-with-two-usernames-and|a question on Stack Overflow]] about this issue. I can also reproduce it myself.

---

Original comment by: Laurens Holst

[jaraco]

If I look at the Credential Manager, it lists:

{{{
Internet or network address: Mercurial
User name: me@@https://example.org
Password: ••••••••
Persistence: Enterprise
}}}

Looks like it matches on the network address, and the part after the @@ should move there to prevent it from getting replaced?

---

Original comment by: Laurens Holst

[jaraco]

(This is in the context of the Mercurial keyring extension which uses this library. I didn’t realise this was a separate project -_-;;.)

---

Original comment by: Laurens Holst

[jaraco]

I believe there is not a straightforward fix for this issue.

I did put together a patch (attached) which simply prepends the username to the servicename in the WinVault target. Although this patch addresses the issue (and causes the failing test to pass), it will have the unfortunate consequence of invalidating all passwords stored by previous versions of keyring. Furthermore, this causes the username to always be stored in the target, even though the username is also specified in the password record, so that field ends up being stored redundantly.

It seems the WinVault and python-keyring paradigms are somewhat in conflict here: WinVault expects the target to specify only one password for a given service (per Windows User context), but python-keyring expects to be able to store multiple passwords per service (one additional per username).

I'm considering an alternative approach which would maintain backward compatibility, specified thus:

• Passwords will be stored under their simple service name first, and moved to a compound service name, "username@servicename" if there is a collision caused by a subsequent set operation.
• When a password is set, it is always stored under its simple service name. If there is already a service defined under than name but with a different username, that credential is re-saved under its compound service name. Also, any compound service name corresponding to the new credential should be removed (if present).
• When a password is read for a given service, it will be queried first under a simple service name, then under a compound service name.
• When a password is deleted, the delete must be attempted under both the simple and compound service name.

I realize this seems unnecessarily convoluted, but it serves two major benefits:

• It's backward compatible, so existing users of keyring won't feel any pain when adopting this change.

• In the most common use-case, where a single Windows user has exactly one password for a given service, the username isn't stored redundantly in the target. The user has his sole password for that service, and the associated username is stored only in the metadata fields for that credential and not in the service identifier.

I'll mull this over for a while, but I'd like to hear feedback from the community as well if there are any opinions on this issue.

---

Original comment by: Jason R. Coombs

[jaraco]

Fixed in <>

---

Original comment by: Jason R. Coombs