[next] Secure credential handling for ProfileInfo API

[t1m0thyj]

• Added ProfileCredentials class to handle initialization of CredentialManager
• Added requireKeytar property to IProfOpts so that VS Code extensions can use a custom Keytar module
• Added secure property to IProfArgAttrs for secure arguments. When set to true, argValue is left undefined until ProfileInfo.loadSecureArg is called, so that loading of sensitive data is delayed. This may resolve Secure credentials - Extensions that are secure by default zowe-cli#957.
• Don't create new ImperativeLogger in ProfileInfo constructor if one already exists (fixes [next] Don't reinitialize Imperative logger in ProfileInfo #577, thanks to @gejohnston for contributing this fix).

[gejohnston]

This is not a comment on the implementation of this PR, but my topic is closely related so it seems appropriate to hold a conversation here, even if the result is a separate issue or story.

• Can we accurately describe how a base profile's user, password, or token is merged with a service profile's user and password?
• Which arguments are returned by the ProfiInfo API?
• I am less interested in how it operates, and more interested in what the consumer should expect, for example:
  • Is a token NOT returned if a user & password are found in a service profile?
  • Do we return a token if the user & password for the service profile is inherited from a higher-level node in the JSON document?
  • Will we return all such arguments and expect the consumer to sort out token vs user & password?
• Do we need automated tests to confirm the expected behavior?

[t1m0thyj]

This is not a comment on the implementation of this PR, but my topic is closely related so it seems appropriate to hold a conversation here, even if the result is a separate issue or story.

• Can we accurately describe how a base profile's user, password, or token is merged with a service profile's user and password?

It would be possible to determine if an argument came from a base profile by looking at its jsonLoc. If this is too inconvenient, perhaps we could add a property to IProfArgAttrs specifying the profile type from which an argument was loaded?

• Which arguments are returned by the ProfiInfo API?

Fernando's PR raised some questions around this:

1. If there is no schema present, how will mergeArgsFromProfile know to "only process properties that were in a profile"? Without a schema, it doesn't know that "user" is a profile property and "response-format-json" is not.
2. When there is a schema present, should mergeArgsForProfile throw an error if non-profile properties like "response-format-json" get loaded into knownArgs?

• I am less interested in how it operates, and more interested in what the consumer should expect, for example:

  • Is a token NOT returned if a user & password are found in a service profile?
  • Do we return a token if the user & password for the service profile is inherited from a higher-level node in the JSON document?
  • Will we return all such arguments and expect the consumer to sort out token vs user & password?

I believe we return all such arguments and leave it up to the consumer (or the ConnectionPropsForSessCfg API) to prioritize user/password over token when both are present.

• Do we need automated tests to confirm the expected behavior?

The behavior of merging args in certain cases seems unclear at this point since we have questions around it, so I think adding tests would help to better define the behavior.

[t1m0thyj]

I think the outstanding questions from this PR have been captured in #568 (comment) and zowe/zowe-cli#1926.

Trying to figure out now why so many integration tests in this PR have started failing.

EDIT: The failing tests have now been resolved.

[codecov]

Codecov Report

Merging #579 (9c91a6d) into profInfo-for-next (0efc3c1) will increase coverage by 0.10%.
The diff coverage is 95.50%.

@@                  Coverage Diff                  @@
##           profInfo-for-next     zowe/imperative#579      +/-   ##
=====================================================
+ Coverage              85.30%   85.40%   +0.10%     
=====================================================
  Files                    182      183       +1     
  Lines                   9201     9285      +84     
  Branches                1681     1708      +27     
=====================================================
+ Hits                    7849     7930      +81     
- Misses                  1348     1351       +3     
  Partials                   4        4              

Impacted Files	Coverage Δ
packages/config/src/ProfileCredentials.ts	89.47% <89.47%> (ø)
packages/config/src/ConfigSchema.ts	100.00% <100.00%> (ø)
packages/config/src/ProfInfoErr.ts	100.00% <100.00%> (ø)
packages/config/src/ProfileInfo.ts	97.67% <100.00%> (+0.65%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0efc3c1...9c91a6d. Read the comment docs.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

95.9% Coverage
0.0% Duplication

[gejohnston]

This looks good.
I think that all remaining concerns about credential handling are bigger than this story, and they should be handled in some future, prioritized, scheduled story.

[zFernand0]

LGTM!