search

Azure / azure-sdk-for-cpp

This repository is for active development of the Azure SDK for C++. For consumers of the SDK we recommend visiting our versioned developer docs at https://azure.github.io/azure-sdk-for-cpp.
MIT License
177 stars 126 forks source link

Fix overflow issue in token cache. #6190

Closed ahsonkhan closed 3 weeks ago

ahsonkhan commented 3 weeks ago

https://github.com/Azure/azure-sdk-for-cpp/pull/6151 exposed an existing potential overflow issue in the Identity token cache logic.

If minimumExpiration is set to a really large number, DateTime(now) + minimumExpiration could overflow, resulting in the comparison within IsFresh() returning true when it should be false.

Fixes most of the failing KeyVault challenged based authentication policy tests:

The following tests FAILED:
    142 - azure-security-keyvault-secrets-unittest.KeyVaultChallengeBasedAuthenticationPolicy.AnotherScopeAsScope (Failed)
    143 - azure-security-keyvault-secrets-unittest.KeyVaultChallengeBasedAuthenticationPolicy.AnotherScopeAsResource (Failed)
    144 - azure-security-keyvault-secrets-unittest.KeyVaultChallengeBasedAuthenticationPolicy.AnotherTenantAsterisk (Failed)
    145 - azure-security-keyvault-secrets-unittest.KeyVaultChallengeBasedAuthenticationPolicy.AnotherTenantAndScopeWithAltNames (Failed)
    146 - azure-security-keyvault-secrets-unittest.KeyVaultChallengeBasedAuthenticationPolicy.AnotherTenantExplicit (Failed)
    157 - azure-security-keyvault-secrets-unittest.KeyVaultChallengeBasedAuthenticationPolicy.AuthorizationLongerPath (Failed)

Except for azure-security-keyvault-secrets-unittest.KeyVaultChallengeBasedAuthenticationPolicy.MultipleTimes which needs to be updated to reflect the new expected cache invalidation behavior (fixed in https://github.com/Azure/azure-sdk-for-cpp/pull/6191)

FYI @RickWinter

azure-sdk commented 3 weeks ago

API change check

API changes are not detected in this pull request.

ahsonkhan commented 3 weeks ago

FYI @chlowell, @christothes