Skip to content

Commit

Permalink
Merge pull request #3111 from zowe/fix/update-credentials-desync
Browse files Browse the repository at this point in the history
Fix credentials not refreshed in ds tree and misc profile related fixes
  • Loading branch information
JillieBeanSim authored Sep 25, 2024
2 parents 3cf3f66 + 409bcd4 commit c1d216b
Show file tree
Hide file tree
Showing 25 changed files with 356 additions and 173 deletions.
2 changes: 2 additions & 0 deletions packages/zowe-explorer-api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ All notable changes to the "zowe-explorer-api" extension will be documented in t
- Deprecated the methods `ZoweVsCodeExtension.loginWithBaseProfile` and `ZoweVsCodeExtension.logoutWithBaseProfile`. Use `ZoweVsCodeExtension.ssoLogin` and `ZoweVsCodeExtension.ssoLogout` instead, which use the `BaseProfileAuthOptions` interface and allow you to choose whether the token value in the base profile should have precedence in case there are conflicts. [#3076](https://github.com/zowe/zowe-explorer-vscode/pull/3076)
- Fixed bug in `ProfilesCache` class where old profiles were still accessible after deleting a Team configuration file. [#3124](https://github.com/zowe/zowe-explorer-vscode/issues/3124)
- Added `extensionRemovedFromPath` private function to the `DsEntry` class to allow removing the extension from a data set before making API calls. [#3121](https://github.com/zowe/zowe-explorer-vscode/issues/3121)
- Deprecated the method `ProfilesCache.updateProfilesArrays`. Use `ProfilesCache.updateCachedProfile` instead, which handles updating credentials cached in memory when `autoStore` is false. [#3120](https://github.com/zowe/zowe-explorer-vscode/issues/3120)

### Bug fixes

- Updated the `TableViewProvider.setTableView` function to show the Zowe Resources panel if a table is provided. If `null` is passed, the Zowe Resources panel will be hidden. [#3113](https://github.com/zowe/zowe-explorer-vscode/issues/3113)
- Fixed behavior of logout action when token is defined in both base profile and parent profile. [#3076](https://github.com/zowe/zowe-explorer-vscode/issues/3076)
- Fixed profile cached by `FileSystemProvider` not refreshing on password change. [#3120](https://github.com/zowe/zowe-explorer-vscode/issues/3120)

## `3.0.0-next.202409132122`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,12 @@ function createProfInfoMock(profiles: Partial<imperative.IProfileLoaded>[]): imp
const teamConfigApi: Partial<imperative.Config> = {
api: {
profiles: {
get: jest.fn().mockReturnValue({}),
get: jest.fn(),
getProfilePathFromName: jest.fn().mockImplementation((x) => x),
},
secure: {
secureFields: jest.fn().mockReturnValue([]),
securePropsForProfile: jest.fn().mockReturnValue([]),
},
} as any,
exists: true,
Expand Down Expand Up @@ -223,6 +224,7 @@ describe("ProfilesCache", () => {
profCache.allProfiles = [lpar1Profile as imperative.IProfileLoaded];
(profCache as any).defaultProfileByType = new Map([["zosmf", { ...profCache.allProfiles[0] }]]);
expect(profCache.allProfiles[0].profile).toMatchObject(lpar1Profile.profile);
// eslint-disable-next-line deprecation/deprecation
profCache.updateProfilesArrays({
...lpar1Profile,
profile: lpar2Profile.profile,
Expand All @@ -231,6 +233,35 @@ describe("ProfilesCache", () => {
expect((profCache as any).defaultProfileByType.get("zosmf").profile).toMatchObject(lpar2Profile.profile);
});

it("updateCachedProfile should refresh all profiles when autoStore is true", async () => {
const profCache = new ProfilesCache(fakeLogger as unknown as imperative.Logger);
jest.spyOn(profCache, "getProfileInfo").mockResolvedValueOnce({
getTeamConfig: jest.fn().mockReturnValue({ properties: { autoStore: true } }),
} as unknown as imperative.ProfileInfo);
const refreshSpy = jest.spyOn(profCache, "refresh").mockImplementation();
await profCache.updateCachedProfile({
...lpar1Profile,
profile: lpar2Profile.profile,
} as imperative.IProfileLoaded);
expect(refreshSpy).toHaveBeenCalledTimes(1);
});

it("updateCachedProfile should update cached profile when autoStore is false", async () => {
const profCache = new ProfilesCache(fakeLogger as unknown as imperative.Logger);
profCache.allProfiles = [lpar1Profile as imperative.IProfileLoaded];
(profCache as any).defaultProfileByType = new Map([["zosmf", { ...profCache.allProfiles[0] }]]);
expect(profCache.allProfiles[0].profile).toMatchObject(lpar1Profile.profile);
jest.spyOn(profCache, "getProfileInfo").mockResolvedValueOnce({
getTeamConfig: jest.fn().mockReturnValue({ properties: { autoStore: false } }),
} as unknown as imperative.ProfileInfo);
await profCache.updateCachedProfile({
...lpar1Profile,
profile: lpar2Profile.profile,
} as imperative.IProfileLoaded);
expect(profCache.allProfiles[0].profile).toMatchObject(lpar2Profile.profile);
expect((profCache as any).defaultProfileByType.get("zosmf").profile).toMatchObject(lpar2Profile.profile);
});

it("getDefaultProfile should find default profile given type", () => {
const profCache = new ProfilesCache(fakeLogger as unknown as imperative.Logger);
(profCache as any).defaultProfileByType = new Map([["zosmf", lpar1Profile]]);
Expand Down Expand Up @@ -566,18 +597,18 @@ describe("ProfilesCache", () => {
expect(profile).toMatchObject({ name: "lpar1", type: "base" });
});

it("fetchBaseProfile should return typeless profile if base profile does not contain token type", async () => {
it("fetchBaseProfile should return typeless profile if base profile does not contain token value", async () => {
const profCache = new ProfilesCache(fakeLogger as unknown as imperative.Logger);
jest.spyOn(profCache, "getProfileInfo").mockResolvedValue(createProfInfoMock([baseProfile]));
const profile = await profCache.fetchBaseProfile("lpar1.zosmf");
expect(profile).toMatchObject({ name: "lpar1", type: "base" });
});

it("fetchBaseProfile should return base profile if it contains token type", async () => {
it("fetchBaseProfile should return base profile if it contains token value", async () => {
const profCache = new ProfilesCache(fakeLogger as unknown as imperative.Logger);
const profInfoMock = createProfInfoMock([baseProfile]);
jest.spyOn(profCache, "getProfileInfo").mockResolvedValue(profInfoMock);
mocked(profInfoMock.getTeamConfig().api.profiles.get).mockReturnValueOnce({ tokenType: imperative.SessConstants.TOKEN_TYPE_JWT });
mocked(profInfoMock.getTeamConfig().api.secure.securePropsForProfile).mockReturnValue(["tokenValue"]);
const profile = await profCache.fetchBaseProfile("lpar1.zosmf");
expect(profile).toMatchObject(baseProfile);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import * as vscode from "vscode";
import { ZoweTreeNode } from "../../../src/tree/ZoweTreeNode";
import { IZoweTreeNode } from "../../../src/tree/IZoweTreeNode";
import * as imperative from "@zowe/imperative";
import { BaseProvider } from "../../../src";

describe("ZoweTreeNode", () => {
const makeNode = (
Expand Down Expand Up @@ -79,4 +80,32 @@ describe("ZoweTreeNode", () => {
expect(node.getProfile()).toBeUndefined();
expect(node.getProfileName()).toBeUndefined();
});

it("setProfileToChoice should update properties on existing profile object", () => {
const node = makeNode("test", vscode.TreeItemCollapsibleState.None, undefined, undefined, {
name: "oldProfile",
profile: { host: "example.com" },
});
node.setProfileToChoice({ name: "newProfile", profile: { host: "example.com", port: 443 } } as unknown as imperative.IProfileLoaded);
// Profile name should not change but properties should
expect(node.getProfileName()).toBe("oldProfile");
expect(node.getProfile().profile?.port).toBeDefined();
});

it("setProfileToChoice should update profile for associated FSProvider entry", () => {
const node = makeNode("test", vscode.TreeItemCollapsibleState.None, undefined);
const fsEntry = {
metadata: {
profile: { name: "oldProfile" },
},
};
node.setProfileToChoice(
{ name: "newProfile" } as unknown as imperative.IProfileLoaded,
{
lookup: jest.fn().mockReturnValue(fsEntry),
} as unknown as BaseProvider
);
expect(node.getProfileName()).toBe("newProfile");
expect(fsEntry.metadata.profile.name).toBe("newProfile");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ describe("ZoweVsCodeExtension", () => {
mProfileSchemaCache: new Map(),
readProfilesFromDisk: jest.fn(),
});
testCache.allProfiles = [serviceProfile, baseProfile];
jest.spyOn(testCache, "getProfileInfo").mockResolvedValue(testProfInfo);

return {
Expand Down Expand Up @@ -338,6 +339,9 @@ describe("ZoweVsCodeExtension", () => {
};

const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation((items) => items[0]);
await ZoweVsCodeExtension.profilesCache.refresh({
registeredApiTypes: jest.fn().mockReturnValue(["service"]),
} as unknown as Types.IApiRegisterClient);
await ZoweVsCodeExtension.ssoLogin({ serviceProfile: "lpar.service" });

const testSession = new imperative.Session(JSON.parse(JSON.stringify(blockMocks.expectedSession.ISession)));
Expand Down Expand Up @@ -541,9 +545,6 @@ describe("ZoweVsCodeExtension", () => {

jest.spyOn(ZoweVsCodeExtension as any, "promptUserPass").mockResolvedValue(["user", "pass"]);
const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation((items) => items[0]);
await ZoweVsCodeExtension.profilesCache.refresh({
registeredApiTypes: jest.fn().mockReturnValue(["service"]),
} as unknown as Types.IApiRegisterClient);
const didLogin = await ZoweVsCodeExtension.ssoLogin({
serviceProfile: serviceProfileLoaded,
defaultTokenType: "apimlAuthenticationToken",
Expand Down Expand Up @@ -633,6 +634,9 @@ describe("ZoweVsCodeExtension", () => {

const testSpy = jest.spyOn(ZoweVsCodeExtension as any, "promptUserPass").mockResolvedValue(["abc", "def"]);
const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation((items) => items[0]);
await ZoweVsCodeExtension.profilesCache.refresh({
registeredApiTypes: jest.fn().mockReturnValue(["service"]),
} as unknown as Types.IApiRegisterClient);
const didLogin = await ZoweVsCodeExtension.ssoLogin({
serviceProfile: serviceProfileLoaded,
defaultTokenType: "apimlAuthenticationToken",
Expand Down Expand Up @@ -686,6 +690,7 @@ describe("ZoweVsCodeExtension", () => {
updateProperty: mockUpdateProperty,
}),
refresh: jest.fn(),
updateCachedProfile: jest.fn(),
});
const showInputBoxSpy = jest.spyOn(Gui, "showInputBox").mockResolvedValueOnce("fakeUser").mockResolvedValueOnce("fakePassword");
const saveCredentialsSpy = jest.spyOn(ZoweVsCodeExtension as any, "saveCredentials");
Expand All @@ -711,6 +716,7 @@ describe("ZoweVsCodeExtension", () => {
updateProperty: mockUpdateProperty,
}),
refresh: jest.fn(),
updateCachedProfile: jest.fn(),
});
const showInputBoxSpy = jest.spyOn(Gui, "showInputBox").mockResolvedValueOnce("fakeUser").mockResolvedValueOnce("fakePassword");
const saveCredentialsSpy = jest.spyOn(ZoweVsCodeExtension as any, "saveCredentials");
Expand Down Expand Up @@ -739,6 +745,7 @@ describe("ZoweVsCodeExtension", () => {
updateProperty: mockUpdateProperty,
}),
refresh: jest.fn(),
updateCachedProfile: jest.fn(),
});
const showInputBoxSpy = jest.spyOn(Gui, "showInputBox").mockResolvedValueOnce("fakeUser").mockResolvedValueOnce("fakePassword");
jest.spyOn(Gui, "showMessage").mockResolvedValueOnce("yes");
Expand All @@ -765,6 +772,7 @@ describe("ZoweVsCodeExtension", () => {
updateProperty: mockUpdateProperty,
}),
refresh: jest.fn(),
updateCachedProfile: jest.fn(),
});
const showInputBoxSpy = jest.spyOn(Gui, "showInputBox").mockResolvedValueOnce("fakeUser").mockResolvedValueOnce("fakePassword");
jest.spyOn(Gui, "showMessage").mockResolvedValueOnce(undefined);
Expand Down
30 changes: 26 additions & 4 deletions packages/zowe-explorer-api/src/profiles/ProfilesCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { Validation } from "./Validation";
import { ZosmfProfile } from "@zowe/zosmf-for-zowe-sdk";
import { ZosTsoProfile } from "@zowe/zos-tso-for-zowe-sdk";
import { ZosUssProfile } from "@zowe/zos-uss-for-zowe-sdk";
import { Types } from "../Types";

export class ProfilesCache {
public profilesForValidation: Validation.IValidationProfile[] = [];
Expand Down Expand Up @@ -79,9 +80,8 @@ export class ProfilesCache {

/**
* Updates profile in allProfiles array and if default updates defaultProfileByType
*
* @deprecated Use `updateCachedProfile` instead
* @param {string} profileLoaded
*
* @returns {void}
*/
public updateProfilesArrays(profileLoaded: imperative.IProfileLoaded): void {
Expand All @@ -97,6 +97,25 @@ export class ProfilesCache {
}
}

public async updateCachedProfile(
profileLoaded: imperative.IProfileLoaded,
profileNode?: Types.IZoweNodeType,
zeRegister?: Types.IApiRegisterClient
): Promise<void> {
if ((await this.getProfileInfo()).getTeamConfig().properties.autoStore) {
await this.refresh(zeRegister);
} else {
// Note: When autoStore is disabled, nested profiles within this service profile may not have their credentials updated.
const profIndex = this.allProfiles.findIndex((profile) => profile.type === profileLoaded.type && profile.name === profileLoaded.name);
this.allProfiles[profIndex].profile = profileLoaded.profile;
const defaultProf = this.defaultProfileByType.get(profileLoaded.type);
if (defaultProf != null && defaultProf.name === profileLoaded.name) {
this.defaultProfileByType.set(profileLoaded.type, profileLoaded);
}
}
profileNode?.setProfileToChoice(profileLoaded);
}

/**
* This returns default profile by type from defaultProfileByType
*
Expand Down Expand Up @@ -319,11 +338,14 @@ export class ProfilesCache {
const mProfileInfo = await this.getProfileInfo();
const baseProfileAttrs = mProfileInfo.getDefaultProfile("base");
const config = mProfileInfo.getTeamConfig();
if (profileName?.includes(".") && (baseProfileAttrs == null || config.api.profiles.get(baseProfileAttrs.profName).tokenType == null)) {
if (
profileName?.includes(".") &&
(baseProfileAttrs == null || !config.api.secure.securePropsForProfile(baseProfileAttrs.profName).includes("tokenValue"))
) {
// Retrieve parent typeless profile as base profile if:
// (1) The active profile name is nested (contains a period) AND
// (2) No default base profile was found OR
// Default base profile does not have tokenType defined
// Default base profile does not have tokenValue in secure array
const parentProfile = this.getParentProfileForToken(profileName, config);
return this.getProfileLoaded(parentProfile, "base", config.api.profiles.get(parentProfile));
} else if (baseProfileAttrs == null) {
Expand Down
14 changes: 12 additions & 2 deletions packages/zowe-explorer-api/src/tree/ZoweTreeNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import * as vscode from "vscode";
import * as imperative from "@zowe/imperative";
import { IZoweTreeNode } from "./IZoweTreeNode";
import type { BaseProvider } from "../fs/BaseProvider";

/**
* Common implementation of functions and methods associated with the
Expand Down Expand Up @@ -99,8 +100,17 @@ export class ZoweTreeNode extends vscode.TreeItem {
*
* @param {imperative.IProfileLoaded} The profile you will set the node to use
*/
public setProfileToChoice(aProfile: imperative.IProfileLoaded): void {
this.profile = aProfile;
public setProfileToChoice(aProfile: imperative.IProfileLoaded, fsProvider?: BaseProvider): void {
if (this.profile == null) {
this.profile = aProfile;
} else {
// Don't reassign profile, we want to keep object reference shared across nodes
this.profile.profile = aProfile.profile;
}
const fsEntry = fsProvider?.lookup(this.resourceUri, true);
if (fsEntry != null) {
fsEntry.metadata.profile = aProfile;
}
}
/**
* Sets the session for this node to the one chosen in parameters.
Expand Down
25 changes: 4 additions & 21 deletions packages/zowe-explorer-api/src/vscode/ZoweVsCodeExtension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export class ZoweVsCodeExtension {
options: PromptCredentialsOptions.ComplexOptions,
apiRegister: Types.IApiRegisterClient
): Promise<imperative.IProfileLoaded> {
const cache = this.profilesCache;
const cache = options.zeProfiles ?? ZoweVsCodeExtension.profilesCache;
const profInfo = await cache.getProfileInfo();
const setSecure = options.secure ?? profInfo.isSecured();

Expand Down Expand Up @@ -101,7 +101,7 @@ export class ZoweVsCodeExtension {
await profInfo.updateProperty({ ...upd, property: "user", value: creds[0], setSecure });
await profInfo.updateProperty({ ...upd, property: "password", value: creds[1], setSecure });
}
await cache.refresh(apiRegister);
await cache.updateCachedProfile(loadProfile, undefined, apiRegister);

return loadProfile;
}
Expand Down Expand Up @@ -215,7 +215,7 @@ export class ZoweVsCodeExtension {

await cache.updateBaseProfileFileLogin(profileToUpdate, updBaseProfile, !connOk);
serviceProfile.profile = { ...serviceProfile.profile, ...updBaseProfile };
await this.updateProfileInCache({ ...opts, serviceProfile });
await cache.updateCachedProfile(serviceProfile, opts.profileNode);
return true;
}

Expand Down Expand Up @@ -278,27 +278,10 @@ export class ZoweVsCodeExtension {
!serviceProfile.name.startsWith(baseProfile.name + ".");
await cache.updateBaseProfileFileLogout(connOk ? baseProfile : serviceProfile);
serviceProfile.profile = { ...serviceProfile.profile, tokenType: undefined, tokenValue: undefined };
await this.updateProfileInCache({ ...opts, serviceProfile });
await cache.updateCachedProfile(serviceProfile, opts.profileNode);
return true;
}

private static async updateProfileInCache(opts: BaseProfileAuthOptions & { serviceProfile: imperative.IProfileLoaded }): Promise<void> {
const cache: ProfilesCache = opts.zeProfiles ?? ZoweVsCodeExtension.profilesCache;
if ((await cache.getProfileInfo()).getTeamConfig().properties.autoStore !== false) {
await cache.refresh();
} else {
// Note: It should be expected that nested profiles within this service profile will have their credentials updated.
const profIndex = cache.allProfiles.findIndex((profile) => profile.name === opts.serviceProfile.name);
cache.allProfiles[profIndex] = { ...cache.allProfiles[profIndex], profile: opts.serviceProfile.profile };
}
if (opts.profileNode) {
opts.profileNode.setProfileToChoice({
...opts.profileNode.getProfile(),
profile: { ...opts.profileNode.getProfile().profile, ...opts.serviceProfile.profile },
});
}
}

/**
* This method is intended to be used for authentication (login, logout) purposes
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import * as imperative from "@zowe/imperative";
import { InputBoxOptions, OpenDialogOptions } from "vscode";
import { ProfilesCache } from "../../profiles";

export namespace PromptCredentialsOptions {
export interface CommonOptions {
Expand All @@ -24,6 +25,7 @@ export namespace PromptCredentialsOptions {
sessionName?: string;
sessionType?: string;
secure?: boolean;
zeProfiles?: ProfilesCache;
}

export interface UserPassOptions extends CommonOptions {
Expand Down
Loading

0 comments on commit c1d216b

Please sign in to comment.