-
Notifications
You must be signed in to change notification settings - Fork 33
add credential caching #154
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,4 +60,4 @@ export AZURE_PROVIDER=cloud | |
| shift | ||
| } | ||
|
|
||
| exec "$@" | ||
| exec env "$@" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,12 +3,14 @@ | |
| #include "duckdb/catalog/catalog_transaction.hpp" | ||
| #include "duckdb/common/exception.hpp" | ||
| #include "duckdb/common/file_opener.hpp" | ||
| #include "duckdb/common/helper.hpp" | ||
| #include "duckdb/common/shared_ptr.hpp" | ||
| #include "duckdb/common/string_util.hpp" | ||
| #include "duckdb/main/client_context.hpp" | ||
| #include "duckdb/main/secret/secret.hpp" | ||
| #include "duckdb/main/secret/secret_manager.hpp" | ||
| #include "http_state_policy.hpp" | ||
| #include "zstd/common/xxhash.hpp" | ||
|
|
||
| #include <azure/core/credentials/token_credential_options.hpp> | ||
| #include <azure/core/http/curl_transport.hpp> | ||
|
|
@@ -225,6 +227,51 @@ CreateAccessTokenCredential(const KeyValueSecret &secret) { | |
| return std::make_shared<AccessTokenCredential>(access_token); | ||
| } | ||
|
|
||
| // Credential cache entry: persists across QueryEnd() calls since we intentionally | ||
| // don't override QueryEnd(). The Azure SDK's internal TokenCache handles per-token | ||
| // expiry and refresh — we just need to keep the credential object alive. | ||
| struct AzureCredentialState : public ClientContextState { | ||
| AzureCredentialState(std::shared_ptr<Azure::Core::Credentials::TokenCredential> cred, string fp) | ||
| : credential(std::move(cred)), fingerprint(std::move(fp)) { | ||
| } | ||
| std::shared_ptr<Azure::Core::Credentials::TokenCredential> credential; | ||
| std::string fingerprint; | ||
| }; | ||
|
|
||
| static std::string FingerprintSecret(const KeyValueSecret &secret) { | ||
| std::string content; | ||
| // NOTE: this key ordering is safe because secret_map is an ordered tree | ||
| for (const auto &kv : secret.secret_map) { | ||
| content += kv.first + "=" + kv.second.ToString() + ";"; | ||
| } | ||
| auto hash = duckdb_zstd::XXH64(content.c_str(), content.size(), 0); | ||
| return std::to_string(hash); | ||
| } | ||
|
|
||
| static std::shared_ptr<Azure::Core::Credentials::TokenCredential> | ||
| GetCachedCredential(optional_ptr<FileOpener> opener, const std::string &key, const std::string &fp) { | ||
| auto ctx = FileOpener::TryGetClientContext(opener); | ||
| if (!ctx) { | ||
| return nullptr; | ||
| } | ||
| auto entry = ctx->registered_state->Get<AzureCredentialState>(key); | ||
| if (!entry || entry->fingerprint != fp) { | ||
| return nullptr; | ||
| } | ||
| return entry->credential; | ||
| } | ||
|
|
||
| static void CacheTokenCredential(optional_ptr<FileOpener> opener, const std::string &key, | ||
| std::shared_ptr<Azure::Core::Credentials::TokenCredential> cred, | ||
| const std::string &fp) { | ||
| auto ctx = FileOpener::TryGetClientContext(opener); | ||
| if (!ctx) { | ||
| return; | ||
| } | ||
| ctx->registered_state->Remove(key); | ||
| ctx->registered_state->Insert(key, make_shared_ptr<AzureCredentialState>(std::move(cred), fp)); | ||
| } | ||
|
|
||
| static std::shared_ptr<Azure::Core::Http::HttpTransport> | ||
| CreateCurlTransport(const std::string &proxy, const std::string &proxy_username, const std::string &proxy_password) { | ||
| Azure::Core::Http::CurlTransportOptions curl_transport_options; | ||
|
|
@@ -384,32 +431,48 @@ GetDfsStorageAccountClientFromConfigProvider(optional_ptr<FileOpener> opener, co | |
| return Azure::Storage::Files::DataLake::DataLakeServiceClient(account_url, dfs_options); | ||
| } | ||
|
|
||
| // Retrieves a cached credential by fingerprint, or creates and caches a new one via create_fn. | ||
| // If secret.cache_token_credential==false, then do not cache. (Useful e.g. when using the az cli, | ||
| // to where a logout is expected to have effect on next query.) | ||
| template <class CreateFn> | ||
| static std::shared_ptr<Azure::Core::Credentials::TokenCredential> | ||
| GetOrCreateCredential(optional_ptr<FileOpener> opener, const KeyValueSecret &secret, CreateFn create_fn) { | ||
| auto fp = FingerprintSecret(secret); | ||
| auto cache_key = std::string("azure_cred:secret:") + secret.GetName(); | ||
| auto ctc_val = secret.TryGetValue("cache_token_credential"); | ||
| auto use_cache = ctc_val.IsNull() || ctc_val.GetValue<bool>(); | ||
| auto cred = use_cache ? GetCachedCredential(opener, cache_key, fp) : nullptr; | ||
| if (!cred) { | ||
| cred = create_fn(); | ||
| if (use_cache) { | ||
| CacheTokenCredential(opener, cache_key, cred, fp); | ||
| } | ||
| } | ||
| return cred; | ||
| } | ||
|
|
||
| static Azure::Storage::Blobs::BlobServiceClient | ||
| GetBlobStorageAccountClientFromCredentialChainProvider(optional_ptr<FileOpener> opener, const KeyValueSecret &secret, | ||
| const AzureParsedUrl &azure_parsed_url) { | ||
| auto transport_options = GetTransportOptions(opener, secret); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the transport options fetch both from a secret and settings. Since you only use the fingerprint based on the secret, this would now result in stale caching potentially
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ay ay ay, good eye! I'll fix this. |
||
| // Create credential chain | ||
| auto credential = CreateChainedTokenCredential(secret, transport_options); | ||
|
|
||
| // Connect to storage account | ||
| auto cred = GetOrCreateCredential(opener, secret, | ||
| [&]() { return CreateChainedTokenCredential(secret, transport_options); }); | ||
| auto account_url = | ||
| azure_parsed_url.is_fully_qualified ? AccountUrl(azure_parsed_url) : AccountUrl(secret, DEFAULT_BLOB_ENDPOINT); | ||
| auto blob_options = ToBlobClientOptions(transport_options, GetHttpState(opener)); | ||
| return Azure::Storage::Blobs::BlobServiceClient(account_url, std::move(credential), blob_options); | ||
| return Azure::Storage::Blobs::BlobServiceClient(account_url, std::move(cred), blob_options); | ||
| } | ||
|
|
||
| static Azure::Storage::Files::DataLake::DataLakeServiceClient | ||
| GetDfsStorageAccountClientFromCredentialChainProvider(optional_ptr<FileOpener> opener, const KeyValueSecret &secret, | ||
| const AzureParsedUrl &azure_parsed_url) { | ||
| auto transport_options = GetTransportOptions(opener, secret); | ||
| // Create credential chain | ||
| auto credential = CreateChainedTokenCredential(secret, transport_options); | ||
|
|
||
| // Connect to storage account | ||
| auto cred = GetOrCreateCredential(opener, secret, | ||
| [&]() { return CreateChainedTokenCredential(secret, transport_options); }); | ||
| auto account_url = | ||
| azure_parsed_url.is_fully_qualified ? AccountUrl(azure_parsed_url) : AccountUrl(secret, DEFAULT_DFS_ENDPOINT); | ||
| auto dfs_options = ToDfsClientOptions(transport_options, GetHttpState(opener)); | ||
| return Azure::Storage::Files::DataLake::DataLakeServiceClient(account_url, std::move(credential), dfs_options); | ||
| return Azure::Storage::Files::DataLake::DataLakeServiceClient(account_url, std::move(cred), dfs_options); | ||
| } | ||
|
|
||
| static std::shared_ptr<Azure::Identity::ManagedIdentityCredential> | ||
|
|
@@ -448,78 +511,71 @@ GetManagedIdentityCredential(optional_ptr<FileOpener> opener, const KeyValueSecr | |
| static Azure::Storage::Blobs::BlobServiceClient | ||
| GetBlobStorageAccountClientFromManagedIdentityProvider(optional_ptr<FileOpener> opener, const KeyValueSecret &secret, | ||
| const AzureParsedUrl &azure_parsed_url) { | ||
| // Connect to (blob) storage account | ||
| auto transport_options = GetTransportOptions(opener, secret); | ||
| auto mi_cred = GetManagedIdentityCredential(opener, secret, transport_options); | ||
| auto cred = GetOrCreateCredential( | ||
| opener, secret, [&]() { return GetManagedIdentityCredential(opener, secret, transport_options); }); | ||
| auto account_url = | ||
| azure_parsed_url.is_fully_qualified ? AccountUrl(azure_parsed_url) : AccountUrl(secret, DEFAULT_BLOB_ENDPOINT); | ||
| auto blob_options = ToBlobClientOptions(transport_options, GetHttpState(opener)); | ||
| return Azure::Storage::Blobs::BlobServiceClient(account_url, mi_cred, blob_options); | ||
| return Azure::Storage::Blobs::BlobServiceClient(account_url, cred, blob_options); | ||
| } | ||
|
|
||
| static Azure::Storage::Files::DataLake::DataLakeServiceClient | ||
| GetDfsStorageAccountClientFromManagedIdentityProvider(optional_ptr<FileOpener> opener, const KeyValueSecret &secret, | ||
| const AzureParsedUrl &azure_parsed_url) { | ||
| auto transport_options = GetTransportOptions(opener, secret); | ||
| auto mi_cred = GetManagedIdentityCredential(opener, secret, transport_options); | ||
|
|
||
| // Connect to ADLS storage account | ||
| auto cred = GetOrCreateCredential( | ||
| opener, secret, [&]() { return GetManagedIdentityCredential(opener, secret, transport_options); }); | ||
| auto account_url = | ||
| azure_parsed_url.is_fully_qualified ? AccountUrl(azure_parsed_url) : AccountUrl(secret, DEFAULT_DFS_ENDPOINT); | ||
| auto dfs_options = ToDfsClientOptions(transport_options, GetHttpState(opener)); | ||
| return Azure::Storage::Files::DataLake::DataLakeServiceClient(account_url, mi_cred, dfs_options); | ||
| return Azure::Storage::Files::DataLake::DataLakeServiceClient(account_url, cred, dfs_options); | ||
| } | ||
|
|
||
| static Azure::Storage::Blobs::BlobServiceClient | ||
| GetBlobStorageAccountClientFromServicePrincipalProvider(optional_ptr<FileOpener> opener, const KeyValueSecret &secret, | ||
| const AzureParsedUrl &azure_parsed_url) { | ||
| auto transport_options = GetTransportOptions(opener, secret); | ||
| auto token_credential = CreateClientCredential(secret, transport_options); | ||
|
|
||
| auto cred = | ||
| GetOrCreateCredential(opener, secret, [&]() { return CreateClientCredential(secret, transport_options); }); | ||
| auto account_url = | ||
| azure_parsed_url.is_fully_qualified ? AccountUrl(azure_parsed_url) : AccountUrl(secret, DEFAULT_BLOB_ENDPOINT); | ||
| ; | ||
| auto blob_options = ToBlobClientOptions(transport_options, GetHttpState(opener)); | ||
| return Azure::Storage::Blobs::BlobServiceClient(account_url, token_credential, blob_options); | ||
| return Azure::Storage::Blobs::BlobServiceClient(account_url, cred, blob_options); | ||
| } | ||
|
|
||
| static Azure::Storage::Files::DataLake::DataLakeServiceClient | ||
| GetDfsStorageAccountClientFromServicePrincipalProvider(optional_ptr<FileOpener> opener, const KeyValueSecret &secret, | ||
| const AzureParsedUrl &azure_parsed_url) { | ||
| auto transport_options = GetTransportOptions(opener, secret); | ||
| auto token_credential = CreateClientCredential(secret, transport_options); | ||
|
|
||
| auto cred = | ||
| GetOrCreateCredential(opener, secret, [&]() { return CreateClientCredential(secret, transport_options); }); | ||
| auto account_url = | ||
| azure_parsed_url.is_fully_qualified ? AccountUrl(azure_parsed_url) : AccountUrl(secret, DEFAULT_DFS_ENDPOINT); | ||
| ; | ||
| auto dfs_options = ToDfsClientOptions(transport_options, GetHttpState(opener)); | ||
| return Azure::Storage::Files::DataLake::DataLakeServiceClient(account_url, token_credential, dfs_options); | ||
| return Azure::Storage::Files::DataLake::DataLakeServiceClient(account_url, cred, dfs_options); | ||
| } | ||
|
|
||
| static Azure::Storage::Blobs::BlobServiceClient | ||
| GetBlobStorageAccountClientFromAccessTokenProvider(optional_ptr<FileOpener> opener, const KeyValueSecret &secret, | ||
| const AzureParsedUrl &azure_parsed_url) { | ||
| auto transport_options = GetTransportOptions(opener, secret); | ||
| auto token_credential = CreateAccessTokenCredential(secret); | ||
|
|
||
| auto cred = GetOrCreateCredential(opener, secret, [&]() { return CreateAccessTokenCredential(secret); }); | ||
| auto account_url = | ||
| azure_parsed_url.is_fully_qualified ? AccountUrl(azure_parsed_url) : AccountUrl(secret, DEFAULT_BLOB_ENDPOINT); | ||
| ; | ||
| auto blob_options = ToBlobClientOptions(transport_options, GetHttpState(opener)); | ||
| return Azure::Storage::Blobs::BlobServiceClient(account_url, token_credential, blob_options); | ||
| return Azure::Storage::Blobs::BlobServiceClient(account_url, cred, blob_options); | ||
| } | ||
|
|
||
| static Azure::Storage::Files::DataLake::DataLakeServiceClient | ||
| GetDfsStorageAccountClientFromAccessTokenProvider(optional_ptr<FileOpener> opener, const KeyValueSecret &secret, | ||
| const AzureParsedUrl &azure_parsed_url) { | ||
| auto transport_options = GetTransportOptions(opener, secret); | ||
| auto token_credential = CreateAccessTokenCredential(secret); | ||
|
|
||
| auto cred = GetOrCreateCredential(opener, secret, [&]() { return CreateAccessTokenCredential(secret); }); | ||
| auto account_url = | ||
| azure_parsed_url.is_fully_qualified ? AccountUrl(azure_parsed_url) : AccountUrl(secret, DEFAULT_DFS_ENDPOINT); | ||
| ; | ||
| auto dfs_options = ToDfsClientOptions(transport_options, GetHttpState(opener)); | ||
| return Azure::Storage::Files::DataLake::DataLakeServiceClient(account_url, token_credential, dfs_options); | ||
| return Azure::Storage::Files::DataLake::DataLakeServiceClient(account_url, cred, dfs_options); | ||
| } | ||
|
|
||
| static Azure::Storage::Blobs::BlobServiceClient GetBlobStorageAccountClient(optional_ptr<FileOpener> opener, | ||
|
|
@@ -609,9 +665,18 @@ static Azure::Storage::Blobs::BlobServiceClient GetBlobStorageAccountClient(opti | |
| // Credential chain secret equivalent | ||
| auto credential_chain = TryGetCurrentSetting(opener, "azure_credential_chain"); | ||
| if (!credential_chain.empty()) { | ||
| auto credential = CreateChainedTokenCredential(credential_chain, transport_options); | ||
|
|
||
| return Azure::Storage::Blobs::BlobServiceClient(account_url, std::move(credential), blob_options); | ||
| // Settings-based credentials use a singleton cache key; any change to | ||
| // azure_credential_chain (the only setting that determines credential type) | ||
| // produces a different fingerprint and evicts the cached credential. | ||
| auto cache_key = std::string("azure_cred:settings"); | ||
| auto hash = duckdb_zstd::XXH64(credential_chain.c_str(), credential_chain.size(), 0); | ||
| auto fp = std::to_string(hash); | ||
| auto cred = GetCachedCredential(opener, cache_key, fp); | ||
| if (!cred) { | ||
| cred = CreateChainedTokenCredential(credential_chain, transport_options); | ||
| CacheTokenCredential(opener, cache_key, cred, fp); | ||
| } | ||
| return Azure::Storage::Blobs::BlobServiceClient(account_url, std::move(cred), blob_options); | ||
| } | ||
|
|
||
| // Anonymous | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will use potentially sensitive data (e.g. token) as a fingerprint that is now stored in the clientcontext state. I feel this would make it quite easy to lose track of things and accidentially have this sensitive data be printed to the screen or otherwise accidentally leaked somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thinking was less paranoid here, but I can also easily hash it and call it a day.