Enable password recovery and multi user support by default

Bug: 992616
Change-Id: I6e0ebdc646766479699c9007c781c286c2ead96a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1755055
Commit-Queue: Yusuf Sengul <yusufsn@google.com>
Reviewed-by: Tien Mai <tienmai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690092}

chrome/credential_provider/gaiacp/associated_user_validator_unittests.cc

@@ -357,6 +357,8 @@ class AssociatedUserValidatorUserAccessBlockingTest
     : public AssociatedUserValidatorTest,
       public ::testing::WithParamInterface<
           std::tuple<CREDENTIAL_PROVIDER_USAGE_SCENARIO,
+                     bool,
+                     bool,
                      bool,
                      bool,
                      bool,
@@ -371,8 +373,14 @@ TEST_P(AssociatedUserValidatorUserAccessBlockingTest, BlockUserAccessAsNeeded) {
   const bool mdm_url_set = std::get<2>(GetParam());
   const bool mdm_enrolled = std::get<3>(GetParam());
   const bool internet_available = std::get<4>(GetParam());
+  const bool password_recovery_enabled = std::get<5>(GetParam());
+  const bool contains_stored_password = std::get<6>(GetParam());
   GoogleMdmEnrolledStatusForTesting forced_status(mdm_enrolled);
 
+#if !defined(GOOGLE_CHROME_BUILD)
+  GoogleMdmEscrowServiceEnablerForTesting escrow_service_enabler(true);
+#endif
+
   FakeAssociatedUserValidator validator;
   fake_internet_checker()->SetHasInternetConnection(
       internet_available ? FakeInternetAvailabilityChecker::kHicForceYes
@@ -381,6 +389,11 @@ TEST_P(AssociatedUserValidatorUserAccessBlockingTest, BlockUserAccessAsNeeded) {
   if (mdm_url_set)
     ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmUrl, L"https://mdm.com"));
 
+  if (password_recovery_enabled) {
+    ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmEscrowServiceServerUrl,
+                                            L"https://escrow.com"));
+  }
+
   bool should_user_locking_be_enabled =
       internet_available && mdm_url_set &&
       CGaiaCredentialProvider::IsUsageScenarioSupported(cpus);
@@ -394,6 +407,14 @@ TEST_P(AssociatedUserValidatorUserAccessBlockingTest, BlockUserAccessAsNeeded) {
                       username, L"password", L"fullname", L"comment",
                       L"gaia-id", base::string16(), &sid));
 
+  if (contains_stored_password) {
+    base::string16 store_key = GetUserPasswordLsaStoreKey(OLE2W(sid));
+    auto policy = ScopedLsaPolicy::Create(POLICY_ALL_ACCESS);
+    EXPECT_TRUE(SUCCEEDED(
+        policy->StorePrivateData(store_key.c_str(), L"encrypted_data")));
+    EXPECT_TRUE(policy->PrivateDataExists(store_key.c_str()));
+  }
+
   // Token handle fetch result.
   fake_http_url_fetcher_factory()->SetFakeResponse(
       GURL(AssociatedUserValidator::kTokenInfoUrl),
@@ -406,10 +427,14 @@ TEST_P(AssociatedUserValidatorUserAccessBlockingTest, BlockUserAccessAsNeeded) {
   DWORD reg_value = 0;
 
   bool should_user_be_blocked =
-      should_user_locking_be_enabled && (!mdm_enrolled || !token_handle_valid);
+      should_user_locking_be_enabled &&
+      (!mdm_enrolled || !token_handle_valid ||
+       (password_recovery_enabled && !contains_stored_password));
 
   EXPECT_EQ(!internet_available || (!mdm_url_set && token_handle_valid) ||
-                (mdm_url_set && mdm_enrolled && token_handle_valid),
+                (mdm_url_set && mdm_enrolled && token_handle_valid &&
+                 (!password_recovery_enabled ||
+                  password_recovery_enabled && contains_stored_password)),
             validator.IsTokenHandleValidForUser(OLE2W(sid)));
   EXPECT_EQ(should_user_be_blocked,
             validator.IsUserAccessBlockedForTesting(OLE2W(sid)));
@@ -433,6 +458,8 @@ INSTANTIATE_TEST_SUITE_P(
                        ::testing::Bool(),
                        ::testing::Bool(),
                        ::testing::Bool(),
+                       ::testing::Bool(),
+                       ::testing::Bool(),
                        ::testing::Bool()));
 
 class TimeClockOverrideValue {

chrome/credential_provider/gaiacp/gaia_credential_base_unittests.cc

@@ -1286,6 +1286,7 @@ TEST_P(GcpGaiaCredentialBasePasswordRecoveryTest, PasswordRecovery) {
   ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmEscrowServiceServerUrl,
                                           L"https://escrow.com"));
   ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmAllowConsumerAccounts, 1));
+  ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmSupportsMultiUser, 0));
 
   GoogleMdmEnrolledStatusForTesting force_success(true);
 
@@ -1504,6 +1505,12 @@ TEST_P(GcpGaiaCredentialBasePasswordRecoveryDisablingTest,
 
   ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmUrl, L"https://mdm.com"));
   ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmAllowConsumerAccounts, 1));
+  ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmSupportsMultiUser, 0));
+  // SetGlobalFlagForTesting effectively deletes the registry when the provided
+  // registry value is empty. That implicitly enables escrow service without a
+  // registry override.
+  ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmEscrowServiceServerUrl, L""));
+
   if (escrow_service_url) {
     base::win::RegKey key;
     ASSERT_EQ(ERROR_SUCCESS,
@@ -1585,9 +1592,9 @@ TEST_P(GcpGaiaCredentialBasePasswordRecoveryDisablingTest,
     CComPtr<ITestCredentialProvider> test_provider;
     ASSERT_EQ(S_OK, created_provider().QueryInterface(&test_provider));
 
-    // Null or empty escrow service url will disable password
+    // Empty escrow service url will disable password
     // recovery and force the user to enter their password.
-    if (!escrow_service_url || escrow_service_url[0] == '\0') {
+    if (escrow_service_url && escrow_service_url[0] == '\0') {
       // Logon should not complete but there is no error message.
       EXPECT_EQ(test_provider->credentials_changed_fired(), false);
 

chrome/credential_provider/gaiacp/gaia_credential_provider.cc

@@ -580,19 +580,8 @@ bool CGaiaCredentialProvider::CanNewUsersBeCreated(
   if (cpus == CPUS_UNLOCK_WORKSTATION)
     return false;
 
-  // If MDM enrollment is required and multiple users is not supported, only
-  // allow a new associated user to be created if there does not yet exist an
-  // OS user created from a Google account.
-  if (MdmEnrollmentEnabled()) {
-    DWORD multi_user_supported = 0;
-    HRESULT hr = GetGlobalFlag(kRegMdmSupportsMultiUser, &multi_user_supported);
-    if (FAILED(hr) || multi_user_supported == 0) {
-      if (AssociatedUserValidator::Get()->GetAssociatedUsersCount() > 0)
-        return false;
-    }
-  }
-
-  return true;
+  return GetGlobalFlagOrDefault(kRegMdmSupportsMultiUser, 1) ||
+         !AssociatedUserValidator::Get()->GetAssociatedUsersCount();
 }
 
 // ICredentialUpdateEventsHandler //////////////////////////////////////////////

chrome/credential_provider/gaiacp/gaia_credential_provider_unittests.cc

@@ -274,11 +274,9 @@ TEST_F(GcpCredentialProviderTest, AutoLogonBeforeUserRefresh) {
 }
 
 TEST_F(GcpCredentialProviderTest, AddPersonAfterUserRemove) {
-  // Set up such that MDM is enabled, mulit-users is not, and a user already
+  // Set up such that multi-users is not enabled, and a user already
   // exists.
-  ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmUrl, L"https://mdm.com"));
   ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmSupportsMultiUser, 0));
-  GoogleMdmEnrolledStatusForTesting forced_status(true);
 
   const wchar_t kDummyUsername[] = L"username";
   const wchar_t kDummyPassword[] = L"password";
@@ -369,8 +367,7 @@ TEST_P(GcpCredentialProviderSetSerializationTest, CheckAutoLogon) {
   const bool valid_token_handles = std::get<0>(GetParam());
   const CREDENTIAL_PROVIDER_USAGE_SCENARIO cpus = std::get<1>(GetParam());
 
-  ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmUrl, L"https://mdm.com"));
-  GoogleMdmEnrolledStatusForTesting forced_status(true);
+  ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmSupportsMultiUser, 0));
 
   CComBSTR first_sid;
   constexpr wchar_t first_username[] = L"username";
@@ -434,61 +431,6 @@ INSTANTIATE_TEST_SUITE_P(
     ::testing::Combine(::testing::Bool(),
                        ::testing::Values(CPUS_UNLOCK_WORKSTATION, CPUS_LOGON)));
 
-// Tests the effect of the MDM settings on the credential provider.
-// Parameters:
-//    bool: whether the MDM URL is configured.
-//    int: whether multi-users is supported:
-//        0: set registry to 0
-//        1: set registry to 1
-//        2: don't set at all
-//    bool: whether an existing user exists.
-class GcpCredentialProviderMdmTest
-    : public GcpCredentialProviderTest,
-      public ::testing::WithParamInterface<std::tuple<bool, int, bool>> {};
-
-TEST_P(GcpCredentialProviderMdmTest, Basic) {
-  const bool config_mdm_url = std::get<0>(GetParam());
-  const int supports_multi_users = std::get<1>(GetParam());
-  const bool user_exists = std::get<2>(GetParam());
-  const DWORD expected_credential_count =
-      config_mdm_url && supports_multi_users != 1 && user_exists ? 0 : 1;
-
-  bool mdm_enrolled = false;
-  if (config_mdm_url) {
-    ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmUrl, L"https://mdm.com"));
-    mdm_enrolled = true;
-  }
-
-  GoogleMdmEnrolledStatusForTesting forced_status(mdm_enrolled);
-
-  if (supports_multi_users != 2) {
-    ASSERT_EQ(S_OK, SetGlobalFlagForTesting(kRegMdmSupportsMultiUser,
-                                            supports_multi_users));
-  }
-
-  if (user_exists) {
-    CComBSTR sid;
-    ASSERT_EQ(S_OK, fake_os_user_manager()->CreateTestOSUser(
-                        L"username", L"password", L"full name", L"comment",
-                        L"gaia-id", L"foo@gmail.com", &sid));
-  }
-
-  CComPtr<ICredentialProviderCredential> cred;
-  CComPtr<ICredentialProvider> provider;
-  DWORD count = 0;
-  ASSERT_EQ(S_OK, InitializeProviderWithCredentials(&count, &provider));
-
-  ASSERT_EQ(expected_credential_count, count);
-
-  // Deactivate the CP.
-  ASSERT_EQ(S_OK, provider->UnAdvise());
-}
-
-INSTANTIATE_TEST_SUITE_P(GcpCredentialProviderMdmTest,
-                         GcpCredentialProviderMdmTest,
-                         ::testing::Combine(::testing::Bool(),
-                                            ::testing::Range(0, 3),
-                                            ::testing::Bool()));
 
 // Check that reauth credentials only exist when the token handle for the
 // associated user is no longer valid and internet is available.

chrome/credential_provider/gaiacp/mdm_utils.cc

@@ -71,10 +71,8 @@ namespace {
 constexpr wchar_t kDefaultMdmUrl[] =
     L"https://deviceenrollmentforwindows.googleapis.com/v1/discovery";
 
-// TODO(crbug.com/973115): Empty escrow service url will implicitly disable the
-// feature. It can be enabled by setting kRegMdmEscrowServiceServerUrl. When the
-// feature is ready, this url should be updated to production endpoint.
-constexpr wchar_t kDefaultEscrowServiceServerUrl[] = L"";
+constexpr wchar_t kDefaultEscrowServiceServerUrl[] =
+    L"https://devicepasswordescrowforwindows-pa.googleapis.com";
 
 template <typename T>
 T GetMdmFunctionPointer(const base::ScopedNativeLibrary& library,

chrome/credential_provider/test/gcp_fakes.cc

@@ -53,6 +53,7 @@ void InitializeRegistryOverrideForTesting(
   ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(kRegMdmUrl, L""));
   ASSERT_EQ(ERROR_SUCCESS,
             SetMachineGuidForTesting(L"f418a124-4d92-469b-afa5-0f8af537b965"));
+  ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(kRegMdmEscrowServiceServerUrl, L""));
 }
 
 ///////////////////////////////////////////////////////////////////////////////