Skip to content

Commit 77227a6

Browse files
mislavmislav
committed
Trigger OAuth flow only when requesting auth token
Previously we would trigger OAuth flow when the config file did not exist. Now we will let an empty Config object be initialized in that case, but trigger OAuth flow when the Context caller requests an AuthToken.
1 parent fd7b87f commit 77227a6

File tree

4 files changed

+41
-62
lines changed

4 files changed

+41
-62
lines changed

‎command/root.go‎

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -180,24 +180,16 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) {
180180

181181
checkScopesFunc := func(appID string) error {
182182
if config.IsGitHubApp(appID) && !tokenFromEnv() && utils.IsTerminal(os.Stdin) && utils.IsTerminal(os.Stderr) {
183-
newToken, loginHandle, err := config.AuthFlow("Notice: additional authorization required")
184-
if err != nil {
185-
return err
186-
}
187183
cfg, err := ctx.Config()
188184
if err != nil {
189185
return err
190186
}
191-
_ = cfg.Set(defaultHostname, "oauth_token", newToken)
192-
_ = cfg.Set(defaultHostname, "user", loginHandle)
193-
// update config file on disk
194-
err = cfg.Write()
187+
newToken, err := config.AuthFlowWithConfig(cfg, defaultHostname, "Notice: additional authorization required")
195188
if err != nil {
196189
return err
197190
}
198191
// update configuration in memory
199192
token = newToken
200-
config.AuthFlowComplete()
201193
} else {
202194
fmt.Fprintln(os.Stderr, "Warning: gh now requires the `read:org` OAuth scope.")
203195
fmt.Fprintln(os.Stderr, "Visit https://github.com/settings/tokens and edit your token to enable `read:org`")
@@ -234,28 +226,19 @@ var ensureScopes = func(ctx context.Context, client *api.Client, wantedScopes ..
234226
tokenFromEnv := len(os.Getenv("GITHUB_TOKEN")) > 0
235227

236228
if config.IsGitHubApp(appID) && !tokenFromEnv && utils.IsTerminal(os.Stdin) && utils.IsTerminal(os.Stderr) {
237-
newToken, loginHandle, err := config.AuthFlow("Notice: additional authorization required")
238-
if err != nil {
239-
return client, err
240-
}
241229
cfg, err := ctx.Config()
242230
if err != nil {
243-
return client, err
231+
return nil, err
244232
}
245-
_ = cfg.Set(defaultHostname, "oauth_token", newToken)
246-
_ = cfg.Set(defaultHostname, "user", loginHandle)
247-
// update config file on disk
248-
err = cfg.Write()
233+
_, err = config.AuthFlowWithConfig(cfg, defaultHostname, "Notice: additional authorization required")
249234
if err != nil {
250-
return client, err
235+
return nil, err
251236
}
252-
// update configuration in memory
253-
config.AuthFlowComplete()
237+
254238
reloadedClient, err := apiClientForContext(ctx)
255239
if err != nil {
256240
return client, err
257241
}
258-
259242
return reloadedClient, nil
260243
} else {
261244
fmt.Fprintln(os.Stderr, fmt.Sprintf("Warning: gh now requires %s OAuth scopes.", wantedScopes))

‎context/context.go‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package context
33
import (
44
"errors"
55
"fmt"
6+
"os"
67
"sort"
78

89
"github.com/cli/cli/api"
@@ -162,11 +163,13 @@ type fsContext struct {
162163

163164
func (c *fsContext) Config() (config.Config, error) {
164165
if c.config == nil {
165-
config, err := config.ParseOrSetupConfigFile(config.ConfigFile())
166-
if err != nil {
166+
cfg, err := config.ParseDefaultConfig()
167+
if errors.Is(err, os.ErrNotExist) {
168+
cfg = config.NewBlankConfig()
169+
} else if err != nil {
167170
return nil, err
168171
}
169-
c.config = config
172+
c.config = cfg
170173
c.authToken = ""
171174
}
172175
return c.config, nil
@@ -182,8 +185,12 @@ func (c *fsContext) AuthToken() (string, error) {
182185
return "", err
183186
}
184187

188+
var notFound *config.NotFoundError
185189
token, err := cfg.Get(defaultHostname, "oauth_token")
186-
if token == "" || err != nil {
190+
if token == "" || errors.As(err, &notFound) {
191+
// interactive OAuth flow
192+
return config.AuthFlowWithConfig(cfg, defaultHostname, "Notice: authentication required")
193+
} else if err != nil {
187194
return "", err
188195
}
189196

‎internal/config/config_file.go‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,6 @@ func ConfigFile() string {
2222
return path.Join(ConfigDir(), "config.yml")
2323
}
2424

25-
func ParseOrSetupConfigFile(fn string) (Config, error) {
26-
config, err := ParseConfig(fn)
27-
if err != nil && errors.Is(err, os.ErrNotExist) {
28-
return setupConfigFile(fn)
29-
}
30-
return config, err
31-
}
32-
3325
func ParseDefaultConfig() (Config, error) {
3426
return ParseConfig(ConfigFile())
3527
}

‎internal/config/config_setup.go‎

Lines changed: 25 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,6 @@ import (
1111
"github.com/cli/cli/auth"
1212
)
1313

14-
const (
15-
oauthHost = "github.com"
16-
)
17-
1814
var (
1915
// The "GitHub CLI" OAuth app
2016
oauthClientID = "178c6fc778ccc68e1d6a"
@@ -29,7 +25,31 @@ func IsGitHubApp(id string) bool {
2925
return id == "178c6fc778ccc68e1d6a" || id == "4d747ba5675d5d66553f"
3026
}
3127

32-
func AuthFlow(notice string) (string, string, error) {
28+
func AuthFlowWithConfig(cfg Config, hostname, notice string) (string, error) {
29+
token, userLogin, err := AuthFlow(hostname, notice)
30+
if err != nil {
31+
return "", err
32+
}
33+
34+
err = cfg.Set(hostname, "user", userLogin)
35+
if err != nil {
36+
return "", err
37+
}
38+
err = cfg.Set(hostname, "oauth_token", token)
39+
if err != nil {
40+
return "", err
41+
}
42+
43+
err = cfg.Write()
44+
if err != nil {
45+
return "", err
46+
}
47+
48+
AuthFlowComplete()
49+
return token, nil
50+
}
51+
52+
func AuthFlow(oauthHost, notice string) (string, string, error) {
3353
var verboseStream io.Writer
3454
if strings.Contains(os.Getenv("DEBUG"), "oauth") {
3555
verboseStream = os.Stderr
@@ -67,29 +87,6 @@ func AuthFlowComplete() {
6787
_ = waitForEnter(os.Stdin)
6888
}
6989

70-
// FIXME: make testable
71-
func setupConfigFile(filename string) (Config, error) {
72-
token, userLogin, err := AuthFlow("Notice: authentication required")
73-
if err != nil {
74-
return nil, err
75-
}
76-
77-
cfg := NewBlankConfig()
78-
err = cfg.Set(oauthHost, "user", userLogin)
79-
if err != nil {
80-
return nil, err
81-
}
82-
err = cfg.Set(oauthHost, "oauth_token", token)
83-
if err != nil {
84-
return nil, err
85-
}
86-
87-
if err = cfg.Write(); err == nil {
88-
AuthFlowComplete()
89-
}
90-
return cfg, err
91-
}
92-
9390
func getViewer(token string) (string, error) {
9491
http := api.NewClient(api.AddHeader("Authorization", fmt.Sprintf("token %s", token)))
9592
return api.CurrentLoginName(http)

0 commit comments

Comments
 (0)