From 5b5cb9b5f70f1f73b7998f0757674661d48e9285 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Thu, 1 Jul 2021 13:45:43 -0500 Subject: [PATCH] ApiView requested changes (#2493) * ApiView requested changes * update test * Apply suggestions from code review Co-authored-by: Ahson Khan * update private field name Co-authored-by: Ahson Khan --- .../azure-security-keyvault-keys/CHANGELOG.md | 9 ++++- .../keys/cryptography/cryptography_client.hpp | 36 ++++++++--------- .../cryptography_client_options.hpp | 12 ------ .../keys/cryptography/decrypt_parameters.hpp | 33 ++++++++++----- .../keys/cryptography/encrypt_parameters.hpp | 40 ++++++++++++------- .../keyvault/keys/key_client_options.hpp | 12 ------ .../cryptography_client_options.cpp | 2 - .../src/cryptography/decrypt_parameters.cpp | 5 ++- .../src/cryptography/encrypt_parameters.cpp | 5 ++- .../src/key_client_options.cpp | 2 - .../test/ut/key_client_test.cpp | 12 ------ 11 files changed, 80 insertions(+), 88 deletions(-) diff --git a/sdk/keyvault/azure-security-keyvault-keys/CHANGELOG.md b/sdk/keyvault/azure-security-keyvault-keys/CHANGELOG.md index b17555340..98b61176c 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/CHANGELOG.md +++ b/sdk/keyvault/azure-security-keyvault-keys/CHANGELOG.md @@ -4,13 +4,20 @@ ### Features Added +- Added `GetIv()` to `EncryptParameters` and `DecryptParameters`. + ### Breaking Changes +- Removed `Azure::Security::KeyVault::Keys::ServiceVersion::V7_0` and `V7_1`. +- Removed `Azure::Security::KeyVault::Keys::Cryptography::ServiceVersion::V7_0` and `V7_1`. +- Removed `CryptographyClient::RemoteClient()` and `CryptographyClient::LocalOnly()`. +- Removed the general constructor from `EncryptParameters` and `DecryptParameters`. +- Removed access to `Iv` field member from `EncryptParameters` and `DecryptParameters`. + ### Key Bugs Fixed ### Fixed - ## 4.0.0-beta.3 (2021-06-08) ### Breaking Changes diff --git a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/cryptography_client.hpp b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/cryptography_client.hpp index 18f42b8fe..66ef1a65d 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/cryptography_client.hpp +++ b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/cryptography_client.hpp @@ -52,6 +52,24 @@ namespace Azure { void Initialize(std::string const& operation, Azure::Core::Context const& context); + /** + * @brief Provides a #CryptographyProvider that performs operations in the Key Vault Keys + * Server. + * + * @return A cryptographic client to perform operations on the server. + */ + std::shared_ptr + RemoteClient() const + { + return m_remoteProvider; + } + + /** + * @brief Gets whether this #CryptographyClient runs only local operations. + * + */ + bool LocalOnly() const noexcept { return m_remoteProvider == nullptr; } + public: /** * @brief Initializes a new instance of the #CryptographyClient class. @@ -71,24 +89,6 @@ namespace Azure { { } - /** - * @brief Provides a #CryptographyProvider that performs operations in the Key Vault Keys - * Server. - * - * @return A cryptographic client to perform operations on the server. - */ - std::shared_ptr - RemoteClient() const - { - return m_remoteProvider; - } - - /** - * @brief Gets whether this #CryptographyClient runs only local operations. - * - */ - bool LocalOnly() const noexcept { return m_remoteProvider == nullptr; } - /** * @brief Encrypts plaintext. * diff --git a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/cryptography_client_options.hpp b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/cryptography_client_options.hpp index 5b386d424..a560cc18f 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/cryptography_client_options.hpp +++ b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/cryptography_client_options.hpp @@ -49,18 +49,6 @@ namespace Azure { */ std::string const& ToString() const { return m_version; } - /** - * @brief Use to send request to the 7.0 version of Key Vault service. - * - */ - AZ_SECURITY_KEYVAULT_KEYS_DLLEXPORT static const ServiceVersion V7_0; - - /** - * @brief Use to send request to the 7.1 version of Key Vault service. - * - */ - AZ_SECURITY_KEYVAULT_KEYS_DLLEXPORT static const ServiceVersion V7_1; - /** * @brief Use to send request to the 7.2 version of Key Vault service. * diff --git a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/decrypt_parameters.hpp b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/decrypt_parameters.hpp index 38a9520c9..d261a7377 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/decrypt_parameters.hpp +++ b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/decrypt_parameters.hpp @@ -24,8 +24,8 @@ namespace Azure { * @brief Parameters for decrypting ciphertext. * */ - struct DecryptParameters final - { + class DecryptParameters final { + private: /** * @brief Construct a new Decrypt Parameters object. * @@ -43,22 +43,17 @@ namespace Azure { std::vector iv, std::vector additionalAuthenticatedData, std::vector authenticationTag) - : Algorithm(std::move(algorithm)), Ciphertext(std::move(ciphertext)), Iv(std::move(iv)), + : m_iv(std::move(iv)), Algorithm(std::move(algorithm)), Ciphertext(std::move(ciphertext)), AdditionalAuthenticatedData(std::move(additionalAuthenticatedData)), AuthenticationTag(std::move(authenticationTag)) { } - DecryptParameters(EncryptionAlgorithm algorithm, std::vector const ciphertext) - : Algorithm(std::move(algorithm)), Ciphertext(std::move(ciphertext)) - { - } - DecryptParameters( EncryptionAlgorithm algorithm, std::vector ciphertext, std::vector iv) - : Algorithm(std::move(algorithm)), Ciphertext(std::move(ciphertext)), Iv(std::move(iv)) + : m_iv(std::move(iv)), Algorithm(std::move(algorithm)), Ciphertext(std::move(ciphertext)) { } @@ -68,6 +63,24 @@ namespace Azure { */ DecryptParameters() = delete; + /** + * @brief Gets the initialization vector for decryption. + * + */ + std::vector m_iv; + + public: + /** + * @brief Construct a new Decrypt Parameters object + * + * @param algorithm The #EncryptionAlgorithm to use for decrypt operation. + * @param ciphertext The content to decrypt. + */ + DecryptParameters(EncryptionAlgorithm algorithm, std::vector const ciphertext) + : Algorithm(std::move(algorithm)), Ciphertext(std::move(ciphertext)) + { + } + /** * @brief Gets or sets the #EncryptionAlgorithm. * @@ -84,7 +97,7 @@ namespace Azure { * @brief Gets the initialization vector for decryption. * */ - std::vector Iv; + std::vector const& GetIv() const { return m_iv; } /** * @brief Gets additional data that is authenticated during decryption but not encrypted. diff --git a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/encrypt_parameters.hpp b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/encrypt_parameters.hpp index 55d43760a..3ba8a5e87 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/encrypt_parameters.hpp +++ b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/encrypt_parameters.hpp @@ -24,8 +24,17 @@ namespace Azure { * @brief Parameters for encrypting plaintext. * */ - struct EncryptParameters final - { + class EncryptParameters final { + private: + /** + * @brief Gets the initialization vector for encryption. + * + * @note Initialization vector should not be set for some encryption algorithms. That's why it + * is private so it is only set by the factory methods. + * + */ + std::vector m_iv; + /** * @brief Construct a new Encrypt Parameters object. * @@ -40,11 +49,18 @@ namespace Azure { std::vector plaintext, std::vector iv, std::vector additionalAuthenticatedData) - : Algorithm(std::move(algorithm)), Plaintext(std::move(plaintext)), Iv(std::move(iv)), + : m_iv(std::move(iv)), Algorithm(std::move(algorithm)), Plaintext(std::move(plaintext)), AdditionalAuthenticatedData(std::move(additionalAuthenticatedData)) { } + /** + * @brief Encrypt Parameters can't be default constructed. + * + */ + EncryptParameters() = delete; + + public: /** * @brief Construct a new Encrypt Parameters object * @@ -56,12 +72,6 @@ namespace Azure { { } - /** - * @brief Encrypt Parameters can't be default constructed. - * - */ - EncryptParameters() = delete; - /** * @brief Gets the #EncryptionAlgorithm. * @@ -74,18 +84,18 @@ namespace Azure { */ std::vector Plaintext; - /** - * @brief Gets the initialization vector for encryption. - * - */ - std::vector Iv; - /** * @brief Gets additional data that is authenticated during decryption but not encrypted. * */ std::vector AdditionalAuthenticatedData; + /** + * @brief Gets the initialization vector for encryption. + * + */ + std::vector const& GetIv() const { return m_iv; } + /** * @brief Creates an instance of the #EncryptParameters class for the * #EncryptionAlgorithm::Rsa15 encryption algorithm. diff --git a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/key_client_options.hpp b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/key_client_options.hpp index 0f069cab2..09ff1c366 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/key_client_options.hpp +++ b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/key_client_options.hpp @@ -41,18 +41,6 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys { */ std::string const& ToString() const { return m_version; } - /** - * @brief Use to send request to the 7.0 version of Key Vault service. - * - */ - AZ_SECURITY_KEYVAULT_KEYS_DLLEXPORT static const ServiceVersion V7_0; - - /** - * @brief Use to send request to the 7.1 version of Key Vault service. - * - */ - AZ_SECURITY_KEYVAULT_KEYS_DLLEXPORT static const ServiceVersion V7_1; - /** * @brief Use to send request to the 7.2 version of Key Vault service. * diff --git a/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/cryptography_client_options.cpp b/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/cryptography_client_options.cpp index 68272594d..8607c9004 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/cryptography_client_options.cpp +++ b/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/cryptography_client_options.cpp @@ -8,7 +8,5 @@ namespace Azure { namespace KeyVault { namespace Keys { namespace Cryptography { - const ServiceVersion ServiceVersion::V7_0("7.0"); - const ServiceVersion ServiceVersion::V7_1("7.1"); const ServiceVersion ServiceVersion::V7_2("7.2"); }}}}} // namespace Azure::Security::KeyVault::Keys::Cryptography diff --git a/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/decrypt_parameters.cpp b/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/decrypt_parameters.cpp index 0c4c3405b..234f1bb5f 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/decrypt_parameters.cpp +++ b/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/decrypt_parameters.cpp @@ -25,10 +25,11 @@ namespace Azure { using namespace Azure::Security::KeyVault::_internal; payload[AlgorithmValue] = parameters.Algorithm.ToString(); payload[ValueParameterValue] = Base64Url::Base64UrlEncode(parameters.Ciphertext); + auto& iv = parameters.GetIv(); - if (parameters.Iv.size() > 0) + if (iv.size() > 0) { - payload[IvValue] = Base64Url::Base64UrlEncode(parameters.Iv); + payload[IvValue] = Base64Url::Base64UrlEncode(iv); } if (parameters.AdditionalAuthenticatedData.size() > 0) diff --git a/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/encrypt_parameters.cpp b/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/encrypt_parameters.cpp index 827352d8c..eeca8898d 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/encrypt_parameters.cpp +++ b/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/encrypt_parameters.cpp @@ -25,10 +25,11 @@ namespace Azure { using namespace Azure::Security::KeyVault::_internal; payload[AlgorithmValue] = parameters.Algorithm.ToString(); payload[ValueParameterValue] = Base64Url::Base64UrlEncode(parameters.Plaintext); + auto& iv = parameters.GetIv(); - if (parameters.Iv.size() > 0) + if (iv.size() > 0) { - payload[IvValue] = Base64Url::Base64UrlEncode(parameters.Iv); + payload[IvValue] = Base64Url::Base64UrlEncode(iv); } if (parameters.AdditionalAuthenticatedData.size() > 0) diff --git a/sdk/keyvault/azure-security-keyvault-keys/src/key_client_options.cpp b/sdk/keyvault/azure-security-keyvault-keys/src/key_client_options.cpp index 3e44dad84..bf1c0c892 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/src/key_client_options.cpp +++ b/sdk/keyvault/azure-security-keyvault-keys/src/key_client_options.cpp @@ -4,7 +4,5 @@ #include "azure/keyvault/keys/key_client_options.hpp" namespace Azure { namespace Security { namespace KeyVault { namespace Keys { - const ServiceVersion ServiceVersion::V7_0("7.0"); - const ServiceVersion ServiceVersion::V7_1("7.1"); const ServiceVersion ServiceVersion::V7_2("7.2"); }}}} // namespace Azure::Security::KeyVault::Keys 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 096ebaacd..49324cd52 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 @@ -30,18 +30,6 @@ TEST(KeyClient, ServiceVersion) { auto credential = std::make_shared("tenantID", "AppId", "SecretId"); - { - // 7.0 - EXPECT_NO_THROW(auto options = KeyClientOptions(ServiceVersion::V7_0); - KeyClient keyClient("vaultUrl", credential, options); - EXPECT_EQ(options.Version.ToString(), "7.0");); - } - { - // 7.1 - EXPECT_NO_THROW(auto options = KeyClientOptions(ServiceVersion::V7_1); - KeyClient keyClient("vaultUrl", credential, options); - EXPECT_EQ(options.Version.ToString(), "7.1");); - } { // 7.2 EXPECT_NO_THROW(auto options = KeyClientOptions(ServiceVersion::V7_2);