Revert "Change proxy disclosure at chrome://management"

This reverts commit 6d3e7bf2.

Reason for revert: Tests fail under msan.
https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/20286
e.g.
[ RUN      ] ManagementUIHandlerTests.ProxyServerDisclosureDeviceOffline
[14456:14456:0819/094542.091708:576654156:WARNING:server_backed_state_keys_broker.cc(72)] Failed to obtain server-backed state keys.
==14456==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x5640b0ca7d20 in ManagementUIHandlerTests::SetUpProfileAndHandler() ./../../chrome/browser/ui/webui/management_ui_handler_unittest.cc:437:9
    #1 0x5640b0cb6cc1 in ManagementUIHandlerTests_ProxyServerDisclosureDeviceOffline_Test::TestBody() ./../../chrome/browser/ui/webui/management_ui_handler_unittest.cc:972:3
...

Original change's description:
> Change proxy disclosure at chrome://management
> 
> Change the wording and location of the proxy server disclosure in the
> transparency panel.
> 
> Bug: 1116961
> Test: unittests
> Change-Id: Id394511a6357329e629a37f19a4f83a4d49ea0db
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2360073
> Reviewed-by: Andreea-Elena Costinas <acostinas@google.com>
> Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
> Reviewed-by: Greg Levin <glevin@chromium.org>
> Commit-Queue: Andreea-Elena Costinas <acostinas@google.com>
> Cr-Commit-Position: refs/heads/master@{#799604}

TBR=antrim@chromium.org,glevin@chromium.org,mgalonsky@chromium.org,acostinas@google.com

Change-Id: I9c1489950f8c2bf71d5ae78ca97f63fce806174e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1116961
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2364481Reviewed-by: Kevin McNee <mcnee@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799769}

chrome/browser/resources/management/management_browser_proxy.js

@@ -40,8 +40,7 @@ export let BrowserReportingResponse;
  *   threatProtectionDescription: string,
  *   showUpdateRequiredEol: boolean,
  *   eolMessage: string,
- *   eolAdminMessage: string,
- *   showProxyServerPrivacyDisclosure: boolean
+ *   eolAdminMessage: string
  * }}
  */
 let ManagedDataResponse;
@@ -80,6 +79,7 @@ export const DeviceReportingType = {
   USERNAME: 'username',
   EXTENSION: 'extension',
   ANDROID_APPLICATION: 'android application',
+  PROXY_SERVER: 'proxy server'
 };
 
 

chrome/browser/resources/management/management_ui.html

@@ -275,10 +275,6 @@
               if="[[showDeviceReportingInfo_(deviceReportingInfo_)]]">
             <section>
               <h2 class="cr-title-text">$i18n{deviceReporting}</h2>
-              <div class="subtitle"
-                hidden="[[!showProxyServerPrivacyDisclosure_]]">
-                $i18n{proxyServerPrivacyDisclosure}
-              </div>
               <div class="subtitle">
                 $i18n{deviceConfiguration}
               </div>

chrome/browser/resources/management/management_ui.js

@@ -81,9 +81,6 @@ Polymer({
     /** @private */
     eolMessage_: String,
 
-    /** @private */
-    showProxyServerPrivacyDisclosure_: Boolean,
-
     // </if>
 
     /** @private */
@@ -270,6 +267,8 @@ Polymer({
         return 'cr:extension';
       case DeviceReportingType.ANDROID_APPLICATION:
         return 'management:play-store';
+      case DeviceReportingType.PROXY_SERVER:
+        return 'management:vpn-lock';
       default:
         return 'cr:computer';
     }
@@ -347,8 +346,6 @@ Polymer({
       this.customerLogo_ = data.customerLogo;
       this.managementOverview_ = data.overview;
       this.eolMessage_ = data.eolMessage;
-      this.showProxyServerPrivacyDisclosure_ =
-          data.showProxyServerPrivacyDisclosure;
       try {
         // Sanitizing the message could throw an error if it contains non
         // supported markup.

chrome/browser/ui/webui/management_ui.cc

@@ -72,8 +72,7 @@ content::WebUIDataSource* CreateManagementUIHtmlSource(Profile* profile) {
     {kManagementReportExtensions, IDS_MANAGEMENT_REPORT_EXTENSIONS},
     {kManagementReportAndroidApplications,
      IDS_MANAGEMENT_REPORT_ANDROID_APPLICATIONS},
-    {"proxyServerPrivacyDisclosure",
-     IDS_MANAGEMENT_PROXY_SERVER_PRIVACY_DISCLOSURE},
+    {kManagementReportProxyServer, IDS_MANAGEMENT_REPORT_PROXY_SERVER},
 #endif  // defined(OS_CHROMEOS)
     {"browserReporting", IDS_MANAGEMENT_BROWSER_REPORTING},
     {"browserReportingExplanation",

chrome/browser/ui/webui/management_ui_handler.cc

@@ -170,6 +170,7 @@ const char kManagementPrinting[] = "managementPrinting";
 const char kManagementCrostini[] = "managementCrostini";
 const char kManagementCrostiniContainerConfiguration[] =
     "managementCrostiniContainerConfiguration";
+const char kManagementReportProxyServer[] = "managementReportProxyServer";
 const char kAccountManagedInfo[] = "accountManagedInfo";
 const char kDeviceManagedInfo[] = "deviceManagedInfo";
 const char kOverview[] = "overview";
@@ -212,7 +213,8 @@ enum class DeviceReportingType {
   kCrostini,
   kUsername,
   kExtensions,
-  kAndroidApplication
+  kAndroidApplication,
+  kProxyServer
 };
 
 // Corresponds to DeviceReportingType in management_browser_proxy.js
@@ -242,6 +244,8 @@ std::string ToJSDeviceReportingType(const DeviceReportingType& type) {
       return "extension";
     case DeviceReportingType::kAndroidApplication:
       return "android application";
+    case DeviceReportingType::kProxyServer:
+      return "proxy server";
     default:
       NOTREACHED() << "Unknown device reporting type";
       return "device";
@@ -605,6 +609,33 @@ void ManagementUIHandler::AddDeviceReportingInfo(
                               kManagementReportAndroidApplications,
                               DeviceReportingType::kAndroidApplication);
   }
+
+  chromeos::NetworkHandler* network_handler = chromeos::NetworkHandler::Get();
+  base::Value proxy_settings(base::Value::Type::DICTIONARY);
+  // |ui_proxy_config_service| may be missing in tests. If the device is offline
+  // (no network connected) the |DefaultNetwork| is null.
+  if (network_handler->has_ui_proxy_config_service() &&
+      network_handler->network_state_handler()->DefaultNetwork()) {
+    // Check if proxy is enforced by user policy, a forced install extension or
+    // ONC policies. This will only read managed settings.
+    network_handler->ui_proxy_config_service()->MergeEnforcedProxyConfig(
+        network_handler->network_state_handler()->DefaultNetwork()->guid(),
+        &proxy_settings);
+  }
+  if (!proxy_settings.DictEmpty()) {
+    // Proxies can be specified by web server url, via a PAC script or via the
+    // web proxy auto-discovery protocol. Chrome also supports the "direct"
+    // mode, in which no proxy is used.
+    base::Value* proxy_specification_mode = proxy_settings.FindPath(
+        {::onc::network_config::kType, ::onc::kAugmentationActiveSetting});
+    bool use_proxy =
+        proxy_specification_mode &&
+        proxy_specification_mode->GetString() != ::onc::proxy::kDirect;
+    if (use_proxy) {
+      AddDeviceReportingElement(report_sources, kManagementReportProxyServer,
+                                DeviceReportingType::kProxyServer);
+    }
+  }
 }
 
 bool ManagementUIHandler::IsUpdateRequiredEol() const {
@@ -632,35 +663,6 @@ void ManagementUIHandler::AddUpdateRequiredEolInfo(
       chromeos::kDeviceMinimumVersionAueMessage, &eol_admin_message);
   response->SetStringPath("eolAdminMessage", eol_admin_message);
 }
-
-void ManagementUIHandler::AddProxyServerPrivacyDisclosure(
-    base::Value* response) const {
-  bool showProxyDisclosure = false;
-  chromeos::NetworkHandler* network_handler = chromeos::NetworkHandler::Get();
-  base::Value proxy_settings(base::Value::Type::DICTIONARY);
-  // |ui_proxy_config_service| may be missing in tests. If the device is offline
-  // (no network connected) the |DefaultNetwork| is null.
-  if (network_handler->has_ui_proxy_config_service() &&
-      network_handler->network_state_handler()->DefaultNetwork()) {
-    // Check if proxy is enforced by user policy, a forced install extension or
-    // ONC policies. This will only read managed settings.
-    network_handler->ui_proxy_config_service()->MergeEnforcedProxyConfig(
-        network_handler->network_state_handler()->DefaultNetwork()->guid(),
-        &proxy_settings);
-  }
-  if (!proxy_settings.DictEmpty()) {
-    // Proxies can be specified by web server url, via a PAC script or via the
-    // web proxy auto-discovery protocol. Chrome also supports the "direct"
-    // mode, in which no proxy is used.
-    base::Value* proxy_specification_mode = proxy_settings.FindPath(
-        {::onc::network_config::kType, ::onc::kAugmentationActiveSetting});
-    showProxyDisclosure =
-        proxy_specification_mode &&
-        proxy_specification_mode->GetString() != ::onc::proxy::kDirect;
-  }
-  response->SetBoolPath("showProxyServerPrivacyDisclosure",
-                        showProxyDisclosure);
-}
 #endif
 
 base::Value ManagementUIHandler::GetContextualManagedData(Profile* profile) {
@@ -670,7 +672,6 @@ base::Value ManagementUIHandler::GetContextualManagedData(Profile* profile) {
   if (management_domain.empty())
     management_domain = GetAccountDomain(profile);
   AddUpdateRequiredEolInfo(&response);
-  AddProxyServerPrivacyDisclosure(&response);
 #else
   std::string management_domain = GetAccountDomain(profile);
 

chrome/browser/ui/webui/management_ui_handler.h

@@ -39,6 +39,7 @@ extern const char kManagementCrostini[];
 extern const char kManagementCrostiniContainerConfiguration[];
 extern const char kManagementReportExtensions[];
 extern const char kManagementReportAndroidApplications[];
+extern const char kManagementReportProxyServer[];
 #endif  // defined(OS_CHROMEOS)
 
 extern const char kCloudReportingExtensionId[];
@@ -153,11 +154,6 @@ class ManagementUIHandler : public content::WebUIMessageHandler,
   // as per device policy but the device cannot be updated due to End of Life
   // (Auto Update Expiration).
   void AddUpdateRequiredEolInfo(base::Value* response) const;
-
-  // Adds a boolean which indicates if there's a proxy on the device enforced by
-  // the admin. If true, a warning will be added to the transparency panel to
-  // inform the user that the admin may be able to see their network traffic.
-  void AddProxyServerPrivacyDisclosure(base::Value* response) const;
 #endif  // defined(OS_CHROMEOS)
  private:
   void GetManagementStatus(Profile* profile, base::Value* status) const;

chrome/browser/ui/webui/management_ui_handler_unittest.cc

-
 // Copyright 2019 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
@@ -86,7 +85,6 @@ struct ContextualManagementSourceUpdate {
 #if defined(OS_CHROMEOS)
   base::string16 management_overview;
   base::string16 update_required_eol;
-  bool show_proxy_server_privacy_disclosure;
 #else
   base::string16 browser_management_notice;
 #endif  // defined(OS_CHROMEOS)
@@ -301,10 +299,6 @@ class ManagementUIHandlerTests : public TestingBaseClass {
 #if defined(OS_CHROMEOS)
     extracted_.management_overview = ExtractPathFromDict(data, "overview");
     extracted_.update_required_eol = ExtractPathFromDict(data, "eolMessage");
-    base::Optional<bool> showProxyDisclosure =
-        data.FindBoolPath("showProxyServerPrivacyDisclosure");
-    extracted_.show_proxy_server_privacy_disclosure =
-        showProxyDisclosure.has_value() && showProxyDisclosure.value();
 #else
     extracted_.browser_management_notice =
         ExtractPathFromDict(data, "browserManagementNotice");
@@ -465,10 +459,6 @@ class ManagementUIHandlerTests : public TestingBaseClass {
   base::string16 GetUpdateRequiredEolMessage() const {
     return extracted_.update_required_eol;
   }
-
-  bool GetShowProxyServerPrivacyDisclosure() const {
-    return extracted_.show_proxy_server_privacy_disclosure;
-  }
 #else
 
   base::string16 GetBrowserManagementNotice() const {
@@ -928,28 +918,32 @@ TEST_F(ManagementUIHandlerTests, AllDisabledDeviceReportingInfo) {
                       expected_elements);
 }
 
-TEST_F(ManagementUIHandlerTests, ShowProxyServerDisclosure) {
-  // Set pref to use a proxy.
+TEST_F(ManagementUIHandlerTests, ProxyServerShowReport) {
   PrefProxyConfigTrackerImpl::RegisterProfilePrefs(user_prefs_.registry());
   chromeos::NetworkHandler::Get()->InitializePrefServices(&user_prefs_,
                                                           &local_state_);
+  // Set pref to use a proxy.
   base::Value policy_prefs_config = ProxyConfigDictionary::CreateAutoDetect();
   user_prefs_.SetUserPref(
       proxy_config::prefs::kProxy,
       base::Value::ToUniquePtrValue(std::move(policy_prefs_config)));
   base::RunLoop().RunUntilIdle();
 
-  GetTestConfig().managed_device = true;
-  SetUpProfileAndHandler();
+  ResetTestConfig(false);
+  const base::Value info = SetUpForReportingInfo();
+
+  const std::map<std::string, std::string> expected_elements = {
+      {kManagementReportProxyServer, "proxy server"}};
 
-  EXPECT_TRUE(GetShowProxyServerPrivacyDisclosure());
+  ASSERT_PRED_FORMAT2(ReportingElementsToBeEQ, info.GetList(),
+                      expected_elements);
 }
 
-TEST_F(ManagementUIHandlerTests, ProxyServerDisclosureDeviceOffline) {
-  // Simulate network disconnected state.
+TEST_F(ManagementUIHandlerTests, ProxyServerShowReportDeviceOffline) {
   PrefProxyConfigTrackerImpl::RegisterProfilePrefs(user_prefs_.registry());
   chromeos::NetworkHandler::Get()->InitializePrefServices(&user_prefs_,
                                                           &local_state_);
+  // Simulate network disconnected state.
   chromeos::NetworkStateHandler::NetworkStateList networks;
   chromeos::NetworkHandler::Get()
       ->network_state_handler()
@@ -968,30 +962,33 @@ TEST_F(ManagementUIHandlerTests, ProxyServerDisclosureDeviceOffline) {
   }
   base::RunLoop().RunUntilIdle();
 
-  GetTestConfig().managed_device = true;
-  SetUpProfileAndHandler();
+  ResetTestConfig(false);
+  const base::Value info = SetUpForReportingInfo();
 
-  EXPECT_FALSE(GetShowProxyServerPrivacyDisclosure());
+  const std::map<std::string, std::string> expected_elements = {};
 
+  ASSERT_PRED_FORMAT2(ReportingElementsToBeEQ, info.GetList(),
+                      expected_elements);
   chromeos::NetworkHandler::Get()->NetworkHandler::ShutdownPrefServices();
 }
 
-TEST_F(ManagementUIHandlerTests, HideProxyServerDisclosureForDirectProxy) {
-  // Set pref not to use proxy.
+TEST_F(ManagementUIHandlerTests, ProxyServerHideReportForDirectProxy) {
   PrefProxyConfigTrackerImpl::RegisterProfilePrefs(user_prefs_.registry());
   chromeos::NetworkHandler::Get()->InitializePrefServices(&user_prefs_,
                                                           &local_state_);
+  // Set pref not to use proxy.
   base::Value policy_prefs_config = ProxyConfigDictionary::CreateDirect();
   user_prefs_.SetUserPref(
       proxy_config::prefs::kProxy,
       base::Value::ToUniquePtrValue(std::move(policy_prefs_config)));
   base::RunLoop().RunUntilIdle();
 
-  GetTestConfig().managed_device = true;
-  SetUpProfileAndHandler();
-
-  EXPECT_FALSE(GetShowProxyServerPrivacyDisclosure());
+  ResetTestConfig(false);
+  const base::Value info = SetUpForReportingInfo();
 
+  const std::map<std::string, std::string> expected_elements = {};
+  ASSERT_PRED_FORMAT2(ReportingElementsToBeEQ, info.GetList(),
+                      expected_elements);
   chromeos::NetworkHandler::Get()->NetworkHandler::ShutdownPrefServices();
 }
 

components/management_strings.grdp

@@ -83,9 +83,6 @@
     <message name="IDS_MANAGEMENT_DEVICE_REPORTING" desc="Title of the types of device reporting section of the page">
       Device
     </message>
-    <message name="IDS_MANAGEMENT_PROXY_SERVER_PRIVACY_DISCLOSURE" desc="Message stating that administrators can see user's traffic when connected to a proxy server.">
-      Administrators of this device have configured your network connection, which may allow them to see your network traffic, including which websites you visit.
-    </message>
     <message name="IDS_MANAGEMENT_DEVICE_CONFIGURATION" desc="Message telling users that their administrator has set some specific policies on their device">
       Your administrator can see:
     </message>
@@ -125,6 +122,9 @@
     <message name="IDS_MANAGEMENT_REPORT_ANDROID_APPLICATIONS" desc="Message stating that administrators can see user's installed Android applications.">
       Which Google Play apps you have installed
     </message>
+    <message name="IDS_MANAGEMENT_REPORT_PROXY_SERVER" desc="Message stating that administrators can see user's traffic when connected to a proxy server.">
+      Websites you visit and the contents of not secure pages
+    </message>
     <message name="IDS_MANAGEMENT_REPORT_PLUGIN_VM" desc="Message telling users that Plugin VM can collect data.">
       Your administrator has allowed <ph name="APP_NAME">$1<ex>Plugin VM</ex></ph> to collect diagnostics data to improve the product experience. See <ph name="BEGIN_LINK">&lt;a target="_blank" href="https://www.parallels.com/pcep"&gt;</ph>https://www.parallels.com/pcep<ph name="END_LINK">&lt;/a&gt;</ph> for more information.
     </message>

components/management_strings_grdp/IDS_MANAGEMENT_PROXY_SERVER_PRIVACY_DISCLOSURE.png.sha1

-b0b948edf91fae1dd9a31455eab621918025fa36
\ No newline at end of file

components/management_strings_grdp/IDS_MANAGEMENT_REPORT_PROXY_SERVER.png.sha1

+a35ceef3ad7c9e9ad08ac68788efc2a63720c506
\ No newline at end of file