refactor: Lock profiles during async operations (phase 1: filesystem) (…

…#3370) * move DeferredPromise to API Signed-off-by: Trae Yelovich <[email protected]> * wip: AuthHandler Signed-off-by: Trae Yelovich <[email protected]> * refactor: 'promptForAuthOnError' -> 'lockProfileOnAuthError' Signed-off-by: Trae Yelovich <[email protected]> * wip: reload workspaces once reauthenticated - note: may need to throttle amount of times the reload is called, if you have a workspace with hundreds of folders it will make a list request for each one Signed-off-by: Trae Yelovich <[email protected]> * wip: lock in other FS providers; fix update credentials logic Signed-off-by: Trae Yelovich <[email protected]> * wip(tests): resolve multiple failing tests Signed-off-by: Trae Yelovich <[email protected]> * tests: resolve failing ProfilesUtils cases Signed-off-by: Trae Yelovich <[email protected]> * wip: rename AuthUtils method, update FS providers Signed-off-by: Trae Yelovich <[email protected]> * refactor: Mutex class & use in AuthHandler; TSDoc - Adds a new `Mutex` class that wraps around a `DeferredPromise`. See the `@brief` for details. - Refactors the `AuthHandler` class to use a map of `Mutex` for each profile used in critical sections. - Added doc: `AuthHandler, DeferredPromise, Mutex` Signed-off-by: Trae Yelovich <[email protected]> * add reloadActiveEditorForProfile and related logic Signed-off-by: Trae Yelovich <[email protected]> * expose Mutex, fix failing tests Signed-off-by: Trae Yelovich <[email protected]> * chore: fix unused warnings in test code Signed-off-by: Trae Yelovich <[email protected]> * chore: fix lint and formatting errors - removed `no-return-await` lint rule since the extra microtask was removed in recent versions of Node, so there is now a benefit to using `return await` - add test `zowe.config.json` to prettierignore to avoid invalid JSON errors Signed-off-by: Trae Yelovich <[email protected]> * tests: DeferredPromise, Mutex Signed-off-by: Trae Yelovich <[email protected]> * chore: rename test file in API to match source file Signed-off-by: Trae Yelovich <[email protected]> * remove unused import Signed-off-by: Trae Yelovich <[email protected]> * refactor: use async-mutex instead of homemade mutex Signed-off-by: Trae Yelovich <[email protected]> * refactor: move fs helper fns, isLocked -> isProfileLocked Signed-off-by: Trae Yelovich <[email protected]> * wip: AuthHandler tests Signed-off-by: Trae Yelovich <[email protected]> * wip: patch coverage for AuthHandler, DeferredPromise Signed-off-by: Trae Yelovich <[email protected]> * refactor: lockedProfiles -> profileLocks, patch cov in ProfilesUtils Signed-off-by: Trae Yelovich <[email protected]> * tests: FileManagement functions Signed-off-by: Trae Yelovich <[email protected]> * remove unused var 'credsEntered' Signed-off-by: Trae Yelovich <[email protected]> * refactor: save decorator for phase 2 (API changes) Signed-off-by: Trae Yelovich <[email protected]> * tests: AuthHandler SSO login during auth prompt Signed-off-by: Trae Yelovich <[email protected]> * add test for basic creds, move logic for updating trees Signed-off-by: Trae Yelovich <[email protected]> * fix lint errors Signed-off-by: Trae Yelovich <[email protected]> * tests: AuthHandler.updateTreeProvidersWithProfile Signed-off-by: Trae Yelovich <[email protected]> * chore: add changelogs Signed-off-by: Trae Yelovich <[email protected]> * fix: refresh resources if updated in 'Manage Profile' option Signed-off-by: Trae Yelovich <[email protected]> * refactor: use enum for DeferredPromise status Signed-off-by: Trae Yelovich <[email protected]> * refactor: use onProfileUpdate event to update tree providers Signed-off-by: Trae Yelovich <[email protected]> * fix(AuthHandler): remove redundant includes check Signed-off-by: Trae Yelovich <[email protected]> * refactor: combine nested ifs into single expression Signed-off-by: Trae Yelovich <[email protected]> * refactor: remove return in AuthHandler.lockProfile Signed-off-by: Trae Yelovich <[email protected]> * refactor: DeferredPromiseStatus.{Fulfilled -> Resolved} Signed-off-by: Trae Yelovich <[email protected]> * refactor: Clean up auth prompt options, adjust interfaces Signed-off-by: Trae Yelovich <[email protected]> * fix: add missing prompt; return expected boolean value Signed-off-by: Trae Yelovich <[email protected]> * fix(ds): remove extra 401 message during fetch Signed-off-by: Trae Yelovich <[email protected]> --------- Signed-off-by: Trae Yelovich <[email protected]>
zowe · Jan 7, 2025 · 28d5a66 · 28d5a66
1 parent 08f4323
commit 28d5a66
Show file tree

Hide file tree

Showing 34 changed files with 792 additions and 142 deletions.
diff --git a/.eslintrc.yaml b/.eslintrc.yaml
@@ -97,7 +97,7 @@ rules:
     - warn
     - "ignore": [-2, -1, 0, 1, 2, 4]
   no-multiple-empty-lines: warn
-  no-return-await: warn
+  no-return-await: off
   no-sequences: warn
   no-shadow: off
   no-sparse-arrays: warn

diff --git a/.prettierignore b/.prettierignore
@@ -9,4 +9,5 @@ node_modules
 pnpm-lock.yaml
 **/bundle.l10n.json
 **/.wdio-vscode-service
+packages/zowe-explorer/__tests__/**/zowe.config.json
 zedc/target
diff --git a/packages/zowe-explorer-api/CHANGELOG.md b/packages/zowe-explorer-api/CHANGELOG.md
@@ -12,6 +12,7 @@ All notable changes to the "zowe-explorer-api" extension will be documented in t
 - Added support for VS Code proxy settings with zosmf profile types. [#3010](https://github.com/zowe/zowe-explorer-vscode/issues/3010)
 - Added optional `getLocalStorage` function to the `IApiExplorerExtender` interface to expose local storage access to Zowe Explorer extenders. [#3180](https://github.com/zowe/zowe-explorer-vscode/issues/3180)
 - Added optional `setEncoding`, `getEncoding`, and `getEncodingInMap` functions to the `IZoweJobTreeNode` interface. [#3361](https://github.com/zowe/zowe-explorer-vscode/pull/3361)
+- Added an `AuthHandler` class with functions for locking/unlocking profiles, prompting for credentials and SSO login support. Extenders can now lock profiles after an authentication error, ensuring that an invalid profile is not used asynchronously until the error is resolved. [#3329](https://github.com/zowe/zowe-explorer-vscode/issues/3329)
 
 ### Bug fixes
 

diff --git a/packages/zowe-explorer-api/__mocks__/vscode.ts b/packages/zowe-explorer-api/__mocks__/vscode.ts
@@ -456,6 +456,8 @@ export namespace window {
         close: jest.fn(),
     };
 
+    export let activeTextEditor: TextDocument | undefined = { fileName: "placeholderFile.txt" } as any;
+
     /**
      * Show an information message to users. Optionally provide an array of items which will be presented as
      * clickable buttons.
@@ -1096,6 +1098,7 @@ export class FileSystemError extends Error {
  */
 export namespace workspace {
     export const textDocuments: TextDocument[] = [];
+    export const workspaceFolders: readonly WorkspaceFolder[] | undefined = [];
     export function getConfiguration(_configuration: string) {
         return {
             update: () => {

diff --git a/packages/zowe-explorer-api/__tests__/__unit__/profiles/AuthHandler.unit.test.ts b/packages/zowe-explorer-api/__tests__/__unit__/profiles/AuthHandler.unit.test.ts
@@ -0,0 +1,159 @@
+/**
+ * This program and the accompanying materials are made available under the terms of the
+ * Eclipse Public License v2.0 which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-v20.html
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Copyright Contributors to the Zowe Project.
+ *
+ */
+
+import { Mutex } from "async-mutex";
+import { AuthHandler, Gui } from "../../../src";
+import { FileManagement } from "../../../src/utils/FileManagement";
+import { ImperativeError } from "@zowe/imperative";
+import { AuthPromptParams } from "@zowe/zowe-explorer-api";
+
+const TEST_PROFILE_NAME = "lpar.zosmf";
+
+describe("AuthHandler.isProfileLocked", () => {
+    it("returns true if the profile is locked", async () => {
+        await AuthHandler.lockProfile(TEST_PROFILE_NAME);
+        expect(AuthHandler.isProfileLocked(TEST_PROFILE_NAME)).toBe(true);
+        AuthHandler.unlockProfile(TEST_PROFILE_NAME);
+    });
+
+    it("returns false if the profile is not locked", async () => {
+        expect(AuthHandler.isProfileLocked(TEST_PROFILE_NAME)).toBe(false);
+    });
+
+    it("returns false if no mutex is present for the given profile", async () => {
+        expect(AuthHandler.isProfileLocked("unused_lpar.zosmf")).toBe(false);
+    });
+});
+
+describe("AuthHandler.lockProfile", () => {
+    it("assigns and acquires a Mutex to the profile in the profile map", async () => {
+        await AuthHandler.lockProfile(TEST_PROFILE_NAME);
+        expect((AuthHandler as any).profileLocks.has(TEST_PROFILE_NAME)).toBe(true);
+        expect((AuthHandler as any).profileLocks.get(TEST_PROFILE_NAME)).toBeInstanceOf(Mutex);
+        AuthHandler.unlockProfile(TEST_PROFILE_NAME);
+    });
+
+    it("handle promptForAuthentication call if error and options are given", async () => {
+        const promptForAuthenticationMock = jest.spyOn(AuthHandler, "promptForAuthentication").mockResolvedValueOnce(true);
+        const imperativeError = new ImperativeError({ msg: "Example auth error" });
+        const authOpts: AuthPromptParams = {
+            authMethods: {
+                promptCredentials: jest.fn(),
+                ssoLogin: jest.fn(),
+            },
+            imperativeError,
+        };
+        const releaseSpy = jest.spyOn(Mutex.prototype, "release");
+        const result = await AuthHandler.lockProfile(TEST_PROFILE_NAME, authOpts);
+        expect(result).toBe(true);
+        expect(promptForAuthenticationMock).toHaveBeenCalledTimes(1);
+        expect(promptForAuthenticationMock).toHaveBeenCalledWith(TEST_PROFILE_NAME, authOpts);
+        expect(releaseSpy).toHaveBeenCalledTimes(1);
+        AuthHandler.unlockProfile(TEST_PROFILE_NAME);
+    });
+
+    it("reuses the same Mutex for the profile if it already exists", async () => {
+        await AuthHandler.lockProfile(TEST_PROFILE_NAME);
+        expect((AuthHandler as any).profileLocks.has(TEST_PROFILE_NAME)).toBe(true);
+        // cache initial mutex for comparison
+        const mutex = (AuthHandler as any).profileLocks.get(TEST_PROFILE_NAME);
+        expect(mutex).toBeInstanceOf(Mutex);
+        AuthHandler.unlockProfile(TEST_PROFILE_NAME);
+
+        // same mutex is still present in map since lock/unlock sequence was used
+        await AuthHandler.lockProfile(TEST_PROFILE_NAME);
+        expect(mutex).toBe((AuthHandler as any).profileLocks.get(TEST_PROFILE_NAME));
+        AuthHandler.unlockProfile(TEST_PROFILE_NAME);
+    });
+});
+
+describe("AuthHandler.promptForAuthentication", () => {
+    it("handles a token-based authentication error - login successful, profile is string", async () => {
+        const tokenNotValidMsg = "Token is not valid or expired.";
+        const imperativeError = new ImperativeError({ additionalDetails: tokenNotValidMsg, msg: tokenNotValidMsg });
+        const ssoLogin = jest.fn().mockResolvedValue(true);
+        const promptCredentials = jest.fn();
+        const showMessageMock = jest.spyOn(Gui, "showMessage").mockResolvedValueOnce("Log in to Authentication Service");
+        const unlockProfileSpy = jest.spyOn(AuthHandler, "unlockProfile");
+        await expect(
+            AuthHandler.promptForAuthentication("lpar.zosmf", { authMethods: { promptCredentials, ssoLogin }, imperativeError })
+        ).resolves.toBe(true);
+        expect(promptCredentials).not.toHaveBeenCalled();
+        expect(ssoLogin).toHaveBeenCalledTimes(1);
+        expect(ssoLogin).toHaveBeenCalledWith(null, "lpar.zosmf");
+        expect(unlockProfileSpy).toHaveBeenCalledTimes(1);
+        expect(unlockProfileSpy).toHaveBeenCalledWith("lpar.zosmf", true);
+        expect(showMessageMock).toHaveBeenCalledTimes(1);
+    });
+
+    it("handles a standard authentication error - credentials provided, profile is string", async () => {
+        const tokenNotValidMsg = "Invalid credentials";
+        const imperativeError = new ImperativeError({ additionalDetails: tokenNotValidMsg, msg: tokenNotValidMsg });
+        const ssoLogin = jest.fn().mockResolvedValue(true);
+        const promptCredentials = jest.fn().mockResolvedValue(["us3r", "p4ssw0rd"]);
+        const errorMessageMock = jest.spyOn(Gui, "errorMessage").mockResolvedValueOnce("Update Credentials");
+        const unlockProfileSpy = jest.spyOn(AuthHandler, "unlockProfile").mockClear();
+        await expect(
+            AuthHandler.promptForAuthentication("lpar.zosmf", { authMethods: { promptCredentials, ssoLogin }, imperativeError })
+        ).resolves.toBe(true);
+        expect(unlockProfileSpy).toHaveBeenCalledTimes(1);
+        expect(unlockProfileSpy).toHaveBeenCalledWith("lpar.zosmf", true);
+        expect(ssoLogin).not.toHaveBeenCalled();
+        expect(errorMessageMock).toHaveBeenCalledTimes(1);
+        expect(promptCredentials).toHaveBeenCalledTimes(1);
+        expect(promptCredentials).toHaveBeenCalledWith("lpar.zosmf", true);
+    });
+});
+
+describe("AuthHandler.unlockProfile", () => {
+    it("releases the Mutex for the profile in the profile map", async () => {
+        await AuthHandler.lockProfile(TEST_PROFILE_NAME);
+        AuthHandler.unlockProfile(TEST_PROFILE_NAME);
+        expect((AuthHandler as any).profileLocks.get(TEST_PROFILE_NAME)!.isLocked()).toBe(false);
+    });
+
+    it("does nothing if there is no mutex in the profile map", async () => {
+        const releaseSpy = jest.spyOn(Mutex.prototype, "release").mockClear();
+        AuthHandler.unlockProfile("unused_lpar.zosmf");
+        expect(releaseSpy).not.toHaveBeenCalled();
+    });
+
+    it("does nothing if the mutex in the map is not locked", async () => {
+        await AuthHandler.lockProfile(TEST_PROFILE_NAME);
+        AuthHandler.unlockProfile(TEST_PROFILE_NAME);
+
+        const releaseSpy = jest.spyOn(Mutex.prototype, "release").mockClear();
+        AuthHandler.unlockProfile(TEST_PROFILE_NAME);
+        expect(releaseSpy).not.toHaveBeenCalled();
+    });
+
+    it("reuses the same Mutex for the profile if it already exists", async () => {
+        await AuthHandler.lockProfile(TEST_PROFILE_NAME);
+        AuthHandler.unlockProfile(TEST_PROFILE_NAME);
+        expect((AuthHandler as any).profileLocks.has(TEST_PROFILE_NAME)).toBe(true);
+        // cache initial mutex for comparison
+        const mutex = (AuthHandler as any).profileLocks.get(TEST_PROFILE_NAME);
+
+        // same mutex is still present in map since lock/unlock sequence was used
+        await AuthHandler.lockProfile(TEST_PROFILE_NAME);
+        AuthHandler.unlockProfile(TEST_PROFILE_NAME);
+        expect(mutex).toBe((AuthHandler as any).profileLocks.get(TEST_PROFILE_NAME));
+    });
+
+    it("refreshes resources if refreshResources parameter is true", async () => {
+        const reloadActiveEditorMock = jest.spyOn(FileManagement, "reloadActiveEditorForProfile").mockResolvedValueOnce(undefined);
+        const reloadWorkspaceMock = jest.spyOn(FileManagement, "reloadWorkspacesForProfile").mockResolvedValueOnce(undefined);
+        await AuthHandler.lockProfile(TEST_PROFILE_NAME);
+        AuthHandler.unlockProfile(TEST_PROFILE_NAME, true);
+        expect(reloadActiveEditorMock).toHaveBeenCalledWith(TEST_PROFILE_NAME);
+        expect(reloadWorkspaceMock).toHaveBeenCalledWith(TEST_PROFILE_NAME);
+    });
+});
diff --git a/packages/zowe-explorer-api/__tests__/__unit__/utils/DeferredPromise.unit.test.ts b/packages/zowe-explorer-api/__tests__/__unit__/utils/DeferredPromise.unit.test.ts
@@ -0,0 +1,47 @@
+/**
+ * This program and the accompanying materials are made available under the terms of the
+ * Eclipse Public License v2.0 which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-v20.html
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Copyright Contributors to the Zowe Project.
+ *
+ */
+
+import { DeferredPromise, DeferredPromiseStatus } from "../../../src";
+
+describe("DeferredPromise constructor", () => {
+    it("sets resolve and reject functions", () => {
+        const deferred = new DeferredPromise();
+        expect(deferred.promise).toBeInstanceOf(Promise);
+        expect(deferred.reject).toBeInstanceOf(Function);
+        expect(deferred.resolve).toBeInstanceOf(Function);
+    });
+});
+
+describe("DeferredPromise.status", () => {
+    it("returns pending when not yet resolved", () => {
+        const deferred = new DeferredPromise();
+        expect(deferred.status).toBe(DeferredPromiseStatus.Pending);
+    });
+
+    it("returns resolved when resolved", () => {
+        const deferred = new DeferredPromise();
+        deferred.resolve(null);
+        expect(deferred.status).toBe(DeferredPromiseStatus.Resolved);
+    });
+
+    it("returns rejected when rejected", async () => {
+        const deferred = new DeferredPromise();
+        let errorCaught = false;
+        setImmediate(() => deferred.reject());
+        try {
+            await deferred.promise;
+        } catch (err) {
+            errorCaught = true;
+        }
+        expect(deferred.status).toBe(DeferredPromiseStatus.Rejected);
+        expect(errorCaught).toBe(true);
+    });
+});
diff --git a/packages/zowe-explorer-api/__tests__/__unit__/utils/FileManagement.unit.test.ts b/packages/zowe-explorer-api/__tests__/__unit__/utils/FileManagement.unit.test.ts
@@ -0,0 +1,111 @@
+/**
+ * This program and the accompanying materials are made available under the terms of the
+ * Eclipse Public License v2.0 which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-v20.html
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Copyright Contributors to the Zowe Project.
+ *
+ */
+
+import { FileSystemError, FileType, Uri, window, workspace } from "vscode";
+import { FileManagement } from "../../../src/utils/FileManagement";
+import { IFileSystemEntry, ZoweScheme } from "../../../src";
+
+describe("permStringToOctal", () => {
+    it("converts drwxrwxrwx to 777", () => {
+        expect(FileManagement.permStringToOctal("drwxrwxrwx")).toBe(777);
+    });
+
+    it("converts d--------- to 0", () => {
+        expect(FileManagement.permStringToOctal("d---------")).toBe(0);
+    });
+
+    it("converts drwxr-xr-x to 755", () => {
+        expect(FileManagement.permStringToOctal("drwxr-xr-x")).toBe(755);
+    });
+
+    it("converts -rwxrwxrwx to 777", () => {
+        expect(FileManagement.permStringToOctal("-rwxrwxrwx")).toBe(777);
+    });
+});
+
+describe("reloadActiveEditorForProfile", () => {
+    it("calls workspace.fs.{readFile,stat} to reload contents of editor", async () => {
+        const fakeFsEntry: IFileSystemEntry = {
+            name: "exampleFile.txt",
+            wasAccessed: true,
+            type: FileType.Directory,
+            metadata: {
+                path: "/sestest/exampleFolder/exampleFile.txt",
+                profile: {
+                    name: "sestest",
+                    message: "",
+                    type: "zosmf",
+                    failNotFound: true,
+                },
+            },
+            ctime: Date.now() - 10,
+            mtime: Date.now(),
+            size: 123,
+        };
+        const fileUri = Uri.from({ scheme: ZoweScheme.USS, path: "/sestest/exampleFolder/exampleFile.txt" });
+        const activeTextEditorMock = jest.replaceProperty(window, "activeTextEditor", {
+            document: {
+                fileName: "exampleFile.txt",
+                uri: fileUri,
+            } as any,
+        } as any);
+        const statMock = jest.spyOn(workspace.fs, "stat").mockResolvedValueOnce(fakeFsEntry);
+        const readFileMock = jest.spyOn(workspace.fs, "readFile").mockImplementationOnce(async (uri): Promise<Uint8Array> => {
+            // wasAccessed flag should be false after reassigning in reloadActiveEditorForProfile
+            expect(fakeFsEntry.wasAccessed).toBe(false);
+            return new Uint8Array([1, 2, 3]);
+        });
+        await FileManagement.reloadActiveEditorForProfile("sestest");
+        expect(statMock).toHaveBeenCalledTimes(1);
+        expect(statMock).toHaveBeenCalledWith(fileUri);
+        expect(readFileMock).toHaveBeenCalledTimes(1);
+        expect(readFileMock).toHaveBeenCalledWith(fileUri);
+        activeTextEditorMock.restore();
+    });
+});
+
+describe("reloadWorkspacesForProfile", () => {
+    it("calls workspace.fs.stat with fetch=true for each workspace folder", async () => {
+        const folderUri = Uri.from({ scheme: ZoweScheme.USS, path: "/sestest/exampleFolder" });
+        const workspaceFoldersMock = jest.replaceProperty(workspace, "workspaceFolders", [
+            {
+                uri: folderUri,
+                name: "exampleFolder",
+                index: 0,
+            },
+        ]);
+        const statMock = jest
+            .spyOn(workspace.fs, "stat")
+            .mockClear()
+            .mockResolvedValueOnce(undefined as any);
+        await FileManagement.reloadWorkspacesForProfile("sestest");
+        expect(statMock).toHaveBeenCalledTimes(1);
+        expect(statMock).toHaveBeenCalledWith(folderUri.with({ query: "fetch=true" }));
+        workspaceFoldersMock.restore();
+    });
+    it("calls console.error in event of an error", async () => {
+        const folderUri = Uri.from({ scheme: ZoweScheme.USS, path: "/sestest/exampleFolder" });
+        const workspaceFoldersMock = jest.replaceProperty(workspace, "workspaceFolders", [
+            {
+                uri: folderUri,
+                name: "exampleFolder",
+                index: 0,
+            },
+        ]);
+        const statMock = jest.spyOn(workspace.fs, "stat").mockClear().mockRejectedValueOnce(FileSystemError.FileNotFound(folderUri));
+        const consoleErrorMock = jest.spyOn(console, "error").mockImplementationOnce(() => {});
+        await FileManagement.reloadWorkspacesForProfile("sestest");
+        expect(statMock).toHaveBeenCalledTimes(1);
+        expect(statMock).toHaveBeenCalledWith(folderUri.with({ query: "fetch=true" }));
+        expect(consoleErrorMock).toHaveBeenCalledWith("reloadWorkspacesForProfile:", "file not found");
+        workspaceFoldersMock.restore();
+    });
+});
diff --git a/packages/zowe-explorer-api/__tests__/__unit__/utils/files.unit.test.ts b/packages/zowe-explorer-api/__tests__/__unit__/utils/files.unit.test.ts