Fix the failing tests on win-asan for [1]. Seems like returning an empty "username" as part of

MakeUserNameForAccount() doesn't fail creation of user on some of the windows instances.

This fix explicitly returns a failure when we want to fail the login UI instead of depending on
"create user" to fail for "empty username" instead.

Note that currently the error_text is set as IDS_INTERNAL_ERROR_BASE. Explicit error handling
with appropriate error messages would be done in a followup change (already filed a bug for it).

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1659512/24

Bug: 976543
Change-Id: Ic4d92f5ef647d0f3cd5fd50c1f52b588c94a3afd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1672234
Commit-Queue: Rakesh Soma <rakeshsoma@google.com>
Reviewed-by: Tien Mai <tienmai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672133}

chrome/credential_provider/gaiacp/gcp_utils.cc

@@ -29,10 +29,12 @@
 #include "base/files/file.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
+#include "base/json/json_reader.h"
 #include "base/macros.h"
 #include "base/no_destructor.h"
 #include "base/path_service.h"
 #include "base/stl_util.h"
+#include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/win/current_module.h"
@@ -727,6 +729,22 @@ void SecurelyClearBuffer(void* buffer, size_t length) {
     ::RtlSecureZeroMemory(buffer, length);
 }
 
+std::string SearchForKeyInStringDictUTF8(
+    const std::string& json_string,
+    const std::initializer_list<base::StringPiece>& path) {
+  DCHECK(path.size() > 0);
+
+  base::Optional<base::Value> json_obj =
+      base::JSONReader::Read(json_string, base::JSON_ALLOW_TRAILING_COMMAS);
+  if (!json_obj || !json_obj->is_dict()) {
+    LOGFN(ERROR) << "base::JSONReader::Read failed to translate to JSON";
+    return std::string();
+  }
+  const std::string* value =
+      json_obj->FindStringPath(base::JoinString(path, "."));
+  return value ? *value : std::string();
+}
+
 base::string16 GetDictString(const base::Value& dict, const char* name) {
   DCHECK(name);
   DCHECK(dict.is_dict());

chrome/credential_provider/gaiacp/gcp_utils.h

@@ -232,9 +232,18 @@ void SecurelyClearBuffer(void* buffer, size_t length);
 
 // Helpers to get strings from base::Values that are expected to be
 // DictionaryValues.
+
 base::string16 GetDictString(const base::Value& dict, const char* name);
 base::string16 GetDictString(const std::unique_ptr<base::Value>& dict,
                              const char* name);
+// Perform a recursive search on a nested dictionary object. Note that the
+// names provided in the input should be in order. Below is an example : Lets
+// say the json object is {"key1": {"key2": {"key3": "value1"}}, "key4":
+// "value2"}. Then to search for the key "key3", this method should be called
+// by providing the names vector as {"key1", "key2", "key3"}.
+std::string SearchForKeyInStringDictUTF8(
+    const std::string& json_string,
+    const std::initializer_list<base::StringPiece>& path);
 std::string GetDictStringUTF8(const base::Value& dict, const char* name);
 std::string GetDictStringUTF8(const std::unique_ptr<base::Value>& dict,
                               const char* name);

chrome/credential_provider/gaiacp/os_user_manager.cc

@@ -72,6 +72,11 @@ void OSUserManager::SetInstanceForTesting(OSUserManager* instance) {
   *GetInstanceStorage() = instance;
 }
 
+// static
+bool OSUserManager::IsDeviceDomainJoined() {
+  return base::win::IsEnrolledToDomain();
+}
+
 // static
 base::string16 OSUserManager::GetLocalDomain() {
   // If the domain is the current computer, then there is no domain controller.

chrome/credential_provider/gaiacp/os_user_manager.h

@@ -99,6 +99,9 @@ class [[clang::lto_visibility_public]] OSUserManager {
   // to another.
   static void SetInstanceForTesting(OSUserManager* instance);
 
+  // Checks if the device is domain joined.
+  virtual bool IsDeviceDomainJoined();
+
  protected:
   OSUserManager() {}
 

chrome/credential_provider/gaiacp/win_http_url_fetcher.cc

@@ -69,6 +69,12 @@ HRESULT WinHttpUrlFetcher::SetRequestBody(const char* body) {
   return S_OK;
 }
 
+HRESULT WinHttpUrlFetcher::SetHttpRequestTimeout(const int timeout_in_millis) {
+  DCHECK(timeout_in_millis);
+  timeout_in_millis_ = timeout_in_millis;
+  return S_OK;
+}
+
 HRESULT WinHttpUrlFetcher::Fetch(std::vector<char>* response) {
   USES_CONVERSION;
   DCHECK(response);
@@ -93,6 +99,19 @@ HRESULT WinHttpUrlFetcher::Fetch(std::vector<char>* response) {
     connect.Set(connect_tmp);
   }
 
+  {
+    // Set timeout if specified.
+    if (timeout_in_millis_ != 0) {
+      if (!::WinHttpSetTimeouts(session_.Get(), timeout_in_millis_,
+                                timeout_in_millis_, timeout_in_millis_,
+                                timeout_in_millis_)) {
+        HRESULT hr = HRESULT_FROM_WIN32(::GetLastError());
+        LOGFN(ERROR) << "WinHttpSetTimeouts hr=" << putHR(hr);
+        return hr;
+      }
+    }
+  }
+
   {
     bool use_post = !body_.empty();
     ScopedWinHttpHandle::Handle request = ::WinHttpOpenRequest(

chrome/credential_provider/gaiacp/win_http_url_fetcher.h

@@ -27,6 +27,7 @@ class WinHttpUrlFetcher {
 
   virtual HRESULT SetRequestHeader(const char* name, const char* value);
   virtual HRESULT SetRequestBody(const char* body);
+  virtual HRESULT SetHttpRequestTimeout(const int timeout_in_millis);
   virtual HRESULT Fetch(std::vector<char>* response);
   virtual HRESULT Close();
 
@@ -48,6 +49,7 @@ class WinHttpUrlFetcher {
   std::string body_;
   ScopedWinHttpHandle session_;
   ScopedWinHttpHandle request_;
+  int timeout_in_millis_ = 0;
 
   // Gets storage of the function pointer used to create instances of this
   // class for tests.

chrome/credential_provider/test/gcp_fakes.cc

@@ -141,6 +141,7 @@ HRESULT FakeOSUserManager::AddUser(const wchar_t* username,
   return AddUser(username, password, fullname, comment, add_to_users_group,
                  OSUserManager::GetLocalDomain().c_str(), sid, error);
 }
+
 HRESULT FakeOSUserManager::AddUser(const wchar_t* username,
                                    const wchar_t* password,
                                    const wchar_t* fullname,
@@ -159,6 +160,11 @@ HRESULT FakeOSUserManager::AddUser(const wchar_t* username,
   if (should_fail_user_creation_)
     return E_FAIL;
 
+  // Username or password cannot be empty.
+  if (username == nullptr || !username[0] || password == nullptr ||
+      !password[0])
+    return E_FAIL;
+
   bool user_found = username_to_info_.count(username) > 0;
 
   if (user_found) {
@@ -271,6 +277,10 @@ HRESULT FakeOSUserManager::CreateLogonToken(const wchar_t* domain,
   return token->IsValid() ? S_OK : HRESULT_FROM_WIN32(::GetLastError());
 }
 
+bool FakeOSUserManager::IsDeviceDomainJoined() {
+  return is_device_domain_joined_;
+}
+
 HRESULT FakeOSUserManager::GetUserSID(const wchar_t* domain,
                                       const wchar_t* username,
                                       PSID* sid) {
@@ -579,17 +589,30 @@ void FakeWinHttpUrlFetcherFactory::SetFakeResponse(
       Response(headers, response, send_response_event_handle);
 }
 
+void FakeWinHttpUrlFetcherFactory::SetFakeFailedResponse(const GURL& url,
+                                                         HRESULT failed_hr) {
+  // Make sure that the HRESULT set is a failed attempt.
+  DCHECK(FAILED(failed_hr));
+  failed_http_fetch_hr_[url] = failed_hr;
+}
+
 std::unique_ptr<WinHttpUrlFetcher> FakeWinHttpUrlFetcherFactory::Create(
     const GURL& url) {
-  if (fake_responses_.count(url) == 0)
+  if (fake_responses_.count(url) == 0 && failed_http_fetch_hr_.count(url) == 0)
     return nullptr;
 
-  const Response& response = fake_responses_[url];
-
   FakeWinHttpUrlFetcher* fetcher = new FakeWinHttpUrlFetcher(std::move(url));
-  fetcher->response_headers_ = response.headers;
-  fetcher->response_ = response.response;
-  fetcher->send_response_event_handle_ = response.send_response_event_handle;
+
+  if (fake_responses_.count(url) != 0) {
+    const Response& response = fake_responses_[url];
+
+    fetcher->response_headers_ = response.headers;
+    fetcher->response_ = response.response;
+    fetcher->send_response_event_handle_ = response.send_response_event_handle;
+  } else {
+    DCHECK(failed_http_fetch_hr_.count(url) > 0);
+    fetcher->response_hr_ = failed_http_fetch_hr_[url];
+  }
   ++requests_created_;
 
   return std::unique_ptr<WinHttpUrlFetcher>(fetcher);
@@ -605,6 +628,9 @@ bool FakeWinHttpUrlFetcher::IsValid() const {
 }
 
 HRESULT FakeWinHttpUrlFetcher::Fetch(std::vector<char>* response) {
+  if (FAILED(response_hr_))
+    return response_hr_;
+
   if (send_response_event_handle_ != INVALID_HANDLE_VALUE)
     ::WaitForSingleObject(send_response_event_handle_, INFINITE);
 

chrome/credential_provider/test/gcp_fakes.h

@@ -69,6 +69,15 @@ class FakeOSUserManager : public OSUserManager {
                   bool add_to_users_group,
                   BSTR* sid,
                   DWORD* error) override;
+  // Add a user to the OS with domain associated with it.
+  HRESULT AddUser(const wchar_t* username,
+                  const wchar_t* password,
+                  const wchar_t* fullname,
+                  const wchar_t* comment,
+                  bool add_to_users_group,
+                  const wchar_t* domain,
+                  BSTR* sid,
+                  DWORD* error);
   HRESULT ChangeUserPassword(const wchar_t* domain,
                              const wchar_t* username,
                              const wchar_t* password,
@@ -103,10 +112,16 @@ class FakeOSUserManager : public OSUserManager {
                                          const wchar_t* username,
                                          bool allow) override;
 
+  bool IsDeviceDomainJoined() override;
+
   void SetShouldFailUserCreation(bool should_fail) {
     should_fail_user_creation_ = should_fail;
   }
 
+  void SetIsDeviceDomainJoined(bool is_device_domain_joined) {
+    is_device_domain_joined_ = is_device_domain_joined;
+  }
+
   struct UserInfo {
     UserInfo(const wchar_t* domain,
              const wchar_t* password,
@@ -165,16 +180,7 @@ class FakeOSUserManager : public OSUserManager {
   DWORD next_rid_ = 0;
   std::map<base::string16, UserInfo> username_to_info_;
   bool should_fail_user_creation_ = false;
-
-  // Add a user to the OS with domain associated with it.
-  HRESULT AddUser(const wchar_t* username,
-                  const wchar_t* password,
-                  const wchar_t* fullname,
-                  const wchar_t* comment,
-                  bool add_to_users_group,
-                  const wchar_t* domain,
-                  BSTR* sid,
-                  DWORD* error);
+  bool is_device_domain_joined_ = false;
 };
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -273,6 +279,12 @@ class FakeWinHttpUrlFetcherFactory {
       const WinHttpUrlFetcher::Headers& headers,
       const std::string& response,
       HANDLE send_response_event_handle = INVALID_HANDLE_VALUE);
+
+  // Sets the response as a failed http attempt. The return result
+  // from http_url_fetcher.Fetch() would be set as the input HRESULT
+  // to this method.
+  void SetFakeFailedResponse(const GURL& url, HRESULT failed_hr);
+
   size_t requests_created() const { return requests_created_; }
 
  private:
@@ -293,6 +305,7 @@ class FakeWinHttpUrlFetcherFactory {
   };
 
   std::map<GURL, Response> fake_responses_;
+  std::map<GURL, HRESULT> failed_http_fetch_hr_;
   size_t requests_created_ = 0;
 };
 
@@ -316,6 +329,7 @@ class FakeWinHttpUrlFetcher : public WinHttpUrlFetcher {
   Headers response_headers_;
   std::string response_;
   HANDLE send_response_event_handle_;
+  HRESULT response_hr_ = S_OK;
 };
 
 ///////////////////////////////////////////////////////////////////////////////