[GCPW] Take custom gls_path into account when checking for Chrome availability.

Bug: 1054976
Change-Id: I1fdb624c49f7901186b552f835a77b88b1e6ee76
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2068919
Commit-Queue: Robin Lewis <wrlewis@google.com>
Reviewed-by: Yusuf Sengul <yusufsn@google.com>
Cr-Commit-Position: refs/heads/master@{#743757}

chrome/credential_provider/gaiacp/chrome_availability_checker.cc

@@ -13,19 +13,21 @@ namespace {
 
 bool IsSupportedChromeVersionInstalled() {
   // Check if Chrome is installed on this machine.
-  base::FilePath gls_path =
-      chrome_launcher_support::GetChromePathForInstallationLevel(
-          chrome_launcher_support::SYSTEM_LEVEL_INSTALLATION, false);
+  base::FilePath gls_path = GetChromePath();
   if (gls_path.empty()) {
     return false;
   }
 
-  // Check if Chrome version is supported.
+  // Check if the Chrome version is supported only if we are using a system
+  // installed Chrome since version number is read from the registry.
+  base::FilePath system_chrome_path = GetSystemChromePath();
   base::Version chrome_version =
       chrome_launcher_support::GetChromeVersionForInstallationLevel(
           chrome_launcher_support::SYSTEM_LEVEL_INSTALLATION, false);
-  if (!chrome_version.IsValid() ||
-      chrome_version < GetMinimumSupportedChromeVersion()) {
+
+  if (gls_path == system_chrome_path &&
+      (!chrome_version.IsValid() ||
+       chrome_version < GetMinimumSupportedChromeVersion())) {
     return false;
   }
   return true;

chrome/credential_provider/gaiacp/gaia_credential_base.cc

@@ -1066,21 +1066,6 @@ HRESULT CGaiaCredentialBase::GetBaseGlsCommandline(
 
   base::FilePath gls_path = GetChromePath();
 
-  constexpr wchar_t kGlsPath[] = L"gls_path";
-
-  wchar_t custom_gls_path_value[MAX_PATH];
-  ULONG path_len = base::size(custom_gls_path_value);
-  HRESULT hr = GetGlobalFlag(kGlsPath, custom_gls_path_value, &path_len);
-  if (SUCCEEDED(hr)) {
-    base::FilePath custom_gls_path(custom_gls_path_value);
-    if (base::PathExists(custom_gls_path)) {
-      gls_path = custom_gls_path;
-    } else {
-      LOGFN(ERROR) << "Specified gls path ('" << custom_gls_path.value()
-                   << "') does not exist, using default gls path.";
-    }
-  }
-
   if (gls_path.empty()) {
     LOGFN(ERROR) << "No path to chrome.exe could be found.";
     return HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND);

chrome/credential_provider/gaiacp/gaia_credential_base_unittests.cc

@@ -7,6 +7,7 @@
 #include <sddl.h>  // For ConvertSidToStringSid()
 #include <wrl/client.h>
 
+#include "base/files/scoped_temp_dir.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
@@ -2907,5 +2908,60 @@ INSTANTIATE_TEST_SUITE_P(
     ::testing::Combine(::testing::Values(true, false),
                        ::testing::Values(0, 2, 1000, 3000)));
 
+// Test if the credential can be created successfully depending on whether a
+// Chrome path is found.
+// Parameters are:
+// 1. bool  true:  A Chrome path is set.
+//          false: No Chrome path set.
+class GcpGaiaCredentialBaseChromeAvailabilityTest
+    : public GcpGaiaCredentialBaseTest,
+      public ::testing::WithParamInterface<bool> {};
+
+TEST_P(GcpGaiaCredentialBaseChromeAvailabilityTest, CustomChromeSpecified) {
+  // Simulate a custom Chrome path being set.
+  fake_chrome_checker()->SetHasSupportedChrome(
+      FakeChromeAvailabilityChecker::kChromeDontForce);
+
+  bool custom_path_set = GetParam();
+  base::ScopedTempDir temp_chrome_path;
+
+  // Set system Chrome path to empty so that we are not influenced by the
+  // runtime environment.
+  GoogleChromePathForTesting google_chrome_path_for_testing(
+      base::FilePath(L""));
+
+  if (custom_path_set) {
+    ASSERT_TRUE(temp_chrome_path.CreateUniqueTempDir());
+    ASSERT_EQ(S_OK,
+              SetGlobalFlagForTesting(
+                  kRegGlsPath, temp_chrome_path.GetPath().AsUTF16Unsafe()));
+  }
+
+  USES_CONVERSION;
+  // Create a fake user that has the same gaia id as the test gaia id.
+  CComBSTR sid;
+  base::string16 username(L"foo");
+  ASSERT_EQ(S_OK,
+            fake_os_user_manager()->CreateTestOSUser(
+                username, L"password", L"name", L"comment",
+                base::UTF8ToUTF16(kDefaultGaiaId), base::string16(), &sid));
+  ASSERT_EQ(2ul, fake_os_user_manager()->GetUserCount());
+
+  // Create provider.
+  Microsoft::WRL::ComPtr<ICredentialProviderCredential> cred;
+
+  if (custom_path_set) {
+    // Don't fail to create the credential.
+    ASSERT_EQ(S_OK, InitializeProviderAndGetCredential(0, &cred));
+  } else {
+    // Credential creation should fail as no chrome will be found.
+    ASSERT_EQ(E_FAIL, InitializeProviderAndGetCredential(0, &cred));
+  }
+}
+
+INSTANTIATE_TEST_SUITE_P(All,
+                         GcpGaiaCredentialBaseChromeAvailabilityTest,
+                         ::testing::Values(true, false));
+
 }  // namespace testing
 }  // namespace credential_provider

chrome/credential_provider/gaiacp/gcp_utils.cc

@@ -49,6 +49,7 @@
 #include "chrome/credential_provider/common/gcp_strings.h"
 #include "chrome/credential_provider/gaiacp/gaia_resources.h"
 #include "chrome/credential_provider/gaiacp/logging.h"
+#include "chrome/credential_provider/gaiacp/mdm_utils.h"
 #include "chrome/credential_provider/gaiacp/reg_utils.h"
 #include "chrome/installer/launcher_support/chrome_launcher_support.h"
 #include "google_apis/gaia/gaia_auth_util.h"
@@ -980,6 +981,25 @@ HRESULT SetGaiaEndpointCommandLineIfNeeded(const wchar_t* override_registry_key,
 }
 
 base::FilePath GetChromePath() {
+  base::FilePath gls_path = GetSystemChromePath();
+
+  wchar_t custom_gls_path_value[MAX_PATH];
+  ULONG path_len = base::size(custom_gls_path_value);
+  HRESULT hr = GetGlobalFlag(kRegGlsPath, custom_gls_path_value, &path_len);
+  if (SUCCEEDED(hr)) {
+    base::FilePath custom_gls_path(custom_gls_path_value);
+    if (base::PathExists(custom_gls_path)) {
+      gls_path = custom_gls_path;
+    } else {
+      LOGFN(ERROR) << "Specified gls path ('" << custom_gls_path.value()
+                   << "') does not exist, using default gls path.";
+    }
+  }
+
+  return gls_path;
+}
+
+base::FilePath GetSystemChromePath() {
   if (g_use_test_chrome_path)
     return g_test_chrome_path;
 

chrome/credential_provider/gaiacp/gcp_utils.h

@@ -345,6 +345,10 @@ HRESULT SetGaiaEndpointCommandLineIfNeeded(const wchar_t* override_registry_key,
 
 // Returns the file path to installed chrome.exe.
 base::FilePath GetChromePath();
+
+// Returns the file path to system installed chrome.exe.
+base::FilePath GetSystemChromePath();
+
 }  // namespace credential_provider
 
 #endif  // CHROME_CREDENTIAL_PROVIDER_GAIACP_GCP_UTILS_H_

chrome/credential_provider/gaiacp/mdm_utils.cc

@@ -41,6 +41,7 @@ constexpr wchar_t kRegMdmSupportsMultiUser[] = L"mdm_mu";
 constexpr wchar_t kRegMdmAllowConsumerAccounts[] = L"mdm_aca";
 constexpr wchar_t kRegDeviceDetailsUploadStatus[] =
     L"device_details_upload_status";
+constexpr wchar_t kRegGlsPath[] = L"gls_path";
 constexpr wchar_t kUserPasswordLsaStoreKeyPrefix[] =
 #if BUILDFLAG(GOOGLE_CHROME_BRANDING)
     L"Chrome-GCPW-";

chrome/credential_provider/gaiacp/mdm_utils.h

@@ -49,6 +49,9 @@ extern const char kErrorKeyInRequestResult[];
 // Upload status for device details.
 extern const wchar_t kRegDeviceDetailsUploadStatus[];
 
+// Specifies custom Chrome path to use for GLS.
+extern const wchar_t kRegGlsPath[];
+
 // Class used in tests to force either a successful on unsuccessful enrollment
 // to google MDM.
 class GoogleMdmEnrollmentStatusForTesting {

chrome/credential_provider/test/gcp_fakes.cc

@@ -763,6 +763,9 @@ FakeChromeAvailabilityChecker::~FakeChromeAvailabilityChecker() {
 }
 
 bool FakeChromeAvailabilityChecker::HasSupportedChromeVersion() {
+  if (has_supported_chrome_ == kChromeDontForce) {
+    return original_checker_->HasSupportedChromeVersion();
+  }
   return has_supported_chrome_ == kChromeForceYes;
 }
 

chrome/credential_provider/test/gcp_fakes.h

@@ -397,7 +397,11 @@ class FakeAssociatedUserValidator : public AssociatedUserValidator {
 
 class FakeChromeAvailabilityChecker : public ChromeAvailabilityChecker {
  public:
-  enum HasSupportedChromeCheckType { kChromeForceYes, kChromeForceNo };
+  enum HasSupportedChromeCheckType {
+    kChromeForceYes,
+    kChromeForceNo,
+    kChromeDontForce  // Uses the original checker to get result.
+  };
 
   FakeChromeAvailabilityChecker(
       HasSupportedChromeCheckType has_supported_chrome = kChromeForceYes);