Reset credentials strategy for corrupted data, new error to bubble up…

… to warn the user

internal/credentials/credentials.go

@@ -31,6 +31,7 @@ package credentials
 
 import (
 	"encoding/json"
+	"errors"
 
 	"github.com/posit-dev/publisher/internal/logging"
 	"github.com/posit-dev/publisher/internal/types"
@@ -86,21 +87,58 @@ type CredentialsService interface {
 	Get(guid string) (*Credential, error)
 	List() ([]Credential, error)
 	Set(name string, url string, ak string) (*Credential, error)
+	Reset() error
 }
 
 // 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.
+// Additionally, tries to load the current credentials to handle resetting in case of data corruption.
 func NewCredentialsService(log logging.Logger) (CredentialsService, error) {
 	krService := NewKeyringCredentialsService(log)
 	if krService.IsSupported() {
-		return krService, nil
+		err := enforceKeyringIntegrity(krService, log)
+		return krService, err
 	}
 
 	log.Debug("Fallback to file managed credentials service due to unavailable system keyring")
 	fcService, err := NewFileCredentialsService(log)
 	if err != nil {
-		return nil, types.NewAgentError(types.ErrorCredentialServiceUnavailable, err, nil)
+		return fcService, enforceFilecredsIntegrity(fcService, err, log)
 	}
-
 	return fcService, nil
 }
+
+func enforceKeyringIntegrity(ks *keyringCredentialsService, log logging.Logger) error {
+	// Catch any error that could require attention early
+	_, err := ks.List()
+	if err == nil {
+		return nil
+	}
+
+	// If error from trying to fetch credentials list is not well known
+	// credentials handling is unavailable
+	if !errors.Is(err, &CorruptedError{}) && !errors.Is(err, &LoadError{}) {
+		return types.NewAgentError(types.ErrorCredentialServiceUnavailable, err, nil)
+	}
+
+	log.Warn("Corrupted credentials data found. A reset was applied to stored data to be able to proceed")
+	err = ks.Reset()
+	if err != nil {
+		return types.NewAgentError(types.ErrorCredentialServiceUnavailable, err, nil)
+	}
+
+	return types.NewAgentError(types.ErrorCredentialCorruptedReset, err, nil)
+}
+
+func enforceFilecredsIntegrity(fc *fileCredentialsService, err error, log logging.Logger) error {
+	if errors.Is(err, &LoadError{}) {
+		log.Warn("Corrupted credentials data found. A reset was applied to stored data to be able to proceed")
+		err = fc.Reset()
+		if err != nil {
+			return types.NewAgentError(types.ErrorCredentialServiceUnavailable, err, nil)
+		}
+		return types.NewAgentError(types.ErrorCredentialCorruptedReset, err, nil)
+	}
+
+	return types.NewAgentError(types.ErrorCredentialServiceUnavailable, err, nil)
+}

internal/credentials/credentials_test.go

@@ -7,6 +7,8 @@ import (
 	"testing"
 
 	"github.com/posit-dev/publisher/internal/logging/loggingtest"
+	"github.com/posit-dev/publisher/internal/types"
+	"github.com/posit-dev/publisher/internal/util"
 	"github.com/posit-dev/publisher/internal/util/utiltest"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/suite"
@@ -124,3 +126,66 @@ func (s *CredentialsServiceTestSuite) TestNewCredentialsService_KeyringErrFallba
 	s.NoError(err)
 	s.Implements((*CredentialsService)(nil), credservice)
 }
+
+func (s *CredentialsServiceTestSuite) TestNewCredentialsService_KeyringResetsOnCorruptedCred() {
+	keyring.MockInit()
+
+	corruptedData := `{"":{"guid":"18cd5640-bee5-4b2a-992a-a2725ab6103d","version":0,"data":"unrecognizbledata"}}`
+	keyring.Set(ServiceName, "credentials", corruptedData)
+
+	keyringCorruptedSave, err := keyring.Get(ServiceName, "credentials")
+	s.NoError(err)
+	s.Equal(keyringCorruptedSave, corruptedData)
+
+	s.log.On("Warn", "Corrupted credentials data found. A reset was applied to stored data to be able to proceed").Return()
+
+	credservice, err := NewCredentialsService(s.log)
+	s.NotNil(err)
+	s.Implements((*CredentialsService)(nil), credservice)
+
+	// Confirm agent error of corrupted credential bubbles up to warn the user
+	_, isCorruptedErr := types.IsAgentErrorOf(err, types.ErrorCredentialCorruptedReset)
+	s.True(isCorruptedErr)
+
+	keyringUpdatedData, err := keyring.Get(ServiceName, "credentials")
+	s.NoError(err)
+	s.Equal(keyringUpdatedData, "{}")
+}
+
+func (s *CredentialsServiceTestSuite) TestNewCredentialsService_FallbackFileResetsOnCorruptedCred() {
+	// Use an in memory filesystem for this test
+	// avoiding to manipulate users ~/.connect-credentials
+	fsys = afero.NewMemMapFs()
+	defer func() { fsys = afero.NewOsFs() }()
+	dirPath, err := util.UserHomeDir(fsys)
+	s.NoError(err)
+
+	dirPath = dirPath.Join(".connect-credentials")
+	corruptedContents := []byte("unrecognizable_key = this is bad")
+	err = afero.WriteFile(fsys, dirPath.String(), corruptedContents, 0644)
+	s.NoError(err)
+
+	keyringErr := errors.New("this is a teapot, unsupported system")
+	keyring.MockInitWithError(keyringErr)
+
+	// Verify the corrupted contents are in
+	credsData, err := afero.ReadFile(fsys, dirPath.String())
+	s.NoError(err)
+	s.Equal(credsData, corruptedContents)
+
+	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()
+	s.log.On("Warn", "Corrupted credentials data found. A reset was applied to stored data to be able to proceed").Return()
+
+	credservice, err := NewCredentialsService(s.log)
+	s.NotNil(err)
+	s.Implements((*CredentialsService)(nil), credservice)
+
+	// Confirm agent error of corrupted credential bubbles up to warn the user
+	_, isCorruptedErr := types.IsAgentErrorOf(err, types.ErrorCredentialCorruptedReset)
+	s.True(isCorruptedErr)
+
+	credsData, err = afero.ReadFile(fsys, dirPath.String())
+	s.NoError(err)
+	s.Equal(credsData, []byte("[credentials]\n"))
+}

internal/credentials/errors.go

@@ -16,6 +16,11 @@ func (e *CorruptedError) Error() string {
 	return fmt.Sprintf("credential '%s' is corrupted", e.GUID)
 }
 
+func (e *CorruptedError) Is(target error) bool {
+	_, isCorrupterErr := target.(*CorruptedError)
+	return isCorrupterErr
+}
+
 type LoadError struct {
 	Err error
 }
@@ -28,6 +33,11 @@ func (e *LoadError) Error() string {
 	return fmt.Sprintf("failed to load credentials: %v", e.Err)
 }
 
+func (e *LoadError) Is(target error) bool {
+	_, isLoadErr := target.(*LoadError)
+	return isLoadErr
+}
+
 type NotFoundError struct {
 	GUID string
 }

internal/credentials/file.go

@@ -93,7 +93,7 @@ func NewFileCredentialsService(log logging.Logger) (*fileCredentialsService, err
 	// Verify file can be modified, will create if not exists
 	err = fservice.setup()
 	if err != nil {
-		return nil, err
+		return fservice, err
 	}
 
 	return fservice, nil
@@ -207,6 +207,11 @@ func (c *fileCredentialsService) Delete(guid string) error {
 	return nil
 }
 
+func (c *fileCredentialsService) Reset() error {
+	newData := newFileCredentials()
+	return c.saveFile(newData)
+}
+
 func (c *fileCredentialsService) setup() error {
 	_, err := c.credsFilepath.Stat()
 	if os.IsNotExist(err) {

internal/credentials/file_test.go

@@ -523,3 +523,31 @@ func (s *FileCredentialsServiceSuite) TestSet_ConflictErr() {
 		})
 	}
 }
+
+func (s *FileCredentialsServiceSuite) TestReset() {
+	cs := &fileCredentialsService{
+		log:           s.loggerMock,
+		credsFilepath: s.testdata.Join("to-reset.toml"),
+	}
+
+	_, err := cs.load()
+	s.NoError(err)
+
+	_, err = cs.Set("newcred", "https://b2.connect-server:3939/connect", "abcdeC2aqbh7dg8TO43XPu7r56YDh002")
+	s.NoError(err)
+
+	_, err = cs.Set("newcredtwo", "https://b5.connect-server:3939/connect", "abcdeC2aqbh7dg8TO43XPu7r56YDh007")
+	s.NoError(err)
+
+	list, err := cs.List()
+	s.NoError(err)
+	s.Len(list, 2)
+
+	err = cs.Reset()
+	s.NoError(err)
+
+	// Creds wiped out
+	list, err = cs.List()
+	s.NoError(err)
+	s.Len(list, 0)
+}

internal/credentials/keyring.go

@@ -127,6 +127,13 @@ func (ks *keyringCredentialsService) Set(name string, url string, ak string) (*C
 	return &cred, nil
 }
 
+// Resets the CredentialTable from keyring
+// it is a last resort in case the keyring data turns out to be irrecognizable
+func (ks *keyringCredentialsService) Reset() error {
+	newTable := make(map[string]CredentialRecord)
+	return ks.save(newTable)
+}
+
 func (ks *keyringCredentialsService) checkForConflicts(
 	table *map[string]CredentialRecord,
 	c *Credential) error {
@@ -159,7 +166,7 @@ func (ks *keyringCredentialsService) save(table CredentialTable) error {
 	return nil
 }
 
-// Loads the CredentialTable from keyRing
+// Loads the CredentialTable from keyring
 func (ks *keyringCredentialsService) load() (CredentialTable, error) {
 	data, err := keyring.Get(ServiceName, "credentials")
 	if err != nil {

internal/credentials/keyring_test.go

@@ -162,3 +162,31 @@ func (s *KeyringCredentialsTestSuite) TestDelete() {
 	s.Error(err)
 	s.log.AssertExpectations(s.T())
 }
+
+func (s *KeyringCredentialsTestSuite) TestReset() {
+	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
+	_, err = cs.Set("example", "https://a.example.com", "12345")
+	s.NoError(err)
+	_, err = cs.Set("example2", "https://b.example.com", "12345")
+	s.NoError(err)
+
+	creds, err = cs.List()
+	s.NoError(err)
+	s.Len(creds, 2)
+
+	err = cs.Reset()
+	s.NoError(err)
+
+	// Creds wiped out
+	creds, err = cs.List()
+	s.NoError(err)
+	s.Len(creds, 0)
+}

internal/credentials/testdata/toml/to-reset.toml

@@ -0,0 +1 @@
+[credentials]

internal/types/error.go

@@ -19,6 +19,7 @@ const (
 	ErrorUnknownTOMLKey               ErrorCode = "unknownTOMLKey"
 	ErrorInvalidConfigFiles           ErrorCode = "invalidConfigFiles"
 	ErrorCredentialServiceUnavailable ErrorCode = "credentialsServiceUnavailable"
+	ErrorCredentialCorruptedReset     ErrorCode = "credentialCorruptedReset"
 	ErrorCertificateVerification      ErrorCode = "errorCertificateVerification"
 	ErrorRenvPackageVersionMismatch   ErrorCode = "renvPackageVersionMismatch"
 	ErrorRenvPackageSourceMissing     ErrorCode = "renvPackageSourceMissing"