Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: add secureFieldsWithDetails 😋 #2241

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/imperative/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

All notable changes to the Imperative package will be documented in this file.

## Recent Changes

- Enhancement: Added a new SDK method (`ConfigSecure.secureFieldsForLayer`) to allow developers to get vault content in the context of the specified layer. [#2206](https://github.com/zowe/zowe-cli/issues/2206)
- Enhancement: Added a new SDK method (`ProfileInfo.secureFieldsWithDetails`) to allow developers to the more details regarding the securely stored properties. [#2206](https://github.com/zowe/zowe-cli/issues/2206)

## `8.0.0-next.202408271330`

- LTS Breaking: [#2231](https://github.com/zowe/zowe-cli/issues/2231)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe("Config secure tests", () => {
config.mLayers = [
{
path: "fake fakety fake",
properties: { profiles: {fake: { secure: ["fake"], properties: {fake: "fake"}}}}
properties: { profiles: { fake: { secure: ["fake"], properties: { fake: "fake" } } } }
}
];
config.mVault = mockVault;
Expand Down Expand Up @@ -137,10 +137,10 @@ describe("Config secure tests", () => {
jest.spyOn(fs, "readFileSync");
let secureError: any;
const vault: IConfigVault = {
load: jest.fn().mockRejectedValue(new ImperativeError({msg: "The vault failed"})),
load: jest.fn().mockRejectedValue(new ImperativeError({ msg: "The vault failed" })),
save: jest.fn()
};
const config = await Config.load(MY_APP, {noLoad: true, vault: vault});
const config = await Config.load(MY_APP, { noLoad: true, vault: vault });
config.mVault = vault;
try {
await config.api.secure.load(vault);
Expand All @@ -167,6 +167,21 @@ describe("Config secure tests", () => {
]);
});

it("should list all secure fields for a layer", async () => {
jest.spyOn(Config, "search").mockReturnValue(projectConfigPath);
jest.spyOn(fs, "existsSync")
.mockReturnValueOnce(false) // Project user layer
.mockReturnValueOnce(true) // Project layer
.mockReturnValueOnce(false) // User layer
.mockReturnValueOnce(false); // Global layer
jest.spyOn(fs, "readFileSync");
const config = await Config.load(MY_APP);
config.mSecure = secureConfigs;
expect(config.api.secure.secureFieldsForLayer(projectConfigPath)).toEqual({ [securePropPath]: "area51" });
expect(config.api.secure.secureFieldsForLayer(projectUserConfigPath)).toEqual(null);
config.mSecure = {};
Copy link
Member

Choose a reason for hiding this comment

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

I know this is being a little picky, but is this line necessary for the locally scoped config variable?

});

it("should list all secure fields for a profile", async () => {
jest.spyOn(Config, "search").mockReturnValue(projectConfigPath).mockReturnValueOnce(projectUserConfigPath);
jest.spyOn(fs, "existsSync")
Expand Down Expand Up @@ -282,7 +297,7 @@ describe("Config secure tests", () => {

it("rmUnusedProps should delete properties for files that do not exist", () => {
const config = new (Config as any)();
config.mSecure = {...secureConfigs};
config.mSecure = { ...secureConfigs };
jest.spyOn(fs, "existsSync").mockReturnValueOnce(true).mockReturnValueOnce(false);
const prunedFiles = config.api.secure.rmUnusedProps();
expect(prunedFiles).toEqual(["fakePath"]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import { IExtendersJsonOpts } from "../src/doc/IExtenderOpts";
import { ConfigSchema } from "../src/ConfigSchema";
import { Logger } from "../../logger/src/Logger";


const testAppNm = "ProfInfoApp";
const testEnvPrefix = testAppNm.toUpperCase();
const profileTypes = ["zosmf", "tso", "base", "dummy"];
Expand Down Expand Up @@ -331,7 +330,7 @@ describe("TeamConfig ProfileInfo tests", () => {
it("should return true if credentials are not secure", async () => {
// ensure that we are not in the team project directory
const profInfo = createNewProfInfo(origDir);
(profInfo as any).mCredentials = {isSecured: false};
(profInfo as any).mCredentials = { isSecured: false };
const response = await profInfo.profileManagerWillLoad();
expect(response).toEqual(true);
});
Expand Down Expand Up @@ -366,6 +365,56 @@ describe("TeamConfig ProfileInfo tests", () => {
});
});

describe("secureFieldsWithDetails", () => {
it("should return an empty array if there are no secure fields in the given layer or if the layer does not exist", async () => {
const profInfo = createNewProfInfo(teamProjDir);
await profInfo.readProfilesFromDisk();

// Temporarily assume that there are no secure properties for this test only
profInfo.getTeamConfig().mLayers[1].properties.profiles["LPAR007"].secure = [];

// Project User does not exist
expect(profInfo.secureFieldsWithDetails({ user: true, global: false })).toEqual([]);

// Project Team dos exist, but has no secure properties
expect(profInfo.secureFieldsWithDetails({ user: false, global: false })).toEqual([]);
});

it("should return secure fields for the active layer even if they have no secure values stored in the vault", async () => {
const profInfo = createNewProfInfo(teamProjDir);
await profInfo.readProfilesFromDisk();

const securePropPath = "profiles.LPAR007.properties.";
const teamProjDirJson = path.join(teamProjDir, testAppNm + ".config.json");
profInfo.getTeamConfig().mSecure = {
[teamProjDirJson]: {
[securePropPath + "string"]: "area51",
[securePropPath + "boolean"]: true,
[securePropPath + "number"]: 1234,
},
};

const getPropAttr = (name: string | any, value: any, type?: string | null): IProfArgAttrs => ({
argLoc: { osLoc: [teamProjDirJson], locType: 1, jsonLoc: securePropPath + name },
argName: name,
argValue: value,
dataType: type !== undefined ? type : name,
});

expect(profInfo.secureFieldsWithDetails()).toEqual([
getPropAttr("string", "area51"),
getPropAttr("boolean", true),
getPropAttr("number", 1234),
getPropAttr("missing-after-this", undefined, null),
getPropAttr("host", undefined, "string"),
getPropAttr("port", undefined, "number"),
getPropAttr("rejectUnauthorized", undefined, "boolean"),
]);

profInfo.getTeamConfig().mSecure = {};
Copy link
Member

@awharn awharn Aug 28, 2024

Choose a reason for hiding this comment

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

Again, is this necessary for the locally scoped profInfo?

});
});

describe("getAllProfiles", () => {
it("should return all profiles if no type is specified", async () => {
const expectedDefaultProfiles = 4;
Expand All @@ -375,7 +424,7 @@ describe("TeamConfig ProfileInfo tests", () => {
const expectedDefaultProfileNameDummy = "LPAR4";
let actualDefaultProfiles = 0;
let expectedProfileNames = ["LPAR1", "LPAR2", "LPAR3", "LPAR1.tsoProfName", "LPAR1.tsoProfName.tsoSubProfName",
"base_glob", "LPAR4", "LPAR5"];
"base_glob", "LPAR4", "LPAR5", "LPAR007"];

const profInfo = createNewProfInfo(teamProjDir);
await profInfo.readProfilesFromDisk();
Expand Down Expand Up @@ -415,7 +464,7 @@ describe("TeamConfig ProfileInfo tests", () => {
const desiredProfType = "zosmf";
const expectedName = "LPAR1";
const expectedDefaultProfiles = 1;
let expectedProfileNames = ["LPAR1", "LPAR2", "LPAR3", "LPAR2_home", "LPAR5"];
let expectedProfileNames = ["LPAR1", "LPAR2", "LPAR3", "LPAR2_home", "LPAR5", "LPAR007"];
let actualDefaultProfiles = 0;

const profInfo = createNewProfInfo(teamProjDir);
Expand Down Expand Up @@ -449,7 +498,7 @@ describe("TeamConfig ProfileInfo tests", () => {
const desiredProfType = "zosmf";
const expectedName = "LPAR1";
const expectedDefaultProfiles = 1;
let expectedProfileNames = ["LPAR1", "LPAR2", "LPAR3", "LPAR5"];
let expectedProfileNames = ["LPAR1", "LPAR2", "LPAR3", "LPAR5", "LPAR007"];
let actualDefaultProfiles = 0;

const profInfo = createNewProfInfo(teamProjDir);
Expand Down Expand Up @@ -1116,7 +1165,7 @@ describe("TeamConfig ProfileInfo tests", () => {
expect(storeSpy).toHaveBeenCalledWith({
config: profInfo.getTeamConfig(), profileName: "LPAR4", profileType: "dummy",
defaultBaseProfileName: "base_glob",
propsToStore: [ "DOES_NOT_EXIST" ], sessCfg: { "DOES_NOT_EXIST": true }, setSecure : undefined,
propsToStore: ["DOES_NOT_EXIST"], sessCfg: { "DOES_NOT_EXIST": true }, setSecure: undefined,
});
});

Expand Down Expand Up @@ -1196,7 +1245,7 @@ describe("TeamConfig ProfileInfo tests", () => {
expect(storeSpy).toHaveBeenCalledWith({
config: profInfo.getTeamConfig(), profileName: "typeless", profileType: null,
defaultBaseProfileName: "base_glob",
propsToStore: [ "areBirdsReal" ], sessCfg: { "areBirdsReal": true }, setSecure : undefined,
propsToStore: ["areBirdsReal"], sessCfg: { "areBirdsReal": true }, setSecure: undefined,
});
});

Expand Down Expand Up @@ -1242,7 +1291,7 @@ describe("TeamConfig ProfileInfo tests", () => {
expect(storeSpy).toHaveBeenCalledWith({
config: profInfo.getTeamConfig(), profileName: "typeless_new", profileType: null,
defaultBaseProfileName: "base_glob",
propsToStore: [ "areBirdsReal" ], sessCfg: { "areBirdsReal": true }, setSecure : undefined,
propsToStore: ["areBirdsReal"], sessCfg: { "areBirdsReal": true }, setSecure: undefined,
});
});
});
Expand Down Expand Up @@ -1683,8 +1732,10 @@ describe("TeamConfig ProfileInfo tests", () => {
}
}
},
res: { success: false, info: "Both the old and new schemas are unversioned for some-type, but the schemas are different. "
.concat("The new schema was not written to disk, but will still be accessible in-memory.") }
res: {
success: false, info: "Both the old and new schemas are unversioned for some-type, but the schemas are different. "
.concat("The new schema was not written to disk, but will still be accessible in-memory.")
}
}
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,19 @@
},
"typeless": {
"properties": {}
},
"LPAR007": {
"type": "zosmf",
"properties": {},
"secure": [
"string",
"boolean",
"number",
"missing-after-this",
"host",
"port",
"rejectUnauthorized"
]
}
},
"defaults": {
Expand Down
63 changes: 61 additions & 2 deletions packages/imperative/src/config/src/ProfileInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import * as lodash from "lodash";
import * as semver from "semver";

// for ProfileInfo structures
import { IProfArgAttrs } from "./doc/IProfArgAttrs";
import { IProfArgAttrs, IProfDataType } from "./doc/IProfArgAttrs";
import { IProfAttrs } from "./doc/IProfAttrs";
import { IArgTeamConfigLoc, IProfLoc, IProfLocOsLoc, IProfLocOsLocLayer, ProfLocType } from "./doc/IProfLoc";
import { IProfMergeArgOpts } from "./doc/IProfMergeArgOpts";
Expand Down Expand Up @@ -1170,7 +1170,7 @@ export class ProfileInfo {
if (semver.major(typeInfo.schema.version) != semver.major(prevTypeVersion)) {
// Warn user if new major schema version is specified
infoMsg =
`Profile type ${profileType} was updated from schema version ${prevTypeVersion} to ${typeInfo.schema.version}.\n` +
`Profile type ${profileType} was updated from schema version ${prevTypeVersion} to ${typeInfo.schema.version}.\n` +
`The following applications may be affected: ${typeMetadata.from.filter((src) => src !== typeInfo.sourceApp)}`;
}
} else if (semver.major(prevTypeVersion) > semver.major(typeInfo.schema.version)) {
Expand Down Expand Up @@ -1388,6 +1388,65 @@ export class ProfileInfo {
return finalSchema;
}

// _______________________________________________________________________
/**
* List of secure properties with more details, like value, location, and type
*
* @param opts The user and global flags that specify one of the four
* config files (aka layers).
* @returns Array of secure property details
*/
public secureFieldsWithDetails(opts?: { user: boolean; global: boolean }): IProfArgAttrs[] {
const config = this.getTeamConfig();
const layer = opts ? config.findLayer(opts.user, opts.global) : config.layerActive();
const fields = config.api.secure.findSecure(layer.properties.profiles, "profiles");
const vault = config.api.secure.secureFieldsForLayer(layer.path);

const response: IProfArgAttrs[] = [];

// Search the vault for each secure field
if (vault) {
fields.forEach(fieldPath => {
// Search inside the secure fields for this layer
Object.entries(vault).map(([propPath, propValue]) => {
if (propPath === fieldPath) {
const dataType = ConfigSchema.findPropertyType(fieldPath, layer.properties, this.buildSchema([], layer)) as IProfDataType;

response.push({
argName: fieldPath.split(".properties.")[1],
dataType: dataType ?? this.argDataType(typeof propValue),
argValue: propValue as IProfDataType,
argLoc: {
locType: ProfLocType.TEAM_CONFIG,
osLoc: [layer.path],
jsonLoc: fieldPath
},
});
}
});
});
}

fields.forEach(fieldPath => {
if (response.find(details => details.argLoc.jsonLoc === fieldPath) == null) {
const dataType = ConfigSchema.findPropertyType(fieldPath, layer.properties, this.buildSchema([], layer)) as IProfDataType ?? null;
response.push({
argName: fieldPath.split(".properties.")[1],
dataType,
argValue: undefined,
argLoc: {
locType: ProfLocType.TEAM_CONFIG,
osLoc: [layer.path],
jsonLoc: fieldPath
}
});
}
});

return response;
}


// _______________________________________________________________________
/**
* Get all of the subprofiles in the configuration.
Expand Down
11 changes: 11 additions & 0 deletions packages/imperative/src/config/src/api/ConfigSecure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,17 @@ export class ConfigSecure extends ConfigApi {
return secureProps;
}

/**
* Retrieve secure properties for a given layer path
*
* @param layerPath Path of the layer to get secure properties for
* @returns the secure properties for the given layer, or null if not found
*/
public secureFieldsForLayer(layerPath: string): IConfigSecureProperties {
Copy link
Member

Choose a reason for hiding this comment

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

Is adding this method necessary? I'm probably being dumb but it seems like the secureFields method has a layer parameter which can be used to filter the secure fields returned for only a specific layer.

const secureLayer = Object.keys(this.mConfig.mSecure).find(osLocation => osLocation === layerPath);
return secureLayer ? this.mConfig.mSecure[secureLayer] : null;
}

/**
* Retrieve info that can be used to store a profile property securely.
*
Expand Down