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

Fix logout to give base profile precedence over parent profile #3076

Merged
merged 16 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 3 additions & 0 deletions packages/zowe-explorer-api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ All notable changes to the "zowe-explorer-api" extension will be documented in t

### New features and enhancements

- Added the `BaseProfileAuthOptions` interface to define base profile authentication options for SSO login and logout. [#3076](https://github.com/zowe/zowe-explorer-vscode/pull/3076)
- 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)

### Bug fixes

- 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ describe("ZoweVsCodeExtension", () => {
const errorMessageSpy = jest.spyOn(Gui, "errorMessage");
const fetchBaseProfileSpy = jest.spyOn(blockMocks.testCache, "fetchBaseProfile").mockResolvedValue(undefined);
const updateBaseProfileFileLoginSpy = jest.spyOn(blockMocks.testCache, "updateBaseProfileFileLogin");
await ZoweVsCodeExtension.loginWithBaseProfile({ serviceProfile: "service" });
await ZoweVsCodeExtension.ssoLogin({ serviceProfile: "service" });
expect(fetchBaseProfileSpy).toHaveBeenCalledTimes(1);
expect(updateBaseProfileFileLoginSpy).not.toHaveBeenCalled();
expect(errorMessageSpy).toHaveBeenCalledWith(expect.stringContaining("Login failed: No base profile found"));
Expand All @@ -195,7 +195,7 @@ describe("ZoweVsCodeExtension", () => {
const errorMessageSpy = jest.spyOn(Gui, "errorMessage");
const fetchBaseProfileSpy = jest.spyOn(blockMocks.testCache, "fetchBaseProfile").mockResolvedValue(undefined);
const updateBaseProfileFileLoginSpy = jest.spyOn(blockMocks.testCache, "updateBaseProfileFileLogin");
await ZoweVsCodeExtension.logoutWithBaseProfile({ serviceProfile: "service" });
await ZoweVsCodeExtension.ssoLogout({ serviceProfile: "service" });
expect(fetchBaseProfileSpy).toHaveBeenCalledTimes(1);
expect(updateBaseProfileFileLoginSpy).not.toHaveBeenCalled();
expect(errorMessageSpy).toHaveBeenCalledWith(expect.stringContaining("Logout failed: No base profile found"));
Expand All @@ -210,7 +210,7 @@ describe("ZoweVsCodeExtension", () => {
const loginSpy = jest.spyOn(Login, "apimlLogin").mockResolvedValue("tokenValue");

const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation((items) => items[0]);
await ZoweVsCodeExtension.loginWithBaseProfile({ serviceProfile: "service" });
await ZoweVsCodeExtension.ssoLogin({ serviceProfile: "service" });

const testSession = new imperative.Session(JSON.parse(JSON.stringify(blockMocks.expectedSession.ISession)));
delete testSession.ISession.user;
Expand All @@ -231,7 +231,7 @@ describe("ZoweVsCodeExtension", () => {
const logoutSpy = jest.spyOn(Logout, "apimlLogout").mockImplementation(jest.fn());

const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation((items) => items[0]);
await ZoweVsCodeExtension.logoutWithBaseProfile({ serviceProfile: "service" });
await ZoweVsCodeExtension.ssoLogout({ serviceProfile: "service" });

const testSession = new imperative.Session(JSON.parse(JSON.stringify(blockMocks.expectedSession.ISession)));
testSession.ISession.tokenValue = "tokenValue";
Expand Down Expand Up @@ -259,7 +259,7 @@ describe("ZoweVsCodeExtension", () => {
const loginSpy = jest.spyOn(Login, "apimlLogin").mockResolvedValue("tokenValue");

const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation((items) => items[0]);
await ZoweVsCodeExtension.loginWithBaseProfile({ serviceProfile: "service" });
await ZoweVsCodeExtension.ssoLogin({ serviceProfile: "service" });

const testSession = new imperative.Session(JSON.parse(JSON.stringify(blockMocks.expectedSession.ISession)));
delete testSession.ISession.user;
Expand Down Expand Up @@ -288,7 +288,7 @@ describe("ZoweVsCodeExtension", () => {
const loginSpy = jest.spyOn(Login, "apimlLogin").mockResolvedValue("tokenValue");

const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation((items) => items[0]);
await ZoweVsCodeExtension.loginWithBaseProfile({ serviceProfile: "service" });
await ZoweVsCodeExtension.ssoLogin({ serviceProfile: "service" });

const testSession = new imperative.Session(JSON.parse(JSON.stringify(blockMocks.expectedSession.ISession)));
delete testSession.ISession.user;
Expand Down Expand Up @@ -338,7 +338,7 @@ describe("ZoweVsCodeExtension", () => {
};

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

const testSession = new imperative.Session(JSON.parse(JSON.stringify(blockMocks.expectedSession.ISession)));
delete testSession.ISession.user;
Expand Down Expand Up @@ -379,7 +379,7 @@ describe("ZoweVsCodeExtension", () => {
const logoutSpy = jest.spyOn(Logout, "apimlLogout").mockImplementation(jest.fn());

const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation((items) => items[0]);
await ZoweVsCodeExtension.logoutWithBaseProfile({ serviceProfile: "service" });
await ZoweVsCodeExtension.ssoLogout({ serviceProfile: "service" });

const testSession = new imperative.Session(JSON.parse(JSON.stringify(blockMocks.expectedSession.ISession)));
testSession.ISession.hostname = "service";
Expand All @@ -401,7 +401,7 @@ describe("ZoweVsCodeExtension", () => {
const loginSpy = jest.spyOn(Login, "apimlLogin").mockResolvedValue("tokenValue");

const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation((items) => items[0]);
await ZoweVsCodeExtension.loginWithBaseProfile({
await ZoweVsCodeExtension.ssoLogin({
serviceProfile: blockMocks.serviceProfile,
defaultTokenType: "apimlAuthenticationToken",
profileNode: blockMocks.testNode,
Expand All @@ -427,7 +427,7 @@ describe("ZoweVsCodeExtension", () => {
const logoutSpy = jest.spyOn(Logout, "apimlLogout").mockImplementation(jest.fn());
const newServiceProfile = { ...blockMocks.serviceProfile, profile: { ...blockMocks.testProfile, ...blockMocks.updProfile } };

await ZoweVsCodeExtension.logoutWithBaseProfile({
await ZoweVsCodeExtension.ssoLogout({
serviceProfile: newServiceProfile,
zeRegister: blockMocks.testRegister,
zeProfiles: blockMocks.testCache,
Expand Down Expand Up @@ -458,7 +458,7 @@ describe("ZoweVsCodeExtension", () => {
// case 1: User selects "user/password" for login quick pick
const promptCertMock = jest.spyOn(ZoweVsCodeExtension as any, "promptCertificate").mockImplementation();
const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation((items) => items[1]);
await ZoweVsCodeExtension.loginWithBaseProfile({ serviceProfile: "service" });
await ZoweVsCodeExtension.ssoLogin({ serviceProfile: "service" });

const testSession = new imperative.Session(JSON.parse(JSON.stringify(blockMocks.expectedSession.ISession)));
delete testSession.ISession.user;
Expand All @@ -485,15 +485,15 @@ describe("ZoweVsCodeExtension", () => {
const promptCertMock = jest
.spyOn(ZoweVsCodeExtension as any, "promptCertificate")
.mockRejectedValueOnce(new Error("invalid certificate"));
await expect(ZoweVsCodeExtension.loginWithBaseProfile({ serviceProfile: "service" })).resolves.toBe(false);
await expect(ZoweVsCodeExtension.ssoLogin({ serviceProfile: "service" })).resolves.toBe(false);
expect(promptCertMock).toHaveBeenCalled();
expect(quickPickMock).toHaveBeenCalled();
});

it("should not login if the user cancels the operation when selecting the authentication method", async () => {
jest.spyOn(blockMocks.testCache, "fetchBaseProfile").mockResolvedValue(blockMocks.baseProfile);
const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation(() => undefined as any);
const didLogin = await ZoweVsCodeExtension.loginWithBaseProfile({
const didLogin = await ZoweVsCodeExtension.ssoLogin({
serviceProfile: blockMocks.serviceProfile,
defaultTokenType: "apimlAuthenticationToken",
profileNode: blockMocks.testNode,
Expand All @@ -512,7 +512,7 @@ describe("ZoweVsCodeExtension", () => {

const testSpy = jest.spyOn(ZoweVsCodeExtension as any, "promptUserPass").mockResolvedValue(undefined);
const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation((items) => items[0]);
const didLogin = await ZoweVsCodeExtension.loginWithBaseProfile({
const didLogin = await ZoweVsCodeExtension.ssoLogin({
serviceProfile: serviceProfileLoaded,
defaultTokenType: "apimlAuthenticationToken",
profileNode: blockMocks.testNode,
Expand Down Expand Up @@ -544,7 +544,7 @@ describe("ZoweVsCodeExtension", () => {
await ZoweVsCodeExtension.profilesCache.refresh({
registeredApiTypes: jest.fn().mockReturnValue(["service"]),
} as unknown as Types.IApiRegisterClient);
const didLogin = await ZoweVsCodeExtension.loginWithBaseProfile({
const didLogin = await ZoweVsCodeExtension.ssoLogin({
serviceProfile: serviceProfileLoaded,
defaultTokenType: "apimlAuthenticationToken",
profileNode: blockMocks.testNode,
Expand All @@ -571,7 +571,7 @@ describe("ZoweVsCodeExtension", () => {

const testSpy = jest.spyOn(ZoweVsCodeExtension as any, "promptUserPass").mockResolvedValue(["abc", "def"]);
const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation((items) => items[0]);
const didLogin = await ZoweVsCodeExtension.loginWithBaseProfile({
const didLogin = await ZoweVsCodeExtension.ssoLogin({
serviceProfile: serviceProfileLoaded,
defaultTokenType: "apimlAuthenticationToken",
profileNode: blockMocks.testNode,
Expand All @@ -596,7 +596,7 @@ describe("ZoweVsCodeExtension", () => {

const testSpy = jest.spyOn(ZoweVsCodeExtension as any, "promptUserPass").mockResolvedValue(["abc", "def"]);
const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation((items) => items[0]);
const didLogin = await ZoweVsCodeExtension.loginWithBaseProfile({
const didLogin = await ZoweVsCodeExtension.ssoLogin({
serviceProfile: serviceProfileLoaded,
defaultTokenType: "apimlAuthenticationToken",
profileNode: blockMocks.testNode,
Expand Down Expand Up @@ -633,7 +633,7 @@ describe("ZoweVsCodeExtension", () => {

const testSpy = jest.spyOn(ZoweVsCodeExtension as any, "promptUserPass").mockResolvedValue(["abc", "def"]);
const quickPickMock = jest.spyOn(Gui, "showQuickPick").mockImplementation((items) => items[0]);
const didLogin = await ZoweVsCodeExtension.loginWithBaseProfile({
const didLogin = await ZoweVsCodeExtension.ssoLogin({
serviceProfile: serviceProfileLoaded,
defaultTokenType: "apimlAuthenticationToken",
profileNode: blockMocks.testNode,
Expand All @@ -655,7 +655,7 @@ describe("ZoweVsCodeExtension", () => {

const testSpy = jest.spyOn(ZoweVsCodeExtension as any, "promptUserPass");
const errorMessageSpy = jest.spyOn(Gui, "errorMessage");
const didLogin = await ZoweVsCodeExtension.loginWithBaseProfile({
const didLogin = await ZoweVsCodeExtension.ssoLogin({
serviceProfile: serviceProfileLoaded,
defaultTokenType: "apimlAuthenticationToken",
profileNode: blockMocks.testNode,
Expand Down
37 changes: 34 additions & 3 deletions packages/zowe-explorer-api/src/vscode/ZoweVsCodeExtension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@
shouldSave = await ZoweVsCodeExtension.saveCredentials(loadProfile);
}

// TODO Should we call updateProfileInCache method here?
if (shouldSave) {
// write changes to the file, autoStore value determines if written to file
const upd = { profileName: loadProfile.name, profileType: loadProfile.type };
Expand All @@ -113,13 +112,30 @@
* Trigger a login operation with the merged contents between the service profile and the base profile.
* If the connection details (host:port) do not match (service vs base), the token will be stored in the service profile.
* If there is no API registered for the profile type, this method defaults the login behavior to that of the APIML.
* @deprecated Use `ZoweVsCodeExtension.ssoLogin` instead
* @param serviceProfile Profile to be used for login purposes (either the name of the IProfileLoaded instance)
* @param loginTokenType The tokenType value for compatibility purposes
* @param node The node for compatibility purposes
* @param zeRegister The ZoweExplorerApiRegister instance for compatibility purposes
* @param zeProfiles The Zowe Explorer "Profiles.ts" instance for compatibility purposes
*/
public static async loginWithBaseProfile(opts: BaseProfileAuthOptions): Promise<boolean> {
public static async loginWithBaseProfile(
serviceProfile: string | imperative.IProfileLoaded,
loginTokenType?: string,
node?: Types.IZoweNodeType,
zeRegister?: Types.IApiRegisterClient, // ZoweExplorerApiRegister
zeProfiles?: ProfilesCache // Profiles extends ProfilesCache
): Promise<boolean> {
return this.ssoLogin({ serviceProfile, defaultTokenType: loginTokenType, profileNode: node, zeRegister, zeProfiles });

Check warning on line 129 in packages/zowe-explorer-api/src/vscode/ZoweVsCodeExtension.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer-api/src/vscode/ZoweVsCodeExtension.ts#L129

Added line #L129 was not covered by tests
}

/**
* Trigger a login operation with the merged contents between the service profile and the base profile.
* If the connection details (host:port) do not match (service vs base), the token will be stored in the service profile.
* If there is no API registered for the profile type, this method defaults the login behavior to that of the APIML.
* @param {BaseProfileAuthOptions} opts Object defining options for base profile authentication
*/
public static async ssoLogin(opts: BaseProfileAuthOptions): Promise<boolean> {
Copy link
Member

Choose a reason for hiding this comment

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

I completely missed that this would've been a breaking change to extenders.
Great catch! 🙏🏽

Thanks for deprecating the old login/-out function in favor of the new ssoLogin/-out 🥳

const cache: ProfilesCache = opts.zeProfiles ?? ZoweVsCodeExtension.profilesCache;
const serviceProfile =
typeof opts.serviceProfile === "string"
Expand Down Expand Up @@ -207,11 +223,26 @@
* Trigger a logout operation with the merged contents between the service profile and the base profile.
* If the connection details (host:port) do not match (service vs base), the token will be removed from the service profile.
* If there is no API registered for the profile type, this method defaults the logout behavior to that of the APIML.
* @deprecated Use `ZoweVsCodeExtension.ssoLogout` instead
* @param serviceProfile Profile to be used for logout purposes (either the name of the IProfileLoaded instance)
* @param zeRegister The ZoweExplorerApiRegister instance for compatibility purposes
* @param zeProfiles The Zowe Explorer "Profiles.ts" instance for compatibility purposes
*/
public static async logoutWithBaseProfile(opts: BaseProfileAuthOptions): Promise<boolean> {
public static async logoutWithBaseProfile(
serviceProfile: string | imperative.IProfileLoaded,
zeRegister?: Types.IApiRegisterClient, // ZoweExplorerApiRegister
zeProfiles?: ProfilesCache // Profiles extends ProfilesCache
): Promise<boolean> {
return this.ssoLogout({ serviceProfile, zeRegister, zeProfiles });

Check warning on line 236 in packages/zowe-explorer-api/src/vscode/ZoweVsCodeExtension.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer-api/src/vscode/ZoweVsCodeExtension.ts#L236

Added line #L236 was not covered by tests
}

/**
* Trigger a logout operation with the merged contents between the service profile and the base profile.
* If the connection details (host:port) do not match (service vs base), the token will be removed from the service profile.
* If there is no API registered for the profile type, this method defaults the logout behavior to that of the APIML.
* @param {BaseProfileAuthOptions} opts Object defining options for base profile authentication
*/
public static async ssoLogout(opts: BaseProfileAuthOptions): Promise<boolean> {
const cache: ProfilesCache = opts.zeProfiles ?? ZoweVsCodeExtension.profilesCache;
const serviceProfile =
typeof opts.serviceProfile === "string"
Expand All @@ -229,7 +260,7 @@
primaryProfile.profile.tokenType ??
secondaryProfile.profile.tokenType ??
opts.defaultTokenType ??
imperative.SessConstants.TOKEN_TYPE_APIML;

Check warning on line 263 in packages/zowe-explorer-api/src/vscode/ZoweVsCodeExtension.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer-api/src/vscode/ZoweVsCodeExtension.ts#L263

Added line #L263 was not covered by tests
const updSession = new imperative.Session({
hostname: serviceProfile.profile.host,
port: serviceProfile.profile.port,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1210,7 +1210,7 @@ describe("Profiles Unit Tests - function ssoLogin", () => {
getTokenTypeName: () => imperative.SessConstants.TOKEN_TYPE_APIML,
login: jest.fn(),
} as never);
const loginBaseProfMock = jest.spyOn(ZoweVsCodeExtension, "loginWithBaseProfile").mockRejectedValueOnce(new Error("test error."));
const loginBaseProfMock = jest.spyOn(ZoweVsCodeExtension, "ssoLogin").mockRejectedValueOnce(new Error("test error."));
jest.spyOn(Profiles.getInstance() as any, "loginCredentialPrompt").mockReturnValue(["fake", "12345"]);
await expect(Profiles.getInstance().ssoLogin(testNode, "fake")).resolves.not.toThrow();
expect(ZoweLogger.error).toHaveBeenCalled();
Expand Down Expand Up @@ -1309,7 +1309,7 @@ describe("Profiles Unit Tests - function handleSwitchAuthentication", () => {
getTokenTypeName: () => "apimlAuthenticationToken",
} as never);

jest.spyOn(ZoweVsCodeExtension, "loginWithBaseProfile").mockResolvedValue(true);
jest.spyOn(ZoweVsCodeExtension, "ssoLogin").mockResolvedValue(true);
await Profiles.getInstance().handleSwitchAuthentication(testNode);
expect(Gui.showMessage).toHaveBeenCalled();
expect(testNode.profile.profile.tokenType).toBe(modifiedTestNode.profile.profile.tokenType);
Expand Down Expand Up @@ -1368,7 +1368,7 @@ describe("Profiles Unit Tests - function handleSwitchAuthentication", () => {
getTokenTypeName: () => "apimlAuthenticationToken",
} as never);

jest.spyOn(ZoweVsCodeExtension, "loginWithBaseProfile").mockResolvedValue(false);
jest.spyOn(ZoweVsCodeExtension, "ssoLogin").mockResolvedValue(false);
await Profiles.getInstance().handleSwitchAuthentication(testNode);
expect(Gui.errorMessage).toHaveBeenCalled();
expect(testNode.profile.profile.tokenType).toBe(modifiedTestNode.profile.profile.tokenType);
Expand Down
Loading
Loading