[GCPW] Enforce login when user policies not found or stale.

Login enforcement is done only when cloud policies is enabled.

Bug: 1142903
Change-Id: If870ec909122a39b18af33ff95f90b49ef72a198
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2503810
Commit-Queue: Robin Lewis <wrlewis@google.com>
Reviewed-by: Yusuf Sengul <yusufsn@google.com>
Reviewed-by: Rakesh Soma <rakeshsoma@google.com>
Cr-Commit-Position: refs/heads/master@{#822695}

chrome/credential_provider/gaiacp/associated_user_validator.cc

@@ -572,6 +572,13 @@ AssociatedUserValidator::GetAuthEnforceReason(const base::string16& sid) {
         UPLOAD_DEVICE_DETAILS_FAILED;
   }
 
+  // Force user to login when policies are missing or stale.
+  if (UserPoliciesManager::Get()->CloudPoliciesEnabled() &&
+      UserPoliciesManager::Get()->IsUserPolicyStaleOrMissing(sid)) {
+    return AssociatedUserValidator::EnforceAuthReason::
+        MISSING_OR_STALE_USER_POLICIES;
+  }
+
   return AssociatedUserValidator::EnforceAuthReason::NOT_ENFORCED;
 }
 

chrome/credential_provider/gaiacp/associated_user_validator.h

@@ -105,7 +105,8 @@ class AssociatedUserValidator {
     INVALID_TOKEN_HANDLE,
     ONLINE_LOGIN_STALE,
     UPLOAD_DEVICE_DETAILS_FAILED,
-    ONLINE_LOGIN_ENFORCED
+    ONLINE_LOGIN_ENFORCED,
+    MISSING_OR_STALE_USER_POLICIES
   };
 
   // Returns the reason for enforcing authentication for the provided |sid|.

chrome/credential_provider/gaiacp/associated_user_validator_unittests.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/credential_provider/gaiacp/stdafx.h"
 
+#include "base/guid.h"
 #include "base/stl_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
@@ -470,8 +471,12 @@ INSTANTIATE_TEST_SUITE_P(All,
 // 7. bool - Password Recovery is enabled.
 // 8. bool - Contains stored password.
 // 9. bool - Last online login is stale.
-// 10. bool - Uploaded device details.
-// 11. bool - Cloud policies enabled.
+// 10. int : 0 - Device details upload failed.
+//           1 - Device details uploaded but GCPW token missing.
+//           2 - Device details uploaded along with GCPW token.
+// 11. int : 0 - Cloud policies disabled.
+//           1 - Cloud policies enabled but user policies are missing.
+//           2 - Cloud policies enabled and user policies are up to date.
 // 12. bool - Cloud policy of whether user is allowed to enroll in Mdm.
 class AssociatedUserValidatorUserAccessBlockingTest
     : public AssociatedUserValidatorTest,
@@ -485,8 +490,8 @@ class AssociatedUserValidatorUserAccessBlockingTest
                      bool,
                      bool,
                      bool,
-                     bool,
-                     bool,
+                     int,
+                     int,
                      bool>> {
  private:
   FakeScopedLsaPolicyFactory fake_scoped_lsa_policy_factory_;
@@ -510,12 +515,13 @@ TEST_P(AssociatedUserValidatorUserAccessBlockingTest, BlockUserAccessAsNeeded) {
   const bool password_recovery_enabled = std::get<6>(GetParam());
   const bool contains_stored_password = std::get<7>(GetParam());
   const bool is_last_login_stale = std::get<8>(GetParam());
-  const bool uploaded_device_details = std::get<9>(GetParam());
-  const bool cloud_policies_enabled = std::get<10>(GetParam());
+  const int upload_device_details_state = std::get<9>(GetParam());
+  const int cloud_policies_state = std::get<10>(GetParam());
   const bool user_allowed_dm_enrollment = std::get<11>(GetParam());
 
   GoogleMdmEnrolledStatusForTesting forced_status(mdm_enrolled);
-  FakeUserPoliciesManager fake_user_policies_manager(cloud_policies_enabled);
+  FakeUserPoliciesManager fake_user_policies_manager(cloud_policies_state != 0);
+  FakeTokenGenerator fake_token_generator;
 
   UserPolicies user_policies;
   user_policies.enable_dm_enrollment = user_allowed_dm_enrollment;
@@ -561,7 +567,7 @@ TEST_P(AssociatedUserValidatorUserAccessBlockingTest, BlockUserAccessAsNeeded) {
               SetUserProperty((BSTR)sid, base::UTF8ToUTF16(kKeyLastTokenValid),
                               last_token_valid_millis));
 
-    if (cloud_policies_enabled) {
+    if (cloud_policies_state == 2) {
       user_policies.validity_period_days = validity_period_in_days;
     } else {
       DWORD validity_period_in_days_dword =
@@ -584,12 +590,22 @@ TEST_P(AssociatedUserValidatorUserAccessBlockingTest, BlockUserAccessAsNeeded) {
     EXPECT_TRUE(policy->PrivateDataExists(store_key.c_str()));
   }
 
-  if (cloud_policies_enabled) {
-    fake_user_policies_manager.SetUserPolicies((BSTR)sid, user_policies);
+  if (upload_device_details_state == 2) {
+    std::string dm_token = base::GenerateGUID();
+    fake_token_generator.SetTokensForTesting({dm_token});
+    ASSERT_EQ(S_OK, GenerateGCPWDmToken((BSTR)sid));
+  }
+
+  if (cloud_policies_state > 0) {
+    if (cloud_policies_state == 1) {
+      fake_user_policies_manager.SetUserPolicyStaleOrMissing((BSTR)sid, true);
+    } else {
+      fake_user_policies_manager.SetUserPolicies((BSTR)sid, user_policies);
+    }
   }
 
   ASSERT_EQ(S_OK, SetUserProperty((BSTR)sid, kRegDeviceDetailsUploadStatus,
-                                  uploaded_device_details ? 1 : 0));
+                                  (upload_device_details_state > 0) ? 1 : 0));
 
   // Remove all user properties associated with the sid if the
   // user isn't associated.
@@ -607,10 +623,18 @@ TEST_P(AssociatedUserValidatorUserAccessBlockingTest, BlockUserAccessAsNeeded) {
 
   DWORD reg_value = 0;
 
+  bool uploaded_device_details = upload_device_details_state > 0;
   bool mdm_enrollment_required = (mdm_url_set && !mdm_enrolled);
-  if (cloud_policies_enabled) {
-    mdm_enrollment_required =
-        mdm_enrollment_required && user_allowed_dm_enrollment;
+  bool reauth_for_missing_policy = false;
+
+  if (cloud_policies_state > 0) {
+    uploaded_device_details = upload_device_details_state == 2;
+    if (cloud_policies_state == 1) {
+      reauth_for_missing_policy = true;
+    } else {
+      mdm_enrollment_required =
+          mdm_enrollment_required && user_allowed_dm_enrollment;
+    }
   }
 
   bool is_get_auth_enforced =
@@ -618,7 +642,7 @@ TEST_P(AssociatedUserValidatorUserAccessBlockingTest, BlockUserAccessAsNeeded) {
       ((!internet_available && is_last_login_stale) ||
        (internet_available &&
         (mdm_enrollment_required || !token_handle_valid ||
-         !uploaded_device_details ||
+         !uploaded_device_details || reauth_for_missing_policy ||
          (password_recovery_enabled && !contains_stored_password))));
 
   bool should_user_be_blocked =
@@ -652,8 +676,8 @@ INSTANTIATE_TEST_SUITE_P(
                        ::testing::Bool(),
                        ::testing::Bool(),
                        ::testing::Bool(),
-                       ::testing::Bool(),
-                       ::testing::Bool(),
+                       ::testing::Values(0, 1, 2),
+                       ::testing::Values(0, 1, 2),
                        ::testing::Bool()));
 
 // Tests auth enforcement when multiple number of device details uploads fail

chrome/credential_provider/gaiacp/gaia_credential_base.cc

@@ -2494,8 +2494,7 @@ HRESULT CGaiaCredentialBase::OnUserAuthenticated(BSTR authentication_info,
 
   base::string16 sid = OLE2CW(user_sid_);
   if (UserPoliciesManager::Get()->CloudPoliciesEnabled() &&
-      UserPoliciesManager::Get()->GetTimeDeltaSinceLastPolicyFetch(sid) >
-          kMaxTimeDeltaSinceLastUserPolicyRefresh) {
+      UserPoliciesManager::Get()->IsUserPolicyStaleOrMissing(sid)) {
     // Save gaia id since it is needed for the cloud policies server request.
     base::string16 gaia_id = GetDictString(*authentication_results_, kKeyId);
     HRESULT hr = SetUserProperty(sid, kUserId, gaia_id);

chrome/credential_provider/gaiacp/gaia_credential_base_unittests.cc

@@ -3563,13 +3563,8 @@ TEST_P(GcpGaiaCredentialBaseFetchCloudPoliciesTest, FetchAndStore) {
   base::string16 sid = OLE2W(sid_str);
 
   if (cloud_policies_enabled) {
-    base::string16 fetch_time_millis = L"0";
-    if (policy_refreshed_recently) {
-      fetch_time_millis = base::NumberToString16(
-          base::Time::Now().ToDeltaSinceWindowsEpoch().InMilliseconds());
-    }
-    ASSERT_EQ(S_OK, SetUserProperty(sid, L"last_policy_refresh_time",
-                                    fetch_time_millis));
+    fake_user_policies_manager.SetUserPolicyStaleOrMissing(
+        sid, !policy_refreshed_recently);
 
     std::string expected_response;
     if (fail_fetch_policies) {

chrome/credential_provider/gaiacp/gaia_resources.grd

@@ -152,6 +152,9 @@
       <message name="IDS_REAUTH_ONLINE_LOGIN_ENFORCED_DESCRIPTION" desc="">
         Sign in with your work account
       </message>
+      <message name="IDS_REAUTH_MISSING_POLICIES_DESCRIPTION" desc="">
+        Sign in with your work account
+      </message>
       <message name="IDS_AUTH_FID_PROVIDER_LABEL" desc="">
         Add work account
       </message>

chrome/credential_provider/gaiacp/gaia_resources_grd/IDS_REAUTH_MISSING_POLICIES_DESCRIPTION.png.sha1

+9bee28b736c85183f962e52ad3fe21b83e173552
\ No newline at end of file

chrome/credential_provider/gaiacp/mdm_utils.cc

@@ -346,7 +346,16 @@ bool UploadDeviceDetailsNeeded(const base::string16& sid) {
   DWORD status = 0;
   GetUserProperty(sid, kRegDeviceDetailsUploadStatus, &status);
 
-  if (status != 1) {
+  // GCPW token is required for ESA to communicate with the GEM backends. So
+  // enforce upload if this token is missing.
+  base::string16 gcpw_token;
+  HRESULT hr = GetGCPWDmToken(sid, &gcpw_token);
+  bool gcpw_token_upload_required = false;
+  if (UserPoliciesManager::Get()->CloudPoliciesEnabled() && FAILED(hr)) {
+    gcpw_token_upload_required = true;
+  }
+
+  if (status != 1 || gcpw_token_upload_required) {
     DWORD device_upload_failures = 1;
     GetUserProperty(sid, kRegDeviceDetailsUploadFailures,
                     &device_upload_failures);

chrome/credential_provider/gaiacp/reauth_credential.cc

@@ -179,6 +179,10 @@ HRESULT CReauthCredential::GetStringValueImpl(DWORD field_id, wchar_t** value) {
           description_label_id =
               IDS_REAUTH_ONLINE_LOGIN_ENFORCED_DESCRIPTION_BASE;
           break;
+        case AssociatedUserValidator::EnforceAuthReason::
+            MISSING_OR_STALE_USER_POLICIES:
+          description_label_id = IDS_REAUTH_MISSING_POLICIES_DESCRIPTION_BASE;
+          break;
         default:
           description_label_id = IDS_REAUTH_FID_DESCRIPTION_BASE;
           break;

chrome/credential_provider/gaiacp/user_policies_manager.cc

@@ -345,7 +345,7 @@ base::TimeDelta UserPoliciesManager::GetTimeDeltaSinceLastPolicyFetch(
 }
 
 bool UserPoliciesManager::GetUserPolicies(const base::string16& sid,
-                                          UserPolicies* user_policies) {
+                                          UserPolicies* user_policies) const {
   DCHECK(user_policies);
 
   uint32_t open_flags = base::File::FLAG_OPEN | base::File::FLAG_READ;
@@ -380,6 +380,21 @@ bool UserPoliciesManager::GetUserPolicies(const base::string16& sid,
   return true;
 }
 
+bool UserPoliciesManager::IsUserPolicyStaleOrMissing(
+    const base::string16& sid) const {
+  UserPolicies user_policies;
+  if (!GetUserPolicies(sid, &user_policies)) {
+    return true;
+  }
+
+  if (GetTimeDeltaSinceLastPolicyFetch(sid) >
+      kMaxTimeDeltaSinceLastUserPolicyRefresh) {
+    return true;
+  }
+
+  return false;
+}
+
 void UserPoliciesManager::SetCloudPoliciesEnabledForTesting(bool value) {
   g_cloud_policies_enabled = value;
 }

chrome/credential_provider/gaiacp/user_policies_manager.h

@@ -60,7 +60,11 @@ class COMPONENT_EXPORT(GCPW_POLICIES) UserPoliciesManager {
   // Retrieves the policies for the user with |sid| from local storage. Returns
   // the default user policy if policy not fetched or on any error.
   virtual bool GetUserPolicies(const base::string16& sid,
-                               UserPolicies* user_policies);
+                               UserPolicies* user_policies) const;
+
+  // Returns true if the policies are missing for the user with |sid| or if
+  // they haven't been refreshed recently.
+  virtual bool IsUserPolicyStaleOrMissing(const base::string16& sid) const;
 
   // For testing only return the status of the last policy fetch.
   HRESULT GetLastFetchStatusForTesting() const;

chrome/credential_provider/gaiacp/user_policies_manager_unittests.cc

@@ -7,6 +7,7 @@
 #include "base/base_paths_win.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/json/json_writer.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_path_override.h"
 #include "chrome/credential_provider/extension/user_device_context.h"
@@ -23,6 +24,7 @@ namespace testing {
 class GcpUserPoliciesBaseTest : public GlsRunnerTestBase {
  protected:
   void SetUp() override;
+  base::string16 CreateUser();
 };
 
 void GcpUserPoliciesBaseTest::SetUp() {
@@ -34,6 +36,16 @@ void GcpUserPoliciesBaseTest::SetUp() {
   UserPoliciesManager::Get()->SetFakesForTesting(&fakes);  // IN-TEST
 }
 
+base::string16 GcpUserPoliciesBaseTest::CreateUser() {
+  // Create a fake user associated to a gaia id.
+  CComBSTR sid_str;
+  EXPECT_EQ(S_OK, fake_os_user_manager()->CreateTestOSUser(
+                      kDefaultUsername, L"password", L"Full Name", L"comment",
+                      base::UTF8ToUTF16(kDefaultGaiaId), L"user@company.com",
+                      &sid_str));
+  return OLE2W(sid_str);
+}
+
 TEST_F(GcpUserPoliciesBaseTest, NonExistentUser) {
   ASSERT_TRUE(FAILED(UserPoliciesManager::Get()->FetchAndStoreCloudUserPolicies(
       L"not-valid-sid", "not-valid-token")));
@@ -43,13 +55,7 @@ TEST_F(GcpUserPoliciesBaseTest, NonExistentUser) {
 }
 
 TEST_F(GcpUserPoliciesBaseTest, NoAccessToken) {
-  // Create a fake user associated to a gaia id.
-  CComBSTR sid_str;
-  ASSERT_EQ(S_OK, fake_os_user_manager()->CreateTestOSUser(
-                      kDefaultUsername, L"password", L"Full Name", L"comment",
-                      base::UTF8ToUTF16(kDefaultGaiaId), L"user@company.com",
-                      &sid_str));
-  base::string16 sid = OLE2W(sid_str);
+  base::string16 sid = CreateUser();
 
   ASSERT_TRUE(FAILED(
       UserPoliciesManager::Get()->FetchAndStoreCloudUserPolicies(sid, "")));
@@ -57,6 +63,35 @@ TEST_F(GcpUserPoliciesBaseTest, NoAccessToken) {
   ASSERT_FALSE(UserPoliciesManager::Get()->GetUserPolicies(sid, &policies));
 }
 
+TEST_F(GcpUserPoliciesBaseTest, DetectMissingAndStalePolicies) {
+  base::string16 sid = CreateUser();
+  ASSERT_TRUE(UserPoliciesManager::Get()->IsUserPolicyStaleOrMissing(sid));
+
+  UserPolicies policies;
+  base::Value expected_response_value(base::Value::Type::DICTIONARY);
+  expected_response_value.SetKey("policies", policies.ToValue());
+  std::string expected_response;
+  base::JSONWriter::Write(expected_response_value, &expected_response);
+
+  fake_http_url_fetcher_factory()->SetFakeResponse(
+      UserPoliciesManager::Get()->GetGcpwServiceUserPoliciesUrl(sid),
+      FakeWinHttpUrlFetcher::Headers(), expected_response);
+
+  ASSERT_TRUE(
+      SUCCEEDED(UserPoliciesManager::Get()->FetchAndStoreCloudUserPolicies(
+          sid, "access_token")));
+
+  ASSERT_FALSE(UserPoliciesManager::Get()->IsUserPolicyStaleOrMissing(sid));
+
+  base::string16 fetch_time_millis = base::NumberToString16(
+      base::Time::Now().ToDeltaSinceWindowsEpoch().InMilliseconds() -
+      kMaxTimeDeltaSinceLastUserPolicyRefresh.InMilliseconds() - 1);
+
+  ASSERT_EQ(S_OK, SetUserProperty(sid, L"last_policy_refresh_time",
+                                  fetch_time_millis));
+  ASSERT_TRUE(UserPoliciesManager::Get()->IsUserPolicyStaleOrMissing(sid));
+}
+
 // Tests effective user policy under various scenarios of cloud policy values.
 // Params:
 // bool : Whether device management enabled.
@@ -87,13 +122,7 @@ void GcpUserPoliciesFetchAndReadTest::SetUp() {
   policies_.enable_multi_user_login = std::get<3>(GetParam());
   policies_.validity_period_days = std::get<4>(GetParam());
 
-  // Create a fake user associated to a gaia id.
-  CComBSTR sid;
-  ASSERT_EQ(S_OK,
-            fake_os_user_manager()->CreateTestOSUser(
-                kDefaultUsername, L"password", L"Full Name", L"comment",
-                base::UTF8ToUTF16(kDefaultGaiaId), L"user@company.com", &sid));
-  sid_ = OLE2W(sid);
+  sid_ = CreateUser();
 
   // Remove the mdm_url value which exists by default as it's added in
   // InitializeRegistryOverrideForTesting and set to an empty value disabling
@@ -152,6 +181,7 @@ TEST_P(GcpUserPoliciesFetchAndReadTest, CloudPoliciesWin) {
   UserPolicies policies_fetched;
   ASSERT_TRUE(
       UserPoliciesManager::Get()->GetUserPolicies(sid_, &policies_fetched));
+  ASSERT_FALSE(UserPoliciesManager::Get()->IsUserPolicyStaleOrMissing(sid_));
 
   ASSERT_EQ(policies_, policies_fetched);
 }

chrome/credential_provider/test/gcp_fakes.cc

@@ -1198,18 +1198,33 @@ HRESULT FakeUserPoliciesManager::FetchAndStoreCloudUserPolicies(
 void FakeUserPoliciesManager::SetUserPolicies(const base::string16& sid,
                                               const UserPolicies& policies) {
   user_policies_[sid] = policies;
+  user_policies_stale_[sid] = false;
 }
 
 bool FakeUserPoliciesManager::GetUserPolicies(const base::string16& sid,
-                                              UserPolicies* policies) {
+                                              UserPolicies* policies) const {
   if (user_policies_.find(sid) != user_policies_.end()) {
-    *policies = user_policies_[sid];
+    *policies = user_policies_.at(sid);
     return true;
   }
 
   return false;
 }
 
+void FakeUserPoliciesManager::SetUserPolicyStaleOrMissing(
+    const base::string16& sid,
+    bool status) {
+  user_policies_stale_[sid] = status;
+}
+
+bool FakeUserPoliciesManager::IsUserPolicyStaleOrMissing(
+    const base::string16& sid) const {
+  if (user_policies_stale_.find(sid) != user_policies_stale_.end())
+    return user_policies_stale_.at(sid);
+
+  return true;
+}
+
 int FakeUserPoliciesManager::GetNumTimesFetchAndStoreCalled() const {
   return num_times_fetch_called_;
 }

chrome/credential_provider/test/gcp_fakes.h

@@ -619,7 +619,12 @@ class FakeUserPoliciesManager : public UserPoliciesManager {
   void SetUserPolicies(const base::string16& sid, const UserPolicies& policies);
 
   bool GetUserPolicies(const base::string16& sid,
-                       UserPolicies* policies) override;
+                       UserPolicies* policies) const override;
+
+  // Specify whether user policy is valid for a user.
+  void SetUserPolicyStaleOrMissing(const base::string16& sid, bool status);
+
+  bool IsUserPolicyStaleOrMissing(const base::string16& sid) const override;
 
   // Returns the number of times FetchAndStoreCloudUserPolicies method was
   // called.
@@ -629,6 +634,7 @@ class FakeUserPoliciesManager : public UserPoliciesManager {
   UserPoliciesManager* original_manager_ = nullptr;
   std::map<base::string16, UserPolicies> user_policies_;
   int num_times_fetch_called_ = 0;
+  std::map<base::string16, bool> user_policies_stale_;
 };
 
 ///////////////////////////////////////////////////////////////////////////////