Skip to content

Commit 7ba6449

Browse files
AbhineetJainAbhineetJain
authored
Improve CLI logout flow (coder#1692)
* Improve CLI logout flow * Fix lint error * Make notLoggedInMessage a const * successful logout with a msg when cfg files are absent * use require, os.remove, show only one message, add prompt
1 parent 33e2e40 commit 7ba6449

File tree

4 files changed

+185
-26
lines changed

4 files changed

+185
-26
lines changed

‎cli/logout.go

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,64 @@ import (
66

77
"github.com/spf13/cobra"
88
"golang.org/x/xerrors"
9+
10+
"github.com/coder/coder/cli/cliui"
911
)
1012

1113
func logout() *cobra.Command {
12-
return &cobra.Command{
14+
cmd := &cobra.Command{
1315
Use: "logout",
14-
Short: "Remove local autheticated session",
16+
Short: "Remove the local authenticated session",
1517
RunE: func(cmd *cobra.Command, args []string) error {
18+
var isLoggedOut bool
19+
1620
config := createConfig(cmd)
17-
err := os.RemoveAll(string(config))
21+
22+
_, err := cliui.Prompt(cmd, cliui.PromptOptions{
23+
Text: "Are you sure you want to logout?",
24+
IsConfirm: true,
25+
Default: "yes",
26+
})
1827
if err != nil {
19-
return xerrors.Errorf("remove files at %s: %w", config, err)
28+
return err
2029
}
2130

22-
_, _ = fmt.Fprintf(cmd.OutOrStdout(), caret+"Successfully logged out.\n")
31+
err = config.URL().Delete()
32+
if err != nil {
33+
// Only throw error if the URL configuration file is present,
34+
// otherwise the user is already logged out, and we proceed
35+
if !os.IsNotExist(err) {
36+
return xerrors.Errorf("remove URL file: %w", err)
37+
}
38+
isLoggedOut = true
39+
}
40+
41+
err = config.Session().Delete()
42+
if err != nil {
43+
// Only throw error if the session configuration file is present,
44+
// otherwise the user is already logged out, and we proceed
45+
if !os.IsNotExist(err) {
46+
return xerrors.Errorf("remove session file: %w", err)
47+
}
48+
isLoggedOut = true
49+
}
50+
51+
err = config.Organization().Delete()
52+
// If the organization configuration file is absent, we still proceed
53+
if err != nil && !os.IsNotExist(err) {
54+
return xerrors.Errorf("remove organization file: %w", err)
55+
}
56+
57+
// If the user was already logged out, we show them a different message
58+
if isLoggedOut {
59+
_, _ = fmt.Fprintf(cmd.OutOrStdout(), notLoggedInMessage+"\n")
60+
} else {
61+
_, _ = fmt.Fprintf(cmd.OutOrStdout(), caret+"Successfully logged out.\n")
62+
}
2363
return nil
2464
},
2565
}
66+
67+
cliui.AllowSkipPrompt(cmd)
68+
return cmd
2669
}

‎cli/logout_test.go

Lines changed: 126 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,148 @@
11
package cli_test
22

33
import (
4+
"os"
45
"testing"
56

67
"github.com/stretchr/testify/assert"
78
"github.com/stretchr/testify/require"
89

910
"github.com/coder/coder/cli/clitest"
11+
"github.com/coder/coder/cli/config"
1012
"github.com/coder/coder/coderd/coderdtest"
1113
"github.com/coder/coder/pty/ptytest"
1214
)
1315

1416
func TestLogout(t *testing.T) {
1517
t.Parallel()
18+
t.Run("Logout", func(t *testing.T) {
19+
t.Parallel()
20+
21+
pty := ptytest.New(t)
22+
config := login(t, pty)
23+
24+
// ensure session files exist
25+
require.FileExists(t, string(config.URL()))
26+
require.FileExists(t, string(config.Session()))
27+
28+
logoutChan := make(chan struct{})
29+
logout, _ := clitest.New(t, "logout", "--global-config", string(config))
30+
logout.SetIn(pty.Input())
31+
logout.SetOut(pty.Output())
32+
33+
go func() {
34+
defer close(logoutChan)
35+
err := logout.Execute()
36+
assert.NoError(t, err)
37+
assert.NoFileExists(t, string(config.URL()))
38+
assert.NoFileExists(t, string(config.Session()))
39+
}()
40+
41+
pty.ExpectMatch("Are you sure you want to logout?")
42+
pty.WriteLine("yes")
43+
pty.ExpectMatch("Successfully logged out")
44+
<-logoutChan
45+
})
46+
t.Run("SkipPrompt", func(t *testing.T) {
47+
t.Parallel()
48+
49+
pty := ptytest.New(t)
50+
config := login(t, pty)
51+
52+
// ensure session files exist
53+
require.FileExists(t, string(config.URL()))
54+
require.FileExists(t, string(config.Session()))
55+
56+
logoutChan := make(chan struct{})
57+
logout, _ := clitest.New(t, "logout", "--global-config", string(config), "-y")
58+
logout.SetIn(pty.Input())
59+
logout.SetOut(pty.Output())
60+
61+
go func() {
62+
defer close(logoutChan)
63+
err := logout.Execute()
64+
assert.NoError(t, err)
65+
assert.NoFileExists(t, string(config.URL()))
66+
assert.NoFileExists(t, string(config.Session()))
67+
}()
68+
69+
pty.ExpectMatch("Successfully logged out")
70+
<-logoutChan
71+
})
72+
t.Run("NoURLFile", func(t *testing.T) {
73+
t.Parallel()
74+
75+
pty := ptytest.New(t)
76+
config := login(t, pty)
77+
78+
// ensure session files exist
79+
require.FileExists(t, string(config.URL()))
80+
require.FileExists(t, string(config.Session()))
81+
82+
err := os.Remove(string(config.URL()))
83+
require.NoError(t, err)
84+
85+
logoutChan := make(chan struct{})
86+
logout, _ := clitest.New(t, "logout", "--global-config", string(config))
87+
88+
logout.SetIn(pty.Input())
89+
logout.SetOut(pty.Output())
90+
91+
go func() {
92+
defer close(logoutChan)
93+
err := logout.Execute()
94+
assert.NoError(t, err)
95+
assert.NoFileExists(t, string(config.URL()))
96+
assert.NoFileExists(t, string(config.Session()))
97+
}()
98+
99+
pty.ExpectMatch("Are you sure you want to logout?")
100+
pty.WriteLine("yes")
101+
pty.ExpectMatch("You are not logged in. Try logging in using 'coder login <url>'.")
102+
<-logoutChan
103+
})
104+
t.Run("NoSessionFile", func(t *testing.T) {
105+
t.Parallel()
106+
107+
pty := ptytest.New(t)
108+
config := login(t, pty)
109+
110+
// ensure session files exist
111+
require.FileExists(t, string(config.URL()))
112+
require.FileExists(t, string(config.Session()))
113+
114+
err := os.Remove(string(config.Session()))
115+
require.NoError(t, err)
116+
117+
logoutChan := make(chan struct{})
118+
logout, _ := clitest.New(t, "logout", "--global-config", string(config))
119+
120+
logout.SetIn(pty.Input())
121+
logout.SetOut(pty.Output())
122+
123+
go func() {
124+
defer close(logoutChan)
125+
err = logout.Execute()
126+
assert.NoError(t, err)
127+
assert.NoFileExists(t, string(config.URL()))
128+
assert.NoFileExists(t, string(config.Session()))
129+
}()
130+
131+
pty.ExpectMatch("Are you sure you want to logout?")
132+
pty.WriteLine("yes")
133+
pty.ExpectMatch("You are not logged in. Try logging in using 'coder login <url>'.")
134+
<-logoutChan
135+
})
136+
}
137+
138+
func login(t *testing.T, pty *ptytest.PTY) config.Root {
139+
t.Helper()
16140

17-
// login
18141
client := coderdtest.New(t, nil)
19142
coderdtest.CreateFirstUser(t, client)
20143

21144
doneChan := make(chan struct{})
22-
root, config := clitest.New(t, "login", "--force-tty", client.URL.String(), "--no-open")
23-
pty := ptytest.New(t)
145+
root, cfg := clitest.New(t, "login", "--force-tty", client.URL.String(), "--no-open")
24146
root.SetIn(pty.Input())
25147
root.SetOut(pty.Output())
26148
go func() {
@@ -34,13 +156,5 @@ func TestLogout(t *testing.T) {
34156
pty.ExpectMatch("Welcome to Coder")
35157
<-doneChan
36158

37-
// ensure session files exist
38-
require.FileExists(t, string(config.URL()))
39-
require.FileExists(t, string(config.Session()))
40-
41-
logout, _ := clitest.New(t, "logout", "--global-config", string(config))
42-
err := logout.Execute()
43-
require.NoError(t, err)
44-
require.NoFileExists(t, string(config.URL()))
45-
require.NoFileExists(t, string(config.Session()))
159+
return cfg
46160
}

‎cli/root.go

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,14 @@ var (
3131
)
3232

3333
const (
34-
varURL = "url"
35-
varToken = "token"
36-
varAgentToken = "agent-token"
37-
varAgentURL = "agent-url"
38-
varGlobalConfig = "global-config"
39-
varNoOpen = "no-open"
40-
varForceTty = "force-tty"
34+
varURL = "url"
35+
varToken = "token"
36+
varAgentToken = "agent-token"
37+
varAgentURL = "agent-url"
38+
varGlobalConfig = "global-config"
39+
varNoOpen = "no-open"
40+
varForceTty = "force-tty"
41+
notLoggedInMessage = "You are not logged in. Try logging in using 'coder login <url>'."
4142
)
4243

4344
func init() {
@@ -117,7 +118,7 @@ func createClient(cmd *cobra.Command) (*codersdk.Client, error) {
117118
if err != nil {
118119
// If the configuration files are absent, the user is logged out
119120
if os.IsNotExist(err) {
120-
return nil, xerrors.New("You are not logged in. Try logging in using 'coder login <url>'.")
121+
return nil, xerrors.New(notLoggedInMessage)
121122
}
122123
return nil, err
123124
}
@@ -132,7 +133,7 @@ func createClient(cmd *cobra.Command) (*codersdk.Client, error) {
132133
if err != nil {
133134
// If the configuration files are absent, the user is logged out
134135
if os.IsNotExist(err) {
135-
return nil, xerrors.New("You are not logged in. Try logging in using 'coder login <url>'.")
136+
return nil, xerrors.New(notLoggedInMessage)
136137
}
137138
return nil, err
138139
}

‎cli/userlist_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
)
1515

1616
func TestUserList(t *testing.T) {
17+
t.Parallel()
1718
t.Run("List", func(t *testing.T) {
1819
t.Parallel()
1920
client := coderdtest.New(t, nil)

0 commit comments

Comments
 (0)