STS profile credentail doesn't get updated 5 minutes before expiry

[phoebusm]

Describe the bug

aws-sdk-cpp/src/aws-cpp-sdk-identity-management/source/auth/STSProfileCredentialsProvider.cpp

Line 48 in 1e5a155

if (!IsTimeToRefresh(static_cast<long>(m_reloadFrequency.count())) || !m_credentials.IsExpiredOrEmpty())

The condition of credential refresh doesn't get refreshed 5 minutes before expiry, as specified in the constructor:

aws-sdk-cpp/src/aws-cpp-sdk-identity-management/source/auth/STSProfileCredentialsProvider.cpp

Line 33 in 1e5a155

m_reloadFrequency(std::chrono::minutes(std::max(int64_t(5), static_cast<int64_t>(duration.count()))) - std::chrono::minutes(5)),

as the refresh can only be made if m_credentials.IsExpiredOrEmpty() == true.

As a result, the token could pass the expiry check here but rejected by the endpoint milisecond later.

Regression Issue

• Select this option if this issue appears to be a regression.

Expected Behavior

Reference: class STSCredentialsProvider :

aws-sdk-cpp/src/aws-cpp-sdk-core/source/auth/STSCredentialsProvider.cpp

Line 158 in 1e5a155

if (!m_credentials.IsEmpty() && !ExpiresSoon())

Have a grace period. Refresh token minutes before expiry

Current Behavior

Refresh only when the credential is found to be expired during checking

Reproduction Steps

1. Enable logging
2. Create a connection to aws endpoint with STSProfileCredentialsProvider.cpp
3. Call some s3 api, e.g. ListObjectV2, per second
4. By reading the log, it can be determined when does the refresh happen

Possible Solution

|| to && in the conditions

Additional Information/Context

No response

AWS CPP SDK version used

1.11.474

Compiler and Version used

gcc version 11.4.0

Operating System and version

Ubuntu 22.04

[sbiscigl]

hey @phoebusm thanks for opening this issue, and yeah looks like this fix was pushed but this bit of logic was not updated.

Expected Behavior

Reference: class STSCredentialsProvider :

aws-sdk-cpp/src/aws-cpp-sdk-core/source/auth/STSCredentialsProvider.cpp

Line 158 in 1e5a155

if (!m_credentials.IsEmpty() && !ExpiresSoon())

Have a grace period. Refresh token minutes before expiry

i very much agree with this, this logic should be the same

Possible Solution
|| to && in the conditions

I dont think this solves it because it wont refresh the token within a threshold, i actually added a test, and it will fail flakly with this fix. I think the correct fix is to duplicate the logic from STSCredentialsProvider.cpp, im creating a PR where we pull up the expirese soon method to the AWS credentials object then we call it from both locations. Will post PR shortly.

[phoebusm]

Hi @sbiscigl I wonder why my patch won't refresh the token within the threshold. By design it should refresh the token if the token is going to expire in 5 minutes., as specifed in the consturctor:

aws-sdk-cpp/src/aws-cpp-sdk-identity-management/source/auth/STSProfileCredentialsProvider.cpp

Line 33 in 1e5a155

m_reloadFrequency(std::chrono::minutes(std::max(int64_t(5), static_cast<int64_t>(duration.count()))) - std::chrono::minutes(5)),

If it's the case, where have I missed?

I have also check the test case in your PR. If I have read it correctly (I am not familar with the SDK test model), there is inconsistency of token life.

constexpr auto roleSessionDuration = std::chrono::seconds(5); vs STSProfileCredentialsProvider credsProvider("default", std::chrono::minutes(60), [&](const AWSCredentials& creds)

5 seconds vs 60 minutes

[sbiscigl]

If it's the case, where have I missed?

m_reloadFrequency != tokenExpired, they are tracked differently. m_reloadFrequency checks when the token was last refreshed in the credentials provider, it never actually looks at the actual expiry of the credential. so in this case where tokenExpiry is default (60 minutes), and the assumed role cred is shorter than that (5 seconds). We need to check the actual expiry of the token, not the reload frequency

[phoebusm]

m_reloadFrequency != tokenExpired, they are tracked differently. m_reloadFrequency checks when the token was last refreshed in the credentials provider, it never actually looks at the actual expiry of the credential. so in this case where tokenExpiry is default (60 minutes), and the assumed role cred is shorter than that (5 seconds). We need to check the actual expiry of the token, not the reload frequency

I see. Are you suggesting sometimes aws s3 endpoint gives shorter token life than requested?

aws-sdk-cpp/src/aws-cpp-sdk-identity-management/source/auth/STSProfileCredentialsProvider.cpp

Line 319 in 1e5a155

.WithDurationSeconds(static_cast<int>(std::chrono::seconds(m_duration).count()));

[sbiscigl]

ah good callout

DurationSeconds
The duration, in seconds, of the role session. The value specified can range from 900 seconds (15 minutes) up to the maximum session duration set for the role. The maximum session duration setting can have a value from 1 hour to 12 hours. If you specify a value higher than this setting or the administrator setting (whichever is lower), the operation fails. For example, if you specify a session duration of 12 hours, but your administrator set the maximum session duration to 6 hours, your operation fails.

I would still prefer the check to be on the actual expiry of the token returned as opposed to the member variable. As it is the actual returned value for expiry instead of a side-car variable.

[sbiscigl]

merged the PR, should be tagged as part of todays release, really appreciate the thorough explanation of the issue and the potential fix, give a shout if you see anything else wrong!

[github-actions]

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

[phoebusm]

@sbiscigl Thanks for fixing it!
I have another PR that should be (hopefull) a quick one as well: #3304