Repository navigation
fix: enhance token refresh handling and improve error reporting - #138
Merged
Merged
Conversation
Signed-off-by: Alex Lovell-Troy <alovelltroy@lanl.gov>
synackd
requested changes
Sep 17, 2026
synackd
left a comment
synackd
left a comment
Contributor
There was a problem hiding this comment.
Looks good, just one concern (with suggested patch) about separate HTTP client for token refresh.
Reuse the SMD HTTP transport and timeout in the dedicated token client so token refresh honors the configured TLS trust and insecure settings. Assign the loaded CA pool to the outer variable instead of shadowing it, ensuring the configured certificate reaches the shared TLS transport. Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
Exercise NewSMDClient against a local HTTPS server with a custom CA and with TLS verification disabled. Reject the initial token to verify that token refresh honors the configured TLS settings and cache loading succeeds after retrying with the refreshed token. Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
synackd
enabled auto-merge (squash)
October 20, 2026 15:28
Raise the Go requirement to 1.26.9 and update golang.org/x/net to v0.60.0 to address the vulnerabilities reported by govulncheck. Update the required transitive modules and tidy the module sums. Align the debug image with Go 1.26.9 so debug builds also use the patched standard library. Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
Contributor
|
@travisbcotton Checks are passing now, this should be ready for review. Auto merge is enabled so this will be merged after your approval. |
travisbcotton
approved these changes
Oct 20, 2026
synackd
approved these changes
Oct 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request improves the reliability and test coverage of the SMD client’s access token refresh logic. The most important changes include stricter error handling and validation in token refresh, dedicated HTTP client usage for token requests, and new tests to ensure correct behavior when token refresh fails.
Error Handling and Validation Improvements:
refreshTokenWithContextmethod now returns detailed errors for various failure modes, such as missing HTTP client, non-2xx responses, malformed or empty tokens, and JSON decoding errors. It also trims whitespace from the received token and rejects empty tokens.getSMD, if the token refresh fails after a rejected access token, the error is now returned immediately instead of being silently ignored, providing better visibility and preventing retries with invalid tokens.HTTP Client Usage and Struct Changes:
tokenClientfield toSMDClient, ensuring token refresh requests use a separate HTTP client from regular SMD requests. Constructors and tests are updated to initialize this field appropriately. [1] [2] [3] [4]Testing Improvements:
getSMDreturns a refresh failure error without retrying, and that the access token remains unchanged.Code Consistency:
oidcTokenDatato use Go naming conventions (e.g.,AccessTokeninstead ofAccess_token).Checklist
make test(or equivalent) locally and all tests passgit commit -s) with my real name and email<filename>.licensesidecarLICENSES/directoryType of Change