X Tutup
Skip to content

Commit 3aaa231

Browse files
mislavmislav
committed
Guide user through re-authorization flow if read:org scope is missing
How this works for people with existing OAuth tokens: $ gh issue list -L1 Notice: additional authorization required Press Enter to open github.com in your browser... [auth flow in the browser...] Authentication complete. Press Enter to continue... Showing 1 of 132 issues in cli/cli ... Users of Personal Access Tokens get a different notice: Warning: gh now requires the `read:org` OAuth scope. Visit https://github.com/settings/tokens and edit your token to enable `read:org` or generate a new token and paste it via `gh config set -h github.com oauth_token MYTOKEN`
1 parent 8ed9a03 commit 3aaa231

File tree

3 files changed

+81
-30
lines changed

3 files changed

+81
-30
lines changed

‎api/client.go‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"io"
88
"io/ioutil"
99
"net/http"
10-
"os"
1110
"regexp"
1211
"strings"
1312

@@ -38,6 +37,16 @@ func AddHeader(name, value string) ClientOption {
3837
}
3938
}
4039

40+
// AddHeaderFunc is an AddHeader that gets the string value from a function
41+
func AddHeaderFunc(name string, value func() string) ClientOption {
42+
return func(tr http.RoundTripper) http.RoundTripper {
43+
return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) {
44+
req.Header.Add(name, value())
45+
return tr.RoundTrip(req)
46+
}}
47+
}
48+
}
49+
4150
// VerboseLog enables request/response logging within a RoundTripper
4251
func VerboseLog(out io.Writer, logTraffic bool, colorize bool) ClientOption {
4352
logger := &httpretty.Logger{
@@ -67,15 +76,15 @@ func ReplaceTripper(tr http.RoundTripper) ClientOption {
6776
var issuedScopesWarning bool
6877

6978
// CheckScopes checks whether an OAuth scope is present in a response
70-
func CheckScopes(wantedScope string) ClientOption {
79+
func CheckScopes(wantedScope string, cb func(string) error) ClientOption {
7180
return func(tr http.RoundTripper) http.RoundTripper {
7281
return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) {
7382
res, err := tr.RoundTrip(req)
74-
if err != nil || issuedScopesWarning {
83+
if err != nil || res.StatusCode > 299 || issuedScopesWarning {
7584
return res, err
7685
}
7786

78-
isApp := res.Header.Get("X-Oauth-Client-Id") != ""
87+
appID := res.Header.Get("X-Oauth-Client-Id")
7988
hasScopes := strings.Split(res.Header.Get("X-Oauth-Scopes"), ",")
8089

8190
hasWanted := false
@@ -87,14 +96,8 @@ func CheckScopes(wantedScope string) ClientOption {
8796
}
8897

8998
if !hasWanted {
90-
fmt.Fprintln(os.Stderr, "Warning: gh now requires the `read:org` OAuth scope.")
91-
// TODO: offer to take the person through the authentication flow again?
92-
// TODO: retry the original request if it was a read?
93-
if isApp {
94-
fmt.Fprintln(os.Stderr, "To re-authenticate, please `rm ~/.config/gh/config.yml` and try again.")
95-
} else {
96-
// the person has pasted a Personal Access Token
97-
fmt.Fprintln(os.Stderr, "Re-generate your token in `rm ~/.config/gh/config.yml` and try again.")
99+
if err := cb(appID); err != nil {
100+
return res, err
98101
}
99102
issuedScopesWarning = true
100103
}

‎command/root.go‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,13 +142,46 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) {
142142
if err != nil {
143143
return nil, err
144144
}
145+
145146
var opts []api.ClientOption
146147
if verbose := os.Getenv("DEBUG"); verbose != "" {
147148
opts = append(opts, apiVerboseLog())
148149
}
150+
151+
getAuthValue := func() string {
152+
return fmt.Sprintf("token %s", token)
153+
}
154+
checkScopesFunc := func(appID string) error {
155+
if config.IsGitHubApp(appID) && utils.IsTerminal(os.Stdin) && utils.IsTerminal(os.Stderr) {
156+
newToken, loginHandle, err := config.AuthFlow("Notice: additional authorization required")
157+
if err != nil {
158+
return err
159+
}
160+
cfg, err := ctx.Config()
161+
if err != nil {
162+
return err
163+
}
164+
_ = cfg.Set(defaultHostname, "oauth_token", newToken)
165+
_ = cfg.Set(defaultHostname, "user", loginHandle)
166+
// update config file on disk
167+
err = cfg.Write()
168+
if err != nil {
169+
return err
170+
}
171+
// update configuration in memory
172+
token = newToken
173+
config.AuthFlowComplete()
174+
} else {
175+
fmt.Fprintln(os.Stderr, "Warning: gh now requires the `read:org` OAuth scope.")
176+
fmt.Fprintln(os.Stderr, "Visit https://github.com/settings/tokens and edit your token to enable `read:org`")
177+
fmt.Fprintln(os.Stderr, "or generate a new token and paste it via `gh config set -h github.com oauth_token MYTOKEN`")
178+
}
179+
return nil
180+
}
181+
149182
opts = append(opts,
150-
api.CheckScopes("read:org"),
151-
api.AddHeader("Authorization", fmt.Sprintf("token %s", token)),
183+
api.CheckScopes("read:org", checkScopesFunc),
184+
api.AddHeaderFunc("Authorization", getAuthValue),
152185
api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version)),
153186
// antiope-preview: Checks
154187
api.AddHeader("Accept", "application/vnd.github.antiope-preview+json"),

‎internal/config/config_setup.go‎

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,14 @@ var (
2424
oauthClientSecret = "34ddeff2b558a23d38fba8a6de74f086ede1cc0b"
2525
)
2626

27-
// TODO: have a conversation about whether this belongs in the "context" package
28-
// FIXME: make testable
29-
func setupConfigFile(filename string) (Config, error) {
27+
// IsGitHubApp reports whether an OAuth app is "GitHub CLI" or "GitHub CLI (dev)"
28+
func IsGitHubApp(id string) bool {
29+
// this intentionally doesn't use `oauthClientID` because that is a variable
30+
// that can potentially be changed at build time via GH_OAUTH_CLIENT_ID
31+
return id == "178c6fc778ccc68e1d6a" || id == "4d747ba5675d5d66553f"
32+
}
33+
34+
func AuthFlow(notice string) (string, string, error) {
3035
var verboseStream io.Writer
3136
if strings.Contains(os.Getenv("DEBUG"), "oauth") {
3237
verboseStream = os.Stderr
@@ -43,15 +48,30 @@ func setupConfigFile(filename string) (Config, error) {
4348
VerboseStream: verboseStream,
4449
}
4550

46-
fmt.Fprintln(os.Stderr, "Notice: authentication required")
51+
fmt.Fprintln(os.Stderr, notice)
4752
fmt.Fprintf(os.Stderr, "Press Enter to open %s in your browser... ", flow.Hostname)
4853
_ = waitForEnter(os.Stdin)
4954
token, err := flow.ObtainAccessToken()
5055
if err != nil {
51-
return nil, err
56+
return "", "", err
5257
}
5358

5459
userLogin, err := getViewer(token)
60+
if err != nil {
61+
return "", "", err
62+
}
63+
64+
return token, userLogin, nil
65+
}
66+
67+
func AuthFlowComplete() {
68+
fmt.Fprintln(os.Stderr, "Authentication complete. Press Enter to continue... ")
69+
_ = waitForEnter(os.Stdin)
70+
}
71+
72+
// FIXME: make testable
73+
func setupConfigFile(filename string) (Config, error) {
74+
token, userLogin, err := AuthFlow("Notice: authentication required")
5575
if err != nil {
5676
return nil, err
5777
}
@@ -62,9 +82,9 @@ func setupConfigFile(filename string) (Config, error) {
6282
}
6383

6484
yamlHosts := map[string]map[string]string{}
65-
yamlHosts[flow.Hostname] = map[string]string{}
66-
yamlHosts[flow.Hostname]["user"] = userLogin
67-
yamlHosts[flow.Hostname]["oauth_token"] = token
85+
yamlHosts[oauthHost] = map[string]string{}
86+
yamlHosts[oauthHost]["user"] = userLogin
87+
yamlHosts[oauthHost]["oauth_token"] = token
6888

6989
defaultConfig := yamlConfig{
7090
Hosts: yamlHosts,
@@ -85,14 +105,9 @@ func setupConfigFile(filename string) (Config, error) {
85105
if err != nil {
86106
return nil, err
87107
}
88-
n, err := cfgFile.Write(yamlData)
89-
if err == nil && n < len(yamlData) {
90-
err = io.ErrShortWrite
91-
}
92-
93-
if err == nil {
94-
fmt.Fprintln(os.Stderr, "Authentication complete. Press Enter to continue... ")
95-
_ = waitForEnter(os.Stdin)
108+
_, err = cfgFile.Write(yamlData)
109+
if err != nil {
110+
return nil, err
96111
}
97112

98113
// TODO cleaner error handling? this "should" always work given that we /just/ wrote the file...

0 commit comments

Comments
 (0)
X Tutup