From 0f804f7cfdf5894de6c5896b8af3d42b354710aa Mon Sep 17 00:00:00 2001
From: Marcos Navarro <m@marcosn.com>
Date: Wed, 25 Sep 2024 11:42:07 -0600
Subject: [PATCH 1/5] Credentials package wrap up. Fallback to file
 crendentials persisting finished

---
 internal/credentials/credentials.go      | 320 ++---------------------
 internal/credentials/credentials_test.go | 300 ++++++---------------
 internal/credentials/errors.go           |  41 ---
 internal/credentials/file.go             |   5 +-
 internal/credentials/file_test.go        |  21 ++
 internal/credentials/keyring.go          | 175 +++++++++++--
 internal/credentials/keyring_test.go     | 142 ++++++++++
 7 files changed, 415 insertions(+), 589 deletions(-)
 create mode 100644 internal/credentials/keyring_test.go

diff --git a/internal/credentials/credentials.go b/internal/credentials/credentials.go
index 71a965ccb..8dd9ddada 100644
--- a/internal/credentials/credentials.go
+++ b/internal/credentials/credentials.go
@@ -1,5 +1,7 @@
 // Package credentials provides a secure storage and management system for user credentials.
 // The service uses the "go-keyring" library to interact with the system's native keyring service.
+// For systems that do not support a native keyring service,
+// an alternative using a file at ~/.connect-credentials to persist credentials is implemented.
 //
 // This package is not thread safe! Manipulation of credentials from multiple threads can result in data loss.
 // A distributed write lock is required to ensure threads do not overwrite the credential store.
@@ -19,7 +21,9 @@
 // Key components include:
 // - Credential: The main structure representing a single credential.
 // - CredentialRecord: A structure for storing credential data along with its version for future compatibility.
-// - CredentialsService: A service that provides methods for managing credentials.
+// - CredentialsService (interface): A service that provides methods for managing credentials.
+//   - keyringCredentialsService: The service using the system's native keyring.
+//   - fileCredentialsService: Fallback service for persising credentials in file when keyring is not available.
 //
 // Author: Posit Software, PBC
 // Copyright (C) 2024 by Posit Software, PBC.
@@ -27,23 +31,15 @@ package credentials
 
 import (
 	"encoding/json"
-	"fmt"
-	"net"
-	"net/url"
 
-	"github.com/google/uuid"
 	"github.com/posit-dev/publisher/internal/logging"
-	"github.com/posit-dev/publisher/internal/util"
-	"github.com/spf13/afero"
-	"github.com/zalando/go-keyring"
+	"github.com/posit-dev/publisher/internal/types"
 )
 
 const ServiceName = "Posit Publisher Safe Storage"
 
 const CurrentVersion = 0
 
-const EnvVarGUID = "00000000-0000-0000-0000-000000000000"
-
 type Credential struct {
 	GUID   string `json:"guid"`
 	Name   string `json:"name"`
@@ -77,314 +73,34 @@ func (cr *CredentialRecord) ToCredential() (*Credential, error) {
 	case 0:
 		var cred CredentialV0
 		if err := json.Unmarshal(cr.Data, &cred); err != nil {
-			return nil, &CorruptedError{GUID: cr.GUID}
+			return nil, NewCorruptedError(cr.GUID)
 		}
 		return &cred, nil
 	default:
-		return nil, &VersionError{Version: cr.Version}
+		return nil, NewVersionError(cr.Version)
 	}
 }
 
 type CredentialsService interface {
-	FileCredentialRecordFactory() (*CredentialRecord, error)
 	Delete(guid string) error
 	Get(guid string) (*Credential, error)
 	List() ([]Credential, error)
 	Set(name string, url string, ak string) (*Credential, error)
 }
 
-type credentialsService struct {
-	afs afero.Fs
-	log logging.Logger
-}
-
-func NewCredentialsService(log logging.Logger) *credentialsService {
-	return &credentialsService{log: log}
-}
-
-// FileCredentialRecordFactory creates a Credential based on the presence of the
-// a file containing CONNECT_SERVER and CONNECT_API_KEY.
-
-type deprcfileCredential struct {
-	URL string `toml:"url"`
-	Key string `toml:"key"`
-}
-
-func (cs *credentialsService) FileCredentialRecordFactory() (*CredentialRecord, error) {
-	homeDir, err := util.UserHomeDir(cs.afs)
-	if err != nil {
-		return nil, err
-	}
-	filePath := homeDir.Join(".connect-credentials")
-	exists, err := filePath.Exists()
-	if err != nil {
-		return nil, err
-	}
-	if !exists {
-		return nil, nil
-	}
-	var credential deprcfileCredential
-	err = util.ReadTOMLFile(filePath, &credential)
-	if err != nil {
-		return nil, err
-	}
-
-	if credential.URL != "" && credential.Key != "" {
-		normalizedUrl, err := util.NormalizeServerURL(credential.URL)
-		if err != nil {
-			return nil, fmt.Errorf("error normalizing environment server URL: %s %v", credential.URL, err)
-		}
-
-		name := normalizedUrl
-		u, err := url.Parse(normalizedUrl)
-		if err == nil {
-			// if we can, we'll use the host for the name
-			// u.Host possibly includes a port, which we don't want
-			host, _, err := net.SplitHostPort(u.Host)
-			if err != nil {
-				name = u.Host
-			} else {
-				name = host
-			}
-		}
-
-		cred := Credential{
-			GUID:   EnvVarGUID, // We'll use a well known GUID to indicate it is from the ENV vars
-			Name:   name,
-			URL:    normalizedUrl,
-			ApiKey: credential.Key,
-		}
-
-		raw, err := json.Marshal(cred)
-		if err != nil {
-			return nil, fmt.Errorf("error marshalling environment credential: %v", err)
-		}
-
-		record := CredentialRecord{
-			GUID:    EnvVarGUID,
-			Version: CurrentVersion,
-			Data:    json.RawMessage(raw),
-		}
-
-		return &record, nil
-	}
-	// None found but not an error
-	return nil, nil
-}
-
-func (cs *credentialsService) checkForConflicts(
-	table *map[string]CredentialRecord,
-	c *Credential) error {
-	// Check if Credential attributes (URL or name) are already used by another credential
-	for guid, record := range *table {
-		cred, err := record.ToCredential()
-		if err != nil {
-			return &CorruptedError{GUID: guid}
-		}
-		if cred.URL == c.URL {
-			if cred.GUID == EnvVarGUID || c.GUID == EnvVarGUID {
-				return &EnvURLCollisionError{
-					Name: c.Name,
-					URL:  c.URL,
-				}
-			}
-			return &URLCollisionError{
-				Name: c.Name,
-				URL:  c.URL,
-			}
-		}
-		if cred.Name == c.Name {
-			if cred.GUID == EnvVarGUID || c.GUID == EnvVarGUID {
-				return &EnvNameCollisionError{
-					Name: c.Name,
-					URL:  c.URL,
-				}
-			}
-			return &NameCollisionError{
-				Name: c.Name,
-				URL:  c.URL,
-			}
-		}
-	}
-	return nil
-}
-
-// Delete removes a Credential by its guid.
-// If lookup by guid fails, a NotFoundError is returned.
-func (cs *credentialsService) Delete(guid string) error {
-
-	table, err := cs.load()
-	if err != nil {
-		return err
-	}
-
-	_, exists := table[guid]
-	if !exists {
-		cs.log.Debug("Credential does not exist", "credential", guid)
-		return &NotFoundError{GUID: guid}
-	}
-
-	// protect against deleting our environment variable credentials
-	if guid == EnvVarGUID {
-		return &EnvURLDeleteError{}
-	}
-
-	delete(table, guid)
-	return cs.save(table)
-}
-
-// Get retrieves a Credential by its guid.
-func (cs *credentialsService) Get(guid string) (*Credential, error) {
-	table, err := cs.load()
-	if err != nil {
-		return nil, err
-	}
-
-	cr, exists := table[guid]
-	if !exists {
-		cs.log.Debug("Credential does not exist", "credential", guid)
-		return nil, &NotFoundError{GUID: guid}
-	}
-
-	return cr.ToCredential()
-}
-
-// List retrieves all Credentials
-func (cs *credentialsService) List() ([]Credential, error) {
-	records, err := cs.load()
-	if err != nil {
-		return nil, err
-	}
-	var creds []Credential = make([]Credential, 0)
-	for _, cr := range records {
-		cred, err := cr.ToCredential()
-		if err != nil {
-			return nil, err
-		}
-		creds = append(creds, *cred)
-	}
-	return creds, nil
-}
-
-// Set creates a Credential.
-// A guid is assigned to the Credential using the UUIDv4 specification.
-func (cs *credentialsService) Set(name string, url string, ak string) (*Credential, error) {
-	table, err := cs.load()
-	if err != nil {
-		return nil, err
-	}
-
-	normalizedUrl, err := util.NormalizeServerURL(url)
-	if err != nil {
-		return nil, err
-	}
-
-	guid := uuid.New().String()
-	cred := Credential{
-		GUID:   guid,
-		Name:   name,
-		URL:    normalizedUrl,
-		ApiKey: ak,
-	}
-
-	err = cs.checkForConflicts(&table, &cred)
-	if err != nil {
-		return nil, err
-	}
-
-	raw, err := json.Marshal(cred)
-	if err != nil {
-		return nil, fmt.Errorf("error marshalling credential: %v", err)
-	}
-
-	table[guid] = CredentialRecord{
-		GUID:    guid,
-		Version: CurrentVersion,
-		Data:    json.RawMessage(raw),
-	}
-
-	err = cs.save(table)
-	if err != nil {
-		return nil, err
-	}
-
-	return &cred, nil
-}
-
-// Saves the CredentialTable, but removes Env Credentials first
-func (cs *credentialsService) save(table CredentialTable) error {
-
-	// remove any environment variable credential from the table
-	// before saving
-	_, found := table[EnvVarGUID]
-	if found {
-		delete(table, EnvVarGUID)
-	}
-
-	return cs.saveToKeyRing(table)
-}
-
-// Saves the CredentialTable to keyring
-func (cs *credentialsService) saveToKeyRing(table CredentialTable) error {
-	data, err := json.Marshal(table)
-	if err != nil {
-		return fmt.Errorf("failed to serialize credentials: %v", err)
-	}
-
-	ks := KeyringService{}
-	err = ks.Set(ServiceName, "credentials", string(data))
-	if err != nil {
-		return fmt.Errorf("failed to set credentials: %v", err)
-	}
-	return nil
-}
-
-// Loads the CredentialTable with keyring and env values
-func (cs *credentialsService) load() (CredentialTable, error) {
-	table, err := cs.loadFromKeyRing()
-	if err != nil {
-		cs.log.Debug("Failed to load credentials from system keyring", "error", err.Error())
-		return nil, err
-	}
-
-	// insert a possible file-based credential before returning
-	record, err := cs.FileCredentialRecordFactory()
-	if err != nil {
-		cs.log.Debug("Failed to create file based credentials records instance", "error", err.Error())
-		return nil, err
-	}
-	if record != nil {
-		c, err := record.ToCredential()
-		if err != nil {
-			cs.log.Debug("Error building credentials from file record", "error", err.Error())
-			return nil, err
-		}
-		err = cs.checkForConflicts(&table, c)
-		if err != nil {
-			cs.log.Debug("Conflict on credential record", "error", err.Error())
-			return nil, err
-		}
-		table[EnvVarGUID] = *record
-	}
-	return table, nil
-}
-
-// Loads the CredentialTable from keyRing
-func (cs *credentialsService) loadFromKeyRing() (CredentialTable, error) {
-	ks := KeyringService{}
-	data, err := ks.Get(ServiceName, "credentials")
-	if err != nil {
-		if err == keyring.ErrNotFound {
-			return make(map[string]CredentialRecord), nil
-		}
-		return nil, &LoadError{Err: err}
+// The main credentials service constructor that determines if the system's keyring is available to be used,
+// if not, returns a file based credentials service.
+func NewCredentialsService(log logging.Logger) (CredentialsService, error) {
+	krService := NewKeyringCredentialsService(log)
+	if krService.IsSupported() {
+		return krService, nil
 	}
 
-	var table CredentialTable
-	err = json.Unmarshal([]byte(data), &table)
+	log.Debug("Fallback to file managed credentials service due to unavailable system keyring")
+	fcService, err := NewFileCredentialsService(log)
 	if err != nil {
-		return nil, fmt.Errorf("failed to deserialize credentials: %v", err)
+		return nil, types.NewAgentError(types.ErrorCredentialServiceUnavailable, err, nil)
 	}
 
-	return table, nil
+	return fcService, nil
 }
diff --git a/internal/credentials/credentials_test.go b/internal/credentials/credentials_test.go
index 086541a0f..6e35e68f9 100644
--- a/internal/credentials/credentials_test.go
+++ b/internal/credentials/credentials_test.go
@@ -3,270 +3,124 @@
 package credentials
 
 import (
+	"errors"
 	"testing"
 
-	"github.com/pelletier/go-toml/v2"
 	"github.com/posit-dev/publisher/internal/logging/loggingtest"
-	"github.com/posit-dev/publisher/internal/util"
 	"github.com/posit-dev/publisher/internal/util/utiltest"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/suite"
 	"github.com/zalando/go-keyring"
 )
 
-type Cred struct {
-	GUID   string
-	Server string
-	ApiKey string
-	Name   string
-}
-
-var fileCred = Credential{
-	GUID:   "00000000-0000-0000-0000-000000000000",
-	URL:    "https://connect.localtest.me/rsc/dev-password-copy",
-	ApiKey: "3456789",
-	Name:   "connect.localtest.me",
-}
-
-type CredentialsTestSuite struct {
+type CredentialsServiceTestSuite struct {
 	utiltest.Suite
 	log *loggingtest.MockLogger
 }
 
-func TestCredentialsTestSuite(t *testing.T) {
-	suite.Run(t, new(CredentialsTestSuite))
+func TestCredentialsServiceTestSuite(t *testing.T) {
+	suite.Run(t, new(CredentialsServiceTestSuite))
 }
 
-func (s *CredentialsTestSuite) SetupTest() {
+func (s *CredentialsServiceTestSuite) SetupTest() {
 	s.log = loggingtest.NewMockLogger()
 }
 
-func (s *CredentialsTestSuite) credentialsFilePath(afs afero.Fs) (util.AbsolutePath, error) {
-	homeDir, err := util.UserHomeDir(afs)
-	if err != nil {
-		return util.AbsolutePath{}, err
+func (s *CredentialsServiceTestSuite) TestCredential() {
+	cred := Credential{
+		GUID:   "18cd5640-bee5-4b2a-992a-a2725ab6103d",
+		Name:   "friedtofu",
+		URL:    "https://a1.connect-server:3939/connect",
+		ApiKey: "abcdeC2aqbh7dg8TO43XPu7r56YDh000",
 	}
-	return homeDir.Join(".connect-credentials"), nil
-}
-
-func (s *CredentialsTestSuite) createFileCredentials(cs *credentialsService, errorCheck bool) {
-	path, err := s.credentialsFilePath(cs.afs)
-	s.NoError(err)
-
-	f, err := path.Create()
-	s.NoError(err)
-	defer f.Close()
-
-	enc := toml.NewEncoder(f)
-	cred := deprcfileCredential{
-		URL: fileCred.URL,
-		Key: fileCred.ApiKey,
-	}
-	err = enc.Encode(cred)
-	s.NoError(err)
-
-	if errorCheck {
-		res, err := cs.Get(fileCred.GUID)
-		s.NoError(err)
-		expected := Credential{
-			GUID:   fileCred.GUID,
-			Name:   fileCred.Name,
-			URL:    fileCred.URL,
-			ApiKey: fileCred.ApiKey,
-		}
-		s.Equal(res, &expected)
-	}
-}
 
-func (s *CredentialsTestSuite) clearFileCredentials(cs *credentialsService) {
-	path, err := s.credentialsFilePath(cs.afs)
-	s.NoError(err)
-	_ = path.Remove()
+	err := cred.ConflictCheck(Credential{
+		Name:   "no friedtofu",
+		URL:    "https://nota1.connect-server:3939/connect",
+		ApiKey: "abcdeC2aqbh7dg8TO43XPu7r56YDh000",
+	})
+	s.NoError(err)
+
+	err = cred.ConflictCheck(Credential{
+		Name:   "friedtofu",
+		URL:    "https://nota1.connect-server:3939/connect",
+		ApiKey: "abcdeC2aqbh7dg8TO43XPu7r56YDh000",
+	})
+	s.EqualError(err, "Name value conflicts with existing credential (friedtofu) URL: https://a1.connect-server:3939/connect")
+
+	err = cred.ConflictCheck(Credential{
+		Name:   "no friedtofu",
+		URL:    "https://a1.connect-server:3939/connect",
+		ApiKey: "abcdeC2aqbh7dg8TO43XPu7r56YDh000",
+	})
+	s.EqualError(err, "URL value conflicts with existing credential (friedtofu) URL: https://a1.connect-server:3939/connect")
 }
 
-func (s *CredentialsTestSuite) TestSet() {
-	keyring.MockInit()
-	cs := credentialsService{
-		afs: afero.NewMemMapFs(),
-		log: s.log,
+func (s *CredentialsServiceTestSuite) TestCredentialRecord() {
+	record := CredentialRecord{
+		GUID:    "18cd5640-bee5-4b2a-992a-a2725ab6103d",
+		Version: 0,
+		Data: []byte(`
+		{"guid":"18cd5640-bee5-4b2a-992a-a2725ab6103d","name":"friedtofu",
+		"url": "https://a1.connect-server:3939/connect","apiKey":"abcdeC2aqbh7dg8TO43XPu7r56YDh000"}`),
 	}
-	s.clearFileCredentials(&cs)
 
-	cred, err := cs.Set("example", "https://example.com", "12345")
+	credResult, err := record.ToCredential()
 	s.NoError(err)
-	s.NotNil(cred.GUID)
-	s.Equal(cred.Name, "example")
-	s.Equal(cred.URL, "https://example.com")
-	s.Equal(cred.ApiKey, "12345")
+	s.Equal(credResult, &Credential{
+		GUID:   "18cd5640-bee5-4b2a-992a-a2725ab6103d",
+		Name:   "friedtofu",
+		URL:    "https://a1.connect-server:3939/connect",
+		ApiKey: "abcdeC2aqbh7dg8TO43XPu7r56YDh000",
+	})
 }
 
-func (s *CredentialsTestSuite) TestSetURLCollisionError() {
-	keyring.MockInit()
-	cs := credentialsService{
-		afs: afero.NewMemMapFs(),
-		log: s.log,
+func (s *CredentialsServiceTestSuite) TestCredentialRecord_CorruptedErr() {
+	record := CredentialRecord{
+		GUID:    "18cd5640-bee5-4b2a-992a-a2725ab6103d",
+		Version: 0,
+		Data: []byte(`
+		$. 8989 guid":"18cd5640-bee5-4b2a-992a-a2725ab6103d","name":"friedtofu",
+		"url": "https://a1.connect-server:3939/connect","apiKey":"abcdeC2aqbh7dg8TO43XPu7r56YDh000"}`),
 	}
-	s.clearFileCredentials(&cs)
-
-	_, err := cs.Set("example", "https://example.com", "12345")
-	s.NoError(err)
-	_, err = cs.Set("example", "https://example.com", "12345")
-	s.Error(err)
-	s.IsType(&URLCollisionError{}, err)
-
-	// validate collision with env var credentials
-	s.createFileCredentials(&cs, true)
 
-	_, err = cs.Set("unique", fileCred.URL, "12345")
-	s.Error(err)
-	s.IsType(&EnvURLCollisionError{}, err)
+	_, err := record.ToCredential()
+	s.EqualError(err, "credential '18cd5640-bee5-4b2a-992a-a2725ab6103d' is corrupted")
 }
 
-func (s *CredentialsTestSuite) TestGet() {
-	keyring.MockInit()
-	cs := credentialsService{
-		afs: afero.NewMemMapFs(),
-		log: s.log,
+func (s *CredentialsServiceTestSuite) TestCredentialRecord_VersionErr() {
+	record := CredentialRecord{
+		GUID:    "18cd5640-bee5-4b2a-992a-a2725ab6103d",
+		Version: 45,
+		Data: []byte(`
+		{"guid":"18cd5640-bee5-4b2a-992a-a2725ab6103d","name":"friedtofu",
+		"url": "https://a1.connect-server:3939/connect","apiKey":"abcdeC2aqbh7dg8TO43XPu7r56YDh000"}`),
 	}
-	s.clearFileCredentials(&cs)
-	testGuid := "5ede880a-acd8-4206-b9fa-7d788c42fbe4"
-
-	// First test without any credentials in environment
-	s.log.On("Debug", "Credential does not exist", "credential", testGuid).Return()
 
-	// error if missing
-	_, err := cs.Get(testGuid)
-	s.Error(err)
-	s.log.AssertExpectations(s.T())
-
-	// pass if exists
-	cred, err := cs.Set("example", "https://example.com", "12345")
-	s.NoError(err)
-	res, err := cs.Get(cred.GUID)
-	s.NoError(err)
-	s.Equal(res, cred)
-
-	// confirm environment credential not available
-	s.log.On("Debug", "Credential does not exist", "credential", fileCred.GUID).Return()
-	_, err = cs.Get(fileCred.GUID)
-	s.Error(err)
-	s.log.AssertExpectations(s.T())
-
-	// Test with credentials in environment
-	s.createFileCredentials(&cs, true)
-
-	// retest prior test
-	res, err = cs.Get(cred.GUID)
-	s.NoError(err)
-	s.Equal(res, cred)
-
-	// request environment credentials
-	res, err = cs.Get(fileCred.GUID)
-	s.NoError(err)
-	s.Equal(res, &fileCred)
-
-	// Test for conflicts where credential was saved ahead of env variable
-	s.clearFileCredentials(&cs)
-	cred, err = cs.Set("env", fileCred.URL, "12345")
-	s.NoError(err)
-	_, err = cs.Get(cred.GUID)
-	s.NoError(err)
-
-	s.log.On("Debug", "Conflict on credential record", "error", "CONNECT_SERVER URL value conflicts with existing credential (connect.localtest.me) URL: https://connect.localtest.me/rsc/dev-password-copy").Return()
-	s.createFileCredentials(&cs, false)
-	_, err = cs.Get(cred.GUID)
-	s.Error(err)
-	s.IsType(&EnvURLCollisionError{}, err)
-	s.log.AssertExpectations(s.T())
+	_, err := record.ToCredential()
+	s.EqualError(err, "credential version not supported: 45")
 }
 
-func (s *CredentialsTestSuite) TestNormalizedSet() {
+func (s *CredentialsServiceTestSuite) TestNewCredentialsService_KeyringOK() {
 	keyring.MockInit()
-	cs := credentialsService{
-		afs: afero.NewMemMapFs(),
-		log: s.log,
-	}
-	s.clearFileCredentials(&cs)
-
-	// pass if no change (already normalized)
-	cred, err := cs.Set("example", "https://example.com", "12345")
+	credservice, err := NewCredentialsService(s.log)
 	s.NoError(err)
-	res, err := cs.Get(cred.GUID)
-	s.NoError(err)
-	s.Equal(res.URL, cred.URL)
-
-	// pass if URL ends up normalized
-	cred, err = cs.Set("example2", "https://example.com///another/seg/", "12345")
-	s.NoError(err)
-	s.NotEqual(cred.URL, "https://example.com///another/seg/")
-
-	res, err = cs.Get(cred.GUID)
-	s.NoError(err)
-	s.Equal(res.URL, "https://example.com/another/seg")
-	s.Equal(cred.URL, res.URL)
+	s.Implements((*CredentialsService)(nil), credservice)
 }
 
-func (s *CredentialsTestSuite) TestSetCollisions() {
-	keyring.MockInit()
-	cs := credentialsService{
-		afs: afero.NewMemMapFs(),
-		log: s.log,
-	}
+func (s *CredentialsServiceTestSuite) TestNewCredentialsService_KeyringErrFallbackFile() {
+	// Use an in memory filesystem for this test
+	// avoiding to manipulate users ~/.connect-credentials
+	fsys = afero.NewMemMapFs()
+	defer func() { fsys = afero.NewOsFs() }()
 
-	// Add credentials into environment
-	s.createFileCredentials(&cs, true)
+	keyringErr := errors.New("this is a teapot, unsupported system")
+	keyring.MockInitWithError(keyringErr)
 
-	// add a non-environment credential
-	_, err := cs.Set("example", "https://example.com", "12345")
-	s.NoError(err)
-
-	// non-environment name collision
-	_, err = cs.Set("example", "https://more_examples.com", "12345")
-	s.Error(err)
-	s.IsType(&NameCollisionError{}, err)
-
-	// environment name collision
-	_, err = cs.Set(fileCred.Name, "https://more_examples2.com", "12345")
-	s.Error(err)
-	s.IsType(&EnvNameCollisionError{}, err)
-
-	// non-environment URL collision
-	_, err = cs.Set("another_example", "https://example.com", "12345")
-	s.Error(err)
-	s.IsType(&URLCollisionError{}, err)
-
-	// environment URL collision
-	_, err = cs.Set("one_more", fileCred.URL, "12345")
-	s.Error(err)
-	s.IsType(&EnvURLCollisionError{}, err)
-}
-
-func (s *CredentialsTestSuite) TestDelete() {
-	keyring.MockInit()
-	cs := credentialsService{
-		afs: afero.NewMemMapFs(),
-		log: s.log,
-	}
-	s.clearFileCredentials(&cs)
-
-	cred, err := cs.Set("example", "https://example.com", "12345")
-	s.NoError(err)
+	s.log.On("Debug", "System keyring service is not available", "error", "failed to load credentials: this is a teapot, unsupported system").Return()
+	s.log.On("Debug", "Fallback to file managed credentials service due to unavailable system keyring").Return()
 
-	// no error if exists
-	err = cs.Delete(cred.GUID)
+	credservice, err := NewCredentialsService(s.log)
 	s.NoError(err)
-
-	// err if missing
-	s.log.On("Debug", "Credential does not exist", "credential", cred.GUID).Return()
-	err = cs.Delete(cred.GUID)
-	s.Error(err)
-	s.log.AssertExpectations(s.T())
-
-	// Add credentials into environment
-	s.createFileCredentials(&cs, true)
-
-	// err for our special GUID
-	err = cs.Delete(fileCred.GUID)
-	s.Error(err)
-	s.IsType(&EnvURLDeleteError{}, err)
+	s.Implements((*CredentialsService)(nil), credservice)
 }
diff --git a/internal/credentials/errors.go b/internal/credentials/errors.go
index 722a50a4b..bd967b8a1 100644
--- a/internal/credentials/errors.go
+++ b/internal/credentials/errors.go
@@ -54,20 +54,6 @@ func (e *URLCollisionError) Error() string {
 	return fmt.Sprintf("URL value conflicts with existing credential (%s) URL: %s", e.Name, e.URL)
 }
 
-// Environment URL overlaps with Credential URL
-type EnvURLCollisionError struct {
-	Name string
-	URL  string
-}
-
-func NewEnvURLCollisionError(name, url string) *EnvURLCollisionError {
-	return &EnvURLCollisionError{name, url}
-}
-
-func (e *EnvURLCollisionError) Error() string {
-	return fmt.Sprintf("CONNECT_SERVER URL value conflicts with existing credential (%s) URL: %s", e.Name, e.URL)
-}
-
 // Name is used by another credential
 type NameCollisionError struct {
 	Name string
@@ -82,33 +68,6 @@ func (e *NameCollisionError) Error() string {
 	return fmt.Sprintf("Name value conflicts with existing credential (%s) URL: %s", e.Name, e.URL)
 }
 
-// Environment URL overlaps with Credential URL
-type EnvNameCollisionError struct {
-	Name string
-	URL  string
-}
-
-func NewEnvNameCollisionError(name, url string) *EnvNameCollisionError {
-	return &EnvNameCollisionError{name, url}
-}
-
-func (e *EnvNameCollisionError) Error() string {
-	return fmt.Sprintf("CONNECT_SERVER Name value conflicts with existing credential (%s) URL: %s", e.Name, e.URL)
-}
-
-// Deleting Environment Credentials Not allowed
-type EnvURLDeleteError struct {
-	GUID string
-}
-
-func NewEnvURLDeleteError(guid string) *EnvURLDeleteError {
-	return &EnvURLDeleteError{guid}
-}
-
-func (e *EnvURLDeleteError) Error() string {
-	return fmt.Sprintf("DELETING an environment credential is not allowed. (GUID=%s)", e.GUID)
-}
-
 type VersionError struct {
 	Version uint
 }
diff --git a/internal/credentials/file.go b/internal/credentials/file.go
index cc9c7f6d7..6d24fd8a3 100644
--- a/internal/credentials/file.go
+++ b/internal/credentials/file.go
@@ -14,6 +14,8 @@ import (
 	"github.com/spf13/afero"
 )
 
+var fsys = afero.NewOsFs()
+
 const ondiskFilename = ".connect-credentials"
 
 type fileCredential struct {
@@ -66,7 +68,6 @@ func (fcs *fileCredentials) RemoveByName(name string) {
 
 type fileCredentialsService struct {
 	mu            sync.Mutex
-	afs           afero.Fs
 	log           logging.Logger
 	credsFilepath util.AbsolutePath
 }
@@ -77,7 +78,7 @@ func NewFileCredentialsService(log logging.Logger) (*fileCredentialsService, err
 	}
 
 	// Set home dir credentials file path
-	homeDir, err := util.UserHomeDir(fservice.afs)
+	homeDir, err := util.UserHomeDir(fsys)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/credentials/file_test.go b/internal/credentials/file_test.go
index 86c85cf7b..2fe1e61c9 100644
--- a/internal/credentials/file_test.go
+++ b/internal/credentials/file_test.go
@@ -10,6 +10,7 @@ import (
 	"github.com/posit-dev/publisher/internal/logging/loggingtest"
 	"github.com/posit-dev/publisher/internal/util"
 	"github.com/posit-dev/publisher/internal/util/utiltest"
+	"github.com/spf13/afero"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/suite"
 )
@@ -72,6 +73,26 @@ func (s *FileCredentialsServiceSuite) SetupTest() {
 	s.fileSetupNewCredsTest()
 }
 
+func (s *FileCredentialsServiceSuite) TestNewFileCredentialsService() {
+	// Use an in memory filesystem for this test
+	// avoiding to manipulate users ~/.connect-credentials
+	fsys = afero.NewMemMapFs()
+	defer func() { fsys = afero.NewOsFs() }()
+
+	fcs, err := NewFileCredentialsService(s.loggerMock)
+	s.NoError(err)
+	s.Implements((*CredentialsService)(nil), fcs)
+
+	expectedCredsPath, err := util.UserHomeDir(fsys)
+	s.NoError(err)
+
+	expectedCredsPath = expectedCredsPath.Join(".connect-credentials")
+	s.Equal(fcs, &fileCredentialsService{
+		log:           s.loggerMock,
+		credsFilepath: expectedCredsPath,
+	})
+}
+
 func (s *FileCredentialsServiceSuite) TestSetupService() {
 	cs := &fileCredentialsService{
 		log:           s.loggerMock,
diff --git a/internal/credentials/keyring.go b/internal/credentials/keyring.go
index 7796f6be5..bcced33cc 100644
--- a/internal/credentials/keyring.go
+++ b/internal/credentials/keyring.go
@@ -3,44 +3,177 @@
 package credentials
 
 import (
-	"strings"
+	"encoding/json"
+	"fmt"
 
+	"github.com/google/uuid"
+	"github.com/posit-dev/publisher/internal/logging"
+	"github.com/posit-dev/publisher/internal/util"
 	"github.com/zalando/go-keyring"
 )
 
-type KeyringService struct {
-	keyring.Keyring
+type keyringCredentialsService struct {
+	log logging.Logger
 }
 
-func (ks *KeyringService) Set(service, user, password string) error {
-	err := keyring.Set(service, user, password)
+func NewKeyringCredentialsService(log logging.Logger) *keyringCredentialsService {
+	return &keyringCredentialsService{
+		log: log,
+	}
+}
+
+func (ks *keyringCredentialsService) IsSupported() bool {
+	_, err := ks.load()
+	if err != nil {
+		ks.log.Debug("System keyring service is not available", "error", err.Error())
+		return false
+	}
+	return true
+}
+
+// Delete removes a Credential by its guid.
+// If lookup by guid fails, a NotFoundError is returned.
+func (ks *keyringCredentialsService) Delete(guid string) error {
+	table, err := ks.load()
+	if err != nil {
+		return err
+	}
+
+	_, exists := table[guid]
+	if !exists {
+		ks.log.Debug("Credential does not exist", "credential", guid)
+		return NewNotFoundError(guid)
+	}
+
+	delete(table, guid)
+	return ks.save(table)
+}
+
+// Get retrieves a Credential by its guid.
+func (ks *keyringCredentialsService) Get(guid string) (*Credential, error) {
+	table, err := ks.load()
 	if err != nil {
-		if strings.HasSuffix(err.Error(), "exec: \"dbus-launch\": executable file not found in $PATH") {
-			keyring.MockInit()
-			return keyring.Set(service, user, password)
+		return nil, err
+	}
+
+	cr, exists := table[guid]
+	if !exists {
+		ks.log.Debug("Credential does not exist", "credential", guid)
+		return nil, NewNotFoundError(guid)
+	}
+
+	return cr.ToCredential()
+}
+
+// List retrieves all Credentials
+func (ks *keyringCredentialsService) List() ([]Credential, error) {
+	records, err := ks.load()
+	if err != nil {
+		return nil, err
+	}
+	var creds []Credential = make([]Credential, 0)
+	for _, cr := range records {
+		cred, err := cr.ToCredential()
+		if err != nil {
+			return nil, err
 		}
+		creds = append(creds, *cred)
 	}
-	return err
+	return creds, nil
 }
 
-func (ks *KeyringService) Get(service, user string) (string, error) {
-	v, err := keyring.Get(service, user)
+// Set creates a Credential.
+// A guid is assigned to the Credential using the UUIDv4 specification.
+func (ks *keyringCredentialsService) Set(name string, url string, ak string) (*Credential, error) {
+	table, err := ks.load()
+	if err != nil {
+		return nil, err
+	}
+
+	normalizedUrl, err := util.NormalizeServerURL(url)
 	if err != nil {
-		if strings.HasSuffix(err.Error(), "exec: \"dbus-launch\": executable file not found in $PATH") {
-			keyring.MockInit()
-			return keyring.Get(service, user)
+		return nil, err
+	}
+
+	guid := uuid.New().String()
+	cred := Credential{
+		GUID:   guid,
+		Name:   name,
+		URL:    normalizedUrl,
+		ApiKey: ak,
+	}
+
+	err = ks.checkForConflicts(&table, &cred)
+	if err != nil {
+		return nil, err
+	}
+
+	raw, err := json.Marshal(cred)
+	if err != nil {
+		return nil, fmt.Errorf("error marshalling credential: %v", err)
+	}
+
+	table[guid] = CredentialRecord{
+		GUID:    guid,
+		Version: CurrentVersion,
+		Data:    json.RawMessage(raw),
+	}
+
+	err = ks.save(table)
+	if err != nil {
+		return nil, err
+	}
+
+	return &cred, nil
+}
+
+func (ks *keyringCredentialsService) checkForConflicts(
+	table *map[string]CredentialRecord,
+	c *Credential) error {
+	// Check if Credential attributes (URL or name) are already used by another credential
+	for guid, record := range *table {
+		cred, err := record.ToCredential()
+		if err != nil {
+			return NewCorruptedError(guid)
+		}
+
+		err = cred.ConflictCheck(*c)
+		if err != nil {
+			return err
 		}
 	}
-	return v, err
+	return nil
 }
 
-func (ks *KeyringService) Delete(service, user string) error {
-	err := keyring.Delete(service, user)
+// Saves the CredentialTable
+func (ks *keyringCredentialsService) save(table CredentialTable) error {
+	data, err := json.Marshal(table)
 	if err != nil {
-		if strings.HasSuffix(err.Error(), "exec: \"dbus-launch\": executable file not found in $PATH") {
-			keyring.MockInit()
-			return keyring.Delete(service, user)
+		return fmt.Errorf("failed to serialize credentials: %v", err)
+	}
+
+	err = keyring.Set(ServiceName, "credentials", string(data))
+	if err != nil {
+		return fmt.Errorf("failed to set credentials: %v", err)
+	}
+	return nil
+}
+
+// Loads the CredentialTable from keyRing
+func (ks *keyringCredentialsService) load() (CredentialTable, error) {
+	data, err := keyring.Get(ServiceName, "credentials")
+	if err != nil {
+		if err == keyring.ErrNotFound {
+			return make(map[string]CredentialRecord), nil
 		}
+		return nil, NewLoadError(err)
+	}
+
+	var table CredentialTable
+	err = json.Unmarshal([]byte(data), &table)
+	if err != nil {
+		return nil, fmt.Errorf("failed to deserialize credentials: %v", err)
 	}
-	return err
+
+	return table, nil
 }
diff --git a/internal/credentials/keyring_test.go b/internal/credentials/keyring_test.go
new file mode 100644
index 000000000..645e3dc2f
--- /dev/null
+++ b/internal/credentials/keyring_test.go
@@ -0,0 +1,142 @@
+// Copyright (C) 2024 by Posit Software, PBC.
+
+package credentials
+
+import (
+	"testing"
+
+	"github.com/posit-dev/publisher/internal/logging/loggingtest"
+	"github.com/posit-dev/publisher/internal/util/utiltest"
+	"github.com/stretchr/testify/suite"
+	"github.com/zalando/go-keyring"
+)
+
+type KeyringCredentialsTestSuite struct {
+	utiltest.Suite
+	log *loggingtest.MockLogger
+}
+
+func TestKeyringCredentialsTestSuite(t *testing.T) {
+	suite.Run(t, new(KeyringCredentialsTestSuite))
+}
+
+func (s *KeyringCredentialsTestSuite) SetupTest() {
+	keyring.MockInit()
+	s.log = loggingtest.NewMockLogger()
+}
+
+func (s *KeyringCredentialsTestSuite) TestNewKeyringCredentialsService() {
+	ks := NewKeyringCredentialsService(s.log)
+	s.Equal(ks, &keyringCredentialsService{s.log})
+	s.Implements((*CredentialsService)(nil), ks)
+}
+
+func (s *KeyringCredentialsTestSuite) TestSet() {
+	cs := keyringCredentialsService{
+		log: s.log,
+	}
+
+	cred, err := cs.Set("example", "https://example.com", "12345")
+	s.NoError(err)
+	s.NotNil(cred.GUID)
+	s.Equal(cred.Name, "example")
+	s.Equal(cred.URL, "https://example.com")
+	s.Equal(cred.ApiKey, "12345")
+}
+
+func (s *KeyringCredentialsTestSuite) TestSetURLCollisionError() {
+	cs := keyringCredentialsService{
+		log: s.log,
+	}
+
+	_, err := cs.Set("example", "https://example.com", "12345")
+	s.NoError(err)
+	_, err = cs.Set("example", "https://example.com", "12345")
+	s.Error(err)
+	s.IsType(&URLCollisionError{}, err)
+}
+
+func (s *KeyringCredentialsTestSuite) TestGet() {
+	cs := keyringCredentialsService{
+		log: s.log,
+	}
+
+	testGuid := "5ede880a-acd8-4206-b9fa-7d788c42fbe4"
+
+	// First test without any credentials in environment
+	s.log.On("Debug", "Credential does not exist", "credential", testGuid).Return()
+
+	// error if missing
+	_, err := cs.Get(testGuid)
+	s.Error(err)
+	s.log.AssertExpectations(s.T())
+
+	// pass if exists
+	cred, err := cs.Set("example", "https://example.com", "12345")
+	s.NoError(err)
+	res, err := cs.Get(cred.GUID)
+	s.NoError(err)
+	s.Equal(res, cred)
+}
+
+func (s *KeyringCredentialsTestSuite) TestNormalizedSet() {
+	cs := keyringCredentialsService{
+		log: s.log,
+	}
+
+	// pass if no change (already normalized)
+	cred, err := cs.Set("example", "https://example.com", "12345")
+	s.NoError(err)
+	res, err := cs.Get(cred.GUID)
+	s.NoError(err)
+	s.Equal(res.URL, cred.URL)
+
+	// pass if URL ends up normalized
+	cred, err = cs.Set("example2", "https://example.com///another/seg/", "12345")
+	s.NoError(err)
+	s.NotEqual(cred.URL, "https://example.com///another/seg/")
+
+	res, err = cs.Get(cred.GUID)
+	s.NoError(err)
+	s.Equal(res.URL, "https://example.com/another/seg")
+	s.Equal(cred.URL, res.URL)
+}
+
+func (s *KeyringCredentialsTestSuite) TestSetCollisions() {
+	cs := keyringCredentialsService{
+		log: s.log,
+	}
+
+	// add a credential
+	_, err := cs.Set("example", "https://example.com", "12345")
+	s.NoError(err)
+
+	// name collision
+	_, err = cs.Set("example", "https://more_examples.com", "12345")
+	s.Error(err)
+	s.IsType(&NameCollisionError{}, err)
+
+	// URL collision
+	_, err = cs.Set("another_example", "https://example.com", "12345")
+	s.Error(err)
+	s.IsType(&URLCollisionError{}, err)
+}
+
+func (s *KeyringCredentialsTestSuite) TestDelete() {
+	cs := keyringCredentialsService{
+		log: s.log,
+	}
+
+	cred, err := cs.Set("example", "https://example.com", "12345")
+	s.NoError(err)
+
+	// no error if exists
+	err = cs.Delete(cred.GUID)
+	s.NoError(err)
+
+	// err if missing
+	s.log.On("Debug", "Credential does not exist", "credential", cred.GUID).Return()
+	err = cs.Delete(cred.GUID)
+	s.Error(err)
+	s.log.AssertExpectations(s.T())
+}

From 55e9fd4d24a22a6bca39767bd3159c570139c0f8 Mon Sep 17 00:00:00 2001
From: Marcos Navarro <m@marcosn.com>
Date: Wed, 25 Sep 2024 11:42:55 -0600
Subject: [PATCH 2/5] Update usage of credentials service on consumers.
 Removing env var guid.

---
 cmd/publisher/commands/credentials.go         | 26 +++++++++++++++----
 cmd/publisher/main.go                         |  9 +++++--
 extensions/vscode/src/constants.ts            |  4 ---
 .../src/components/views/Credentials.vue      | 11 +-------
 internal/accounts/account_list.go             | 15 ++++++-----
 internal/accounts/account_list_test.go        |  3 ++-
 internal/accounts/provider_credentials.go     |  9 ++++---
 internal/services/api/api_errors.go           | 14 ++++++++++
 internal/services/api/delete_credentials.go   | 18 +++++++++++--
 .../services/api/delete_credentials_test.go   |  4 ++-
 internal/services/api/get_credential.go       | 16 +++++++++++-
 internal/services/api/get_credentials.go      | 15 ++++++++++-
 internal/services/api/post_credentials.go     | 19 +++++++++++---
 .../services/api/post_credentials_test.go     |  6 +++--
 internal/types/error.go                       | 11 ++++----
 15 files changed, 133 insertions(+), 47 deletions(-)

diff --git a/cmd/publisher/commands/credentials.go b/cmd/publisher/commands/credentials.go
index 133519c39..14e1f676a 100644
--- a/cmd/publisher/commands/credentials.go
+++ b/cmd/publisher/commands/credentials.go
@@ -25,7 +25,11 @@ type CreateCredentialCommand struct {
 }
 
 func (cmd *CreateCredentialCommand) Run(args *cli_types.CommonArgs, ctx *cli_types.CLIContext) error {
-	cs := credentials.NewCredentialsService(logging.NewDiscardLogger())
+	cs, err := credentials.NewCredentialsService(logging.NewDiscardLogger())
+	if err != nil {
+		return err
+	}
+
 	cred, err := cs.Set(cmd.Name, cmd.URL, cmd.ApiKey)
 	if err != nil {
 		return err
@@ -45,8 +49,12 @@ type DeleteCredentialCommand struct {
 }
 
 func (cmd *DeleteCredentialCommand) Run(args *cli_types.CommonArgs, ctx *cli_types.CLIContext) error {
-	cs := credentials.NewCredentialsService(logging.NewDiscardLogger())
-	err := cs.Delete(cmd.GUID)
+	cs, err := credentials.NewCredentialsService(logging.NewDiscardLogger())
+	if err != nil {
+		return err
+	}
+
+	err = cs.Delete(cmd.GUID)
 	if err != nil {
 		return err
 	}
@@ -60,7 +68,11 @@ type GetCredentialCommand struct {
 }
 
 func (cmd *GetCredentialCommand) Run(args *cli_types.CommonArgs, ctx *cli_types.CLIContext) error {
-	cs := credentials.NewCredentialsService(logging.NewDiscardLogger())
+	cs, err := credentials.NewCredentialsService(logging.NewDiscardLogger())
+	if err != nil {
+		return err
+	}
+
 	cred, err := cs.Get(cmd.GUID)
 	if err != nil {
 		return err
@@ -79,7 +91,11 @@ type ListCredentialsCommand struct {
 }
 
 func (cmd *ListCredentialsCommand) Run(args *cli_types.CommonArgs, ctx *cli_types.CLIContext) error {
-	cs := credentials.NewCredentialsService(logging.NewDiscardLogger())
+	cs, err := credentials.NewCredentialsService(logging.NewDiscardLogger())
+	if err != nil {
+		return err
+	}
+
 	creds, err := cs.List()
 	if err != nil {
 		return err
diff --git a/cmd/publisher/main.go b/cmd/publisher/main.go
index 95f985ca4..802e3dd9c 100644
--- a/cmd/publisher/main.go
+++ b/cmd/publisher/main.go
@@ -61,9 +61,14 @@ func main() {
 		defer pprof.StopCPUProfile()
 	}
 	ctx.Logger = events.NewCLILogger(cli.Verbose, os.Stderr)
-	ctx.Accounts = accounts.NewAccountList(ctx.Fs, ctx.Logger)
+	accounts, err := accounts.NewAccountList(ctx.Fs, ctx.Logger)
+	if err != nil {
+		Fatal(err)
+	}
+	ctx.Accounts = accounts
+
 	logVersion(ctx.Logger)
-	err := args.Run(&cli.CommonArgs)
+	err = args.Run(&cli.CommonArgs)
 	if err != nil {
 		Fatal(err)
 	}
diff --git a/extensions/vscode/src/constants.ts b/extensions/vscode/src/constants.ts
index 97148df39..1b96dc7a3 100644
--- a/extensions/vscode/src/constants.ts
+++ b/extensions/vscode/src/constants.ts
@@ -33,10 +33,6 @@ const credentialsContexts = {
   EnvironmentVars: "posit.publisher.credentials.tree.item.environmentVars",
 };
 
-export const CredentialGUIs = {
-  EnvironmentGUID: "00000000-0000-0000-0000-000000000000",
-};
-
 const filesCommands = {
   Refresh: "posit.publisher.files.refresh",
   Exclude: "posit.publisher.files.exclude",
diff --git a/extensions/vscode/webviews/homeView/src/components/views/Credentials.vue b/extensions/vscode/webviews/homeView/src/components/views/Credentials.vue
index 9b551fdfd..fc32aabb8 100644
--- a/extensions/vscode/webviews/homeView/src/components/views/Credentials.vue
+++ b/extensions/vscode/webviews/homeView/src/components/views/Credentials.vue
@@ -13,11 +13,7 @@
       :title="credential.name"
       :description="credential.url"
       :data-automation="`${credential.name}-list`"
-      :codicon="
-        credential.guid === CredentialGUIs.EnvironmentGUID
-          ? 'codicon-bracket'
-          : 'codicon-key'
-      "
+      codicon="codicon-key"
       align-icon-with-twisty
       :data-vscode-context="vscodeContext(credential)"
     />
@@ -34,7 +30,6 @@ import { useHomeStore } from "src/stores/home";
 import { useHostConduitService } from "src/HostConduitService";
 
 import { Credential } from "../../../../../src/api";
-import { CredentialGUIs } from "../../../../../src/constants";
 import { WebviewToHostMessageType } from "../../../../../src/types/messages/webviewToHostMessages";
 
 const home = useHomeStore();
@@ -62,10 +57,6 @@ const sectionActions = computed(() => {
 });
 
 const vscodeContext = (credential: Credential) => {
-  if (credential.guid === CredentialGUIs.EnvironmentGUID) {
-    return undefined;
-  }
-
   return JSON.stringify({
     credentialGUID: credential.guid,
     credentialName: credential.name,
diff --git a/internal/accounts/account_list.go b/internal/accounts/account_list.go
index 7ae6e598c..1b90ab0fe 100644
--- a/internal/accounts/account_list.go
+++ b/internal/accounts/account_list.go
@@ -28,13 +28,16 @@ type defaultAccountList struct {
 
 var _ AccountList = &defaultAccountList{}
 
-func NewAccountList(fs afero.Fs, log logging.Logger) *defaultAccountList {
-	return &defaultAccountList{
-		providers: []AccountProvider{
-			NewCredentialsProvider(log),
-		},
-		log: log,
+func NewAccountList(fs afero.Fs, log logging.Logger) (*defaultAccountList, error) {
+	cprovider, err := NewCredentialsProvider(log)
+	if err != nil {
+		return nil, err
 	}
+
+	return &defaultAccountList{
+		providers: []AccountProvider{cprovider},
+		log:       log,
+	}, nil
 }
 
 func (l *defaultAccountList) GetAllAccounts() (accounts []Account, err error) {
diff --git a/internal/accounts/account_list_test.go b/internal/accounts/account_list_test.go
index 736775c71..a14fbc98f 100644
--- a/internal/accounts/account_list_test.go
+++ b/internal/accounts/account_list_test.go
@@ -41,7 +41,8 @@ func (s *AccountListSuite) SetupSuite() {
 func (s *AccountListSuite) TestNewAccountList() {
 	log := logging.New()
 	fs := utiltest.NewMockFs()
-	accountList := NewAccountList(fs, log)
+	accountList, err := NewAccountList(fs, log)
+	s.NoError(err)
 	s.Len(accountList.providers, 1)
 	s.Equal(log, accountList.log)
 }
diff --git a/internal/accounts/provider_credentials.go b/internal/accounts/provider_credentials.go
index 9dc22ae47..276307906 100644
--- a/internal/accounts/provider_credentials.go
+++ b/internal/accounts/provider_credentials.go
@@ -11,10 +11,13 @@ type CredentialsProvider struct {
 	cs credentials.CredentialsService
 }
 
-func NewCredentialsProvider(log logging.Logger) *CredentialsProvider {
-	return &CredentialsProvider{
-		cs: credentials.NewCredentialsService(log),
+func NewCredentialsProvider(log logging.Logger) (*CredentialsProvider, error) {
+	cs, err := credentials.NewCredentialsService(log)
+	if err != nil {
+		return nil, err
 	}
+
+	return &CredentialsProvider{cs}, nil
 }
 
 func (p *CredentialsProvider) Load() ([]Account, error) {
diff --git a/internal/services/api/api_errors.go b/internal/services/api/api_errors.go
index cea58de6c..001b49128 100644
--- a/internal/services/api/api_errors.go
+++ b/internal/services/api/api_errors.go
@@ -65,3 +65,17 @@ func APIErrorInvalidTOMLFileFromAgentError(aerr types.AgentError) APIErrorInvali
 func (apierr *APIErrorInvalidTOMLFileDetails) JSONResponse(w http.ResponseWriter) {
 	JsonResult(w, http.StatusBadRequest, apierr)
 }
+
+type APIErrorCredentialServiceUnavailable struct {
+	Code types.ErrorCode `json:"code"`
+}
+
+func APIErrorCredentialsUnavailableFromAgentError(aerr types.AgentError) APIErrorCredentialServiceUnavailable {
+	return APIErrorCredentialServiceUnavailable{
+		Code: types.ErrorCredentialServiceUnavailable,
+	}
+}
+
+func (apierr *APIErrorCredentialServiceUnavailable) JSONResponse(w http.ResponseWriter) {
+	JsonResult(w, http.StatusServiceUnavailable, apierr)
+}
diff --git a/internal/services/api/delete_credentials.go b/internal/services/api/delete_credentials.go
index 3020b9b0e..99985c264 100644
--- a/internal/services/api/delete_credentials.go
+++ b/internal/services/api/delete_credentials.go
@@ -8,13 +8,27 @@ import (
 	"github.com/gorilla/mux"
 	"github.com/posit-dev/publisher/internal/credentials"
 	"github.com/posit-dev/publisher/internal/logging"
+	"github.com/posit-dev/publisher/internal/types"
 )
 
 func DeleteCredentialHandlerFunc(log logging.Logger) http.HandlerFunc {
 	return func(w http.ResponseWriter, req *http.Request) {
 		guid := mux.Vars(req)["guid"]
-		cs := credentials.NewCredentialsService(log)
-		err := cs.Delete(guid)
+
+		cs, err := credentials.NewCredentialsService(log)
+		if err != nil {
+			if aerr, ok := err.(*types.AgentError); ok {
+				if aerr.Code == types.ErrorCredentialServiceUnavailable {
+					apiErr := APIErrorCredentialsUnavailableFromAgentError(*aerr)
+					apiErr.JSONResponse(w)
+					return
+				}
+			}
+			InternalError(w, req, log, err)
+			return
+		}
+
+		err = cs.Delete(guid)
 		if err != nil {
 			switch e := err.(type) {
 			case *credentials.NotFoundError:
diff --git a/internal/services/api/delete_credentials_test.go b/internal/services/api/delete_credentials_test.go
index 299a05866..9a68fb1eb 100644
--- a/internal/services/api/delete_credentials_test.go
+++ b/internal/services/api/delete_credentials_test.go
@@ -34,7 +34,9 @@ func (s *DeleteCredentialsSuite) SetupTest() {
 }
 
 func (s *DeleteCredentialsSuite) Test204() {
-	cs := credentials.NewCredentialsService(s.log)
+	cs, err := credentials.NewCredentialsService(s.log)
+	s.NoError(err)
+
 	cred, err := cs.Set("example", "https://example.com", "12345")
 	s.NoError(err)
 
diff --git a/internal/services/api/get_credential.go b/internal/services/api/get_credential.go
index fd95ff579..ec1bf06d1 100644
--- a/internal/services/api/get_credential.go
+++ b/internal/services/api/get_credential.go
@@ -9,12 +9,26 @@ import (
 	"github.com/gorilla/mux"
 	"github.com/posit-dev/publisher/internal/credentials"
 	"github.com/posit-dev/publisher/internal/logging"
+	"github.com/posit-dev/publisher/internal/types"
 )
 
 func GetCredentialHandlerFunc(log logging.Logger) http.HandlerFunc {
 	return func(w http.ResponseWriter, req *http.Request) {
 		guid := mux.Vars(req)["guid"]
-		cs := credentials.NewCredentialsService(log)
+
+		cs, err := credentials.NewCredentialsService(log)
+		if err != nil {
+			if aerr, ok := err.(*types.AgentError); ok {
+				if aerr.Code == types.ErrorCredentialServiceUnavailable {
+					apiErr := APIErrorCredentialsUnavailableFromAgentError(*aerr)
+					apiErr.JSONResponse(w)
+					return
+				}
+			}
+			InternalError(w, req, log, err)
+			return
+		}
+
 		cred, err := cs.Get(guid)
 		if err != nil {
 			switch e := err.(type) {
diff --git a/internal/services/api/get_credentials.go b/internal/services/api/get_credentials.go
index d10fc1787..4987261b9 100644
--- a/internal/services/api/get_credentials.go
+++ b/internal/services/api/get_credentials.go
@@ -8,11 +8,24 @@ import (
 
 	"github.com/posit-dev/publisher/internal/credentials"
 	"github.com/posit-dev/publisher/internal/logging"
+	"github.com/posit-dev/publisher/internal/types"
 )
 
 func GetCredentialsHandlerFunc(log logging.Logger) http.HandlerFunc {
 	return func(w http.ResponseWriter, req *http.Request) {
-		cs := credentials.NewCredentialsService(log)
+		cs, err := credentials.NewCredentialsService(log)
+		if err != nil {
+			if aerr, ok := err.(*types.AgentError); ok {
+				if aerr.Code == types.ErrorCredentialServiceUnavailable {
+					apiErr := APIErrorCredentialsUnavailableFromAgentError(*aerr)
+					apiErr.JSONResponse(w)
+					return
+				}
+			}
+			InternalError(w, req, log, err)
+			return
+		}
+
 		creds, err := cs.List()
 		if err != nil {
 			InternalError(w, req, log, err)
diff --git a/internal/services/api/post_credentials.go b/internal/services/api/post_credentials.go
index 1bd9b28e6..fab400e2a 100644
--- a/internal/services/api/post_credentials.go
+++ b/internal/services/api/post_credentials.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/posit-dev/publisher/internal/credentials"
 	"github.com/posit-dev/publisher/internal/logging"
+	"github.com/posit-dev/publisher/internal/types"
 )
 
 type PostCredentialsRequest struct {
@@ -30,7 +31,19 @@ func PostCredentialFuncHandler(log logging.Logger) http.HandlerFunc {
 			return
 		}
 
-		cs := credentials.NewCredentialsService(log)
+		cs, err := credentials.NewCredentialsService(log)
+		if err != nil {
+			if aerr, ok := err.(*types.AgentError); ok {
+				if aerr.Code == types.ErrorCredentialServiceUnavailable {
+					apiErr := APIErrorCredentialsUnavailableFromAgentError(*aerr)
+					apiErr.JSONResponse(w)
+					return
+				}
+			}
+			InternalError(w, req, log, err)
+			return
+		}
+
 		cred, err := cs.Set(body.Name, body.URL, body.ApiKey)
 		if err != nil {
 			if _, ok := err.(*credentials.URLCollisionError); ok {
@@ -41,8 +54,6 @@ func PostCredentialFuncHandler(log logging.Logger) http.HandlerFunc {
 			return
 		}
 
-		w.WriteHeader(http.StatusCreated)
-		w.Header().Set("content-type", "application/json")
-		json.NewEncoder(w).Encode(cred)
+		JsonResult(w, http.StatusCreated, cred)
 	}
 }
diff --git a/internal/services/api/post_credentials_test.go b/internal/services/api/post_credentials_test.go
index c6da6d176..4e4af8179 100644
--- a/internal/services/api/post_credentials_test.go
+++ b/internal/services/api/post_credentials_test.go
@@ -60,8 +60,10 @@ func (s *PostCredentialTestSuite) Test409() {
 	url := "http://example.com"
 	ak := "12345"
 
-	cs := credentials.NewCredentialsService(s.log)
-	_, err := cs.Set(name, url, ak)
+	cs, err := credentials.NewCredentialsService(s.log)
+	s.NoError(err)
+
+	_, err = cs.Set(name, url, ak)
 	s.NoError(err)
 
 	cred := PostCredentialsRequest{
diff --git a/internal/types/error.go b/internal/types/error.go
index 1e1e8ba17..b099c865d 100644
--- a/internal/types/error.go
+++ b/internal/types/error.go
@@ -11,11 +11,12 @@ type ErrorData map[string]any
 type Operation string
 
 const (
-	ErrorResourceNotFound   ErrorCode = "resourceNotFound"
-	ErrorInvalidTOML        ErrorCode = "invalidTOML"
-	ErrorUnknownTOMLKey     ErrorCode = "unknownTOMLKey"
-	ErrorInvalidConfigFiles ErrorCode = "invalidConfigFiles"
-	ErrorUnknown            ErrorCode = "unknown"
+	ErrorResourceNotFound             ErrorCode = "resourceNotFound"
+	ErrorInvalidTOML                  ErrorCode = "invalidTOML"
+	ErrorUnknownTOMLKey               ErrorCode = "unknownTOMLKey"
+	ErrorInvalidConfigFiles           ErrorCode = "invalidConfigFiles"
+	ErrorCredentialServiceUnavailable ErrorCode = "credentialsServiceUnavailable"
+	ErrorUnknown                      ErrorCode = "unknown"
 )
 
 type EventableError interface {

From a825c820bf91da1c4b70a68499ac819b2706a513 Mon Sep 17 00:00:00 2001
From: Marcos Navarro <m@marcosn.com>
Date: Wed, 25 Sep 2024 13:04:14 -0600
Subject: [PATCH 3/5] Ensure file credentials List returns a list even when
 empty

---
 internal/credentials/file.go                       |  2 +-
 internal/credentials/file_test.go                  | 12 ++++++++++++
 internal/credentials/testdata/toml/emptycreds.toml |  0
 3 files changed, 13 insertions(+), 1 deletion(-)
 create mode 100644 internal/credentials/testdata/toml/emptycreds.toml

diff --git a/internal/credentials/file.go b/internal/credentials/file.go
index 6d24fd8a3..c54e4c20e 100644
--- a/internal/credentials/file.go
+++ b/internal/credentials/file.go
@@ -34,7 +34,7 @@ type fileCredentials struct {
 }
 
 func (fcs *fileCredentials) CredentialsList() []Credential {
-	var list []Credential
+	list := []Credential{}
 	for credName, fileCred := range fcs.Credentials {
 		list = append(list, Credential{
 			Name:   credName,
diff --git a/internal/credentials/file_test.go b/internal/credentials/file_test.go
index 2fe1e61c9..765fbc573 100644
--- a/internal/credentials/file_test.go
+++ b/internal/credentials/file_test.go
@@ -240,6 +240,18 @@ func (s *FileCredentialsServiceSuite) TestList() {
 	})
 }
 
+func (s *FileCredentialsServiceSuite) TestList_Empty() {
+	cs := &fileCredentialsService{
+		log:           s.loggerMock,
+		credsFilepath: s.testdata.Join("emptycreds.toml"),
+	}
+
+	creds, err := cs.List()
+	s.NoError(err)
+
+	s.Equal(creds, []Credential{})
+}
+
 func (s *FileCredentialsServiceSuite) TestList_CannotLoadErr() {
 	cs := &fileCredentialsService{
 		log:           s.loggerMock,
diff --git a/internal/credentials/testdata/toml/emptycreds.toml b/internal/credentials/testdata/toml/emptycreds.toml
new file mode 100644
index 000000000..e69de29bb

From 6f1973965b035f590044b95f07b7d85316444e8a Mon Sep 17 00:00:00 2001
From: Marcos Navarro <m@marcosn.com>
Date: Wed, 25 Sep 2024 14:27:37 -0600
Subject: [PATCH 4/5] Fix empty file loading creds as nil

---
 internal/credentials/file.go      |  8 +++++++-
 internal/credentials/file_test.go | 14 ++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/internal/credentials/file.go b/internal/credentials/file.go
index c54e4c20e..d4f2f9b88 100644
--- a/internal/credentials/file.go
+++ b/internal/credentials/file.go
@@ -33,6 +33,12 @@ type fileCredentials struct {
 	Credentials map[string]fileCredential `toml:"credentials"`
 }
 
+func newFileCredentials() fileCredentials {
+	return fileCredentials{
+		Credentials: make(map[string]fileCredential),
+	}
+}
+
 func (fcs *fileCredentials) CredentialsList() []Credential {
 	list := []Credential{}
 	for credName, fileCred := range fcs.Credentials {
@@ -221,7 +227,7 @@ func (c *fileCredentialsService) setup() error {
 }
 
 func (c *fileCredentialsService) load() (fileCredentials, error) {
-	var creds fileCredentials
+	creds := newFileCredentials()
 	err := util.ReadTOMLFile(c.credsFilepath, &creds)
 	if err != nil {
 		return creds, NewLoadError(err)
diff --git a/internal/credentials/file_test.go b/internal/credentials/file_test.go
index 765fbc573..78435482f 100644
--- a/internal/credentials/file_test.go
+++ b/internal/credentials/file_test.go
@@ -162,6 +162,20 @@ func (s *FileCredentialsServiceSuite) TestLoadFile() {
 	})
 }
 
+func (s *FileCredentialsServiceSuite) TestLoad_EmptyFile() {
+	cs := &fileCredentialsService{
+		log:           s.loggerMock,
+		credsFilepath: s.testdata.Join("emptycreds.toml"),
+	}
+
+	creds, err := cs.load()
+	s.NoError(err)
+
+	s.Equal(creds, fileCredentials{
+		Credentials: map[string]fileCredential{},
+	})
+}
+
 func (s *FileCredentialsServiceSuite) TestGet() {
 	cs := &fileCredentialsService{
 		log:           s.loggerMock,

From c6156cdea85c6de56f9eddb568a723e2121871df Mon Sep 17 00:00:00 2001
From: Marcos Navarro <m@marcosn.com>
Date: Thu, 26 Sep 2024 10:16:56 -0600
Subject: [PATCH 5/5] PR Feedback, test for credentials keyring List method

---
 internal/credentials/keyring_test.go | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/internal/credentials/keyring_test.go b/internal/credentials/keyring_test.go
index 645e3dc2f..15a2e7564 100644
--- a/internal/credentials/keyring_test.go
+++ b/internal/credentials/keyring_test.go
@@ -122,6 +122,28 @@ func (s *KeyringCredentialsTestSuite) TestSetCollisions() {
 	s.IsType(&URLCollisionError{}, err)
 }
 
+func (s *KeyringCredentialsTestSuite) TestList() {
+	cs := keyringCredentialsService{
+		log: s.log,
+	}
+
+	creds, err := cs.List()
+	s.NoError(err)
+	s.Equal(creds, []Credential{})
+
+	// Add a couple creds to be assert on the list again
+	nc1, err := cs.Set("example", "https://a.example.com", "12345")
+	s.NoError(err)
+	nc2, err := cs.Set("example2", "https://b.example.com", "12345")
+	s.NoError(err)
+
+	creds, err = cs.List()
+	s.NoError(err)
+	s.Len(creds, 2)
+	s.Contains(creds, *nc1)
+	s.Contains(creds, *nc2)
+}
+
 func (s *KeyringCredentialsTestSuite) TestDelete() {
 	cs := keyringCredentialsService{
 		log: s.log,