Adding the ability to switch between basic authentication and token-based authentication #2944

SanthoshiBoyina1 · 2024-06-10T08:35:54Z

Proposed changes

To add the ability to switch between basic authentication and token-based authentication.

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 apply

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Updates to Documentation or Tests (if none of the other choices apply)

Checklist

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 reviewer

Further comments

Signed-off-by: Santhoshi Boyina <[email protected]>

…vscode-extension-for-zowe into feat/authSwitch

Signed-off-by: Santhoshi Boyina <[email protected]>

codecov · 2024-06-10T14:08:38Z

Codecov Report

Attention: Patch coverage is 96.62921% with 3 lines in your changes missing coverage. Please review.

Project coverage is 93.47%. Comparing base (adf0f28) to head (8457c99).

Files	Patch %	Lines
packages/zowe-explorer/src/Profiles.ts	97.59%	2 Missing ⚠️
...kages/zowe-explorer/src/utils/ProfileManagement.ts	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2944      +/-   ##
==========================================
- Coverage   93.54%   93.47%   -0.07%     
==========================================
  Files         104      104              
  Lines       10889    10978      +89     
  Branches     2293     2400     +107     
==========================================
+ Hits        10186    10262      +76     
- Misses        702      715      +13     
  Partials        1        1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

t1m0thyj

LGTM, thanks @SanthoshiBoyina1!

Tested this PR with a config file that had user/password in base profile. Switching first time stored token in base profile, and then switching back stored user/password directly on service profile. Although this is different from the original state of my config file, I think it is expected behavior 👍

t1m0thyj · 2024-07-15T13:55:14Z

packages/zowe-explorer/src/Profiles.ts

+        configApi.delete(`${profInfo.mergeArgsForProfile(profAttrs).knownArgs.find((arg) => arg.argName === "user")?.argLoc.jsonLoc}`);
+        configApi.delete(`${profInfo.mergeArgsForProfile(profAttrs).knownArgs.find((arg) => arg.argName === "password")?.argLoc.jsonLoc}`);


The string interpolation isn't necessary here:

Suggested change

configApi.delete(`${profInfo.mergeArgsForProfile(profAttrs).knownArgs.find((arg) => arg.argName === "user")?.argLoc.jsonLoc}`);

configApi.delete(`${profInfo.mergeArgsForProfile(profAttrs).knownArgs.find((arg) => arg.argName === "password")?.argLoc.jsonLoc}`);

configApi.delete(profInfo.mergeArgsForProfile(profAttrs).knownArgs.find((arg) => arg.argName === "user")?.argLoc.jsonLoc);

configApi.delete(profInfo.mergeArgsForProfile(profAttrs).knownArgs.find((arg) => arg.argName === "password")?.argLoc.jsonLoc);

t1m0thyj · 2024-07-15T13:55:48Z

packages/zowe-explorer/src/Profiles.ts

+            configApi.delete(`${profInfo.mergeArgsForProfile(profAttrs).knownArgs.find((arg) => arg.argName === "tokenType")?.argLoc.jsonLoc}`);
+            configApi.delete(`${profInfo.mergeArgsForProfile(profAttrs).knownArgs.find((arg) => arg.argName === "tokenValue")?.argLoc.jsonLoc}`);
+            configApi.delete(`${profInfo.mergeArgsForProfile(profAttrs).knownArgs.find((arg) => arg.argName === "tokenExpiration")?.argLoc.jsonLoc}`);
+        } else {
+            const profAttrs = await this.getProfileFromConfig(profileName);
+            configApi.set(`${profAttrs.profLoc.jsonLoc}.secure`, ["user", "password"]);
+            configApi.delete(`${profInfo.mergeArgsForProfile(profAttrs).knownArgs.find((arg) => arg.argName === "tokenType")?.argLoc.jsonLoc}`);
+            configApi.delete(`${profInfo.mergeArgsForProfile(profAttrs).knownArgs.find((arg) => arg.argName === "tokenValue")?.argLoc.jsonLoc}`);
+            configApi.delete(`${profInfo.mergeArgsForProfile(profAttrs).knownArgs.find((arg) => arg.argName === "tokenExpiration")?.argLoc.jsonLoc}`);


Same comment as above about string interpolation 😋

t1m0thyj · 2024-07-15T14:00:28Z

packages/zowe-explorer/src/Profiles.ts

@@ -1361,7 +1511,7 @@ export class Profiles extends ProfilesCache {
            .map((arg) => arg.argName);
    }

-    private async loginWithRegularProfile(serviceProfile: zowe.imperative.IProfileLoaded, node?: IZoweNodeType): Promise<boolean> {
+    public async loginWithRegularProfile(serviceProfile: zowe.imperative.IProfileLoaded, node?: IZoweNodeType): Promise<boolean> {


This method seems like it could be kept private if the only added call is inside this file.

seeing some odd behavior, but I believe the changes I requested earlier were addressed

zFernand0

Could we wait for one more day before merging?
Just seeing a few things that may or may not be related.

Signed-off-by: Santhoshi Boyina <[email protected]>

…/vscode-extension-for-zowe into feat/authSwitch Signed-off-by: Santhoshi Boyina <[email protected]>

t1m0thyj

LGTM, thanks @SanthoshiBoyina1!

zFernand0

apologies for the delay.
I'll open the issue I mentioned in a few moments

t1m0thyj · 2024-07-30T19:18:58Z

@SanthoshiBoyina1 @JillieBeanSim Should this feature be ported to the vNext branch?

SanthoshiBoyina1 · 2024-08-01T12:52:48Z

@SanthoshiBoyina1 @JillieBeanSim Should this feature be ported to the vNext branch?

I will move this feature to vNext branch

t1m0thyj · 2024-08-30T19:20:54Z

I will move this feature to vNext branch

Done in #3062

SanthoshiBoyina1 and others added 25 commits April 22, 2024 22:43

Switch Basic auth. to Token auth. and viceversa

01423e1

Signed-off-by: Santhoshi Boyina <[email protected]>

Updated code

dfb1cc8

Signed-off-by: Santhoshi Boyina <[email protected]>

Merge branch 'main' into feat/authSwitch

954590d

Merge branch 'main' into feat/authSwitch

ee2a96d

Add Unit test coverage

59e2416

Signed-off-by: Santhoshi Boyina <[email protected]>

Merge branch 'main' into feat/authSwitch

1279b2f

Merge branch 'feat/authSwitch' of https://github.com/SanthoshiBoyina/…

a925bbd

…vscode-extension-for-zowe into feat/authSwitch

handled tokenType and tokenValue

7c66fcb

Signed-off-by: Santhoshi Boyina <[email protected]>

address circular login issue

55f696b

Signed-off-by: Santhoshi Boyina <[email protected]>

Merge branch 'main' into feat/authSwitch

bcfd290

update CHANGELoG.md file

d94f975

Signed-off-by: Santhoshi Boyina <[email protected]>

code coverage for missing lines

925768d

Signed-off-by: Santhoshi Boyina <[email protected]>

address review comments

d0d8ffe

Signed-off-by: Santhoshi Boyina <[email protected]>

Merge branch 'main' into feat/authSwitch

0e62cd0

Merge branch 'main' into feat/authSwitch

8dcc6fb

Merge branch 'main' into feat/authSwitch

a931e3d

update switch to token-auth

519fac6

Signed-off-by: Santhoshi Boyina <[email protected]>

update switch to basic auth

49070fc

Signed-off-by: Santhoshi Boyina <[email protected]>

update unit test case for switch from basic to token-auth

67b6430

Signed-off-by: Santhoshi Boyina <[email protected]>

update unit test case for switch from token-auth to basic-auth

9ebd39a

Signed-off-by: Santhoshi Boyina <[email protected]>

update unit test case for switch authentication edge cases

7c1c4e6

Signed-off-by: Santhoshi Boyina <[email protected]>

update unit test case for token-auth based on the profile used

0e93f04

Signed-off-by: Santhoshi Boyina <[email protected]>

to add authentication successful message

84e1881

Signed-off-by: Santhoshi Boyina <[email protected]>

update error messages

1236bc4

Signed-off-by: Santhoshi Boyina <[email protected]>

update unit test cases

fb2f70e

Signed-off-by: Santhoshi Boyina <[email protected]>

JillieBeanSim mentioned this pull request Jun 11, 2024

Added the ability to switch between basic authentication and token-based authentication #2867

Closed

16 tasks

JTonda requested review from zFernand0, JillieBeanSim and traeok June 11, 2024 15:21

SanthoshiBoyina1 dismissed adam-wolfe’s stale review via ffe4b40 July 14, 2024 11:28

SanthoshiBoyina1 requested review from traeok and adam-wolfe July 15, 2024 02:32

traeok previously approved these changes Jul 15, 2024

View reviewed changes

adam-wolfe previously approved these changes Jul 15, 2024

View reviewed changes

t1m0thyj previously approved these changes Jul 15, 2024

View reviewed changes

zFernand0 reviewed Jul 15, 2024

View reviewed changes

SanthoshiBoyina1 added 3 commits July 15, 2024 22:55

To remove unnecessary use of string interpolation

c0437ac

Signed-off-by: Santhoshi Boyina <[email protected]>

To change loginWithRegualarProfile() method to private

7cb3048

Signed-off-by: Santhoshi Boyina <[email protected]>

Merge branch 'feat/authSwitch' of https://github.com/SanthoshiBoyina1…

c781f13

…/vscode-extension-for-zowe into feat/authSwitch Signed-off-by: Santhoshi Boyina <[email protected]>

SanthoshiBoyina1 dismissed stale reviews from t1m0thyj, adam-wolfe, and traeok via c781f13 July 15, 2024 17:47

SanthoshiBoyina1 requested review from adam-wolfe, traeok, t1m0thyj and zFernand0 July 15, 2024 17:49

t1m0thyj approved these changes Jul 17, 2024

View reviewed changes

traeok approved these changes Jul 17, 2024

View reviewed changes

Merge branch 'main' into feat/authSwitch

8457c99

traeok merged commit e6a0174 into zowe:main Jul 17, 2024
15 checks passed

zFernand0 approved these changes Jul 17, 2024

View reviewed changes

JillieBeanSim added this to the v2.17.0 milestone Jul 18, 2024

zFernand0 mentioned this pull request Jul 22, 2024

Token related operations may cause invalid profiles when multiple layers are involved #3011

Open

t1m0thyj added the needs-ported Apply to issues or PRs that need ported label Jul 30, 2024

t1m0thyj removed the needs-ported Apply to issues or PRs that need ported label Aug 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding the ability to switch between basic authentication and token-based authentication #2944

Adding the ability to switch between basic authentication and token-based authentication #2944

SanthoshiBoyina1 commented Jun 10, 2024

codecov bot commented Jun 10, 2024 •

edited

Loading

t1m0thyj left a comment

t1m0thyj Jul 15, 2024

t1m0thyj Jul 15, 2024

t1m0thyj Jul 15, 2024

zFernand0 left a comment

t1m0thyj left a comment

zFernand0 left a comment

t1m0thyj commented Jul 30, 2024

SanthoshiBoyina1 commented Aug 1, 2024

t1m0thyj commented Aug 30, 2024

		configApi.delete(`${profInfo.mergeArgsForProfile(profAttrs).knownArgs.find((arg) => arg.argName === "user")?.argLoc.jsonLoc}`);
		configApi.delete(`${profInfo.mergeArgsForProfile(profAttrs).knownArgs.find((arg) => arg.argName === "password")?.argLoc.jsonLoc}`);

Adding the ability to switch between basic authentication and token-based authentication #2944

Adding the ability to switch between basic authentication and token-based authentication #2944

Conversation

SanthoshiBoyina1 commented Jun 10, 2024

Proposed changes

Release Notes

Types of changes

Checklist

Further comments

codecov bot commented Jun 10, 2024 • edited Loading

Codecov Report

t1m0thyj left a comment

Choose a reason for hiding this comment

t1m0thyj Jul 15, 2024

Choose a reason for hiding this comment

t1m0thyj Jul 15, 2024

Choose a reason for hiding this comment

t1m0thyj Jul 15, 2024

Choose a reason for hiding this comment

zFernand0 left a comment

Choose a reason for hiding this comment

t1m0thyj left a comment

Choose a reason for hiding this comment

zFernand0 left a comment

Choose a reason for hiding this comment

t1m0thyj commented Jul 30, 2024

SanthoshiBoyina1 commented Aug 1, 2024

t1m0thyj commented Aug 30, 2024

codecov bot commented Jun 10, 2024 •

edited

Loading