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

[next] Custom secure properties not loaded #1663

Closed
Tracked by #1833
zFernand0 opened this issue Feb 9, 2022 · 25 comments
Closed
Tracked by #1833

[next] Custom secure properties not loaded #1663

zFernand0 opened this issue Feb 9, 2022 · 25 comments
Assignees

Comments

@zFernand0
Copy link
Member

Describe the bug

The ZE extension won't load a profile (i.e. fails to validate) when provided with a team config that has a profile required property (host, port) in the secure array.

To Reproduce

  1. Copy the zowe.config.json (see below) into a new workspace
  2. Open the Zowe Explorer on the above workspace
  3. Profile gets automatically added, but it's practically unusable 😢
{
    "$schema": "./zowe.schema.json",
    "profiles": {
        "ibmzosmf": {
            "type": "zosmf",
            "properties": {
                "basePath": "/ibmzosmf/api/v1"
            }
        },
        "base": {
            "type": "base",
            "properties": {
                "port": 1234,
                "rejectUnauthorized": true,
                "tokenType": "apimlAuthenticationToken"
            },
            "secure": [
                "host",
                "tokenValue"
            ]
        }
    },
    "defaults": {
        "zosmf": "ibmzosmf",
        "base": "base"
    },
    "autoStore": true
}

Expected behavior

All properties are loaded regardless of whether or not they are stored securely

Screenshots

image

Desktop (please complete the following information):

Additional context

Using a VSIX from #1637

@zFernand0 zFernand0 added the bug Something isn't working label Feb 9, 2022
@github-actions
Copy link

github-actions bot commented Feb 9, 2022

Thank you for creating a bug report. If you haven't already, please ensure you have provided steps to reproduce it and as much context as possible.

@JillieBeanSim
Copy link
Contributor

@zFernand0 without the host in the config you will receive the error shown for missing host. isn't that expected behavior?

@zFernand0
Copy link
Member Author

I guess I would expect Zowe Explorer to work the same way as the CLI here, and be able to look for the host in the vault.
And if it's not set (in the vault or in the config file) then it can ask for it, and store it back (depending on the config file)

@JillieBeanSim
Copy link
Contributor

JillieBeanSim commented Apr 6, 2022

That could be a great enhancement to add after v2 goes out! 😁 In the past we have only prompted for user/pass but it would be cool to prompt for missing items other than that.

Maybe we can add the enhancement tag and add it to the PI Planning list?

@zFernand0
Copy link
Member Author

The prompting for missing properties I agree that it could be an enhancement after v2 (as a 2.1).
However, I do believe that storing port or host securely and not reading it from ZE could be a patch release (e.g. 2.0.1)

@JillieBeanSim
Copy link
Contributor

@zFernand0 when you test my PR could you check again and see if it's grabbing the secure values? It should if stored securely.

@zFernand0
Copy link
Member Author

Will do! 😋

@zFernand0
Copy link
Member Author

I'm still seeing the host must not be blank issue in the PR
I'm fine if we want to do this as a patch release after v2 goes out 😋

@JillieBeanSim
Copy link
Contributor

JillieBeanSim commented Apr 7, 2022

@zFernand0 we are using this snippet of code to get all knownArgs from imperative and checking for secure items to get the info

const mergedArgs = mProfileInfo.mergeArgsForProfile(profAttrs);
for (const arg of mergedArgs.knownArgs) {
      // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
      profile[arg.argName] = arg.secure ? await mProfileInfo.loadSecureArg(arg) : arg.argValue;
}

should we look into imperative to see why we aren't getting all of the secure items?

@zFernand0
Copy link
Member Author

FYI, there is a getSecureVals options that we can pass to the mergeArgsForProfile method to avoid the extra loop on our end 😋

In my testing, I was confused thinking it wasn't extracting values from the secure vault, but it was due to the CLI storing them under Zowe/secure_config_props and ZE storing things under Zowe-Plugin/secure_config_props.

I'll retest the host bit again. 😋
Thanks!

@zFernand0
Copy link
Member Author

This seems to be a problem.

Even though ZE is reading form the same Zowe/secure_config_props vault property, it can't find custom properties.

However, when using the ProfileInfo APIs outside of ZE (e.g. sample project, or small VSCE) it's able to load any custom properties.

That makes me believe that somewhere in ZE we are only considering the user and password as secure fields.

Same thing is happening with the APIML Token (#1713)

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@JillieBeanSim
Copy link
Contributor

@zFernand0 I removed pending closure tag, is that enough of an activity along with comment to keep it from auto-closing?

@zFernand0
Copy link
Member Author

I believe so, 👍

@github-actions
Copy link

github-actions bot commented Aug 3, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@zFernand0
Copy link
Member Author

I'm considering disabling the auto-close-issue bot 😋

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions
Copy link

github-actions bot commented Sep 3, 2022

This issue has been automatically closed due to lack of activity. In an effort to reduce noise, please do not comment any further.

@github-actions github-actions bot closed this as completed Sep 3, 2022
@zFernand0
Copy link
Member Author

I'm still seeing this error 😢
image

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@KutluOzel-b
Copy link
Contributor

As a result of the tests I have made , I came up with a conclusion that the Issue is no longer exists.

Any securely stored profile attribute on schema can be loaded under the the scenarios I have tested are : base profile, nested profile, profile that overrides base profile and regular profile.

If you can reproduce this issue please provide the steps to repro so I can test again.

Awaiting for inputs to make a final decision about this issue ...

@zFernand0
Copy link
Member Author

Thank you for retesting!
you are correct, ZE behaves as expected when there are custom properties stored securely now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants