From 767521c05556746b4241ac34d58d5bc7b788f04d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 20 May 2020 17:09:13 +0200 Subject: [PATCH 1/5] Stop providing `AuthLogin` from context The login name of the authenticated user will be readily available only if authentication info comes from the config file. With other upcoming authentication modes (for example, the GITHUB_TOKEN environment variable), the token is the only piece of information we got, so we would need to additionally query for the login name. Since `issue status` and `pr status` are the only commands that need the name of the authenticated user right now, have those commands explicitly query for the login name. This results in an additional API query, but simplifies Context implementation and future authentication approaches. --- api/queries_user.go | 18 +++++++++++++++++ command/issue.go | 2 +- command/issue_test.go | 9 +++++++++ command/pr.go | 2 +- command/pr_test.go | 34 +++++++++++++++++++++++++++++++++ context/blank_context.go | 8 -------- context/context.go | 15 --------------- internal/config/config_setup.go | 9 +-------- 8 files changed, 64 insertions(+), 33 deletions(-) create mode 100644 api/queries_user.go diff --git a/api/queries_user.go b/api/queries_user.go new file mode 100644 index 000000000..c83d3aac2 --- /dev/null +++ b/api/queries_user.go @@ -0,0 +1,18 @@ +package api + +import ( + "context" + + "github.com/shurcooL/githubv4" +) + +func CurrentLoginName(client *Client) (string, error) { + var query struct { + Viewer struct { + Login string + } + } + v4 := githubv4.NewClient(client.http) + err := v4.Query(context.Background(), &query, nil) + return query.Viewer.Login, err +} diff --git a/command/issue.go b/command/issue.go index 3d3f01708..46648d914 100644 --- a/command/issue.go +++ b/command/issue.go @@ -172,7 +172,7 @@ func issueStatus(cmd *cobra.Command, args []string) error { return err } - currentUser, err := ctx.AuthLogin() + currentUser, err := api.CurrentLoginName(apiClient) if err != nil { return err } diff --git a/command/issue_test.go b/command/issue_test.go index 3df6910f5..0bb2707dc 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -20,6 +20,9 @@ func TestIssueStatus(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") + http.Register( + httpmock.GraphQL(`\bviewer\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) jsonFile, _ := os.Open("../test/fixtures/issueStatus.json") defer jsonFile.Close() @@ -49,6 +52,9 @@ func TestIssueStatus_blankSlate(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") + http.Register( + httpmock.GraphQL(`\bviewer\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { @@ -86,6 +92,9 @@ func TestIssueStatus_disabledIssues(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") + http.Register( + httpmock.GraphQL(`\bviewer\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { diff --git a/command/pr.go b/command/pr.go index 83ddf7d13..b02dd2ceb 100644 --- a/command/pr.go +++ b/command/pr.go @@ -105,7 +105,7 @@ func prStatus(cmd *cobra.Command, args []string) error { return err } - currentUser, err := ctx.AuthLogin() + currentUser, err := api.CurrentLoginName(apiClient) if err != nil { return err } diff --git a/command/pr_test.go b/command/pr_test.go index 24fce9d63..f72849d82 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -14,6 +14,7 @@ import ( "github.com/cli/cli/api" "github.com/cli/cli/internal/run" + "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/test" "github.com/google/go-cmp/cmp" ) @@ -28,6 +29,9 @@ func eq(t *testing.T, got interface{}, expected interface{}) { func TestPRStatus(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() + http.Register( + httpmock.GraphQL(`\bviewer\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prStatus.json") @@ -56,6 +60,9 @@ func TestPRStatus(t *testing.T) { func TestPRStatus_fork(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() + http.Register( + httpmock.GraphQL(`\bviewer\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubForkedRepoResponse("OWNER/REPO", "PARENT/REPO") jsonFile, _ := os.Open("../test/fixtures/prStatusFork.json") @@ -86,6 +93,9 @@ branch.blueberries.merge refs/heads/blueberries`)} func TestPRStatus_reviewsAndChecks(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() + http.Register( + httpmock.GraphQL(`\bviewer\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prStatusChecks.json") @@ -113,6 +123,9 @@ func TestPRStatus_reviewsAndChecks(t *testing.T) { func TestPRStatus_currentBranch_showTheMostRecentPR(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() + http.Register( + httpmock.GraphQL(`\bviewer\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranch.json") @@ -145,6 +158,9 @@ func TestPRStatus_currentBranch_showTheMostRecentPR(t *testing.T) { func TestPRStatus_currentBranch_defaultBranch(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() + http.Register( + httpmock.GraphQL(`\bviewer\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranch.json") @@ -166,6 +182,9 @@ func TestPRStatus_currentBranch_defaultBranch(t *testing.T) { func TestPRStatus_currentBranch_defaultBranch_repoFlag(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() + http.Register( + httpmock.GraphQL(`\bviewer\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json") defer jsonFile.Close() @@ -186,6 +205,9 @@ func TestPRStatus_currentBranch_defaultBranch_repoFlag(t *testing.T) { func TestPRStatus_currentBranch_Closed(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() + http.Register( + httpmock.GraphQL(`\bviewer\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchClosed.json") @@ -207,6 +229,9 @@ func TestPRStatus_currentBranch_Closed(t *testing.T) { func TestPRStatus_currentBranch_Closed_defaultBranch(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() + http.Register( + httpmock.GraphQL(`\bviewer\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json") @@ -228,6 +253,9 @@ func TestPRStatus_currentBranch_Closed_defaultBranch(t *testing.T) { func TestPRStatus_currentBranch_Merged(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() + http.Register( + httpmock.GraphQL(`\bviewer\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchMerged.json") @@ -249,6 +277,9 @@ func TestPRStatus_currentBranch_Merged(t *testing.T) { func TestPRStatus_currentBranch_Merged_defaultBranch(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() + http.Register( + httpmock.GraphQL(`\bviewer\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json") @@ -270,6 +301,9 @@ func TestPRStatus_currentBranch_Merged_defaultBranch(t *testing.T) { func TestPRStatus_blankSlate(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() + http.Register( + httpmock.GraphQL(`\bviewer\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` diff --git a/context/blank_context.go b/context/blank_context.go index 343f32c7e..587d556a1 100644 --- a/context/blank_context.go +++ b/context/blank_context.go @@ -39,14 +39,6 @@ func (c *blankContext) SetAuthToken(t string) { c.authToken = t } -func (c *blankContext) SetAuthLogin(login string) { - c.authLogin = login -} - -func (c *blankContext) AuthLogin() (string, error) { - return c.authLogin, nil -} - func (c *blankContext) Branch() (string, error) { if c.branch == "" { return "", fmt.Errorf("branch was not initialized") diff --git a/context/context.go b/context/context.go index 2a25f5dda..ddeb82c4d 100644 --- a/context/context.go +++ b/context/context.go @@ -18,7 +18,6 @@ const defaultHostname = "github.com" type Context interface { AuthToken() (string, error) SetAuthToken(string) - AuthLogin() (string, error) Branch() (string, error) SetBranch(string) Remotes() (Remotes, error) @@ -195,20 +194,6 @@ func (c *fsContext) SetAuthToken(t string) { c.authToken = t } -func (c *fsContext) AuthLogin() (string, error) { - config, err := c.Config() - if err != nil { - return "", err - } - - login, err := config.Get(defaultHostname, "user") - if login == "" || err != nil { - return "", err - } - - return login, nil -} - func (c *fsContext) Branch() (string, error) { if c.branch != "" { return c.branch, nil diff --git a/internal/config/config_setup.go b/internal/config/config_setup.go index 4d2a929cb..48ef6ec8a 100644 --- a/internal/config/config_setup.go +++ b/internal/config/config_setup.go @@ -116,14 +116,7 @@ func setupConfigFile(filename string) (Config, error) { func getViewer(token string) (string, error) { http := api.NewClient(api.AddHeader("Authorization", fmt.Sprintf("token %s", token))) - - response := struct { - Viewer struct { - Login string - } - }{} - err := http.GraphQL("{ viewer { login } }", nil, &response) - return response.Viewer.Login, err + return api.CurrentLoginName(http) } func waitForEnter(r io.Reader) error { From db9014fd7f07b8061a087e18b16d880c1425e21f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 20 May 2020 17:20:15 +0200 Subject: [PATCH 2/5] Respect auth token from GITHUB_TOKEN environment variable If GITHUB_TOKEN is non-blank, it overrides authentication info found in the config file. The config file is, in fact, never consulted. --- command/root.go | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/command/root.go b/command/root.go index 6233be957..0db69a36a 100644 --- a/command/root.go +++ b/command/root.go @@ -100,6 +100,9 @@ var initContext = func() context.Context { if repo := os.Getenv("GH_REPO"); repo != "" { ctx.SetBaseRepo(repo) } + if token := os.Getenv("GITHUB_TOKEN"); token != "" { + ctx.SetAuthToken(token) + } return ctx } @@ -112,11 +115,15 @@ func BasicClient() (*api.Client, error) { } opts = append(opts, api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version))) - if c, err := config.ParseDefaultConfig(); err == nil { - if token, _ := c.Get(defaultHostname, "oauth_token"); token != "" { - opts = append(opts, api.AddHeader("Authorization", fmt.Sprintf("token %s", token))) + token := os.Getenv("GITHUB_TOKEN") + if token == "" { + if c, err := config.ParseDefaultConfig(); err == nil { + token, _ = c.Get(defaultHostname, "oauth_token") } } + if token != "" { + opts = append(opts, api.AddHeader("Authorization", fmt.Sprintf("token %s", token))) + } return api.NewClient(opts...), nil } @@ -144,8 +151,12 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) { return fmt.Sprintf("token %s", token) } + tokenFromEnv := func() bool { + return os.Getenv("GITHUB_TOKEN") == token + } + checkScopesFunc := func(appID string) error { - if config.IsGitHubApp(appID) && utils.IsTerminal(os.Stdin) && utils.IsTerminal(os.Stderr) { + if config.IsGitHubApp(appID) && !tokenFromEnv() && utils.IsTerminal(os.Stdin) && utils.IsTerminal(os.Stderr) { newToken, loginHandle, err := config.AuthFlow("Notice: additional authorization required") if err != nil { return err @@ -167,7 +178,11 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) { } else { fmt.Fprintln(os.Stderr, "Warning: gh now requires the `read:org` OAuth scope.") fmt.Fprintln(os.Stderr, "Visit https://github.com/settings/tokens and edit your token to enable `read:org`") - fmt.Fprintln(os.Stderr, "or generate a new token and paste it via `gh config set -h github.com oauth_token MYTOKEN`") + if tokenFromEnv() { + fmt.Fprintln(os.Stderr, "or generate a new token for the GITHUB_TOKEN environment variable") + } else { + fmt.Fprintln(os.Stderr, "or generate a new token and paste it via `gh config set -h github.com oauth_token MYTOKEN`") + } } return nil } From d9e39226caa2ee823ce8bab021a52e3095e18bad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 20 May 2020 17:41:33 +0200 Subject: [PATCH 3/5] Clean up unused struct field https://github.com/cli/cli/pull/976/checks?check_run_id=693382299 --- context/blank_context.go | 1 - 1 file changed, 1 deletion(-) diff --git a/context/blank_context.go b/context/blank_context.go index 587d556a1..ed8784cfc 100644 --- a/context/blank_context.go +++ b/context/blank_context.go @@ -17,7 +17,6 @@ func NewBlank() *blankContext { // A Context implementation that queries the filesystem type blankContext struct { authToken string - authLogin string branch string baseRepo ghrepo.Interface remotes Remotes From 1f3725c94d6ccac0a73c5133510d822a8248fa7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 22 May 2020 15:58:57 +0200 Subject: [PATCH 4/5] Use `@me` to avoid having to look up current user in `pr status` --- command/pr.go | 7 ++----- command/pr_test.go | 34 ---------------------------------- 2 files changed, 2 insertions(+), 39 deletions(-) diff --git a/command/pr.go b/command/pr.go index b02dd2ceb..fc3ec9d64 100644 --- a/command/pr.go +++ b/command/pr.go @@ -105,11 +105,6 @@ func prStatus(cmd *cobra.Command, args []string) error { return err } - currentUser, err := api.CurrentLoginName(apiClient) - if err != nil { - return err - } - baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) if err != nil { return err @@ -122,6 +117,8 @@ func prStatus(cmd *cobra.Command, args []string) error { return fmt.Errorf("could not query for pull request for current branch: %w", err) } + // the `@me` macro is available because the API lookup is ElasticSearch-based + currentUser := "@me" prPayload, err := api.PullRequests(apiClient, baseRepo, currentPRNumber, currentPRHeadRef, currentUser) if err != nil { return err diff --git a/command/pr_test.go b/command/pr_test.go index f72849d82..24fce9d63 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -14,7 +14,6 @@ import ( "github.com/cli/cli/api" "github.com/cli/cli/internal/run" - "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/test" "github.com/google/go-cmp/cmp" ) @@ -29,9 +28,6 @@ func eq(t *testing.T, got interface{}, expected interface{}) { func TestPRStatus(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() - http.Register( - httpmock.GraphQL(`\bviewer\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prStatus.json") @@ -60,9 +56,6 @@ func TestPRStatus(t *testing.T) { func TestPRStatus_fork(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() - http.Register( - httpmock.GraphQL(`\bviewer\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubForkedRepoResponse("OWNER/REPO", "PARENT/REPO") jsonFile, _ := os.Open("../test/fixtures/prStatusFork.json") @@ -93,9 +86,6 @@ branch.blueberries.merge refs/heads/blueberries`)} func TestPRStatus_reviewsAndChecks(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() - http.Register( - httpmock.GraphQL(`\bviewer\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prStatusChecks.json") @@ -123,9 +113,6 @@ func TestPRStatus_reviewsAndChecks(t *testing.T) { func TestPRStatus_currentBranch_showTheMostRecentPR(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() - http.Register( - httpmock.GraphQL(`\bviewer\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranch.json") @@ -158,9 +145,6 @@ func TestPRStatus_currentBranch_showTheMostRecentPR(t *testing.T) { func TestPRStatus_currentBranch_defaultBranch(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() - http.Register( - httpmock.GraphQL(`\bviewer\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranch.json") @@ -182,9 +166,6 @@ func TestPRStatus_currentBranch_defaultBranch(t *testing.T) { func TestPRStatus_currentBranch_defaultBranch_repoFlag(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() - http.Register( - httpmock.GraphQL(`\bviewer\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json") defer jsonFile.Close() @@ -205,9 +186,6 @@ func TestPRStatus_currentBranch_defaultBranch_repoFlag(t *testing.T) { func TestPRStatus_currentBranch_Closed(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() - http.Register( - httpmock.GraphQL(`\bviewer\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchClosed.json") @@ -229,9 +207,6 @@ func TestPRStatus_currentBranch_Closed(t *testing.T) { func TestPRStatus_currentBranch_Closed_defaultBranch(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() - http.Register( - httpmock.GraphQL(`\bviewer\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json") @@ -253,9 +228,6 @@ func TestPRStatus_currentBranch_Closed_defaultBranch(t *testing.T) { func TestPRStatus_currentBranch_Merged(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() - http.Register( - httpmock.GraphQL(`\bviewer\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponse("OWNER", "REPO") jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchMerged.json") @@ -277,9 +249,6 @@ func TestPRStatus_currentBranch_Merged(t *testing.T) { func TestPRStatus_currentBranch_Merged_defaultBranch(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() - http.Register( - httpmock.GraphQL(`\bviewer\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") jsonFile, _ := os.Open("../test/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json") @@ -301,9 +270,6 @@ func TestPRStatus_currentBranch_Merged_defaultBranch(t *testing.T) { func TestPRStatus_blankSlate(t *testing.T) { initBlankContext("", "OWNER/REPO", "blueberries") http := initFakeHTTP() - http.Register( - httpmock.GraphQL(`\bviewer\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` From c6643821dc0be6e801f0daf2928e28b8a78214f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 27 May 2020 11:57:03 +0200 Subject: [PATCH 5/5] Don't offer to reauthenticate for gist if GITHUB_TOKEN is used --- command/root.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/command/root.go b/command/root.go index 60af47c41..cabf9a7c0 100644 --- a/command/root.go +++ b/command/root.go @@ -209,7 +209,9 @@ var ensureScopes = func(ctx context.Context, client *api.Client, wantedScopes .. return client, nil } - if config.IsGitHubApp(appID) && utils.IsTerminal(os.Stdin) && utils.IsTerminal(os.Stderr) { + tokenFromEnv := len(os.Getenv("GITHUB_TOKEN")) > 0 + + if config.IsGitHubApp(appID) && !tokenFromEnv && utils.IsTerminal(os.Stdin) && utils.IsTerminal(os.Stderr) { newToken, loginHandle, err := config.AuthFlow("Notice: additional authorization required") if err != nil { return client, err @@ -234,9 +236,13 @@ var ensureScopes = func(ctx context.Context, client *api.Client, wantedScopes .. return reloadedClient, nil } else { - fmt.Fprintln(os.Stderr, fmt.Sprintf("Warning: gh now requires the `%s` OAuth scope(s).", wantedScopes)) + fmt.Fprintln(os.Stderr, fmt.Sprintf("Warning: gh now requires %s OAuth scopes.", wantedScopes)) fmt.Fprintln(os.Stderr, fmt.Sprintf("Visit https://github.com/settings/tokens and edit your token to enable %s", wantedScopes)) - fmt.Fprintln(os.Stderr, "or generate a new token and paste it via `gh config set -h github.com oauth_token MYTOKEN`") + if tokenFromEnv { + fmt.Fprintln(os.Stderr, "or generate a new token for the GITHUB_TOKEN environment variable") + } else { + fmt.Fprintln(os.Stderr, "or generate a new token and paste it via `gh config set -h github.com oauth_token MYTOKEN`") + } return client, errors.New("Unable to reauthenticate") }