zowe / zowe-cli

Zowe CLI
Eclipse Public License 2.0
108 stars 85 forks source link

Reinstate token precedence over password in AbstractRestClient #2119

Closed gejohnston closed 2 months ago

gejohnston commented 2 months ago

What It Does

Modified this description to reflect changes made in response to reviews.

The AbstractRestClient class was originally released with logic that chose a token over user and password. Other commonly used Zowe SDK APIs enforce the order back to PASSWORD_OVER_TOKEN. Consumers which directly extended AbstractRestClient came to rely on the unintended order of TOKEN_OVER_PASSWORD. Later changes in this class to adhere to the Zowe policy of PASSWORD_OVER_TOKEN inadvertently broke such extenders. Until a means is provided for consumers (and/or end-users) to customize their credential order of precedence, we now revert the behavior of AbstractRestClient of back to TOKEN_OVER_PASSWORD.

A new array named authTypeOrder was added to ISession. It contains the order of precedence for authentication types. It can provide a building block for a future, customizable order of precedence. For now, that array is hard-coded in the AbstractRestClient constructor. Logic in another function within AbstractRestClient uses the order of that array to determine the type of authentication to use.

Related issue: Breaking Change in User/Password/Token Order of Precedence

How to Test

Zowe CLI and ZE cannot be used to test the TOKEN_OVER_PASSWORD option. Too many up-stream safeguards ensure that using a token is not even a possibility when user and password are present. We would have to write a specialized test app that confirms that a token can be given precedence over password. Even then we would only confirm that our test app works. I think that everyone is better served if the consuming team confirms that the modifications address their needs.

Therefore, this pull request has been opened as a draft. I propose that we take the following actions:

Review Checklist I certify that I have:

Additional Comments

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 98.33333% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 91.12%. Comparing base (be499e0) to head (bb9b314).

Files Patch % Lines
...perative/src/rest/src/client/AbstractRestClient.ts 98.33% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2119 +/- ## ========================================== + Coverage 91.05% 91.12% +0.06% ========================================== Files 636 636 Lines 19004 19035 +31 Branches 3997 4009 +12 ========================================== + Hits 17305 17346 +41 + Misses 1698 1688 -10 Partials 1 1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sonarcloud[bot] commented 2 months ago

Quality Gate Passed Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
97.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

github-actions[bot] commented 2 months ago

Release succeeded for the master branch. :tada:

The following packages have been published:

Powered by Octorelease :rocket: