-
Notifications
You must be signed in to change notification settings - Fork 1k
Re-resolve SSO access token on each credential refresh instead of caching it at construction time #7097
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
base: master
Are you sure you want to change the base?
Re-resolve SSO access token on each credential refresh instead of caching it at construction time #7097
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "type": "bugfix", | ||
| "category": "AWS SDK for Java v2", | ||
| "contributor": "", | ||
| "description": "Re-resolve SSO access token on each credential refresh in the SSOCredentialsProvider instead of caching it at construction time ensuring that refreshed tokens (for example from running `aws sso login`) are always used." | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,7 +29,6 @@ | |
| import software.amazon.awssdk.services.sso.auth.ExpiredTokenException; | ||
| import software.amazon.awssdk.services.sso.auth.SsoCredentialsProvider; | ||
| import software.amazon.awssdk.utils.IoUtils; | ||
| import software.amazon.awssdk.utils.Validate; | ||
|
|
||
| /** | ||
| * Resolve the access token from the cached token file. If the token has expired then throw out an exception to ask the users to | ||
|
|
@@ -48,7 +47,15 @@ public SsoAccessTokenProvider(Path cachedTokenFilePath) { | |
|
|
||
| @Override | ||
| public SdkToken resolveToken() { | ||
| return tokenFromFile(); | ||
| try { | ||
| return tokenFromFile(); | ||
| } catch (ExpiredTokenException e) { | ||
| throw e; | ||
| } catch (Exception e) { | ||
| throw ExpiredTokenException.builder() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add a Java doc explaining why any Exception is treated as ExpiredTokenException? |
||
| .cause(e) | ||
| .build(); | ||
| } | ||
| } | ||
|
|
||
| private SdkToken tokenFromFile() { | ||
|
|
@@ -61,25 +68,22 @@ private SdkToken tokenFromFile() { | |
|
|
||
| private SdkToken getTokenFromJson(String json) { | ||
| JsonNode jsonNode = PARSER.parse(json); | ||
| String expiration = jsonNode.field("expiresAt").map(JsonNode::text).orElse(null); | ||
| String expirationStr = jsonNode.field("expiresAt").map(JsonNode::text).orElse(null); | ||
|
|
||
| if (expirationStr == null) { | ||
| throw ExpiredTokenException.builder().build(); | ||
| } | ||
|
|
||
| Validate.notNull(expiration, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (non-blocking): When expiresAt is missing, this throws the same generic DEFAULT_MESSAGE as an expired token. The old code gave a specific message here: "The SSO session's expiration time could not be determined." Can we keep a distinct message for the expiresAt == null case so a malformed token file is easy to tell apart from an expired one. |
||
| "The SSO session's expiration time could not be determined. Please refresh your SSO session."); | ||
| Instant expiration = Instant.parse(expirationStr); | ||
|
|
||
| if (tokenIsInvalid(expiration)) { | ||
| throw ExpiredTokenException.builder().message("The SSO session associated with this profile has expired or is" | ||
| + " otherwise invalid. To refresh this SSO session run aws sso" | ||
| + " login with the corresponding profile.").build(); | ||
| if (Instant.now().isAfter(expiration)) { | ||
| throw ExpiredTokenException.builder().build(); | ||
| } | ||
|
|
||
| return SsoAccessToken.builder() | ||
| .accessToken(jsonNode.asObject().get("accessToken").text()) | ||
| .expiresAt(Instant.parse(expiration)).build(); | ||
|
|
||
| .expiresAt(expiration).build(); | ||
| } | ||
|
|
||
| private boolean tokenIsInvalid(String expirationTime) { | ||
| return Instant.now().isAfter(Instant.parse(expirationTime)); | ||
| } | ||
|
|
||
| } | ||
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.
can we make it private or package-private ?