Switch to native bindings for macOS Keychain #7123
Labels
blocked
core
This issue is not accepting PRs from outside contributors
enhancement
a request to improve CLI
gh-auth
relating to the gh auth command
Comments
YorikSar
added a commit
to YorikSar/cli
that referenced
this issue
Jul 24, 2023
Switch from github.com/zalando/go-keyring to github.com/99designs/keyring that uses native bindings on macOS and Windows. Originally suggested in cli#7023 (comment) Fixes cli#7123
samcoe
added
the
discuss
samcoe
added
core
|
@YorikSar You are correct in that the prerequisites for this work are now in place. The team is unsure if now is the right time to take on this work as it would be a breaking change and likely require all our users to re-authenticate. |
|
@samcoe Thank you for your reply. I will add code that will convert go-keyring format (prefixed base64-encoded string) to the "new" format (plain data). That would allow to reuse existing token. After that all that user will have to do is authorise |
YorikSar
added a commit
to YorikSar/cli
that referenced
this issue
Aug 7, 2023
Switch from github.com/zalando/go-keyring to github.com/99designs/keyring that uses native bindings on macOS and Windows. On darwin, github.com/zalando/go-keyring encodes secret data in hex or base64, adding a prefix indicating that. If new library returns this prefix, convert the secret to plain data to keep using existing token and not require reauthentication. Originally suggested in cli#7023 (comment) Fixes cli#7123
YorikSar
added a commit
to YorikSar/cli
that referenced
this issue
Aug 7, 2023
Switch from github.com/zalando/go-keyring to github.com/99designs/keyring that uses native bindings on macOS and Windows. On darwin, github.com/zalando/go-keyring encodes secret data in hex or base64, adding a prefix indicating that. If new library returns this prefix, convert the secret to plain data to keep using existing token and not require reauthentication. Originally suggested in cli#7023 (comment) Fixes cli#7123
YorikSar
added a commit
to YorikSar/cli
that referenced
this issue
Aug 7, 2023
Switch from github.com/zalando/go-keyring to github.com/99designs/keyring that uses native bindings on macOS and Windows. On darwin, github.com/zalando/go-keyring encodes secret data in hex or base64, adding a prefix indicating that. If new library returns this prefix, convert the secret to plain data to keep using existing token and not require reauthentication. Originally suggested in cli#7023 (comment) Fixes cli#7123
|
Done. Please see the PR for the additional compatibility code. |
|
Thanks for the new commits! Without having to force people to re-authenticate we are far more interested in merging this work. Unfortunately our team is stretched so thin right now we can't give this work the QA attention it deserves. I'm going to mark this as blocked for now for us to revisit once we have more bandwidth towards the end of the year. |
YorikSar
added a commit
to YorikSar/cli
that referenced
this issue
Nov 3, 2023
Switch from github.com/zalando/go-keyring to github.com/99designs/keyring that uses native bindings on macOS and Windows. On darwin, github.com/zalando/go-keyring encodes secret data in hex or base64, adding a prefix indicating that. If new library returns this prefix, convert the secret to plain data to keep using existing token and not require reauthentication. Originally suggested in cli#7023 (comment) Fixes cli#7123
rwjblue
added a commit
to rwjblue/test-github-device-flow-auth
that referenced
this issue
Feb 8, 2024
This isn't *great*, but it does significantly improve local developer ergonomics. Prior to this change, things worked just fine (WRT the keychain parts), but the ergonomics kinda sucked. This is because, by default, only the application that writes the keychain entry is allowed to read from it. So what would happen during development is that you'd compile, go through the oauth device flow then realize you have to fix some bug, fix the bug, then try to run the application again: 💥 macOS password prompt. The changes here in this commit work around the issue by using `security` (the macOS built-in CLI for handling this stuff). That results in the keychain entry always being read/written by a stable binary so debug builds/re-builds don't have issue. The downside here, is that "anyone" that can execute `security` can read the password. For what it's worth, this is exactly what `gh` does. See some references below: - cli/cli#7043 - https://github.com/zalando/go-keyring - cli/cli#7123 - cli/cli#7023 (comment)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Prerequisites:
Any reason why you are not using https://github.com/99designs/keyring besides the cgo problem? The zalando one forks a
securitycommand on MacOS, which is not a secure practice really.I have to grant access to the
securitycli for the github auth token access, andsecuritycan then be invoked with any other shell script after that, losing control of who I grant access to those creds.The 99designs lib does not have this problem, as it uses native API-s, so MacOS would prompt me to grant access to
ghonly.Using the
securitycli tool directly opens up people's hosts to malicious shell scripts also being able to use thesecuritycli tool and gaining access to the credentials, partially defeating the purpose of storing those secrets in the keychain.Originally posted by @reegnz in #7023 (comment)
The text was updated successfully, but these errors were encountered: