Implementation of keyring-search

[wiimmers]

An implementation of keyring-search into the keyring crate.

So far efforts have been made for all platform specific modules. However, the mock module remains untouched. Some work has been done to attempt to get this to work using generics and now modified keyring-search MockCredentialStore. I am hoping for some collaboration in getting this to work.

The CLI example has been modified to show the functionality of the crate, with the ability to search and create entries from those search results.

The search function uses all three of the keyring-search by methods (by_user, by_target, by_service) and then filters duplicate results. This way clients need not specify extra parameters and simply pass a search query.

[wiimmers]

It is worth mentioning that linux-test.sh has also been modified to test the keyutils specific tests for the search feature.

The line: cargo test --features "linux-no-secret-service" --verbose has been added to handle this change. I would imagine that Cargo.toml could potentially be modified to alleviate the need for this but this was how I chose to handle the change for the time being.

[brotskydotcom]

Thanks so much for this contribution! I'm very busy right now working on a libdbus-based (fully synchronous) version of secret-service to go into a new major version (along with the newly-released async secret-service crate), and I'm hoping we can get this PR ready to go into that new release as well (which will allow us to make non-backwards-compatible changes to the API if necessary). Are there any such changes that you think you make this a better fit?

[wiimmers]

The current API worked well with the search library, and implementing entry creation from search results was relatively painless. From there they work as any other entry would. One thing that I found through working with this was a minor inconvenience regarding 'service' and 'comment' on the Windows platform. If an entry is created from a searched credential then keyring chooses to build a comment for that credential. This leads to long comments that, after a password is set and stored, don't necessarily make sense. If there is some way to prevent the duplicity or mangled nature of these comments, I believe that would improve the functionality a bit. However, unless using keyring-search or keyring, Windows credential manager does not display these comments and thus is not imperative to proper functionality of the crates.

[brotskydotcom]

Hey @wiimmers, I see your point. I can probably fix this issue with the v3 release, because that's where I can make breaking changes in the way the credentials are formatted (or introduce new API in the credential stores to allow setting these values). Can you try rebasing this PR against the current master and we can take a look at what might be needed? I can also look at supporting search in the mock store.

[brotskydotcom]

Hey there, I've had a chance to look over the code now, and I think maybe if we change approach this can be done much more simply (and with a bit more generality). Instead of doing the search in our entry structure, I think we can just let people do the search in your crate, and then once they have the desired credential in hand they can just hand it to the keyring's keystore and let the keystone create the Entry. That way there's just one new entry point in each keystore - call it entry_from_credential, and there are no new Entry apis at all (except maybe one that explains whether this entry was created directly from a specific credential or not). The other advantage of this approach is that you can pretty much make any credential into an Entry, because the keystore can add whatever attributes it needs to the entry to make that work.

Is this making any sense to you? Does it sound like a good idea?

[wiimmers]

Yes! I like the sound of that and I am all for simplicity and added generality in my mind just means more use cases for keyring. Apologies for my sparse replies, big changes for family and me this past week. I began rebasing and going through conflicts yesterday but have not finished. How do you suggest I move forward from here? I can start creating entry_from_credential in the keystores and create a new PR unless you have another preference.

[brotskydotcom]

Hi @wiimmers my fingers are crossed that the "big changes" for your family are good ones! Yes, you and I agree: I would like to close this PR and then if you can update to the released master and start work on a separate PR for entry_from_credential that would be ideal. No rush! Since it will be an additive API we can always release it in a dot whenever it's ready.