[GCPW] Change error message to include valid email domains.

Bug: 1027346
Change-Id: I30fac3fbc2dbfcc38a31c0c950c7fbaed4d3382d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1929102Reviewed-by: Tien Mai <tienmai@chromium.org>
Commit-Queue: Robin Lewis <wrlewis@google.com>
Cr-Commit-Position: refs/heads/master@{#721524}

chrome/credential_provider/gaiacp/gaia_credential_base.cc

@@ -99,6 +99,27 @@ base::string16 GetEmailDomains() {
   return base::string16(&email_domains[0]);
 }
 
+// Get a pretty-printed string of the list of email domains that we can display
+// to the end-user.
+base::string16 GetEmailDomainsPrintableString() {
+  base::string16 email_domains_reg = GetEmailDomains();
+  if (email_domains_reg.empty())
+    return email_domains_reg;
+
+  std::vector<base::string16> domains =
+      base::SplitString(base::ToLowerASCII(email_domains_reg),
+                        base::ASCIIToUTF16(kEmailDomainsSeparator),
+                        base::WhitespaceHandling::TRIM_WHITESPACE,
+                        base::SplitResult::SPLIT_WANT_NONEMPTY);
+  base::string16 email_domains_str;
+  for (size_t i = 0; i < domains.size(); ++i) {
+    email_domains_str += domains[i];
+    if (i < domains.size() - 1)
+      email_domains_str += L", ";
+  }
+  return email_domains_str;
+}
+
 // Use WinHttpUrlFetcher to communicate with the admin sdk and fetch the active
 // directory UPN from the admin configured custom attributes.
 HRESULT GetAdUpnFromCloudDirectory(const base::string16& email,
@@ -478,7 +499,7 @@ HRESULT ValidateResult(const base::Value& result, BSTR* status_text) {
         break;
       case kUiecInvalidEmailDomain:
         *status_text = CGaiaCredentialBase::AllocErrorString(
-            IDS_INVALID_EMAIL_DOMAIN_BASE);
+            IDS_INVALID_EMAIL_DOMAIN_BASE, {GetEmailDomainsPrintableString()});
         break;
       case kUiecMissingSigninData:
         *status_text =
@@ -1152,6 +1173,14 @@ BSTR CGaiaCredentialBase::AllocErrorString(UINT id) {
   return str.Detach();
 }
 
+// static
+BSTR CGaiaCredentialBase::AllocErrorString(
+    UINT id,
+    const std::vector<base::string16>& replacements) {
+  CComBSTR str(GetStringResource(id, replacements).c_str());
+  return str.Detach();
+}
+
 // static
 HRESULT CGaiaCredentialBase::GetInstallDirectory(base::FilePath* path) {
   DCHECK(path);

chrome/credential_provider/gaiacp/gaia_credential_base.h

@@ -56,6 +56,11 @@ class ATL_NO_VTABLE CGaiaCredentialBase
   // Allocates a BSTR from a DLL string resource given by |id|.
   static BSTR AllocErrorString(UINT id);
 
+  // Allocates a BSTR from a DLL string resource given by |id| replacing the
+  // placeholders in the string by the provided replacements.
+  static BSTR AllocErrorString(UINT id,
+                               const std::vector<base::string16>& replacements);
+
   // Gets the directory where the credential provider is installed.
   static HRESULT GetInstallDirectory(base::FilePath* path);
 

chrome/credential_provider/gaiacp/gaia_credential_base_unittests.cc

@@ -8,6 +8,7 @@
 #include <wrl/client.h>
 
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time_override.h"
@@ -604,6 +605,33 @@ TEST_F(GcpGaiaCredentialBaseTest, FailedUserCreation) {
   ASSERT_EQ(S_OK, FinishLogonProcess(false, false, IDS_INTERNAL_ERROR_BASE));
 }
 
+TEST_F(GcpGaiaCredentialBaseTest, FailOnInvalidDomain) {
+  const base::string16 allowed_email_domains =
+      L"acme.com,acme2.com,acme3.com";
+  ASSERT_EQ(S_OK, SetGlobalFlagForTesting(L"ed", allowed_email_domains));
+
+  // Create provider and start logon.
+  Microsoft::WRL::ComPtr<ICredentialProviderCredential> cred;
+
+  ASSERT_EQ(S_OK, InitializeProviderAndGetCredential(0, &cred));
+  Microsoft::WRL::ComPtr<ITestCredential> test;
+  ASSERT_EQ(S_OK, cred.As(&test));
+
+  // Fail due to invalid domain.
+  ASSERT_EQ(S_OK, test->SetDefaultExitCode(kUiecInvalidEmailDomain));
+
+  ASSERT_EQ(S_OK, StartLogonProcessAndWait());
+
+  const base::string16 formatted_domains_str =
+      L"acme.com, acme2.com, acme3.com";
+  base::string16 expected_error_msg = base::ReplaceStringPlaceholders(
+      GetStringResource(IDS_INVALID_EMAIL_DOMAIN_BASE), {formatted_domains_str},
+      nullptr);
+
+  // Logon process should fail with the specified error message.
+  ASSERT_EQ(S_OK, FinishLogonProcess(false, false, expected_error_msg));
+}
+
 TEST_F(GcpGaiaCredentialBaseTest, StripEmailTLD) {
   USES_CONVERSION;
   // Create provider and start logon.

chrome/credential_provider/gaiacp/gaia_resources.grd

@@ -104,7 +104,7 @@
          Signing in with this account is not allowed. Try a different account.
       </message>
       <message name="IDS_INVALID_EMAIL_DOMAIN" desc="">
-        Signing in with a work account on this domain is not allowed. Try a different account.
+        Your administrator has restricted device sign-in to the following domains: <ph name="EMAIL_DOMAINS">$1<ex>acme.com, acme2.com, acme3.com</ex></ph>. Try again using a valid work account.
       </message>
       <message name="IDS_DISALLOWED_CONSUMER_EMAIL" desc="">
         Signing in with a personal account on this device is not allowed. Please login with a work account.

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

-88d3a339f08b760c49168bbfc77b9f70a580d162
+fe6d8dc582a7ed082ce0dc49707a31974f946be9
\ No newline at end of file

chrome/credential_provider/gaiacp/gcp_utils.cc

@@ -698,6 +698,15 @@ base::string16 GetStringResource(int base_message_id) {
   return localized_string;
 }
 
+base::string16 GetStringResource(int base_message_id,
+                                 const std::vector<base::string16>& subst) {
+  base::string16 format_string = GetStringResource(base_message_id);
+  base::string16 formatted =
+      base::ReplaceStringPlaceholders(format_string, subst, nullptr);
+
+  return formatted;
+}
+
 base::string16 GetSelectedLanguage() {
   return GetLanguageSelector().matched_candidate();
 }

chrome/credential_provider/gaiacp/gcp_utils.h

@@ -215,6 +215,11 @@ void DeleteStartupSentinel();
 // Gets a string resource from the DLL with the given id.
 base::string16 GetStringResource(int base_message_id);
 
+// Gets a string resource from the DLL with the given id after replacing the
+// placeholders with the provided substitutions.
+base::string16 GetStringResource(int base_message_id,
+                                 const std::vector<base::string16>& subst);
+
 // Gets the language selected by the base::win::i18n::LanguageSelector.
 base::string16 GetSelectedLanguage();
 

chrome/credential_provider/test/gls_runner_test_base.cc

@@ -527,6 +527,15 @@ HRESULT GlsRunnerTestBase::FinishLogonProcess(
     bool expected_success,
     bool expected_credentials_change_fired,
     int expected_error_message) {
+  return FinishLogonProcess(
+      expected_success, expected_credentials_change_fired,
+      expected_error_message ? GetStringResource(expected_error_message) : L"");
+}
+
+HRESULT GlsRunnerTestBase::FinishLogonProcess(
+    bool expected_success,
+    bool expected_credentials_change_fired,
+    const base::string16& expected_error_message) {
   // If no logon process was started, there is nothing to finish.
   if (!logon_process_started_successfully_)
     return S_OK;
@@ -559,6 +568,18 @@ HRESULT GlsRunnerTestBase::FinishLogonProcessWithCred(
     int expected_error_message,
     const Microsoft::WRL::ComPtr<ICredentialProviderCredential>&
         local_testing_cred) {
+  return FinishLogonProcessWithCred(
+      expected_success, expected_credentials_change_fired,
+      expected_error_message ? GetStringResource(expected_error_message) : L"",
+      local_testing_cred);
+}
+
+HRESULT GlsRunnerTestBase::FinishLogonProcessWithCred(
+    bool expected_success,
+    bool expected_credentials_change_fired,
+    const base::string16& expected_error_message,
+    const Microsoft::WRL::ComPtr<ICredentialProviderCredential>&
+        local_testing_cred) {
   // If no logon process was started, there is nothing to finish.
   if (!logon_process_started_successfully_)
     return S_OK;
@@ -585,9 +606,8 @@ HRESULT GlsRunnerTestBase::FinishLogonProcessWithCred(
     EXPECT_EQ(0u, test_provider->password().Length());
     EXPECT_EQ(0u, test_provider->sid().Length());
 
-    if (expected_error_message) {
-      EXPECT_STREQ(test_cred->GetErrorText(),
-                   GetStringResource(expected_error_message).c_str());
+    if (!expected_error_message.empty()) {
+      EXPECT_STREQ(test_cred->GetErrorText(), expected_error_message.c_str());
     } else {
       EXPECT_EQ(test_cred->GetErrorText(), nullptr);
     }

chrome/credential_provider/test/gls_runner_test_base.h

@@ -157,12 +157,21 @@ class GlsRunnerTestBase : public ::testing::Test {
   HRESULT FinishLogonProcess(bool expected_success,
                              bool expected_credentials_change_fired,
                              int expected_error_message);
+  HRESULT FinishLogonProcess(bool expected_success,
+                             bool expected_credentials_change_fired,
+                             const base::string16& expected_error_message);
   HRESULT FinishLogonProcessWithCred(
       bool expected_success,
       bool expected_credentials_change_fired,
       int expected_error_message,
       const Microsoft::WRL::ComPtr<ICredentialProviderCredential>&
           local_testing_cred);
+  HRESULT FinishLogonProcessWithCred(
+      bool expected_success,
+      bool expected_credentials_change_fired,
+      const base::string16& expected_error_message,
+      const Microsoft::WRL::ComPtr<ICredentialProviderCredential>&
+          local_testing_cred);
   HRESULT ReportLogonProcessResult(
       const Microsoft::WRL::ComPtr<ICredentialProviderCredential>&
           local_testing_cred);