Skip to content

Commit

Permalink
Updates from API review part 2 (#3775)
Browse files Browse the repository at this point in the history
* Per request

* const

* Jeff feedback , clang, and test

* API review feedback updates, missed client to update , and some comments that somehow got reverted from the branch

* missed comment
  • Loading branch information
gearama authored Jun 29, 2022
1 parent 417c7fa commit 53cecae
Show file tree
Hide file tree
Showing 10 changed files with 23 additions and 76 deletions.
1 change: 0 additions & 1 deletion sdk/keyvault/azure-security-keyvault-keys/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ set(
set(
AZURE_KEYVAULT_KEYS_SOURCE
src/cryptography/cryptography_client.cpp
src/cryptography/cryptography_client_options.cpp
src/cryptography/decrypt_parameters.cpp
src/cryptography/decrypt_result.cpp
src/cryptography/encrypt_parameters.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,48 +20,6 @@ namespace Azure {
namespace Keys {
namespace Cryptography {

/**
* @brief Represent the Key Vault Keys Service Version.
*
*/
class ServiceVersion final {
private:
std::string m_version;

public:
/**
* @brief Construct a new Service Version object
*
* @param version The string version for the Key Vault keys service.
*/
ServiceVersion(std::string version) : m_version(std::move(version)) {}

/**
* @brief Enable comparing the ext enum.
*
* @param other Another #ServiceVersion to be compared.
*/
bool operator==(ServiceVersion const& other) const { return m_version == other.m_version; }

/**
* @brief Return the #ServiceVersion string representation.
*
*/
std::string const& ToString() const { return m_version; }

/**
* @brief Use to send request to the 7.2 version of Key Vault service.
*
*/
AZ_SECURITY_KEYVAULT_KEYS_DLLEXPORT static const ServiceVersion V7_2;

/**
* @brief Use to send request to the 7.2 version of Key Vault service.
*
*/
AZ_SECURITY_KEYVAULT_KEYS_DLLEXPORT static const ServiceVersion V7_3;
};

/**
* @brief Options that allow you to configure the #CryptographyClient for local or remote
* operations on Key Vault.
Expand All @@ -75,16 +33,12 @@ namespace Azure {
* versions](https://docs.microsoft.com/rest/api/keyvault/key-vault-versions).
*
*/
ServiceVersion Version;
std::string Version;

/**
* @brief Construct a new Key Client Options object.
*
* @param version Optional version for the client.
*/
CryptographyClientOptions(ServiceVersion version = ServiceVersion::V7_3)
: Azure::Core::_internal::ClientOptions(), Version(version)
{
}
CryptographyClientOptions() : Azure::Core::_internal::ClientOptions() { Version = "7.3"; }
};
}}}}} // namespace Azure::Security::KeyVault::Keys::Cryptography
Original file line number Diff line number Diff line change
Expand Up @@ -246,14 +246,12 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
* marked exportable. This operation requires the keys/release permission.
*
* @param name The name of the key.
* @param version The key version.
* @param options The options for the key release operation.
* @param context A cancellation token controlling the request lifetime.
* @return ReleaseKeyResult object.
*/
Azure::Response<ReleaseKeyResult> ReleaseKey(
std::string const& name,
std::string const& version,
KeyReleaseOptions const& options,
Azure::Core::Context const& context = Azure::Core::Context()) const;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
*
* @remark Format: base64url
*/
std::string Data;
std::string EncodedPolicy;
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -407,10 +407,16 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
Azure::Nullable<std::string> Nonce;

/**
* @brief The encryption algorithm to use to protected the exported key material
* @brief The encryption algorithm to use to protected the exported key material.
*
*/
Azure::Nullable<KeyEncryptionAlgorithm> Encryption;

/**
* @brief The version of the key to release.
*
*/
Azure::Nullable<std::string> Version;
};

}}}} // namespace Azure::Security::KeyVault::Keys
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ CryptographyClient::CryptographyClient(
std::string const& keyId,
std::shared_ptr<Core::Credentials::TokenCredential const> credential,
CryptographyClientOptions const& options)
: m_keyId(Azure::Core::Url(keyId)), m_apiVersion(options.Version.ToString())
: m_keyId(Azure::Core::Url(keyId)), m_apiVersion(options.Version)
{
std::vector<std::unique_ptr<HttpPolicy>> perRetrypolicies;
{
Expand Down

This file was deleted.

6 changes: 4 additions & 2 deletions sdk/keyvault/azure-security-keyvault-keys/src/key_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ Azure::Security::KeyVault::Keys::DeleteKeyOperation KeyClient::StartDeleteKey(

Azure::Response<ReleaseKeyResult> KeyClient::ReleaseKey(
std::string const& name,
std::string const& version,
KeyReleaseOptions const& options,
Azure::Core::Context const& context) const
{
Expand All @@ -247,7 +246,10 @@ Azure::Response<ReleaseKeyResult> KeyClient::ReleaseKey(

// Request and settings
auto request = CreateRequest(
HttpMethod::Post, {_detail::KeysPath, name, version, _detail::ReleaseValue}, &payloadStream);
HttpMethod::Post,
{_detail::KeysPath, name, options.Version.ValueOr(""), _detail::ReleaseValue},
&payloadStream);

request.SetHeader(HttpShared::ContentType, HttpShared::ApplicationJson);
// Send and parse respone
auto rawResponse = SendRequest(request, context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ Azure::Security::KeyVault::Keys::_detail::KeyReleasePolicySerializer::KeyRelease

payload[_detail::ContentTypeValue] = policy.ContentType.ValueOr(_detail::ContentTypeDefaultValue);
payload[_detail::ImmutableValue] = policy.Immutable;
payload[_detail::DataValue]
= Base64Url::Base64UrlEncode(std::vector<uint8_t>(policy.Data.begin(), policy.Data.end()));
payload[_detail::DataValue] = policy.EncodedPolicy;

return payload;
}
Expand All @@ -35,11 +34,10 @@ Azure::Security::KeyVault::Keys::_detail::KeyReleasePolicySerializer::KeyRelease
Azure::Core::Json::_internal::json const& rawResponse)
{
KeyReleasePolicy policy;
auto decodedData = Base64Url::Base64UrlDecode(rawResponse[_detail::DataValue].get<std::string>());

policy.ContentType = rawResponse[_detail::ContentTypeValue].get<std::string>();
policy.Immutable = rawResponse[_detail::ImmutableValue].get<bool>();
policy.Data = std::string(decodedData.begin(), decodedData.end());
policy.EncodedPolicy = rawResponse[_detail::DataValue].get<std::string>();

return policy;
}
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,9 @@ TEST_F(KeyVaultKeyClient, CreateKeyWithReleasePolicyOptions)
" \"version\" : \"1.0.0\""
"} ";
auto jsonParser = json::parse(dataStr);
options.ReleasePolicy.Value().Data = jsonParser.dump();
auto parsedJson = jsonParser.dump();
options.ReleasePolicy.Value().EncodedPolicy
= Base64Url::Base64UrlEncode(std::vector<uint8_t>(parsedJson.begin(), parsedJson.end()));
options.Exportable = true;
{
auto keyResponse = client.CreateKey(
Expand Down Expand Up @@ -372,7 +374,8 @@ TEST_F(KeyVaultKeyClient, CreateKeyWithReleasePolicyOptions)
EXPECT_FALSE(policy.Immutable);

EXPECT_EQ(
json::parse(options.ReleasePolicy.Value().Data).dump(1, ' ', true),
json::parse(policy.Data).dump(1, ' ', true));
json::parse(Base64Url::Base64UrlDecode(options.ReleasePolicy.Value().EncodedPolicy))
.dump(1, ' ', true),
json::parse(Base64Url::Base64UrlDecode(policy.EncodedPolicy)).dump(1, ' ', true));
}
}

0 comments on commit 53cecae

Please sign in to comment.