Replace time restrictions with deny interactive logon type instead.

Bug: 1091161
Change-Id: I1ea622b856df81f4bf3c302a0399a39adb9d588e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2230919
Commit-Queue: Rakesh Soma <rakeshsoma@google.com>
Reviewed-by: Yusuf Sengul <yusufsn@google.com>
Cr-Commit-Position: refs/heads/master@{#775704}

chrome/credential_provider/gaiacp/associated_user_validator.cc

@@ -139,7 +139,28 @@ HRESULT ModifyUserAccess(const std::unique_ptr<ScopedLsaPolicy>& policy,
     return hr;
   }
 
-  return manager->ModifyUserAccessWithLogonHours(domain, username, allow);
+  PSID psid;
+  if (!::ConvertStringSidToSidW(sid.c_str(), &psid)) {
+    hr = HRESULT_FROM_WIN32(::GetLastError());
+    LOGFN(ERROR) << "ConvertStringSidToSidW sid=" << sid << " hr=" << putHR(hr);
+    return hr;
+  }
+
+  std::vector<base::string16> account_rights{
+      SE_DENY_INTERACTIVE_LOGON_NAME, SE_DENY_NETWORK_LOGON_NAME,
+      SE_DENY_REMOTE_INTERACTIVE_LOGON_NAME};
+  if (!allow) {
+    return policy->AddAccountRights(psid, account_rights);
+  } else {
+    // Note: We are still going to keep this time restrictions flow to avoid
+    // any cornercase scenario where user is blocked on login UI because
+    // time restrictions were set but were never added back.
+    hr = manager->ModifyUserAccessWithLogonHours(domain, username, allow);
+    if (FAILED(hr))
+      LOGFN(ERROR) << "Failed to remove time restrictions for sid : " << sid;
+
+    return policy->RemoveAccountRights(psid, account_rights);
+  }
 }
 
 }  // namespace

chrome/credential_provider/gaiacp/gaia_credential_base.cc

@@ -897,7 +897,8 @@ HRESULT CGaiaCredentialBase::OnDllRegisterServer() {
   }
 
   // Add "logon as batch" right.
-  hr = policy->AddAccountRights(sid, SE_BATCH_LOGON_NAME);
+  std::vector<base::string16> rights{SE_BATCH_LOGON_NAME};
+  hr = policy->AddAccountRights(sid, rights);
   ::LocalFree(sid);
   if (FAILED(hr)) {
     LOGFN(ERROR) << "policy.AddAccountRights hr=" << putHR(hr);
@@ -1513,9 +1514,8 @@ bool CGaiaCredentialBase::CanProceedToLogonStub(wchar_t** status_text) {
     can_proceed_to_logon_stub = false;
     error_message = AllocErrorString(IDS_EMAIL_MISMATCH_BASE);
     LOGFN(ERROR) << "Restricted domains registry key must be set";
-  }
-  // If there is no internet connection, just abort right away.
-  else if (!InternetAvailabilityChecker::Get()->HasInternetConnection()) {
+  } else if (!InternetAvailabilityChecker::Get()->HasInternetConnection()) {
+    // If there is no internet connection, just abort right away.
     can_proceed_to_logon_stub = false;
     error_message = AllocErrorString(IDS_NO_NETWORK_BASE);
     LOGFN(VERBOSE) << "No internet connection";

chrome/credential_provider/gaiacp/scoped_lsa_policy.cc

@@ -124,10 +124,16 @@ bool ScopedLsaPolicy::PrivateDataExists(const wchar_t* key) {
   return true;
 }
 
-HRESULT ScopedLsaPolicy::AddAccountRights(PSID sid, const wchar_t* right) {
-  LSA_UNICODE_STRING lsa_right;
-  InitLsaString(right, &lsa_right);
-  NTSTATUS sts = ::LsaAddAccountRights(handle_, sid, &lsa_right, 1);
+HRESULT ScopedLsaPolicy::AddAccountRights(
+    PSID sid,
+    const std::vector<base::string16>& rights) {
+  std::vector<LSA_UNICODE_STRING> lsa_rights;
+  for (auto& right : rights) {
+    LSA_UNICODE_STRING lsa_right;
+    InitLsaString(right.c_str(), &lsa_right);
+    lsa_rights.push_back(lsa_right);
+  }
+  NTSTATUS sts = ::LsaAddAccountRights(handle_, sid, lsa_rights.data(), 1);
   if (sts != STATUS_SUCCESS) {
     HRESULT hr = HRESULT_FROM_NT(sts);
     LOGFN(ERROR) << "LsaAddAccountRights sts=" << putHR(sts)
@@ -137,6 +143,26 @@ HRESULT ScopedLsaPolicy::AddAccountRights(PSID sid, const wchar_t* right) {
   return S_OK;
 }
 
+HRESULT ScopedLsaPolicy::RemoveAccountRights(
+    PSID sid,
+    const std::vector<base::string16>& rights) {
+  std::vector<LSA_UNICODE_STRING> lsa_rights;
+  for (auto& right : rights) {
+    LSA_UNICODE_STRING lsa_right;
+    InitLsaString(right.c_str(), &lsa_right);
+    lsa_rights.push_back(lsa_right);
+  }
+  NTSTATUS sts =
+      ::LsaRemoveAccountRights(handle_, sid, FALSE, lsa_rights.data(), 1);
+  if (sts != STATUS_SUCCESS) {
+    HRESULT hr = HRESULT_FROM_NT(sts);
+    LOGFN(ERROR) << "LsaRemoveAccountRights sts=" << putHR(sts)
+                 << " hr=" << putHR(hr);
+    return hr;
+  }
+  return S_OK;
+}
+
 HRESULT ScopedLsaPolicy::RemoveAccount(PSID sid) {
   // When all rights are removed from an account, the account itself is also
   // deleted.

chrome/credential_provider/gaiacp/scoped_lsa_policy.h

@@ -30,8 +30,13 @@ class [[clang::lto_visibility_public]] ScopedLsaPolicy {
                                       size_t length);
   virtual bool PrivateDataExists(const wchar_t* key);
 
-  // Adds the given right to the given user.
-  virtual HRESULT AddAccountRights(PSID sid, const wchar_t* right);
+  // Adds the set of given rights to the given user.
+  virtual HRESULT AddAccountRights(PSID sid,
+                                   const std::vector<base::string16>& rights);
+
+  // Removes the set of given rights to the given user.
+  virtual HRESULT RemoveAccountRights(
+      PSID sid, const std::vector<base::string16>& rights);
 
   // Removes the user account from the system.
   virtual HRESULT RemoveAccount(PSID sid);

chrome/credential_provider/test/gcp_fakes.cc

@@ -535,7 +535,15 @@ bool FakeScopedLsaPolicy::PrivateDataExists(const wchar_t* key) {
   return private_data().count(key) != 0;
 }
 
-HRESULT FakeScopedLsaPolicy::AddAccountRights(PSID sid, const wchar_t* right) {
+HRESULT FakeScopedLsaPolicy::AddAccountRights(
+    PSID sid,
+    const std::vector<base::string16>& rights) {
+  return S_OK;
+}
+
+HRESULT FakeScopedLsaPolicy::RemoveAccountRights(
+    PSID sid,
+    const std::vector<base::string16>& rights) {
   return S_OK;
 }
 

chrome/credential_provider/test/gcp_fakes.h

@@ -239,7 +239,11 @@ class FakeScopedLsaPolicy : public ScopedLsaPolicy {
                               wchar_t* value,
                               size_t length) override;
   bool PrivateDataExists(const wchar_t* key) override;
-  HRESULT AddAccountRights(PSID sid, const wchar_t* right) override;
+  HRESULT AddAccountRights(PSID sid,
+                           const std::vector<base::string16>& rights) override;
+  HRESULT RemoveAccountRights(
+      PSID sid,
+      const std::vector<base::string16>& rights) override;
   HRESULT RemoveAccount(PSID sid) override;
 
  private: