X Tutup
Skip to content

Commit 2ca18e0

Browse files
mislavmislav
committed
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
1 parent 9f1a1d8 commit 2ca18e0

File tree

7 files changed

+159
-52
lines changed

7 files changed

+159
-52
lines changed

‎api/client.go‎

Lines changed: 64 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,12 @@ func (gr GraphQLErrorResponse) Error() string {
142142

143143
// HTTPError is an error returned by a failed API call
144144
type HTTPError struct {
145-
StatusCode int
146-
RequestURL *url.URL
147-
Message string
148-
OAuthScopes string
149-
Errors []HTTPErrorItem
145+
StatusCode int
146+
RequestURL *url.URL
147+
Message string
148+
Errors []HTTPErrorItem
149+
150+
scopesSuggestion string
150151
}
151152

152153
type HTTPErrorItem struct {
@@ -165,6 +166,61 @@ func (err HTTPError) Error() string {
165166
return fmt.Sprintf("HTTP %d (%s)", err.StatusCode, err.RequestURL)
166167
}
167168

169+
func (err HTTPError) ScopesSuggestion() string {
170+
return err.scopesSuggestion
171+
}
172+
173+
// ScopesSuggestion is an error messaging utility that prints the suggestion to request additional OAuth
174+
// scopes in case a server response indicates that there are missing scopes.
175+
func ScopesSuggestion(resp *http.Response) string {
176+
if resp.StatusCode < 400 || resp.StatusCode > 499 {
177+
return ""
178+
}
179+
180+
endpointNeedsScopes := resp.Header.Get("X-Accepted-Oauth-Scopes")
181+
tokenHasScopes := resp.Header.Get("X-Oauth-Scopes")
182+
if tokenHasScopes == "" {
183+
return ""
184+
}
185+
186+
gotScopes := map[string]struct{}{}
187+
for _, s := range strings.Split(tokenHasScopes, ",") {
188+
s = strings.TrimSpace(s)
189+
gotScopes[s] = struct{}{}
190+
if strings.HasPrefix(s, "admin:") {
191+
gotScopes["read:"+strings.TrimPrefix(s, "admin:")] = struct{}{}
192+
gotScopes["write:"+strings.TrimPrefix(s, "admin:")] = struct{}{}
193+
} else if strings.HasPrefix(s, "write:") {
194+
gotScopes["read:"+strings.TrimPrefix(s, "write:")] = struct{}{}
195+
}
196+
}
197+
198+
for _, s := range strings.Split(endpointNeedsScopes, ",") {
199+
s = strings.TrimSpace(s)
200+
if _, gotScope := gotScopes[s]; s == "" || gotScope {
201+
continue
202+
}
203+
return fmt.Sprintf(
204+
"This API operation needs the %[1]q scope. To request it, run: gh auth refresh -h %[2]s -s %[1]s",
205+
s,
206+
ghinstance.NormalizeHostname(resp.Request.URL.Hostname()),
207+
)
208+
}
209+
210+
return ""
211+
}
212+
213+
// EndpointNeedsScopes adds additional OAuth scopes to an HTTP response as if they were returned from the
214+
// server endpoint. This improves HTTP 4xx error messaging for endpoints that don't explicitly list the
215+
// OAuth scopes they need.
216+
func EndpointNeedsScopes(resp *http.Response, s string) *http.Response {
217+
if resp.StatusCode >= 400 && resp.StatusCode < 500 {
218+
oldScopes := resp.Header.Get("X-Accepted-Oauth-Scopes")
219+
resp.Header.Set("X-Accepted-Oauth-Scopes", fmt.Sprintf("%s, %s", oldScopes, s))
220+
}
221+
return resp
222+
}
223+
168224
// GraphQL performs a GraphQL request and parses the response
169225
func (c Client) GraphQL(hostname string, query string, variables map[string]interface{}, data interface{}) error {
170226
reqBody, err := json.Marshal(map[string]interface{}{"query": query, "variables": variables})
@@ -261,9 +317,9 @@ func handleResponse(resp *http.Response, data interface{}) error {
261317

262318
func HandleHTTPError(resp *http.Response) error {
263319
httpError := HTTPError{
264-
StatusCode: resp.StatusCode,
265-
RequestURL: resp.Request.URL,
266-
OAuthScopes: resp.Header.Get("X-Oauth-Scopes"),
320+
StatusCode: resp.StatusCode,
321+
RequestURL: resp.Request.URL,
322+
scopesSuggestion: ScopesSuggestion(resp),
267323
}
268324

269325
if !jsonTypeRE.MatchString(resp.Header.Get("Content-Type")) {

‎api/client_test.go‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,3 +146,67 @@ func TestHandleHTTPError_GraphQL502(t *testing.T) {
146146
t.Errorf("got error: %v", err)
147147
}
148148
}
149+
150+
func TestHTTPError_ScopesSuggestion(t *testing.T) {
151+
makeResponse := func(s int, u, haveScopes, needScopes string) *http.Response {
152+
req, err := http.NewRequest("GET", u, nil)
153+
if err != nil {
154+
t.Fatal(err)
155+
}
156+
return &http.Response{
157+
Request: req,
158+
StatusCode: s,
159+
Body: ioutil.NopCloser(bytes.NewBufferString(`{}`)),
160+
Header: map[string][]string{
161+
"Content-Type": {"application/json"},
162+
"X-Oauth-Scopes": {haveScopes},
163+
"X-Accepted-Oauth-Scopes": {needScopes},
164+
},
165+
}
166+
}
167+
168+
tests := []struct {
169+
name string
170+
resp *http.Response
171+
want string
172+
}{
173+
{
174+
name: "has necessary scopes",
175+
resp: makeResponse(404, "https://api.github.com/gists", "repo, gist, read:org", "gist"),
176+
want: ``,
177+
},
178+
{
179+
name: "normalizes scopes",
180+
resp: makeResponse(404, "https://api.github.com/orgs/ORG/discussions", "admin:org, write:discussion", "read:org, read:discussion"),
181+
want: ``,
182+
},
183+
{
184+
name: "no scopes on endpoint",
185+
resp: makeResponse(404, "https://api.github.com/user", "repo", ""),
186+
want: ``,
187+
},
188+
{
189+
name: "missing a scope",
190+
resp: makeResponse(404, "https://api.github.com/gists", "repo, read:org", "gist, delete_repo"),
191+
want: `This API operation needs the "gist" scope. To request it, run: gh auth refresh -h github.com -s gist`,
192+
},
193+
{
194+
name: "server error",
195+
resp: makeResponse(500, "https://api.github.com/gists", "repo", "gist"),
196+
want: ``,
197+
},
198+
{
199+
name: "no scopes on token",
200+
resp: makeResponse(404, "https://api.github.com/gists", "", "gist, delete_repo"),
201+
want: ``,
202+
},
203+
}
204+
for _, tt := range tests {
205+
t.Run(tt.name, func(t *testing.T) {
206+
httpError := HandleHTTPError(tt.resp)
207+
if got := httpError.(HTTPError).ScopesSuggestion(); got != tt.want {
208+
t.Errorf("HTTPError.ScopesSuggestion() = %v, want %v", got, tt.want)
209+
}
210+
})
211+
}
212+
}

‎cmd/gh/main.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,8 @@ func mainRun() exitCode {
226226
fmt.Fprintln(stderr, "Try authenticating with: gh auth login")
227227
} else if strings.Contains(err.Error(), "Resource protected by organization SAML enforcement") {
228228
fmt.Fprintln(stderr, "Try re-authenticating with: gh auth refresh")
229+
} else if msg := httpErr.ScopesSuggestion(); msg != "" {
230+
fmt.Fprintln(stderr, msg)
229231
}
230232

231233
return exitError

‎pkg/cmd/api/api.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -384,12 +384,14 @@ func processResponse(resp *http.Response, opts *ApiOptions, headersOutputStream
384384
}
385385
}
386386

387+
if serverError == "" && resp.StatusCode > 299 {
388+
serverError = fmt.Sprintf("HTTP %d", resp.StatusCode)
389+
}
387390
if serverError != "" {
388391
fmt.Fprintf(opts.IO.ErrOut, "gh: %s\n", serverError)
389-
err = cmdutil.SilentError
390-
return
391-
} else if resp.StatusCode > 299 {
392-
fmt.Fprintf(opts.IO.ErrOut, "gh: HTTP %d\n", resp.StatusCode)
392+
if msg := api.ScopesSuggestion(resp); msg != "" {
393+
fmt.Fprintf(opts.IO.ErrOut, "gh: %s\n", msg)
394+
}
393395
err = cmdutil.SilentError
394396
return
395397
}

‎pkg/cmd/gist/create/create.go‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/MakeNowJust/heredoc"
1717
"github.com/cli/cli/v2/api"
1818
"github.com/cli/cli/v2/internal/config"
19+
"github.com/cli/cli/v2/internal/ghinstance"
1920
"github.com/cli/cli/v2/pkg/cmd/gist/shared"
2021
"github.com/cli/cli/v2/pkg/cmdutil"
2122
"github.com/cli/cli/v2/pkg/iostreams"
@@ -150,9 +151,6 @@ func createRun(opts *CreateOptions) error {
150151
if err != nil {
151152
var httpError api.HTTPError
152153
if errors.As(err, &httpError) {
153-
if httpError.OAuthScopes != "" && !strings.Contains(httpError.OAuthScopes, "gist") {
154-
return fmt.Errorf("This command requires the 'gist' OAuth scope.\nPlease re-authenticate with: gh auth refresh -h %s -s gist", host)
155-
}
156154
if httpError.StatusCode == http.StatusUnprocessableEntity {
157155
if detectEmptyFiles(files) {
158156
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 {
248246
}
249247

250248
func createGist(client *http.Client, hostname, description string, public bool, files map[string]*shared.GistFile) (*shared.Gist, error) {
251-
path := "gists"
252-
253249
body := &shared.Gist{
254250
Description: description,
255251
Public: public,
256252
Files: files,
257253
}
258254

259-
result := shared.Gist{}
255+
requestBody := &bytes.Buffer{}
256+
enc := json.NewEncoder(requestBody)
257+
if err := enc.Encode(body); err != nil {
258+
return nil, err
259+
}
260260

261-
requestByte, err := json.Marshal(body)
261+
u := ghinstance.RESTPrefix(hostname) + "gists"
262+
req, err := http.NewRequest(http.MethodPost, u, requestBody)
262263
if err != nil {
263264
return nil, err
264265
}
265-
requestBody := bytes.NewReader(requestByte)
266+
req.Header.Set("Content-Type", "application/json; charset=utf-8")
266267

267-
apiClient := api.NewClientFromHTTP(client)
268-
err = apiClient.REST(hostname, "POST", path, requestBody, &result)
268+
resp, err := client.Do(req)
269269
if err != nil {
270270
return nil, err
271271
}
272+
defer resp.Body.Close()
273+
274+
if resp.StatusCode > 299 {
275+
return nil, api.HandleHTTPError(api.EndpointNeedsScopes(resp, "gist"))
276+
}
277+
278+
result := &shared.Gist{}
279+
dec := json.NewDecoder(resp.Body)
280+
if err := dec.Decode(result); err != nil {
281+
return nil, err
282+
}
272283

273-
return &result, nil
284+
return result, nil
274285
}
275286

276287
func detectEmptyFiles(files map[string]*shared.GistFile) bool {

‎pkg/cmd/gist/create/create_test.go‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -388,32 +388,3 @@ func Test_detectEmptyFiles(t *testing.T) {
388388
assert.Equal(t, tt.isEmptyFile, isEmptyFile)
389389
}
390390
}
391-
392-
func Test_CreateRun_reauth(t *testing.T) {
393-
reg := &httpmock.Registry{}
394-
reg.Register(httpmock.REST("POST", "gists"), func(req *http.Request) (*http.Response, error) {
395-
return &http.Response{
396-
StatusCode: 404,
397-
Request: req,
398-
Header: map[string][]string{
399-
"X-Oauth-Scopes": {"repo, read:org"},
400-
},
401-
Body: ioutil.NopCloser(bytes.NewBufferString("oh no")),
402-
}, nil
403-
})
404-
405-
io, _, _, _ := iostreams.Test()
406-
407-
opts := &CreateOptions{
408-
IO: io,
409-
HttpClient: func() (*http.Client, error) {
410-
return &http.Client{Transport: reg}, nil
411-
},
412-
Config: func() (config.Config, error) {
413-
return config.NewBlankConfig(), nil
414-
},
415-
}
416-
417-
err := createRun(opts)
418-
assert.EqualError(t, err, "This command requires the 'gist' OAuth scope.\nPlease re-authenticate with: gh auth refresh -h github.com -s gist")
419-
}

‎pkg/httpmock/stub.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,5 +162,6 @@ func httpResponse(status int, req *http.Request, body io.Reader) *http.Response
162162
StatusCode: status,
163163
Request: req,
164164
Body: ioutil.NopCloser(body),
165+
Header: http.Header{},
165166
}
166167
}

0 commit comments

Comments
 (0)
X Tutup