From f7369a96a45cde4564e7193e1ae3ed475bccbbc3 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Fri, 23 Jun 2023 14:47:20 +0000 Subject: [PATCH 01/26] add ImperativeExpect.toMatchRegExp Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- packages/expect/src/ImperativeExpect.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/expect/src/ImperativeExpect.ts b/packages/expect/src/ImperativeExpect.ts index a68f6aba3..4aab004e5 100644 --- a/packages/expect/src/ImperativeExpect.ts +++ b/packages/expect/src/ImperativeExpect.ts @@ -78,6 +78,21 @@ export class ImperativeExpect { } } + /** + * Expect that value matches the regular expression (via ".test()" method). + * @static + * @param {*} value - Value + * @param {*} myRegex - Regular expression + * @param {string} [msg] - The message to throw - overrides the default message + * @memberof ImperativeExpect + */ + public static toMatchRegExp(value: any, myRegex: string, msg?: string) { + if (!(new RegExp(myRegex).test(value))) { + throw new ImperativeError({msg: msg || "Input object/value does not match the regular expression"}, + {tag: ImperativeExpect.ERROR_TAG}); + } + } + /** * Expect the object passed to be defined. * @static From 323858f27ecc5ba2486cb97d67b985245422139d Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Fri, 23 Jun 2023 14:48:53 +0000 Subject: [PATCH 02/26] prevent multiple logout from failing Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- .../src/auth/handlers/BaseAuthHandler.ts | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/packages/imperative/src/auth/handlers/BaseAuthHandler.ts b/packages/imperative/src/auth/handlers/BaseAuthHandler.ts index 989c0e049..dc507a8f9 100644 --- a/packages/imperative/src/auth/handlers/BaseAuthHandler.ts +++ b/packages/imperative/src/auth/handlers/BaseAuthHandler.ts @@ -197,27 +197,29 @@ export abstract class BaseAuthHandler extends AbstractAuthHandler { * @param {IHandlerParameters} params Command parameters sent by imperative. */ protected async processLogout(params: IHandlerParameters) { - ImperativeExpect.toNotBeNullOrUndefined(params.arguments.tokenValue, "Token value not supplied, but is required for logout."); - - // Force to use of token value, in case user and/or password also on base profile, make user undefined. - if (params.arguments.user != null) { - params.arguments.user = undefined; - } + if (params.arguments.tokenValue != null) { + // Force to use of token value, in case user and/or password also on base profile, make user undefined. + if (params.arguments.user != null) { + params.arguments.user = undefined; + } - params.arguments.tokenType = this.mDefaultTokenType; + if (params.arguments.tokenType == null) { + params.arguments.tokenType = this.mDefaultTokenType; + } - const sessCfg: ISession = this.createSessCfgFromArgs( - params.arguments - ); + const sessCfg: ISession = this.createSessCfgFromArgs( + params.arguments + ); - const sessCfgWithCreds = await ConnectionPropsForSessCfg.addPropsOrPrompt( - sessCfg, params.arguments, - { requestToken: false, parms: params }, - ); + const sessCfgWithCreds = await ConnectionPropsForSessCfg.addPropsOrPrompt( + sessCfg, params.arguments, + { requestToken: false, doPrompting: false, parms: params }, + ); - this.mSession = new Session(sessCfgWithCreds); + this.mSession = new Session(sessCfgWithCreds); - await this.doLogout(this.mSession); + await this.doLogout(this.mSession); + } if (!ImperativeConfig.instance.config.exists) { await this.processLogoutOld(params); From a5c432af7f76f8bd83a7172ea9e13102bbe68a78 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Mon, 26 Jun 2023 14:45:21 +0000 Subject: [PATCH 03/26] allow autoStore for dynamic apiml token types Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- packages/config/src/ConfigAutoStore.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/config/src/ConfigAutoStore.ts b/packages/config/src/ConfigAutoStore.ts index 1aa578cd3..97fadd52e 100644 --- a/packages/config/src/ConfigAutoStore.ts +++ b/packages/config/src/ConfigAutoStore.ts @@ -106,7 +106,7 @@ export class ConfigAutoStore { if (authHandlerClass instanceof AbstractAuthHandler) { const { promptParams } = authHandlerClass.getAuthHandlerApi(); - if (profile.tokenType === promptParams.defaultTokenType) { + if (profile.tokenType.startsWith(promptParams.defaultTokenType)) { return authHandlerClass; // Auth service must have matching token type } } From 45e775a6112074a6be415bfe596bad079838c897 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Mon, 26 Jun 2023 14:45:49 +0000 Subject: [PATCH 04/26] allow logout to remove token value event if token type is not defined and viceversa Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- packages/imperative/src/auth/handlers/BaseAuthHandler.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/imperative/src/auth/handlers/BaseAuthHandler.ts b/packages/imperative/src/auth/handlers/BaseAuthHandler.ts index dc507a8f9..9827ee901 100644 --- a/packages/imperative/src/auth/handlers/BaseAuthHandler.ts +++ b/packages/imperative/src/auth/handlers/BaseAuthHandler.ts @@ -230,8 +230,8 @@ export abstract class BaseAuthHandler extends AbstractAuthHandler { let profileWithToken: string = null; // If you specified a token on the command line, then don't delete the one in the profile if it doesn't match - if (Object.keys(profileProps).length > 0 && profileProps.tokenType != null && profileProps.tokenValue != null && - profileProps.tokenType === params.arguments.tokenType && profileProps.tokenValue === params.arguments.tokenValue) { + if (Object.keys(profileProps).length > 0 && (profileProps.tokenType != null || profileProps.tokenValue != null) && + (profileProps.tokenType === params.arguments.tokenType || profileProps.tokenValue === params.arguments.tokenValue)) { const profilePath = config.api.profiles.expandPath(profileName); config.delete(`${profilePath}.properties.tokenType`); config.delete(`${profilePath}.properties.tokenValue`); From edcce76cf981bfc06859adad6a0f1058591a7191 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Mon, 26 Jun 2023 15:59:31 +0000 Subject: [PATCH 05/26] add utility method to better support nested configuration Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- packages/config/src/api/ConfigProfiles.ts | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/config/src/api/ConfigProfiles.ts b/packages/config/src/api/ConfigProfiles.ts index 98dec9684..5a99ce61c 100644 --- a/packages/config/src/api/ConfigProfiles.ts +++ b/packages/config/src/api/ConfigProfiles.ts @@ -110,6 +110,27 @@ export class ConfigProfiles extends ConfigApi { return shortPath.replace(/(^|\.)/g, "$1profiles."); } + // _______________________________________________________________________ + /** + * Obtain the profile name (either nested or not) based on a property path. + * + * @param path The property path. + * + * @returns The corresponding profile name. + * + * @note This may be useful for supporting token authentication in a nested configuration + * + */ + public getProfileNameFromPath(path: string): string { + let profileName = ""; + for (const p of path.split(".")) { + if (p === "profiles") continue; + if (p === "properties") break; + profileName += profileName.length > 0 ? "." + p : p; + } + return profileName; + } + // _______________________________________________________________________ /** * Build the set of properties contained within a set of nested profiles. From d76da654b5b259eda60c74bc76f5985abeef2771 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Mon, 26 Jun 2023 16:00:11 +0000 Subject: [PATCH 06/26] Add support for multiple tokenType authentication processes in a single config-secure call Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- .../src/config/cmd/secure/secure.handler.ts | 39 ++++++++----------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/packages/imperative/src/config/cmd/secure/secure.handler.ts b/packages/imperative/src/config/cmd/secure/secure.handler.ts index b71c20ed7..af9de5b79 100644 --- a/packages/imperative/src/config/cmd/secure/secure.handler.ts +++ b/packages/imperative/src/config/cmd/secure/secure.handler.ts @@ -57,29 +57,24 @@ export default class SecureHandler implements ICommandHandler { } // Prompt for values designated as secure - let authTokenProp: string; for (const propName of secureProps) { - if (authTokenProp == null && propName.endsWith(".tokenValue")) { - authTokenProp = propName; - continue; - } - - let propValue = await params.response.console.prompt(`Enter ${propName} - blank to skip: `, { hideText: true }); - - // Save the value in the config securely - if (propValue) { - propValue = coercePropValue(propValue, ConfigSchema.findPropertyType(propName, config.properties)); - config.set(propName, propValue, { secure: true }); - } - } - - if (authTokenProp != null) { - const propValue = await this.handlePromptForAuthToken(config, authTokenProp) || - await params.response.console.prompt(`Enter ${authTokenProp} - blank to skip: `, {hideText: true}); - - // Save the value in the config securely - if (propValue) { - config.set(authTokenProp, propValue, { secure: true }); + if (propName.endsWith(".tokenValue")) { + params.response.console.log(`Processing secure properties for profile: ${config.api.profiles.getProfileNameFromPath(propName)}`); + const propValue = await this.handlePromptForAuthToken(config, propName) || + await params.response.console.prompt(`Enter ${propName} - blank to skip: `, {hideText: true}); + + // Save the value in the config securely + if (propValue) { + config.set(propName, propValue, { secure: true }); + } + } else { + let propValue = await params.response.console.prompt(`Enter ${propName} - blank to skip: `, { hideText: true }); + + // Save the value in the config securely + if (propValue) { + propValue = coercePropValue(propValue, ConfigSchema.findPropertyType(propName, config.properties)); + config.set(propName, propValue, { secure: true }); + } } } From 4aa454769d7072a26e023e213389796bbb4c0018 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Mon, 26 Jun 2023 18:48:26 +0000 Subject: [PATCH 07/26] prevent auto-init from performing a second login operation if we already have a token Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- .../src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts b/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts index 1926ad4da..9154ce856 100644 --- a/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts +++ b/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts @@ -89,6 +89,9 @@ export abstract class BaseAutoInitHandler implements ICommandHandler { sessCfg, params.arguments, { parms: params, doPrompting: true, serviceDescription: this.mServiceDescription }, ); this.mSession = new Session(sessCfgWithCreds); + if (this.mSession.ISession.tokenValue) { + this.mSession.ISession.user = this.mSession.ISession.password = this.mSession.ISession.cert = this.mSession.ISession.certKey = undefined; + } // Use params to set which config layer to apply to await OverridesLoader.ensureCredentialManagerLoaded(); From 274c368dd8131b3c88d5d922bbfa64c779a63fc7 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Wed, 28 Jun 2023 14:45:31 +0000 Subject: [PATCH 08/26] fix lint issue and revert one scenario Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- packages/imperative/src/auth/handlers/BaseAuthHandler.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/imperative/src/auth/handlers/BaseAuthHandler.ts b/packages/imperative/src/auth/handlers/BaseAuthHandler.ts index 9827ee901..5f155c453 100644 --- a/packages/imperative/src/auth/handlers/BaseAuthHandler.ts +++ b/packages/imperative/src/auth/handlers/BaseAuthHandler.ts @@ -12,7 +12,6 @@ import { IHandlerParameters, IHandlerResponseApi } from "../../../../cmd"; import { AbstractSession, ConnectionPropsForSessCfg, IOptionsForAddConnProps, ISession, SessConstants, Session } from "../../../../rest"; import { Imperative } from "../../Imperative"; -import { ImperativeExpect } from "../../../../expect"; import { ImperativeError } from "../../../../error"; import { ISaveProfileFromCliArgs } from "../../../../profiles"; import { ImperativeConfig } from "../../../../utilities"; @@ -230,8 +229,8 @@ export abstract class BaseAuthHandler extends AbstractAuthHandler { let profileWithToken: string = null; // If you specified a token on the command line, then don't delete the one in the profile if it doesn't match - if (Object.keys(profileProps).length > 0 && (profileProps.tokenType != null || profileProps.tokenValue != null) && - (profileProps.tokenType === params.arguments.tokenType || profileProps.tokenValue === params.arguments.tokenValue)) { + if (Object.keys(profileProps).length > 0 && profileProps.tokenType != null && profileProps.tokenValue != null && + profileProps.tokenType === params.arguments.tokenType && profileProps.tokenValue === params.arguments.tokenValue) { const profilePath = config.api.profiles.expandPath(profileName); config.delete(`${profilePath}.properties.tokenType`); config.delete(`${profilePath}.properties.tokenValue`); From fcad31362f0257fce3ceb623e2bd380fd36cf297 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Wed, 28 Jun 2023 14:48:56 +0000 Subject: [PATCH 09/26] fix unit and integration test Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- ...t.cli.auth.logout.fruit.integration.test.ts | 5 +++-- .../config/__tests__/ConfigAutoStore.test.ts | 18 ++++++++++++++++++ .../__tests__/BaseAuthHandler.config.test.ts | 2 +- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/__tests__/__integration__/imperative/__tests__/__integration__/cli/auth/imperative.test.cli.auth.logout.fruit.integration.test.ts b/__tests__/__integration__/imperative/__tests__/__integration__/cli/auth/imperative.test.cli.auth.logout.fruit.integration.test.ts index e3504b7e0..37387912b 100644 --- a/__tests__/__integration__/imperative/__tests__/__integration__/cli/auth/imperative.test.cli.auth.logout.fruit.integration.test.ts +++ b/__tests__/__integration__/imperative/__tests__/__integration__/cli/auth/imperative.test.cli.auth.logout.fruit.integration.test.ts @@ -19,9 +19,9 @@ import * as keytar from "keytar"; let TEST_ENVIRONMENT: ITestEnvironment; describe("imperative-test-cli auth logout", () => { async function loadSecureProp(profileName: string): Promise { - const securedValue = await keytar.getPassword("imperative-test-cli", "secure_config_props"); + const securedValue = await keytar.getPassword("imperative-test-cli", "secure_config_props") as string; const securedValueJson = JSON.parse(Buffer.from(securedValue, "base64").toString()); - return Object.values(securedValueJson)[0][`profiles.${profileName}.properties.tokenValue`]; + return Object.values(securedValueJson)[0]?.[`profiles.${profileName}.properties.tokenValue`]; } // Create the unique test environment @@ -121,6 +121,7 @@ describe("imperative-test-cli auth logout", () => { it("should have auth logout command that invalidates another token", async () => { let response = runCliScript(__dirname + "/__scripts__/base_profile_and_auth_login_config_local.sh", TEST_ENVIRONMENT.workingDir + "/testDir", ["fakeUser", "fakePass"]); + expect(response.error).toBeFalsy(); expect(response.stderr.toString()).toBe(""); expect(response.status).toBe(0); diff --git a/packages/config/__tests__/ConfigAutoStore.test.ts b/packages/config/__tests__/ConfigAutoStore.test.ts index bf939d4d4..42f6dc7b5 100644 --- a/packages/config/__tests__/ConfigAutoStore.test.ts +++ b/packages/config/__tests__/ConfigAutoStore.test.ts @@ -67,6 +67,24 @@ describe("ConfigAutoStore tests", () => { expect(authHandler instanceof AbstractAuthHandler).toBe(true); }); + it("should be able to find auth handler for base profile with a dynamic token type", async () => { + await setupConfigToLoad({ + profiles: { + base: { + type: "base", + properties: { + tokenType: SessConstants.TOKEN_TYPE_JWT + ".1" + } + } + }, + defaults: { base: "base" } + }); + + const authHandler = ConfigAutoStore.findAuthHandlerForProfile("profiles.base", {} as any); + expect(authHandler).toBeDefined(); + expect(authHandler instanceof AbstractAuthHandler).toBe(true); + }); + it("should be able to find auth handler for service profile", async () => { await setupConfigToLoad({ profiles: { diff --git a/packages/imperative/src/auth/__tests__/BaseAuthHandler.config.test.ts b/packages/imperative/src/auth/__tests__/BaseAuthHandler.config.test.ts index a77cdeb82..c54f6bd50 100644 --- a/packages/imperative/src/auth/__tests__/BaseAuthHandler.config.test.ts +++ b/packages/imperative/src/auth/__tests__/BaseAuthHandler.config.test.ts @@ -532,7 +532,7 @@ describe("BaseAuthHandler config", () => { expect(caughtError).toBeUndefined(); expect(doLogoutSpy).toBeCalledTimes(1); - expect(writeFileSpy).not.toHaveBeenCalled(); + expect(writeFileSpy).toBeCalledTimes(0); expect(fakeConfig.properties.profiles.fruit.properties.tokenType).toBeDefined(); expect(fakeConfig.properties.profiles.fruit.properties.tokenValue).toBeDefined(); }); From f98f819d141645ba6f1e8fbf406e8a48ba5d0aa8 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Thu, 29 Jun 2023 13:22:40 +0000 Subject: [PATCH 10/26] add unit tests Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- packages/config/__tests__/Config.api.test.ts | 29 ++++++++++++ .../expect/__tests__/ImperativeExpect.test.ts | 20 ++++++++ .../cmd/auto-init/BaseAutoInitHandler.test.ts | 18 ++++++- .../auto-init/__data__/FakeAutoInitHandler.ts | 2 +- .../config/cmd/secure/secure.handler.test.ts | 47 ++++++++++++------- .../auto-init/handlers/BaseAutoInitHandler.ts | 2 +- 6 files changed, 96 insertions(+), 22 deletions(-) diff --git a/packages/config/__tests__/Config.api.test.ts b/packages/config/__tests__/Config.api.test.ts index fc281bd34..e4af6849e 100644 --- a/packages/config/__tests__/Config.api.test.ts +++ b/packages/config/__tests__/Config.api.test.ts @@ -216,6 +216,35 @@ describe("Config API tests", () => { expect(profile).toBeNull(); }); }); + describe("expandPath", () => { + it("should expand a short proeprty path", async () => { + const config = await Config.load(MY_APP); + const profilePath = "lpar1.zosmf"; + expect(config.api.profiles.expandPath(profilePath)).toEqual("profiles.lpar1.profiles.zosmf"); + }); + it.skip("should not expand a complete path", async () => { + const config = await Config.load(MY_APP); + const profilePath = "profiles.lpar1.profiles.zosmf"; + expect(config.api.profiles.expandPath(profilePath)).toEqual("profiles.lpar1.profiles.zosmf"); + }); + it.skip("should expand an incomplete path", async () => { + const config = await Config.load(MY_APP); + const profilePath = "profiles.lpar1.zosmf"; + expect(config.api.profiles.expandPath(profilePath)).toEqual("profiles.lpar1.profiles.zosmf"); + }); + }); + describe("getProfileNameFromPath", () => { + it("should shrink profile paths", async () => { + const config = await Config.load(MY_APP); + const propertyPath = "profiles.lpar1.profiles.zosmf.properties.host"; + expect(config.api.profiles.getProfileNameFromPath(propertyPath)).toEqual("lpar1.zosmf"); + }); + it("should shrink incorrect profile paths", async () => { + const config = await Config.load(MY_APP); + const propertyPath = "profiles.profiles.lpar1.profiles.profiles.zosmf.profiles.properties.host"; + expect(config.api.profiles.getProfileNameFromPath(propertyPath)).toEqual("lpar1.zosmf"); + }); + }); }); describe("plugins", () => { describe("get", () => { diff --git a/packages/expect/__tests__/ImperativeExpect.test.ts b/packages/expect/__tests__/ImperativeExpect.test.ts index 24ca8622a..12599c2d1 100644 --- a/packages/expect/__tests__/ImperativeExpect.test.ts +++ b/packages/expect/__tests__/ImperativeExpect.test.ts @@ -33,6 +33,26 @@ const nestedObj = { }; describe("ImperativeExpect tests", () => { + describe("toMatchRegExp", () => { + it("should not throw an error if the value matches the provided regular expression", () => { + let error: ImperativeError = {} as any; + try { + ImperativeExpect.toMatchRegExp("token", "^token$"); + } catch(thrownError) { + error = thrownError; + } + expect(error).toBeUndefined(); + }); + it("should throw an error if the value does not match the provided regular expression with custom message", () => { + let error: ImperativeError = {} as any; + try { + ImperativeExpect.toMatchRegExp("token", "^token1", "test"); + } catch(thrownError) { + error = thrownError; + } + expect(error.message).toContain("test"); + }); + }); it("Should throw an error for an undefined key when we expect it to be defined", () => { let error: ImperativeError; diff --git a/packages/imperative/__tests__/config/cmd/auto-init/BaseAutoInitHandler.test.ts b/packages/imperative/__tests__/config/cmd/auto-init/BaseAutoInitHandler.test.ts index f32b3b2d9..7c0098991 100644 --- a/packages/imperative/__tests__/config/cmd/auto-init/BaseAutoInitHandler.test.ts +++ b/packages/imperative/__tests__/config/cmd/auto-init/BaseAutoInitHandler.test.ts @@ -17,7 +17,7 @@ import * as jestDiff from "jest-diff"; import stripAnsi = require("strip-ansi"); import { ConfigSchema } from "../../../../../config"; import { CredentialManagerFactory } from "../../../../../security"; -import { SessConstants } from "../../../../../rest"; +import { SessConstants, Session } from "../../../../../rest"; import { OverridesLoader } from "../../../../src/OverridesLoader"; jest.mock("strip-ansi"); @@ -122,11 +122,18 @@ describe("BaseAutoInitHandler", () => { it("should call init with token", async () => { const handler = new FakeAutoInitHandler(); + handler.createSessCfgFromArgs = jest.fn().mockReturnValue({ + hostname: "fakeHost", port: 3000, tokenValue: "fake" + }); const params: IHandlerParameters = { ...mockParams, arguments: { tokenType: "fake", - tokenValue: "fake" + tokenValue: "fake", + user: "toBeDeleted", + password: "toBeDeleted", + cert: "toBeDeleted", + certKey: "toBeDeleted" } } as any; @@ -158,8 +165,15 @@ describe("BaseAutoInitHandler", () => { caughtError = error; } + const mSession: Session = doInitSpy.mock.calls[0][0] as any; + expect(caughtError).toBeUndefined(); expect(doInitSpy).toBeCalledTimes(1); + expect(mSession.ISession.user).toBeUndefined(); + expect(mSession.ISession.password).toBeUndefined(); + expect(mSession.ISession.base64EncodedAuth).toBeUndefined(); + expect(mSession.ISession.cert).toBeUndefined(); + expect(mSession.ISession.certKey).toBeUndefined(); expect(processAutoInitSpy).toBeCalledTimes(1); expect(createSessCfgFromArgsSpy).toBeCalledTimes(1); expect(mockConfigApi.layers.merge).toHaveBeenCalledTimes(1); diff --git a/packages/imperative/__tests__/config/cmd/auto-init/__data__/FakeAutoInitHandler.ts b/packages/imperative/__tests__/config/cmd/auto-init/__data__/FakeAutoInitHandler.ts index e01813d86..75f4b0d1f 100644 --- a/packages/imperative/__tests__/config/cmd/auto-init/__data__/FakeAutoInitHandler.ts +++ b/packages/imperative/__tests__/config/cmd/auto-init/__data__/FakeAutoInitHandler.ts @@ -16,7 +16,7 @@ import { ISession, AbstractSession } from "../../../../../../rest"; export class FakeAutoInitHandler extends BaseAutoInitHandler { public mProfileType: string = "fruit"; - protected createSessCfgFromArgs(args: ICommandArguments): ISession { + public createSessCfgFromArgs(args: ICommandArguments): ISession { return { hostname: "fakeHost", port: 3000 }; } diff --git a/packages/imperative/__tests__/config/cmd/secure/secure.handler.test.ts b/packages/imperative/__tests__/config/cmd/secure/secure.handler.test.ts index 1ff51ee9f..2e2e350ff 100644 --- a/packages/imperative/__tests__/config/cmd/secure/secure.handler.test.ts +++ b/packages/imperative/__tests__/config/cmd/secure/secure.handler.test.ts @@ -9,9 +9,9 @@ * */ -import { IHandlerParameters } from "../../../../.."; +import { IHandlerParameters, Logger } from "../../../../.."; import { Config } from "../../../../../config/src/Config"; -import { IConfig, IConfigOpts } from "../../../../../config"; +import { IConfig, IConfigOpts, IConfigProfile } from "../../../../../config"; import { ImperativeConfig } from "../../../../../utilities"; import { IImperativeConfig } from "../../../../src/doc/IImperativeConfig"; import { ICredentialManagerInit } from "../../../../../security/src/doc/ICredentialManagerInit"; @@ -497,20 +497,22 @@ describe("Configuration Secure command handler", () => { }); describe("special prompting for auth token", () => { + const baseProfile: IConfigProfile = { + type: "base", + properties: { + host: "example.com", + port: 443, + tokenType: SessConstants.TOKEN_TYPE_JWT + }, + secure: [ + "tokenValue" + ] + }; + const expectedConfigObjectWithToken: IConfig = { $schema: "./fakeapp.schema.json", profiles: { - base: { - type: "base", - properties: { - host: "example.com", - port: 443, - tokenType: SessConstants.TOKEN_TYPE_JWT - }, - secure: [ - "tokenValue" - ] - }, + base: baseProfile, }, defaults: { base: "base" @@ -569,6 +571,12 @@ describe("Configuration Secure command handler", () => { it("should invoke auth handler to obtain token and store it securely", async () => { const eco = lodash.cloneDeep(expectedConfigObjectWithToken); + // Create another base profile and mock the loggers to test multiple login operations in a single config-secure + eco.profiles["base2"] = baseProfile; + const dummyLogger: any = {debug: jest.fn(), info: jest.fn()}; + jest.spyOn(Logger, "getImperativeLogger").mockReturnValue(dummyLogger); + jest.spyOn(Logger, "getAppLogger").mockReturnValue(dummyLogger); + readFileSyncSpy.mockReturnValueOnce(JSON.stringify(eco)); existsSyncSpy.mockReturnValueOnce(false).mockReturnValueOnce(true).mockReturnValue(false); // Only the project config exists writeFileSyncSpy.mockImplementation(); @@ -576,7 +584,8 @@ describe("Configuration Secure command handler", () => { await setupConfigToLoad(undefined, configOpts); // Setup the config - jest.spyOn(ImperativeConfig.instance, "loadedConfig", "get").mockReturnValueOnce({ + jest.spyOn(ImperativeConfig.instance, "loadedConfig", "get").mockReturnValue({ + ...fakeConfig, profiles: [{ type: "base", authConfig: [{ handler: authHandlerPath } as any] @@ -588,7 +597,8 @@ describe("Configuration Secure command handler", () => { const fakeSecureDataExpectedJson = lodash.cloneDeep(fakeSecureDataJson); delete fakeSecureDataExpectedJson[fakeProjPath]; fakeSecureDataExpectedJson[fakeProjPath] = { - "profiles.base.properties.tokenValue": "fakeLoginData" + "profiles.base.properties.tokenValue": "fakeLoginData", + "profiles.base2.properties.tokenValue": "fakeLoginData" }; const fakeSecureDataExpected = Buffer.from(JSON.stringify(fakeSecureDataExpectedJson)).toString("base64"); @@ -597,13 +607,14 @@ describe("Configuration Secure command handler", () => { compObj.$schema = "./fakeapp.schema.json"; // Fill in the name of the schema file, and make it first lodash.merge(compObj, ImperativeConfig.instance.config.properties); // Add the properties from the config delete compObj.profiles.base.properties.tokenValue; // Delete the secret + delete compObj.profiles.base2.properties.tokenValue; // Delete the secret expect(keytarDeletePasswordSpy).toHaveBeenCalledTimes(process.platform === "win32" ? 4 : 3); expect(keytarGetPasswordSpy).toHaveBeenCalledTimes(1); expect(keytarSetPasswordSpy).toHaveBeenCalledTimes(1); - expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(2); // User and password - expect(mockAuthHandlerApi.createSessCfg).toHaveBeenCalledTimes(1); - expect(mockAuthHandlerApi.sessionLogin).toHaveBeenCalledTimes(1); + expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(4); // User and password + expect(mockAuthHandlerApi.createSessCfg).toHaveBeenCalledTimes(2); + expect(mockAuthHandlerApi.sessionLogin).toHaveBeenCalledTimes(2); expect(keytarSetPasswordSpy).toHaveBeenCalledWith("Zowe", "secure_config_props", fakeSecureDataExpected); expect(writeFileSyncSpy).toHaveBeenCalledTimes(1); expect(writeFileSyncSpy).toHaveBeenNthCalledWith(1, fakeProjPath, JSON.stringify(compObj, null, 4)); // Config diff --git a/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts b/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts index 9154ce856..571209cfe 100644 --- a/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts +++ b/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts @@ -90,7 +90,7 @@ export abstract class BaseAutoInitHandler implements ICommandHandler { ); this.mSession = new Session(sessCfgWithCreds); if (this.mSession.ISession.tokenValue) { - this.mSession.ISession.user = this.mSession.ISession.password = this.mSession.ISession.cert = this.mSession.ISession.certKey = undefined; + this.mSession.ISession.base64EncodedAuth = this.mSession.ISession.user = this.mSession.ISession.password = this.mSession.ISession.cert = this.mSession.ISession.certKey = undefined; } // Use params to set which config layer to apply to From 433cea2c20ca1481f53e8432ed1856b16dbb36b3 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Fri, 30 Jun 2023 11:32:32 +0000 Subject: [PATCH 11/26] Prevent a breaking change on multiple logout operations with V1 profiles Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- .../expect/__tests__/ImperativeExpect.test.ts | 2 +- .../auth/__tests__/BaseAuthHandler.test.ts | 52 ++++++++++++++++++- .../src/auth/handlers/BaseAuthHandler.ts | 36 +++++++------ 3 files changed, 72 insertions(+), 18 deletions(-) diff --git a/packages/expect/__tests__/ImperativeExpect.test.ts b/packages/expect/__tests__/ImperativeExpect.test.ts index 12599c2d1..9c13f1657 100644 --- a/packages/expect/__tests__/ImperativeExpect.test.ts +++ b/packages/expect/__tests__/ImperativeExpect.test.ts @@ -41,7 +41,7 @@ describe("ImperativeExpect tests", () => { } catch(thrownError) { error = thrownError; } - expect(error).toBeUndefined(); + expect(error).toEqual({}); }); it("should throw an error if the value does not match the provided regular expression with custom message", () => { let error: ImperativeError = {} as any; diff --git a/packages/imperative/src/auth/__tests__/BaseAuthHandler.test.ts b/packages/imperative/src/auth/__tests__/BaseAuthHandler.test.ts index 56f6328a0..e05e1e899 100644 --- a/packages/imperative/src/auth/__tests__/BaseAuthHandler.test.ts +++ b/packages/imperative/src/auth/__tests__/BaseAuthHandler.test.ts @@ -144,6 +144,52 @@ describe("BaseAuthHandler", () => { }); it("should process logout successfully", async () => { + const handler = new FakeAuthHandler(); + const params: IHandlerParameters = { + response: { + console: { + log: jest.fn() + } + }, + arguments: { + host: "fakeHost", + port: "fakePort", + // Purposely remove the token type to test that we still provide a default token type + tokenValue: "fakeToken", + // Add user and password to prove that they get removed and are not required for logout + user: "fakeUser", + password: "fakePass" + }, + positionals: ["auth", "logout"], + profiles: { + getMeta: jest.fn(() => ({ + name: "fakeName", + profile: { + tokenValue: "fakeToken" + } + })) + } + } as any; + + const doLogoutSpy = jest.spyOn(handler as any, "doLogout"); + const processLogoutOldSpy = jest.spyOn(handler as any, "processLogoutOld"); + let caughtError; + + try { + await handler.process(params); + } catch (error) { + caughtError = error; + } + + expect(caughtError).toBeUndefined(); + expect(doLogoutSpy).toBeCalledTimes(1); + expect(processLogoutOldSpy).toBeCalledTimes(1); + expect(params.arguments.tokenType).toEqual(handler.mDefaultTokenType); + expect(params.arguments.user).toBeUndefined(); + expect(params.arguments.password).toBeUndefined(); + }); + + it("should process logout successfully even when tokenValue is not provided", async () => { const handler = new FakeAuthHandler(); const params: IHandlerParameters = { response: { @@ -155,7 +201,7 @@ describe("BaseAuthHandler", () => { host: "fakeHost", port: "fakePort", tokenType: handler.mDefaultTokenType, - tokenValue: "fakeToken" + tokenValue: null, }, positionals: ["auth", "logout"], profiles: { @@ -166,6 +212,7 @@ describe("BaseAuthHandler", () => { } as any; const doLogoutSpy = jest.spyOn(handler as any, "doLogout"); + const processLogoutOldSpy = jest.spyOn(handler as any, "processLogoutOld"); let caughtError; try { @@ -175,7 +222,8 @@ describe("BaseAuthHandler", () => { } expect(caughtError).toBeUndefined(); - expect(doLogoutSpy).toBeCalledTimes(1); + expect(doLogoutSpy).toBeCalledTimes(0); + expect(processLogoutOldSpy).toBeCalledTimes(1); }); it("should fail to process invalid action name", async () => { diff --git a/packages/imperative/src/auth/handlers/BaseAuthHandler.ts b/packages/imperative/src/auth/handlers/BaseAuthHandler.ts index 5f155c453..0b8ec79c9 100644 --- a/packages/imperative/src/auth/handlers/BaseAuthHandler.ts +++ b/packages/imperative/src/auth/handlers/BaseAuthHandler.ts @@ -196,31 +196,37 @@ export abstract class BaseAuthHandler extends AbstractAuthHandler { * @param {IHandlerParameters} params Command parameters sent by imperative. */ protected async processLogout(params: IHandlerParameters) { - if (params.arguments.tokenValue != null) { - // Force to use of token value, in case user and/or password also on base profile, make user undefined. - if (params.arguments.user != null) { - params.arguments.user = undefined; - } + // Force the use of token value, in case user and/or password are also provided. + if (params.arguments.tokenValue != null && + (params.arguments.user != null || params.arguments.password != null)) { + params.arguments.user = undefined; + params.arguments.password = undefined; + } - if (params.arguments.tokenType == null) { - params.arguments.tokenType = this.mDefaultTokenType; - } + if (params.arguments.tokenType == null) { + params.arguments.tokenType = this.mDefaultTokenType; + } - const sessCfg: ISession = this.createSessCfgFromArgs( - params.arguments - ); + const sessCfg: ISession = this.createSessCfgFromArgs(params.arguments); - const sessCfgWithCreds = await ConnectionPropsForSessCfg.addPropsOrPrompt( - sessCfg, params.arguments, + const sessCfgWithCreds = await ConnectionPropsForSessCfg.addPropsOrPrompt( + sessCfg, params.arguments, { requestToken: false, doPrompting: false, parms: params }, - ); + ); + if (params.arguments.tokenValue != null) { this.mSession = new Session(sessCfgWithCreds); - await this.doLogout(this.mSession); } if (!ImperativeConfig.instance.config.exists) { + if (sessCfgWithCreds.tokenValue == null) { + // Provide dummy token information to prevent multiple V1 logout operations from failing + sessCfgWithCreds.type = SessConstants.AUTH_TYPE_TOKEN; + sessCfgWithCreds.tokenType = this.mDefaultTokenType; + sessCfgWithCreds.tokenValue = SessConstants.AUTH_TYPE_TOKEN; + } + this.mSession = new Session(sessCfgWithCreds); await this.processLogoutOld(params); } else { const config = ImperativeConfig.instance.config; From 71cb2662b7ad2085f646f90183ee147e0bc93b01 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Fri, 30 Jun 2023 11:53:59 +0000 Subject: [PATCH 12/26] update changelog Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74155a9f4..05fa53d09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ All notable changes to the Imperative package will be documented in this file. +## Recent Changes + +- Enhancement: Handled unique cookie identifier in the form of dynamic token types. [#966](https://github.com/zowe/imperative/pull/996) +- Enhancement: Added a new utility method to `ImperativeExpect` to match regular expressions. [#966](https://github.com/zowe/imperative/pull/996) +- Enhancement: Added support for mutliple login operations in a single `config secure` command execution. [#966](https://github.com/zowe/imperative/pull/996) +- BugFix: Allowed for multiple `auth logout` operations. [#966](https://github.com/zowe/imperative/pull/996) +- BugFix: Prevented `auto-init` from sending two `login` requests to the server. [#966](https://github.com/zowe/imperative/pull/996) + + ## `5.15.0` - Enhancement: Enabled users to display errors in a more user-friendly format with the ZOWE_V3_ERR_FORMAT environment variable. [zowe-cli#935](https://github.com/zowe/zowe-cli/issues/935) From 9571e980021d850fde1e8dc8e9634318d54fc135 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Wed, 5 Jul 2023 13:07:06 +0000 Subject: [PATCH 13/26] address PR comments Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- packages/config/__tests__/Config.api.test.ts | 17 ++++++----------- packages/config/src/api/ConfigProfiles.ts | 9 ++++++--- .../src/auth/handlers/BaseAuthHandler.ts | 2 +- .../auto-init/handlers/BaseAutoInitHandler.ts | 4 +++- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/config/__tests__/Config.api.test.ts b/packages/config/__tests__/Config.api.test.ts index e4af6849e..120166a7d 100644 --- a/packages/config/__tests__/Config.api.test.ts +++ b/packages/config/__tests__/Config.api.test.ts @@ -222,15 +222,10 @@ describe("Config API tests", () => { const profilePath = "lpar1.zosmf"; expect(config.api.profiles.expandPath(profilePath)).toEqual("profiles.lpar1.profiles.zosmf"); }); - it.skip("should not expand a complete path", async () => { + it("should expand a path with the keyword profiles", async () => { const config = await Config.load(MY_APP); - const profilePath = "profiles.lpar1.profiles.zosmf"; - expect(config.api.profiles.expandPath(profilePath)).toEqual("profiles.lpar1.profiles.zosmf"); - }); - it.skip("should expand an incomplete path", async () => { - const config = await Config.load(MY_APP); - const profilePath = "profiles.lpar1.zosmf"; - expect(config.api.profiles.expandPath(profilePath)).toEqual("profiles.lpar1.profiles.zosmf"); + const profilePath = "profiles.zosmf"; + expect(config.api.profiles.expandPath(profilePath)).toEqual("profiles.profiles.profiles.zosmf"); }); }); describe("getProfileNameFromPath", () => { @@ -239,10 +234,10 @@ describe("Config API tests", () => { const propertyPath = "profiles.lpar1.profiles.zosmf.properties.host"; expect(config.api.profiles.getProfileNameFromPath(propertyPath)).toEqual("lpar1.zosmf"); }); - it("should shrink incorrect profile paths", async () => { + it("should shrink profile paths with the keyword profiles", async () => { const config = await Config.load(MY_APP); - const propertyPath = "profiles.profiles.lpar1.profiles.profiles.zosmf.profiles.properties.host"; - expect(config.api.profiles.getProfileNameFromPath(propertyPath)).toEqual("lpar1.zosmf"); + const propertyPath = "profiles.profiles.profiles.zosmf.properties.host"; + expect(config.api.profiles.getProfileNameFromPath(propertyPath)).toEqual("profiles.zosmf"); }); }); }); diff --git a/packages/config/src/api/ConfigProfiles.ts b/packages/config/src/api/ConfigProfiles.ts index 5a99ce61c..92643494b 100644 --- a/packages/config/src/api/ConfigProfiles.ts +++ b/packages/config/src/api/ConfigProfiles.ts @@ -123,10 +123,13 @@ export class ConfigProfiles extends ConfigApi { */ public getProfileNameFromPath(path: string): string { let profileName = ""; - for (const p of path.split(".")) { - if (p === "profiles") continue; + const segments = path.split("."); + for (let i = 0; i < segments.length; i++) { + const p = segments[i]; if (p === "properties") break; - profileName += profileName.length > 0 ? "." + p : p; + if (i%2) { + profileName += profileName.length > 0 ? "." + p : p; + } } return profileName; } diff --git a/packages/imperative/src/auth/handlers/BaseAuthHandler.ts b/packages/imperative/src/auth/handlers/BaseAuthHandler.ts index 0b8ec79c9..0595f4f26 100644 --- a/packages/imperative/src/auth/handlers/BaseAuthHandler.ts +++ b/packages/imperative/src/auth/handlers/BaseAuthHandler.ts @@ -211,7 +211,7 @@ export abstract class BaseAuthHandler extends AbstractAuthHandler { const sessCfgWithCreds = await ConnectionPropsForSessCfg.addPropsOrPrompt( sessCfg, params.arguments, - { requestToken: false, doPrompting: false, parms: params }, + { requestToken: false, doPrompting: false, parms: params }, ); if (params.arguments.tokenValue != null) { diff --git a/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts b/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts index 571209cfe..b3e0d249a 100644 --- a/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts +++ b/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts @@ -90,7 +90,9 @@ export abstract class BaseAutoInitHandler implements ICommandHandler { ); this.mSession = new Session(sessCfgWithCreds); if (this.mSession.ISession.tokenValue) { - this.mSession.ISession.base64EncodedAuth = this.mSession.ISession.user = this.mSession.ISession.password = this.mSession.ISession.cert = this.mSession.ISession.certKey = undefined; + this.mSession.ISession.base64EncodedAuth = + this.mSession.ISession.user = this.mSession.ISession.password = + this.mSession.ISession.cert = this.mSession.ISession.certKey = undefined; } // Use params to set which config layer to apply to From c248e164ae1d2b96844a13ed669f754bd0c59077 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Wed, 5 Jul 2023 18:35:59 +0000 Subject: [PATCH 14/26] addres PR comments Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- CHANGELOG.md | 2 +- .../config/__tests__/ConfigAutoStore.test.ts | 21 +++++++++++++++++-- packages/config/src/ConfigAutoStore.ts | 9 ++++---- packages/config/src/ProfileInfo.ts | 4 ++-- packages/config/src/api/ConfigLayers.ts | 2 +- packages/config/src/api/ConfigProfiles.ts | 13 ++++++++++++ packages/config/src/api/ConfigSecure.ts | 2 +- .../src/auth/handlers/BaseAuthHandler.ts | 4 ++-- 8 files changed, 44 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9140d56c1..01156e3b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to the Imperative package will be documented in this file. - Enhancement: Handled unique cookie identifier in the form of dynamic token types. [#966](https://github.com/zowe/imperative/pull/996) - Enhancement: Added a new utility method to `ImperativeExpect` to match regular expressions. [#966](https://github.com/zowe/imperative/pull/996) -- Enhancement: Added support for mutliple login operations in a single `config secure` command execution. [#966](https://github.com/zowe/imperative/pull/996) +- Enhancement: Added support for multiple login operations in a single `config secure` command execution. [#966](https://github.com/zowe/imperative/pull/996) - BugFix: Allowed for multiple `auth logout` operations. [#966](https://github.com/zowe/imperative/pull/996) - BugFix: Prevented `auto-init` from sending two `login` requests to the server. [#966](https://github.com/zowe/imperative/pull/996) diff --git a/packages/config/__tests__/ConfigAutoStore.test.ts b/packages/config/__tests__/ConfigAutoStore.test.ts index 42f6dc7b5..9cace3ea2 100644 --- a/packages/config/__tests__/ConfigAutoStore.test.ts +++ b/packages/config/__tests__/ConfigAutoStore.test.ts @@ -67,13 +67,13 @@ describe("ConfigAutoStore tests", () => { expect(authHandler instanceof AbstractAuthHandler).toBe(true); }); - it("should be able to find auth handler for base profile with a dynamic token type", async () => { + it("should be able to find auth handler for base profile with a dynamic APIML token type", async () => { await setupConfigToLoad({ profiles: { base: { type: "base", properties: { - tokenType: SessConstants.TOKEN_TYPE_JWT + ".1" + tokenType: SessConstants.TOKEN_TYPE_APIML + ".1" } } }, @@ -85,6 +85,23 @@ describe("ConfigAutoStore tests", () => { expect(authHandler instanceof AbstractAuthHandler).toBe(true); }); + it("should not be able to find auth handler for base profile with a dynamic JWT token type", async () => { + await setupConfigToLoad({ + profiles: { + base: { + type: "base", + properties: { + tokenType: SessConstants.TOKEN_TYPE_JWT + ".1" + } + } + }, + defaults: { base: "base" } + }); + + const authHandler = ConfigAutoStore.findAuthHandlerForProfile("profiles.base", {} as any); + expect(authHandler).toBeUndefined(); + }); + it("should be able to find auth handler for service profile", async () => { await setupConfigToLoad({ profiles: { diff --git a/packages/config/src/ConfigAutoStore.ts b/packages/config/src/ConfigAutoStore.ts index 97fadd52e..b5d5d165e 100644 --- a/packages/config/src/ConfigAutoStore.ts +++ b/packages/config/src/ConfigAutoStore.ts @@ -25,6 +25,7 @@ import { IConfigAutoStoreFindAuthHandlerForProfileOpts, IConfigAutoStoreStoreSessCfgPropsOpts } from "./doc/IConfigAutoStoreOpts"; +import { TOKEN_TYPE_APIML } from "../../rest/src/session/SessConstants"; /** * Class to manage automatic storage of properties in team config. @@ -89,7 +90,7 @@ export class ConfigAutoStore { return; } else if (profile.tokenType == null) { // If tokenType undefined in service profile, fall back to base profile const baseProfileName = ConfigUtils.getActiveProfileName("base", opts.cmdArguments, opts.defaultBaseProfileName); - return this._findAuthHandlerForProfile({ ...opts, profilePath: config.api.profiles.expandPath(baseProfileName) }); + return this._findAuthHandlerForProfile({ ...opts, profilePath: config.api.profiles.getProfilePathFromName(baseProfileName) }); } } @@ -106,7 +107,7 @@ export class ConfigAutoStore { if (authHandlerClass instanceof AbstractAuthHandler) { const { promptParams } = authHandlerClass.getAuthHandlerApi(); - if (profile.tokenType.startsWith(promptParams.defaultTokenType)) { + if (profile.tokenType === promptParams.defaultTokenType || profile.tokenType.startsWith(TOKEN_TYPE_APIML)) { return authHandlerClass; // Auth service must have matching token type } } @@ -140,7 +141,7 @@ export class ConfigAutoStore { return; } const [profileType, profileName] = profileData ?? [opts.profileType, opts.profileName]; - const profilePath = config.api.profiles.expandPath(profileName); + const profilePath = config.api.profiles.getProfilePathFromName(profileName); // Replace user and password with tokenValue if tokenType is defined in config if (profileProps.includes("user") && profileProps.includes("password") && await this._fetchTokenForSessCfg({ ...opts, profilePath })) { @@ -179,7 +180,7 @@ export class ConfigAutoStore { (propName === "tokenValue" && profileObj.tokenType == null && baseProfileObj.tokenType != null || profileType === "base") ) { - propProfilePath = config.api.profiles.expandPath(baseProfileName); + propProfilePath = config.api.profiles.getProfilePathFromName(baseProfileName); isSecureProp = baseProfileSchema.properties[propName].secure || baseProfileSecureProps.includes(propName); } diff --git a/packages/config/src/ProfileInfo.ts b/packages/config/src/ProfileInfo.ts index f663b512b..e85ad1038 100644 --- a/packages/config/src/ProfileInfo.ts +++ b/packages/config/src/ProfileInfo.ts @@ -449,7 +449,7 @@ export class ProfileInfo { const foundProfNm = configProperties.defaults[profileType]; // for a team config, we use the last node of the jsonLoc as the name - const foundJson = this.mLoadedConfig.api.profiles.expandPath(foundProfNm); + const foundJson = this.mLoadedConfig.api.profiles.getProfilePathFromName(foundProfNm); const teamOsLocation: string[] = this.findTeamOsLocation(foundJson); // assign the required poperties to defaultProfile @@ -1330,7 +1330,7 @@ export class ProfileInfo { * @param opts Set of options that allow this method to get the profile location */ private argTeamConfigLoc(opts: IArgTeamConfigLoc): [IProfLoc, boolean] { - const segments = this.mLoadedConfig.api.profiles.expandPath(opts.profileName).split(".profiles."); + const segments = this.mLoadedConfig.api.profiles.getProfilePathFromName(opts.profileName).split(".profiles."); let osLocInfo: IProfLocOsLocLayer; if (opts.osLocInfo?.user != null || opts.osLocInfo?.global != null) osLocInfo = { user: opts.osLocInfo?.user, global: opts.osLocInfo?.global }; diff --git a/packages/config/src/api/ConfigLayers.ts b/packages/config/src/api/ConfigLayers.ts index 745a370db..3e90e8337 100644 --- a/packages/config/src/api/ConfigLayers.ts +++ b/packages/config/src/api/ConfigLayers.ts @@ -196,7 +196,7 @@ export class ConfigLayers extends ConfigApi { * @returns User and global properties, or undefined if profile does not exist */ public find(profileName: string): { user: boolean, global: boolean } { - const profilePath = this.mConfig.api.profiles.expandPath(profileName); + const profilePath = this.mConfig.api.profiles.getProfilePathFromName(profileName); for (const layer of this.mConfig.layers) { if (lodash.get(layer.properties, profilePath) != null) { return layer; diff --git a/packages/config/src/api/ConfigProfiles.ts b/packages/config/src/api/ConfigProfiles.ts index 92643494b..3ccaadcef 100644 --- a/packages/config/src/api/ConfigProfiles.ts +++ b/packages/config/src/api/ConfigProfiles.ts @@ -105,8 +105,21 @@ export class ConfigProfiles extends ConfigApi { * * @returns The expanded path. * + * @deprecated Please use getProfilePathFromName */ public expandPath(shortPath: string): string { + return this.getProfilePathFromName(shortPath); + } + + // _______________________________________________________________________ + /** + * Expands a short path into an expanded path. + * + * @param shortPath The short path. + * + * @returns The expanded path. + */ + public getProfilePathFromName(shortPath: string): string { return shortPath.replace(/(^|\.)/g, "$1profiles."); } diff --git a/packages/config/src/api/ConfigSecure.ts b/packages/config/src/api/ConfigSecure.ts index 1e2f997e0..3251e880a 100644 --- a/packages/config/src/api/ConfigSecure.ts +++ b/packages/config/src/api/ConfigSecure.ts @@ -201,7 +201,7 @@ export class ConfigSecure extends ConfigApi { * @returns Array of secure property names */ public securePropsForProfile(profileName: string) { - const profilePath = this.mConfig.api.profiles.expandPath(profileName); + const profilePath = this.mConfig.api.profiles.getProfilePathFromName(profileName); const secureProps = []; for (const propPath of this.secureFields()) { const pathSegments = propPath.split("."); // profiles.XXX.properties.YYY diff --git a/packages/imperative/src/auth/handlers/BaseAuthHandler.ts b/packages/imperative/src/auth/handlers/BaseAuthHandler.ts index 0595f4f26..0dd46e920 100644 --- a/packages/imperative/src/auth/handlers/BaseAuthHandler.ts +++ b/packages/imperative/src/auth/handlers/BaseAuthHandler.ts @@ -153,7 +153,7 @@ export abstract class BaseAuthHandler extends AbstractAuthHandler { } } - const profilePath = config.api.profiles.expandPath(profileName); + const profilePath = config.api.profiles.getProfilePathFromName(profileName); config.set(`${profilePath}.properties.tokenType`, this.mSession.ISession.tokenType); config.set(`${profilePath}.properties.tokenValue`, tokenValue, { secure: true }); @@ -237,7 +237,7 @@ export abstract class BaseAuthHandler extends AbstractAuthHandler { // If you specified a token on the command line, then don't delete the one in the profile if it doesn't match if (Object.keys(profileProps).length > 0 && profileProps.tokenType != null && profileProps.tokenValue != null && profileProps.tokenType === params.arguments.tokenType && profileProps.tokenValue === params.arguments.tokenValue) { - const profilePath = config.api.profiles.expandPath(profileName); + const profilePath = config.api.profiles.getProfilePathFromName(profileName); config.delete(`${profilePath}.properties.tokenType`); config.delete(`${profilePath}.properties.tokenValue`); From 1b33b862d6eae3af2172361668e4687e839a76d3 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Fri, 7 Jul 2023 15:54:06 +0000 Subject: [PATCH 15/26] Address PR comment Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- .../config/cmd/init/init.handler.test.ts | 18 +++++++++--------- .../src/config/cmd/init/init.handler.ts | 2 +- .../src/config/cmd/secure/secure.handler.ts | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/imperative/__tests__/config/cmd/init/init.handler.test.ts b/packages/imperative/__tests__/config/cmd/init/init.handler.test.ts index 5f9c669ee..1cdff1cb0 100644 --- a/packages/imperative/__tests__/config/cmd/init/init.handler.test.ts +++ b/packages/imperative/__tests__/config/cmd/init/init.handler.test.ts @@ -152,7 +152,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(user ? 0 : 1); // User config is a skeleton - no prompting should occur // Prompting for secure property - if (!user) expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("blank to skip:"), {"hideText": true}); + if (!user) expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); expect(editFileSpy).not.toHaveBeenCalled(); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); @@ -241,7 +241,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(user ? 0 : 1); // Prompting for secure property - if (!user) expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("blank to skip:"), {"hideText": true}); + if (!user) expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); @@ -449,7 +449,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(1); // Prompting for secure property - expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("blank to skip:"), {"hideText": true}); + expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); // 1 = Schema and 2 = Config @@ -493,7 +493,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(1); // Prompting for secure property - expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("blank to skip:"), {"hideText": true}); + expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); // 1 = Schema and 2 = Config @@ -539,7 +539,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(1); // Prompting for secure property - expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("blank to skip:"), {"hideText": true}); + expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); // 1 = Schema and 2 = Config @@ -583,7 +583,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(1); // Prompting for secure property - expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("blank to skip:"), {"hideText": true}); + expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); // 1 = Schema and 2 = Config @@ -637,7 +637,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(1); // Prompting for secure property - expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("blank to skip:"), {"hideText": true}); + expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); // 1 = Schema and 2 = Config @@ -691,7 +691,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(1); // Prompting for secure property - expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("blank to skip:"), {"hideText": true}); + expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); // 1 = Schema and 2 = Config @@ -747,7 +747,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(2); // Prompting for secure property - expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("blank to skip:"), {"hideText": true}); + expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); // 1 = Schema and 2 = Config diff --git a/packages/imperative/src/config/cmd/init/init.handler.ts b/packages/imperative/src/config/cmd/init/init.handler.ts index d04b29b1d..d5649e009 100644 --- a/packages/imperative/src/config/cmd/init/init.handler.ts +++ b/packages/imperative/src/config/cmd/init/init.handler.ts @@ -186,7 +186,7 @@ export default class InitHandler implements ICommandHandler { } this.promptProps.push(propName); - const propValue: any = await this.params.response.console.prompt(`Enter ${propDesc} - blank to skip: `, { hideText: property.secure }); + const propValue: any = await this.params.response.console.prompt(`Enter ${propDesc} - Press ENTER to skip: `, { hideText: property.secure }); // coerce to correct type if (propValue && propValue.trim().length > 0) { diff --git a/packages/imperative/src/config/cmd/secure/secure.handler.ts b/packages/imperative/src/config/cmd/secure/secure.handler.ts index af9de5b79..a91b4a5f7 100644 --- a/packages/imperative/src/config/cmd/secure/secure.handler.ts +++ b/packages/imperative/src/config/cmd/secure/secure.handler.ts @@ -61,14 +61,14 @@ export default class SecureHandler implements ICommandHandler { if (propName.endsWith(".tokenValue")) { params.response.console.log(`Processing secure properties for profile: ${config.api.profiles.getProfileNameFromPath(propName)}`); const propValue = await this.handlePromptForAuthToken(config, propName) || - await params.response.console.prompt(`Enter ${propName} - blank to skip: `, {hideText: true}); + await params.response.console.prompt(`Enter ${propName} - Press ENTER to skip: `, {hideText: true}); // Save the value in the config securely if (propValue) { config.set(propName, propValue, { secure: true }); } } else { - let propValue = await params.response.console.prompt(`Enter ${propName} - blank to skip: `, { hideText: true }); + let propValue = await params.response.console.prompt(`Enter ${propName} - Press ENTER to skip: `, { hideText: true }); // Save the value in the config securely if (propValue) { From 1f707fc08ca8cd7131561f480cbf6a72c1e4c5c4 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Wed, 5 Jul 2023 13:44:06 +0000 Subject: [PATCH 16/26] prevent throwing an error if no credentials are provided Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- packages/config/src/ConfigAutoStore.ts | 12 ++++------- .../src/config/cmd/secure/secure.handler.ts | 18 ++++++++++------- .../src/session/ConnectionPropsForSessCfg.ts | 20 +++++++++++++++++++ 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/packages/config/src/ConfigAutoStore.ts b/packages/config/src/ConfigAutoStore.ts index b5d5d165e..c822a67f2 100644 --- a/packages/config/src/ConfigAutoStore.ts +++ b/packages/config/src/ConfigAutoStore.ts @@ -79,19 +79,15 @@ export class ConfigAutoStore { const profileType = lodash.get(config.properties, `${opts.profilePath}.type`); const profile = config.api.profiles.get(opts.profilePath.replace(/profiles\./g, ""), false); - if (profile == null || profileType == null) { // Profile must exist and have type defined + if (profile == null || (profileType == null && profile.tokenType == null)) { // Profile must exist and have type defined return; } else if (profileType === "base") { if (profile.tokenType == null) { // Base profile must have tokenType defined return; } - } else { - if (profile.basePath == null) { // Service profiles must have basePath defined - return; - } else if (profile.tokenType == null) { // If tokenType undefined in service profile, fall back to base profile - const baseProfileName = ConfigUtils.getActiveProfileName("base", opts.cmdArguments, opts.defaultBaseProfileName); - return this._findAuthHandlerForProfile({ ...opts, profilePath: config.api.profiles.getProfilePathFromName(baseProfileName) }); - } + } else if (profile.tokenType == null) { // If tokenType undefined in service profile, fall back to base profile + const baseProfileName = ConfigUtils.getActiveProfileName("base", opts.cmdArguments, opts.defaultBaseProfileName); + return this._findAuthHandlerForProfile({ ...opts, profilePath: config.api.profiles.getProfilePathFromName(baseProfileName) }); } const authConfigs: ICommandProfileAuthConfig[] = []; diff --git a/packages/imperative/src/config/cmd/secure/secure.handler.ts b/packages/imperative/src/config/cmd/secure/secure.handler.ts index a91b4a5f7..ea3f53fef 100644 --- a/packages/imperative/src/config/cmd/secure/secure.handler.ts +++ b/packages/imperative/src/config/cmd/secure/secure.handler.ts @@ -106,13 +106,17 @@ export default class SecureHandler implements ICommandHandler { { parms: this.params, doPrompting: true, requestToken: true, ...api.promptParams }); Logger.getAppLogger().info(`Fetching ${profile.tokenType} for ${propPath}`); - try { - return await api.sessionLogin(new Session(sessCfgWithCreds)); - } catch (error) { - throw new ImperativeError({ - msg: `Failed to fetch ${profile.tokenType} for ${propPath}: ${error.message}`, - causeErrors: error - }); + if (ConnectionPropsForSessCfg.sessHasCreds(sessCfgWithCreds)) { + try { + return await api.sessionLogin(new Session(sessCfgWithCreds)); + } catch (error) { + throw new ImperativeError({ + msg: `Failed to fetch ${profile.tokenType} for ${propPath}: ${error.message}`, + causeErrors: error + }); + } + } else { + this.params.response.console.log("No credentials provided."); } } } diff --git a/packages/rest/src/session/ConnectionPropsForSessCfg.ts b/packages/rest/src/session/ConnectionPropsForSessCfg.ts index a2c971c23..1d319ece2 100644 --- a/packages/rest/src/session/ConnectionPropsForSessCfg.ts +++ b/packages/rest/src/session/ConnectionPropsForSessCfg.ts @@ -303,6 +303,26 @@ export class ConnectionPropsForSessCfg { ConnectionPropsForSessCfg.logSessCfg(sessCfg); } + // *********************************************************************** + /** + * Confirm whether the given session has a credentials. + * + * @param sessToTest + * the session to be confirmed. + * + * @returns true is the session has credentials. false otherwise. + */ + public static sessHasCreds(sessToTest: ISession) { + if (sessToTest == null) { + return false; + } + const hasToken = sessToTest.tokenType != null && sessToTest.tokenValue != null; + const hasCert = sessToTest.certKey != null && sessToTest.cert; + const hasBasicAuth = sessToTest.base64EncodedAuth != null; + const hasCreds = sessToTest.user != null && sessToTest.password; + return hasToken || hasCert || hasBasicAuth || hasCreds; + } + /** * List of properties on `sessCfg` object that should be kept secret and * may not appear in Imperative log files. From ddb2d45a86b4237702e1614bf82fc866d8b56d1f Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Wed, 5 Jul 2023 13:51:30 +0000 Subject: [PATCH 17/26] avoid prompting for tokenValue if no creds are provided Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- .../src/config/cmd/secure/secure.handler.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/imperative/src/config/cmd/secure/secure.handler.ts b/packages/imperative/src/config/cmd/secure/secure.handler.ts index ea3f53fef..007b7d756 100644 --- a/packages/imperative/src/config/cmd/secure/secure.handler.ts +++ b/packages/imperative/src/config/cmd/secure/secure.handler.ts @@ -60,8 +60,10 @@ export default class SecureHandler implements ICommandHandler { for (const propName of secureProps) { if (propName.endsWith(".tokenValue")) { params.response.console.log(`Processing secure properties for profile: ${config.api.profiles.getProfileNameFromPath(propName)}`); - const propValue = await this.handlePromptForAuthToken(config, propName) || - await params.response.console.prompt(`Enter ${propName} - Press ENTER to skip: `, {hideText: true}); + let propValue = await this.handlePromptForAuthToken(config, propName); + if (propValue === undefined) { + propValue = await params.response.console.prompt(`Enter ${propName} - Press ENTER to skip: `, {hideText: true}); + } // Save the value in the config securely if (propValue) { @@ -97,7 +99,7 @@ export default class SecureHandler implements ICommandHandler { if (authHandlerClass != null) { const api = authHandlerClass.getAuthHandlerApi(); if (api.promptParams.serviceDescription != null) { - this.params.response.console.log(`Logging in to ${api.promptParams.serviceDescription}`); + this.params.response.console.log(`Logging in to ${api.promptParams.serviceDescription} - blank to skip:`); } const profile = config.api.profiles.get(profilePath.replace(/profiles\./g, ""), false); @@ -108,7 +110,9 @@ export default class SecureHandler implements ICommandHandler { if (ConnectionPropsForSessCfg.sessHasCreds(sessCfgWithCreds)) { try { - return await api.sessionLogin(new Session(sessCfgWithCreds)); + const tokenValue = await api.sessionLogin(new Session(sessCfgWithCreds)); + this.params.response.console.log("Logged in successfully"); + return tokenValue; } catch (error) { throw new ImperativeError({ msg: `Failed to fetch ${profile.tokenType} for ${propPath}: ${error.message}`, @@ -117,6 +121,8 @@ export default class SecureHandler implements ICommandHandler { } } else { this.params.response.console.log("No credentials provided."); + // return null to avoid propting for .tokenValue for this profile + return null; } } } From a7167f89eb632624c8a66aee4afaf404f571ec21 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Mon, 10 Jul 2023 13:17:05 +0000 Subject: [PATCH 18/26] fix unit tests Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- packages/config/__tests__/ConfigAutoStore.test.ts | 5 +++-- packages/config/src/ConfigAutoStore.ts | 8 +++----- .../imperative/src/config/cmd/secure/secure.handler.ts | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/config/__tests__/ConfigAutoStore.test.ts b/packages/config/__tests__/ConfigAutoStore.test.ts index 9cace3ea2..c07bb0ce3 100644 --- a/packages/config/__tests__/ConfigAutoStore.test.ts +++ b/packages/config/__tests__/ConfigAutoStore.test.ts @@ -167,7 +167,7 @@ describe("ConfigAutoStore tests", () => { expect(authHandler).toBeUndefined(); }); - it("should not find auth handler if profile base path is undefined", async () => { + it("should find auth handler if profile base path is undefined", async () => { await setupConfigToLoad({ profiles: { base: { @@ -185,7 +185,8 @@ describe("ConfigAutoStore tests", () => { }); const authHandler = ConfigAutoStore.findAuthHandlerForProfile("profiles.zosmf", {} as any); - expect(authHandler).toBeUndefined(); + expect(authHandler).toBeDefined(); + expect(authHandler instanceof AbstractAuthHandler).toBe(true); }); }); diff --git a/packages/config/src/ConfigAutoStore.ts b/packages/config/src/ConfigAutoStore.ts index c822a67f2..579005abd 100644 --- a/packages/config/src/ConfigAutoStore.ts +++ b/packages/config/src/ConfigAutoStore.ts @@ -79,12 +79,10 @@ export class ConfigAutoStore { const profileType = lodash.get(config.properties, `${opts.profilePath}.type`); const profile = config.api.profiles.get(opts.profilePath.replace(/profiles\./g, ""), false); - if (profile == null || (profileType == null && profile.tokenType == null)) { // Profile must exist and have type defined + if (profile == null || profileType == null) { // Profile must exist and have type defined + return; + } else if (profileType === "base" && profile.tokenType == null) { // Base profile must have tokenType defined return; - } else if (profileType === "base") { - if (profile.tokenType == null) { // Base profile must have tokenType defined - return; - } } else if (profile.tokenType == null) { // If tokenType undefined in service profile, fall back to base profile const baseProfileName = ConfigUtils.getActiveProfileName("base", opts.cmdArguments, opts.defaultBaseProfileName); return this._findAuthHandlerForProfile({ ...opts, profilePath: config.api.profiles.getProfilePathFromName(baseProfileName) }); diff --git a/packages/imperative/src/config/cmd/secure/secure.handler.ts b/packages/imperative/src/config/cmd/secure/secure.handler.ts index 007b7d756..bef4e8a0e 100644 --- a/packages/imperative/src/config/cmd/secure/secure.handler.ts +++ b/packages/imperative/src/config/cmd/secure/secure.handler.ts @@ -99,7 +99,7 @@ export default class SecureHandler implements ICommandHandler { if (authHandlerClass != null) { const api = authHandlerClass.getAuthHandlerApi(); if (api.promptParams.serviceDescription != null) { - this.params.response.console.log(`Logging in to ${api.promptParams.serviceDescription} - blank to skip:`); + this.params.response.console.log(`Logging in to ${api.promptParams.serviceDescription} - Press ENTER to skip:`); } const profile = config.api.profiles.get(profilePath.replace(/profiles\./g, ""), false); From 54319c8d41845203339fc99e186ab08daea0843a Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Tue, 11 Jul 2023 15:24:41 +0000 Subject: [PATCH 19/26] add basePath check back to prevent ze issues Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- .../config/__tests__/ConfigAutoStore.test.ts | 5 ++--- packages/config/src/ConfigAutoStore.ts | 19 +++++++++++++------ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/config/__tests__/ConfigAutoStore.test.ts b/packages/config/__tests__/ConfigAutoStore.test.ts index c07bb0ce3..a861671f6 100644 --- a/packages/config/__tests__/ConfigAutoStore.test.ts +++ b/packages/config/__tests__/ConfigAutoStore.test.ts @@ -167,7 +167,7 @@ describe("ConfigAutoStore tests", () => { expect(authHandler).toBeUndefined(); }); - it("should find auth handler if profile base path is undefined", async () => { + it("should not find auth handler if service profile base path is undefined", async () => { await setupConfigToLoad({ profiles: { base: { @@ -185,8 +185,7 @@ describe("ConfigAutoStore tests", () => { }); const authHandler = ConfigAutoStore.findAuthHandlerForProfile("profiles.zosmf", {} as any); - expect(authHandler).toBeDefined(); - expect(authHandler instanceof AbstractAuthHandler).toBe(true); + expect(authHandler).toBeUndefined(); }); }); diff --git a/packages/config/src/ConfigAutoStore.ts b/packages/config/src/ConfigAutoStore.ts index 579005abd..72f87962f 100644 --- a/packages/config/src/ConfigAutoStore.ts +++ b/packages/config/src/ConfigAutoStore.ts @@ -79,13 +79,20 @@ export class ConfigAutoStore { const profileType = lodash.get(config.properties, `${opts.profilePath}.type`); const profile = config.api.profiles.get(opts.profilePath.replace(/profiles\./g, ""), false); - if (profile == null || profileType == null) { // Profile must exist and have type defined + if (profile == null || profileType == null) { // Profile must exist and have type defined return; - } else if (profileType === "base" && profile.tokenType == null) { // Base profile must have tokenType defined - return; - } else if (profile.tokenType == null) { // If tokenType undefined in service profile, fall back to base profile - const baseProfileName = ConfigUtils.getActiveProfileName("base", opts.cmdArguments, opts.defaultBaseProfileName); - return this._findAuthHandlerForProfile({ ...opts, profilePath: config.api.profiles.getProfilePathFromName(baseProfileName) }); + } else if (profileType === "base") { + if (profile.tokenType == null) { // Base profile must have tokenType defined + return; + } + } else { + if (profile.basePath == null) { // Service profiles must have basePath defined + return; + } + if (profile.tokenType == null) { // If tokenType undefined in service profile, fall back to base profile + const baseProfileName = ConfigUtils.getActiveProfileName("base", opts.cmdArguments, opts.defaultBaseProfileName); + return this._findAuthHandlerForProfile({ ...opts, profilePath: config.api.profiles.getProfilePathFromName(baseProfileName) }); + } } const authConfigs: ICommandProfileAuthConfig[] = []; From 528896c8660a19c6bf26a985ec2c197da2ab24a5 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Tue, 11 Jul 2023 15:34:22 +0000 Subject: [PATCH 20/26] move press enter to skip to a constant value Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- packages/config/src/ConfigConstants.ts | 5 +++++ .../config/cmd/init/init.handler.test.ts | 18 +++++++++--------- .../src/config/cmd/init/init.handler.ts | 2 +- .../src/config/cmd/secure/secure.handler.ts | 8 ++++---- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/packages/config/src/ConfigConstants.ts b/packages/config/src/ConfigConstants.ts index 90886391a..f74451e80 100644 --- a/packages/config/src/ConfigConstants.ts +++ b/packages/config/src/ConfigConstants.ts @@ -27,4 +27,9 @@ export class ConfigConstants { * ID used for storing secure credentials in vault */ public static readonly SECURE_ACCT = "secure_config_props"; + + /** + * ID used for storing secure credentials in vault + */ + public static readonly SKIP_PROMPT = "- Press ENTER to skip: "; } diff --git a/packages/imperative/__tests__/config/cmd/init/init.handler.test.ts b/packages/imperative/__tests__/config/cmd/init/init.handler.test.ts index 1cdff1cb0..df45527c1 100644 --- a/packages/imperative/__tests__/config/cmd/init/init.handler.test.ts +++ b/packages/imperative/__tests__/config/cmd/init/init.handler.test.ts @@ -152,7 +152,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(user ? 0 : 1); // User config is a skeleton - no prompting should occur // Prompting for secure property - if (!user) expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); + if (!user) expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("to skip:"), {"hideText": true}); expect(editFileSpy).not.toHaveBeenCalled(); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); @@ -241,7 +241,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(user ? 0 : 1); // Prompting for secure property - if (!user) expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); + if (!user) expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("to skip:"), {"hideText": true}); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); @@ -449,7 +449,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(1); // Prompting for secure property - expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); + expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("to skip:"), {"hideText": true}); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); // 1 = Schema and 2 = Config @@ -493,7 +493,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(1); // Prompting for secure property - expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); + expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("to skip:"), {"hideText": true}); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); // 1 = Schema and 2 = Config @@ -539,7 +539,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(1); // Prompting for secure property - expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); + expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("to skip:"), {"hideText": true}); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); // 1 = Schema and 2 = Config @@ -583,7 +583,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(1); // Prompting for secure property - expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); + expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("to skip:"), {"hideText": true}); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); // 1 = Schema and 2 = Config @@ -637,7 +637,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(1); // Prompting for secure property - expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); + expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("to skip:"), {"hideText": true}); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); // 1 = Schema and 2 = Config @@ -691,7 +691,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(1); // Prompting for secure property - expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); + expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("to skip:"), {"hideText": true}); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); // 1 = Schema and 2 = Config @@ -747,7 +747,7 @@ describe("Configuration Initialization command handler", () => { expect(promptWithTimeoutSpy).toHaveBeenCalledTimes(2); // Prompting for secure property - expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("Press ENTER to skip:"), {"hideText": true}); + expect(promptWithTimeoutSpy).toHaveBeenCalledWith(expect.stringContaining("to skip:"), {"hideText": true}); expect(writeFileSyncSpy).toHaveBeenCalledTimes(2); // 1 = Schema and 2 = Config diff --git a/packages/imperative/src/config/cmd/init/init.handler.ts b/packages/imperative/src/config/cmd/init/init.handler.ts index d5649e009..b73278020 100644 --- a/packages/imperative/src/config/cmd/init/init.handler.ts +++ b/packages/imperative/src/config/cmd/init/init.handler.ts @@ -186,7 +186,7 @@ export default class InitHandler implements ICommandHandler { } this.promptProps.push(propName); - const propValue: any = await this.params.response.console.prompt(`Enter ${propDesc} - Press ENTER to skip: `, { hideText: property.secure }); + const propValue: any = await this.params.response.console.prompt(`Enter ${propDesc} ${ConfigConstants.SKIP_PROMPT}`, { hideText: property.secure }); // coerce to correct type if (propValue && propValue.trim().length > 0) { diff --git a/packages/imperative/src/config/cmd/secure/secure.handler.ts b/packages/imperative/src/config/cmd/secure/secure.handler.ts index bef4e8a0e..3c88d7919 100644 --- a/packages/imperative/src/config/cmd/secure/secure.handler.ts +++ b/packages/imperative/src/config/cmd/secure/secure.handler.ts @@ -10,7 +10,7 @@ */ import { ICommandArguments, ICommandHandler, IHandlerParameters } from "../../../../../cmd"; -import { Config, ConfigAutoStore, ConfigSchema } from "../../../../../config"; +import { Config, ConfigAutoStore, ConfigConstants, ConfigSchema } from "../../../../../config"; import { coercePropValue, secureSaveError } from "../../../../../config/src/ConfigUtils"; import { ImperativeError } from "../../../../../error"; import { Logger } from "../../../../../logger"; @@ -62,7 +62,7 @@ export default class SecureHandler implements ICommandHandler { params.response.console.log(`Processing secure properties for profile: ${config.api.profiles.getProfileNameFromPath(propName)}`); let propValue = await this.handlePromptForAuthToken(config, propName); if (propValue === undefined) { - propValue = await params.response.console.prompt(`Enter ${propName} - Press ENTER to skip: `, {hideText: true}); + propValue = await params.response.console.prompt(`Enter ${propName} ${ConfigConstants.SKIP_PROMPT}`, {hideText: true}); } // Save the value in the config securely @@ -70,7 +70,7 @@ export default class SecureHandler implements ICommandHandler { config.set(propName, propValue, { secure: true }); } } else { - let propValue = await params.response.console.prompt(`Enter ${propName} - Press ENTER to skip: `, { hideText: true }); + let propValue = await params.response.console.prompt(`Enter ${propName} ${ConfigConstants.SKIP_PROMPT}`, { hideText: true }); // Save the value in the config securely if (propValue) { @@ -99,7 +99,7 @@ export default class SecureHandler implements ICommandHandler { if (authHandlerClass != null) { const api = authHandlerClass.getAuthHandlerApi(); if (api.promptParams.serviceDescription != null) { - this.params.response.console.log(`Logging in to ${api.promptParams.serviceDescription} - Press ENTER to skip:`); + this.params.response.console.log(`Logging in to ${api.promptParams.serviceDescription} ${ConfigConstants.SKIP_PROMPT}`); } const profile = config.api.profiles.get(profilePath.replace(/profiles\./g, ""), false); From 262f4fc84411e075e8e3af77ba2f8e663119be7d Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Tue, 11 Jul 2023 20:56:45 +0000 Subject: [PATCH 21/26] force user-pass to take precedence over tokens in the abstract rest client Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- .../src/config/cmd/init/init.handler.ts | 9 ++++++--- packages/rest/src/client/AbstractRestClient.ts | 17 ++++++++++------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/imperative/src/config/cmd/init/init.handler.ts b/packages/imperative/src/config/cmd/init/init.handler.ts index b73278020..98338dd42 100644 --- a/packages/imperative/src/config/cmd/init/init.handler.ts +++ b/packages/imperative/src/config/cmd/init/init.handler.ts @@ -85,10 +85,12 @@ export default class InitHandler implements ICommandHandler { original = JSONC.stringify(originalProperties, null, ConfigConstants.INDENT); dryRun = JSONC.stringify(dryRunProperties, null, ConfigConstants.INDENT); - let jsonDiff = diff(original, dryRun, {aAnnotation: "Removed", + let jsonDiff = diff(original, dryRun, { + aAnnotation: "Removed", bAnnotation: "Added", aColor: TextUtils.chalk.red, - bColor: TextUtils.chalk.green}); + bColor: TextUtils.chalk.green + }); if (stripAnsi(jsonDiff) === "Compared values have no visual difference.") { jsonDiff = dryRun; @@ -186,7 +188,8 @@ export default class InitHandler implements ICommandHandler { } this.promptProps.push(propName); - const propValue: any = await this.params.response.console.prompt(`Enter ${propDesc} ${ConfigConstants.SKIP_PROMPT}`, { hideText: property.secure }); + const propValue: any = await this.params.response.console.prompt( + `Enter ${propDesc} ${ConfigConstants.SKIP_PROMPT}`, { hideText: property.secure }); // coerce to correct type if (propValue && propValue.trim().length > 0) { diff --git a/packages/rest/src/client/AbstractRestClient.ts b/packages/rest/src/client/AbstractRestClient.ts index d173e3c17..1962d6e35 100644 --- a/packages/rest/src/client/AbstractRestClient.ts +++ b/packages/rest/src/client/AbstractRestClient.ts @@ -463,7 +463,15 @@ export abstract class AbstractRestClient { */ if (this.session.ISession.type === SessConstants.AUTH_TYPE_BASIC || this.session.ISession.type === SessConstants.AUTH_TYPE_TOKEN) { - if (this.session.ISession.tokenValue) { + if (this.session.ISession.base64EncodedAuth || (this.session.ISession.user && this.session.ISession.password)) { + this.log.trace("Using basic authentication"); + const headerKeys: string[] = Object.keys(Headers.BASIC_AUTHORIZATION); + const authentication: string = AbstractSession.BASIC_PREFIX + this.session.ISession.base64EncodedAuth ?? + AbstractSession.getBase64Auth(this.session.ISession.user, this.session.ISession.password); + headerKeys.forEach((property) => { + options.headers[property] = authentication; + }); + } else if (this.session.ISession.tokenValue) { this.log.trace("Using cookie authentication with token %s", this.session.ISession.tokenValue); const headerKeys: string[] = Object.keys(Headers.COOKIE_AUTHORIZATION); const authentication: string = `${this.session.ISession.tokenType}=${this.session.ISession.tokenValue}`; @@ -471,12 +479,7 @@ export abstract class AbstractRestClient { options.headers[property] = authentication; }); } else { - this.log.trace("Using basic authentication"); - const headerKeys: string[] = Object.keys(Headers.BASIC_AUTHORIZATION); - const authentication: string = AbstractSession.BASIC_PREFIX + this.session.ISession.base64EncodedAuth; - headerKeys.forEach((property) => { - options.headers[property] = authentication; - }); + throw new ImperativeError({msg: "No credentials for a BASIC or TOKEN type of session."}); } } else if (this.session.ISession.type === SessConstants.AUTH_TYPE_BEARER) { this.log.trace("Using bearer authentication"); From 8a0f56efe0cfa9fec33233b3ae86e77a8dcc3090 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Wed, 12 Jul 2023 12:35:57 +0000 Subject: [PATCH 22/26] fix code smell and sonar bug Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- packages/config/src/ConfigAutoStore.ts | 3 +-- packages/rest/src/client/AbstractRestClient.ts | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/config/src/ConfigAutoStore.ts b/packages/config/src/ConfigAutoStore.ts index 72f87962f..7f44c2105 100644 --- a/packages/config/src/ConfigAutoStore.ts +++ b/packages/config/src/ConfigAutoStore.ts @@ -18,14 +18,13 @@ import { AbstractAuthHandler } from "../../imperative/src/auth/handlers/Abstract import { ImperativeConfig } from "../../utilities"; import { ISession } from "../../rest/src/session/doc/ISession"; import { Session } from "../../rest/src/session/Session"; -import { AUTH_TYPE_TOKEN } from "../../rest/src/session/SessConstants"; +import { AUTH_TYPE_TOKEN, TOKEN_TYPE_APIML } from "../../rest/src/session/SessConstants"; import { Logger } from "../../logger"; import { IConfigAutoStoreFindActiveProfileOpts, IConfigAutoStoreFindAuthHandlerForProfileOpts, IConfigAutoStoreStoreSessCfgPropsOpts } from "./doc/IConfigAutoStoreOpts"; -import { TOKEN_TYPE_APIML } from "../../rest/src/session/SessConstants"; /** * Class to manage automatic storage of properties in team config. diff --git a/packages/rest/src/client/AbstractRestClient.ts b/packages/rest/src/client/AbstractRestClient.ts index 1962d6e35..e2cee7496 100644 --- a/packages/rest/src/client/AbstractRestClient.ts +++ b/packages/rest/src/client/AbstractRestClient.ts @@ -466,8 +466,8 @@ export abstract class AbstractRestClient { if (this.session.ISession.base64EncodedAuth || (this.session.ISession.user && this.session.ISession.password)) { this.log.trace("Using basic authentication"); const headerKeys: string[] = Object.keys(Headers.BASIC_AUTHORIZATION); - const authentication: string = AbstractSession.BASIC_PREFIX + this.session.ISession.base64EncodedAuth ?? - AbstractSession.getBase64Auth(this.session.ISession.user, this.session.ISession.password); + const authentication: string = AbstractSession.BASIC_PREFIX + (this.session.ISession.base64EncodedAuth ?? + AbstractSession.getBase64Auth(this.session.ISession.user, this.session.ISession.password)); headerKeys.forEach((property) => { options.headers[property] = authentication; }); From 9d79c841e0ddd94441a8f727d2514ff0e10e9c19 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Thu, 20 Jul 2023 12:30:04 +0000 Subject: [PATCH 23/26] make sure to use the same tokentype if provided + remove creds as soon as possible Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- packages/config/src/ConfigAutoStore.ts | 3 ++- .../src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts | 5 ----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/config/src/ConfigAutoStore.ts b/packages/config/src/ConfigAutoStore.ts index 7f44c2105..ab85e7acb 100644 --- a/packages/config/src/ConfigAutoStore.ts +++ b/packages/config/src/ConfigAutoStore.ts @@ -236,7 +236,7 @@ export class ConfigAutoStore { const api = authHandlerClass.getAuthHandlerApi(); opts.sessCfg.type = AUTH_TYPE_TOKEN; - opts.sessCfg.tokenType = api.promptParams.defaultTokenType; + opts.sessCfg.tokenType = opts.params.arguments.tokenType ?? api.promptParams.defaultTokenType; const baseSessCfg: ISession = { type: opts.sessCfg.type }; for (const propName of Object.keys(ImperativeConfig.instance.loadedConfig.baseProfile.schema.properties)) { @@ -248,6 +248,7 @@ export class ConfigAutoStore { Logger.getAppLogger().info(`Fetching ${opts.sessCfg.tokenType} for ${opts.profilePath}`); opts.sessCfg.tokenValue = await api.sessionLogin(new Session(baseSessCfg)); + opts.sessCfg.user = opts.sessCfg.password = undefined; return true; } } diff --git a/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts b/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts index b3e0d249a..1926ad4da 100644 --- a/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts +++ b/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts @@ -89,11 +89,6 @@ export abstract class BaseAutoInitHandler implements ICommandHandler { sessCfg, params.arguments, { parms: params, doPrompting: true, serviceDescription: this.mServiceDescription }, ); this.mSession = new Session(sessCfgWithCreds); - if (this.mSession.ISession.tokenValue) { - this.mSession.ISession.base64EncodedAuth = - this.mSession.ISession.user = this.mSession.ISession.password = - this.mSession.ISession.cert = this.mSession.ISession.certKey = undefined; - } // Use params to set which config layer to apply to await OverridesLoader.ensureCredentialManagerLoaded(); From 3cf8573015f23ed77b4b368c07b0969984de1a39 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Thu, 20 Jul 2023 13:02:26 +0000 Subject: [PATCH 24/26] fix unit tests and make sure that the ISession doesn't have creds if we had the token already Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- packages/config/src/ConfigAutoStore.ts | 2 +- .../src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/config/src/ConfigAutoStore.ts b/packages/config/src/ConfigAutoStore.ts index ab85e7acb..a36a921fe 100644 --- a/packages/config/src/ConfigAutoStore.ts +++ b/packages/config/src/ConfigAutoStore.ts @@ -236,7 +236,7 @@ export class ConfigAutoStore { const api = authHandlerClass.getAuthHandlerApi(); opts.sessCfg.type = AUTH_TYPE_TOKEN; - opts.sessCfg.tokenType = opts.params.arguments.tokenType ?? api.promptParams.defaultTokenType; + opts.sessCfg.tokenType = opts.params?.arguments?.tokenType ?? api.promptParams.defaultTokenType; const baseSessCfg: ISession = { type: opts.sessCfg.type }; for (const propName of Object.keys(ImperativeConfig.instance.loadedConfig.baseProfile.schema.properties)) { diff --git a/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts b/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts index 1926ad4da..b3e0d249a 100644 --- a/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts +++ b/packages/imperative/src/config/cmd/auto-init/handlers/BaseAutoInitHandler.ts @@ -89,6 +89,11 @@ export abstract class BaseAutoInitHandler implements ICommandHandler { sessCfg, params.arguments, { parms: params, doPrompting: true, serviceDescription: this.mServiceDescription }, ); this.mSession = new Session(sessCfgWithCreds); + if (this.mSession.ISession.tokenValue) { + this.mSession.ISession.base64EncodedAuth = + this.mSession.ISession.user = this.mSession.ISession.password = + this.mSession.ISession.cert = this.mSession.ISession.certKey = undefined; + } // Use params to set which config layer to apply to await OverridesLoader.ensureCredentialManagerLoaded(); From c89b1918eebecccc492bc906315f3d1868d5dfb3 Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Mon, 24 Jul 2023 17:46:54 +0000 Subject: [PATCH 25/26] display better error messages on logout Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- .../__tests__/BaseAuthHandler.config.test.ts | 121 ++++++++++++++++++ .../__tests__/__resources__/auth.config.json | 10 ++ .../src/auth/handlers/BaseAuthHandler.ts | 46 ++++++- 3 files changed, 171 insertions(+), 6 deletions(-) diff --git a/packages/imperative/src/auth/__tests__/BaseAuthHandler.config.test.ts b/packages/imperative/src/auth/__tests__/BaseAuthHandler.config.test.ts index c54f6bd50..e4ebcf369 100644 --- a/packages/imperative/src/auth/__tests__/BaseAuthHandler.config.test.ts +++ b/packages/imperative/src/auth/__tests__/BaseAuthHandler.config.test.ts @@ -440,7 +440,12 @@ describe("BaseAuthHandler config", () => { const logoutParams: IHandlerParameters = { response: { console: { + error: jest.fn(), + errorHeader: jest.fn(), log: jest.fn() + }, + data: { + setExitCode: jest.fn() } }, arguments: { @@ -454,6 +459,7 @@ describe("BaseAuthHandler config", () => { } as any; beforeEach(async () => { + jest.restoreAllMocks(); jest.spyOn(Config, "search").mockReturnValue(configPath); jest.spyOn(fs, "existsSync").mockReturnValueOnce(false).mockReturnValueOnce(true).mockReturnValue(false); fakeConfig = await Config.load(MY_APP, { vault: fakeVault }); @@ -555,5 +561,120 @@ describe("BaseAuthHandler config", () => { expect(doLogoutSpy).toBeCalledTimes(1); expect(writeFileSpy).not.toHaveBeenCalled(); }); + + it("should not logout without a token", async () => { + const handler = new FakeAuthHandler(); + const params = lodash.cloneDeep(logoutParams); + delete params.arguments.tokenValue; + + const doLogoutSpy = jest.spyOn(handler as any, "doLogout"); + const writeFileSpy = jest.spyOn(fs, "writeFileSync"); + + let caughtError; + + try { + await handler.process(params); + } catch (error) { + caughtError = error; + } + + expect(caughtError).toBeUndefined(); + expect(doLogoutSpy).not.toHaveBeenCalled(); + expect(writeFileSpy).not.toHaveBeenCalled(); + expect((params.response.console.error as any).mock.calls[0][0]).toContain("Token was not provided"); + expect(params.response.data.setExitCode).toHaveBeenCalledWith(1); + }); + + it("should not touch the config file if there is no profile to modify", async () => { + const handler = new FakeAuthHandler(); + const params = lodash.cloneDeep(logoutParams); + + const doLogoutSpy = jest.spyOn(handler as any, "doLogout").mockRejectedValueOnce({errorCode: "401"}); + const writeFileSpy = jest.spyOn(fs, "writeFileSync"); + + let caughtError; + + try { + await handler.process(params); + } catch (error) { + caughtError = error; + } + + expect(caughtError).toBeUndefined(); + expect(doLogoutSpy).toHaveBeenCalled(); + expect(writeFileSpy).not.toHaveBeenCalled(); + expect((params.response.console.log as any).mock.calls[0][0]).toContain("Token is not valid or expired"); + expect((params.response.console.log as any).mock.calls[0][0]).toContain("Empty profile was provided"); + }); + + it("should not touch the config file if the token type was not specified", async () => { + const handler = new FakeAuthHandler(); + const params = lodash.cloneDeep(logoutParams); + params.arguments["fruit-profile"] = "fruity"; + + const doLogoutSpy = jest.spyOn(handler as any, "doLogout"); + const writeFileSpy = jest.spyOn(fs, "writeFileSync"); + + let caughtError; + + try { + await handler.process(params); + } catch (error) { + caughtError = error; + } + + expect(caughtError).toBeUndefined(); + expect(doLogoutSpy).toHaveBeenCalled(); + expect(writeFileSpy).not.toHaveBeenCalled(); + expect((params.response.console.log as any).mock.calls[0][0]).toContain("Token type was not provided"); + }); + + it("should not touch the config file if the token type does not match", async () => { + const handler = new FakeAuthHandler(); + const params = lodash.cloneDeep(logoutParams); + params.arguments.tokenType = "fakeType.1"; + params.arguments["fruit-profile"] = "fruit"; + fakeConfig.properties.profiles.fruit.properties.tokenType = "fakeType.2"; + + const doLogoutSpy = jest.spyOn(handler as any, "doLogout"); + const writeFileSpy = jest.spyOn(fs, "writeFileSync"); + + let caughtError; + + try { + await handler.process(params); + } catch (error) { + caughtError = error; + } + + expect(caughtError).toBeUndefined(); + expect(doLogoutSpy).toHaveBeenCalled(); + expect(writeFileSpy).not.toHaveBeenCalled(); + expect((params.response.console.log as any).mock.calls[0][0]).toContain("Token type does not match the authentication service"); + }); + + it("should not touch the config file if the token value does not match", async () => { + const handler = new FakeAuthHandler(); + const params = lodash.cloneDeep(logoutParams); + params.arguments.tokenValue = "fakeValue.1"; + params.arguments["fruit-profile"] = "fruit"; + fakeConfig.properties.profiles.fruit.properties.tokenValue = "fakeValue.2"; + + const doLogoutSpy = jest.spyOn(handler as any, "doLogout"); + const writeFileSpy = jest.spyOn(fs, "writeFileSync"); + + let caughtError; + + try { + await handler.process(params); + } catch (error) { + caughtError = error; + } + + expect(caughtError).toBeUndefined(); + expect(doLogoutSpy).toHaveBeenCalled(); + expect(writeFileSpy).not.toHaveBeenCalled(); + expect((params.response.console.log as any).mock.calls[0][0]).toContain("Token value does not match the securely stored value"); + }); }); }); diff --git a/packages/imperative/src/auth/__tests__/__resources__/auth.config.json b/packages/imperative/src/auth/__tests__/__resources__/auth.config.json index 76aaf7632..204107e5c 100644 --- a/packages/imperative/src/auth/__tests__/__resources__/auth.config.json +++ b/packages/imperative/src/auth/__tests__/__resources__/auth.config.json @@ -22,6 +22,16 @@ "secure": [ "tokenValue" ] + }, + "fruity": { + "type": "fruit", + "properties": { + "host": "example.com", + "port": 12345 + }, + "secure": [ + "tokenValue" + ] } }, "defaults": {}, diff --git a/packages/imperative/src/auth/handlers/BaseAuthHandler.ts b/packages/imperative/src/auth/handlers/BaseAuthHandler.ts index 0dd46e920..a7cdfdf67 100644 --- a/packages/imperative/src/auth/handlers/BaseAuthHandler.ts +++ b/packages/imperative/src/auth/handlers/BaseAuthHandler.ts @@ -10,9 +10,17 @@ */ import { IHandlerParameters, IHandlerResponseApi } from "../../../../cmd"; -import { AbstractSession, ConnectionPropsForSessCfg, IOptionsForAddConnProps, ISession, SessConstants, Session } from "../../../../rest"; +import { + AbstractSession, + ConnectionPropsForSessCfg, + IOptionsForAddConnProps, + ISession, + RestConstants, + SessConstants, + Session +} from "../../../../rest"; import { Imperative } from "../../Imperative"; -import { ImperativeError } from "../../../../error"; +import { IImperativeError, ImperativeError } from "../../../../error"; import { ISaveProfileFromCliArgs } from "../../../../profiles"; import { ImperativeConfig } from "../../../../utilities"; import { getActiveProfileName, secureSaveError } from "../../../../config/src/ConfigUtils"; @@ -214,9 +222,14 @@ export abstract class BaseAuthHandler extends AbstractAuthHandler { { requestToken: false, doPrompting: false, parms: params }, ); + let logoutError: IImperativeError; if (params.arguments.tokenValue != null) { this.mSession = new Session(sessCfgWithCreds); - await this.doLogout(this.mSession); + try { + await this.doLogout(this.mSession); + } catch (err) { + logoutError = err; + } } if (!ImperativeConfig.instance.config.exists) { @@ -234,6 +247,7 @@ export abstract class BaseAuthHandler extends AbstractAuthHandler { const profileProps = config.api.profiles.get(profileName, false); let profileWithToken: string = null; + let noDeleteReason = ""; // If you specified a token on the command line, then don't delete the one in the profile if it doesn't match if (Object.keys(profileProps).length > 0 && profileProps.tokenType != null && profileProps.tokenValue != null && profileProps.tokenType === params.arguments.tokenType && profileProps.tokenValue === params.arguments.tokenValue) { @@ -243,11 +257,31 @@ export abstract class BaseAuthHandler extends AbstractAuthHandler { await config.save(); profileWithToken = profileName; + } else { + if (Object.keys(profileProps).length === 0) noDeleteReason = "Empty profile was provided."; + else if (profileProps.tokenType == null) noDeleteReason = "Token type was not provided."; + else if (profileProps.tokenValue == null) noDeleteReason = "Token value was not provided."; + else if (profileProps.tokenType !== params.arguments.tokenType) + noDeleteReason = "Token type does not match the authentication service"; + else if (profileProps.tokenValue !== params.arguments.tokenValue) + noDeleteReason = "Token value does not match the securely stored value"; } - params.response.console.log("Logout successful. The authentication token has been revoked" + - (profileWithToken != null ? ` and removed from your '${profileWithToken}' ${this.mProfileType} profile` : "") + - "."); + if (params.arguments.tokenValue != null) { + let logoutMessage = "Logout successful. The authentication token has been revoked."; + if (logoutError?.errorCode === RestConstants.HTTP_STATUS_401.toString()) { + logoutMessage = "Token is not valid or expired."; + } + logoutMessage += `\nToken was${profileWithToken == null ? " not" : ""} removed from ` + + `your '${profileName}' ${this.mProfileType} profile.`; + logoutMessage += `${!noDeleteReason ? "" : "\nReason: " + noDeleteReason}`; + params.response.console.log(logoutMessage); + } else { + params.response.console.errorHeader("Command Error"); + params.response.console.error("Token was not provided, so can't log out!"+ + "\nYou need to authenticate using `zowe auth login`"); + params.response.data.setExitCode(1); + } } } From 96a5c6af71b8359672b8219d93e1f7b445ddbc1f Mon Sep 17 00:00:00 2001 From: zFernand0 <37381190+zFernand0@users.noreply.github.com> Date: Tue, 25 Jul 2023 20:20:46 +0000 Subject: [PATCH 26/26] fix wording of error message Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com> --- packages/imperative/src/auth/handlers/BaseAuthHandler.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/imperative/src/auth/handlers/BaseAuthHandler.ts b/packages/imperative/src/auth/handlers/BaseAuthHandler.ts index a7cdfdf67..95938b3f6 100644 --- a/packages/imperative/src/auth/handlers/BaseAuthHandler.ts +++ b/packages/imperative/src/auth/handlers/BaseAuthHandler.ts @@ -278,8 +278,8 @@ export abstract class BaseAuthHandler extends AbstractAuthHandler { params.response.console.log(logoutMessage); } else { params.response.console.errorHeader("Command Error"); - params.response.console.error("Token was not provided, so can't log out!"+ - "\nYou need to authenticate using `zowe auth login`"); + params.response.console.error("Token was not provided, so can't log out."+ + "\nYou need to authenticate first using `zowe auth login`."); params.response.data.setExitCode(1); } }