-
Notifications
You must be signed in to change notification settings - Fork 90
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
To Switch Basic auth. to Token auth. and viceversa #2867
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Santhoshi Boyina <Santhoshi.Boyina1@ibm.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2867 +/- ##
==========================================
+ Coverage 93.52% 93.53% +0.01%
==========================================
Files 104 104
Lines 10891 10942 +51
Branches 2354 2372 +18
==========================================
+ Hits 10186 10235 +49
- Misses 704 706 +2
Partials 1 1 ☔ View full report in Codecov by Sentry. |
@@ -1237,6 +1237,24 @@ export class Profiles extends ProfilesCache { | |||
} | |||
} | |||
|
|||
public async handleSwitchAuthentication(node?: IZoweNodeType, label?: string): Promise<void> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @SanthoshiBoyina, this is a good start for handling this scenario. We can't assume the profile names. I see the node is passed and not being used, we can use it to get the profile with node.getProfile() which will supply the name of the service profile being used. Once we have that we can check if it has any secure properties and change appropriately at the service profile level then if not there the base type profile level. I believe our ZE API has methods in ProfilesCache to get the base profile that is associated with the service profile to obtain the info and name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If label is not being used I don't see a reason to pass to the method.
Signed-off-by: Santhoshi Boyina <Santhoshi.Boyina1@ibm.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- When I "Login to authentication service" for apimlzosmf the tokenvalue and tokentype are stored in the base profile which is pretty good. But then when "Change the authentication method" is done from token to basic I see the base profile populating with the tokenValue which should be eradicated and the user and pw is being stored in the secure array of apiml profile
- When the "Login into Authentication service" done twice, for the first time it is populating the base profile with tokenValue and tokenType but for the second time it is populating the profile. Don't know if this comes under scope of this PR. If yes, should be handled.
Signed-off-by: Santhoshi Boyina <Santhoshi.Boyina1@ibm.com>
I recorded some videos of what I am seeing with this branch Screen.Recording.2024-05-09.at.7.18.48.PM.movScreen.Recording.2024-05-09.at.7.16.16.PM.mov |
Hey @SanthoshiBoyina, we should be able to handle either instance of port location for the action. |
Signed-off-by: Santhoshi Boyina <Santhoshi.Boyina1@ibm.com>
Signed-off-by: Santhoshi Boyina <Santhoshi.Boyina1@ibm.com>
It's possible that reviewers might run into this issue: (re: multiple layers) |
Signed-off-by: Santhoshi Boyina <Santhoshi.Boyina1@ibm.com>
Signed-off-by: Santhoshi Boyina <Santhoshi.Boyina1@ibm.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far, thanks @SanthoshiBoyina! I left a couple comments about the "config set" logic and phrasing for the entry in the quick pick.
packages/zowe-explorer/i18n/sample/src/utils/ProfileManagement.i18n.json
Outdated
Show resolved
Hide resolved
Signed-off-by: Santhoshi Boyina <Santhoshi.Boyina1@ibm.com>
Quality Gate failedFailed conditions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good, thanks @SanthoshiBoyina!
If I have a profile with basic auth, and select the option to Change Authentication Method, then I am presented with a quick pick to choose between user/password or token 👍
If I press Escape to cancel out of this quickpick, then I would expect that my profiles should not be changed. However, the basic auth has already been removed from my profile as indicated by the notification below:
Could we wait to remove the basic auth from configuration until the point where the user has accepted all input prompts?
Proposed changes
To switch from Basic authentication to Token based authentication and viceversa
Release Notes
Milestone:
Changelog:
Types of changes
What types of changes does your code introduce to Zowe Explorer?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This checklist will be used as reference for both the contributor and the revieweryarn workspace vscode-extension-for-zowe vscode:prepublish
has been executedFurther comments