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

Print more user-friendly HTTP 40x errors #2446

Merged
merged 5 commits into from
Jan 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions commands/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,14 @@ func apiCommand(_ *Command, args *Args) {
response.Body.Close()

if !success {
if ssoErr := github.ValidateGitHubSSO(response.Response); ssoErr != nil {
ui.Errorln()
ui.Errorln(ssoErr)
}
if scopeErr := github.ValidateSufficientOAuthScopes(response.Response); scopeErr != nil {
ui.Errorln()
ui.Errorln(scopeErr)
}
os.Exit(22)
}

Expand Down
37 changes: 37 additions & 0 deletions features/api.feature
Original file line number Diff line number Diff line change
Expand Up @@ -491,3 +491,40 @@ Feature: hub api
When I run `hub api --obey-ratelimit hello`
Then the exit status should be 22
Then the stderr should not contain "API rate limit exceeded"

Scenario: Warn about insufficient OAuth scopes
Given the GitHub API server:
"""
get('/hello') {
response.headers['X-Accepted-Oauth-Scopes'] = 'repo, admin'
response.headers['X-Oauth-Scopes'] = 'public_repo'
status 403
json({})
}
"""
When I run `hub api hello`
Then the exit status should be 22
And the output should contain exactly:
"""
{}
Your access token may have insufficient scopes. Visit http://github.com/settings/tokens
to edit the 'hub' token and enable one of the following scopes: admin, repo
"""

Scenario: Print the SSO challenge to stderr
Given the GitHub API server:
"""
get('/orgs/acme') {
response.headers['X-GitHub-SSO'] = 'required; url=http://example.com?auth=HASH'
status 403
json({})
}
"""
When I run `hub api orgs/acme`
Then the exit status should be 22
And the stderr should contain exactly:
"""

You must authorize your token to access this organization:
http://example.com?auth=HASH
"""
18 changes: 18 additions & 0 deletions features/authentication.feature
Original file line number Diff line number Diff line change
Expand Up @@ -520,3 +520,21 @@ Feature: OAuth authentication
"""
And the exit status should be 1
And the file "../home/.config/hub" should not exist

Scenario: GitHub SSO challenge
Given I am "monalisa" on github.com with OAuth token "OTOKEN"
And I am in "git://github.com/acme/playground.git" git repo
Given the GitHub API server:
"""
get('/repos/acme/playground/releases') {
response.headers['X-GitHub-SSO'] = 'required; url=http://example.com?auth=HASH'
status 403
}
"""
When I run `hub release show v1.2.0`
Then the stderr should contain exactly:
"""
Error fetching releases: Forbidden (HTTP 403)
You must authorize your token to access this organization:
http://example.com?auth=HASH
"""
27 changes: 26 additions & 1 deletion features/gist.feature
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ Feature: hub gist
"""
post('/gists') {
status 404
response.headers['x-accepted-oauth-scopes'] = 'gist'
response.headers['x-oauth-scopes'] = 'repos'
json({})
}
Expand All @@ -170,14 +171,38 @@ Feature: hub gist
And the stderr should contain exactly:
"""
Error creating gist: Not Found (HTTP 404)
Go to https://github.com/settings/tokens and enable the 'gist' scope for hub\n
Your access token may have insufficient scopes. Visit http://github.com/settings/tokens
to edit the 'hub' token and enable one of the following scopes: gist
"""

Scenario: Infer correct OAuth scopes for gist
Given the GitHub API server:
"""
post('/gists') {
status 404
response.headers['x-oauth-scopes'] = 'repos'
json({})
}
"""
Given a file named "testfile.txt" with:
"""
this is a test file
"""
When I run `hub gist create testfile.txt`
Then the exit status should be 1
And the stderr should contain exactly:
"""
Error creating gist: Not Found (HTTP 404)
Your access token may have insufficient scopes. Visit http://github.com/settings/tokens
to edit the 'hub' token and enable one of the following scopes: gist
"""

Scenario: Create error
Given the GitHub API server:
"""
post('/gists') {
status 404
response.headers['x-accepted-oauth-scopes'] = 'gist'
response.headers['x-oauth-scopes'] = 'repos, gist'
json({})
}
Expand Down
185 changes: 138 additions & 47 deletions github/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package github
import (
"bytes"
"encoding/json"
"errors"
"fmt"
"io"
"io/ioutil"
Expand Down Expand Up @@ -1123,9 +1124,6 @@ func (client *Client) CreateGist(filenames []string, public bool) (gist *Gist, e

res, err := api.PostJSON("gists", &g)
if err = checkStatus(201, "creating gist", res, err); err != nil {
if res != nil && res.StatusCode == 404 && !strings.Contains(res.Header.Get("x-oauth-scopes"), "gist") {
err = fmt.Errorf("%s\nGo to https://%s/settings/tokens and enable the 'gist' scope for hub", err, client.Host.Host)
}
return
}

Expand All @@ -1145,68 +1143,161 @@ func normalizeHost(host string) string {
}
}

func reverseNormalizeHost(host string) string {
switch host {
case "api.github.com":
return GitHubHost
case "api.github.localhost":
return "github.localhost"
default:
return host
}
}

func checkStatus(expectedStatus int, action string, response *simpleResponse, err error) error {
if err != nil {
return fmt.Errorf("Error %s: %s", action, err.Error())
} else if response.StatusCode != expectedStatus {
errInfo, err := response.ErrorInfo()
if err == nil {
return FormatError(action, errInfo)
} else {
if err != nil {
return fmt.Errorf("Error %s: %s (HTTP %d)", action, err.Error(), response.StatusCode)
}
} else {
return nil
return FormatError(action, errInfo)
}
return nil
}

func FormatError(action string, err error) (ee error) {
switch e := err.(type) {
default:
ee = err
case *errorInfo:
statusCode := e.Response.StatusCode
var reason string
if s := strings.SplitN(e.Response.Status, " ", 2); len(s) >= 2 {
reason = strings.TrimSpace(s[1])
}
// FormatError annotates an HTTP response error with user-friendly messages
func FormatError(action string, err error) error {
if e, ok := err.(*errorInfo); ok {
return formatError(action, e)
}
return err
}

errStr := fmt.Sprintf("Error %s: %s (HTTP %d)", action, reason, statusCode)

var errorSentences []string
for _, err := range e.Errors {
switch err.Code {
case "custom":
errorSentences = append(errorSentences, err.Message)
case "missing_field":
errorSentences = append(errorSentences, fmt.Sprintf("Missing field: \"%s\"", err.Field))
case "already_exists":
errorSentences = append(errorSentences, fmt.Sprintf("Duplicate value for \"%s\"", err.Field))
case "invalid":
errorSentences = append(errorSentences, fmt.Sprintf("Invalid value for \"%s\"", err.Field))
case "unauthorized":
errorSentences = append(errorSentences, fmt.Sprintf("Not allowed to change field \"%s\"", err.Field))
}
}
func formatError(action string, e *errorInfo) error {
var reason string
if s := strings.SplitN(e.Response.Status, " ", 2); len(s) >= 2 {
reason = strings.TrimSpace(s[1])
}

var errorMessage string
if len(errorSentences) > 0 {
errorMessage = strings.Join(errorSentences, "\n")
} else {
errorMessage = e.Message
if action == "getting current user" && e.Message == "Resource not accessible by integration" {
errorMessage = errorMessage + "\nYou must specify GITHUB_USER via environment variable."
}
errStr := fmt.Sprintf("Error %s: %s (HTTP %d)", action, reason, e.Response.StatusCode)

var errorSentences []string
for _, err := range e.Errors {
switch err.Code {
case "custom":
errorSentences = append(errorSentences, err.Message)
case "missing_field":
errorSentences = append(errorSentences, fmt.Sprintf("Missing field: \"%s\"", err.Field))
case "already_exists":
errorSentences = append(errorSentences, fmt.Sprintf("Duplicate value for \"%s\"", err.Field))
case "invalid":
errorSentences = append(errorSentences, fmt.Sprintf("Invalid value for \"%s\"", err.Field))
case "unauthorized":
errorSentences = append(errorSentences, fmt.Sprintf("Not allowed to change field \"%s\"", err.Field))
}
}

if errorMessage != "" {
errStr = fmt.Sprintf("%s\n%s", errStr, errorMessage)
var errorMessage string
if len(errorSentences) > 0 {
errorMessage = strings.Join(errorSentences, "\n")
} else {
errorMessage = e.Message
if action == "getting current user" && e.Message == "Resource not accessible by integration" {
errorMessage = errorMessage + "\nYou must specify GITHUB_USER via environment variable."
}
}
if errorMessage != "" {
errStr = fmt.Sprintf("%s\n%s", errStr, errorMessage)
}

ee = fmt.Errorf(errStr)
if ssoErr := ValidateGitHubSSO(e.Response); ssoErr != nil {
return fmt.Errorf("%s\n%s", errStr, ssoErr)
}

return
if scopeErr := ValidateSufficientOAuthScopes(e.Response); scopeErr != nil {
return fmt.Errorf("%s\n%s", errStr, scopeErr)
}

return errors.New(errStr)
}

// ValidateGitHubSSO checks for the challenge via `X-Github-Sso` header
func ValidateGitHubSSO(res *http.Response) error {
if res.StatusCode != 403 {
return nil
}

sso := res.Header.Get("X-Github-Sso")
if !strings.HasPrefix(sso, "required; url=") {
return nil
}

url := sso[strings.IndexByte(sso, '=')+1:]
return fmt.Errorf("You must authorize your token to access this organization:\n%s", url)
}

// ValidateSufficientOAuthScopes warns about insufficient OAuth scopes
func ValidateSufficientOAuthScopes(res *http.Response) error {
if res.StatusCode != 404 && res.StatusCode != 403 {
return nil
}

needScopes := newScopeSet(res.Header.Get("X-Accepted-Oauth-Scopes"))
if len(needScopes) == 0 && isGistWrite(res.Request) {
// compensate for a GitHub bug: gist APIs omit proper `X-Accepted-Oauth-Scopes` in responses
needScopes = newScopeSet("gist")
}

haveScopes := newScopeSet(res.Header.Get("X-Oauth-Scopes"))
if len(needScopes) == 0 || needScopes.Intersects(haveScopes) {
return nil
}

return fmt.Errorf("Your access token may have insufficient scopes. Visit %s://%s/settings/tokens\n"+
"to edit the 'hub' token and enable one of the following scopes: %s",
res.Request.URL.Scheme,
reverseNormalizeHost(res.Request.Host),
needScopes)
}

func isGistWrite(req *http.Request) bool {
if req.Method == "GET" {
return false
}
path := strings.TrimPrefix(req.URL.Path, "/v3")
return strings.HasPrefix(path, "/gists")
}

type scopeSet map[string]struct{}

func (s scopeSet) String() string {
scopes := make([]string, 0, len(s))
for scope := range s {
scopes = append(scopes, scope)
}
sort.Sort(sort.StringSlice(scopes))
return strings.Join(scopes, ", ")
}

func (s scopeSet) Intersects(other scopeSet) bool {
for scope := range s {
if _, found := other[scope]; found {
return true
}
}
return false
}

func newScopeSet(s string) scopeSet {
scopes := scopeSet{}
for _, s := range strings.SplitN(s, ",", -1) {
if s = strings.TrimSpace(s); s != "" {
scopes[s] = struct{}{}
}
}
return scopes
}

func authTokenNote(num int) (string, error) {
Expand Down
10 changes: 8 additions & 2 deletions github/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ const (
var inspectHeaders = []string{
"Authorization",
"X-GitHub-OTP",
"X-GitHub-SSO",
"X-Oauth-Scopes",
"X-Accepted-Oauth-Scopes",
"X-Oauth-Client-Id",
"X-GitHub-Enterprise-Version",
"Location",
"Link",
"Accept",
Expand Down Expand Up @@ -336,8 +341,9 @@ func (c *simpleClient) cacheRead(key string, req *http.Request) (res *http.Respo
}

res = &http.Response{
Body: ioutil.NopCloser(bytes.NewBufferString(parts[1])),
Header: http.Header{},
Body: ioutil.NopCloser(bytes.NewBufferString(parts[1])),
Header: http.Header{},
Request: req,
}
headerLines := strings.Split(parts[0], "\r\n")
if len(headerLines) < 1 {
Expand Down