Skip to content

Commit af850f4

Browse files
committed
feat(cli): add macOS support for session token keyring storage
Add support for storing the CLI session token in the OS keyring on macOS when the --use-keyring flag is provided.
1 parent 7ae3fdc commit af850f4

File tree

7 files changed

+236
-70
lines changed

7 files changed

+236
-70
lines changed

cli/keyring_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -282,9 +282,9 @@ func TestUseKeyringUnsupportedOS(t *testing.T) {
282282
// a helpful error message.
283283
t.Parallel()
284284

285-
// Skip on Windows since the keyring is actually supported.
286-
if runtime.GOOS == "windows" {
287-
t.Skip("Skipping unsupported OS test on Windows where keyring is supported")
285+
// Only run this on an unsupported OS.
286+
if runtime.GOOS == "windows" || runtime.GOOS == "darwin" {
287+
t.Skipf("Skipping unsupported OS test on %s where keyring is supported", runtime.GOOS)
288288
}
289289

290290
const expMessage = "keyring storage is not supported on this operating system; remove the --use-keyring flag"
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
//go:build darwin
2+
3+
package sessionstore
4+
5+
import (
6+
"encoding/base64"
7+
"fmt"
8+
"io"
9+
"os"
10+
"os/exec"
11+
"regexp"
12+
"strings"
13+
)
14+
15+
const (
16+
// defaultServiceName is the service name used in the macOS Keychain
17+
// for storing Coder CLI session tokens.
18+
defaultServiceName = "coder-v2-credentials"
19+
20+
// fixedUsername is the fixed username used for all keychain entries.
21+
// Since our interface only uses service names, we use a constant username.
22+
fixedUsername = "coder-session-token"
23+
24+
execPathKeychain = "/usr/bin/security"
25+
notFoundStr = "could not be found"
26+
)
27+
28+
// operatingSystemKeyring implements keyringProvider for macOS.
29+
// It is largely adapted from the zalando/go-keyring package.
30+
type operatingSystemKeyring struct{}
31+
32+
func (operatingSystemKeyring) Set(service, credential string) error {
33+
// if the added secret has multiple lines or some non ascii,
34+
// macOS will hex encode it on return. To avoid getting garbage, we
35+
// encode all passwords
36+
password := base64.StdEncoding.EncodeToString([]byte(credential))
37+
38+
cmd := exec.Command(execPathKeychain, "-i")
39+
stdIn, err := cmd.StdinPipe()
40+
if err != nil {
41+
return err
42+
}
43+
44+
if err = cmd.Start(); err != nil {
45+
return err
46+
}
47+
48+
command := fmt.Sprintf("add-generic-password -U -s %s -a %s -w %s\n",
49+
shellEscape(service),
50+
shellEscape(fixedUsername),
51+
shellEscape(password))
52+
if len(command) > 4096 {
53+
return ErrSetDataTooBig
54+
}
55+
56+
if _, err := io.WriteString(stdIn, command); err != nil {
57+
return err
58+
}
59+
60+
if err = stdIn.Close(); err != nil {
61+
return err
62+
}
63+
64+
return cmd.Wait()
65+
}
66+
67+
func (operatingSystemKeyring) Get(service string) ([]byte, error) {
68+
out, err := exec.Command(
69+
execPathKeychain,
70+
"find-generic-password",
71+
"-s", service,
72+
"-wa", fixedUsername).CombinedOutput()
73+
if err != nil {
74+
if strings.Contains(string(out), notFoundStr) {
75+
return nil, os.ErrNotExist
76+
}
77+
return nil, err
78+
}
79+
80+
trimStr := strings.TrimSpace(string(out))
81+
return base64.StdEncoding.DecodeString(trimStr)
82+
}
83+
84+
func (operatingSystemKeyring) Delete(service string) error {
85+
out, err := exec.Command(
86+
execPathKeychain,
87+
"delete-generic-password",
88+
"-s", service,
89+
"-a", fixedUsername).CombinedOutput()
90+
if strings.Contains(string(out), notFoundStr) {
91+
return os.ErrNotExist
92+
}
93+
return err
94+
}
95+
96+
// shellEscape returns a shell-escaped version of the string s.
97+
// This is adapted from github.com/zalando/go-keyring/internal/shellescape.
98+
func shellEscape(s string) string {
99+
if len(s) == 0 {
100+
return "''"
101+
}
102+
103+
pattern := regexp.MustCompile(`[^\w@%+=:,./-]`)
104+
if pattern.MatchString(s) {
105+
return "'" + strings.ReplaceAll(s, "'", "'\"'\"'") + "'"
106+
}
107+
108+
return s
109+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
//go:build darwin
2+
3+
package sessionstore_test
4+
5+
import (
6+
"encoding/base64"
7+
"os/exec"
8+
"testing"
9+
)
10+
11+
const (
12+
execPathKeychain = "/usr/bin/security"
13+
fixedUsername = "coder-session-token"
14+
)
15+
16+
func readRawKeychainCredential(t *testing.T, service string) []byte {
17+
t.Helper()
18+
19+
out, err := exec.Command(
20+
execPathKeychain,
21+
"find-generic-password",
22+
"-s", service,
23+
"-wa", fixedUsername).CombinedOutput()
24+
if err != nil {
25+
t.Fatal(err)
26+
}
27+
28+
dst := make([]byte, base64.StdEncoding.DecodedLen(len(out)))
29+
n, err := base64.StdEncoding.Decode(dst, out)
30+
if err != nil {
31+
t.Fatal(err)
32+
}
33+
return dst[:n]
34+
}

cli/sessionstore/sessionstore_other.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//go:build !windows
1+
//go:build !windows && !darwin
22

33
package sessionstore
44

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
//go:build !windows && !darwin
2+
3+
package sessionstore_test
4+
5+
import "testing"
6+
7+
func readRawKeychainCredential(t *testing.T, _ string) []byte {
8+
t.Fatal("not implemented")
9+
return nil
10+
}

cli/sessionstore/sessionstore_test.go

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package sessionstore_test
22

33
import (
4+
"encoding/json"
45
"errors"
56
"fmt"
67
"net/url"
@@ -16,6 +17,11 @@ import (
1617
"github.com/coder/coder/v2/cli/sessionstore"
1718
)
1819

20+
type storedCredentials map[string]struct {
21+
CoderURL string `json:"coder_url"`
22+
APIToken string `json:"api_token"`
23+
}
24+
1925
// Generate a test service name for use with the OS keyring. It uses a combination
2026
// of the test name and a nanosecond timestamp to prevent collisions.
2127
func keyringTestServiceName(t *testing.T) string {
@@ -26,8 +32,8 @@ func keyringTestServiceName(t *testing.T) string {
2632
func TestKeyring(t *testing.T) {
2733
t.Parallel()
2834

29-
if runtime.GOOS != "windows" {
30-
t.Skip("linux and darwin are not supported yet")
35+
if runtime.GOOS != "windows" && runtime.GOOS != "darwin" {
36+
t.Skip("linux is not supported yet")
3137
}
3238

3339
// This test exercises use of the operating system keyring. As a result,
@@ -199,6 +205,66 @@ func TestKeyring(t *testing.T) {
199205
err = backend.Delete(srvURL2)
200206
require.NoError(t, err)
201207
})
208+
209+
t.Run("StorageFormat", func(t *testing.T) {
210+
t.Parallel()
211+
// The storage format must remain consistent to ensure we don't break
212+
// compatibility with other Coder related applications that may read
213+
// or decode the same credential.
214+
215+
const testURL1 = "http://127.0.0.1:1337"
216+
srv1URL, err := url.Parse(testURL1)
217+
require.NoError(t, err)
218+
219+
const testURL2 = "http://127.0.0.1:1338"
220+
srv2URL, err := url.Parse(testURL2)
221+
require.NoError(t, err)
222+
223+
serviceName := keyringTestServiceName(t)
224+
backend := sessionstore.NewKeyringWithService(serviceName)
225+
t.Cleanup(func() {
226+
_ = backend.Delete(srv1URL)
227+
_ = backend.Delete(srv2URL)
228+
})
229+
230+
// Write token for server 1
231+
const token1 = "token-server-1"
232+
err = backend.Write(srv1URL, token1)
233+
require.NoError(t, err)
234+
235+
// Write token for server 2 (should NOT overwrite server 1's token)
236+
const token2 = "token-server-2"
237+
err = backend.Write(srv2URL, token2)
238+
require.NoError(t, err)
239+
240+
// Verify both credentials are stored in the raw format and can
241+
// be extracted through the Backend API.
242+
rawCredential := readRawKeychainCredential(t, serviceName)
243+
244+
storedCreds := make(storedCredentials)
245+
err = json.Unmarshal(rawCredential, &storedCreds)
246+
require.NoError(t, err, "unmarshalling stored credentials")
247+
248+
// Both credentials should exist
249+
require.Len(t, storedCreds, 2)
250+
require.Equal(t, token1, storedCreds[srv1URL.Host].APIToken)
251+
require.Equal(t, token2, storedCreds[srv2URL.Host].APIToken)
252+
253+
// Read individual credentials
254+
token, err := backend.Read(srv1URL)
255+
require.NoError(t, err)
256+
require.Equal(t, token1, token)
257+
258+
token, err = backend.Read(srv2URL)
259+
require.NoError(t, err)
260+
require.Equal(t, token2, token)
261+
262+
// Cleanup
263+
err = backend.Delete(srv1URL)
264+
require.NoError(t, err)
265+
err = backend.Delete(srv2URL)
266+
require.NoError(t, err)
267+
})
202268
}
203269

204270
func TestFile(t *testing.T) {

cli/sessionstore/sessionstore_windows_test.go

Lines changed: 11 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,16 @@ import (
1414
"github.com/coder/coder/v2/cli/sessionstore"
1515
)
1616

17+
func readRawKeychainCredential(t *testing.T, serviceName string) []byte {
18+
t.Helper()
19+
20+
winCred, err := wincred.GetGenericCredential(serviceName)
21+
if err != nil {
22+
t.Fatal(err)
23+
}
24+
return winCred.CredentialBlob
25+
}
26+
1727
func TestWindowsKeyring_WriteReadDelete(t *testing.T) {
1828
t.Parallel()
1929

@@ -38,10 +48,7 @@ func TestWindowsKeyring_WriteReadDelete(t *testing.T) {
3848
winCred, err := wincred.GetGenericCredential(serviceName)
3949
require.NoError(t, err, "getting windows credential")
4050

41-
var storedCreds map[string]struct {
42-
CoderURL string `json:"coder_url"`
43-
APIToken string `json:"api_token"`
44-
}
51+
storedCreds := make(storedCredentials)
4552
err = json.Unmarshal(winCred.CredentialBlob, &storedCreds)
4653
require.NoError(t, err, "unmarshalling stored credentials")
4754

@@ -65,63 +72,3 @@ func TestWindowsKeyring_WriteReadDelete(t *testing.T) {
6572
_, err = backend.Read(srvURL)
6673
require.ErrorIs(t, err, os.ErrNotExist)
6774
}
68-
69-
func TestWindowsKeyring_MultipleServers(t *testing.T) {
70-
t.Parallel()
71-
72-
const testURL1 = "http://127.0.0.1:1337"
73-
srv1URL, err := url.Parse(testURL1)
74-
require.NoError(t, err)
75-
76-
const testURL2 = "http://127.0.0.1:1338"
77-
srv2URL, err := url.Parse(testURL2)
78-
require.NoError(t, err)
79-
80-
serviceName := keyringTestServiceName(t)
81-
backend := sessionstore.NewKeyringWithService(serviceName)
82-
t.Cleanup(func() {
83-
_ = backend.Delete(srv1URL)
84-
_ = backend.Delete(srv2URL)
85-
})
86-
87-
// Write token for server 1
88-
const token1 = "token-server-1"
89-
err = backend.Write(srv1URL, token1)
90-
require.NoError(t, err)
91-
92-
// Write token for server 2 (should NOT overwrite server 1's token)
93-
const token2 = "token-server-2"
94-
err = backend.Write(srv2URL, token2)
95-
require.NoError(t, err)
96-
97-
// Verify both credentials are stored in Windows Credential Manager
98-
winCred, err := wincred.GetGenericCredential(serviceName)
99-
require.NoError(t, err, "getting windows credential")
100-
101-
var storedCreds map[string]struct {
102-
CoderURL string `json:"coder_url"`
103-
APIToken string `json:"api_token"`
104-
}
105-
err = json.Unmarshal(winCred.CredentialBlob, &storedCreds)
106-
require.NoError(t, err, "unmarshalling stored credentials")
107-
108-
// Both credentials should exist
109-
require.Len(t, storedCreds, 2)
110-
require.Equal(t, token1, storedCreds[srv1URL.Host].APIToken)
111-
require.Equal(t, token2, storedCreds[srv2URL.Host].APIToken)
112-
113-
// Read individual credentials
114-
token, err := backend.Read(srv1URL)
115-
require.NoError(t, err)
116-
require.Equal(t, token1, token)
117-
118-
token, err = backend.Read(srv2URL)
119-
require.NoError(t, err)
120-
require.Equal(t, token2, token)
121-
122-
// Cleanup
123-
err = backend.Delete(srv1URL)
124-
require.NoError(t, err)
125-
err = backend.Delete(srv2URL)
126-
require.NoError(t, err)
127-
}

0 commit comments

Comments
 (0)