Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In-memory Identity token cache #4024

Merged
merged 37 commits into from
Oct 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
bcf6d69
Run-time token cache
antkmsft Oct 13, 2022
c37f52e
Doxygen tag fix
antkmsft Oct 13, 2022
7c35923
Fix linux and mac warnings (initialization order)
antkmsft Oct 13, 2022
2b322e5
MSVC warning
antkmsft Oct 13, 2022
5e2f626
Expose token cache internals for testing
antkmsft Oct 14, 2022
614c673
OnBeforeWriteLock
antkmsft Oct 14, 2022
a5fe314
mac fix
antkmsft Oct 14, 2022
89b441b
First unit test
antkmsft Oct 14, 2022
0bf6833
mac constexpr fix
antkmsft Oct 14, 2022
a362e90
More tests
antkmsft Oct 16, 2022
bf7d87e
Undo perf test change
antkmsft Oct 16, 2022
f752ecc
Update changelog
antkmsft Oct 16, 2022
7237403
Update sdk/identity/azure-identity/src/private/token_cache_internals.hpp
antkmsft Oct 17, 2022
a2cc325
clang format
antkmsft Oct 17, 2022
b51365b
Refreshing one token does not lock the entire container
antkmsft Oct 18, 2022
5a08e7f
Nit PR feedback
antkmsft Oct 18, 2022
4f1db6e
valule
antkmsft Oct 18, 2022
666124c
mac fix: remove unused variable
antkmsft Oct 18, 2022
733fd4a
Add cache cleanup every ==2^n insertions, where n>=5
antkmsft Oct 18, 2022
03dd3ca
Improve tests for element locks during cleanup
antkmsft Oct 18, 2022
150cb1c
Spelling
antkmsft Oct 18, 2022
489c8b6
Avoid clang-format inconsistencies between versions
antkmsft Oct 18, 2022
eff8672
&
antkmsft Oct 19, 2022
0a188a5
struct final
antkmsft Oct 21, 2022
7439fb1
PR feedback
antkmsft Oct 24, 2022
2ba9679
Expand changelog
antkmsft Oct 24, 2022
9359a14
2 minutes is enough
antkmsft Oct 24, 2022
9b78eeb
Mac build fix
antkmsft Oct 24, 2022
197f688
Another attempt at mac fix
antkmsft Oct 24, 2022
086fcf1
Brackets just in case
antkmsft Oct 24, 2022
d1b6c75
Mac fix?
antkmsft Oct 25, 2022
34fb2e9
Max fix??
antkmsft Oct 25, 2022
504e929
Disable code analysis warning
antkmsft Oct 25, 2022
cfc79f2
Undo
antkmsft Oct 25, 2022
c275101
Disable code analysis warning via command line
antkmsft Oct 25, 2022
13ba0a5
Undo
antkmsft Oct 25, 2022
6391798
Merge remote-tracking branch 'upstream/main' into token-cache
antkmsft Oct 25, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions sdk/core/azure-core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

### Other Changes

- Added support for Identity token caching, and for configuring token refresh offset in `BearerTokenAuthenticationPolicy`.

## 1.8.0-beta.1 (2022-10-06)

### Features Added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ namespace Azure { namespace Core { namespace Credentials {
*
*/
std::vector<std::string> Scopes;

/**
* @brief Minimum token expiration suggestion.
*
*/
DateTime::duration MinimumExpiration = std::chrono::minutes(2);
};

/**
Expand All @@ -61,6 +67,8 @@ namespace Azure { namespace Core { namespace Credentials {
* @param tokenRequestContext A context to get the token in.
* @param context A context to control the request lifetime.
*
* @return Authentication token.
*
* @throw Azure::Core::Credentials::AuthenticationException Authentication error occurred.
*/
virtual AccessToken GetToken(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ std::unique_ptr<RawResponse> BearerTokenAuthenticationPolicy::Send(
{
std::lock_guard<std::mutex> lock(m_accessTokenMutex);

// Refresh the token in 2 or less minutes before the actual expiration.
if (std::chrono::system_clock::now() > (m_accessToken.ExpiresOn - std::chrono::minutes(2)))
if (std::chrono::system_clock::now()
> (m_accessToken.ExpiresOn - m_tokenRequestContext.MinimumExpiration))
{
m_accessToken = m_credential->GetToken(m_tokenRequestContext, context);
}
Expand Down
2 changes: 2 additions & 0 deletions sdk/identity/azure-identity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### Features Added

- Added token caching.

### Breaking Changes

### Bugs Fixed
Expand Down
6 changes: 6 additions & 0 deletions sdk/identity/azure-identity/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,16 @@ set(
AZURE_IDENTITY_SOURCE
src/private/managed_identity_source.hpp
src/private/package_version.hpp
src/private/token_cache.hpp
src/private/token_cache_internals.hpp
src/private/token_credential_impl.hpp
src/chained_token_credential.cpp
src/client_certificate_credential.cpp
src/client_secret_credential.cpp
src/environment_credential.cpp
src/managed_identity_credential.cpp
src/managed_identity_source.cpp
src/token_cache.cpp
src/token_credential_impl.cpp
)

Expand Down Expand Up @@ -106,6 +109,9 @@ az_rtti_setup(
)

if(BUILD_TESTING)
# define a symbol that enables some test hooks in code
add_compile_definitions(TESTING_BUILD)

# tests
if (NOT AZ_ALL_LIBRARIES OR FETCH_SOURCE_DEPS)
include(AddGoogleTest)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,18 @@ namespace Azure { namespace Identity {
std::unique_ptr<_detail::TokenCredentialImpl> m_tokenCredentialImpl;
Core::Url m_requestUrl;
std::string m_requestBody;

std::string m_tenantId;
std::string m_clientId;
std::string m_authorityHost;

bool m_isAdfs;

ClientSecretCredential(
std::string const& tenantId,
std::string const& clientId,
std::string tenantId,
std::string clientId,
std::string const& clientSecret,
std::string const& authorityHost,
std::string authorityHost,
Core::Credentials::TokenCredentialOptions const& options);

public:
Expand All @@ -70,8 +75,8 @@ namespace Azure { namespace Identity {
* @param options Options for token retrieval.
*/
explicit ClientSecretCredential(
std::string const& tenantId,
std::string const& clientId,
std::string tenantId,
std::string clientId,
std::string const& clientSecret,
ClientSecretCredentialOptions const& options);

Expand All @@ -86,7 +91,7 @@ namespace Azure { namespace Identity {
explicit ClientSecretCredential(
std::string tenantId,
std::string clientId,
std::string clientSecret,
std::string const& clientSecret,
Core::Credentials::TokenCredentialOptions const& options
= Core::Credentials::TokenCredentialOptions());

Expand All @@ -102,6 +107,8 @@ namespace Azure { namespace Identity {
* @param tokenRequestContext A context to get the token in.
* @param context A context to control the request lifetime.
*
* @return Authentication token.
*
* @throw Azure::Core::Credentials::AuthenticationException Authentication error occurred.
*/
Core::Credentials::AccessToken GetToken(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ namespace Azure { namespace Identity {
* @param tokenRequestContext A context to get the token in.
* @param context A context to control the request lifetime.
*
* @return Authentication token.
*
* @throw Azure::Core::Credentials::AuthenticationException Authentication error occurred.
*/
Core::Credentials::AccessToken GetToken(
Expand Down
89 changes: 58 additions & 31 deletions sdk/identity/azure-identity/src/client_secret_credential.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,45 @@

#include "azure/identity/client_secret_credential.hpp"

#include "private/token_cache.hpp"
#include "private/token_credential_impl.hpp"

#include <sstream>
#include <utility>

using namespace Azure::Identity;

std::string const Azure::Identity::_detail::g_aadGlobalAuthority
= "https://login.microsoftonline.com/";

ClientSecretCredential::ClientSecretCredential(
std::string const& tenantId,
std::string const& clientId,
std::string tenantId,
std::string clientId,
std::string const& clientSecret,
std::string const& authorityHost,
std::string authorityHost,
Azure::Core::Credentials::TokenCredentialOptions const& options)
: m_tokenCredentialImpl(std::make_unique<_detail::TokenCredentialImpl>(options)),
m_isAdfs(tenantId == "adfs")
m_tenantId(std::move(tenantId)), m_clientId(std::move(clientId)),
m_authorityHost(std::move(authorityHost))
{
using Azure::Core::Url;
m_requestUrl = Url(authorityHost);
m_requestUrl.AppendPath(tenantId);

m_isAdfs = (m_tenantId == "adfs");

m_requestUrl = Url(m_authorityHost);
m_requestUrl.AppendPath(m_tenantId);
m_requestUrl.AppendPath(m_isAdfs ? "oauth2/token" : "oauth2/v2.0/token");

std::ostringstream body;
body << "grant_type=client_credentials&client_id=" << Url::Encode(clientId)
body << "grant_type=client_credentials&client_id=" << Url::Encode(m_clientId)
<< "&client_secret=" << Url::Encode(clientSecret);

m_requestBody = body.str();
}

ClientSecretCredential::ClientSecretCredential(
std::string const& tenantId,
std::string const& clientId,
std::string tenantId,
std::string clientId,
std::string const& clientSecret,
ClientSecretCredentialOptions const& options)
: ClientSecretCredential(tenantId, clientId, clientSecret, options.AuthorityHost, options)
Expand All @@ -45,7 +51,7 @@ ClientSecretCredential::ClientSecretCredential(
ClientSecretCredential::ClientSecretCredential(
std::string tenantId,
std::string clientId,
std::string clientSecret,
std::string const& clientSecret,
LarryOsterman marked this conversation as resolved.
Show resolved Hide resolved
Core::Credentials::TokenCredentialOptions const& options)
: ClientSecretCredential(
tenantId,
Expand All @@ -62,28 +68,49 @@ Azure::Core::Credentials::AccessToken ClientSecretCredential::GetToken(
Azure::Core::Credentials::TokenRequestContext const& tokenRequestContext,
Azure::Core::Context const& context) const
{
return m_tokenCredentialImpl->GetToken(context, [&]() {
using _detail::TokenCredentialImpl;
using Azure::Core::Http::HttpMethod;

std::ostringstream body;
body << m_requestBody;
{
auto const& scopes = tokenRequestContext.Scopes;
if (!scopes.empty())
{
body << "&scope=" << TokenCredentialImpl::FormatScopes(scopes, m_isAdfs);
}
}

auto request = std::make_unique<TokenCredentialImpl::TokenRequest>(
HttpMethod::Post, m_requestUrl, body.str());
using _detail::TokenCache;
using _detail::TokenCredentialImpl;

if (m_isAdfs)
std::string scopesStr;
{
auto const& scopes = tokenRequestContext.Scopes;
if (!scopes.empty())
{
request->HttpRequest.SetHeader("Host", m_requestUrl.GetHost());
scopesStr = TokenCredentialImpl::FormatScopes(scopes, m_isAdfs);
}

return request;
});
}

// TokenCache::GetToken() and m_tokenCredentialImpl->GetToken() can only use the lambda argument
// when they are being executed. They are not supposed to keep a reference to lambda argument to
// call it later. Therefore, any capture made here will outlive the possible time frame when the
// lambda might get called.
return TokenCache::GetToken(
m_tenantId,
m_clientId,
m_authorityHost,
scopesStr,
tokenRequestContext.MinimumExpiration,
antkmsft marked this conversation as resolved.
Show resolved Hide resolved
[&]() {
return m_tokenCredentialImpl->GetToken(context, [&]() {
using Azure::Core::Http::HttpMethod;

std::ostringstream body;
body << m_requestBody;

if (!scopesStr.empty())
{
body << "&scope=" << scopesStr;
}

auto request = std::make_unique<TokenCredentialImpl::TokenRequest>(
HttpMethod::Post, m_requestUrl, body.str());

if (m_isAdfs)
{
request->HttpRequest.SetHeader("Host", m_requestUrl.GetHost());
}

return request;
});
});
}
Loading