diff --git a/extensions/vscode/src/state.ts b/extensions/vscode/src/state.ts index b5e5aa357..e9d788b04 100644 --- a/extensions/vscode/src/state.ts +++ b/extensions/vscode/src/state.ts @@ -19,6 +19,10 @@ import { getStatusFromError, getSummaryStringFromError, } from "src/utils/errors"; +import { + isErrCredentialsReset, + errCredentialsResetMessage, +} from "src/utils/errorTypes"; import { DeploymentSelector, SelectionState } from "src/types/shared"; import { LocalState, Views } from "./constants"; @@ -283,6 +287,11 @@ export class PublisherState implements Disposable { this.credentials = response.data; }); } catch (error: unknown) { + if (isErrCredentialsReset(error)) { + const warnMsg = errCredentialsResetMessage(); + window.showWarningMessage(warnMsg); + return; + } const summary = getSummaryStringFromError("refreshCredentials", error); window.showErrorMessage(summary); } diff --git a/extensions/vscode/src/utils/errorTypes.ts b/extensions/vscode/src/utils/errorTypes.ts index 5902eef56..e447e7063 100644 --- a/extensions/vscode/src/utils/errorTypes.ts +++ b/extensions/vscode/src/utils/errorTypes.ts @@ -17,7 +17,8 @@ export type ErrorCode = | "deployedContentNotRunning" | "tomlValidationError" | "tomlUnknownError" - | "pythonExecNotFound"; + | "pythonExecNotFound" + | "credentialCorruptedReset"; export type axiosErrorWithJson = AxiosError & { @@ -162,6 +163,15 @@ export type ErrInvalidConfigFiles = MkErrorDataType< export const isErrInvalidConfigFile = mkErrorTypeGuard("invalidConfigFile"); +// Invalid configuration file(s) +export type ErrCredentialsReset = MkErrorDataType<"credentialCorruptedReset">; +export const isErrCredentialsReset = mkErrorTypeGuard( + "credentialCorruptedReset", +); +export const errCredentialsResetMessage = () => { + return "Stored credentials for Posit Publisher found in an unrecognizable state. A reset was required in order to proceed."; +}; + // Tries to match an Axios error that comes with an identifiable Json structured data // defaulting to be ErrUnknown message when export function resolveAgentJsonErrorMsg(err: axiosErrorWithJson) { @@ -185,5 +195,11 @@ export function resolveAgentJsonErrorMsg(err: axiosErrorWithJson) { return errPythonExecNotFoundErrorMessage(err); } + // Ignore errors coming from credentials being reset, + // a warning is shown when PublisherState is updated. + if (isErrPythonExecNotFoundError(err)) { + return errPythonExecNotFoundErrorMessage(err); + } + return errUnknownMessage(err as axiosErrorWithJson); } diff --git a/internal/accounts/provider_credentials.go b/internal/accounts/provider_credentials.go index 276307906..1c16d0fb1 100644 --- a/internal/accounts/provider_credentials.go +++ b/internal/accounts/provider_credentials.go @@ -5,6 +5,7 @@ package accounts import ( "github.com/posit-dev/publisher/internal/credentials" "github.com/posit-dev/publisher/internal/logging" + "github.com/posit-dev/publisher/internal/types" ) type CredentialsProvider struct { @@ -14,7 +15,11 @@ type CredentialsProvider struct { func NewCredentialsProvider(log logging.Logger) (*CredentialsProvider, error) { cs, err := credentials.NewCredentialsService(log) if err != nil { - return nil, err + // Ignore errors from credentials reset at this point + _, isCredsReset := types.IsAgentErrorOf(err, types.ErrorCredentialCorruptedReset) + if !isCredsReset { + return nil, err + } } return &CredentialsProvider{cs}, nil diff --git a/internal/credentials/credentials.go b/internal/credentials/credentials.go index 8dd9ddada..615e933d3 100644 --- a/internal/credentials/credentials.go +++ b/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) +} diff --git a/internal/credentials/credentials_test.go b/internal/credentials/credentials_test.go index 6e35e68f9..6710089e5 100644 --- a/internal/credentials/credentials_test.go +++ b/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")) +} diff --git a/internal/credentials/errors.go b/internal/credentials/errors.go index bd967b8a1..8e0536923 100644 --- a/internal/credentials/errors.go +++ b/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 } diff --git a/internal/credentials/file.go b/internal/credentials/file.go index d4f2f9b88..fb483e8df 100644 --- a/internal/credentials/file.go +++ b/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) { diff --git a/internal/credentials/file_test.go b/internal/credentials/file_test.go index 78435482f..bc1bbc777 100644 --- a/internal/credentials/file_test.go +++ b/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) +} diff --git a/internal/credentials/keyring.go b/internal/credentials/keyring.go index bcced33cc..2210c0703 100644 --- a/internal/credentials/keyring.go +++ b/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 { diff --git a/internal/credentials/keyring_test.go b/internal/credentials/keyring_test.go index 15a2e7564..013b1b3a7 100644 --- a/internal/credentials/keyring_test.go +++ b/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) +} diff --git a/internal/credentials/testdata/toml/to-reset.toml b/internal/credentials/testdata/toml/to-reset.toml new file mode 100644 index 000000000..5bbe47685 --- /dev/null +++ b/internal/credentials/testdata/toml/to-reset.toml @@ -0,0 +1 @@ +[credentials] diff --git a/internal/services/api/get_credentials.go b/internal/services/api/get_credentials.go index 24280cf1c..633204d71 100644 --- a/internal/services/api/get_credentials.go +++ b/internal/services/api/get_credentials.go @@ -21,6 +21,11 @@ func GetCredentialsHandlerFunc(log logging.Logger) http.HandlerFunc { apiErr.JSONResponse(w) return } + if aerr.Code == types.ErrorCredentialCorruptedReset { + apiErr := types.APIErrorCredentialCorruptedResetFromAgentError(*aerr) + apiErr.JSONResponse(w) + return + } } InternalError(w, req, log, err) return diff --git a/internal/types/api_errors.go b/internal/types/api_errors.go index 03a2435ea..aa1cc814d 100644 --- a/internal/types/api_errors.go +++ b/internal/types/api_errors.go @@ -162,6 +162,20 @@ func APIErrorCredentialsUnavailableFromAgentError(aerr AgentError) APIErrorCrede } } +type APIErrorCredentialCorruptedReset struct { + Code ErrorCode `json:"code"` +} + +func (apierr *APIErrorCredentialCorruptedReset) JSONResponse(w http.ResponseWriter) { + jsonResult(w, http.StatusConflict, apierr) +} + +func APIErrorCredentialCorruptedResetFromAgentError(aerr AgentError) APIErrorCredentialCorruptedReset { + return APIErrorCredentialCorruptedReset{ + Code: ErrorCredentialCorruptedReset, + } +} + type APIErrorPythonExecNotFound struct { Code ErrorCode `json:"code"` } diff --git a/internal/types/api_errors_test.go b/internal/types/api_errors_test.go index e59488887..1d0c3ac80 100644 --- a/internal/types/api_errors_test.go +++ b/internal/types/api_errors_test.go @@ -88,3 +88,23 @@ func (s *ApiErrorsSuite) TestAPIErrorPythonExecNotFoundFromAgentError() { s.Equal(http.StatusUnprocessableEntity, rec.Result().StatusCode) s.Contains(bodyRes, `{"code":"pythonExecNotFound"}`) } + +func (s *ApiErrorsSuite) TestAPIErrorCredentialCorruptedResetFromAgentError() { + agentErr := AgentError{ + Message: "", + Code: ErrorCredentialCorruptedReset, + Err: errors.New("unknown field error"), + Data: ErrorData{}, + } + + rec := httptest.NewRecorder() + + apiError := APIErrorCredentialCorruptedResetFromAgentError(agentErr) + s.Equal(apiError.Code, ErrorCredentialCorruptedReset) + + apiError.JSONResponse(rec) + + bodyRes := rec.Body.String() + s.Equal(http.StatusConflict, rec.Result().StatusCode) + s.Contains(bodyRes, `{"code":"credentialCorruptedReset"}`) +} diff --git a/internal/types/error.go b/internal/types/error.go index 0af9943e4..705f9d240 100644 --- a/internal/types/error.go +++ b/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"