X Tutup
Skip to content

Commit 8198cce

Browse files
mislavmislav
authored
Merge pull request cli#3997 from nicknotfun/non-interactive-auth
Add interactive mode to auth flow, pass from login and refresh Closes cli#4506
2 parents ad8d7bb + be9f011 commit 8198cce

File tree

6 files changed

+47
-49
lines changed

6 files changed

+47
-49
lines changed

‎internal/authflow/flow.go‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,11 @@ type iconfig interface {
2727
Write() error
2828
}
2929

30-
func AuthFlowWithConfig(cfg iconfig, IO *iostreams.IOStreams, hostname, notice string, additionalScopes []string) (string, error) {
30+
func AuthFlowWithConfig(cfg iconfig, IO *iostreams.IOStreams, hostname, notice string, additionalScopes []string, isInteractive bool) (string, error) {
3131
// TODO this probably shouldn't live in this package. It should probably be in a new package that
3232
// depends on both iostreams and config.
33-
stderr := IO.ErrOut
34-
cs := IO.ColorScheme()
3533

36-
token, userLogin, err := authFlow(hostname, IO, notice, additionalScopes)
34+
token, userLogin, err := authFlow(hostname, IO, notice, additionalScopes, isInteractive)
3735
if err != nil {
3836
return "", err
3937
}
@@ -47,19 +45,10 @@ func AuthFlowWithConfig(cfg iconfig, IO *iostreams.IOStreams, hostname, notice s
4745
return "", err
4846
}
4947

50-
err = cfg.Write()
51-
if err != nil {
52-
return "", err
53-
}
54-
55-
fmt.Fprintf(stderr, "%s Authentication complete. %s to continue...\n",
56-
cs.SuccessIcon(), cs.Bold("Press Enter"))
57-
_ = waitForEnter(IO.In)
58-
59-
return token, nil
48+
return token, cfg.Write()
6049
}
6150

62-
func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string) (string, string, error) {
51+
func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string, isInteractive bool) (string, string, error) {
6352
w := IO.ErrOut
6453
cs := IO.ColorScheme()
6554

@@ -90,7 +79,12 @@ func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, addition
9079
return nil
9180
},
9281
BrowseURL: func(url string) error {
93-
fmt.Fprintf(w, "- %s to open %s in your browser... ", cs.Bold("Press Enter"), oauthHost)
82+
if !isInteractive {
83+
fmt.Fprintf(w, "%s to continue in your web browser: %s\n", cs.Bold("Open this URL"), url)
84+
return nil
85+
}
86+
87+
fmt.Fprintf(w, "%s to open %s in your browser... ", cs.Bold("Press Enter"), oauthHost)
9488
_ = waitForEnter(IO.In)
9589

9690
// FIXME: read the browser from cmd Factory rather than recreating it
@@ -103,7 +97,7 @@ func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, addition
10397
return nil
10498
},
10599
WriteSuccessHTML: func(w io.Writer) {
106-
fmt.Fprintln(w, oauthSuccessPage)
100+
fmt.Fprint(w, oauthSuccessPage)
107101
},
108102
HTTPClient: httpClient,
109103
Stdin: IO.In,

‎pkg/cmd/auth/login/login.go‎

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -68,37 +68,34 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm
6868
$ gh auth login --hostname enterprise.internal
6969
`),
7070
RunE: func(cmd *cobra.Command, args []string) error {
71-
if !opts.IO.CanPrompt() && !(tokenStdin || opts.Web) {
72-
return cmdutil.FlagErrorf("--web or --with-token required when not running interactively")
73-
}
74-
7571
if tokenStdin && opts.Web {
76-
return cmdutil.FlagErrorf("specify only one of --web or --with-token")
72+
return cmdutil.FlagErrorf("specify only one of `--web` or `--with-token`")
73+
}
74+
if tokenStdin && len(opts.Scopes) > 0 {
75+
return cmdutil.FlagErrorf("specify only one of `--scopes` or `--with-token`")
7776
}
7877

7978
if tokenStdin {
8079
defer opts.IO.In.Close()
8180
token, err := ioutil.ReadAll(opts.IO.In)
8281
if err != nil {
83-
return fmt.Errorf("failed to read token from STDIN: %w", err)
82+
return fmt.Errorf("failed to read token from standard input: %w", err)
8483
}
8584
opts.Token = strings.TrimSpace(string(token))
8685
}
8786

88-
if opts.IO.CanPrompt() && opts.Token == "" && !opts.Web {
87+
if opts.IO.CanPrompt() && opts.Token == "" {
8988
opts.Interactive = true
9089
}
9190

9291
if cmd.Flags().Changed("hostname") {
9392
if err := ghinstance.HostnameValidator(opts.Hostname); err != nil {
94-
return cmdutil.FlagErrorf("error parsing --hostname: %w", err)
93+
return cmdutil.FlagErrorf("error parsing hostname: %w", err)
9594
}
9695
}
9796

98-
if !opts.Interactive {
99-
if opts.Hostname == "" {
100-
opts.Hostname = ghinstance.Default()
101-
}
97+
if opts.Hostname == "" && (!opts.Interactive || opts.Web) {
98+
opts.Hostname = ghinstance.Default()
10299
}
103100

104101
opts.MainExecutable = f.Executable()
@@ -125,15 +122,11 @@ func loginRun(opts *LoginOptions) error {
125122
}
126123

127124
hostname := opts.Hostname
128-
if hostname == "" {
129-
if opts.Interactive {
130-
var err error
131-
hostname, err = promptForHostname()
132-
if err != nil {
133-
return err
134-
}
135-
} else {
136-
return errors.New("must specify --hostname")
125+
if opts.Interactive && hostname == "" {
126+
var err error
127+
hostname, err = promptForHostname()
128+
if err != nil {
129+
return err
137130
}
138131
}
139132

‎pkg/cmd/auth/login/login_test.go‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,19 @@ func Test_NewCmdLogin(t *testing.T) {
5050
name: "nontty, hostname",
5151
stdinTTY: false,
5252
cli: "--hostname claire.redfield",
53-
wantsErr: true,
53+
wants: LoginOptions{
54+
Hostname: "claire.redfield",
55+
Token: "",
56+
},
5457
},
5558
{
5659
name: "nontty",
5760
stdinTTY: false,
5861
cli: "",
59-
wantsErr: true,
62+
wants: LoginOptions{
63+
Hostname: "github.com",
64+
Token: "",
65+
},
6066
},
6167
{
6268
name: "nontty, with-token, hostname",
@@ -102,8 +108,9 @@ func Test_NewCmdLogin(t *testing.T) {
102108
stdinTTY: true,
103109
cli: "--web",
104110
wants: LoginOptions{
105-
Hostname: "github.com",
106-
Web: true,
111+
Hostname: "github.com",
112+
Web: true,
113+
Interactive: true,
107114
},
108115
},
109116
{

‎pkg/cmd/auth/refresh/refresh.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ type RefreshOptions struct {
2626

2727
Hostname string
2828
Scopes []string
29-
AuthFlow func(config.Config, *iostreams.IOStreams, string, []string) error
29+
AuthFlow func(config.Config, *iostreams.IOStreams, string, []string, bool) error
3030

3131
Interactive bool
3232
}
@@ -35,8 +35,8 @@ func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra.
3535
opts := &RefreshOptions{
3636
IO: f.IOStreams,
3737
Config: f.Config,
38-
AuthFlow: func(cfg config.Config, io *iostreams.IOStreams, hostname string, scopes []string) error {
39-
_, err := authflow.AuthFlowWithConfig(cfg, io, hostname, "", scopes)
38+
AuthFlow: func(cfg config.Config, io *iostreams.IOStreams, hostname string, scopes []string, interactive bool) error {
39+
_, err := authflow.AuthFlowWithConfig(cfg, io, hostname, "", scopes, interactive)
4040
return err
4141
},
4242
httpClient: http.DefaultClient,
@@ -154,10 +154,13 @@ func refreshRun(opts *RefreshOptions) error {
154154
additionalScopes = append(additionalScopes, credentialFlow.Scopes()...)
155155
}
156156

157-
if err := opts.AuthFlow(cfg, opts.IO, hostname, append(opts.Scopes, additionalScopes...)); err != nil {
157+
if err := opts.AuthFlow(cfg, opts.IO, hostname, append(opts.Scopes, additionalScopes...), opts.Interactive); err != nil {
158158
return err
159159
}
160160

161+
cs := opts.IO.ColorScheme()
162+
fmt.Fprintf(opts.IO.ErrOut, "%s Authentication complete.\n", cs.SuccessIcon())
163+
161164
if credentialFlow.ShouldSetup() {
162165
username, _ := cfg.Get(hostname, "user")
163166
password, _ := cfg.Get(hostname, "oauth_token")

‎pkg/cmd/auth/refresh/refresh_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ func Test_refreshRun(t *testing.T) {
232232
for _, tt := range tests {
233233
t.Run(tt.name, func(t *testing.T) {
234234
aa := authArgs{}
235-
tt.opts.AuthFlow = func(_ config.Config, _ *iostreams.IOStreams, hostname string, scopes []string) error {
235+
tt.opts.AuthFlow = func(_ config.Config, _ *iostreams.IOStreams, hostname string, scopes []string, interactive bool) error {
236236
aa.hostname = hostname
237237
aa.scopes = scopes
238238
return nil

‎pkg/cmd/auth/shared/login_flow.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func Login(opts *LoginOptions) error {
9999
var authMode int
100100
if opts.Web {
101101
authMode = 0
102-
} else {
102+
} else if opts.Interactive {
103103
err := prompt.SurveyAskOne(&survey.Select{
104104
Message: "How would you like to authenticate GitHub CLI?",
105105
Options: []string{
@@ -117,10 +117,11 @@ func Login(opts *LoginOptions) error {
117117

118118
if authMode == 0 {
119119
var err error
120-
authToken, err = authflow.AuthFlowWithConfig(cfg, opts.IO, hostname, "", append(opts.Scopes, additionalScopes...))
120+
authToken, err = authflow.AuthFlowWithConfig(cfg, opts.IO, hostname, "", append(opts.Scopes, additionalScopes...), opts.Interactive)
121121
if err != nil {
122122
return fmt.Errorf("failed to authenticate via web browser: %w", err)
123123
}
124+
fmt.Fprintf(opts.IO.ErrOut, "%s Authentication complete.\n", cs.SuccessIcon())
124125
userValidated = true
125126
} else {
126127
minimumScopes := append([]string{"repo", "read:org"}, additionalScopes...)

0 commit comments

Comments
 (0)
X Tutup