Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(v3): Support logging in to multiple API ML instances #3019

Merged
merged 17 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/zowe-explorer-api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ All notable changes to the "zowe-explorer-api" extension will be documented in t

- Implemented support for building, exposing and displaying table views within Zowe Explorer. Tables can be customized and exposed using the helper facilities (`TableBuilder` and `TableMediator`) for an extender's specific use case. For more information on how to configure and show tables, please refer to the [wiki article on Table Views](https://github.com/zowe/zowe-explorer-vscode/wiki/Table-Views). [#2258](https://github.com/zowe/zowe-explorer-vscode/issues/2258)
- **Breaking:** Consolidated WebView API options into a single object (`WebViewOpts` type), both for developer convenience and to support future options.
- Enhanced the `ZoweVsCodeExtension.loginWithBaseProfile` and `ZoweVsCodeExtension.logoutWithBaseProfile` methods to store SSO token in parent profile when nested profiles are in use. [#2264](https://github.com/zowe/zowe-explorer-vscode/issues/2264)
- **Next Breaking:** Changed return type of `ZoweVsCodeExtension.logoutWithBaseProfile` method from `void` to `boolean` to indicate whether logout was successful.
t1m0thyj marked this conversation as resolved.
Show resolved Hide resolved

### Bug fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import * as fs from "fs";
import * as imperative from "@zowe/imperative";
import { ProfilesCache } from "../../../src/profiles/ProfilesCache";
import { FileManagement, Types } from "../../../src";
import { mocked } from "../../../__mocks__/mockUtils";

jest.mock("fs");

Expand Down Expand Up @@ -70,7 +71,7 @@ const baseProfileWithToken = {
tokenValue: "baseToken",
},
};
const profilemetadata: imperative.ICommandProfileTypeConfiguration[] = [
const profileMetadata: imperative.ICommandProfileTypeConfiguration[] = [
{
type: "acme",
schema: {
Expand All @@ -83,6 +84,19 @@ const profilemetadata: imperative.ICommandProfileTypeConfiguration[] = [
];

function createProfInfoMock(profiles: Partial<imperative.IProfileLoaded>[]): imperative.ProfileInfo {
const teamConfigApi: Partial<imperative.Config> = {
api: {
profiles: {
get: jest.fn(),
getProfilePathFromName: jest.fn().mockImplementation((x) => x),
},
secure: {
secureFields: jest.fn().mockReturnValue([]),
securePropsForProfile: jest.fn().mockReturnValue([]),
},
} as any,
exists: true,
};
return {
getAllProfiles: (profType?: string) =>
profiles
Expand Down Expand Up @@ -113,7 +127,7 @@ function createProfInfoMock(profiles: Partial<imperative.IProfileLoaded>[]): imp
knownArgs: Object.entries(profile.profile as object).map(([k, v]) => ({ argName: k, argValue: v as unknown })),
};
},
getTeamConfig: () => ({ exists: true }),
getTeamConfig: () => teamConfigApi,
updateProperty: jest.fn(),
updateKnownProperty: jest.fn(),
isSecured: jest.fn(),
Expand Down Expand Up @@ -157,16 +171,16 @@ describe("ProfilesCache", () => {

it("addToConfigArray should set the profileTypeConfigurations array", () => {
const profCache = new ProfilesCache(fakeLogger as unknown as imperative.Logger);
profilemetadata.push(profilemetadata[0]);
profCache.addToConfigArray(profilemetadata);
expect(profCache.profileTypeConfigurations).toEqual(profilemetadata.filter((a, index) => index == 0));
profileMetadata.push(profileMetadata[0]);
profCache.addToConfigArray(profileMetadata);
expect(profCache.profileTypeConfigurations).toEqual(profileMetadata.filter((a, index) => index == 0));
});

it("getConfigArray should return the data of profileTypeConfigurations Array", () => {
const profCache = new ProfilesCache(fakeLogger as unknown as imperative.Logger);
profCache.profileTypeConfigurations = profilemetadata;
profCache.profileTypeConfigurations = profileMetadata;
const res = profCache.getConfigArray();
expect(res).toEqual(profilemetadata);
expect(res).toEqual(profileMetadata);
});

it("loadNamedProfile should find profiles by name and type", () => {
Expand Down Expand Up @@ -537,6 +551,38 @@ describe("ProfilesCache", () => {
expect(profile).toMatchObject(baseProfile);
});

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

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 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.secure.securePropsForProfile).mockReturnValue(["tokenValue"]);
const profile = await profCache.fetchBaseProfile("lpar1.zosmf");
expect(profile).toMatchObject(baseProfile);
});

it("fetchBaseProfile should return typeless profile up one level if it contains token value", async () => {
const profCache = new ProfilesCache(fakeLogger as unknown as imperative.Logger);
const profInfoMock = createProfInfoMock([]);
jest.spyOn(profCache, "getProfileInfo").mockResolvedValue(profInfoMock);
mocked(profInfoMock.getTeamConfig().api.secure.secureFields).mockReturnValue(["sysplex1.properties.tokenValue"]);
const profile = await profCache.fetchBaseProfile("sysplex1.lpar1.zosmf");
expect(profile).toMatchObject({ name: "sysplex1", type: "base" });
});

it("fetchBaseProfile should return undefined if base profile not found", async () => {
const profCache = new ProfilesCache(fakeLogger as unknown as imperative.Logger);
jest.spyOn(profCache, "getProfileInfo").mockResolvedValue(createProfInfoMock([lpar1Profile]));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ describe("ZoweVsCodeExtension", () => {
allProfiles,
allExternalTypes: [],
fetchBaseProfile: jest.fn(),
loadNamedProfile: jest.fn().mockReturnValue({ profile: testProfile }),
loadNamedProfile: jest.fn().mockReturnValue(serviceProfile),
updateBaseProfileFileLogin: jest.fn(),
updateBaseProfileFileLogout: jest.fn(),
getLoadedProfConfig: jest.fn().mockReturnValue({ profile: {} }),
Expand All @@ -139,16 +139,20 @@ describe("ZoweVsCodeExtension", () => {
});

it("should not login if the base profile cannot be fetched", async () => {
const errorMessageSpy = jest.spyOn(Gui, "errorMessage");
testCache.fetchBaseProfile.mockResolvedValue(null);
await ZoweVsCodeExtension.loginWithBaseProfile("service");
expect(testCache.fetchBaseProfile).toHaveBeenCalledTimes(1);
expect(testCache.updateBaseProfileFileLogin).not.toHaveBeenCalled();
expect(errorMessageSpy).toHaveBeenCalledWith(expect.stringContaining("Login failed: No base profile found"));
});
it("should not logout if the base profile cannot be fetched", async () => {
const errorMessageSpy = jest.spyOn(Gui, "errorMessage");
testCache.fetchBaseProfile.mockResolvedValue(null);
await ZoweVsCodeExtension.logoutWithBaseProfile("service");
expect(testCache.fetchBaseProfile).toHaveBeenCalledTimes(1);
expect(testCache.updateBaseProfileFileLogin).not.toHaveBeenCalled();
expect(errorMessageSpy).toHaveBeenCalledWith(expect.stringContaining("Logout failed: No base profile found"));
});

describe("user and password chosen", () => {
Expand All @@ -175,7 +179,7 @@ describe("ZoweVsCodeExtension", () => {
it("should logout using the base profile given a simple profile name", async () => {
testCache.fetchBaseProfile.mockResolvedValue(baseProfile);
const testSpy = jest.spyOn(ZoweVsCodeExtension as any, "getServiceProfileForAuthPurposes");
testSpy.mockResolvedValue({ profile: { ...testProfile, ...updProfile } });
testSpy.mockResolvedValue({ ...serviceProfile, profile: { ...testProfile, ...updProfile } });
const logoutSpy = jest.spyOn(Logout, "apimlLogout").mockImplementation(jest.fn());

const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation((items) => items[0]);
Expand Down Expand Up @@ -250,6 +254,51 @@ describe("ZoweVsCodeExtension", () => {
);
quickPickMock.mockRestore();
});
it("should login using the parent profile given a nested profile name", async () => {
const tempBaseProfile = JSON.parse(JSON.stringify(baseProfile));
tempBaseProfile.name = "lpar";
tempBaseProfile.profile.tokenType = "some-dummy-token-type";
testCache.fetchBaseProfile.mockResolvedValue(tempBaseProfile);
const testSpy = jest.spyOn(ZoweVsCodeExtension as any, "getServiceProfileForAuthPurposes");
const newServiceProfile = {
...serviceProfile,
name: "lpar.service",
profile: { ...testProfile, tokenValue: "tokenValue", host: "dummy" },
};
testSpy.mockResolvedValue(newServiceProfile);
jest.spyOn(ZoweVsCodeExtension as any, "promptUserPass").mockResolvedValue(["user", "pass"]);
const loginSpy = jest.spyOn(Login, "apimlLogin").mockResolvedValue("tokenValue");

const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation((items) => items[0]);
await ZoweVsCodeExtension.loginWithBaseProfile("lpar.service");

const testSession = new Session(JSON.parse(JSON.stringify(expectedSession.ISession)));
delete testSession.ISession.user;
delete testSession.ISession.password;
testSession.ISession.hostname = "dummy";
testSession.ISession.base64EncodedAuth = "dXNlcjpwYXNz";
testSession.ISession.tokenType = tempBaseProfile.profile.tokenType;
testSession.ISession.storeCookie = false;

expect(loginSpy).toHaveBeenCalledWith(testSession);
expect(testSpy).toHaveBeenCalledWith(testCache, "lpar.service");
expect(testCache.updateBaseProfileFileLogin).toHaveBeenCalledWith(
{
name: tempBaseProfile.name,
type: null,
profile: {
...serviceProfile.profile,
tokenType: tempBaseProfile.profile.tokenType,
},
},
{
tokenType: tempBaseProfile.profile.tokenType,
tokenValue: "tokenValue",
},
false
);
quickPickMock.mockRestore();
});
it("should logout using the service profile given a simple profile name", async () => {
testCache.fetchBaseProfile.mockResolvedValue(baseProfile);
const testSpy = jest.spyOn(ZoweVsCodeExtension as any, "getServiceProfileForAuthPurposes");
Expand Down
37 changes: 34 additions & 3 deletions packages/zowe-explorer-api/src/profiles/ProfilesCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,11 +315,28 @@ export class ProfilesCache {
return baseProfile;
}

// This will retrieve the base profile from imperative
public async fetchBaseProfile(): Promise<imperative.IProfileLoaded | undefined> {
/**
* Retrieves the base profile from Imperative to use for log in/out. If a
* nested profile name is specified (e.g. "lpar.zosmf"), then its parent
* profile is returned unless token is already stored in the base profile.
Comment on lines +319 to +321
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add this new behavior to the changelog as a new breaking change?

* @param profileName Name of profile that was selected in the tree
* @returns IProfileLoaded object or undefined if no profile was found
*/
public async fetchBaseProfile(profileName?: string): Promise<imperative.IProfileLoaded | undefined> {
const mProfileInfo = await this.getProfileInfo();
const baseProfileAttrs = mProfileInfo.getDefaultProfile("base");
if (baseProfileAttrs == null) {
const config = mProfileInfo.getTeamConfig();
if (
profileName?.includes(".") &&
t1m0thyj marked this conversation as resolved.
Show resolved Hide resolved
(baseProfileAttrs == null || !config.api.secure.securePropsForProfile(baseProfileAttrs.profName).includes("tokenValue"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question about this line here.
In order for this function to return the default base profile (old behavior), the token only needs to be in the secure array of the base profile and not actually stored in the vault. Is that assumption correct?

Follow up question, Does that means that this function will return the default base profile even if I already have a token stored in the parent profile?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good questions, sorry I didn't see this until after the PR was merged. Your assumption is correct 😁

For the 2nd question, that's right. I think the reason was for backwards compatibility - however we probably want to give precedence to whichever profile has tokenValue already defined 😋

) {
// 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 tokenValue in secure array
const parentProfile = this.getParentProfileForToken(profileName, config);
return this.getProfileLoaded(parentProfile, "base", config.api.profiles.get(parentProfile));
} else if (baseProfileAttrs == null) {
return undefined;
}
const profAttr = this.getMergedAttrs(mProfileInfo, baseProfileAttrs);
Expand Down Expand Up @@ -399,6 +416,20 @@ export class ProfilesCache {
return allTypes;
}

private getParentProfileForToken(profileName: string, config: imperative.Config): string {
const secureProps = config.api.secure.secureFields();
let parentProfile = profileName.slice(0, profileName.lastIndexOf("."));
let tempProfile = profileName;
while (tempProfile.includes(".")) {
tempProfile = tempProfile.slice(0, tempProfile.lastIndexOf("."));
if (secureProps.includes(`${config.api.profiles.getProfilePathFromName(tempProfile)}.properties.tokenValue`)) {
t1m0thyj marked this conversation as resolved.
Show resolved Hide resolved
parentProfile = tempProfile;
break;
}
}
return parentProfile;
}

private shouldRemoveTokenFromProfile(profile: imperative.IProfileLoaded, baseProfile: imperative.IProfileLoaded): boolean {
return ((baseProfile?.profile?.host || baseProfile?.profile?.port) &&
profile?.profile?.host &&
Expand Down
47 changes: 27 additions & 20 deletions packages/zowe-explorer-api/src/vscode/ZoweVsCodeExtension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,14 @@ export class ZoweVsCodeExtension {
zeProfiles?: ProfilesCache // Profiles extends ProfilesCache
): Promise<boolean> {
const cache: ProfilesCache = zeProfiles ?? ZoweVsCodeExtension.profilesCache;
const baseProfile = await cache.fetchBaseProfile();
if (baseProfile == null) {
return false;
}
if (typeof serviceProfile === "string") {
serviceProfile = await ZoweVsCodeExtension.getServiceProfileForAuthPurposes(cache, serviceProfile);
}
const baseProfile = await cache.fetchBaseProfile(serviceProfile.name);
if (baseProfile == null) {
Gui.errorMessage(`Login failed: No base profile found to store SSO token for profile "${serviceProfile.name}"`);
return false;
}
const tokenType =
serviceProfile.profile.tokenType ?? baseProfile.profile.tokenType ?? loginTokenType ?? imperative.SessConstants.TOKEN_TYPE_APIML;
const updSession = new imperative.Session({
Expand All @@ -144,7 +145,10 @@ export class ZoweVsCodeExtension {
{ label: "$(account) User and Password", description: "Log in with basic authentication" },
{ label: "$(note) Certificate", description: "Log in with PEM format certificate file" },
];
const response = await Gui.showQuickPick(qpItems, { placeHolder: "Select an authentication method for obtaining token" });
const response = await Gui.showQuickPick(qpItems, {
placeHolder: "Select an authentication method for obtaining token",
title: `[${baseProfile.name}] Log in to authentication service`,
});
if (response === qpItems[0]) {
const creds = await ZoweVsCodeExtension.promptUserPass({ session: updSession.ISession, rePrompt: true });
if (!creds) {
Expand Down Expand Up @@ -180,11 +184,10 @@ export class ZoweVsCodeExtension {
// If base profile already has a token type stored, then we check whether or not the connection details are the same
(serviceProfile.profile.host === baseProfile.profile.host && serviceProfile.profile.port === baseProfile.profile.port);
// If the connection details do not match, then we MUST forcefully store the token in the service profile
let profileToUpdate: imperative.IProfileLoaded;
let profileToUpdate = serviceProfile;
if (connOk) {
profileToUpdate = baseProfile;
} else {
profileToUpdate = serviceProfile;
// If active profile is nested (e.g. lpar.zosmf), then set type to null so token can be stored in parent typeless profile
profileToUpdate = serviceProfile.name.startsWith(baseProfile.name + ".") ? { ...baseProfile, type: null } : baseProfile;
t1m0thyj marked this conversation as resolved.
Show resolved Hide resolved
}

await cache.updateBaseProfileFileLogin(profileToUpdate, updBaseProfile, !connOk);
Expand Down Expand Up @@ -212,13 +215,13 @@ export class ZoweVsCodeExtension {
serviceProfile: string | imperative.IProfileLoaded,
zeRegister?: Types.IApiRegisterClient, // ZoweExplorerApiRegister
zeProfiles?: ProfilesCache // Profiles extends ProfilesCache
): Promise<void> {
): Promise<boolean> {
const cache: ProfilesCache = zeProfiles ?? ZoweVsCodeExtension.profilesCache;
const baseProfile = await cache.fetchBaseProfile();
if (typeof serviceProfile === "string") {
serviceProfile = await ZoweVsCodeExtension.getServiceProfileForAuthPurposes(cache, serviceProfile);
}
traeok marked this conversation as resolved.
Show resolved Hide resolved
const baseProfile = await cache.fetchBaseProfile(serviceProfile.name);
if (baseProfile) {
if (typeof serviceProfile === "string") {
serviceProfile = await ZoweVsCodeExtension.getServiceProfileForAuthPurposes(cache, serviceProfile);
}
const tokenType =
serviceProfile.profile.tokenType ??
baseProfile.profile.tokenType ??
Expand All @@ -234,12 +237,16 @@ export class ZoweVsCodeExtension {
});
await (zeRegister?.getCommonApi(serviceProfile).logout ?? Logout.apimlLogout)(updSession);

const connOk = serviceProfile.profile.host === baseProfile.profile.host && serviceProfile.profile.port === baseProfile.profile.port;
if (connOk) {
await cache.updateBaseProfileFileLogout(baseProfile);
} else {
await cache.updateBaseProfileFileLogout(serviceProfile);
}
// If active profile is nested (e.g. lpar.zosmf), then update service profile since base profile may be typeless
const connOk =
serviceProfile.profile.host === baseProfile.profile.host &&
serviceProfile.profile.port === baseProfile.profile.port &&
!serviceProfile.name.startsWith(baseProfile.name + ".");
await cache.updateBaseProfileFileLogout(connOk ? baseProfile : serviceProfile);
t1m0thyj marked this conversation as resolved.
Show resolved Hide resolved
return true;
} else {
Gui.errorMessage(`Logout failed: No base profile found to remove SSO token for profile "${serviceProfile.name}"`);
return false;
}
}

Expand Down
Loading