search

cli / go-gh

A Go module for interacting with gh and the GitHub API from the command line.
https://pkg.go.dev/github.com/cli/go-gh/v2
MIT License
322 stars 45 forks source link

Check auth status #32

Closed heaths closed 2 years ago

heaths commented 2 years ago

If the CLI isn't already authenticated, extensions can fail in various ways depending on what they call. For example, using gh.CurrentRepository() - even in a repo - fails with errors.New("unable to determine current repository, none of the git remotes configured for this repository point to a known GitHub host"). If you specify a repo explicitly, gh.GQLClient() can fail with an internal config.NotFoundError which, given it's in internal, cannot be referenced so might as well be a generic error. To the user, the error simply prints "not found" which isn't actionable. Even the more wordy error mentioned first isn't that actionable, though more useful.

Instead, what about something like gh.IsAuthenticated() bool? It could, for example, call config.Load() and then check the Config.AuthToken for a known host. This would allow extensions to easily check up front if they should prompt the user to authenticate.

Alternatively, expose any auth-related errors in pkg or something so we can do type assertions.

samcoe commented 2 years ago

@heaths This does seem like a useful feature to have. I took a quick look at #33 and it looks good so far, but I am going to hold off on a full review for now since I have immediate plans to add lots of functionality for working with gh configs and so the implementation of this feature might look significantly different after that work lands. For now I think we can leave #33 open and I will circle back around to it later. Hope you understand.

heaths commented 2 years ago

Understood. #33 also had a couple options I was expecting to explore. Making the errors public, for example, would at least give callers a way to check for specific errors (apart from taking a fragile dependency on the error string) but if the new work could at least provide a consistent, clear message that the user might not be authenticated that would also be helpful. Then it's just a single error type we'd have to check to give a more actionable message - unless go-gh were to do that itself; however, I understand with the current config implementation there could be other underlying reasons no auth token was found for a given host, for example.

samcoe commented 2 years ago

@heaths Thanks for your patients here, I got https://github.com/cli/go-gh/pull/44 merged in which included auth.TokenForHost function which I think solves this issue. I am going to close for now, please let me know if you think there is some functionality missing.