From 5a6176706d40b5e837560b9b92ab1ff69b8588c3 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Tue, 1 Oct 2024 23:42:01 -0700 Subject: [PATCH] Add basic input validation to ResourceIdentifier for ensuring the prefix match what is expected. (#6056) * Initial verification and tests. * Simplify validation checks to what's needed. * Update changelog entry. * Fix typo and disable cspell for a test line. * Address PR feedback. --- sdk/core/azure-core/CHANGELOG.md | 1 + sdk/core/azure-core/CMakeLists.txt | 1 + .../inc/azure/core/resource_identifier.hpp | 2 +- .../azure-core/src/resource_identifier.cpp | 26 +++++++++ .../test/ut/resource_identifier_test.cpp | 55 +++++++++++++++++++ 5 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 sdk/core/azure-core/src/resource_identifier.cpp diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 9cc155d8c..4bcc01592 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -9,6 +9,7 @@ ### Bugs Fixed - Make the HTTP transport behavior consistent between WinHTTP and libcurl by disabling automatically following redirects on Windows. +- Added basic input validation to `Azure::Core::ResourceIdentifier` to ensure the prefix match what is expected. ### Other Changes diff --git a/sdk/core/azure-core/CMakeLists.txt b/sdk/core/azure-core/CMakeLists.txt index 30a4f9d4c..87c5cfd9b 100644 --- a/sdk/core/azure-core/CMakeLists.txt +++ b/sdk/core/azure-core/CMakeLists.txt @@ -149,6 +149,7 @@ set( src/operation_status.cpp src/private/environment_log_level_listener.hpp src/private/package_version.hpp + src/resource_identifier.cpp src/tracing/tracing.cpp src/uuid.cpp ) diff --git a/sdk/core/azure-core/inc/azure/core/resource_identifier.hpp b/sdk/core/azure-core/inc/azure/core/resource_identifier.hpp index e3baaa278..93d41fbe9 100644 --- a/sdk/core/azure-core/inc/azure/core/resource_identifier.hpp +++ b/sdk/core/azure-core/inc/azure/core/resource_identifier.hpp @@ -24,7 +24,7 @@ namespace Azure { namespace Core { * * @param resourceId The id string to create the ResourceIdentifier from. */ - explicit ResourceIdentifier(std::string const& resourceId) : m_resourceId(resourceId){}; + explicit ResourceIdentifier(std::string const& resourceId); /** * @brief The string representation of this resource identifier. diff --git a/sdk/core/azure-core/src/resource_identifier.cpp b/sdk/core/azure-core/src/resource_identifier.cpp new file mode 100644 index 000000000..f23f853eb --- /dev/null +++ b/sdk/core/azure-core/src/resource_identifier.cpp @@ -0,0 +1,26 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#include "azure/core/resource_identifier.hpp" + +#include +#include + +namespace { +const std::string SubscriptionStart = "/subscriptions/"; +const std::string ProviderStart = "/providers/"; +} // namespace + +namespace Azure { namespace Core { + + ResourceIdentifier::ResourceIdentifier(std::string const& resourceId) : m_resourceId(resourceId) + { + // Validate prefix + if (resourceId.find(SubscriptionStart) != 0 && resourceId.find(ProviderStart) != 0) + { + throw std::invalid_argument( + "The ResourceIdentifier must start with '" + SubscriptionStart + "' or '" + ProviderStart + + "'."); + } + } +}} // namespace Azure::Core diff --git a/sdk/core/azure-core/test/ut/resource_identifier_test.cpp b/sdk/core/azure-core/test/ut/resource_identifier_test.cpp index de08d20e5..c966d0287 100644 --- a/sdk/core/azure-core/test/ut/resource_identifier_test.cpp +++ b/sdk/core/azure-core/test/ut/resource_identifier_test.cpp @@ -16,3 +16,58 @@ TEST(ResourceIdentifier, Basic) ResourceIdentifier resourceIdentifier(resourceId); EXPECT_EQ(resourceIdentifier.ToString(), resourceId); } + +class ValidValues : public ::testing::TestWithParam { +public: + std::string GetValidResourceId() { return GetParam(); } +}; + +TEST_P(ValidValues, ) +{ + ResourceIdentifier resourceIdentifier(GetValidResourceId()); + EXPECT_EQ(resourceIdentifier.ToString(), GetValidResourceId()); +} + +INSTANTIATE_TEST_SUITE_P( + ResourceIdentifier, + ValidValues, + testing::Values( + "/subscriptions/0c2f6471-1bf0-4dda-aec3-cb9272f09575/resourceGroups/myRg/providers/" + "Microsoft.Compute/virtualMachines/myVm", + "/subscriptions/0c2f6471-1bf0-4dda-aec3-cb9272f09575/resourceGroups/myRg/providers/" + "Microsoft.Network/virtualNetworks/myNet/subnets/mySubnet", + "/subscriptions/0c2f6471-1bf0-4dda-aec3-cb9272f09575/resourceGroups/myRg", + "/subscriptions/0c2f6471-1bf0-4dda-aec3-cb9272f09575/locations/MyLocation", + "/subscriptions/0c2f6471-1bf0-4dda-aec3-cb9272f09575", + "/providers/Microsoft.Billing/billingAccounts/" + "3984c6f4-2d2a-4b04-93ce-43cf4824b698%3Ae2f1492a-a492-468d-909f-bf7fe6662c01_2019-05-31", + "/subscriptions/17fecd63-33d8-4e43-ac6f-0aafa111b38d/locations/westus2", + "/subscriptions/db1ab6f0-4769-4b27-930e-01e2ef9c123c/" + "providers/Microsoft.Compute/locations/westus2")); + +TEST(ResourceIdentifier, Invalid) +{ + // empty + EXPECT_THROW(ResourceIdentifier(""), std::invalid_argument); + + // invalid tenant + EXPECT_THROW(ResourceIdentifier("/MicrosoftSomething/billingAccounts/"), std::invalid_argument); + EXPECT_THROW( + ResourceIdentifier("providers/subscription/MicrosoftSomething/billingAccounts/"), + std::invalid_argument); + EXPECT_THROW(ResourceIdentifier("/providers"), std::invalid_argument); + + // invalid input + EXPECT_THROW(ResourceIdentifier(" "), std::invalid_argument); + EXPECT_THROW(ResourceIdentifier("asdfghj"), std::invalid_argument); // cspell:disable-line + EXPECT_THROW(ResourceIdentifier("123456"), std::invalid_argument); + EXPECT_THROW(ResourceIdentifier("!@#$%^&*/"), std::invalid_argument); + EXPECT_THROW( + ResourceIdentifier("/0c2f6471-1bf0-4dda-aec3-cb9272f09575/myRg/"), std::invalid_argument); + + // too few elements + EXPECT_THROW(ResourceIdentifier("UnformattedString"), std::invalid_argument); + + // no known parts + EXPECT_THROW(ResourceIdentifier("/subs/sub1/rgs/rg1/"), std::invalid_argument); +}