From ddee45759fad2bcd5d7af9ad13bf1bc3baca8b5a Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Mon, 30 Aug 2021 16:39:54 -0700 Subject: [PATCH] Add support for non public scope for secret client credential on Key Vault (#2763) * add support for non public scope for secret client credential * improve function comments * update tests with good url * update secret url tests * update logic to no-throw * use class * Update sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp Co-authored-by: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> * format * fix * remove default scope * move patch out of Core to use package-per-client patch * update includes * Update sdk/keyvault/azure-security-keyvault-keys/CHANGELOG.md Co-authored-by: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> * update patch Co-authored-by: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> --- .../azure-security-keyvault-keys/CHANGELOG.md | 2 ++ .../src/cryptography/cryptography_client.cpp | 25 +++++++++++++- .../src/key_client.cpp | 34 ++++++++++++++----- .../src/private/key_constants.hpp | 1 - .../test/ut/key_client_test.cpp | 8 ++--- .../test/ut/mocked_client_test.cpp | 18 +++++----- .../src/secret_client.cpp | 27 +++++++++++++-- .../test/ut/secret_client_test.cpp | 19 ++++++----- 8 files changed, 99 insertions(+), 35 deletions(-) diff --git a/sdk/keyvault/azure-security-keyvault-keys/CHANGELOG.md b/sdk/keyvault/azure-security-keyvault-keys/CHANGELOG.md index 3002bb5c4..5e53bd55d 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/CHANGELOG.md +++ b/sdk/keyvault/azure-security-keyvault-keys/CHANGELOG.md @@ -10,6 +10,8 @@ ### Bugs Fixed +- [2750](https://github.com/Azure/azure-sdk-for-cpp/issues/2750) Support for Azure `managedhsm` cloud and any other non-public Azure cloud. + ### Other Changes ## 4.0.0 (2021-08-10) diff --git a/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/cryptography_client.cpp b/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/cryptography_client.cpp index 5e0074612..f618ab0d8 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/cryptography_client.cpp +++ b/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/cryptography_client.cpp @@ -17,6 +17,7 @@ #include "../private/key_wrap_parameters.hpp" #include "../private/keyvault_protocol.hpp" +#include #include #include #include @@ -56,6 +57,27 @@ inline std::vector CreateDigest( auto hashAlgorithm = algorithm.GetHashAlgorithm(); return hashAlgorithm->Final(data.data(), data.size()); } + +// This is a Key-Vault only patch to calculate token scope/audience +std::string GetScopeFromUrl(Azure::Core::Url const& url) +{ + std::string calculatedScope(url.GetScheme() + "://"); + auto const& hostWithAccount = url.GetHost(); + auto hostNoAccountStart = std::find(hostWithAccount.begin(), hostWithAccount.end(), '.'); + + // Insert the calculated scope only when then host in the url contains at least a `.` + // Otherwise, only the default scope will be there. + // We don't want to throw/validate input but just leave the values go to azure to decide what to + // do. + if (hostNoAccountStart != hostWithAccount.end()) + { + calculatedScope.append(hostNoAccountStart + 1, hostWithAccount.end()); + calculatedScope.append("/.default"); + } + + return calculatedScope; +} + } // namespace Request CryptographyClient::CreateRequest( @@ -97,7 +119,8 @@ CryptographyClient::CryptographyClient( m_apiVersion = options.Version.ToString(); std::vector> perRetrypolicies; { - Azure::Core::Credentials::TokenRequestContext const tokenContext = {{TokenContextValue}}; + Azure::Core::Credentials::TokenRequestContext const tokenContext + = {{::GetScopeFromUrl(m_keyId)}}; perRetrypolicies.emplace_back( std::make_unique(credential, tokenContext)); diff --git a/sdk/keyvault/azure-security-keyvault-keys/src/key_client.cpp b/sdk/keyvault/azure-security-keyvault-keys/src/key_client.cpp index 02c434e53..73f06cbee 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/src/key_client.cpp +++ b/sdk/keyvault/azure-security-keyvault-keys/src/key_client.cpp @@ -14,6 +14,7 @@ #include "private/keyvault_protocol.hpp" #include "private/package_version.hpp" +#include #include #include #include @@ -28,6 +29,27 @@ using namespace Azure::Core::Http::_internal; namespace { constexpr static const char KeyVaultServicePackageName[] = "keyvault-keys"; constexpr static const char CreateValue[] = "create"; + +// This is a Key-Vault only patch to calculate token scope/audience +std::string GetScopeFromUrl(Azure::Core::Url const& url) +{ + std::string calculatedScope(url.GetScheme() + "://"); + auto const& hostWithAccount = url.GetHost(); + auto hostNoAccountStart = std::find(hostWithAccount.begin(), hostWithAccount.end(), '.'); + + // Insert the calculated scope only when then host in the url contains at least a `.` + // Otherwise, only the default scope will be there. + // We don't want to throw/validate input but just leave the values go to azure to decide what to + // do. + if (hostNoAccountStart != hostWithAccount.end()) + { + calculatedScope.append(hostNoAccountStart + 1, hostWithAccount.end()); + calculatedScope.append("/.default"); + } + + return calculatedScope; +} + } // namespace std::unique_ptr KeyClient::SendRequest( @@ -69,19 +91,13 @@ KeyClient::KeyClient( { auto apiVersion = options.Version.ToString(); - std::vector> perRetrypoliciesOld; - { - Azure::Core::Credentials::TokenRequestContext const tokenContext = {{TokenContextValue}}; - - perRetrypoliciesOld.emplace_back( - std::make_unique(credential, tokenContext)); - } std::vector> perRetrypolicies; { - Azure::Core::Credentials::TokenRequestContext const tokenContext = {{TokenContextValue}}; + Azure::Core::Credentials::TokenRequestContext const tokenContext + = {{::GetScopeFromUrl(m_vaultUrl)}}; perRetrypolicies.emplace_back( - std::make_unique(credential, tokenContext)); + std::make_unique(credential, std::move(tokenContext))); } std::vector> perCallpolicies; diff --git a/sdk/keyvault/azure-security-keyvault-keys/src/private/key_constants.hpp b/sdk/keyvault/azure-security-keyvault-keys/src/private/key_constants.hpp index 8d10fd5bc..1af3249a2 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/src/private/key_constants.hpp +++ b/sdk/keyvault/azure-security-keyvault-keys/src/private/key_constants.hpp @@ -125,6 +125,5 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys { nam /***************** Service *********/ constexpr static const char ApiVersionValue[] = "api-version"; - constexpr static const char TokenContextValue[] = "https://vault.azure.net/.default"; }}}}} // namespace Azure::Security::KeyVault::Keys::_detail diff --git a/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_test.cpp b/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_test.cpp index d9a6fa11b..b1cc67354 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_test.cpp +++ b/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_test.cpp @@ -17,12 +17,12 @@ TEST(KeyClient, initClient) auto credential = std::make_shared("tenantID", "AppId", "SecretId"); { - EXPECT_NO_THROW(KeyClient keyClient("vaultUrl", credential)); + EXPECT_NO_THROW(KeyClient keyClient("http://account.vault.azure.net", credential)); } { KeyClientOptions options; options.Retry.MaxRetries = 10; - EXPECT_NO_THROW(KeyClient keyClient("vaultUrl", credential, options)); + EXPECT_NO_THROW(KeyClient keyClient("http://account.vault.azure.net", credential, options)); } } @@ -33,13 +33,13 @@ TEST(KeyClient, ServiceVersion) { // 7.2 EXPECT_NO_THROW(auto options = KeyClientOptions(ServiceVersion::V7_2); - KeyClient keyClient("vaultUrl", credential, options); + KeyClient keyClient("http://account.vault.azure.net", credential, options); EXPECT_EQ(options.Version.ToString(), "7.2");); } { // arbitrary version EXPECT_NO_THROW(auto options = KeyClientOptions(ServiceVersion("1.0")); - KeyClient keyClient("vaultUrl", credential, options); + KeyClient keyClient("http://account.vault.azure.net", credential, options); EXPECT_EQ(options.Version.ToString(), "1.0");); } } diff --git a/sdk/keyvault/azure-security-keyvault-keys/test/ut/mocked_client_test.cpp b/sdk/keyvault/azure-security-keyvault-keys/test/ut/mocked_client_test.cpp index 0893b3125..59b4986ad 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/test/ut/mocked_client_test.cpp +++ b/sdk/keyvault/azure-security-keyvault-keys/test/ut/mocked_client_test.cpp @@ -22,7 +22,7 @@ TEST_F(MockedTransportAdapterTest, keyvaultTelemetryId) m_clientOptions.Telemetry.ApplicationId = applicationId; m_client = std::make_unique< Azure::Security::KeyVault::Keys::Test::KeyClientWithNoAuthenticationPolicy>( - "url", m_clientOptions); + "http://account.vault.azure.net", m_clientOptions); // The fake response from the mocked transport adapter is good for parsing a Key back auto response = m_client->GetKey("name"); @@ -89,7 +89,7 @@ TEST_F(MockedTransportAdapterTest, CreateKeyRSA) m_clientOptions.Telemetry.ApplicationId = applicationId; m_client = std::make_unique< Azure::Security::KeyVault::Keys::Test::KeyClientWithNoAuthenticationPolicy>( - "url", m_clientOptions); + "http://account.vault.azure.net", m_clientOptions); // The fake response from the mocked transport adapter is good for parsing a Key back auto response = m_client->CreateKey("name", KeyVaultKeyType::Rsa); @@ -103,7 +103,7 @@ TEST_F(MockedTransportAdapterTest, CreateKeyRSA2) m_clientOptions.Telemetry.ApplicationId = applicationId; m_client = std::make_unique< Azure::Security::KeyVault::Keys::Test::KeyClientWithNoAuthenticationPolicy>( - "url", m_clientOptions); + "http://account.vault.azure.net", m_clientOptions); auto options = CreateRsaKeyOptions("name"); // The fake response from the mocked transport adapter is good for parsing a Key back @@ -118,7 +118,7 @@ TEST_F(MockedTransportAdapterTest, CreateKeyRSAHSM) m_clientOptions.Telemetry.ApplicationId = applicationId; m_client = std::make_unique< Azure::Security::KeyVault::Keys::Test::KeyClientWithNoAuthenticationPolicy>( - "url", m_clientOptions); + "http://account.vault.azure.net", m_clientOptions); auto options = CreateRsaKeyOptions("name", true); // The fake response from the mocked transport adapter is good for parsing a Key back @@ -133,7 +133,7 @@ TEST_F(MockedTransportAdapterTest, CreateKeyEC) m_clientOptions.Telemetry.ApplicationId = applicationId; m_client = std::make_unique< Azure::Security::KeyVault::Keys::Test::KeyClientWithNoAuthenticationPolicy>( - "url", m_clientOptions); + "http://account.vault.azure.net", m_clientOptions); auto options = CreateEcKeyOptions("name"); // The fake response from the mocked transport adapter is good for parsing a Key back @@ -148,7 +148,7 @@ TEST_F(MockedTransportAdapterTest, CreateKeyECHSM) m_clientOptions.Telemetry.ApplicationId = applicationId; m_client = std::make_unique< Azure::Security::KeyVault::Keys::Test::KeyClientWithNoAuthenticationPolicy>( - "url", m_clientOptions); + "http://account.vault.azure.net", m_clientOptions); auto options = CreateEcKeyOptions("name", true); // The fake response from the mocked transport adapter is good for parsing a Key back @@ -163,7 +163,7 @@ TEST_F(MockedTransportAdapterTest, CreateKeyOCT) m_clientOptions.Telemetry.ApplicationId = applicationId; m_client = std::make_unique< Azure::Security::KeyVault::Keys::Test::KeyClientWithNoAuthenticationPolicy>( - "url", m_clientOptions); + "http://account.vault.azure.net", m_clientOptions); auto options = CreateOctKeyOptions("name"); // The fake response from the mocked transport adapter is good for parsing a Key back @@ -178,7 +178,7 @@ TEST_F(MockedTransportAdapterTest, CreateKeyOCTHSM) m_clientOptions.Telemetry.ApplicationId = applicationId; m_client = std::make_unique< Azure::Security::KeyVault::Keys::Test::KeyClientWithNoAuthenticationPolicy>( - "url", m_clientOptions); + "http://account.vault.azure.net", m_clientOptions); auto options = CreateOctKeyOptions("name", true); // The fake response from the mocked transport adapter is good for parsing a Key back @@ -193,7 +193,7 @@ TEST_F(MockedTransportAdapterTest, GetPropertiesOfKeys) m_clientOptions.Telemetry.ApplicationId = applicationId; m_client = std::make_unique< Azure::Security::KeyVault::Keys::Test::KeyClientWithNoAuthenticationPolicy>( - "url", m_clientOptions); + "http://account.vault.azure.net", m_clientOptions); auto options = GetPropertiesOfKeysOptions(); // The fake response from the mocked transport adapter is good for parsing a Key back diff --git a/sdk/keyvault/azure-security-keyvault-secrets/src/secret_client.cpp b/sdk/keyvault/azure-security-keyvault-secrets/src/secret_client.cpp index 467e5c61e..54c0da5c2 100644 --- a/sdk/keyvault/azure-security-keyvault-secrets/src/secret_client.cpp +++ b/sdk/keyvault/azure-security-keyvault-secrets/src/secret_client.cpp @@ -16,6 +16,7 @@ #include #include +#include #include using namespace Azure::Security::KeyVault::Secrets; @@ -51,6 +52,26 @@ static inline RequestWithContinuationToken BuildRequestFromContinuationToken( } return request; } + +// This is a Key-Vault only patch to calculate token scope/audience +std::string GetScopeFromUrl(Azure::Core::Url const& url) +{ + std::string calculatedScope(url.GetScheme() + "://"); + auto const& hostWithAccount = url.GetHost(); + auto hostNoAccountStart = std::find(hostWithAccount.begin(), hostWithAccount.end(), '.'); + + // Insert the calculated scope only when then host in the url contains at least a `.` + // Otherwise, only the default scope will be there. + // We don't want to throw/validate input but just leave the values go to azure to decide what to + // do. + if (hostNoAccountStart != hostWithAccount.end()) + { + calculatedScope.append(hostNoAccountStart + 1, hostWithAccount.end()); + calculatedScope.append("/.default"); + } + + return calculatedScope; +} } // namespace const ServiceVersion ServiceVersion::V7_2("7.2"); @@ -61,18 +82,18 @@ SecretClient::SecretClient( SecretClientOptions options) { auto apiVersion = options.Version.ToString(); + Azure::Core::Url url(vaultUrl); std::vector> perRetrypolicies; { - Azure::Core::Credentials::TokenRequestContext const tokenContext - = {{"https://vault.azure.net/.default"}}; + Azure::Core::Credentials::TokenRequestContext const tokenContext = {{::GetScopeFromUrl(url)}}; perRetrypolicies.emplace_back( std::make_unique(credential, tokenContext)); } m_protocolClient = std::make_shared( - Azure::Core::Url(vaultUrl), + std::move(url), apiVersion, Azure::Core::Http::_internal::HttpPipeline( options, TelemetryName, PackageVersion::ToString(), std::move(perRetrypolicies), {})); diff --git a/sdk/keyvault/azure-security-keyvault-secrets/test/ut/secret_client_test.cpp b/sdk/keyvault/azure-security-keyvault-secrets/test/ut/secret_client_test.cpp index 8851bf1fd..ba1f9eebd 100644 --- a/sdk/keyvault/azure-security-keyvault-secrets/test/ut/secret_client_test.cpp +++ b/sdk/keyvault/azure-security-keyvault-secrets/test/ut/secret_client_test.cpp @@ -14,12 +14,13 @@ TEST(SecretClient, InitClient) auto credential = std::make_shared("tenantID", "AppId", "SecretId"); { - EXPECT_NO_THROW(SecretClient SecretClient("vaultUrl", credential)); + EXPECT_NO_THROW(SecretClient SecretClient("http://account.vault.azure.net", credential)); } { SecretClientOptions options; options.Retry.MaxRetries = 10; - EXPECT_NO_THROW(SecretClient secretClient("vaultUrl", credential, options)); + EXPECT_NO_THROW( + SecretClient secretClient("http://account.vault.azure.net", credential, options)); } } @@ -29,15 +30,17 @@ TEST(SecretClient, ServiceVersion) = std::make_shared("tenantID", "AppId", "SecretId"); { // 7.2 - EXPECT_NO_THROW(auto options = SecretClientOptions(ServiceVersion::V7_2); - SecretClient SecretClient("vaultUrl", credential, options); - EXPECT_EQ(options.Version.ToString(), "7.2");); + EXPECT_NO_THROW( + auto options = SecretClientOptions(ServiceVersion::V7_2); + SecretClient SecretClient("http://account.vault.azure.net", credential, options); + EXPECT_EQ(options.Version.ToString(), "7.2");); } { // arbitrary version - EXPECT_NO_THROW(auto options = SecretClientOptions(ServiceVersion("1.0")); - SecretClient secretClient("vaultUrl", credential, options); - EXPECT_EQ(options.Version.ToString(), "1.0");); + EXPECT_NO_THROW( + auto options = SecretClientOptions(ServiceVersion("1.0")); + SecretClient secretClient("http://account.vault.azure.net", credential, options); + EXPECT_EQ(options.Version.ToString(), "1.0");); } }