Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read gh token from gh proper if not found in other locations #107

Merged
merged 2 commits into from
Feb 22, 2023

Conversation

mislav
Copy link
Contributor

@mislav mislav commented Feb 20, 2023

GitHub CLI is soon moving away from storing the token in plain text configuration file. It will prefer to read the token from system's keyring storage.

As an unfortunate result, extensions will start failing now that token isn't in the YAML file anymore. But, we also don't want individual extensions to interface with the keyring. Instead, extensions will now shell out to gh config get oauth_token --host HOSTNAME to obtain the token.

This introduces a dependency on gh proper to be in PATH, but I consider that to be a reasonable request for extensions.

Delegating to the gh executable to obtain the token allows us to fine-tune how GitHub CLI interfaces with keyring storage in the future without affecting extensions again.

GitHub CLI is moving away from storing the token in plain text configuration file. It will prefer to read the token from system's keyring storage.

As an unfortunate result, extensions will start failing now that token isn't in the YAML file anymore. But, we also don't want individual extensions to interface with the keyring. Instead, extensions will now shell out to `gh config get oauth_token --host HOSTNAME` to obtain the token.

This introduces a dependency on gh proper to be in PATH, but I consider that to be a reasonable request for extensions.
Copy link
Contributor

@samcoe samcoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Just left one question for you and would be nice to add a test for this new behavior.

pkg/auth/auth.go Outdated Show resolved Hide resolved
@samcoe samcoe merged commit 6f61141 into trunk Feb 22, 2023
@samcoe samcoe deleted the token-from-gh branch February 22, 2023 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants