STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Tweak auth flow re: interactivity

Browse source 
Fixes non-interactive login flow and make sure "prompt" configuration is
respected by never prompting if it was explicitly disabled.

No longer asks to press Enter again after "Authentication complete"
message, since that didn't provide any value to the user.
This commit is contained in:
Mislav Marohnić 2022-01-11 19:18:11 +01:00
parent a408f24d3a
commit be9f01101a
5 changed files with 48 additions and 57 deletions
Download patch file Download diff file Expand all files Collapse all files

49
internal/authflow/flow.go
View file

@ -27,13 +27,11 @@ type iconfig interface {
Write() error
}
func AuthFlowWithConfig(cfg iconfig, IO *iostreams.IOStreams, hostname, notice string, additionalScopes []string, interactive bool) (string, error) {
func AuthFlowWithConfig(cfg iconfig, IO *iostreams.IOStreams, hostname, notice string, additionalScopes []string, isInteractive bool) (string, error) {
// TODO this probably shouldn't live in this package. It should probably be in a new package that
// depends on both iostreams and config.
stderr := IO.ErrOut
cs := IO.ColorScheme()
token, userLogin, err := authFlow(hostname, IO, notice, additionalScopes, interactive)
token, userLogin, err := authFlow(hostname, IO, notice, additionalScopes, isInteractive)
if err != nil {
return "", err
}
@ -47,22 +45,10 @@ func AuthFlowWithConfig(cfg iconfig, IO *iostreams.IOStreams, hostname, notice s
return "", err
}
err = cfg.Write()
if err != nil {
return "", err
}
if interactive {
fmt.Fprintf(stderr, "%s Authentication complete. %s to continue...\n",
cs.SuccessIcon(), cs.Bold("Press Enter"))
_ = waitForEnter(IO.In)
} else {
fmt.Fprintf(stderr, "%s Authentication complete.\n", cs.SuccessIcon())
}
return token, nil
return token, cfg.Write()
}
func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string, interactive bool) (string, string, error) {
func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string, isInteractive bool) (string, string, error) {
w := IO.ErrOut
cs := IO.ColorScheme()
@ -93,24 +79,25 @@ func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, addition
return nil
},
BrowseURL: func(url string) error {
if interactive {
fmt.Fprintf(w, "- %s to open %s in your browser... ", cs.Bold("Press Enter"), oauthHost)
_ = waitForEnter(IO.In)
if !isInteractive {
fmt.Fprintf(w, "%s to continue in your web browser: %s\n", cs.Bold("Open this URL"), url)
return nil
}
// FIXME: read the browser from cmd Factory rather than recreating it
browser := cmdutil.NewBrowser(os.Getenv("BROWSER"), IO.Out, IO.ErrOut)
if err := browser.Browse(url); err != nil {
fmt.Fprintf(w, "%s Failed opening a web browser at %s\n", cs.Red("!"), url)
fmt.Fprintf(w, " %s\n", err)
fmt.Fprint(w, " Please try entering the URL in your browser manually\n")
}
} else {
fmt.Fprintf(w, "- Open this URL in your browser: %s", cs.Bold(url))
fmt.Fprintf(w, "%s to open %s in your browser... ", cs.Bold("Press Enter"), oauthHost)
_ = waitForEnter(IO.In)
// FIXME: read the browser from cmd Factory rather than recreating it
browser := cmdutil.NewBrowser(os.Getenv("BROWSER"), IO.Out, IO.ErrOut)
if err := browser.Browse(url); err != nil {
fmt.Fprintf(w, "%s Failed opening a web browser at %s\n", cs.Red("!"), url)
fmt.Fprintf(w, " %s\n", err)
fmt.Fprint(w, " Please try entering the URL in your browser manually\n")
}
return nil
},
WriteSuccessHTML: func(w io.Writer) {
fmt.Fprintln(w, oauthSuccessPage)
fmt.Fprint(w, oauthSuccessPage)
},
HTTPClient: httpClient,
Stdin: IO.In,

35
pkg/cmd/auth/login/login.go
View file

@ -68,37 +68,34 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm
$ gh auth login --hostname enterprise.internal
`),
RunE: func(cmd *cobra.Command, args []string) error {
if !opts.IO.CanPrompt() && !(tokenStdin || opts.Web) {
return cmdutil.FlagErrorf("--web or --with-token required when not running interactively")
}
if tokenStdin && opts.Web {
return cmdutil.FlagErrorf("specify only one of --web or --with-token")
return cmdutil.FlagErrorf("specify only one of `--web` or `--with-token`")
}
if tokenStdin && len(opts.Scopes) > 0 {
return cmdutil.FlagErrorf("specify only one of `--scopes` or `--with-token`")
}
if tokenStdin {
defer opts.IO.In.Close()
token, err := ioutil.ReadAll(opts.IO.In)
if err != nil {
return fmt.Errorf("failed to read token from STDIN: %w", err)
return fmt.Errorf("failed to read token from standard input: %w", err)
}
opts.Token = strings.TrimSpace(string(token))
}
if opts.IO.CanPrompt() && opts.Token == "" && !opts.Web {
if opts.IO.CanPrompt() && opts.Token == "" {
opts.Interactive = true
}
if cmd.Flags().Changed("hostname") {
if err := ghinstance.HostnameValidator(opts.Hostname); err != nil {
return cmdutil.FlagErrorf("error parsing --hostname: %w", err)
return cmdutil.FlagErrorf("error parsing hostname: %w", err)
}
}
if !opts.Interactive {
if opts.Hostname == "" {
opts.Hostname = ghinstance.Default()
}
if opts.Hostname == "" && (!opts.Interactive || opts.Web) {
opts.Hostname = ghinstance.Default()
}
opts.MainExecutable = f.Executable()
@ -125,15 +122,11 @@ func loginRun(opts *LoginOptions) error {
}
hostname := opts.Hostname
if hostname == "" {
if opts.Interactive {
var err error
hostname, err = promptForHostname()
if err != nil {
return err
}
} else {
return errors.New("must specify --hostname")
if opts.Interactive && hostname == "" {
var err error
hostname, err = promptForHostname()
if err != nil {
return err
}
}

15
pkg/cmd/auth/login/login_test.go
View file

@ -50,13 +50,19 @@ func Test_NewCmdLogin(t *testing.T) {
name: "nontty, hostname",
stdinTTY: false,
cli: "--hostname claire.redfield",
wantsErr: true,
wants: LoginOptions{
Hostname: "claire.redfield",
Token: "",
},
},
{
name: "nontty",
stdinTTY: false,
cli: "",
wantsErr: true,
wants: LoginOptions{
Hostname: "github.com",
Token: "",
},
},
{
name: "nontty, with-token, hostname",
@ -102,8 +108,9 @@ func Test_NewCmdLogin(t *testing.T) {
stdinTTY: true,
cli: "--web",
wants: LoginOptions{
Hostname: "github.com",
Web: true,
Hostname: "github.com",
Web: true,
Interactive: true,
},
},
{

3
pkg/cmd/auth/refresh/refresh.go
View file

@ -158,6 +158,9 @@ func refreshRun(opts *RefreshOptions) error {
return err
}
cs := opts.IO.ColorScheme()
fmt.Fprintf(opts.IO.ErrOut, "%s Authentication complete.\n", cs.SuccessIcon())
if credentialFlow.ShouldSetup() {
username, _ := cfg.Get(hostname, "user")
password, _ := cfg.Get(hostname, "oauth_token")

3
pkg/cmd/auth/shared/login_flow.go
View file

@ -99,7 +99,7 @@ func Login(opts *LoginOptions) error {
var authMode int
if opts.Web {
authMode = 0
} else {
} else if opts.Interactive {
err := prompt.SurveyAskOne(&survey.Select{
Message: "How would you like to authenticate GitHub CLI?",
Options: []string{
@ -121,6 +121,7 @@ func Login(opts *LoginOptions) error {
if err != nil {
return fmt.Errorf("failed to authenticate via web browser: %w", err)
}
fmt.Fprintf(opts.IO.ErrOut, "%s Authentication complete.\n", cs.SuccessIcon())
userValidated = true
} else {
minimumScopes := append([]string{"repo", "read:org"}, additionalScopes...)