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

feat(ProfileInfo): JWT token expiration detection #2298

Merged
merged 13 commits into from
Oct 10, 2024

Conversation

traeok
Copy link
Member

@traeok traeok commented Oct 8, 2024

What It Does

Fernando mentioned that extenders would benefit from having access to the JWT token expiration feature from Zowe Explorer, so I figured it made the most sense to put it the logic for it in ProfileInfo. That way, anyone using Imperative to access/manage profiles can leverage this functionality.

How to Test

Once merged, this can be tested in action after this PR is updated: zowe/zowe-explorer-vscode#3174

Here's a manual test to show the behavior of the function itself:

  • Checkout this branch and run npm run build && npm pack from the packages/imperative folder
  • Copy the zowe-imperative-8.1.0.tgz file from this folder and place it into a new folder outside of the repo
  • cd to the empty folder and run zowe config init --no-prompt && npm init -y && npm install zowe-imperative-8.1.0.tgz @zowe/secrets-for-zowe-sdk (you don't need secrets SDK, but its easier than getting around cred manager setup in ProfileInfo)
  • Copy this config and replace the contents of zowe.config.json:
{
    "$schema": "./zowe.schema.json",
    "profiles": {
        "zosmf": {
            "type": "zosmf",
            "properties": {
                "port": 1337
            }
        },
        "project_base": {
            "type": "base",
            "properties": {
                "host": "localhost",
                // this token expired in 2001
                // if you want to test an unexpired token, this one expires in 2128: 123.eyJleHAiOjUwMDAwMDAwMDB9.456
                "tokenValue": "123.eyJleHAiOjEwMDAwMDAwMDB9.456"
            }
        }
    },
    "defaults": {
        "zosmf": "zosmf",
        "base": "project_base"
    }
}
  • Save the following code snippet as script.mjs within this test folder and run it using node script.mjs
const { ProfileInfo } = await import("@zowe/imperative");
const profInfo = new ProfileInfo("zowe");
await profInfo.readProfilesFromDisk({ projectDir: process.cwd() });
console.log("token expired:", profInfo.hasTokenExpiredForProfile("zosmf"));

The script should output token expired: true.

Review Checklist
I certify that I have:

Additional Comments

Copy link

codecov bot commented Oct 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.08%. Comparing base (a4b4d45) to head (7e1b0ab).
Report is 15 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2298      +/-   ##
==========================================
+ Coverage   91.07%   91.08%   +0.01%     
==========================================
  Files         628      628              
  Lines       17874    17896      +22     
  Branches     3842     3848       +6     
==========================================
+ Hits        16278    16300      +22     
  Misses       1595     1595              
  Partials        1        1              

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

@traeok traeok force-pushed the feat/imperative/jwt-token-expiration branch from fbf581e to f52417e Compare October 8, 2024 21:38
@traeok traeok marked this pull request as ready for review October 8, 2024 22:41
@traeok
Copy link
Member Author

traeok commented Oct 8, 2024

I don't have permissions on Zowe CLI to update the new issue reported by SonarCloud, but it is a false positive as argValue will almost always be type string. Even in some abnormal case where it isn't a string, all exceptions will be handled in the try/catch block.

@adam-wolfe adam-wolfe added this to the V3.1.0 milestone Oct 9, 2024
Comment on lines 200 to 221
// Cannot decode LTPA tokens without private key
if (tokenTypeProp?.argValue == "LtpaToken2") {
return false;
}

const fullToken = tokenValueProp.argValue.toString();
// JWT format: [header].[payload].[signature]
const tokenParts = fullToken.split(".");
try {
const payloadJson = JSON.parse(Buffer.from(tokenParts[1], "base64url").toString("utf8"));
if ("exp" in payloadJson) {
// The expire time is stored in seconds since UNIX epoch.
// The Date constructor expects a timestamp in milliseconds.
const msPerSec = 1000;
const expireDate = new Date(payloadJson.exp * msPerSec);
return expireDate < new Date();
}
} catch (err) {
return false;
}

return false;
Copy link
Member

@t1m0thyj t1m0thyj Oct 10, 2024

Choose a reason for hiding this comment

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

Thanks for adding this to the ProfileInfo API! As the ProfileInfo API is used only by extenders like ZE, I'm curious if we'd want to move this part of the method to make it usable by CLI itself?

Perhaps we could have a static isTokenExpired method on the ConfigUtils or RestStandaloneUtils classes, that only checks if the token is expired and doesn't handle loading it from config.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call - I added a function to ConfigUtils called hasTokenExpired and updated the ProfileInfo function to use it.
Also added an entry for the new function in the changelog 👍

@traeok traeok requested a review from t1m0thyj October 10, 2024 15:52
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!

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! 😋

looking at the code reminded me of this issue:

Copy link

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.

Requesting some edits to the changelog

packages/imperative/CHANGELOG.md Outdated Show resolved Hide resolved
packages/imperative/CHANGELOG.md Outdated Show resolved Hide resolved
@traeok traeok requested a review from anaxceron October 10, 2024 17:57
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 changelog @traeok!

@traeok traeok merged commit ad015e0 into master Oct 10, 2024
16 of 17 checks passed
@traeok traeok deleted the feat/imperative/jwt-token-expiration branch October 10, 2024 18:50
@traeok traeok added the release-minor Indicates a minor feature has been added label Oct 10, 2024
Copy link

Release succeeded for the master branch. 🎉

The following packages have been published:

Powered by Octorelease 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-minor Indicates a minor feature has been added released
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

7 participants