Merge pull request #4513 from cli/missing-oauth-scopes
Warn about missing OAuth scopes when reporting HTTP 4xx errors
This commit is contained in:
commit
78443964d4
17 changed files with 320 additions and 172 deletions
|
|
@ -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")) {
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -241,12 +241,23 @@ func FetchRepository(client *Client, repo ghrepo.Interface, fields []string) (*R
|
|||
}
|
||||
|
||||
var result struct {
|
||||
Repository Repository
|
||||
Repository *Repository
|
||||
}
|
||||
if err := client.GraphQL(repo.RepoHost(), query, variables, &result); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return InitRepoHostname(&result.Repository, repo.RepoHost()), nil
|
||||
// The GraphQL API should have returned an error in case of a missing repository, but this isn't
|
||||
// guaranteed to happen when an authentication token with insufficient permissions is being used.
|
||||
if result.Repository == nil {
|
||||
return nil, GraphQLErrorResponse{
|
||||
Errors: []GraphQLError{{
|
||||
Type: "NOT_FOUND",
|
||||
Message: fmt.Sprintf("Could not resolve to a Repository with the name '%s/%s'.", repo.RepoOwner(), repo.RepoName()),
|
||||
}},
|
||||
}
|
||||
}
|
||||
|
||||
return InitRepoHostname(result.Repository, repo.RepoHost()), nil
|
||||
}
|
||||
|
||||
func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
|
||||
|
|
@ -280,16 +291,24 @@ func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
|
|||
"name": repo.RepoName(),
|
||||
}
|
||||
|
||||
result := struct {
|
||||
Repository Repository
|
||||
}{}
|
||||
err := client.GraphQL(repo.RepoHost(), query, variables, &result)
|
||||
|
||||
if err != nil {
|
||||
var result struct {
|
||||
Repository *Repository
|
||||
}
|
||||
if err := client.GraphQL(repo.RepoHost(), query, variables, &result); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
// The GraphQL API should have returned an error in case of a missing repository, but this isn't
|
||||
// guaranteed to happen when an authentication token with insufficient permissions is being used.
|
||||
if result.Repository == nil {
|
||||
return nil, GraphQLErrorResponse{
|
||||
Errors: []GraphQLError{{
|
||||
Type: "NOT_FOUND",
|
||||
Message: fmt.Sprintf("Could not resolve to a Repository with the name '%s/%s'.", repo.RepoOwner(), repo.RepoName()),
|
||||
}},
|
||||
}
|
||||
}
|
||||
|
||||
return InitRepoHostname(&result.Repository, repo.RepoHost()), nil
|
||||
return InitRepoHostname(result.Repository, repo.RepoHost()), nil
|
||||
}
|
||||
|
||||
func RepoDefaultBranch(client *Client, repo ghrepo.Interface) (string, error) {
|
||||
|
|
|
|||
|
|
@ -10,6 +10,27 @@ import (
|
|||
"github.com/cli/cli/v2/pkg/httpmock"
|
||||
)
|
||||
|
||||
func TestGitHubRepo_notFound(t *testing.T) {
|
||||
httpReg := &httpmock.Registry{}
|
||||
defer httpReg.Verify(t)
|
||||
|
||||
httpReg.Register(
|
||||
httpmock.GraphQL(`query RepositoryInfo\b`),
|
||||
httpmock.StringResponse(`{ "data": { "repository": null } }`))
|
||||
|
||||
client := NewClient(ReplaceTripper(httpReg))
|
||||
repo, err := GitHubRepo(client, ghrepo.New("OWNER", "REPO"))
|
||||
if err == nil {
|
||||
t.Fatal("GitHubRepo did not return an error")
|
||||
}
|
||||
if wants := "GraphQL error: Could not resolve to a Repository with the name 'OWNER/REPO'."; err.Error() != wants {
|
||||
t.Errorf("GitHubRepo error: want %q, got %q", wants, err.Error())
|
||||
}
|
||||
if repo != nil {
|
||||
t.Errorf("GitHubRepo: expected nil repo, got %v", repo)
|
||||
}
|
||||
}
|
||||
|
||||
func Test_RepoMetadata(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
client := NewClient(ReplaceTripper(http))
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -85,15 +85,15 @@ func (a *API) GetUser(ctx context.Context) (*User, error) {
|
|||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, api.HandleHTTPError(resp)
|
||||
}
|
||||
|
||||
b, err := ioutil.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("error reading response body: %w", err)
|
||||
}
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, jsonErrorResponse(b)
|
||||
}
|
||||
|
||||
var response User
|
||||
if err := json.Unmarshal(b, &response); err != nil {
|
||||
return nil, fmt.Errorf("error unmarshaling response: %w", err)
|
||||
|
|
@ -102,18 +102,6 @@ func (a *API) GetUser(ctx context.Context) (*User, error) {
|
|||
return &response, nil
|
||||
}
|
||||
|
||||
// jsonErrorResponse returns the error message from a JSON response.
|
||||
func jsonErrorResponse(b []byte) error {
|
||||
var response struct {
|
||||
Message string `json:"message"`
|
||||
}
|
||||
if err := json.Unmarshal(b, &response); err != nil {
|
||||
return fmt.Errorf("error unmarshaling error response: %w", err)
|
||||
}
|
||||
|
||||
return errors.New(response.Message)
|
||||
}
|
||||
|
||||
// Repository represents a GitHub repository.
|
||||
type Repository struct {
|
||||
ID int `json:"id"`
|
||||
|
|
@ -133,15 +121,15 @@ func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error
|
|||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, api.HandleHTTPError(resp)
|
||||
}
|
||||
|
||||
b, err := ioutil.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("error reading response body: %w", err)
|
||||
}
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, jsonErrorResponse(b)
|
||||
}
|
||||
|
||||
var response Repository
|
||||
if err := json.Unmarshal(b, &response); err != nil {
|
||||
return nil, fmt.Errorf("error unmarshaling response: %w", err)
|
||||
|
|
@ -286,15 +274,15 @@ func (a *API) GetCodespace(ctx context.Context, codespaceName string, includeCon
|
|||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, api.HandleHTTPError(resp)
|
||||
}
|
||||
|
||||
b, err := ioutil.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("error reading response body: %w", err)
|
||||
}
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, jsonErrorResponse(b)
|
||||
}
|
||||
|
||||
var response Codespace
|
||||
if err := json.Unmarshal(b, &response); err != nil {
|
||||
return nil, fmt.Errorf("error unmarshaling response: %w", err)
|
||||
|
|
@ -322,26 +310,12 @@ func (a *API) StartCodespace(ctx context.Context, codespaceName string) error {
|
|||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
b, err := ioutil.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
return fmt.Errorf("error reading response body: %w", err)
|
||||
}
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
if resp.StatusCode == http.StatusConflict {
|
||||
// 409 means the codespace is already running which we can safely ignore
|
||||
return nil
|
||||
}
|
||||
|
||||
// Error response may be a numeric code or a JSON {"message": "..."}.
|
||||
if bytes.HasPrefix(b, []byte("{")) {
|
||||
return jsonErrorResponse(b) // probably JSON
|
||||
}
|
||||
|
||||
if len(b) > 100 {
|
||||
b = append(b[:97], "..."...)
|
||||
}
|
||||
return fmt.Errorf("failed to start codespace: %s (%s)", b, resp.Status)
|
||||
return api.HandleHTTPError(resp)
|
||||
}
|
||||
|
||||
return nil
|
||||
|
|
@ -388,15 +362,15 @@ func (a *API) GetCodespaceRegionLocation(ctx context.Context) (string, error) {
|
|||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return "", api.HandleHTTPError(resp)
|
||||
}
|
||||
|
||||
b, err := ioutil.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("error reading response body: %w", err)
|
||||
}
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return "", jsonErrorResponse(b)
|
||||
}
|
||||
|
||||
var response getCodespaceRegionLocationResponse
|
||||
if err := json.Unmarshal(b, &response); err != nil {
|
||||
return "", fmt.Errorf("error unmarshaling response: %w", err)
|
||||
|
|
@ -430,15 +404,15 @@ func (a *API) GetCodespacesMachines(ctx context.Context, repoID int, branch, loc
|
|||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, api.HandleHTTPError(resp)
|
||||
}
|
||||
|
||||
b, err := ioutil.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("error reading response body: %w", err)
|
||||
}
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, jsonErrorResponse(b)
|
||||
}
|
||||
|
||||
var response struct {
|
||||
Machines []*Machine `json:"machines"`
|
||||
}
|
||||
|
|
@ -523,18 +497,17 @@ func (a *API) startCreate(ctx context.Context, repoID int, machine, branch, loca
|
|||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode == http.StatusAccepted {
|
||||
return nil, errProvisioningInProgress // RPC finished before result of creation known
|
||||
} else if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated {
|
||||
return nil, api.HandleHTTPError(resp)
|
||||
}
|
||||
|
||||
b, err := ioutil.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("error reading response body: %w", err)
|
||||
}
|
||||
|
||||
switch {
|
||||
case resp.StatusCode > http.StatusAccepted:
|
||||
return nil, jsonErrorResponse(b)
|
||||
case resp.StatusCode == http.StatusAccepted:
|
||||
return nil, errProvisioningInProgress // RPC finished before result of creation known
|
||||
}
|
||||
|
||||
var response Codespace
|
||||
if err := json.Unmarshal(b, &response); err != nil {
|
||||
return nil, fmt.Errorf("error unmarshaling response: %w", err)
|
||||
|
|
@ -557,12 +530,8 @@ func (a *API) DeleteCodespace(ctx context.Context, codespaceName string) error {
|
|||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode > http.StatusAccepted {
|
||||
b, err := ioutil.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
return fmt.Errorf("error reading response body: %w", err)
|
||||
}
|
||||
return jsonErrorResponse(b)
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return api.HandleHTTPError(resp)
|
||||
}
|
||||
|
||||
return nil
|
||||
|
|
@ -591,6 +560,8 @@ func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Cod
|
|||
|
||||
if resp.StatusCode == http.StatusNotFound {
|
||||
return nil, nil
|
||||
} else if resp.StatusCode != http.StatusOK {
|
||||
return nil, api.HandleHTTPError(resp)
|
||||
}
|
||||
|
||||
b, err := ioutil.ReadAll(resp.Body)
|
||||
|
|
@ -598,10 +569,6 @@ func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Cod
|
|||
return nil, fmt.Errorf("error reading response body: %w", err)
|
||||
}
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, jsonErrorResponse(b)
|
||||
}
|
||||
|
||||
var response getCodespaceRepositoryContentsResponse
|
||||
if err := json.Unmarshal(b, &response); err != nil {
|
||||
return nil, fmt.Errorf("error unmarshaling response: %w", err)
|
||||
|
|
@ -629,14 +596,14 @@ func (a *API) AuthorizedKeys(ctx context.Context, user string) ([]byte, error) {
|
|||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, fmt.Errorf("server returned %s", resp.Status)
|
||||
}
|
||||
|
||||
b, err := ioutil.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("error reading response body: %w", err)
|
||||
}
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, fmt.Errorf("server returned %s", resp.Status)
|
||||
}
|
||||
return b, nil
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,6 +3,8 @@ package refresh
|
|||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
"github.com/AlecAivazis/survey/v2"
|
||||
"github.com/MakeNowJust/heredoc"
|
||||
|
|
@ -16,8 +18,9 @@ import (
|
|||
)
|
||||
|
||||
type RefreshOptions struct {
|
||||
IO *iostreams.IOStreams
|
||||
Config func() (config.Config, error)
|
||||
IO *iostreams.IOStreams
|
||||
Config func() (config.Config, error)
|
||||
httpClient *http.Client
|
||||
|
||||
MainExecutable string
|
||||
|
||||
|
|
@ -36,6 +39,7 @@ func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra.
|
|||
_, err := authflow.AuthFlowWithConfig(cfg, io, hostname, "", scopes)
|
||||
return err
|
||||
},
|
||||
httpClient: http.DefaultClient,
|
||||
}
|
||||
|
||||
cmd := &cobra.Command{
|
||||
|
|
@ -128,6 +132,16 @@ func refreshRun(opts *RefreshOptions) error {
|
|||
}
|
||||
|
||||
var additionalScopes []string
|
||||
if oldToken, _ := cfg.Get(hostname, "oauth_token"); oldToken != "" {
|
||||
if oldScopes, err := shared.GetScopes(opts.httpClient, hostname, oldToken); err == nil {
|
||||
for _, s := range strings.Split(oldScopes, ",") {
|
||||
s = strings.TrimSpace(s)
|
||||
if s != "" {
|
||||
additionalScopes = append(additionalScopes, s)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
credentialFlow := &shared.GitCredentialFlow{
|
||||
Executable: opts.MainExecutable,
|
||||
|
|
|
|||
|
|
@ -2,6 +2,9 @@ package refresh
|
|||
|
||||
import (
|
||||
"bytes"
|
||||
"io/ioutil"
|
||||
"net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/cli/cli/v2/internal/config"
|
||||
|
|
@ -134,6 +137,7 @@ func Test_refreshRun(t *testing.T) {
|
|||
opts *RefreshOptions
|
||||
askStubs func(*prompt.AskStubber)
|
||||
cfgHosts []string
|
||||
oldScopes string
|
||||
wantErr string
|
||||
nontty bool
|
||||
wantAuthArgs authArgs
|
||||
|
|
@ -211,6 +215,20 @@ func Test_refreshRun(t *testing.T) {
|
|||
scopes: []string{"repo:invite", "public_key:read"},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "scopes provided",
|
||||
cfgHosts: []string{
|
||||
"github.com",
|
||||
},
|
||||
oldScopes: "delete_repo, codespace",
|
||||
opts: &RefreshOptions{
|
||||
Scopes: []string{"repo:invite", "public_key:read"},
|
||||
},
|
||||
wantAuthArgs: authArgs{
|
||||
hostname: "github.com",
|
||||
scopes: []string{"repo:invite", "public_key:read", "delete_repo", "codespace"},
|
||||
},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
|
|
@ -234,10 +252,26 @@ func Test_refreshRun(t *testing.T) {
|
|||
for _, hostname := range tt.cfgHosts {
|
||||
_ = cfg.Set(hostname, "oauth_token", "abc123")
|
||||
}
|
||||
reg := &httpmock.Registry{}
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`query UserCurrent\b`),
|
||||
httpmock.StringResponse(`{"data":{"viewer":{"login":"cybilb"}}}`))
|
||||
|
||||
httpReg := &httpmock.Registry{}
|
||||
httpReg.Register(
|
||||
httpmock.REST("GET", ""),
|
||||
func(req *http.Request) (*http.Response, error) {
|
||||
statusCode := 200
|
||||
if req.Header.Get("Authorization") != "token abc123" {
|
||||
statusCode = 400
|
||||
}
|
||||
return &http.Response{
|
||||
Request: req,
|
||||
StatusCode: statusCode,
|
||||
Body: ioutil.NopCloser(strings.NewReader(``)),
|
||||
Header: http.Header{
|
||||
"X-Oauth-Scopes": {tt.oldScopes},
|
||||
},
|
||||
}, nil
|
||||
},
|
||||
)
|
||||
tt.opts.httpClient = &http.Client{Transport: httpReg}
|
||||
|
||||
mainBuf := bytes.Buffer{}
|
||||
hostsBuf := bytes.Buffer{}
|
||||
|
|
@ -258,8 +292,8 @@ func Test_refreshRun(t *testing.T) {
|
|||
assert.NoError(t, err)
|
||||
}
|
||||
|
||||
assert.Equal(t, aa.hostname, tt.wantAuthArgs.hostname)
|
||||
assert.Equal(t, aa.scopes, tt.wantAuthArgs.scopes)
|
||||
assert.Equal(t, tt.wantAuthArgs.hostname, aa.hostname)
|
||||
assert.Equal(t, tt.wantAuthArgs.scopes, aa.scopes)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -32,19 +32,19 @@ type httpClient interface {
|
|||
Do(*http.Request) (*http.Response, error)
|
||||
}
|
||||
|
||||
func HasMinimumScopes(httpClient httpClient, hostname, authToken string) error {
|
||||
func GetScopes(httpClient httpClient, hostname, authToken string) (string, error) {
|
||||
apiEndpoint := ghinstance.RESTPrefix(hostname)
|
||||
|
||||
req, err := http.NewRequest("GET", apiEndpoint, nil)
|
||||
if err != nil {
|
||||
return err
|
||||
return "", err
|
||||
}
|
||||
|
||||
req.Header.Set("Authorization", "token "+authToken)
|
||||
|
||||
res, err := httpClient.Do(req)
|
||||
if err != nil {
|
||||
return err
|
||||
return "", err
|
||||
}
|
||||
|
||||
defer func() {
|
||||
|
|
@ -55,10 +55,18 @@ func HasMinimumScopes(httpClient httpClient, hostname, authToken string) error {
|
|||
}()
|
||||
|
||||
if res.StatusCode != 200 {
|
||||
return api.HandleHTTPError(res)
|
||||
return "", api.HandleHTTPError(res)
|
||||
}
|
||||
|
||||
return res.Header.Get("X-Oauth-Scopes"), nil
|
||||
}
|
||||
|
||||
func HasMinimumScopes(httpClient httpClient, hostname, authToken string) error {
|
||||
scopesHeader, err := GetScopes(httpClient, hostname, authToken)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
scopesHeader := res.Header.Get("X-Oauth-Scopes")
|
||||
if scopesHeader == "" {
|
||||
// if the token reports no scopes, assume that it's an integration token and give up on
|
||||
// detecting its capabilities
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -85,12 +85,6 @@ func runAdd(opts *AddOptions) error {
|
|||
|
||||
err = SSHKeyUpload(httpClient, hostname, keyReader, opts.Title)
|
||||
if err != nil {
|
||||
if errors.Is(err, scopesError) {
|
||||
cs := opts.IO.ColorScheme()
|
||||
fmt.Fprint(opts.IO.ErrOut, "Error: insufficient OAuth scopes to list SSH keys\n")
|
||||
fmt.Fprintf(opts.IO.ErrOut, "Run the following to grant scopes: %s\n", cs.Bold("gh auth refresh -s write:public_key"))
|
||||
return cmdutil.SilentError
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -12,8 +12,6 @@ import (
|
|||
"github.com/cli/cli/v2/internal/ghinstance"
|
||||
)
|
||||
|
||||
var scopesError = errors.New("insufficient OAuth scopes")
|
||||
|
||||
func SSHKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader, title string) error {
|
||||
url := ghinstance.RESTPrefix(hostname) + "user/keys"
|
||||
|
||||
|
|
@ -43,9 +41,7 @@ func SSHKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader, t
|
|||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode == 404 {
|
||||
return scopesError
|
||||
} else if resp.StatusCode > 299 {
|
||||
if resp.StatusCode > 299 {
|
||||
var httpError api.HTTPError
|
||||
err := api.HandleHTTPError(resp)
|
||||
if errors.As(err, &httpError) && isDuplicateError(&httpError) {
|
||||
|
|
|
|||
|
|
@ -2,7 +2,6 @@ package list
|
|||
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io/ioutil"
|
||||
"net/http"
|
||||
|
|
@ -12,8 +11,6 @@ import (
|
|||
"github.com/cli/cli/v2/internal/ghinstance"
|
||||
)
|
||||
|
||||
var scopesError = errors.New("insufficient OAuth scopes")
|
||||
|
||||
type sshKey struct {
|
||||
Key string
|
||||
Title string
|
||||
|
|
@ -37,9 +34,7 @@ func userKeys(httpClient *http.Client, host, userHandle string) ([]sshKey, error
|
|||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode == 404 {
|
||||
return nil, scopesError
|
||||
} else if resp.StatusCode > 299 {
|
||||
if resp.StatusCode > 299 {
|
||||
return nil, api.HandleHTTPError(resp)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,7 +1,6 @@
|
|||
package list
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"time"
|
||||
|
|
@ -59,12 +58,6 @@ func listRun(opts *ListOptions) error {
|
|||
|
||||
sshKeys, err := userKeys(apiClient, host, "")
|
||||
if err != nil {
|
||||
if errors.Is(err, scopesError) {
|
||||
cs := opts.IO.ColorScheme()
|
||||
fmt.Fprint(opts.IO.ErrOut, "Error: insufficient OAuth scopes to list SSH keys\n")
|
||||
fmt.Fprintf(opts.IO.ErrOut, "Run the following to grant scopes: %s\n", cs.Bold("gh auth refresh -s read:public_key"))
|
||||
return cmdutil.SilentError
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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{},
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue