zalando / go-keyring

Cross-platform keyring interface for Go
MIT License
811 stars 82 forks source link

kwallet support #66

Open SoMuchForSubtlety opened 2 years ago

SoMuchForSubtlety commented 2 years ago

This PR replaces #52 from @bearsh

szuecs commented 2 years ago

I see multiple possible issues with this change:

I let @mikkeloscar decide if it's great or not.

SoMuchForSubtlety commented 2 years ago
  • public interface was changed quite a lot

I had to change the signature of SecretService.Delete for it to satisfy the Keyring interface. It would probably be better to create a wrapper type?

Same for the errors, in order to make them accessible in the kwallet and secret_service packages they need to be in a separate package, but this could also be solved with wrapper types. (although I don't think declaring the errors in a different package is a breaking change)

syncjuncture commented 2 years ago

Any progress? @mikkeloscar @SoMuchForSubtlety. Looks like 99designs/keyring supports KWallet.

technodrome commented 2 years ago

Is this still being actively developed?

SoMuchForSubtlety commented 2 years ago

Is this still being actively developed?

I would need feedback from someone from the zalando team to continue.

I ended up only using go-keyring for macOS and falling back to 99designs/keyring for everything else.

szuecs commented 2 years ago

Maybe suggest the change for the SecretService.Delete wrapper type? For me this sounds ok if we can hold the public interface. Error types I am not too much concerned about.

I hope this helps.

mikkeloscar commented 2 years ago

I have lost track of this one, but will have a look tomorrow and give some feedback. Sorry for letting you hanging like this @SoMuchForSubtlety

mikkeloscar commented 2 years ago

I have now had a look and I think it looks quite good. I believe it's ok to break the public interface for the SecretService interface as we only really aim to offer stable support for the public keyring API.

It would be good if you could provide a small guide for the steps needed to migrate in case anyone depends on the SecretService interface.

Also the PR needs a rebase :)

technodrome commented 2 years ago

Nice! Looking forward to trying this out.

technodrome commented 2 years ago

Any news when this is going to be merged?

sm1999 commented 1 year ago

Kwallet recently added support for Secret Service API.

mrueg commented 1 year ago

Kwallet recently added support for Secret Service API.

Looks like this does not work as "plain" Algorithm is not supported. Trying to use it, I get: Algorithm plain is not supported. (only dh-ietf1024-sha256-aes128-cbc-pkcs7 is supported) See: https://invent.kde.org/frameworks/kwallet/-/blob/master/src/runtime/kwalletd/kwalletfreedesktopservice.cpp#L265

Probably either kwallet should add support for plain, or maybe https://github.com/zalando/go-keyring/blob/2a119601fb0034fffe89c0af1f6e7be2ac16f6ab/secret_service/secret_service.go#L66 could be changed to use other algorithms as well.

edit: fwiw I've created an issue with kwallet to support plain as well: https://invent.kde.org/frameworks/kwallet/-/issues/3

SuperSandro2000 commented 1 year ago

FYI kwallet since plasma frameworks 5.97.0 supports the secrets api https://kde.org/announcements/frameworks/5/5.97.0/#:~:text=Introduce%20Secret%20Service%20API