-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[v2] SSOTokenLoader Expiration Check #9356
Merged
Merged
Conversation
This file contains 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
ashovlin
requested changes
Mar 17, 2025
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.
- I think it's still worth a changelog entry explaining the change in behavior.
- The Windows test failures look valid.
- Can you add a test case asserting that the expected error was thrown and that no calls were made to
GetRoleCredentials
?
…ase of an expired cached legacy token.
aemous
commented
Mar 21, 2025
ashovlin
reviewed
Mar 25, 2025
ashovlin
approved these changes
Mar 25, 2025
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 of changes:
SSOTokenLoader
to check if the cached legacy token is expired according to the local clock. If expired, it will raise anUnauthorizedSSOTokenError
instead of sending an expired token to Identity Center'sGetRoleCredentials
API.UnauthorizedSSOTokenError
is raised.Description of tests:
aws sso login
.aws s3 ls --profile SSOProfile --debug
)UnauthorizedSSOTokenError
was raised, and checked the stacktrace (pasted below) to verify this exception was raised withoutGetRoleCredentials
being called.Stacktrace from manual workflow on this branch
Stacktrace from manual workflow on v2
Notice the above exception is raised due to a returned
UnauthorizedException
fromGetRoleCredentials
, while this is not the case in the previous stacktrace.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.