From 2e7511cd182c9196c259ca480cb1dba33838d039 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 5 Aug 2020 12:01:47 -0500 Subject: [PATCH 1/3] add helpers to config type for hosts --- internal/config/config_type.go | 37 +++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/internal/config/config_type.go b/internal/config/config_type.go index 85e5c0d4a..523430c54 100644 --- a/internal/config/config_type.go +++ b/internal/config/config_type.go @@ -4,7 +4,9 @@ import ( "bytes" "errors" "fmt" + "sort" + "github.com/cli/cli/internal/ghinstance" "gopkg.in/yaml.v3" ) @@ -14,6 +16,8 @@ const defaultGitProtocol = "https" type Config interface { Get(string, string) (string, error) Set(string, string, string) error + UnsetHost(string) + Hosts() ([]string, error) Aliases() (*AliasConfig, error) Write() error } @@ -29,7 +33,7 @@ type HostConfig struct { // This type implements a low-level get/set config that is backed by an in-memory tree of Yaml // nodes. It allows us to interact with a yaml-based config programmatically, preserving any -// comments that were present when the yaml waas parsed. +// comments that were present when the yaml was parsed. type ConfigMap struct { Root *yaml.Node } @@ -236,6 +240,20 @@ func (c *fileConfig) Set(hostname, key, value string) error { } } +func (c *fileConfig) UnsetHost(hostname string) { + if hostname == "" { + return + } + + hostsEntry, err := c.FindEntry("hosts") + if err != nil { + return + } + + cm := ConfigMap{hostsEntry.ValueNode} + cm.RemoveEntry(hostname) +} + func (c *fileConfig) configForHost(hostname string) (*HostConfig, error) { hosts, err := c.hostEntries() if err != nil { @@ -357,6 +375,23 @@ func (c *fileConfig) hostEntries() ([]*HostConfig, error) { return hostConfigs, nil } +// Hosts returns a list of all known hostnames configred in hosts.yml +func (c *fileConfig) Hosts() ([]string, error) { + entries, err := c.hostEntries() + if err != nil { + return nil, err + } + + hostnames := []string{} + for _, entry := range entries { + hostnames = append(hostnames, entry.Host) + } + + sort.SliceStable(hostnames, func(i, j int) bool { return hostnames[i] == ghinstance.Default() }) + + return hostnames, nil +} + func (c *fileConfig) makeConfigForHost(hostname string) *HostConfig { hostRoot := &yaml.Node{Kind: yaml.MappingNode} hostCfg := &HostConfig{ From da33b65d32c65a6d8040024ba60a6bb900ae2cfc Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 5 Aug 2020 12:02:51 -0500 Subject: [PATCH 2/3] remove vestigial httpclient struct member --- pkg/cmd/auth/login/login.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 60687dad8..72710f5d0 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "io/ioutil" - "net/http" "os" "strings" @@ -21,9 +20,8 @@ import ( ) type LoginOptions struct { - HttpClient func() (*http.Client, error) - IO *iostreams.IOStreams - Config func() (config.Config, error) + IO *iostreams.IOStreams + Config func() (config.Config, error) Hostname string Token string @@ -32,9 +30,8 @@ type LoginOptions struct { func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Command { opts := &LoginOptions{ - HttpClient: f.HttpClient, - IO: f.IOStreams, - Config: f.Config, + IO: f.IOStreams, + Config: f.Config, } cmd := &cobra.Command{ From 9da75395005f92232c1793340a3106632b975263 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 5 Aug 2020 12:03:08 -0500 Subject: [PATCH 3/3] gh auth logout --- command/root.go | 2 + pkg/cmd/auth/logout/logout.go | 157 +++++++++++++++++ pkg/cmd/auth/logout/logout_test.go | 259 +++++++++++++++++++++++++++++ 3 files changed, 418 insertions(+) create mode 100644 pkg/cmd/auth/logout/logout.go create mode 100644 pkg/cmd/auth/logout/logout_test.go diff --git a/command/root.go b/command/root.go index cd89f05d7..218b3a5eb 100644 --- a/command/root.go +++ b/command/root.go @@ -24,6 +24,7 @@ import ( apiCmd "github.com/cli/cli/pkg/cmd/api" authCmd "github.com/cli/cli/pkg/cmd/auth" authLoginCmd "github.com/cli/cli/pkg/cmd/auth/login" + authLogoutCmd "github.com/cli/cli/pkg/cmd/auth/logout" gistCreateCmd "github.com/cli/cli/pkg/cmd/gist/create" prCheckoutCmd "github.com/cli/cli/pkg/cmd/pr/checkout" prDiffCmd "github.com/cli/cli/pkg/cmd/pr/diff" @@ -138,6 +139,7 @@ func init() { RootCmd.AddCommand(authCmd.Cmd) authCmd.Cmd.AddCommand(authLoginCmd.NewCmdLogin(cmdFactory, nil)) + authCmd.Cmd.AddCommand(authLogoutCmd.NewCmdLogout(cmdFactory, nil)) resolvedBaseRepo := func() (ghrepo.Interface, error) { httpClient, err := cmdFactory.HttpClient() diff --git a/pkg/cmd/auth/logout/logout.go b/pkg/cmd/auth/logout/logout.go new file mode 100644 index 000000000..744cd6a3f --- /dev/null +++ b/pkg/cmd/auth/logout/logout.go @@ -0,0 +1,157 @@ +package logout + +import ( + "errors" + "fmt" + "net/http" + + "github.com/AlecAivazis/survey/v2" + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/api" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/prompt" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type LogoutOptions struct { + HttpClient func() (*http.Client, error) + IO *iostreams.IOStreams + Config func() (config.Config, error) + + Hostname string +} + +func NewCmdLogout(f *cmdutil.Factory, runF func(*LogoutOptions) error) *cobra.Command { + opts := &LogoutOptions{ + HttpClient: f.HttpClient, + IO: f.IOStreams, + Config: f.Config, + } + + cmd := &cobra.Command{ + Use: "logout", + Args: cobra.ExactArgs(0), + Short: "Log out of a GitHub host", + Long: heredoc.Doc(`Remove authentication for a GitHub host. + + This command removes the authentication configuration for a host either specified + interactively or via --hostname. + `), + Example: heredoc.Doc(` + $ gh auth logout + # => select what host to log out of via a prompt + + $ gh auth logout --hostname enterprise.internal + # => log out of specified host + `), + RunE: func(cmd *cobra.Command, args []string) error { + if runF != nil { + return runF(opts) + } + + return logoutRun(opts) + }, + } + + cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance to log out of") + + return cmd +} + +func logoutRun(opts *LogoutOptions) error { + isTTY := opts.IO.IsStdinTTY() && opts.IO.IsStdoutTTY() + + hostname := opts.Hostname + + if !isTTY && hostname == "" { + return errors.New("--hostname required when not attached to a terminal") + } + + showConfirm := isTTY && hostname == "" + + cfg, err := opts.Config() + if err != nil { + return err + } + + candidates, err := cfg.Hosts() + if err != nil { + return fmt.Errorf("not logged in to any hosts") + } + + if hostname == "" { + if len(candidates) == 1 { + hostname = candidates[0] + } else { + err = prompt.SurveyAskOne(&survey.Select{ + Message: "What account do you want to log out of?", + Options: candidates, + }, &hostname) + + if err != nil { + return fmt.Errorf("could not prompt: %w", err) + } + } + } else { + var found bool + for _, c := range candidates { + if c == hostname { + found = true + break + } + } + + if !found { + return fmt.Errorf("not logged into %s", hostname) + } + } + + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + username, err := api.CurrentLoginName(apiClient, hostname) + if err != nil { + // suppressing; the user is trying to delete this token and it might be bad. + // we'll see if the username is in the config and fall back to that. + username, _ = cfg.Get(hostname, "user") + } + + usernameStr := "" + if username != "" { + usernameStr = fmt.Sprintf(" account '%s'", username) + } + + if showConfirm { + var keepGoing bool + err := prompt.SurveyAskOne(&survey.Confirm{ + Message: fmt.Sprintf("Are you sure you want to log out of %s%s?", hostname, usernameStr), + Default: true, + }, &keepGoing) + if err != nil { + return fmt.Errorf("could not prompt: %w", err) + } + + if !keepGoing { + return nil + } + } + + cfg.UnsetHost(hostname) + err = cfg.Write() + if err != nil { + return fmt.Errorf("failed to write config, authentication configuration not updated: %w", err) + } + + if isTTY { + fmt.Fprintf(opts.IO.ErrOut, "%s Logged out of %s%s\n", + utils.GreenCheck(), utils.Bold(hostname), usernameStr) + } + + return nil +} diff --git a/pkg/cmd/auth/logout/logout_test.go b/pkg/cmd/auth/logout/logout_test.go new file mode 100644 index 000000000..83d13aad3 --- /dev/null +++ b/pkg/cmd/auth/logout/logout_test.go @@ -0,0 +1,259 @@ +package logout + +import ( + "bytes" + "net/http" + "regexp" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/prompt" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func Test_NewCmdLogout(t *testing.T) { + tests := []struct { + name string + cli string + wants LogoutOptions + }{ + { + name: "with hostname", + cli: "--hostname harry.mason", + wants: LogoutOptions{ + Hostname: "harry.mason", + }, + }, + { + name: "no arguments", + cli: "", + wants: LogoutOptions{ + Hostname: "", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, _, _, _ := iostreams.Test() + f := &cmdutil.Factory{ + IOStreams: io, + } + + argv, err := shlex.Split(tt.cli) + assert.NoError(t, err) + + var gotOpts *LogoutOptions + cmd := NewCmdLogout(f, func(opts *LogoutOptions) error { + gotOpts = opts + return nil + }) + // TODO cobra hack-around + cmd.Flags().BoolP("help", "x", false, "") + + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err = cmd.ExecuteC() + assert.NoError(t, err) + + assert.Equal(t, tt.wants.Hostname, gotOpts.Hostname) + }) + + } +} + +func Test_logoutRun_tty(t *testing.T) { + tests := []struct { + name string + opts *LogoutOptions + askStubs func(*prompt.AskStubber) + cfgHosts []string + wantHosts string + wantErrOut *regexp.Regexp + wantErr *regexp.Regexp + }{ + { + name: "no arguments, multiple hosts", + opts: &LogoutOptions{}, + cfgHosts: []string{"cheryl.mason", "github.com"}, + wantHosts: "cheryl.mason:\n oauth_token: abc123\n", + askStubs: func(as *prompt.AskStubber) { + as.StubOne("github.com") + as.StubOne(true) + }, + wantErrOut: regexp.MustCompile(`Logged out of github.com account 'cybilb'`), + }, + { + name: "no arguments, one host", + opts: &LogoutOptions{}, + cfgHosts: []string{"github.com"}, + askStubs: func(as *prompt.AskStubber) { + as.StubOne(true) + }, + wantErrOut: regexp.MustCompile(`Logged out of github.com account 'cybilb'`), + }, + { + name: "no arguments, no hosts", + opts: &LogoutOptions{}, + wantErr: regexp.MustCompile(`not logged in to any hosts`), + }, + { + name: "hostname", + opts: &LogoutOptions{ + Hostname: "cheryl.mason", + }, + cfgHosts: []string{"cheryl.mason", "github.com"}, + wantHosts: "github.com:\n oauth_token: abc123\n", + askStubs: func(as *prompt.AskStubber) { + as.StubOne(true) + }, + wantErrOut: regexp.MustCompile(`Logged out of cheryl.mason account 'cybilb'`), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, _, _, stderr := iostreams.Test() + + io.SetStdinTTY(true) + io.SetStdoutTTY(true) + + tt.opts.IO = io + cfg := config.NewBlankConfig() + tt.opts.Config = func() (config.Config, error) { + return cfg, nil + } + + for _, hostname := range tt.cfgHosts { + _ = cfg.Set(hostname, "oauth_token", "abc123") + } + + reg := &httpmock.Registry{} + reg.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"cybilb"}}}`)) + + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + + mainBuf := bytes.Buffer{} + hostsBuf := bytes.Buffer{} + defer config.StubWriteConfig(&mainBuf, &hostsBuf)() + + as, teardown := prompt.InitAskStubber() + defer teardown() + if tt.askStubs != nil { + tt.askStubs(as) + } + + err := logoutRun(tt.opts) + assert.Equal(t, tt.wantErr == nil, err == nil) + if err != nil { + if tt.wantErr != nil { + assert.True(t, tt.wantErr.MatchString(err.Error())) + return + } else { + t.Fatalf("unexpected error: %s", err) + } + } + + if tt.wantErrOut == nil { + assert.Equal(t, "", stderr.String()) + } else { + assert.True(t, tt.wantErrOut.MatchString(stderr.String())) + } + + assert.Equal(t, tt.wantHosts, hostsBuf.String()) + reg.Verify(t) + }) + } +} + +func Test_logoutRun_nontty(t *testing.T) { + tests := []struct { + name string + opts *LogoutOptions + cfgHosts []string + wantHosts string + wantErr *regexp.Regexp + }{ + { + name: "no arguments", + wantErr: regexp.MustCompile(`hostname required when not`), + opts: &LogoutOptions{}, + }, + { + name: "hostname, one host", + opts: &LogoutOptions{ + Hostname: "harry.mason", + }, + cfgHosts: []string{"harry.mason"}, + }, + { + name: "hostname, multiple hosts", + opts: &LogoutOptions{ + Hostname: "harry.mason", + }, + cfgHosts: []string{"harry.mason", "cheryl.mason"}, + wantHosts: "cheryl.mason:\n oauth_token: abc123\n", + }, + { + name: "hostname, no hosts", + opts: &LogoutOptions{ + Hostname: "harry.mason", + }, + wantErr: regexp.MustCompile(`not logged in to any hosts`), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, _, _, stderr := iostreams.Test() + + io.SetStdinTTY(false) + io.SetStdoutTTY(false) + + tt.opts.IO = io + cfg := config.NewBlankConfig() + tt.opts.Config = func() (config.Config, error) { + return cfg, nil + } + + for _, hostname := range tt.cfgHosts { + _ = cfg.Set(hostname, "oauth_token", "abc123") + } + + reg := &httpmock.Registry{} + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + + mainBuf := bytes.Buffer{} + hostsBuf := bytes.Buffer{} + defer config.StubWriteConfig(&mainBuf, &hostsBuf)() + + err := logoutRun(tt.opts) + assert.Equal(t, tt.wantErr == nil, err == nil) + if err != nil { + if tt.wantErr != nil { + assert.True(t, tt.wantErr.MatchString(err.Error())) + return + } else { + t.Fatalf("unexpected error: %s", err) + } + } + + assert.Equal(t, "", stderr.String()) + + assert.Equal(t, tt.wantHosts, hostsBuf.String()) + reg.Verify(t) + }) + } +}