Fix regression which made libsecret backend unusable in v23.8.0 #587
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
libsecret/secret-schema.h has this enum:
typedef enum {
SECRET_SCHEMA_ATTRIBUTE_STRING = 0,
SECRET_SCHEMA_ATTRIBUTE_INTEGER = 1,
SECRET_SCHEMA_ATTRIBUTE_BOOLEAN = 2,
} SecretSchemaAttributeType;
Because of this, bool(Secret.SchemaAttributeType.STRING) evaluates to
False. So when we do this in schema property, _query code ignores the
second argument:
self._query(
Secret.SchemaAttributeType.STRING,
Secret.SchemaAttributeType.STRING,
application=Secret.SchemaAttributeType.STRING,
)
And it causes any use of libsecret backend to fail with this error:
>>> import keyring.backends.libsecret
>>> k = keyring.backends.libsecret.Keyring()
>>> k.set_password('foo', 'bar', 'baz')
(process:26311): libsecret-CRITICAL **: 19:22:53.299: secret_password_storev_sync: invalid username attribute for org.freedesktop.Secret.Generic schema
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/dmitry/upstream/keyring/keyring/backends/libsecret.py", line 98, in set_password
raise PasswordSetError("Failed to store password!")
keyring.errors.PasswordSetError: Failed to store password!
jaraco
reviewed
Aug 7, 2022
| @@ -250,7 +250,7 @@ def _query(self, service, username=None, **base): | |||
| scheme['username']: username, | |||
| scheme['service']: service, | |||
| } | |||
| if username | |||
| if username is not None | |||
There was a problem hiding this comment.
It's possible some users are relying on username = '' as a non-value for username. We can address that separately if needed.
|
Thanks for the patch. I'd like to develop a regression test for the missed expectation. I'll do that and then merge it. |
|
I realize now we don't have libsecret tested anywhere, so I'm going to pull the test. |
|
Released as v23.8.1. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
libsecret/secret-schema.h has this enum:
Because of this,
bool(Secret.SchemaAttributeType.STRING)evaluates to False. So when we do this, _query code ignores the second argument:keyring/keyring/backends/libsecret.py
Lines 40 to 44 in 0ed196d
And it causes any use of libsecret backend to fail with this error: