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

Commita02a708

Browse files
committed
feat(cli): store session token in OS keyring with file fallback
This change implements secure storage of the CLI token using the operating system keyringwith a fallback to the previous plaintext file storage. Previously, the Coder CLI storedauthentication tokens in plaintext configuration files, which posed a security riskbecause users' tokens are stored unencrypted and can be easily accessed by otherprocesses or users with file system access.The secure storage is platform dependent. The security command is used on macOS.Windows Credential Manager API is used on Windows. Linux depends on GNOME keyring andSecret Service API (via D-Bus).
1 parentbe22c38 commita02a708

File tree

7 files changed

+286
-22
lines changed

7 files changed

+286
-22
lines changed

‎cli/login.go‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/coder/pretty"
2020

2121
"github.com/coder/coder/v2/cli/cliui"
22+
"github.com/coder/coder/v2/cli/sessionstore"
2223
"github.com/coder/coder/v2/coderd/userpassword"
2324
"github.com/coder/coder/v2/codersdk"
2425
"github.com/coder/serpent"
@@ -115,9 +116,12 @@ func (r *RootCmd) loginWithPassword(
115116

116117
sessionToken:=resp.SessionToken
117118
config:=r.createConfig()
118-
err=config.Session().Write(sessionToken)
119-
iferr!=nil {
120-
returnxerrors.Errorf("write session token: %w",err)
119+
location,werr:=sessionstore.Write(config,client.URL,sessionToken)
120+
ifwerr!=nil {
121+
returnxerrors.Errorf("write session token: %w",werr)
122+
}
123+
iflocation==sessionstore.LocationFile {
124+
cliui.Warn(inv.Stderr,"⚠️ Token stored in PLAIN TEXT because OS keyring access failed. This is less secure than keyring storage.")
121125
}
122126

123127
client.SetSessionToken(sessionToken)
@@ -149,8 +153,14 @@ func (r *RootCmd) login() *serpent.Command {
149153
useTokenForSessionbool
150154
)
151155
cmd:=&serpent.Command{
152-
Use:"login [<url>]",
153-
Short:"Authenticate with Coder deployment",
156+
Use:"login [<url>]",
157+
Short:"Authenticate with Coder deployment",
158+
Long:"Stores the session token in the OS keyring, with a fallback to plain text if the "+
159+
"keyring is not available. The security command is used on macOS. Windows Credential "+
160+
"Manager API is used on Windows. Linux depends on GNOME keyring and Secret Service API "+
161+
"(via D-Bus). Be aware that there are known issues with this Linux keyring approach in "+
162+
"headless environments. See https://keyring.readthedocs.io/en/latest/#using-keyring-on-headless-linux-systems "+
163+
" for a workaround.",
154164
Middleware:serpent.RequireRangeArgs(0,1),
155165
Handler:func(inv*serpent.Invocation)error {
156166
ctx:=inv.Context()
@@ -394,9 +404,12 @@ func (r *RootCmd) login() *serpent.Command {
394404
}
395405

396406
config:=r.createConfig()
397-
err=config.Session().Write(sessionToken)
398-
iferr!=nil {
399-
returnxerrors.Errorf("write session token: %w",err)
407+
location,werr:=sessionstore.Write(config,client.URL,sessionToken)
408+
ifwerr!=nil {
409+
returnxerrors.Errorf("write session token: %w",werr)
410+
}
411+
iflocation==sessionstore.LocationFile {
412+
cliui.Warn(inv.Stderr,"⚠️ Token stored in PLAIN TEXT because OS keyring access failed. This is less secure than keyring storage.")
400413
}
401414
err=config.URL().Write(serverURL.String())
402415
iferr!=nil {

‎cli/logout.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"golang.org/x/xerrors"
99

1010
"github.com/coder/coder/v2/cli/cliui"
11+
"github.com/coder/coder/v2/cli/sessionstore"
1112
"github.com/coder/serpent"
1213
)
1314

@@ -46,7 +47,8 @@ func (r *RootCmd) logout() *serpent.Command {
4647
errors=append(errors,xerrors.Errorf("remove URL file: %w",err))
4748
}
4849

49-
err=config.Session().Delete()
50+
// Remove from OS keyring and file (fallback)
51+
err=sessionstore.Delete(config,client.URL)
5052
// Only throw error if the session configuration file is present,
5153
// otherwise the user is already logged out, and we proceed
5254
iferr!=nil&&!os.IsNotExist(err) {

‎cli/root.go‎

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
"github.com/coder/coder/v2/cli/cliui"
3838
"github.com/coder/coder/v2/cli/config"
3939
"github.com/coder/coder/v2/cli/gitauth"
40+
"github.com/coder/coder/v2/cli/sessionstore"
4041
"github.com/coder/coder/v2/cli/telemetry"
4142
"github.com/coder/coder/v2/codersdk"
4243
"github.com/coder/coder/v2/codersdk/agentsdk"
@@ -549,13 +550,19 @@ func (r *RootCmd) InitClient(inv *serpent.Invocation) (*codersdk.Client, error)
549550
returnnil,err
550551
}
551552
}
552-
// Read the token storedon disk.
553+
// Read the token storedsecurely (platform-specific) with file fallback.
553554
ifr.token=="" {
554-
r.token,err=conf.Session().Read()
555-
// Even if there isn't a token, we don't care.
556-
// Some API routes can be unauthenticated.
557-
iferr!=nil&&!os.IsNotExist(err) {
558-
returnnil,err
555+
tok,location,rerr:=sessionstore.Read(conf,r.clientURL)
556+
ifrerr!=nil&&!os.IsNotExist(rerr) {
557+
returnnil,rerr
558+
}
559+
iftok!="" {
560+
r.token=tok
561+
}
562+
// If we fell back to file, warn once.
563+
iflocation==sessionstore.LocationFile {
564+
//_, _ = fmt.Fprintln(inv.Stderr, pretty.Sprint(cliui.DefaultStyles.Warn,
565+
//"Token stored in plaintext config because OS keyring access failed. This is less secure than keychain storage."))
559566
}
560567
}
561568

@@ -588,7 +595,6 @@ func (r *RootCmd) InitClient(inv *serpent.Invocation) (*codersdk.Client, error)
588595
// This allows commands to run without requiring authentication, but still use auth if available.
589596
func (r*RootCmd)TryInitClient(inv*serpent.Invocation) (*codersdk.Client,error) {
590597
conf:=r.createConfig()
591-
varerrerror
592598
// Read the client URL stored on disk.
593599
ifr.clientURL==nil||r.clientURL.String()=="" {
594600
rawURL,err:=conf.URL().Read()
@@ -605,13 +611,19 @@ func (r *RootCmd) TryInitClient(inv *serpent.Invocation) (*codersdk.Client, erro
605611
}
606612
}
607613
}
608-
// Read the token storedon disk.
614+
// Read the token storedsecurely (platform-specific) with file fallback.
609615
ifr.token=="" {
610-
r.token,err=conf.Session().Read()
611-
// Even if there isn't a token, we don't care.
612-
// Some API routes can be unauthenticated.
613-
iferr!=nil&&!os.IsNotExist(err) {
614-
returnnil,err
616+
tok,location,rerr:=sessionstore.Read(conf,r.clientURL)
617+
ifrerr!=nil&&!os.IsNotExist(rerr) {
618+
returnnil,rerr
619+
}
620+
iftok!="" {
621+
r.token=tok
622+
}
623+
// If we fell back to file, warn once.
624+
iflocation==sessionstore.LocationFile {
625+
//_, _ = fmt.Fprintln(inv.Stderr, pretty.Sprint(cliui.DefaultStyles.Warn,
626+
//"Token stored in plaintext config because OS keyring access failed. This is less secure than keychain storage."))
615627
}
616628
}
617629

‎cli/sessionstore/sessionstore.go‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
package sessionstore
2+
3+
import (
4+
"errors"
5+
"net/url"
6+
"os"
7+
"runtime"
8+
"strings"
9+
10+
"github.com/zalando/go-keyring"
11+
12+
"github.com/coder/coder/v2/cli/config"
13+
)
14+
15+
const (
16+
servicePrefix="coder-cli:"
17+
keyringAccount="session"
18+
)
19+
20+
// Location represents where a session token is stored.
21+
typeLocationstring
22+
23+
const (
24+
LocationNoneLocation="none"
25+
LocationFileLocation="file"
26+
LocationKeyringLocation="keyring"
27+
)
28+
29+
funcserviceName(u*url.URL)string {
30+
ifu==nil||u.Host=="" {
31+
returnservicePrefix+"default"
32+
}
33+
host:=strings.TrimSpace(strings.ToLower(u.Host))
34+
returnservicePrefix+host
35+
}
36+
37+
// Read returns the session token and where it is stored for the given server URL
38+
// or an error, if any. It prefers the operating system keyring and falls back to
39+
// file storage if the system keyring is unavailable.
40+
funcRead(conf config.Root,serverURL*url.URL) (string,Location,error) {
41+
svc:=serviceName(serverURL)
42+
tok,err:=keyring.Get(svc,keyringAccount)
43+
iferr==nil&&tok!="" {
44+
returntok,LocationKeyring,nil
45+
}
46+
// Fallback to file storage.
47+
tok,ferr:=conf.Session().Read()
48+
ifferr==nil {
49+
returntok,LocationFile,nil
50+
}
51+
// If the file doesn't exist, preserve the not-exist error semantics.
52+
ifos.IsNotExist(ferr) {
53+
return"",LocationNone,ferr
54+
}
55+
// Some other file read error.
56+
return"",LocationNone,ferr
57+
}
58+
59+
// Write stores the session token for the given server URL. It returns where the
60+
// token was stored or an error, if any. It prefers the operating system keyring
61+
// and falls back to file storage if the keyring operation fails.
62+
funcWrite(conf config.Root,serverURL*url.URL,tokenstring) (Location,error) {
63+
svc:=serviceName(serverURL)
64+
err:=keyring.Set(svc,keyringAccount,token)
65+
iferr==nil {
66+
// Best effort: remove plaintext file if it exists.
67+
_=conf.Session().Delete()
68+
returnLocationKeyring,nil
69+
}
70+
iferr:=conf.Session().Write(token);err!=nil {
71+
returnLocationNone,err
72+
}
73+
returnLocationFile,nil
74+
}
75+
76+
// Delete removes any stored session token from both the keyring and file.
77+
// It ignores not-found conditions on either backend.
78+
funcDelete(conf config.Root,serverURL*url.URL)error {
79+
svc:=serviceName(serverURL)
80+
varerrs []error
81+
82+
iferr:=keyring.Delete(svc,keyringAccount);err!=nil {
83+
ifruntime.GOOS=="linux" {
84+
// It's possible the Linux keyring dependencies are not installed (e.g. go-keyring
85+
// on Linux requires D-Bus and GNOME Keyring). In this case, when keyring.Delete
86+
// fails we can't reliably differentiate between the dependencies not being
87+
// installed (customer doesn't want to opt into using the OS keyring) or a genuine
88+
// problem deleting the token. As a result, we unfortunately silently swallow the
89+
// error here to prevent bubbling up a misleading error.
90+
//
91+
// This could be addressed by alternative approaches such as requiring use of the
92+
// OS keyring unless a flag/env var specifies exclusive use of a file on disk.
93+
// This approach would require Linux users to have the dependencies installed when
94+
// updating CLI versions.
95+
}elseif!errors.Is(err,keyring.ErrNotFound) {
96+
errs=append(errs,err)
97+
}
98+
}
99+
iferr:=conf.Session().Delete();err!=nil&&!os.IsNotExist(err) {
100+
errs=append(errs,err)
101+
}
102+
returnerrors.Join(errs...)
103+
}
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
package sessionstore
2+
3+
import (
4+
"errors"
5+
"net/http"
6+
"net/http/httptest"
7+
"net/url"
8+
"os"
9+
"runtime"
10+
"testing"
11+
12+
"github.com/coder/coder/v2/cli/config"
13+
"github.com/zalando/go-keyring"
14+
)
15+
16+
funcTestStoreKeyring(t*testing.T) {
17+
// This test exercises use of the operating system keyring. As a result,
18+
// the operating system keyring is expected to be available.
19+
dir:=t.TempDir()
20+
cfg:=config.Root(dir)
21+
srv:=httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter,r*http.Request) {}))
22+
defersrv.Close()
23+
url,err:=url.Parse(srv.URL)
24+
iferr!=nil {
25+
t.Fatal(err)
26+
}
27+
28+
_,_,err=Read(cfg,url)
29+
if!os.IsNotExist(err) {
30+
t.Fatal("expected error when no token in keyring or on disk")
31+
}
32+
33+
ifruntime.GOOS=="linux" {
34+
// Linux needs a dbus daemon and gnome-keyring to be installed. To avoid
35+
// adding these dependencies to CI, we only test against built-in keychains
36+
// for Windows/macOS and rely on testing done in the go-keyring package
37+
// for Linux coverage.
38+
return
39+
}
40+
41+
// Write a token and read it back.
42+
constinputToken="abc1234"
43+
src,err:=Write(cfg,url,inputToken)
44+
iferr!=nil {
45+
t.Fatal(err)
46+
}
47+
wantSrc:=LocationKeyring
48+
ifsrc!=wantSrc {
49+
t.Fatalf("got source %v want %v",src,wantSrc)
50+
}
51+
52+
token,src,err:=Read(cfg,url)
53+
iferr!=nil {
54+
t.Fatalf("unexpected error reading token: %v",err)
55+
}
56+
ifsrc!=wantSrc {
57+
t.Fatalf("got source %v want %v",src,wantSrc)
58+
}
59+
iftoken!=inputToken {
60+
t.Fatalf("got token %v want %v",token,inputToken)
61+
}
62+
63+
// Delete the token and attempt to read it back.
64+
err=Delete(cfg,url)
65+
iferr!=nil {
66+
t.Fatal(err)
67+
}
68+
_,_,err=Read(cfg,url)
69+
if!os.IsNotExist(err) {
70+
t.Fatal("expected error when no token in keyring or on disk")
71+
}
72+
}
73+
74+
funcTestStoreFileFallback(t*testing.T) {
75+
errKeyring:=errors.New("this keyring never works")
76+
keyring.MockInitWithError(errKeyring)
77+
78+
dir:=t.TempDir()
79+
cfg:=config.Root(dir)
80+
srv:=httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter,r*http.Request) {}))
81+
defersrv.Close()
82+
url,err:=url.Parse(srv.URL)
83+
iferr!=nil {
84+
t.Fatal(err)
85+
}
86+
87+
_,_,err=Read(cfg,url)
88+
if!os.IsNotExist(err) {
89+
t.Fatal("expected error when no token in keyring or on disk")
90+
}
91+
92+
// Write a token and read it back.
93+
constinputToken="abc1234"
94+
src,err:=Write(cfg,url,inputToken)
95+
iferr!=nil {
96+
t.Fatal(err)
97+
}
98+
wantSrc:=LocationFile
99+
ifsrc!=wantSrc {
100+
t.Fatalf("got source %v want %v",src,wantSrc)
101+
}
102+
103+
token,src,err:=Read(cfg,url)
104+
iferr!=nil {
105+
t.Fatalf("unexpected error reading token: %v",err)
106+
}
107+
ifsrc!=wantSrc {
108+
t.Fatalf("got source %v want %v",src,wantSrc)
109+
}
110+
iftoken!=inputToken {
111+
t.Fatalf("got token %v want %v",token,inputToken)
112+
}
113+
114+
// Delete the token and attempt to read it back. This is expected to return
115+
// the error from reading the keyring since Delete will attempt deleting the
116+
// token from the keyring and a file on disk.
117+
err=Delete(cfg,url)
118+
iferr!=nil {
119+
t.Fatal(err)
120+
}
121+
_,_,err=Read(cfg,url)
122+
if!os.IsNotExist(err) {
123+
t.Fatal("expected error when no token in keyring or on disk")
124+
}
125+
}

‎go.mod‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,8 +467,10 @@ require github.com/coder/clistat v1.0.2
467467
requiregithub.com/SherClockHolmes/webpush-gov1.4.0
468468

469469
require (
470+
github.com/alessio/shellescapev1.4.1// indirect
470471
github.com/charmbracelet/colorprofilev0.2.3-0.20250311203215-f60798e515dc// indirect
471472
github.com/charmbracelet/x/cellbufv0.0.13// indirect
473+
github.com/danieljoos/wincredv1.2.0// indirect
472474
github.com/go-json-experiment/jsonv0.0.0-20250725192818-e39067aee2d2// indirect
473475
github.com/golang-jwt/jwt/v5v5.2.3// indirect
474476
github.com/xo/terminfov0.0.0-20220910002029-abceb7e1c41e// indirect
@@ -547,6 +549,7 @@ require (
547549
github.com/vektah/gqlparser/v2v2.5.28// indirect
548550
github.com/wk8/go-ordered-map/v2v2.1.8// indirect
549551
github.com/yosida95/uritemplate/v3v3.0.2// indirect
552+
github.com/zalando/go-keyringv0.2.4
550553
github.com/zeebo/xxh3v1.0.2// indirect
551554
go.opentelemetry.io/contrib/detectors/gcpv1.37.0// indirect
552555
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpcv0.62.0// indirect

0 commit comments

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