Need help understanding CredentialsRefreshState.GetTimeToLive

[BFCatalin]

In file aws-sdk-net/sdk/src/Core/Amazon.Runtime/Credentials/RefreshingAWSCredentials.cs the method CredentialsRefreshState.GetTimeToLive is supposed to return time to live for the token, but the value will always be greater than the remaining time if PreemptExpiryTime is set to a value greater than TimeSpan.Zero. From my understanding of time to live, the value should never be greater than the Expiration - Now; by this logic the correct formula should be Expiration - Now - PreemptExpiryTime.
What am I missing here?

[dscpinheiro]

Are you running into credentials being expired when they shouldn't be? Or were you just looking at the code trying to understand its logic?

If it's the former please create an issue so that we can take a look.

[BFCatalin]

Both actually. We ran into an issue where when the daylight savings happened the token expired when it should not have and it was not refreshed. That lead me to look at the code and saw how PreempExpiryTime is handled and decided to start the discussion before posting an issue so not to waste time for someone to look into it. I will do some tests next week and maybe come back and post an issue.

[azbw2]

@dscpinheiro

I was looking at this and I think, while confusing, the value returned from GetTimeToLive is correct. In UpdateToGeneratedCredentials the PreemptExpiryTime is subtracted from the Expiration:

aws-sdk-net/sdk/src/Core/Amazon.Runtime/Credentials/RefreshingAWSCredentials.cs

Lines 256 to 258 in 617e843

	// Offset the Expiration by PreemptExpiryTime. This produces the expiration window
	// where the credentials should be updated before they actually expire.
	state.Expiration -= PreemptExpiryTime;

So, GetTimeToLive is adding it back which gives the actual remaining time during which the underlying credential is valid:

aws-sdk-net/sdk/src/Core/Amazon.Runtime/Credentials/RefreshingAWSCredentials.cs

Lines 72 to 78 in 617e843

	internal TimeSpan GetTimeToLive(TimeSpan preemptExpiryTime)
	{
	var now = AWSSDKUtils.CorrectedUtcNow;
	var exp = Expiration.ToUniversalTime();
	return exp - now + preemptExpiryTime;
	}

The implementation for IsExpiredWithin appears to be incorrect since it subtracts this value from the Expiration, so the return value is checking if the current time is past the expiration with the preempt expiry time as well as another preempt expiry time removed:

aws-sdk-net/sdk/src/Core/Amazon.Runtime/Credentials/RefreshingAWSCredentials.cs

Lines 65 to 70 in 617e843

	internal bool IsExpiredWithin(TimeSpan preemptExpiryTime)
	{
	var now = AWSSDKUtils.CorrectedUtcNow;
	var exp = Expiration.ToUniversalTime();
	return now > exp - preemptExpiryTime;
	}

Most usages pass zero as the additional preempt expiry and are thus not impacted, but FederatedAWSCredentials passes the actual preempt expiry time of 15 minutes, though it seems this class handles its own expiration per the code comments:

aws-sdk-net/sdk/src/Core/Amazon.Runtime/Credentials/FederatedAWSCredentials.cs

Lines 116 to 130 in 617e843

	// Since cached role session credentials can be obtained, stored credentials exist
	// and they were not expired. However, since the stored credentials are actual
	// expiration time and not preempt expiration time we must preempt the expiration
	// to check that they will not expire before this call completes.
	var cachedState = new CredentialsRefreshState(currentSession, currentSession.Expires);
	// Use the cached credentials as long as they are not within the preempt expiry window
	// or have since actually expired since the prior call to TryGetRoleSession.
	//Verify the actual expiration is not within the preempt expiration time.
	if (!cachedState.IsExpiredWithin(PreemptExpiryTime))
	{
	// The credentials have plenty of time left before they expire so they can be used. After
	// return the expiration time will be preempted for future checks using ShouldUpdate.
	return cachedState;
	}

In RefreshingAWSCredentials, the logic for determining if the credential should be refreshed in the background makes sense since ttl > TimeSpan.Zero indicates that the credential is not expired based on the real underlying expiration while ttl < PreemptExpiryTime indicates that the credential is within the preempt expiry window (imagine subtracting PreemptExpiryTime from both sides and you get an expiration check based on the expiration with this preempt expiry time baked in):

aws-sdk-net/sdk/src/Core/Amazon.Runtime/Credentials/RefreshingAWSCredentials.cs

Lines 135 to 145 in 617e843

	if (ttl > TimeSpan.Zero)
	{
	if (ttl < PreemptExpiryTime)
	{
	// background refresh (fire & forget)
	if (_updateGeneratedCredentialsSemaphore.Wait(0))
	{
	_ = System.Threading.Tasks.Task.Run(GenerateCredentialsAndUpdateState);
	}
	}
	}

Please let me know if you see any issues with my train of thought.