Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd623eeb

Browse files
authored
feat: delete API token in /logout API (#1770)
* delete API token in logout api* add deleteapikeybyid to databasefake* set blank cookie on logout always* refactor logout flow, add unit tests* update logout messsage* use read-only file mode for windows* fix file mode on windows for cleanup* change file permissions on windows* assert error is not nil* refactor cli* try different file mode on windows* try different file mode on windows* try keeping the files open on Windows* fix the error message on Windows
1 parentd0ed107 commitd623eeb

File tree

10 files changed

+239
-65
lines changed

10 files changed

+239
-65
lines changed

‎cli/logout.go

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cli
33
import (
44
"fmt"
55
"os"
6+
"strings"
67

78
"github.com/spf13/cobra"
89
"golang.org/x/xerrors"
@@ -15,11 +16,16 @@ func logout() *cobra.Command {
1516
Use:"logout",
1617
Short:"Remove the local authenticated session",
1718
RunE:func(cmd*cobra.Command,args []string)error {
18-
varisLoggedOutbool
19+
client,err:=createClient(cmd)
20+
iferr!=nil {
21+
returnerr
22+
}
23+
24+
varerrors []error
1925

2026
config:=createConfig(cmd)
2127

22-
_,err:=cliui.Prompt(cmd, cliui.PromptOptions{
28+
_,err=cliui.Prompt(cmd, cliui.PromptOptions{
2329
Text:"Are you sure you want to logout?",
2430
IsConfirm:true,
2531
Default:"yes",
@@ -28,38 +34,40 @@ func logout() *cobra.Command {
2834
returnerr
2935
}
3036

31-
err=config.URL().Delete()
37+
err=client.Logout(cmd.Context())
3238
iferr!=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-
returnxerrors.Errorf("remove URL file: %w",err)
37-
}
38-
isLoggedOut=true
39+
errors=append(errors,xerrors.Errorf("logout api: %w",err))
40+
}
41+
42+
err=config.URL().Delete()
43+
// Only throw error if the URL configuration file is present,
44+
// otherwise the user is already logged out, and we proceed
45+
iferr!=nil&&!os.IsNotExist(err) {
46+
errors=append(errors,xerrors.Errorf("remove URL file: %w",err))
3947
}
4048

4149
err=config.Session().Delete()
42-
iferr!=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-
returnxerrors.Errorf("remove session file: %w",err)
47-
}
48-
isLoggedOut=true
50+
// Only throw error if the session configuration file is present,
51+
// otherwise the user is already logged out, and we proceed
52+
iferr!=nil&&!os.IsNotExist(err) {
53+
errors=append(errors,xerrors.Errorf("remove session file: %w",err))
4954
}
5055

5156
err=config.Organization().Delete()
5257
// If the organization configuration file is absent, we still proceed
5358
iferr!=nil&&!os.IsNotExist(err) {
54-
returnxerrors.Errorf("remove organization file: %w",err)
59+
errors=append(errors,xerrors.Errorf("remove organization file: %w",err))
5560
}
5661

57-
// If the user was already logged out, we show them a different message
58-
ifisLoggedOut {
59-
_,_=fmt.Fprintf(cmd.OutOrStdout(),notLoggedInMessage+"\n")
60-
}else {
61-
_,_=fmt.Fprintf(cmd.OutOrStdout(),caret+"Successfully logged out.\n")
62+
iflen(errors)>0 {
63+
varerrorStringBuilder strings.Builder
64+
for_,err:=rangeerrors {
65+
_,_=fmt.Fprint(&errorStringBuilder,"\t"+err.Error()+"\n")
66+
}
67+
errorString:=strings.TrimRight(errorStringBuilder.String(),"\n")
68+
returnxerrors.New("Failed to log out.\n"+errorString)
6269
}
70+
_,_=fmt.Fprintf(cmd.OutOrStdout(),caret+"You are no longer logged in. You can log in using 'coder login <url>'.\n")
6371
returnnil
6472
},
6573
}

‎cli/logout_test.go

Lines changed: 73 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
package cli_test
22

33
import (
4+
"fmt"
45
"os"
6+
"regexp"
7+
"runtime"
58
"testing"
69

710
"github.com/stretchr/testify/assert"
@@ -21,7 +24,7 @@ func TestLogout(t *testing.T) {
2124
pty:=ptytest.New(t)
2225
config:=login(t,pty)
2326

24-
//ensure session files exist
27+
//Ensure session files exist.
2528
require.FileExists(t,string(config.URL()))
2629
require.FileExists(t,string(config.Session()))
2730

@@ -40,7 +43,7 @@ func TestLogout(t *testing.T) {
4043

4144
pty.ExpectMatch("Are you sure you want to logout?")
4245
pty.WriteLine("yes")
43-
pty.ExpectMatch("Successfullyloggedout")
46+
pty.ExpectMatch("You are no longerloggedin. You can log in using 'coder login <url>'.")
4447
<-logoutChan
4548
})
4649
t.Run("SkipPrompt",func(t*testing.T) {
@@ -49,7 +52,7 @@ func TestLogout(t *testing.T) {
4952
pty:=ptytest.New(t)
5053
config:=login(t,pty)
5154

52-
//ensure session files exist
55+
//Ensure session files exist.
5356
require.FileExists(t,string(config.URL()))
5457
require.FileExists(t,string(config.Session()))
5558

@@ -66,7 +69,7 @@ func TestLogout(t *testing.T) {
6669
assert.NoFileExists(t,string(config.Session()))
6770
}()
6871

69-
pty.ExpectMatch("Successfullyloggedout")
72+
pty.ExpectMatch("You are no longerloggedin. You can log in using 'coder login <url>'.")
7073
<-logoutChan
7174
})
7275
t.Run("NoURLFile",func(t*testing.T) {
@@ -75,7 +78,7 @@ func TestLogout(t *testing.T) {
7578
pty:=ptytest.New(t)
7679
config:=login(t,pty)
7780

78-
//ensure session files exist
81+
//Ensure session files exist.
7982
require.FileExists(t,string(config.URL()))
8083
require.FileExists(t,string(config.Session()))
8184

@@ -91,14 +94,9 @@ func TestLogout(t *testing.T) {
9194
gofunc() {
9295
deferclose(logoutChan)
9396
err:=logout.Execute()
94-
assert.NoError(t,err)
95-
assert.NoFileExists(t,string(config.URL()))
96-
assert.NoFileExists(t,string(config.Session()))
97+
assert.EqualError(t,err,"You are not logged in. Try logging in using 'coder login <url>'.")
9798
}()
9899

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>'.")
102100
<-logoutChan
103101
})
104102
t.Run("NoSessionFile",func(t*testing.T) {
@@ -107,7 +105,7 @@ func TestLogout(t *testing.T) {
107105
pty:=ptytest.New(t)
108106
config:=login(t,pty)
109107

110-
//ensure session files exist
108+
//Ensure session files exist.
111109
require.FileExists(t,string(config.URL()))
112110
require.FileExists(t,string(config.Session()))
113111

@@ -123,14 +121,73 @@ func TestLogout(t *testing.T) {
123121
gofunc() {
124122
deferclose(logoutChan)
125123
err=logout.Execute()
126-
assert.NoError(t,err)
127-
assert.NoFileExists(t,string(config.URL()))
128-
assert.NoFileExists(t,string(config.Session()))
124+
assert.EqualError(t,err,"You are not logged in. Try logging in using 'coder login <url>'.")
125+
}()
126+
127+
<-logoutChan
128+
})
129+
t.Run("CannotDeleteFiles",func(t*testing.T) {
130+
t.Parallel()
131+
132+
pty:=ptytest.New(t)
133+
config:=login(t,pty)
134+
135+
// Ensure session files exist.
136+
require.FileExists(t,string(config.URL()))
137+
require.FileExists(t,string(config.Session()))
138+
139+
var (
140+
errerror
141+
urlFile*os.File
142+
sessionFile*os.File
143+
)
144+
ifruntime.GOOS=="windows" {
145+
// Opening the files so Windows does not allow deleting them.
146+
urlFile,err=os.Open(string(config.URL()))
147+
require.NoError(t,err)
148+
sessionFile,err=os.Open(string(config.Session()))
149+
require.NoError(t,err)
150+
}else {
151+
// Changing the permissions to throw error during deletion.
152+
err=os.Chmod(string(config),0500)
153+
require.NoError(t,err)
154+
}
155+
t.Cleanup(func() {
156+
ifruntime.GOOS=="windows" {
157+
// Closing the opened files for cleanup.
158+
err=urlFile.Close()
159+
require.NoError(t,err)
160+
err=sessionFile.Close()
161+
require.NoError(t,err)
162+
}else {
163+
// Setting the permissions back for cleanup.
164+
err=os.Chmod(string(config),0700)
165+
require.NoError(t,err)
166+
}
167+
})
168+
169+
logoutChan:=make(chanstruct{})
170+
logout,_:=clitest.New(t,"logout","--global-config",string(config))
171+
172+
logout.SetIn(pty.Input())
173+
logout.SetOut(pty.Output())
174+
175+
gofunc() {
176+
deferclose(logoutChan)
177+
err:=logout.Execute()
178+
assert.NotNil(t,err)
179+
varerrorMessagestring
180+
ifruntime.GOOS=="windows" {
181+
errorMessage="The process cannot access the file because it is being used by another process."
182+
}else {
183+
errorMessage="permission denied"
184+
}
185+
errRegex:=regexp.MustCompile(fmt.Sprintf("Failed to log out.\n\tremove URL file: .+: %s\n\tremove session file: .+: %s",errorMessage,errorMessage))
186+
assert.Regexp(t,errRegex,err.Error())
129187
}()
130188

131189
pty.ExpectMatch("Are you sure you want to logout?")
132190
pty.WriteLine("yes")
133-
pty.ExpectMatch("You are not logged in. Try logging in using 'coder login <url>'.")
134191
<-logoutChan
135192
})
136193
}

‎coderd/coderd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,6 @@ func New(options *Options) *API {
219219
r.Get("/first",api.firstUser)
220220
r.Post("/first",api.postFirstUser)
221221
r.Post("/login",api.postLogin)
222-
r.Post("/logout",api.postLogout)
223222
r.Get("/authmethods",api.userAuthMethods)
224223
r.Route("/oauth2",func(r chi.Router) {
225224
r.Route("/github",func(r chi.Router) {
@@ -234,6 +233,7 @@ func New(options *Options) *API {
234233
)
235234
r.Post("/",api.postUser)
236235
r.Get("/",api.users)
236+
r.Post("/logout",api.postLogout)
237237
// These routes query information about site wide roles.
238238
r.Route("/roles",func(r chi.Router) {
239239
r.Get("/",api.assignableSiteRoles)

‎coderd/coderd_test.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
104104
workspaceRBACObj:=rbac.ResourceWorkspace.InOrg(organization.ID).WithID(workspace.ID.String()).WithOwner(workspace.OwnerID.String())
105105

106106
// skipRoutes allows skipping routes from being checked.
107+
skipRoutes:=map[string]string{
108+
"POST:/api/v2/users/logout":"Logging out deletes the API Key for other routes",
109+
}
110+
107111
typerouteCheckstruct {
108112
NoAuthorizebool
109113
AssertAction rbac.Action
@@ -117,7 +121,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
117121
"GET:/api/v2/users/first": {NoAuthorize:true},
118122
"POST:/api/v2/users/first": {NoAuthorize:true},
119123
"POST:/api/v2/users/login": {NoAuthorize:true},
120-
"POST:/api/v2/users/logout": {NoAuthorize:true},
121124
"GET:/api/v2/users/authmethods": {NoAuthorize:true},
122125
"POST:/api/v2/csp/reports": {NoAuthorize:true},
123126

@@ -310,8 +313,20 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
310313
assertRoute[noTrailSlash]=v
311314
}
312315

316+
fork,v:=rangeskipRoutes {
317+
noTrailSlash:=strings.TrimRight(k,"/")
318+
if_,ok:=skipRoutes[noTrailSlash];ok&&noTrailSlash!=k {
319+
t.Errorf("route %q & %q is declared twice",noTrailSlash,k)
320+
t.FailNow()
321+
}
322+
skipRoutes[noTrailSlash]=v
323+
}
324+
313325
err=chi.Walk(api.Handler,func(methodstring,routestring,handler http.Handler,middlewares...func(http.Handler) http.Handler)error {
314326
name:=method+":"+route
327+
if_,ok:=skipRoutes[strings.TrimRight(name,"/")];ok {
328+
returnnil
329+
}
315330
t.Run(name,func(t*testing.T) {
316331
authorizer.reset()
317332
routeAssertions,ok:=assertRoute[strings.TrimRight(name,"/")]

‎coderd/database/databasefake/databasefake.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,21 @@ func (q *fakeQuerier) GetAPIKeyByID(_ context.Context, id string) (database.APIK
126126
return database.APIKey{},sql.ErrNoRows
127127
}
128128

129+
func (q*fakeQuerier)DeleteAPIKeyByID(_ context.Context,idstring)error {
130+
q.mutex.Lock()
131+
deferq.mutex.Unlock()
132+
133+
forindex,apiKey:=rangeq.apiKeys {
134+
ifapiKey.ID!=id {
135+
continue
136+
}
137+
q.apiKeys[index]=q.apiKeys[len(q.apiKeys)-1]
138+
q.apiKeys=q.apiKeys[:len(q.apiKeys)-1]
139+
returnnil
140+
}
141+
returnsql.ErrNoRows
142+
}
143+
129144
func (q*fakeQuerier)GetFileByHash(_ context.Context,hashstring) (database.File,error) {
130145
q.mutex.RLock()
131146
deferq.mutex.RUnlock()

‎coderd/database/querier.go

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 13 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/queries/apikeys.sql

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,10 @@ SET
3838
oauth_expiry= $6
3939
WHERE
4040
id= $1;
41+
42+
-- name: DeleteAPIKeyByID :exec
43+
DELETE
44+
FROM
45+
api_keys
46+
WHERE
47+
id= $1;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp