diff --git a/cli/keyring_test.go b/cli/keyring_test.go index 1a4e90895a6eb..646f4ae19a034 100644 --- a/cli/keyring_test.go +++ b/cli/keyring_test.go @@ -282,9 +282,9 @@ func TestUseKeyringUnsupportedOS(t *testing.T) { // a helpful error message. t.Parallel() - // Skip on Windows since the keyring is actually supported. - if runtime.GOOS == "windows" { - t.Skip("Skipping unsupported OS test on Windows where keyring is supported") + // Only run this on an unsupported OS. + if runtime.GOOS == "windows" || runtime.GOOS == "darwin" { + t.Skipf("Skipping unsupported OS test on %s where keyring is supported", runtime.GOOS) } const expMessage = "keyring storage is not supported on this operating system; remove the --use-keyring flag" diff --git a/cli/sessionstore/sessionstore.go b/cli/sessionstore/sessionstore.go index 8ae14c2786c2f..029e86ad7e99b 100644 --- a/cli/sessionstore/sessionstore.go +++ b/cli/sessionstore/sessionstore.go @@ -46,6 +46,12 @@ var ( ErrNotImplemented = xerrors.New("not implemented") ) +const ( + // defaultServiceName is the service name used in keyrings for storing Coder CLI session + // tokens. + defaultServiceName = "coder-v2-credentials" +) + // keyringProvider represents an operating system keyring. The expectation // is these methods operate on the user/login keyring. type keyringProvider interface { diff --git a/cli/sessionstore/sessionstore_darwin.go b/cli/sessionstore/sessionstore_darwin.go new file mode 100644 index 0000000000000..be398d42e7049 --- /dev/null +++ b/cli/sessionstore/sessionstore_darwin.go @@ -0,0 +1,105 @@ +//go:build darwin + +package sessionstore + +import ( + "encoding/base64" + "fmt" + "io" + "os" + "os/exec" + "regexp" + "strings" +) + +const ( + // fixedUsername is the fixed username used for all keychain entries. + // Since our interface only uses service names, we use a constant username. + fixedUsername = "coder-login-credentials" + + execPathKeychain = "/usr/bin/security" + notFoundStr = "could not be found" +) + +// operatingSystemKeyring implements keyringProvider for macOS. +// It is largely adapted from the zalando/go-keyring package. +type operatingSystemKeyring struct{} + +func (operatingSystemKeyring) Set(service, credential string) error { + // if the added secret has multiple lines or some non ascii, + // macOS will hex encode it on return. To avoid getting garbage, we + // encode all passwords + password := base64.StdEncoding.EncodeToString([]byte(credential)) + + cmd := exec.Command(execPathKeychain, "-i") + stdIn, err := cmd.StdinPipe() + if err != nil { + return err + } + + if err = cmd.Start(); err != nil { + return err + } + + command := fmt.Sprintf("add-generic-password -U -s %s -a %s -w %s\n", + shellEscape(service), + shellEscape(fixedUsername), + shellEscape(password)) + if len(command) > 4096 { + return ErrSetDataTooBig + } + + if _, err := io.WriteString(stdIn, command); err != nil { + return err + } + + if err = stdIn.Close(); err != nil { + return err + } + + return cmd.Wait() +} + +func (operatingSystemKeyring) Get(service string) ([]byte, error) { + out, err := exec.Command( + execPathKeychain, + "find-generic-password", + "-s", service, + "-wa", fixedUsername).CombinedOutput() + if err != nil { + if strings.Contains(string(out), notFoundStr) { + return nil, os.ErrNotExist + } + return nil, err + } + + trimStr := strings.TrimSpace(string(out)) + return base64.StdEncoding.DecodeString(trimStr) +} + +func (operatingSystemKeyring) Delete(service string) error { + out, err := exec.Command( + execPathKeychain, + "delete-generic-password", + "-s", service, + "-a", fixedUsername).CombinedOutput() + if strings.Contains(string(out), notFoundStr) { + return os.ErrNotExist + } + return err +} + +// shellEscape returns a shell-escaped version of the string s. +// This is adapted from github.com/zalando/go-keyring/internal/shellescape. +func shellEscape(s string) string { + if len(s) == 0 { + return "''" + } + + pattern := regexp.MustCompile(`[^\w@%+=:,./-]`) + if pattern.MatchString(s) { + return "'" + strings.ReplaceAll(s, "'", "'\"'\"'") + "'" + } + + return s +} diff --git a/cli/sessionstore/sessionstore_darwin_test.go b/cli/sessionstore/sessionstore_darwin_test.go new file mode 100644 index 0000000000000..a90ee12d96cc1 --- /dev/null +++ b/cli/sessionstore/sessionstore_darwin_test.go @@ -0,0 +1,34 @@ +//go:build darwin + +package sessionstore_test + +import ( + "encoding/base64" + "os/exec" + "testing" +) + +const ( + execPathKeychain = "/usr/bin/security" + fixedUsername = "coder-login-credentials" +) + +func readRawKeychainCredential(t *testing.T, service string) []byte { + t.Helper() + + out, err := exec.Command( + execPathKeychain, + "find-generic-password", + "-s", service, + "-wa", fixedUsername).CombinedOutput() + if err != nil { + t.Fatal(err) + } + + dst := make([]byte, base64.StdEncoding.DecodedLen(len(out))) + n, err := base64.StdEncoding.Decode(dst, out) + if err != nil { + t.Fatal(err) + } + return dst[:n] +} diff --git a/cli/sessionstore/sessionstore_other.go b/cli/sessionstore/sessionstore_other.go index d930790135aca..a71458a360c94 100644 --- a/cli/sessionstore/sessionstore_other.go +++ b/cli/sessionstore/sessionstore_other.go @@ -1,9 +1,7 @@ -//go:build !windows +//go:build !windows && !darwin package sessionstore -const defaultServiceName = "not-implemented" - type operatingSystemKeyring struct{} func (operatingSystemKeyring) Set(_, _ string) error { diff --git a/cli/sessionstore/sessionstore_other_test.go b/cli/sessionstore/sessionstore_other_test.go new file mode 100644 index 0000000000000..b924a95d12897 --- /dev/null +++ b/cli/sessionstore/sessionstore_other_test.go @@ -0,0 +1,10 @@ +//go:build !windows && !darwin + +package sessionstore_test + +import "testing" + +func readRawKeychainCredential(t *testing.T, _ string) []byte { + t.Fatal("not implemented") + return nil +} diff --git a/cli/sessionstore/sessionstore_test.go b/cli/sessionstore/sessionstore_test.go index c01a1256eb085..1ecb0279918fd 100644 --- a/cli/sessionstore/sessionstore_test.go +++ b/cli/sessionstore/sessionstore_test.go @@ -1,6 +1,7 @@ package sessionstore_test import ( + "encoding/json" "errors" "fmt" "net/url" @@ -16,6 +17,11 @@ import ( "github.com/coder/coder/v2/cli/sessionstore" ) +type storedCredentials map[string]struct { + CoderURL string `json:"coder_url"` + APIToken string `json:"api_token"` +} + // Generate a test service name for use with the OS keyring. It uses a combination // of the test name and a nanosecond timestamp to prevent collisions. func keyringTestServiceName(t *testing.T) string { @@ -26,8 +32,8 @@ func keyringTestServiceName(t *testing.T) string { func TestKeyring(t *testing.T) { t.Parallel() - if runtime.GOOS != "windows" { - t.Skip("linux and darwin are not supported yet") + if runtime.GOOS != "windows" && runtime.GOOS != "darwin" { + t.Skip("linux is not supported yet") } // This test exercises use of the operating system keyring. As a result, @@ -199,6 +205,66 @@ func TestKeyring(t *testing.T) { err = backend.Delete(srvURL2) require.NoError(t, err) }) + + t.Run("StorageFormat", func(t *testing.T) { + t.Parallel() + // The storage format must remain consistent to ensure we don't break + // compatibility with other Coder related applications that may read + // or decode the same credential. + + const testURL1 = "http://127.0.0.1:1337" + srv1URL, err := url.Parse(testURL1) + require.NoError(t, err) + + const testURL2 = "http://127.0.0.1:1338" + srv2URL, err := url.Parse(testURL2) + require.NoError(t, err) + + serviceName := keyringTestServiceName(t) + backend := sessionstore.NewKeyringWithService(serviceName) + t.Cleanup(func() { + _ = backend.Delete(srv1URL) + _ = backend.Delete(srv2URL) + }) + + // Write token for server 1 + const token1 = "token-server-1" + err = backend.Write(srv1URL, token1) + require.NoError(t, err) + + // Write token for server 2 (should NOT overwrite server 1's token) + const token2 = "token-server-2" + err = backend.Write(srv2URL, token2) + require.NoError(t, err) + + // Verify both credentials are stored in the raw format and can + // be extracted through the Backend API. + rawCredential := readRawKeychainCredential(t, serviceName) + + storedCreds := make(storedCredentials) + err = json.Unmarshal(rawCredential, &storedCreds) + require.NoError(t, err, "unmarshalling stored credentials") + + // Both credentials should exist + require.Len(t, storedCreds, 2) + require.Equal(t, token1, storedCreds[srv1URL.Host].APIToken) + require.Equal(t, token2, storedCreds[srv2URL.Host].APIToken) + + // Read individual credentials + token, err := backend.Read(srv1URL) + require.NoError(t, err) + require.Equal(t, token1, token) + + token, err = backend.Read(srv2URL) + require.NoError(t, err) + require.Equal(t, token2, token) + + // Cleanup + err = backend.Delete(srv1URL) + require.NoError(t, err) + err = backend.Delete(srv2URL) + require.NoError(t, err) + }) } func TestFile(t *testing.T) { diff --git a/cli/sessionstore/sessionstore_windows.go b/cli/sessionstore/sessionstore_windows.go index 6c48db0154f3f..3dd38c19da31d 100644 --- a/cli/sessionstore/sessionstore_windows.go +++ b/cli/sessionstore/sessionstore_windows.go @@ -10,12 +10,6 @@ import ( "github.com/danieljoos/wincred" ) -const ( - // defaultServiceName is the service name used in the Windows Credential Manager - // for storing Coder CLI session tokens. - defaultServiceName = "coder-v2-credentials" -) - // operatingSystemKeyring implements keyringProvider and uses Windows Credential Manager. // It is largely adapted from the zalando/go-keyring package. type operatingSystemKeyring struct{} diff --git a/cli/sessionstore/sessionstore_windows_test.go b/cli/sessionstore/sessionstore_windows_test.go index 4d90e27f5c132..ef643d3033dba 100644 --- a/cli/sessionstore/sessionstore_windows_test.go +++ b/cli/sessionstore/sessionstore_windows_test.go @@ -14,6 +14,16 @@ import ( "github.com/coder/coder/v2/cli/sessionstore" ) +func readRawKeychainCredential(t *testing.T, serviceName string) []byte { + t.Helper() + + winCred, err := wincred.GetGenericCredential(serviceName) + if err != nil { + t.Fatal(err) + } + return winCred.CredentialBlob +} + func TestWindowsKeyring_WriteReadDelete(t *testing.T) { t.Parallel() @@ -38,10 +48,7 @@ func TestWindowsKeyring_WriteReadDelete(t *testing.T) { winCred, err := wincred.GetGenericCredential(serviceName) require.NoError(t, err, "getting windows credential") - var storedCreds map[string]struct { - CoderURL string `json:"coder_url"` - APIToken string `json:"api_token"` - } + storedCreds := make(storedCredentials) err = json.Unmarshal(winCred.CredentialBlob, &storedCreds) require.NoError(t, err, "unmarshalling stored credentials") @@ -65,63 +72,3 @@ func TestWindowsKeyring_WriteReadDelete(t *testing.T) { _, err = backend.Read(srvURL) require.ErrorIs(t, err, os.ErrNotExist) } - -func TestWindowsKeyring_MultipleServers(t *testing.T) { - t.Parallel() - - const testURL1 = "http://127.0.0.1:1337" - srv1URL, err := url.Parse(testURL1) - require.NoError(t, err) - - const testURL2 = "http://127.0.0.1:1338" - srv2URL, err := url.Parse(testURL2) - require.NoError(t, err) - - serviceName := keyringTestServiceName(t) - backend := sessionstore.NewKeyringWithService(serviceName) - t.Cleanup(func() { - _ = backend.Delete(srv1URL) - _ = backend.Delete(srv2URL) - }) - - // Write token for server 1 - const token1 = "token-server-1" - err = backend.Write(srv1URL, token1) - require.NoError(t, err) - - // Write token for server 2 (should NOT overwrite server 1's token) - const token2 = "token-server-2" - err = backend.Write(srv2URL, token2) - require.NoError(t, err) - - // Verify both credentials are stored in Windows Credential Manager - winCred, err := wincred.GetGenericCredential(serviceName) - require.NoError(t, err, "getting windows credential") - - var storedCreds map[string]struct { - CoderURL string `json:"coder_url"` - APIToken string `json:"api_token"` - } - err = json.Unmarshal(winCred.CredentialBlob, &storedCreds) - require.NoError(t, err, "unmarshalling stored credentials") - - // Both credentials should exist - require.Len(t, storedCreds, 2) - require.Equal(t, token1, storedCreds[srv1URL.Host].APIToken) - require.Equal(t, token2, storedCreds[srv2URL.Host].APIToken) - - // Read individual credentials - token, err := backend.Read(srv1URL) - require.NoError(t, err) - require.Equal(t, token1, token) - - token, err = backend.Read(srv2URL) - require.NoError(t, err) - require.Equal(t, token2, token) - - // Cleanup - err = backend.Delete(srv1URL) - require.NoError(t, err) - err = backend.Delete(srv2URL) - require.NoError(t, err) -}