Improve error messages when loading credentials.

google/cloud/storage/CMakeLists.txt

@@ -174,13 +174,15 @@ add_library(storage_client
             oauth2/anonymous_credentials.h
             oauth2/anonymous_credentials.cc
             oauth2/authorized_user_credentials.h
+            oauth2/authorized_user_credentials.cc
             oauth2/credential_constants.h
             oauth2/credentials.h
             oauth2/google_application_default_credentials_file.h
             oauth2/google_application_default_credentials_file.cc
             oauth2/google_credentials.h
             oauth2/google_credentials.cc
             oauth2/service_account_credentials.h
+            oauth2/service_account_credentials.cc
             object_access_control.h
             object_access_control.cc
             object_metadata.h

google/cloud/storage/oauth2/authorized_user_credentials.cc

@@ -0,0 +1,57 @@
+// Copyright 2018 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "google/cloud/storage/oauth2/authorized_user_credentials.h"
+
+namespace google {
+namespace cloud {
+namespace storage {
+inline namespace STORAGE_CLIENT_NS {
+namespace oauth2 {
+AuthorizedUserCredentialsInfo ParseAuthorizedUserCredentials(
+    std::string const& content, std::string const& source) {
+  auto credentials =
+      storage::internal::nl::json::parse(content, nullptr, false);
+  if (credentials.is_discarded()) {
+    google::cloud::internal::RaiseInvalidArgument(
+        "Invalid AuthorizedUserCredentials, parsing failed on data from " +
+        source);
+  }
+
+  char const client_id_key[] = "client_id";
+  char const client_secret_key[] = "client_secret";
+  char const refresh_token_key[] = "refresh_token";
+  for (auto const& key :
+       {client_id_key, client_secret_key, refresh_token_key}) {
+    if (credentials.count(key) == 0U) {
+      google::cloud::internal::RaiseInvalidArgument(
+          "Invalid AuthorizedUserCredentials, the " + std::string(key) +
+          " field is missing on data loaded from " + source);
+    }
+    if (credentials.value(key, "").empty()) {
+      google::cloud::internal::RaiseInvalidArgument(
+          "Invalid AuthorizedUserCredentials, the " + std::string(key) +
+          " field is empty on data loaded from " + source);
+    }
+  }
+  return AuthorizedUserCredentialsInfo{
+      credentials.value(client_id_key, ""),
+      credentials.value(client_secret_key, ""),
+      credentials.value(refresh_token_key, "")};
+}
+}  // namespace oauth2
+}  // namespace STORAGE_CLIENT_NS
+}  // namespace storage
+}  // namespace cloud
+}  // namespace google

google/cloud/storage/oauth2/authorized_user_credentials.h

@@ -30,6 +30,17 @@ namespace cloud {
 namespace storage {
 inline namespace STORAGE_CLIENT_NS {
 namespace oauth2 {
+/// A plain object to hold the result of parsing authorized user credentials.
+struct AuthorizedUserCredentialsInfo {
+  std::string client_id;
+  std::string client_secret;
+  std::string refresh_token;
+};
+
+/// Parse a JSON object string as an AuthorizedUserCredentials.
+AuthorizedUserCredentialsInfo ParseAuthorizedUserCredentials(
+    std::string const& content, std::string const& source);
+
 /**
  * A C++ wrapper for Google's Authorized User Credentials.
  *
@@ -53,26 +64,26 @@ template <typename HttpRequestBuilderType =
               storage::internal::CurlRequestBuilder>
 class AuthorizedUserCredentials : public Credentials {
  public:
-  explicit AuthorizedUserCredentials(std::string const& contents)
-      : AuthorizedUserCredentials(contents, GoogleOAuthRefreshEndpoint()) {}
+  explicit AuthorizedUserCredentials(std::string const& contents,
+                                     std::string const& source)
+      : AuthorizedUserCredentials(contents, source,
+                                  GoogleOAuthRefreshEndpoint()) {}
 
   explicit AuthorizedUserCredentials(std::string const& content,
+                                     std::string const& source,
                                      std::string oauth_server)
       : expiration_time_() {
     HttpRequestBuilderType request_builder(
         std::move(oauth_server),
         storage::internal::GetDefaultCurlHandleFactory());
-    auto credentials = storage::internal::nl::json::parse(content);
+    auto info = ParseAuthorizedUserCredentials(content, source);
     std::string payload("grant_type=refresh_token");
     payload += "&client_id=";
-    payload +=
-        request_builder.MakeEscapedString(credentials["client_id"]).get();
+    payload += request_builder.MakeEscapedString(info.client_id).get();
     payload += "&client_secret=";
-    payload +=
-        request_builder.MakeEscapedString(credentials["client_secret"]).get();
+    payload += request_builder.MakeEscapedString(info.client_secret).get();
     payload += "&refresh_token=";
-    payload +=
-        request_builder.MakeEscapedString(credentials["refresh_token"]).get();
+    payload += request_builder.MakeEscapedString(info.refresh_token).get();
     payload_ = std::move(payload);
     request_ = request_builder.BuildRequest();
   }
@@ -92,16 +103,23 @@ class AuthorizedUserCredentials : public Credentials {
 
     // TODO(#516) - use retry policies to refresh the credentials.
     auto response = request_.MakeRequest(payload_);
-    if (200 != response.status_code) {
+    if (response.status_code >= 300) {
+      return false;
+    }
+    nl::json access_token = nl::json::parse(response.payload, nullptr, false);
+    if (access_token.is_discarded() or access_token.count("token_type") == 0U or
+        access_token.count("access_token") == 0U or
+        access_token.count("id_token") == 0U or
+        access_token.count("expires_in") == 0U) {
       return false;
     }
-    nl::json access_token = nl::json::parse(response.payload);
     std::string header = "Authorization: ";
-    header += access_token["token_type"].get_ref<std::string const&>();
+    header += access_token.value("token_type", "");
     header += ' ';
-    header += access_token["access_token"].get_ref<std::string const&>();
-    std::string new_id = access_token["id_token"];
-    auto expires_in = std::chrono::seconds(access_token["expires_in"]);
+    header += access_token.value("access_token", "");
+    std::string new_id = access_token.value("id_token", "");
+    auto expires_in =
+        std::chrono::seconds(access_token.value("expires_in", int(0)));
     auto new_expiration = std::chrono::system_clock::now() + expires_in -
                           GoogleOAuthAccessTokenExpirationSlack();
     // Do not update any state until all potential exceptions are raised.

google/cloud/storage/oauth2/authorized_user_credentials_test.cc

@@ -87,7 +87,7 @@ TEST_F(AuthorizedUserCredentialsTest, Simple) {
       "type": "magic_type"
 })""";
 
-  AuthorizedUserCredentials<MockHttpRequestBuilder> credentials(config);
+  AuthorizedUserCredentials<MockHttpRequestBuilder> credentials(config, "test");
   EXPECT_EQ("Authorization: Type access-token-value",
             credentials.AuthorizationHeader());
 }
@@ -137,7 +137,7 @@ TEST_F(AuthorizedUserCredentialsTest, Refresh) {
       "refresh_token": "1/THETOKEN",
       "type": "magic_type"
 })""";
-  AuthorizedUserCredentials<MockHttpRequestBuilder> credentials(config);
+  AuthorizedUserCredentials<MockHttpRequestBuilder> credentials(config, "test");
   EXPECT_EQ("Authorization: Type access-token-r1",
             credentials.AuthorizationHeader());
   EXPECT_EQ("Authorization: Type access-token-r2",
@@ -146,6 +146,161 @@ TEST_F(AuthorizedUserCredentialsTest, Refresh) {
             credentials.AuthorizationHeader());
 }
 
+/// @test Verify that invalid contents result in a readable error.
+TEST_F(AuthorizedUserCredentialsTest, InvalidContents) {
+  std::string config = R"""( not-a-valid-json-string })""";
+
+#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
+  auto mock_builder = MockHttpRequestBuilder::mock;
+  EXPECT_CALL(*mock_builder, Constructor(GoogleOAuthRefreshEndpoint()))
+      .Times(1);
+  EXPECT_THROW(
+      try {
+        AuthorizedUserCredentials<MockHttpRequestBuilder> credentials(
+            config, "test-as-a-source");
+      } catch (std::invalid_argument const& ex) {
+        EXPECT_THAT(ex.what(), HasSubstr("Invalid AuthorizedUserCredentials"));
+        EXPECT_THAT(ex.what(), HasSubstr("test-as-a-source"));
+        throw;
+      },
+      std::invalid_argument);
+#else
+  EXPECT_DEATH_IF_SUPPORTED(AuthorizedUserCredentials<MockHttpRequestBuilder>(
+                                config, "test-as-a-source"),
+                            "exceptions are disabled");
+#endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
+}
+
+/// @test Verify that missing fields result in a readable error.
+TEST_F(AuthorizedUserCredentialsTest, MissingContents) {
+  std::string config = R"""({
+      "client_secret": "a-123456ABCDEF",
+      "refresh_token": "1/THETOKEN",
+      "type": "magic_type"
+})""";
+
+#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
+  auto mock_builder = MockHttpRequestBuilder::mock;
+  EXPECT_CALL(*mock_builder, Constructor(GoogleOAuthRefreshEndpoint()))
+      .Times(1);
+  EXPECT_THROW(
+      try {
+        AuthorizedUserCredentials<MockHttpRequestBuilder> credentials(
+            config, "test-as-a-source");
+      } catch (std::invalid_argument const& ex) {
+        EXPECT_THAT(ex.what(), HasSubstr("the client_id field is missing"));
+        EXPECT_THAT(ex.what(), HasSubstr("test-as-a-source"));
+        throw;
+      },
+      std::invalid_argument);
+#else
+  EXPECT_DEATH_IF_SUPPORTED(AuthorizedUserCredentials<MockHttpRequestBuilder>(
+                                config, "test-as-a-source"),
+                            "exceptions are disabled");
+#endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
+}
+
+/// @test Verify that parsing works in the easy case.
+TEST_F(AuthorizedUserCredentialsTest, ParseSimple) {
+  std::string contents = R"""({
+      "client_id": "a-client-id.example.com",
+      "client_secret": "a-123456ABCDEF",
+      "refresh_token": "1/THETOKEN",
+      "type": "magic_type"
+})""";
+
+  auto actual = ParseAuthorizedUserCredentials(contents, "test-data");
+  EXPECT_EQ("a-client-id.example.com", actual.client_id);
+  EXPECT_EQ("a-123456ABCDEF", actual.client_secret);
+  EXPECT_EQ("1/THETOKEN", actual.refresh_token);
+}
+/// @test Verify that invalid contents result in a readable error.
+TEST_F(AuthorizedUserCredentialsTest, ParseInvalid) {
+  std::string contents = R"""( not-a-valid-json-string )""";
+
+#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
+  EXPECT_THROW(
+      try {
+        ParseAuthorizedUserCredentials(contents, "test-data");
+      } catch (std::invalid_argument const& ex) {
+        EXPECT_THAT(ex.what(), HasSubstr("Invalid AuthorizedUserCredentials"));
+        EXPECT_THAT(ex.what(), HasSubstr("test-data"));
+        throw;
+      },
+      std::invalid_argument);
+#else
+  EXPECT_DEATH_IF_SUPPORTED(
+      ParseAuthorizedUserCredentials(contents, "test-data"),
+      "exceptions are disabled");
+#endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
+}
+
+/// @test Parsing a service account JSON string should detect empty fields.
+TEST_F(AuthorizedUserCredentialsTest, ParseEmptyField) {
+  std::string contents = R"""({
+      "client_id": "a-client-id.example.com",
+      "client_secret": "a-123456ABCDEF",
+      "refresh_token": "1/THETOKEN",
+      "type": "magic_type"
+})""";
+
+  for (auto const& field : {"client_id", "client_secret", "refresh_token"}) {
+    internal::nl::json json = internal::nl::json::parse(contents);
+    json[field] = "";
+#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
+    EXPECT_THROW(
+        try {
+          ParseAuthorizedUserCredentials(json.dump(), "test-data");
+        } catch (std::invalid_argument const& ex) {
+          EXPECT_THAT(ex.what(), HasSubstr(field));
+          EXPECT_THAT(ex.what(), HasSubstr(" field is empty"));
+          EXPECT_THAT(ex.what(), HasSubstr("test-data"));
+          throw;
+        },
+        std::invalid_argument)
+        << "field=" << field;
+#else
+    EXPECT_DEATH_IF_SUPPORTED(
+        ParseAuthorizedUserCredentials(json.dump(), "test-data"),
+        "exceptions are disabled")
+        << "field=" << field;
+#endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
+  }
+}
+
+/// @test Parsing a service account JSON string should detect missing fields.
+TEST_F(AuthorizedUserCredentialsTest, ParseMissingField) {
+  std::string contents = R"""({
+      "client_id": "a-client-id.example.com",
+      "client_secret": "a-123456ABCDEF",
+      "refresh_token": "1/THETOKEN",
+      "type": "magic_type"
+})""";
+
+  for (auto const& field : {"client_id", "client_secret", "refresh_token"}) {
+    internal::nl::json json = internal::nl::json::parse(contents);
+    json.erase(field);
+#if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
+    EXPECT_THROW(
+        try {
+          ParseAuthorizedUserCredentials(json.dump(), "test-data");
+        } catch (std::invalid_argument const& ex) {
+          EXPECT_THAT(ex.what(), HasSubstr(field));
+          EXPECT_THAT(ex.what(), HasSubstr(" field is missing"));
+          EXPECT_THAT(ex.what(), HasSubstr("test-data"));
+          throw;
+        },
+        std::invalid_argument)
+        << "field=" << field;
+#else
+    EXPECT_DEATH_IF_SUPPORTED(
+        ParseAuthorizedUserCredentials(json.dump(), "test-data"),
+        "exceptions are disabled")
+        << "field=" << field;
+#endif  // GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS
+  }
+}
+
 }  // namespace
 }  // namespace oauth2
 }  // namespace STORAGE_CLIENT_NS

google/cloud/storage/oauth2/google_credentials.cc

@@ -29,18 +29,28 @@ std::shared_ptr<Credentials> GoogleDefaultCredentials() {
   auto path = GoogleAdcFilePathOrEmpty();
   if (not path.empty()) {
     std::ifstream is(path);
+    if (not is.is_open()) {
+      google::cloud::internal::RaiseRuntimeError(
+          "Cannot open credentials file " + path);
+    }
     std::string contents(std::istreambuf_iterator<char>{is}, {});
-    auto cred_json = storage::internal::nl::json::parse(contents);
+    auto cred_json =
+        storage::internal::nl::json::parse(contents, nullptr, false);
+    if (cred_json.is_discarded()) {
+      google::cloud::internal::RaiseRuntimeError(
+          "Invalid contents in credentials file " + path);
+    }
     std::string cred_type = cred_json.value("type", "no type given");
     if (cred_type == "authorized_user") {
-      return std::make_shared<AuthorizedUserCredentials<>>(contents);
+      return std::make_shared<AuthorizedUserCredentials<>>(contents, path);
     }
     if (cred_type == "service_account") {
-      return std::make_shared<ServiceAccountCredentials<>>(contents);
+      return std::make_shared<ServiceAccountCredentials<>>(contents, path);
     }
     google::cloud::internal::RaiseRuntimeError(
         "Unsupported credential type (" + cred_type +
-        ") when reading Application Default Credentials file.");
+        ") when reading Application Default Credentials file from " + path +
+        ".");
   }
 
   // TODO(#579): Check for implicit environment-based credentials if no ADC file
@@ -58,24 +68,24 @@ std::shared_ptr<AuthorizedUserCredentials<>>
 CreateAuthorizedUserCredentialsFromJsonFilePath(std::string const& path) {
   std::ifstream is(path);
   std::string contents(std::istreambuf_iterator<char>{is}, {});
-  return CreateAuthorizedUserCredentialsFromJsonContents(contents);
+  return std::make_shared<AuthorizedUserCredentials<>>(contents, path);
 }
 
 std::shared_ptr<AuthorizedUserCredentials<>>
 CreateAuthorizedUserCredentialsFromJsonContents(std::string const& contents) {
-  return std::make_shared<AuthorizedUserCredentials<>>(contents);
+  return std::make_shared<AuthorizedUserCredentials<>>(contents, "memory");
 }
 
 std::shared_ptr<ServiceAccountCredentials<>>
 CreateServiceAccountCredentialsFromJsonFilePath(std::string const& path) {
   std::ifstream is(path);
   std::string contents(std::istreambuf_iterator<char>{is}, {});
-  return CreateServiceAccountCredentialsFromJsonContents(contents);
+  return std::make_shared<ServiceAccountCredentials<>>(contents, path);
 }
 
 std::shared_ptr<ServiceAccountCredentials<>>
 CreateServiceAccountCredentialsFromJsonContents(std::string const& contents) {
-  return std::make_shared<ServiceAccountCredentials<>>(contents);
+  return std::make_shared<ServiceAccountCredentials<>>(contents, "memory");
 }
 
 }  // namespace oauth2