Merge pull request #2504 from posit-dev/mnv-creds-reset

Reset corrupted credentials storage when applicable

extensions/vscode/src/api/resources/Credentials.ts

@@ -46,6 +46,13 @@ export class Credentials {
     return this.client.delete(`credentials/${guid}`);
   }
 
+  // Returns:
+  // 204 - success (no response)
+  // 503 - credentials service unavailable
+  reset() {
+    return this.client.delete<{ backupFile: string }>(`credentials`);
+  }
+
   // Returns:
   // 200 - with possible results in TestResult object
   // for URL only: no user or error in TestResult

extensions/vscode/src/state.test.ts

@@ -8,6 +8,7 @@ import {
   selectionStateFactory,
   preContentRecordFactory,
   configurationFactory,
+  credentialFactory,
 } from "src/test/unit-test-utils/factories";
 import { mkExtensionContextStateMock } from "src/test/unit-test-utils/vscode-mocks";
 import { LocalState } from "./constants";
@@ -27,6 +28,7 @@ class mockApiClient {
 
   readonly credentials = {
     list: vi.fn(),
+    reset: vi.fn(),
   };
 }
 
@@ -39,6 +41,14 @@ vi.mock("src/api", async (importOriginal) => {
   };
 });
 
+vi.mock("src/utils/progress", () => {
+  return {
+    showProgress: vi.fn((_title, _view: string, until: () => Promise<void>) => {
+      return until();
+    }),
+  };
+});
+
 vi.mock("vscode", () => {
   // mock Disposable
   const disposableMock = vi.fn();
@@ -47,6 +57,7 @@ vi.mock("vscode", () => {
   // mock window
   const windowMock = {
     showErrorMessage: vi.fn(),
+    showWarningMessage: vi.fn(),
     showInformationMessage: vi.fn(),
   };
 
@@ -385,6 +396,127 @@ describe("PublisherState", () => {
     });
   });
 
+  describe("refreshCredentials", () => {
+    test("Calls to fetch credentials and stores to state", async () => {
+      const fakeCredsFetch = credentialFactory.buildList(3);
+      mockClient.credentials.list.mockResolvedValue({
+        data: fakeCredsFetch,
+      });
+
+      const { mockContext } = mkExtensionContextStateMock({});
+      const publisherState = new PublisherState(mockContext);
+
+      // Creds are empty initially
+      expect(publisherState.credentials).toEqual([]);
+
+      // Populated with API results
+      await publisherState.refreshCredentials();
+      expect(publisherState.credentials).toEqual(fakeCredsFetch);
+    });
+
+    describe("errors", () => {
+      test("api error - not corrupted data", async () => {
+        const axiosErr = new AxiosError();
+        axiosErr.response = {
+          data: "this is terrible",
+          status: 500,
+          statusText: "500",
+          headers: {},
+          config: { headers: new AxiosHeaders() },
+        };
+        mockClient.credentials.list.mockRejectedValueOnce(axiosErr);
+
+        const { mockContext } = mkExtensionContextStateMock({});
+        const publisherState = new PublisherState(mockContext);
+
+        // Creds are empty initially
+        expect(publisherState.credentials).toEqual([]);
+
+        // Creds are still empty
+        await publisherState.refreshCredentials();
+        expect(publisherState.credentials).toEqual([]);
+
+        // Error mssage is processed
+        expect(window.showErrorMessage).toHaveBeenCalledWith(
+          "this is terrible",
+        );
+      });
+
+      test("api error - corrupted data - defers to reset creds", async () => {
+        const axiosErr = new AxiosError();
+        axiosErr.response = {
+          data: {
+            code: "credentialsCorrupted",
+          },
+          status: 409,
+          statusText: "409",
+          headers: {},
+          config: { headers: new AxiosHeaders() },
+        };
+        mockClient.credentials.list.mockRejectedValue(axiosErr);
+        mockClient.credentials.reset.mockResolvedValue({});
+
+        const { mockContext } = mkExtensionContextStateMock({});
+        const publisherState = new PublisherState(mockContext);
+
+        // Creds are empty initially
+        expect(publisherState.credentials).toEqual([]);
+
+        // Creds are still empty
+        await publisherState.refreshCredentials();
+        expect(publisherState.credentials).toEqual([]);
+
+        // Error message is not called
+        expect(window.showErrorMessage).not.toHaveBeenCalled();
+
+        // Calls to reset
+        expect(mockClient.credentials.reset).toHaveBeenCalled();
+      });
+    });
+  });
+
+  describe("resetCredentials", () => {
+    test("calls to reset credentials and shows a warning", async () => {
+      mockClient.credentials.reset.mockImplementation(() => {
+        mockClient.credentials.list.mockResolvedValue({ data: [] });
+        return { data: { backupFile: "backup-file" } };
+      });
+
+      const { mockContext } = mkExtensionContextStateMock({});
+      const publisherState = new PublisherState(mockContext);
+
+      publisherState.credentials = credentialFactory.buildList(3);
+      await publisherState.resetCredentials();
+
+      expect(publisherState.credentials).toEqual([]);
+
+      // Warning message is called
+      expect(window.showWarningMessage).toHaveBeenCalledWith(
+        "Unrecognizable credentials for Posit Publisher were found and removed. Credentials may need to be recreated. Previous credentials data backed up at backup-file",
+      );
+    });
+
+    test("on api error - shows message", async () => {
+      const axiosErr = new AxiosError();
+      axiosErr.response = {
+        data: "terrible, could not reset",
+        status: 500,
+        statusText: "500",
+        headers: {},
+        config: { headers: new AxiosHeaders() },
+      };
+      mockClient.credentials.reset.mockRejectedValueOnce(axiosErr);
+
+      const { mockContext } = mkExtensionContextStateMock({});
+      const publisherState = new PublisherState(mockContext);
+
+      await publisherState.resetCredentials();
+      expect(window.showErrorMessage).toHaveBeenCalledWith(
+        "terrible, could not reset",
+      );
+    });
+  });
+
   test.todo("getSelectedConfiguration", () => {});
 
   test.todo("refreshContentRecords", () => {});
@@ -394,6 +526,4 @@ describe("PublisherState", () => {
   test.todo("validConfigs", () => {});
 
   test.todo("configsInError", () => {});
-
-  test.todo("refreshCredentials", () => {});
 });

extensions/vscode/src/state.ts

@@ -19,6 +19,10 @@ import {
   getStatusFromError,
   getSummaryStringFromError,
 } from "src/utils/errors";
+import {
+  isErrCredentialsCorrupted,
+  errCredentialsCorruptedMessage,
+} from "src/utils/errorTypes";
 import { DeploymentSelector, SelectionState } from "src/types/shared";
 import { LocalState, Views } from "./constants";
 
@@ -283,11 +287,32 @@ export class PublisherState implements Disposable {
         this.credentials = response.data;
       });
     } catch (error: unknown) {
+      if (isErrCredentialsCorrupted(error)) {
+        this.resetCredentials();
+        return;
+      }
       const summary = getSummaryStringFromError("refreshCredentials", error);
       window.showErrorMessage(summary);
     }
   }
 
+  // Calls to reset all credentials data stored.
+  // Meant to be a last resort when we get loading data or corrupted data errors.
+  async resetCredentials() {
+    try {
+      const api = await useApi();
+      const response = await api.credentials.reset();
+      const warnMsg = errCredentialsCorruptedMessage(response.data.backupFile);
+      window.showWarningMessage(warnMsg);
+
+      const listResponse = await api.credentials.list();
+      this.credentials = listResponse.data;
+    } catch (err: unknown) {
+      const summary = getSummaryStringFromError("resetCredentials", err);
+      window.showErrorMessage(summary);
+    }
+  }
+
   findCredential(name: string) {
     return findCredential(name, this.credentials);
   }

extensions/vscode/src/test/unit-test-utils/factories.ts

@@ -8,6 +8,7 @@ import {
   ContentRecordState,
 } from "src/api/types/contentRecords";
 import { ContentType, Configuration } from "src/api/types/configurations";
+import { Credential } from "src/api/types/credentials";
 import { DeploymentSelectorState } from "src/types/shared";
 
 export const selectionStateFactory = Factory.define<DeploymentSelectorState>(
@@ -85,3 +86,10 @@ export const contentRecordFactory = Factory.define<ContentRecord>(
     configurationRelPath: `report/path/configuration-${sequence}`,
   }),
 );
+
+export const credentialFactory = Factory.define<Credential>(({ sequence }) => ({
+  guid: `44a468b8-09c7-4c6d-a7a3-8cf164ddbaf${sequence}`,
+  name: `Credential ${sequence}`,
+  url: `https://connect.${sequence}.site.com/connect`,
+  apiKey: `qwerty-${sequence}`,
+}));

extensions/vscode/src/utils/errorTypes.ts

@@ -17,7 +17,8 @@ export type ErrorCode =
   | "deployedContentNotRunning"
   | "tomlValidationError"
   | "tomlUnknownError"
-  | "pythonExecNotFound";
+  | "pythonExecNotFound"
+  | "credentialsCorrupted";
 
 export type axiosErrorWithJson<T = { code: ErrorCode; details: unknown }> =
   AxiosError & {
@@ -162,6 +163,19 @@ export type ErrInvalidConfigFiles = MkErrorDataType<
 export const isErrInvalidConfigFile =
   mkErrorTypeGuard<ErrInvalidConfigFiles>("invalidConfigFile");
 
+// Invalid configuration file(s)
+export type ErrCredentialsCorrupted = MkErrorDataType<"credentialsCorrupted">;
+export const isErrCredentialsCorrupted =
+  mkErrorTypeGuard<ErrCredentialsCorrupted>("credentialsCorrupted");
+export const errCredentialsCorruptedMessage = (backupFile: string) => {
+  let msg =
+    "Unrecognizable credentials for Posit Publisher were found and removed. Credentials may need to be recreated.";
+  if (backupFile) {
+    msg += ` Previous credentials data backed up at ${backupFile}`;
+  }
+  return msg;
+};
+
 // 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) {

internal/accounts/provider_credentials.go

@@ -3,6 +3,8 @@
 package accounts
 
 import (
+	"errors"
+
 	"github.com/posit-dev/publisher/internal/credentials"
 	"github.com/posit-dev/publisher/internal/logging"
 )
@@ -11,9 +13,15 @@ type CredentialsProvider struct {
 	cs credentials.CredentialsService
 }
 
+// We can ignore errors related to malformed data on the initial loader
+// API consumers handle resetting malformed data when needed.
+func errIsNotLoadError(err error) bool {
+	return err != nil && !errors.Is(err, &credentials.LoadError{}) && !errors.Is(err, &credentials.CorruptedError{})
+}
+
 func NewCredentialsProvider(log logging.Logger) (*CredentialsProvider, error) {
 	cs, err := credentials.NewCredentialsService(log)
-	if err != nil {
+	if errIsNotLoadError(err) {
 		return nil, err
 	}
 

internal/credentials/credentials.go

@@ -33,7 +33,6 @@ import (
 	"encoding/json"
 
 	"github.com/posit-dev/publisher/internal/logging"
-	"github.com/posit-dev/publisher/internal/types"
 )
 
 const ServiceName = "Posit Publisher Safe Storage"
@@ -81,11 +80,14 @@ func (cr *CredentialRecord) ToCredential() (*Credential, error) {
 	}
 }
 
+type CredServiceFactory = func(log logging.Logger) (CredentialsService, error)
+
 type CredentialsService interface {
 	Delete(guid string) error
 	Get(guid string) (*Credential, error)
 	List() ([]Credential, error)
 	Set(name string, url string, ak string) (*Credential, error)
+	Reset() (string, error)
 }
 
 // The main credentials service constructor that determines if the system's keyring is available to be used,
@@ -99,7 +101,7 @@ func NewCredentialsService(log logging.Logger) (CredentialsService, error) {
 	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, err
 	}
 
 	return fcService, nil

internal/credentials/credentialstest/mock_credentials.go

@@ -0,0 +1,42 @@
+package credentialstest
+
+// Copyright (C) 2024 by Posit Software, PBC.
+
+import (
+	"github.com/stretchr/testify/mock"
+
+	"github.com/posit-dev/publisher/internal/credentials"
+)
+
+type CredentialsServiceMock struct {
+	mock.Mock
+}
+
+func NewCredentialsServiceMock() *CredentialsServiceMock {
+	return &CredentialsServiceMock{}
+}
+
+func (m *CredentialsServiceMock) Delete(guid string) error {
+	args := m.Called(guid)
+	return args.Error(0)
+}
+
+func (m *CredentialsServiceMock) Get(guid string) (*credentials.Credential, error) {
+	args := m.Called(guid)
+	return args.Get(0).(*credentials.Credential), args.Error(1)
+}
+
+func (m *CredentialsServiceMock) List() ([]credentials.Credential, error) {
+	args := m.Called()
+	return args.Get(0).([]credentials.Credential), args.Error(1)
+}
+
+func (m *CredentialsServiceMock) Set(name string, url string, ak string) (*credentials.Credential, error) {
+	args := m.Called(name, url, ak)
+	return args.Get(0).(*credentials.Credential), args.Error(1)
+}
+
+func (m *CredentialsServiceMock) Reset() (string, error) {
+	args := m.Called()
+	return args.Get(0).(string), args.Error(1)
+}