feat(cli): add macOS support for session token keyring storage (#20613)

Add support for storing the CLI session token in the OS keyring on macOS
when the --use-keyring flag is provided.

#19403

https://www.notion.so/coderhq/CLI-Session-Token-in-OS-Keyring-293d579be592808b8b7fd235304e50d5

Author: zedkipp

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"

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 {

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
+}

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]
+}

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 {

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
+}

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) {

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{}

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)
-}