Debounce credential prompts and check profile locks in more places (#…

…3480) * refactor: check locks in dataset FS remote lookup; update AuthHandler.waitForUnlock Signed-off-by: Trae Yelovich <[email protected]> * refactor: handle auth prompts for more USS scenarios Signed-off-by: Trae Yelovich <[email protected]> * refactor: use latest profile in more places in FSPs; part 2 to recent bugfix Signed-off-by: Trae Yelovich <[email protected]> * fix: debounce auth prompts during parallel requests Signed-off-by: Trae Yelovich <[email protected]> * feat: useModal optional property for auth prompts Signed-off-by: Trae Yelovich <[email protected]> * fix(AuthHandler): use modal prompt for FSPs Signed-off-by: Trae Yelovich <[email protected]> * refactor: debounce prompts using separate prompt mutexes Signed-off-by: Trae Yelovich <[email protected]> * fix(tests): use fake timers in waitForUnlock test Signed-off-by: Trae Yelovich <[email protected]> * refactor: unlock all profiles after vault changed Signed-off-by: Trae Yelovich <[email protected]> * fix tests; update changelog Signed-off-by: Trae Yelovich <[email protected]> * fix: await AuthHandler.shouldHandleAuthError Signed-off-by: Trae Yelovich <[email protected]> * chore: update changelog Signed-off-by: Trae Yelovich <[email protected]> * wip: AuthHandler & AuthUtils patch coverage Signed-off-by: Trae Yelovich <[email protected]> * fix(AuthHandler): remove unnecessary branch Signed-off-by: Trae Yelovich <[email protected]> * tests: Data Set and USS profile lock tests Signed-off-by: Trae Yelovich <[email protected]> * refactor: use ZoweLogger.warn instead of debug Signed-off-by: Trae Yelovich <[email protected]> * refactor: remove unused useModal param Signed-off-by: Trae Yelovich <[email protected]> * chore: add ZE API changelog Signed-off-by: Trae Yelovich <[email protected]> * tests: fix logic & remove skip on listFiles cases Signed-off-by: Trae Yelovich <[email protected]> * more test cases for profile lock checks Signed-off-by: Trae Yelovich <[email protected]> * chore: fix changelog Signed-off-by: Trae Yelovich <[email protected]> --------- Signed-off-by: Trae Yelovich <[email protected]> Co-authored-by: Billie Simmons <[email protected]>
zowe · Mar 4, 2025 · 8e57200 · 8e57200
1 parent 0643906
commit 8e57200
Show file tree

Hide file tree

Showing 12 changed files with 780 additions and 59 deletions.
diff --git a/packages/zowe-explorer-api/CHANGELOG.md b/packages/zowe-explorer-api/CHANGELOG.md
@@ -14,6 +14,7 @@ All notable changes to the "zowe-explorer-api" extension will be documented in t
 ### Bug fixes
 
 - Fixes an issue where properties of the `TableViewProvider` class were not accessible when the class was extended by developers. [#3456](https://github.com/zowe/zowe-explorer-vscode/pull/3456)
+- Fixed issue where the `AuthHandler.waitForUnlock` function could hang indefinitely if the profile is never unlocked. Now, as a safety measure, the function returns after a 30-second timeout. This function should be used alongside the `AuthHandler.isProfileLocked` function to verify that the profile is unlocked before making API requests. [#3480](https://github.com/zowe/zowe-explorer-vscode/pull/3480)
 
 ## `3.1.1`
 

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
@@ -36,10 +36,14 @@ describe("AuthHandler.enableLocksForType", () => {
 
 describe("AuthHandler.waitForUnlock", () => {
     it("calls Mutex.waitForUnlock if the profile lock is present", async () => {
+        // Used so that `setTimeout` can be invoked from 30sec timeout promise
+        jest.useFakeTimers();
         const mutex = new Mutex();
-        const waitForUnlockMock = jest.spyOn(mutex, "waitForUnlock");
+        const isLockedMock = jest.spyOn(mutex, "isLocked").mockReturnValueOnce(true);
+        const waitForUnlockMock = jest.spyOn(mutex, "waitForUnlock").mockResolvedValueOnce(undefined);
         (AuthHandler as any).profileLocks.set(TEST_PROFILE_NAME, mutex);
         await AuthHandler.waitForUnlock(TEST_PROFILE_NAME);
+        expect(isLockedMock).toHaveBeenCalled();
         expect(waitForUnlockMock).toHaveBeenCalled();
         (AuthHandler as any).profileLocks.clear();
     });
@@ -50,6 +54,23 @@ describe("AuthHandler.waitForUnlock", () => {
     });
 });
 
+describe("AuthHandler.unlockAllProfiles", () => {
+    it("unlocks all profiles in the AuthHandler.profileLocks map", async () => {
+        const mutexAuthPrompt = new Mutex();
+        const mutexProfile = new Mutex();
+        const releaseAuthPromptMutex = jest.spyOn(mutexAuthPrompt, "release");
+        const releaseProfileMutex = jest.spyOn(mutexProfile, "release");
+        (AuthHandler as any).authPromptLocks.set(TEST_PROFILE_NAME, mutexAuthPrompt);
+        (AuthHandler as any).profileLocks.set(TEST_PROFILE_NAME, mutexProfile);
+
+        AuthHandler.unlockAllProfiles();
+        expect(releaseAuthPromptMutex).toHaveBeenCalledTimes(1);
+        expect(releaseProfileMutex).toHaveBeenCalledTimes(1);
+        (AuthHandler as any).authPromptLocks.clear();
+        (AuthHandler as any).profileLocks.clear();
+    });
+});
+
 describe("AuthHandler.isProfileLocked", () => {
     it("returns true if the profile is locked", async () => {
         await AuthHandler.lockProfile(TEST_PROFILE_NAME);
@@ -201,3 +222,12 @@ describe("AuthHandler.unlockProfile", () => {
         expect(reloadWorkspaceMock).toHaveBeenCalledWith(TEST_PROFILE_NAME);
     });
 });
+
+describe("AuthHandler.shouldHandleAuthError", () => {
+    it("returns true if a credential prompt was not yet shown to the user", async () => {
+        await expect(AuthHandler.shouldHandleAuthError(TEST_PROFILE_NAME)).resolves.toBe(true);
+    });
+    it("returns false if the user is currently responding to a credential prompt", async () => {
+        await expect(AuthHandler.shouldHandleAuthError(TEST_PROFILE_NAME)).resolves.toBe(false);
+    });
+});
diff --git a/packages/zowe-explorer-api/src/profiles/AuthHandler.ts b/packages/zowe-explorer-api/src/profiles/AuthHandler.ts
@@ -38,7 +38,8 @@ export type AuthPromptParams = {
 
 export type ProfileLike = string | imperative.IProfileLoaded;
 export class AuthHandler {
-    private static profileLocks: Map<string, Mutex> = new Map();
+    private static authPromptLocks = new Map<string, Mutex>();
+    private static profileLocks = new Map<string, Mutex>();
     private static enabledProfileTypes: Set<string> = new Set(["zosmf"]);
 
     /**
@@ -78,6 +79,7 @@ export class AuthHandler {
      */
     public static unlockProfile(profile: ProfileLike, refreshResources?: boolean): void {
         const profileName = AuthHandler.getProfileName(profile);
+        this.authPromptLocks.get(profileName)?.release();
         const mutex = this.profileLocks.get(profileName);
         // If a mutex doesn't exist for this profile or the mutex is no longer locked, return
         if (mutex == null || !mutex.isLocked()) {
@@ -99,6 +101,26 @@ export class AuthHandler {
         }
     }
 
+    /**
+     * Determines whether to handle an authentication error for a given profile.
+     * This uses a mutex to prevent additional authentication prompts until the first prompt is resolved.
+     * @param profileName The name of the profile to check
+     * @returns {boolean} Whether to handle the authentication error
+     */
+    public static async shouldHandleAuthError(profileName: string): Promise<boolean> {
+        if (!this.authPromptLocks.has(profileName)) {
+            this.authPromptLocks.set(profileName, new Mutex());
+        }
+
+        const mutex = this.authPromptLocks.get(profileName);
+        if (mutex.isLocked()) {
+            return false;
+        }
+
+        await mutex.acquire();
+        return true;
+    }
+
     /**
      * Prompts the user to authenticate over SSO or a credential prompt in the event of an error.
      * @param profile The profile to authenticate
@@ -194,7 +216,43 @@ export class AuthHandler {
             return;
         }
 
-        return this.profileLocks.get(profileName)?.waitForUnlock();
+        const mutex = this.profileLocks.get(profileName);
+        // If the mutex isn't locked, no need to wait
+        if (!mutex.isLocked()) {
+            return;
+        }
+
+        // Wait for the mutex to be unlocked with a timeout to prevent indefinite waiting
+        const timeoutMs = 30000; // 30 seconds timeout
+        const timeoutPromise = new Promise<void>((_, reject) => {
+            setTimeout(() => {
+                reject(new Error(`Timeout waiting for profile ${profileName} to be unlocked`));
+            }, timeoutMs);
+        });
+
+        try {
+            await Promise.race([mutex.waitForUnlock(), timeoutPromise]);
+        } catch (error) {
+            // Log the timeout to console since we don't have access to the logger in the API
+            // This is acceptable as this is just a fallback for an edge case where the user did not respond to a credential prompt in time
+            // eslint-disable-next-line no-console
+            console.log(`Timeout waiting for profile ${profileName} to be unlocked`);
+        }
+    }
+
+    /**
+     * Releases locks for all profiles.
+     * Used for scenarios such as the `onVaultChanged` event, where we don't know what secure values have changed,
+     * but we can't assume that the profile still has invalid credentials.
+     */
+    public static unlockAllProfiles(): void {
+        for (const mutex of this.authPromptLocks.values()) {
+            mutex.release();
+        }
+
+        for (const mutex of this.profileLocks.values()) {
+            mutex.release();
+        }
     }
 
     /**

diff --git a/packages/zowe-explorer/CHANGELOG.md b/packages/zowe-explorer/CHANGELOG.md
@@ -20,13 +20,13 @@ All notable changes to the "vscode-extension-for-zowe" extension will be documen
 - Fixed an issue where user is unable to open a renamed sequential data set from the Data Sets tree view.. [#3345](https://github.com/zowe/zowe-explorer-vscode/issues/3345)
 - Fixed missing z/OS Console icon when `Workbench > Panel: Show Labels` is set to false. [#3293](https://github.com/zowe/zowe-explorer-vscode/issues/3293)
 - Fixed z/OS Console panel background colour to be in sync with the rest of the VS Code styling. [#3445](https://github.com/zowe/zowe-explorer-vscode/pull/3445)
-- Fixed an issue seen with outdated profile information in the z/OS tree view data during upload and download of data set and USS files
-  [#3457](https://github.com/zowe/zowe-explorer-vscode/issues/3457)
+- Fixed an issue seen with outdated profile information in the z/OS tree view data during upload and download of data set and USS files [#3457](https://github.com/zowe/zowe-explorer-vscode/issues/3457)
 - Fixed issue where deleting too many nodes at once would cause the confirmation prompt to be oversized. [#3254](https://github.com/zowe/zowe-explorer-vscode/issues/3254)
 - Fixed an issue with UNIX file edit attributes refresh button not updating/reverting values correctly. [#3238](https://github.com/zowe/zowe-explorer-vscode/issues/3238)
 - Fixed an issue seen where extender favorites not showing in the tree views. [#3470](https://github.com/zowe/zowe-explorer-vscode/issues/3470)
 - Fixed an issue where selecting items in table views would reset the column sort order. [#3473](https://github.com/zowe/zowe-explorer-vscode/issues/3473)
 - Fixed an issue where data set migration status was incorrectly handled when the `migr` attribute was not present in the API response. [#3471](https://github.com/zowe/zowe-explorer-vscode/issues/3471)
+- Fixed issue where users were prompted several times when using a profile with invalid credentials in a VS Code workspace. Now, the user is only prompted once per profile, allowing the user to enter in new credentials. [#3480](https://github.com/zowe/zowe-explorer-vscode/pull/3480)
 
 ## `3.1.1`
 

diff --git a/packages/zowe-explorer/__tests__/__unit__/trees/dataset/DatasetFSProvider.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/trees/dataset/DatasetFSProvider.unit.test.ts
@@ -12,6 +12,7 @@
 import { Disposable, FilePermission, FileSystemError, FileType, TextEditor, Uri } from "vscode";
 import { createIProfile } from "../../../__mocks__/mockCreators/shared";
 import {
+    AuthHandler,
     DirEntry,
     DsEntry,
     DsEntryMetadata,
@@ -30,6 +31,7 @@ import { ZoweExplorerApiRegister } from "../../../../src/extending/ZoweExplorerA
 import { Profiles } from "../../../../src/configuration/Profiles";
 import { AuthUtils } from "../../../../src/utils/AuthUtils";
 import * as path from "path";
+import { ZoweLogger } from "../../../../src/tools/ZoweLogger";
 const dayjs = require("dayjs");
 
 const testProfile = createIProfile();
@@ -94,6 +96,19 @@ describe("createDirectory", () => {
 });
 
 describe("readDirectory", () => {
+    let mockedProperty: MockedProperty;
+    beforeEach(() => {
+        mockedProperty = new MockedProperty(Profiles, "getInstance", {
+            value: jest.fn().mockReturnValue({
+                loadNamedProfile: jest.fn().mockReturnValue(testProfile),
+            } as any),
+        });
+    });
+
+    afterAll(() => {
+        mockedProperty[Symbol.dispose]();
+    });
+
     describe("filter entry (session)", () => {
         it("calls dataSetsMatchingPattern when reading directories if it exists", async () => {
             const mockSessionEntry = { ...testEntries.session, filter: {}, metadata: { profile: testProfile, path: "/" } };
@@ -1240,3 +1255,105 @@ describe("rename", () => {
         _lookupParentDirectoryMock.mockRestore();
     });
 });
+
+describe("Expected behavior for functions w/ profile locks", () => {
+    let isProfileLockedMock;
+    let warnLoggerMock;
+
+    beforeEach(() => {
+        isProfileLockedMock = jest.spyOn(AuthHandler, "isProfileLocked");
+        warnLoggerMock = jest.spyOn(ZoweLogger, "warn").mockImplementation();
+    });
+
+    afterEach(() => {
+        isProfileLockedMock.mockRestore();
+        warnLoggerMock.mockRestore();
+    });
+
+    describe("stat", () => {
+        it("returns entry without API calls when profile is locked", async () => {
+            const fakeEntry = { ...testEntries.ps };
+            const lookupMock = jest.spyOn(DatasetFSProvider.instance, "lookup").mockReturnValueOnce(fakeEntry);
+            const getInfoForUriMock = jest.spyOn(FsAbstractUtils, "getInfoForUri").mockReturnValueOnce({
+                profile: testProfile,
+                isRoot: false,
+                slashAfterProfilePos: testUris.ps.path.indexOf("/", 1),
+                profileName: "sestest",
+            });
+
+            isProfileLockedMock.mockReturnValueOnce(true);
+            const waitForUnlockMock = jest.spyOn(AuthHandler, "waitForUnlock").mockResolvedValueOnce(undefined);
+
+            const datasetMock = jest.fn().mockResolvedValueOnce({});
+            const getMvsApiMock = jest.spyOn(ZoweExplorerApiRegister, "getMvsApi").mockReturnValueOnce({ dataSet: datasetMock } as any);
+
+            const result = await DatasetFSProvider.instance.stat(testUris.ps);
+
+            expect(waitForUnlockMock).toHaveBeenCalledWith(testProfile);
+            expect(isProfileLockedMock).toHaveBeenCalledWith(testProfile);
+            expect(warnLoggerMock).toHaveBeenCalledWith("[DatasetFSProvider] Profile sestest is locked, waiting for authentication");
+            expect(datasetMock).not.toHaveBeenCalled();
+            expect(result).toBe(fakeEntry);
+
+            lookupMock.mockRestore();
+            getInfoForUriMock.mockRestore();
+            waitForUnlockMock.mockRestore();
+            getMvsApiMock.mockRestore();
+        });
+    });
+
+    describe("fetchEntriesForProfile", () => {
+        it("returns early without making API calls when profile is locked", async () => {
+            const fakeEntry = { ...testEntries.session, entries: new Map() };
+            const lookupAsDirectoryMock = jest.spyOn(DatasetFSProvider.instance as any, "_lookupAsDirectory").mockReturnValueOnce(fakeEntry);
+            const uriInfo = { profile: testProfile };
+
+            isProfileLockedMock.mockReturnValueOnce(true);
+            const waitForUnlockMock = jest.spyOn(AuthHandler, "waitForUnlock").mockResolvedValueOnce(undefined);
+
+            const datasetMock = jest.fn().mockResolvedValueOnce({});
+            const getMvsApiMock = jest.spyOn(ZoweExplorerApiRegister, "getMvsApi").mockReturnValueOnce({ dataSet: datasetMock } as any);
+
+            const result = await (DatasetFSProvider.instance as any).fetchEntriesForProfile(testUris.session, uriInfo, "USER.*");
+
+            expect(waitForUnlockMock).toHaveBeenCalledWith(testProfile);
+            expect(isProfileLockedMock).toHaveBeenCalledWith(testProfile);
+            expect(warnLoggerMock).toHaveBeenCalledWith("[DatasetFSProvider] Profile sestest is locked, waiting for authentication");
+            expect(datasetMock).not.toHaveBeenCalled();
+            expect(result).toBe(fakeEntry);
+
+            lookupAsDirectoryMock.mockRestore();
+            waitForUnlockMock.mockRestore();
+            getMvsApiMock.mockRestore();
+        });
+    });
+
+    describe("fetchDatasetAtUri", () => {
+        it("returns null without making API calls when profile is locked", async () => {
+            const file = new DsEntry("TEST.DS", false);
+            file.metadata = new DsEntryMetadata({ profile: testProfile, path: "/TEST.DS" });
+
+            const lookupMock = jest.spyOn(DatasetFSProvider.instance as any, "_lookupAsFile").mockReturnValueOnce(file);
+            const getInfoFromUriMock = jest.spyOn(DatasetFSProvider.instance as any, "_getInfoFromUri").mockReturnValueOnce(file.metadata);
+
+            isProfileLockedMock.mockReturnValueOnce(true);
+            const waitForUnlockMock = jest.spyOn(AuthHandler, "waitForUnlock").mockResolvedValueOnce(undefined);
+
+            const getContentsMock = jest.fn().mockResolvedValueOnce({});
+            const getMvsApiMock = jest.spyOn(ZoweExplorerApiRegister, "getMvsApi").mockReturnValueOnce({ getContents: getContentsMock } as any);
+
+            const result = await DatasetFSProvider.instance.fetchDatasetAtUri(testUris.ps);
+
+            expect(waitForUnlockMock).toHaveBeenCalled();
+            expect(isProfileLockedMock).toHaveBeenCalled();
+            expect(warnLoggerMock).toHaveBeenCalledWith("[DatasetFSProvider] Profile sestest is locked, waiting for authentication");
+            expect(getContentsMock).not.toHaveBeenCalled();
+            expect(result).toBeNull();
+
+            lookupMock.mockRestore();
+            getInfoFromUriMock.mockRestore();
+            waitForUnlockMock.mockRestore();
+            getMvsApiMock.mockRestore();
+        });
+    });
+});