diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index be3b2d465a..c5b9b56ff0 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -13,6 +13,8 @@ ### Other Changes +- [[#4756]] (https://github.com/Azure/azure-sdk-for-cpp/issues/4756) `BearerTokenAuthenticationPolicy` now uses shared mutex lock for read operations. + ## 1.11.0-beta.2 (2023-11-02) ### Features Added diff --git a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp index c0dbe7d9ea..186c17308c 100644 --- a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -549,7 +550,7 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { Credentials::TokenRequestContext m_tokenRequestContext; mutable Credentials::AccessToken m_accessToken; - mutable std::mutex m_accessTokenMutex; + mutable std::shared_timed_mutex m_accessTokenMutex; mutable Credentials::TokenRequestContext m_accessTokenContext; public: @@ -581,6 +582,9 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { BearerTokenAuthenticationPolicy(BearerTokenAuthenticationPolicy const& other) : BearerTokenAuthenticationPolicy(other.m_credential, other.m_tokenRequestContext) { + std::shared_lock readLock(other.m_accessTokenMutex); + m_accessToken = other.m_accessToken; + m_accessTokenContext = other.m_accessTokenContext; } void operator=(BearerTokenAuthenticationPolicy const&) = delete; diff --git a/sdk/core/azure-core/src/http/bearer_token_authentication_policy.cpp b/sdk/core/azure-core/src/http/bearer_token_authentication_policy.cpp index 66cd6b00e0..bfce41f86b 100644 --- a/sdk/core/azure-core/src/http/bearer_token_authentication_policy.cpp +++ b/sdk/core/azure-core/src/http/bearer_token_authentication_policy.cpp @@ -61,21 +61,49 @@ bool BearerTokenAuthenticationPolicy::AuthorizeRequestOnChallenge( return false; } +namespace { +bool TokenNeedsRefresh( + Azure::Core::Credentials::AccessToken const& cachedToken, + Azure::Core::Credentials::TokenRequestContext const& cachedTokenRequestContext, + Azure::DateTime const& currentTime, + Azure::Core::Credentials::TokenRequestContext const& newTokenRequestContext) +{ + return newTokenRequestContext.TenantId != cachedTokenRequestContext.TenantId + || newTokenRequestContext.Scopes != cachedTokenRequestContext.Scopes + || currentTime > (cachedToken.ExpiresOn - newTokenRequestContext.MinimumExpiration); +} + +void ApplyBearerToken( + Azure::Core::Http::Request& request, + Azure::Core::Credentials::AccessToken const& token) +{ + request.SetHeader("authorization", "Bearer " + token.Token); +} +} // namespace + void BearerTokenAuthenticationPolicy::AuthenticateAndAuthorizeRequest( Request& request, Credentials::TokenRequestContext const& tokenRequestContext, Context const& context) const { - std::lock_guard lock(m_accessTokenMutex); + DateTime const currentTime = std::chrono::system_clock::now(); + + { + std::shared_lock readLock(m_accessTokenMutex); + if (!TokenNeedsRefresh(m_accessToken, m_accessTokenContext, currentTime, tokenRequestContext)) + { + ApplyBearerToken(request, m_accessToken); + return; + } + } - if (tokenRequestContext.TenantId != m_accessTokenContext.TenantId - || tokenRequestContext.Scopes != m_accessTokenContext.Scopes - || std::chrono::system_clock::now() - > (m_accessToken.ExpiresOn - tokenRequestContext.MinimumExpiration)) + std::unique_lock writeLock(m_accessTokenMutex); + // Check if token needs refresh for the second time in case another thread has just updated it. + if (TokenNeedsRefresh(m_accessToken, m_accessTokenContext, currentTime, tokenRequestContext)) { m_accessToken = m_credential->GetToken(tokenRequestContext, context); m_accessTokenContext = tokenRequestContext; } - request.SetHeader("authorization", "Bearer " + m_accessToken.Token); + ApplyBearerToken(request, m_accessToken); }