diff --git a/sdk/identity/azure-identity/src/token_credential_impl.cpp b/sdk/identity/azure-identity/src/token_credential_impl.cpp index 3c5a8ffc8..8e832f7f2 100644 --- a/sdk/identity/azure-identity/src/token_credential_impl.cpp +++ b/sdk/identity/azure-identity/src/token_credential_impl.cpp @@ -3,6 +3,7 @@ #include "private/token_credential_impl.hpp" +#include #include #include "private/package_version.hpp" @@ -143,16 +144,6 @@ namespace { throw std::runtime_error( std::string("Token JSON object: \'") + propertyName + "\' property was not found."); } - -bool GetPropertyValueAsInt64( - std::string const& jsonString, - std::string const& propertyName, - int64_t& outValue); - -bool GetPropertyValueAsString( - std::string const& jsonString, - std::string const& propertyName, - std::string& outValue); } // namespace AccessToken TokenCredentialImpl::ParseToken( @@ -161,140 +152,52 @@ AccessToken TokenCredentialImpl::ParseToken( std::string const& expiresInPropertyName, std::string const& expiresOnPropertyName) { - // TODO: use JSON parser. - AccessToken accessToken; - if (!GetPropertyValueAsString(jsonString, accessTokenPropertyName, accessToken.Token)) + try { - ThrowMissingJsonPropertyError(accessTokenPropertyName); - } + auto const parsedJson = Azure::Core::Json::_internal::json::parse(jsonString); + + if (!parsedJson.contains(accessTokenPropertyName)) + { + ThrowMissingJsonPropertyError(accessTokenPropertyName); + } + + AccessToken accessToken; + accessToken.Token = parsedJson[accessTokenPropertyName].get(); + + if (parsedJson.contains(expiresInPropertyName)) + { + accessToken.ExpiresOn + = std::chrono::system_clock::now() + + std::chrono::seconds( + parsedJson[expiresInPropertyName].get()); + } + else if (expiresOnPropertyName.empty()) + { + ThrowMissingJsonPropertyError(expiresInPropertyName); + } + else if (!parsedJson.contains(expiresOnPropertyName)) + { + ThrowMissingJsonPropertyError(expiresOnPropertyName); + } + else + { + auto expiresOn = parsedJson[expiresOnPropertyName].get(); + { + auto const spacePos = expiresOn.find(' '); + if (spacePos != std::string::npos) + { + expiresOn = expiresOn.replace(spacePos, 1, 1, 'T'); + } + } + + accessToken.ExpiresOn + = Azure::DateTime::Parse(expiresOn, Azure::DateTime::DateFormat::Rfc3339); + } - int64_t expiresIn = 0; - if (GetPropertyValueAsInt64(jsonString, expiresInPropertyName, expiresIn)) - { - accessToken.ExpiresOn = std::chrono::system_clock::now() + std::chrono::seconds(expiresIn); return accessToken; } - - if (expiresOnPropertyName.empty()) + catch (Azure::Core::Json::_internal::json::parse_error const& ex) { - ThrowMissingJsonPropertyError(expiresInPropertyName); + throw std::runtime_error(std::string("Error parsing token JSON: ") + ex.what()); } - - std::string expiresOn; - if (!GetPropertyValueAsString(jsonString, expiresOnPropertyName, expiresOn)) - { - ThrowMissingJsonPropertyError(expiresInPropertyName + "\' or \'" + expiresOnPropertyName); - } - - { - auto const spacePos = expiresOn.find(' '); - if (spacePos != std::string::npos) // LCOV_EXCL_LINE - { - expiresOn = expiresOn.replace(spacePos, 1, 1, 'T'); - } - } - - accessToken.ExpiresOn = Azure::DateTime::Parse(expiresOn, Azure::DateTime::DateFormat::Rfc3339); - return accessToken; } - -namespace { -std::string::size_type GetPropertyValueStart( - std::string const& jsonString, - std::string const& propertyName); - -bool GetPropertyValueAsInt64( - std::string const& jsonString, - std::string const& propertyName, - int64_t& outValue) -{ - auto const valueStartPos = GetPropertyValueStart(jsonString, propertyName); - if (valueStartPos == std::string::npos) - { - return false; - } - - int64_t value = 0; - { - auto const size = jsonString.size(); - for (auto pos = valueStartPos; pos < size; ++pos) - { - auto c = jsonString[pos]; - if (c < '0' || c > '9') - { - break; - } - - value = (value * 10) + (static_cast(c) - '0'); - } - } - - outValue = value; - - return true; -} - -std::string::size_type GetPropertyValueEnd(std::string const& str, std::string::size_type startPos); - -bool GetPropertyValueAsString( - std::string const& jsonString, - std::string const& propertyName, - std::string& outValue) -{ - auto const valueStartPos = GetPropertyValueStart(jsonString, propertyName); - if (valueStartPos == std::string::npos) - { - return false; - } - auto const jsonStringBegin = jsonString.begin(); - outValue = std::string( - jsonStringBegin + valueStartPos, - jsonStringBegin + GetPropertyValueEnd(jsonString, valueStartPos)); - - return true; -} - -std::string::size_type GetPropertyValueStart( - std::string const& jsonString, - std::string const& propertyName) -{ - auto const propertyNameValueSeparator = jsonString.find(':', jsonString.find(propertyName)); - if (propertyNameValueSeparator == std::string::npos) - { - return std::string::npos; - } - - auto pos = propertyNameValueSeparator; - { - auto const jsonStringSize = jsonString.size(); - for (; pos < jsonStringSize; ++pos) - { - auto c = jsonString[pos]; - if (c != ':' && c != ' ' && c != '\"' && c != '\'') - { - break; - } - } - } - - return pos; -} - -std::string::size_type GetPropertyValueEnd(std::string const& str, std::string::size_type startPos) -{ - auto pos = startPos; - { - auto const strSize = str.size(); - for (; pos < strSize; ++pos) - { - auto c = str[pos]; - if (c == '\"' || c == '\'') - { - break; - } - } - } - - return pos; -} -} // namespace diff --git a/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp b/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp index 0b0d3d5df..4fb9125e0 100644 --- a/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp @@ -290,3 +290,21 @@ TEST(AzureCliCredential, UnsafeChars) EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); } } + +TEST(AzureCliCredential, StrictIso8601TimeFormat) +{ + constexpr auto Token = "{\"accessToken\":\"ABCDEFGHIJKLMNOPQRSTUVWXYZ\"," + "\"expiresOn\":\"2022-08-24T00:43:08\"}"; // With the "T" + + AzureCliTestCredential const azCliCred(EchoCommand(Token)); + + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); + auto const token = azCliCred.GetToken(trc, {}); + + EXPECT_EQ(token.Token, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); + + EXPECT_EQ( + token.ExpiresOn, + DateTime::Parse("2022-08-24T00:43:08.000000Z", DateTime::DateFormat::Rfc3339)); +} diff --git a/sdk/identity/azure-identity/test/ut/token_credential_impl_test.cpp b/sdk/identity/azure-identity/test/ut/token_credential_impl_test.cpp index fb0670842..f9bf9f7f8 100644 --- a/sdk/identity/azure-identity/test/ut/token_credential_impl_test.cpp +++ b/sdk/identity/azure-identity/test/ut/token_credential_impl_test.cpp @@ -340,72 +340,6 @@ TEST(TokenCredentialImpl, NoToken) })); } -TEST(TokenCredentialImpl, CurrentJsonParserQuirksAndLimitations) -{ - // This test case is to cover all the current behavior in the JSON parsing code. - // It is to verify the edge cases to define limitations. A better behavior is possible, and it - // won't be a breaking change to update it. This parsing is internal. At some point, we will - // update the code to use the real JSON parser, instead of parsing ourselves. If when that - // happens, at any point, this test case gets broken, it is ok to drop/update this test case - it - // is likely that the proper JSON parser has better behavior. - - auto const actual = CredentialTestHelper::SimulateTokenRequest( - [](auto transport) { - TokenCredentialOptions options; - options.Transport.Transport = transport; - - return std::make_unique( - HttpMethod::Delete, Url("https://microsoft.com/"), options); - }, - {{{"https://azure.com/.default"}}, {{"https://azure.com/.default"}}}, - std::vector{ - {"{\"access_token\":\'ACCESSTOKEN\', \"expires_in\": \"\'"}, - {"{\"expires_in\": 3600, \"access_token\": \"\'"}}); - - EXPECT_EQ(actual.Requests.size(), 2U); - EXPECT_EQ(actual.Responses.size(), 2U); - - auto const& request0 = actual.Requests.at(0); - auto const& request1 = actual.Requests.at(1); - - auto const& response0 = actual.Responses.at(0); - auto const& response1 = actual.Responses.at(1); - - EXPECT_EQ(request0.HttpMethod, HttpMethod::Delete); - EXPECT_EQ(request1.HttpMethod, HttpMethod::Delete); - - EXPECT_EQ(request0.AbsoluteUrl, "https://microsoft.com"); - EXPECT_EQ(request1.AbsoluteUrl, "https://microsoft.com"); - - { - constexpr char expectedBody[] = "https://azure.com/.default "; - EXPECT_EQ(request0.Body, expectedBody); - EXPECT_EQ(request1.Body, expectedBody); - - EXPECT_NE(request0.Headers.find("Content-Length"), request0.Headers.end()); - EXPECT_EQ(request0.Headers.at("Content-Length"), std::to_string(sizeof(expectedBody) - 1)); - - EXPECT_NE(request1.Headers.find("Content-Length"), request1.Headers.end()); - EXPECT_EQ(request1.Headers.at("Content-Length"), std::to_string(sizeof(expectedBody) - 1)); - } - - EXPECT_NE(request0.Headers.find("Content-Type"), request0.Headers.end()); - EXPECT_EQ(request0.Headers.at("Content-Type"), "application/x-www-form-urlencoded"); - - EXPECT_NE(request1.Headers.find("Content-Type"), request1.Headers.end()); - EXPECT_EQ(request1.Headers.at("Content-Type"), "application/x-www-form-urlencoded"); - - EXPECT_EQ(response0.AccessToken.Token, "ACCESSTOKEN"); - EXPECT_EQ(response1.AccessToken.Token, std::string()); - - using namespace std::chrono_literals; - EXPECT_GE(response0.AccessToken.ExpiresOn, response0.EarliestExpiration + 0s); - EXPECT_LE(response0.AccessToken.ExpiresOn, response0.LatestExpiration + 0s); - - EXPECT_GE(response1.AccessToken.ExpiresOn, response1.EarliestExpiration + 3600s); - EXPECT_LE(response1.AccessToken.ExpiresOn, response1.LatestExpiration + 3600s); -} - TEST(TokenCredentialImpl, NullResponse) { using Azure::Core::Http::RawResponse;