Repository navigation
show token scopes on auth status - #6546
Conversation
cliAutomation
added
the
external
samcoe
left a comment
samcoe
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the contribution. I pushed a small 💅 commit to cleanup some small style nits I had.
| if scopesHeader != "" { | ||
| addMsg("%s Token Scopes: %s", cs.SuccessIcon(), scopesHeader) | ||
| } else { | ||
| addMsg("%s Token Scopes: None found", cs.Red("X")) |
There was a problem hiding this comment.
Rendering this as an error in red color might be misleading for Integration tokens and PAT v2, which have a concept of "permissions" but not scopes, and thus do not render any x-oauth-scopes response header.
Maybe it's best to just not render information about "scopes" in that case and instead print the type of token used?
There was a problem hiding this comment.
@mislav thanks, I made the changes and updated the PR body to reflect if we don't know the token type and offer more information. If you think it's clearer, I can remove it entirely in those cases.
Thinking about this, I wonder if
- the current output is perhaps not correct with a server-to-server-token, as we can't know if the token has the scopes needed for cli operations
- we should modify the message when we don't know the user's login (see the output from a fine-grained token)
- we should modify some of the other output when using a fine-grained token, where we also show an error message
I feel like these may be out of scope from the original issue, but wanted to bring them up.
- show path to config file - display token prefix - refine how missing scopes are shown
There was a problem hiding this comment.
Thanks @mntlty for the contribution and @samcoe for reviewing.
I've pushed some changes:
- When a token for a specific host is invalid (i.e. results in a HTTP 4xx), this restores the original error message that prints:
The {host} token in {source} is no longer valid. - Unless the token comes from environment variables, the value for
{source}is now a file path tohosts.ymlinstead of a hardcoded stringoauth_token(which might be a bug in go-gh) - When printing a masked token, print the token prefix like
ghp_unmasked so that the user can inspect which type of token they are using - Only print scopes information for “classic” PATs and OAuth tokens
gh auth statustests now assert exact command output instead of matching with regexp
|
@mislav can this be merged? |
|
Yes. I guess that because the scope of my added changes, I wanted to give you a chance to weigh in. |
Closes #6520
The current query to get the logged in user uses the graphql package https://pkg.go.dev/github.com/cli/shurcooL-graphql which does not expose the http headers in the response, which is where the scopes information is found. I also looked at
cli/api/client.go
Line 14 in 37e343a
I also added some documentation to the
httpmock.RESTcommand.Output looks like:
In the case that no scopes are found, the output will say
There are different prefixes for token types https://github.blog/2022-10-18-introducing-fine-grained-personal-access-tokens-for-github/
Server-to-server and Fine grained permissions tokens do not contain the
"X-Oauth-Scopes"header, which would make the output above confusing. In those cases, we show a warning to the user.For a fine-grained token (prefixed with
github_pat_):For a server-to-server token (prefixed with
ghs_):