Re-resolve SSO access token on each credential refresh instead of caching it at construction time#7097
Re-resolve SSO access token on each credential refresh instead of caching it at construction time#7097alextwoods wants to merge 3 commits into
Conversation
…hing it at construction time
alextwoods
force-pushed
the
alexwoo/fix_sso_token_refresh
branch
from
810c829 to
e35f5fa
Compare
alextwoods
force-pushed
the
alexwoo/fix_sso_token_refresh
branch
from
df6dd8c to
5f9d648
Compare
alextwoods
force-pushed
the
alexwoo/fix_sso_token_refresh
branch
from
5f9d648 to
a49acc3
Compare
alextwoods
added
the
no-api-surface-area-change
| } catch (ExpiredTokenException e) { | ||
| throw e; | ||
| } catch (Exception e) { | ||
| throw ExpiredTokenException.builder() |
There was a problem hiding this comment.
Can we add a Java doc explaining why any Exception is treated as ExpiredTokenException?
| @@ -31,6 +31,10 @@ | |||
| @SdkPublicApi | |||
| public final class ExpiredTokenException extends SdkClientException { | |||
|
|
|||
| public static final String DEFAULT_MESSAGE = | |||
There was a problem hiding this comment.
can we make it private or package-private ?
|
|
||
| Validate.notNull(expiration, |
There was a problem hiding this comment.
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.
| credentialsProvider.resolveCredentials(); | ||
|
|
||
| // Second call: token provider throws InvalidTokenException, but CachedSupplier with | ||
| // StaleValueBehavior.ALLOW returns stale cached credentials (static stability) |
There was a problem hiding this comment.
Just trying to make sure I understand this correctly. As far as I can tell SsoCredentialsProvider doesn't set staleValueBehavior anywhere, so wouldn't this be STRICT by default rather than ALLOW? Curious where ALLOW kicks in?
2 participants
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 usedMotivation and Context
Fixes #3777
Currently we are incorrectly caching the token at construction time instead of refreshing it each time we perform a credentials refresh.
Modifications
Testing
New and existing test
Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License