Warn about missing OAuth scopes when reporting HTTP 4xx errors

If a 4xx server response lists scopes in the X-Accepted-Oauth-Scopes
header that are not present in the X-Oauth-Scopes header, the final
error messaging on stderr will now include a hint for the user that they
might need to request the additional scope:

    $ gh codespace list
    error getting codespaces: HTTP 403: Must have admin rights to Repository. (https://api.github.com/user/codespaces?per_page=30)
    This API operation needs the "codespace" scope. To request it, run:  gh auth refresh -h github.com -s codespace
This commit is contained in:
Mislav Marohnić 2021-10-13 23:24:14 +02:00
parent 9f1a1d8805
commit 2ca18e0600
7 changed files with 165 additions and 58 deletions

View file

@ -142,11 +142,12 @@ func (gr GraphQLErrorResponse) Error() string {
// HTTPError is an error returned by a failed API call
type HTTPError struct {
StatusCode int
RequestURL *url.URL
Message string
OAuthScopes string
Errors []HTTPErrorItem
StatusCode int
RequestURL *url.URL
Message string
Errors []HTTPErrorItem
scopesSuggestion string
}
type HTTPErrorItem struct {
@ -165,6 +166,61 @@ func (err HTTPError) Error() string {
return fmt.Sprintf("HTTP %d (%s)", err.StatusCode, err.RequestURL)
}
func (err HTTPError) ScopesSuggestion() string {
return err.scopesSuggestion
}
// ScopesSuggestion is an error messaging utility that prints the suggestion to request additional OAuth
// scopes in case a server response indicates that there are missing scopes.
func ScopesSuggestion(resp *http.Response) string {
if resp.StatusCode < 400 || resp.StatusCode > 499 {
return ""
}
endpointNeedsScopes := resp.Header.Get("X-Accepted-Oauth-Scopes")
tokenHasScopes := resp.Header.Get("X-Oauth-Scopes")
if tokenHasScopes == "" {
return ""
}
gotScopes := map[string]struct{}{}
for _, s := range strings.Split(tokenHasScopes, ",") {
s = strings.TrimSpace(s)
gotScopes[s] = struct{}{}
if strings.HasPrefix(s, "admin:") {
gotScopes["read:"+strings.TrimPrefix(s, "admin:")] = struct{}{}
gotScopes["write:"+strings.TrimPrefix(s, "admin:")] = struct{}{}
} else if strings.HasPrefix(s, "write:") {
gotScopes["read:"+strings.TrimPrefix(s, "write:")] = struct{}{}
}
}
for _, s := range strings.Split(endpointNeedsScopes, ",") {
s = strings.TrimSpace(s)
if _, gotScope := gotScopes[s]; s == "" || gotScope {
continue
}
return fmt.Sprintf(
"This API operation needs the %[1]q scope. To request it, run: gh auth refresh -h %[2]s -s %[1]s",
s,
ghinstance.NormalizeHostname(resp.Request.URL.Hostname()),
)
}
return ""
}
// EndpointNeedsScopes adds additional OAuth scopes to an HTTP response as if they were returned from the
// server endpoint. This improves HTTP 4xx error messaging for endpoints that don't explicitly list the
// OAuth scopes they need.
func EndpointNeedsScopes(resp *http.Response, s string) *http.Response {
if resp.StatusCode >= 400 && resp.StatusCode < 500 {
oldScopes := resp.Header.Get("X-Accepted-Oauth-Scopes")
resp.Header.Set("X-Accepted-Oauth-Scopes", fmt.Sprintf("%s, %s", oldScopes, s))
}
return resp
}
// GraphQL performs a GraphQL request and parses the response
func (c Client) GraphQL(hostname string, query string, variables map[string]interface{}, data interface{}) error {
reqBody, err := json.Marshal(map[string]interface{}{"query": query, "variables": variables})
@ -261,9 +317,9 @@ func handleResponse(resp *http.Response, data interface{}) error {
func HandleHTTPError(resp *http.Response) error {
httpError := HTTPError{
StatusCode: resp.StatusCode,
RequestURL: resp.Request.URL,
OAuthScopes: resp.Header.Get("X-Oauth-Scopes"),
StatusCode: resp.StatusCode,
RequestURL: resp.Request.URL,
scopesSuggestion: ScopesSuggestion(resp),
}
if !jsonTypeRE.MatchString(resp.Header.Get("Content-Type")) {

View file

@ -146,3 +146,67 @@ func TestHandleHTTPError_GraphQL502(t *testing.T) {
t.Errorf("got error: %v", err)
}
}
func TestHTTPError_ScopesSuggestion(t *testing.T) {
makeResponse := func(s int, u, haveScopes, needScopes string) *http.Response {
req, err := http.NewRequest("GET", u, nil)
if err != nil {
t.Fatal(err)
}
return &http.Response{
Request: req,
StatusCode: s,
Body: ioutil.NopCloser(bytes.NewBufferString(`{}`)),
Header: map[string][]string{
"Content-Type": {"application/json"},
"X-Oauth-Scopes": {haveScopes},
"X-Accepted-Oauth-Scopes": {needScopes},
},
}
}
tests := []struct {
name string
resp *http.Response
want string
}{
{
name: "has necessary scopes",
resp: makeResponse(404, "https://api.github.com/gists", "repo, gist, read:org", "gist"),
want: ``,
},
{
name: "normalizes scopes",
resp: makeResponse(404, "https://api.github.com/orgs/ORG/discussions", "admin:org, write:discussion", "read:org, read:discussion"),
want: ``,
},
{
name: "no scopes on endpoint",
resp: makeResponse(404, "https://api.github.com/user", "repo", ""),
want: ``,
},
{
name: "missing a scope",
resp: makeResponse(404, "https://api.github.com/gists", "repo, read:org", "gist, delete_repo"),
want: `This API operation needs the "gist" scope. To request it, run: gh auth refresh -h github.com -s gist`,
},
{
name: "server error",
resp: makeResponse(500, "https://api.github.com/gists", "repo", "gist"),
want: ``,
},
{
name: "no scopes on token",
resp: makeResponse(404, "https://api.github.com/gists", "", "gist, delete_repo"),
want: ``,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
httpError := HandleHTTPError(tt.resp)
if got := httpError.(HTTPError).ScopesSuggestion(); got != tt.want {
t.Errorf("HTTPError.ScopesSuggestion() = %v, want %v", got, tt.want)
}
})
}
}

View file

@ -226,6 +226,8 @@ func mainRun() exitCode {
fmt.Fprintln(stderr, "Try authenticating with: gh auth login")
} else if strings.Contains(err.Error(), "Resource protected by organization SAML enforcement") {
fmt.Fprintln(stderr, "Try re-authenticating with: gh auth refresh")
} else if msg := httpErr.ScopesSuggestion(); msg != "" {
fmt.Fprintln(stderr, msg)
}
return exitError

View file

@ -384,12 +384,14 @@ func processResponse(resp *http.Response, opts *ApiOptions, headersOutputStream
}
}
if serverError == "" && resp.StatusCode > 299 {
serverError = fmt.Sprintf("HTTP %d", resp.StatusCode)
}
if serverError != "" {
fmt.Fprintf(opts.IO.ErrOut, "gh: %s\n", serverError)
err = cmdutil.SilentError
return
} else if resp.StatusCode > 299 {
fmt.Fprintf(opts.IO.ErrOut, "gh: HTTP %d\n", resp.StatusCode)
if msg := api.ScopesSuggestion(resp); msg != "" {
fmt.Fprintf(opts.IO.ErrOut, "gh: %s\n", msg)
}
err = cmdutil.SilentError
return
}

View file

@ -16,6 +16,7 @@ import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/pkg/cmd/gist/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
@ -150,9 +151,6 @@ func createRun(opts *CreateOptions) error {
if err != nil {
var httpError api.HTTPError
if errors.As(err, &httpError) {
if httpError.OAuthScopes != "" && !strings.Contains(httpError.OAuthScopes, "gist") {
return fmt.Errorf("This command requires the 'gist' OAuth scope.\nPlease re-authenticate with: gh auth refresh -h %s -s gist", host)
}
if httpError.StatusCode == http.StatusUnprocessableEntity {
if detectEmptyFiles(files) {
fmt.Fprintf(errOut, "%s Failed to create gist: %s\n", cs.FailureIcon(), "a gist file cannot be blank")
@ -248,29 +246,42 @@ func guessGistName(files map[string]*shared.GistFile) string {
}
func createGist(client *http.Client, hostname, description string, public bool, files map[string]*shared.GistFile) (*shared.Gist, error) {
path := "gists"
body := &shared.Gist{
Description: description,
Public: public,
Files: files,
}
result := shared.Gist{}
requestByte, err := json.Marshal(body)
if err != nil {
return nil, err
}
requestBody := bytes.NewReader(requestByte)
apiClient := api.NewClientFromHTTP(client)
err = apiClient.REST(hostname, "POST", path, requestBody, &result)
if err != nil {
requestBody := &bytes.Buffer{}
enc := json.NewEncoder(requestBody)
if err := enc.Encode(body); err != nil {
return nil, err
}
return &result, nil
u := ghinstance.RESTPrefix(hostname) + "gists"
req, err := http.NewRequest(http.MethodPost, u, requestBody)
if err != nil {
return nil, err
}
req.Header.Set("Content-Type", "application/json; charset=utf-8")
resp, err := client.Do(req)
if err != nil {
return nil, err
}
defer resp.Body.Close()
if resp.StatusCode > 299 {
return nil, api.HandleHTTPError(api.EndpointNeedsScopes(resp, "gist"))
}
result := &shared.Gist{}
dec := json.NewDecoder(resp.Body)
if err := dec.Decode(result); err != nil {
return nil, err
}
return result, nil
}
func detectEmptyFiles(files map[string]*shared.GistFile) bool {

View file

@ -388,32 +388,3 @@ func Test_detectEmptyFiles(t *testing.T) {
assert.Equal(t, tt.isEmptyFile, isEmptyFile)
}
}
func Test_CreateRun_reauth(t *testing.T) {
reg := &httpmock.Registry{}
reg.Register(httpmock.REST("POST", "gists"), func(req *http.Request) (*http.Response, error) {
return &http.Response{
StatusCode: 404,
Request: req,
Header: map[string][]string{
"X-Oauth-Scopes": {"repo, read:org"},
},
Body: ioutil.NopCloser(bytes.NewBufferString("oh no")),
}, nil
})
io, _, _, _ := iostreams.Test()
opts := &CreateOptions{
IO: io,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
Config: func() (config.Config, error) {
return config.NewBlankConfig(), nil
},
}
err := createRun(opts)
assert.EqualError(t, err, "This command requires the 'gist' OAuth scope.\nPlease re-authenticate with: gh auth refresh -h github.com -s gist")
}

View file

@ -162,5 +162,6 @@ func httpResponse(status int, req *http.Request, body io.Reader) *http.Response
StatusCode: status,
Request: req,
Body: ioutil.NopCloser(body),
Header: http.Header{},
}
}