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

Debounce credential prompts and check profile locks in more places #3480

Merged
merged 24 commits into from
Mar 4, 2025

Conversation

traeok
Copy link
Member

@traeok traeok commented Feb 24, 2025

Proposed changes

  • Adds a 30 second timeout to waitForUnlock to prevent deadlocks when using the function (safety mechanism)
  • "Debounces" credential prompts from parallel requests to a mainframe system by only showing one credential prompt, until the user answers that prompt.
    • Behavior remains unchanged: If the user selects "Cancel" or closes the dialog, the profile remains locked. The user can update their profile through the tree views through the "Manage Profile" to unlock the profile in the filesystem.
    • Resolves an issue where the user was spammed with multiple credential prompts when using a virtual workspace with a profile w/ invalid credentials.
  • Eliminated cached profile use in filesystems: All filesystem requests will get the latest profile object to prevent invalid, cached credentials from being used.
    • The issue still exists in tree views for credential desync in the quick picks. I have filed issue # to track/resolve this in the future.

Release Notes

Milestone: 3.1.2

Changelog:

  • Fixed issue where users were prompted several times when using a profile with invalid credentials in a VS Code workspace. Now, the user is only prompted once per profile, allowing the user to enter in new credentials.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (non-breaking change which adds or improves functionality)
  • Breaking change (a change that would cause existing functionality to not work as expected)
  • Documentation (Markdown, README updates)
  • Other (please specify above in "Proposed changes" section)

Checklist

General

  • I have read the CONTRIBUTOR GUIDANCE wiki
  • All PR dependencies have been merged and published (if applicable)
  • A GIF or screenshot is included in the PR for visual changes
  • The pre-publish command has been executed:
    • v2 and below: yarn workspace vscode-extension-for-zowe vscode:prepublish
    • v3: pnpm --filter vscode-extension-for-zowe vscode:prepublish

Code coverage

  • There is coverage for the code that I have added
  • I have added new test cases and they are passing
  • I have manually tested the changes

Deployment

  • I have added developer documentation (if applicable)
  • Documentation should be added to Zowe Docs (discussing w/ Ana offline)
    • If you're an outside contributor, please post in the #zowe-doc Slack channel to coordinate documentation.
    • Otherwise, please check with the rest of the squad about any needed documentation before merging.
  • These changes may need ported to the appropriate branches (list here):

Copy link

codecov bot commented Feb 24, 2025

Codecov Report

Attention: Patch coverage is 90.19608% with 15 lines in your changes missing coverage. Please review.

Project coverage is 93.49%. Comparing base (0643906) to head (d2a70f4).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...kages/zowe-explorer/src/trees/uss/UssFSProvider.ts 93.02% 6 Missing ⚠️
...we-explorer/src/trees/dataset/DatasetFSProvider.ts 86.11% 5 Missing ⚠️
...ages/zowe-explorer-api/src/profiles/AuthHandler.ts 88.46% 3 Missing ⚠️
packages/zowe-explorer/src/utils/AuthUtils.ts 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3480      +/-   ##
==========================================
- Coverage   93.55%   93.49%   -0.06%     
==========================================
  Files         120      120              
  Lines       12714    12822     +108     
  Branches     2820     2909      +89     
==========================================
+ Hits        11894    11988      +94     
- Misses        819      833      +14     
  Partials        1        1              

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

@traeok traeok changed the title refactor: Use locks in FS remote lookup to avoid credential loops in virtual workspaces Use locks in FS remote lookup to avoid credential loops in virtual workspaces Feb 24, 2025
@pull-request-size pull-request-size bot added size/L and removed size/M labels Feb 24, 2025
@traeok traeok force-pushed the fix/cred-loop-virtual-workspace branch from 510342d to f693249 Compare February 25, 2025 01:45
@pull-request-size pull-request-size bot added size/M and removed size/L labels Feb 25, 2025
@pull-request-size pull-request-size bot added size/L and removed size/M labels Feb 25, 2025
@traeok traeok changed the title Use locks in FS remote lookup to avoid credential loops in virtual workspaces Use locks in FS Feb 25, 2025
@traeok traeok changed the title Use locks in FS Debounce credential prompts and use locks in more places Feb 25, 2025
@traeok traeok changed the title Debounce credential prompts and use locks in more places Debounce credential prompts and check profile locks in more places Feb 25, 2025
@traeok traeok force-pushed the fix/cred-loop-virtual-workspace branch from 00bfe7d to 9da0610 Compare February 25, 2025 20:47
@traeok traeok removed the no-changelog Add to PR's that don't require a CHANGELOG update label Feb 27, 2025
@traeok traeok marked this pull request as ready for review February 27, 2025 15:49
Copy link
Contributor

@anaxceron anaxceron left a comment

Choose a reason for hiding this comment

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

Made a comment re: one of the changelogs

Signed-off-by: Trae Yelovich <[email protected]>
@traeok traeok force-pushed the fix/cred-loop-virtual-workspace branch from 94fec29 to 81d2501 Compare February 27, 2025 17:14
@traeok traeok requested a review from anaxceron February 27, 2025 17:19
Copy link
Contributor

@anaxceron anaxceron left a comment

Choose a reason for hiding this comment

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

Fantastic changelogs, thanks @traeok!

@traeok traeok linked an issue Feb 28, 2025 that may be closed by this pull request
@adam-wolfe
Copy link
Contributor

Should this be in the 3.1.2 milestone?

@traeok
Copy link
Member Author

traeok commented Feb 28, 2025

Should this be in the 3.1.2 milestone?

Yes, thanks for the heads up - I've added it to the milestone

@traeok traeok added this to the v3.1.2 milestone Feb 28, 2025
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

LGTM! 😋

Thanks for adding these safeguards for virtual workspace users. 🥳
I know we have a few things we can't work around (#3475) but preventing multiple prompts goes a long way! 🙏

Copy link
Member

@t1m0thyj t1m0thyj left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @traeok!

@zFernand0 zFernand0 merged commit 8e57200 into main Mar 4, 2025
33 checks passed
@zFernand0 zFernand0 deleted the fix/cred-loop-virtual-workspace branch March 4, 2025 18:45
traeok added a commit that referenced this pull request Mar 7, 2025
…3480)

* refactor: check locks in dataset FS remote lookup; update AuthHandler.waitForUnlock

Signed-off-by: Trae Yelovich <[email protected]>

* refactor: handle auth prompts for more USS scenarios

Signed-off-by: Trae Yelovich <[email protected]>

* refactor: use latest profile in more places in FSPs; part 2 to recent bugfix

Signed-off-by: Trae Yelovich <[email protected]>

* fix: debounce auth prompts during parallel requests

Signed-off-by: Trae Yelovich <[email protected]>

* feat: useModal optional property for auth prompts

Signed-off-by: Trae Yelovich <[email protected]>

* fix(AuthHandler): use modal prompt for FSPs

Signed-off-by: Trae Yelovich <[email protected]>

* refactor: debounce prompts using separate prompt mutexes

Signed-off-by: Trae Yelovich <[email protected]>

* fix(tests): use fake timers in waitForUnlock test

Signed-off-by: Trae Yelovich <[email protected]>

* refactor: unlock all profiles after vault changed

Signed-off-by: Trae Yelovich <[email protected]>

* fix tests; update changelog

Signed-off-by: Trae Yelovich <[email protected]>

* fix: await AuthHandler.shouldHandleAuthError

Signed-off-by: Trae Yelovich <[email protected]>

* chore: update changelog

Signed-off-by: Trae Yelovich <[email protected]>

* wip: AuthHandler & AuthUtils patch coverage

Signed-off-by: Trae Yelovich <[email protected]>

* fix(AuthHandler): remove unnecessary branch

Signed-off-by: Trae Yelovich <[email protected]>

* tests: Data Set and USS profile lock tests

Signed-off-by: Trae Yelovich <[email protected]>

* refactor: use ZoweLogger.warn instead of debug

Signed-off-by: Trae Yelovich <[email protected]>

* refactor: remove unused useModal param

Signed-off-by: Trae Yelovich <[email protected]>

* chore: add ZE API changelog

Signed-off-by: Trae Yelovich <[email protected]>

* tests: fix logic & remove skip on listFiles cases

Signed-off-by: Trae Yelovich <[email protected]>

* more test cases for profile lock checks

Signed-off-by: Trae Yelovich <[email protected]>

* chore: fix changelog

Signed-off-by: Trae Yelovich <[email protected]>

---------

Signed-off-by: Trae Yelovich <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

Invalid credentials popup blocking all actions
6 participants