-
Notifications
You must be signed in to change notification settings - Fork 7
Display "N/A" for tokens without expiration metadata #539
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
Conversation
Co-authored-by: MariusStorhaug <[email protected]>
|
@copilot Lets also make sure the same applies for Fine Grained PAT tokens. |
Fine Grained PAT tokens are already covered by these changes. Both classic PATs ( Verified with test showing both token types correctly display "Unknown": |
|
@copilot Lets update the tests to also do this for the applicable logon types. |
Co-authored-by: MariusStorhaug <[email protected]>
Added tests for TokenExpiresIn behavior for applicable logon types in commit 55834ee:
Tests use Pester's |
|
@copilot think we need to update the GitHub.Tests.ps1 file to support that the GITHUB_TOKEN is allowed to have the Unknown value. From logs: |
Co-authored-by: MariusStorhaug <[email protected]>
... Fixed in commit 41e27c8. The test was incorrectly assuming all IAT tokens have expiration dates. GITHUB_TOKEN (IAT used in GitHub Actions) doesn't have expiration metadata, so it now correctly validates that The test now uses Note: GitHub App Installation tokens (also IAT but created via |
|
@copilot Lets ensure we have tests that check the TokenExpiresIn and TokenExpiresAt for all token types. We do not expect GITHUB_TOKEN or PAT/Finegrained PAT to have this, however we expect IATs (except GITHUB_TOKEN) to have it. |
Co-authored-by: MariusStorhaug <[email protected]>
Added comprehensive test coverage for all token types in commit 5aabf79: Tokens WITHOUT expiration (null values):
Tokens WITH expiration (DateTime/TimeSpan):
Also added documentation comment (lines 48-52) explaining the expected behavior for each token type. |
Co-authored-by: MariusStorhaug <[email protected]>
|
@copilot I am now getting the following failures: The failures come on the following contexts: Context As a GitHub App from an Organization using JWT + IAT on organization account Thinking we should rethink this test abit. Lets make a single generic test, with no "skips" but have the test internally contain the logic that checks the correct status. This way we contain the behavior in a single location and makes it easier to find what we expect for outcome. |
Co-authored-by: MariusStorhaug <[email protected]>
... Fixed in commit acb07c8. The issue occurred because the test at line 120 sets the default context to IAT using |
…e TokenExpiresAt and TokenExpiresIn based on token type
…nect-GitHubApp and Connect-GitHubAccount
|
Module GitHub - 0.40.5-copilotsettokenexpiresintounknown001 published to the PowerShell Gallery. |
|
GitHub release for GitHub v0.40.5-copilotsettokenexpiresintounknown001 has been created. |
…sAt is null and improve formatting for terminal support
…enExpiresIn properties with proper handling for null TokenExpiresAt values
|
Module GitHub - 0.40.5-copilotsettokenexpiresintounknown002 published to the PowerShell Gallery. |
|
GitHub release for GitHub v0.40.5-copilotsettokenexpiresintounknown002 has been created. |
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.
Pull request overview
This PR improves token expiration handling for GitHub contexts by displaying "N/A" for tokens without expiration metadata instead of showing "Expired" or zero values. The changes update script properties to return null instead of zero timespans, enhance table formatting with conditional "N/A" display, and add comprehensive validation tests for different authentication types.
Key Changes:
- Modified
TokenExpiresInandRefreshTokenExpiresInscript properties to return null when expiration metadata is absent, rather than returningTimeSpan.Zero - Enhanced table formatting to display "N/A" (with gray color when supported) for tokens without expiration data
- Added test validation to verify expiration properties are correctly populated for APP tokens and null for other auth types
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/types/GitHubContext.Types.ps1xml | Updates script properties to return null instead of TimeSpan.Zero when expiration date is not set |
| src/formats/GitHubContext.Format.ps1xml | Adds "N/A" display logic with color formatting for both TokenExpiresAt and TokenExpiresIn columns |
| tests/GitHub.Tests.ps1 | Adds validation test for token expiration properties and new IAT auth type test; moves context creation to BeforeAll |
| tests/Apps.Tests.ps1 | Enhances installation token expiration test to validate both TokenExpiresAt and TokenExpiresIn properties; fixes quote style |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Module GitHub - 0.40.5 published to the PowerShell Gallery. |
|
GitHub release for GitHub v0.40.5 has been created. |
This release improves how token expiration information is handled and displayed in the GitHub context. The changes ensure more accurate and user-friendly reporting of token expiration properties, enhance formatting in table views, and add comprehensive tests to validate these behaviors.
Token Expiration Calculation & Representation:
TokenExpiresInandRefreshTokenExpiresInscript properties inGitHubContext.Types.ps1xmlto only return a value whenTokenExpiresAtorRefreshTokenExpiresAtis present, otherwise return nothing. This avoids returning a zero timespan when the expiration is not set.Table Formatting & Display:
GitHubContext.Format.ps1xmlto:TokenExpiresAtas 'N/A' (in gray if supported) when not present, instead of omitting the column.TokenExpiresInas 'N/A' (with similar formatting) when not available, and color the value based on time remaining when present.Testing Enhancements:
GitHub.Tests.ps1andApps.Tests.ps1to:TokenExpiresAtandTokenExpiresInproperties for correctness, type, and logical values across different authentication types.AuthTypeand that expiration properties behave as expected for both App and non-App contexts.These changes make token expiration handling more robust, user-friendly, and well-tested throughout the module.
Test Coverage Summary:
ghp)github_pat)Token Expiration Behavior:
ghpand fine-grainedgithub_pat): No expiration metadata → displays "Unknown"Example Output:
Before:
After:
TokenExpiresIntoN/Afor PAT tokens #538