osxkeychain: switch to github.com/keybase/go-keychain

[crazy-max]

closes #280

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 71.15385% with 15 lines in your changes missing coverage. Please review.

Project coverage is 50.54%. Comparing base (8438667) to head (ffe5a98).

Files with missing lines	Patch %	Lines
osxkeychain/osxkeychain.go	71.15%	8 Missing and 7 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #282      +/-   ##
==========================================
- Coverage   53.10%   50.54%   -2.57%     
==========================================
  Files           9        9              
  Lines         676      645      -31     
==========================================
- Hits          359      326      -33     
- Misses        272      273       +1     
- Partials       45       46       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[thaJeztah]

Haven't checked yet, but possibly this also fixes #177

[thaJeztah]

So the only concerns I have is that this

• adds a new dependency (it looks actively maintained, but doesn't have tagged releases yet)
• do we know what the scope is of the dependency? you mentioned secretservice, and it looks indeed that they implemented a package for that as well in https://github.com/keybase/go-keychain/tree/c2ce0606900517e98f5dbfbd7d03f91424bc5867/secretservice
• ^^ that can be both a "positive" (perhaps it's an implementation we could also use), but could also be a "negative" if they start adding <unlimited> other back-ends

In general, I'm wondering if we should start to reorganise this repository into multiple modules, because it's now a "mono-repo" containing both the client / API / library code (client, credentials, registryurl), as well as actual implementations (wincreds, osxkeychain, secretservice, pass), with (possibly) more implementations to be added (see #268, #235)

Some possible approaches;

• make each implementation its own module (which can also be tagged separately)
• somehow refactor the client / API / library code to become its own module (and tag that separately)
• ^^ using vendoring would probably be a bit of a challenge though (vendoring usually would be for binary code, which would now potentially be spread over multiple locations, unless we have a implementation/ or cmd/ directory shared between implementations)

[crazy-max]

• do we know what the scope is of the dependency? you mentioned secretservice, and it looks indeed that they implemented a package for that as well in https://github.com/keybase/go-keychain/tree/c2ce0606900517e98f5dbfbd7d03f91424bc5867/secretservice

• ^^ that can be both a "positive" (perhaps it's an implementation we could also use), but could also be a "negative" if they start adding <unlimited> other back-ends

Added support for secretservice as well: #290

[akerouanton]

We can probably start testing it on macOS 15. Otherwise code LGTM.

[akerouanton]

adds a new dependency (it looks actively maintained, but doesn't have tagged releases yet)
do we know what the scope is of the dependency? you mentioned secretservice, and it looks indeed that they implemented a package for that as well in https://github.com/keybase/go-keychain/tree/c2ce0606900517e98f5dbfbd7d03f91424bc5867/secretservice
^^ that can be both a "positive" (perhaps it's an implementation we could also use), but could also be a "negative" if they start adding other back-ends

It didn't receive any new backend implementation since this PR was opened. So this should clear out any concerns about it becoming bloated (from our point-of-view).

It seems to be in 'maintenance mode' but given that it supports a limited number of backends, and these tend to be stable by nature I guess that's a good thing.

We can still decide to check out / fork it if we have any problems in the future, but for I now I guess it's okay to pull that depedency in.

@thaJeztah I think your other comments are not actionable at this point (or at least not in this PR). Do you have any other concerns, or can we merge it as is?

[crazy-max]

rebased after #357 merged

[thaJeztah]

LGTM