Support disabling Chrome OS native VPNs by policy

This CL does the following:
- Allows adding "VPN" to ONC's DisableNetworkTypes list.
- Adds a VPN device state entry in the list returned by
  CrosNetworkConfig::GetDeviceStateList() to be used by UI.
- If VPN is disabled by policy:
  * Removes VPN pod from the system tray.
  * Removes VPN entry from the network summary page in settings.
  * Disables (but not removes) the "Add OpenVPN/L2TP..." button
    in the network summary page.

Bug: 1064893, 1082327
Change-Id: I0441e3ab0eae27b2af7eafc7dd7b0f2ad5fcd5b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2190216
Commit-Queue: Omar Morsi <omorsi@google.com>
Reviewed-by: dpapad <dpapad@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781806}

ash/system/network/tray_network_state_model.cc

@@ -164,7 +164,7 @@ const DeviceStateProperties* TrayNetworkStateModel::GetDevice(
   return iter->second.get();
 }
 
-DeviceStateType TrayNetworkStateModel::GetDeviceState(NetworkType type) {
+DeviceStateType TrayNetworkStateModel::GetDeviceState(NetworkType type) const {
   const DeviceStateProperties* device = GetDevice(type);
   return device ? device->device_state : DeviceStateType::kUnavailable;
 }
@@ -174,6 +174,12 @@ void TrayNetworkStateModel::SetNetworkTypeEnabledState(NetworkType type,
   impl_->SetNetworkTypeEnabledState(type, enabled);
 }
 
+bool TrayNetworkStateModel::IsBuiltinVpnEnabled() const {
+  return TrayNetworkStateModel::GetDeviceState(
+             chromeos::network_config::mojom::NetworkType::kVPN) ==
+         chromeos::network_config::mojom::DeviceStateType::kEnabled;
+}
+
 chromeos::network_config::mojom::CrosNetworkConfig*
 TrayNetworkStateModel::cros_network_config() {
   return impl_->cros_network_config();

ash/system/network/tray_network_state_model.h

@@ -39,13 +39,17 @@ class ASH_EXPORT TrayNetworkStateModel {
 
   // Returns the DeviceStateType for |type| if a device exists or kUnavailable.
   chromeos::network_config::mojom::DeviceStateType GetDeviceState(
-      chromeos::network_config::mojom::NetworkType type);
+      chromeos::network_config::mojom::NetworkType type) const;
 
   // Convenience method to call the |remote_cros_network_config_| method.
   void SetNetworkTypeEnabledState(
       chromeos::network_config::mojom::NetworkType type,
       bool enabled);
 
+  // Returns true if built-in VPN is enabled.
+  // Note: Currently only built-in VPNs can be disabled by policy.
+  bool IsBuiltinVpnEnabled() const;
+
   // This used to be inlined but now requires details from the Impl class.
   chromeos::network_config::mojom::CrosNetworkConfig* cros_network_config();
 

ash/system/network/vpn_feature_pod_controller.cc

@@ -39,8 +39,8 @@ bool IsVPNVisibleInSystemTray() {
   if (model->vpn_list()->HaveExtensionOrArcVpnProviders())
     return true;
 
-  // Also show the VPN entry if at least one VPN network is configured.
-  return model->has_vpn();
+  // Note: At this point, only built-in VPNs are considered.
+  return model->IsBuiltinVpnEnabled() && model->has_vpn();
 }
 
 }  // namespace

ash/system/network/vpn_list.cc

@@ -41,9 +41,12 @@ void VpnList::RemoveObserver(Observer* observer) {
   observer_list_.RemoveObserver(observer);
 }
 
+void VpnList::ActiveNetworkStateChanged() {
+  Update();
+}
+
 void VpnList::VpnProvidersChanged() {
-  model_->cros_network_config()->GetVpnProviders(
-      base::BindOnce(&VpnList::OnGetVpnProviders, base::Unretained(this)));
+  Update();
 }
 
 void VpnList::SetVpnProvidersForTest(std::vector<VpnProviderPtr> providers) {
@@ -53,7 +56,6 @@ void VpnList::SetVpnProvidersForTest(std::vector<VpnProviderPtr> providers) {
 void VpnList::OnGetVpnProviders(std::vector<VpnProviderPtr> providers) {
   extension_vpn_providers_.clear();
   arc_vpn_providers_.clear();
-  // Add the OpenVPN/L2TP provider.
   AddBuiltInProvider();
   // Add Third Party (Extension and Arc) providers.
   for (auto& provider : providers) {
@@ -89,4 +91,9 @@ void VpnList::AddBuiltInProvider() {
                        /*last_launch_time=*/base::Time()));
 }
 
+void VpnList::Update() {
+  model_->cros_network_config()->GetVpnProviders(
+      base::BindOnce(&VpnList::OnGetVpnProviders, base::Unretained(this)));
+}
+
 }  // namespace ash

ash/system/network/vpn_list.h

@@ -61,6 +61,7 @@ class ASH_EXPORT VpnList : public TrayNetworkStateObserver {
   void RemoveObserver(Observer* observer);
 
   // TrayNetworkStateObserver
+  void ActiveNetworkStateChanged() override;
   void VpnProvidersChanged() override;
 
   void SetVpnProvidersForTest(std::vector<VpnProviderPtr> providers);
@@ -75,6 +76,10 @@ class ASH_EXPORT VpnList : public TrayNetworkStateObserver {
   // Adds the built-in OpenVPN/L2TP provider to |extension_vpn_providers_|.
   void AddBuiltInProvider();
 
+  // Called when either ActiveNetworkStateChanged() or VpnProvidersChanged() is
+  // called.
+  void Update();
+
   TrayNetworkStateModel* model_;
 
   // Cache of VPN providers, including the built-in OpenVPN/L2TP provider and

ash/system/network/vpn_list_view.cc

@@ -105,9 +105,12 @@ bool IsVpnConfigAllowed() {
 // A list entry that represents a VPN provider.
 class VPNListProviderEntry : public views::ButtonListener, public views::View {
  public:
+  // Currently the |enabled| flag will be always true for VPN providers other
+  // than the built-in VPNs.
   VPNListProviderEntry(const VpnProviderPtr& vpn_provider,
                        bool top_item,
                        const std::string& name,
+                       bool enabled,
                        int button_accessible_name_id)
       : vpn_provider_(vpn_provider->Clone()) {
     TrayPopupUtils::ConfigureAsStickyHeader(this);
@@ -117,22 +120,35 @@ class VPNListProviderEntry : public views::ButtonListener, public views::View {
                       TrayPopupUtils::CreateMainImageView());
     AddChildView(tri_view);
 
+    // Add the VPN label.
     views::Label* label = TrayPopupUtils::CreateDefaultLabel();
     TrayPopupItemStyle style(TrayPopupItemStyle::FontStyle::SUB_HEADER);
     style.SetupLabel(label);
     label->SetText(base::ASCIIToUTF16(name));
     tri_view->AddView(TriView::Container::CENTER, label);
 
+    // Add the VPN policy indicator if using this |vpn_provider| is disabled.
+    if (!enabled) {
+      views::ImageView* policy_indicator_icon = GetPolicyIndicatorIcon();
+      tri_view->AddView(TriView::Container::END, policy_indicator_icon);
+    }
+
+    // Add the VPN add button.
     const SkColor image_color = AshColorProvider::Get()->GetContentLayerColor(
         AshColorProvider::ContentLayerType::kButtonIconColorProminent,
         AshColorProvider::AshColorMode::kDark);
-    const gfx::ImageSkia icon =
+
+    const gfx::ImageSkia enabled_icon =
         gfx::CreateVectorIcon(kSystemMenuAddConnectionIcon, image_color);
-    SystemMenuButton* add_vpn_button =
-        new SystemMenuButton(this, icon, icon, button_accessible_name_id);
+    const gfx::ImageSkia disabled_icon =
+        gfx::CreateVectorIcon(kSystemMenuAddConnectionIcon,
+                              AshColorProvider::GetDisabledColor(image_color));
+
+    SystemMenuButton* add_vpn_button = new SystemMenuButton(
+        this, enabled_icon, disabled_icon, button_accessible_name_id);
     add_vpn_button->SetInkDropColor(
         UnifiedSystemTrayView::GetBackgroundColor());
-    add_vpn_button->SetEnabled(true);
+    add_vpn_button->SetEnabled(enabled);
     tri_view->AddView(TriView::Container::END, add_vpn_button);
   }
 
@@ -162,6 +178,20 @@ class VPNListProviderEntry : public views::ButtonListener, public views::View {
   }
 
  private:
+  views::ImageView* GetPolicyIndicatorIcon() {
+    views::ImageView* policy_indicator_icon =
+        TrayPopupUtils::CreateMainImageView();
+    policy_indicator_icon->SetImage(gfx::CreateVectorIcon(
+        kSystemMenuBusinessIcon,
+        AshColorProvider::Get()->GetContentLayerColor(
+            AshColorProvider::ContentLayerType::kIconColorPrimary,
+            AshColorProvider::AshColorMode::kDark)));
+    policy_indicator_icon->SetAccessibleName(l10n_util::GetStringFUTF16(
+        IDS_ASH_ACCESSIBILITY_FEATURE_MANAGED,
+        l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_VPN_BUILT_IN_PROVIDER)));
+    return policy_indicator_icon;
+  }
+
   VpnProviderPtr vpn_provider_;
 
   DISALLOW_COPY_AND_ASSIGN(VPNListProviderEntry);
@@ -413,17 +443,26 @@ void VPNListView::AddProviderAndNetworks(VpnProviderPtr vpn_provider,
 
   // Add a list entry for the VPN provider.
   views::View* provider_view = nullptr;
-  provider_view = new VPNListProviderEntry(vpn_provider, list_empty_, vpn_name,
-                                           IDS_ASH_STATUS_TRAY_ADD_CONNECTION);
+
+  // Note: Currently only built-in VPNs can be disabled by policy.
+  bool vpn_enabled =
+      vpn_provider->type != VpnType::kOpenVPN || model()->IsBuiltinVpnEnabled();
+
+  provider_view =
+      new VPNListProviderEntry(vpn_provider, list_empty_, vpn_name, vpn_enabled,
+                               IDS_ASH_STATUS_TRAY_ADD_CONNECTION);
   scroll_content()->AddChildView(provider_view);
   const VpnProvider* vpn_providerp = vpn_provider.get();
   provider_view_map_[provider_view] = std::move(vpn_provider);
   list_empty_ = false;
-  // Add the networks belonging to this provider, in the priority order returned
-  // by shill.
-  for (const auto& network : networks) {
-    if (VpnProviderMatchesNetwork(vpn_providerp, network.get()))
-      AddNetwork(network.get());
+
+  if (vpn_enabled) {
+    // Add the networks belonging to this provider, in the priority order
+    // returned by shill.
+    for (const auto& network : networks) {
+      if (VpnProviderMatchesNetwork(vpn_providerp, network.get()))
+        AddNetwork(network.get());
+    }
   }
 }
 

chrome/browser/chromeos/net/network_health/network_health_unittest.cc

@@ -60,14 +60,20 @@ class NetworkHealthTest : public ::testing::Test {
     const auto& initial_network_health_state =
         network_health_.GetNetworkHealthState();
 
-    ASSERT_EQ(1u, initial_network_health_state->networks.size());
+    ASSERT_EQ(std::size_t(2), initial_network_health_state->networks.size());
+
+    // Check that VPN device state is always reported even if no VPNs exist.
+    ASSERT_EQ(network_config::mojom::NetworkType::kVPN,
+              initial_network_health_state->networks[0]->type);
+    ASSERT_EQ(network_health::mojom::NetworkState::kNotConnected,
+              initial_network_health_state->networks[0]->state);
 
     // Check that the default wifi device created by CrosNetworkConfigTestHelper
     // exists.
     ASSERT_EQ(network_config::mojom::NetworkType::kWiFi,
-              initial_network_health_state->networks[0]->type);
+              initial_network_health_state->networks[1]->type);
     ASSERT_EQ(network_health::mojom::NetworkState::kNotConnected,
-              initial_network_health_state->networks[0]->state);
+              initial_network_health_state->networks[1]->state);
   }
 
   mojom::NetworkPtr GetNetworkHealthStateByType(

chrome/browser/resources/settings/chromeos/internet_page/internet_page.html

@@ -6,6 +6,7 @@
 <link rel="import" href="chrome://resources/cr_elements/cr_expand_button/cr_expand_button.html">
 <link rel="import" href="chrome://resources/cr_elements/cr_icon_button/cr_icon_button.html">
 <link rel="import" href="chrome://resources/cr_elements/icons.html">
+<link rel="import" href="chrome://resources/cr_elements/policy/cr_policy_indicator.html">
 <link rel="import" href="chrome://resources/html/i18n_behavior.html">
 <link rel="import" href="chrome://resources/html/web_ui_listener_behavior.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/iron-icon/iron-icon.html">
@@ -64,13 +65,23 @@
                       aria-labelledby="add-wifi-label"></cr-icon-button>
                 </div>
               </template>
-              <div actionable class="list-item" on-click="onAddVPNTap_">
+              <div actionable$="[[vpnIsEnabled_]]" class="list-item"
+                  on-click="onAddVPNTap_">
                 <div class="start settings-box-text"
                     id="add-vpn-label" aria-hidden="true">
                   $i18n{internetAddVPN}
                 </div>
+                <template is="dom-if" if="[[!vpnIsEnabled_]]">
+                  <cr-policy-indicator
+                      icon-aria-label="$i18n{networkVpnBuiltin}"
+                      indicator-type="devicePolicy"
+                      on-click="doNothing_">
+                  </cr-policy-indicator>
+                </template>
                 <cr-icon-button class="icon-add-circle"
-                    aria-labelledby="add-vpn-label"></cr-icon-button>
+                    aria-labelledby="add-vpn-label"
+                    disabled="[[!vpnIsEnabled_]]">
+                </cr-icon-button>
               </div>
               <template is="dom-repeat" items="[[vpnProviders_]]">
                 <div actionable class="list-item"

chrome/browser/resources/settings/chromeos/internet_page/internet_page.js

@@ -79,6 +79,15 @@ Polymer({
       value: false,
     },
 
+    /**
+     * False if VPN is disabled by policy.
+     * @private {boolean}
+     */
+    vpnIsEnabled_: {
+      type: Boolean,
+      value: false,
+    },
+
     /** @private {!chromeos.networkConfig.mojom.GlobalPolicy|undefined} */
     globalPolicy_: Object,
 
@@ -373,6 +382,11 @@ Polymer({
       this.managedNetworkAvailable = managedNetworkAvailable;
     }
 
+    const vpn = this.deviceStates[mojom.NetworkType.kVPN];
+    this.vpnIsEnabled_ = !!vpn &&
+        vpn.deviceState ===
+            chromeos.networkConfig.mojom.DeviceStateType.kEnabled;
+
     if (this.detailType_ && !this.deviceStates[this.detailType_]) {
       // If the device type associated with the current network has been
       // removed (e.g., due to unplugging a Cellular dongle), the details page,
@@ -408,9 +422,11 @@ Polymer({
 
   /** @private */
   onAddVPNTap_() {
-    this.showConfig_(
-        true /* configAndConnect */,
-        chromeos.networkConfig.mojom.NetworkType.kVPN);
+    if (this.vpnIsEnabled_) {
+      this.showConfig_(
+          true /* configAndConnect */,
+          chromeos.networkConfig.mojom.NetworkType.kVPN);
+    }
   },
 
   /**

chrome/browser/resources/settings/chromeos/internet_page/internet_subpage.html

@@ -7,6 +7,7 @@
 <link rel="import" href="chrome://resources/cr_elements/cr_link_row/cr_link_row.html">
 <link rel="import" href="chrome://resources/cr_elements/cr_toggle/cr_toggle.html">
 <link rel="import" href="chrome://resources/cr_elements/policy/cr_policy_network_behavior_mojo.html">
+<link rel="import" href="chrome://resources/cr_elements/policy/cr_policy_indicator.html">
 <link rel="import" href="chrome://resources/cr_elements/shared_style_css.html">
 <link rel="import" href="chrome://resources/cr_elements/shared_vars_css.html">
 <link rel="import" href="chrome://resources/html/i18n_behavior.html">
@@ -125,7 +126,12 @@
         <template is="dom-if" if="[[matchesType_('VPN', deviceState)]]">
           <div class="vpn-header layout horizontal center">
             <div class="flex settings-box-text">$i18n{networkVpnBuiltin}</div>
+            <template is="dom-if" if="[[!vpnIsEnabled_]]">
+              <cr-policy-indicator indicator-type="devicePolicy">
+              </cr-policy-indicator>
+            </template>
             <cr-icon-button class="icon-add-circle"
+                disabled="[[!vpnIsEnabled_]]"
                 aria-label="$i18n{internetAddVPN}" on-click="onAddButtonTap_"
                 tabindex$="[[tabindex]]"></cr-icon-button>
           </div>

chrome/browser/resources/settings/chromeos/internet_page/internet_subpage.js

@@ -111,6 +111,15 @@ Polymer({
       type: Boolean,
       value: false,
     },
+
+    /**
+     * False if VPN is disabled by policy.
+     * @private {boolean}
+     */
+    vpnIsEnabled_: {
+      type: Boolean,
+      value: false,
+    },
   },
 
   /** settings.RouteOriginBehavior override */
@@ -201,6 +210,12 @@ Polymer({
   /** @private */
   deviceStateChanged_() {
     if (this.deviceState !== undefined) {
+      // Set |vpnIsEnabled_| to be used for VPN special cases.
+      if (this.deviceState.type === mojom.NetworkType.kVPN) {
+        this.vpnIsEnabled_ = this.deviceState.deviceState ===
+            chromeos.networkConfig.mojom.DeviceStateType.kEnabled;
+      }
+
       // A scan has completed if the spinner was active (i.e., scanning was
       // active) and the device is no longer scanning.
       this.hasCompletedScanSinceLastEnabled_ = this.showSpinner &&
@@ -336,7 +351,7 @@ Polymer({
             if (!OncMojo.connectionStateIsConnected(state.connectionState)) {
               break;
             }
-            // Otherwise Arc VPNs are treated the same as Extension VPNs.
+          // Otherwise Arc VPNs are treated the same as Extension VPNs.
           case mojom.VpnType.kExtension:
             const providerId = state.typeState.vpn.providerId;
             thirdPartyVpns[providerId] = thirdPartyVpns[providerId] || [];
@@ -401,13 +416,18 @@ Polymer({
 
   /**
    * @param {!OncMojo.DeviceStateProperties|undefined} deviceState
-   * @return {boolean} Whether or not the device state is enabled.
+   * @return {boolean} True if the device is enabled or if it is a VPN.
+   *     Note: This function will always return true for VPN because VPNs can be
+   *     disabled by policy only for built-in VPNs (OpenVPN & L2TP). So even
+   *     when VPNs are disabled by policy; the VPN network summary item should
+   *     still be visible and actionable to show details for other VPN
+   *     providers.
    * @private
    */
   deviceIsEnabled_(deviceState) {
     return !!deviceState &&
-        deviceState.deviceState ==
-        chromeos.networkConfig.mojom.DeviceStateType.kEnabled;
+        (deviceState.type == mojom.NetworkType.kVPN ||
+         deviceState.deviceState == mojom.DeviceStateType.kEnabled);
   },
 
   /**
@@ -684,9 +704,23 @@ Polymer({
    * @private
    */
   shouldShowNetworkList_(networkStateList) {
+    if (!!this.deviceState &&
+        this.deviceState.type === mojom.NetworkType.kVPN) {
+      return this.shouldShowVpnList_(networkStateList);
+    }
     return networkStateList.length > 0;
   },
 
+  /**
+   * @param {!Array<!OncMojo.NetworkStateProperties>} networkStateList
+   * @return {boolean} True if native VPN is not disabled by policy and there
+   *     are more than one VPN network configured.
+   * @private
+   */
+  shouldShowVpnList_(networkStateList) {
+    return this.vpnIsEnabled_ && networkStateList.length > 0;
+  },
+
   /**
    * @param {!OncMojo.DeviceStateProperties|undefined} deviceState
    * @param {!OncMojo.DeviceStateProperties|undefined} tetherDeviceState

chrome/browser/resources/settings/chromeos/internet_page/network_summary.js

@@ -205,13 +205,6 @@ Polymer({
 
     this.defaultNetwork = firstConnectedNetwork;
 
-    // Create a VPN entry in deviceStates if there are any VPN networks.
-    if (newNetworkStateLists[mojom.NetworkType.kVPN].length > 0) {
-      newDeviceStates[mojom.NetworkType.kVPN] = {
-        type: mojom.NetworkType.kVPN,
-        deviceState: chromeos.networkConfig.mojom.DeviceStateType.kEnabled,
-      };
-    }
 
     // Push the active networks onto newActiveNetworkStates in order based on
     // device priority, creating an empty state for devices with no networks.
@@ -223,6 +216,14 @@ Polymer({
         continue;  // The technology for this device type is unavailable.
       }
 
+      // A VPN device state will always exist in |deviceStateList| even if there
+      // is no active VPN. This check is to add the VPN network summary item
+      // only if there is at least one active VPN.
+      if (device.type === mojom.NetworkType.kVPN &&
+          !activeNetworkStatesByType.has(device.type)) {
+        continue;
+      }
+
       // If both 'Tether' and 'Cellular' technologies exist, merge the network
       // lists and do not add an active network for 'Tether' so that there is
       // only one 'Mobile data' section / subpage.

chrome/browser/resources/settings/chromeos/internet_page/network_summary_item.html

@@ -64,10 +64,9 @@
 
         <template is="dom-if" if="[[showPolicyIndicator_(activeNetworkState)]]">
           <cr-policy-indicator
-            icon-aria-label="[[getTitleText_(activeNetworkState)]]"
-            indicator-type="[[getIndicatorTypeForSource(
-                                activeNetworkState.source)]]"
-            on-click="doNothing_">
+              icon-aria-label="[[getTitleText_(activeNetworkState)]]"
+              indicator-type="[[getPolicyIndicatorType_(activeNetworkState)]]"
+              on-click="doNothing_">
           </cr-policy-indicator>
         </template>
 

chrome/browser/resources/settings/chromeos/internet_page/network_summary_item.js

@@ -160,7 +160,23 @@ Polymer({
     return (activeNetworkState !== undefined &&
             OncMojo.connectionStateIsConnected(
                 activeNetworkState.connectionState)) ||
-        this.isPolicySource(activeNetworkState.source);
+        this.isPolicySource(activeNetworkState.source) ||
+        this.isProhibitedVpn_();
+  },
+
+  /**
+   * @param {!OncMojo.NetworkStateProperties} activeNetworkState
+   * @return {!CrPolicyIndicatorType} Device policy indicator for VPN when
+   *     disabled by policy and an indicator corresponding to the source of the
+   *     active network state otherwise.
+   * @private
+   */
+  getPolicyIndicatorType_(activeNetworkState) {
+    if (this.isProhibitedVpn_()) {
+      return this.getIndicatorTypeForSource(
+          chromeos.networkConfig.mojom.OncSource.kDevicePolicy);
+    }
+    return this.getIndicatorTypeForSource(activeNetworkState.source);
   },
 
   /**
@@ -196,12 +212,17 @@ Polymer({
 
   /**
    * @param {!OncMojo.DeviceStateProperties|undefined} deviceState
-   * @return {boolean} Whether or not the device state is enabled.
+   * @return {boolean} True if the device is enabled or if it is a VPN. Note:
+   *     This function will always return true for VPNs because VPNs can be
+   *     disabled by policy only for built-in VPNs (OpenVPN & L2TP), but always
+   *     enabled for other VPN providers. To know whether built-in VPNs are
+   *     disabled, use builtInVpnProhibited_() instead.
    * @private
    */
   deviceIsEnabled_(deviceState) {
     return !!deviceState &&
-        deviceState.deviceState == mojom.DeviceStateType.kEnabled;
+        (deviceState.type == mojom.NetworkType.kVPN ||
+         deviceState.deviceState == mojom.DeviceStateType.kEnabled);
   },
 
   /**
@@ -262,13 +283,75 @@ Polymer({
   },
 
   /**
-   * @param {!OncMojo.NetworkStateProperties} activeNetworkState
+   * @return {boolean} True if VPNs are disabled by policy and the current
+   *     device is VPN.
+   * @private
+   */
+  isProhibitedVpn_() {
+    return !!this.deviceState &&
+        this.deviceState.type === mojom.NetworkType.kVPN &&
+        this.builtInVpnProhibited_(this.deviceState);
+  },
+
+  /**
+   * @param {!chromeos.networkConfig.mojom.VpnType} vpnType
+   * @return {boolean}
+   * @private
+   */
+  isBuiltInVpnType_(vpnType) {
+    return vpnType === chromeos.networkConfig.mojom.VpnType.kL2TPIPsec ||
+        vpnType === chromeos.networkConfig.mojom.VpnType.kOpenVPN;
+  },
+
+  /**
+   * @param {!Array<!OncMojo.NetworkStateProperties>} networkStateList
+   * @return {boolean} True if at least one non-native VPN is configured.
+   * @private
+   */
+  hasNonBuiltInVpn_(networkStateList) {
+    const nonBuiltInVpnIndex = networkStateList.findIndex((networkState) => {
+      return !this.isBuiltInVpnType_(networkState.typeState.vpn.type);
+    });
+    return nonBuiltInVpnIndex !== -1;
+  },
+
+  /**
+   * @param {!OncMojo.DeviceStateProperties|undefined} deviceState
+   * @return {boolean} True if the built-in VPNs are disabled by policy.
+   * @private
+   */
+  builtInVpnProhibited_(deviceState) {
+    return !!deviceState &&
+        deviceState.deviceState ===
+        chromeos.networkConfig.mojom.DeviceStateType.kProhibited;
+  },
+
+  /**
+   * @param {!OncMojo.DeviceStateProperties|undefined} deviceState
+   * @param {!Array<!OncMojo.NetworkStateProperties>} networkStateList
+   * @return {boolean} True if there is any configured VPN for a non-disabled
+   *     VPN provider. Note: Only built-in VPN providers can be disabled by
+   *     policy at the moment.
+   * @private
+   */
+  anyVpnExists_(deviceState, networkStateList) {
+    return this.hasNonBuiltInVpn_(networkStateList) ||
+        (!this.builtInVpnProhibited_(deviceState) &&
+         networkStateList.length > 0);
+  },
+
+  /**
+   * @param {!OncMojo.NetworkStateProperties|undefined} activeNetworkState
    * @param {!OncMojo.DeviceStateProperties|undefined} deviceState
    * @param {!Array<!OncMojo.NetworkStateProperties>} networkStateList
    * @return {boolean}
    * @private
    */
   showDetailsIsVisible_(activeNetworkState, deviceState, networkStateList) {
+    if (!!deviceState && deviceState.type == mojom.NetworkType.kVPN) {
+      return this.anyVpnExists_(deviceState, networkStateList);
+    }
+
     return this.deviceIsEnabled_(deviceState) &&
         (!!activeNetworkState.guid || networkStateList.length > 0);
   },
@@ -290,11 +373,13 @@ Polymer({
       // available, even if there are currently no associated networks.
       return true;
     }
+
+    if (type === mojom.NetworkType.kVPN) {
+      return this.anyVpnExists_(deviceState, networkStateList);
+    }
+
     let minlen;
-    if (type == mojom.NetworkType.kVPN) {
-      // VPN subpage provides provider info so show if there are any networks.
-      minlen = 1;
-    } else if (type == mojom.NetworkType.kWiFi) {
+    if (type == mojom.NetworkType.kWiFi) {
       // WiFi subpage includes 'Known Networks' so always show, even if the
       // technology is still enabling / scanning, or none are visible.
       minlen = 0;
@@ -306,6 +391,10 @@ Polymer({
   },
 
   /**
+   * This handles clicking the network summary item row. Clicking this row can
+   * lead to toggling device enablement or showing the corresponding networks
+   * list or showing details about a network or doing nothing based on the
+   * device and networks states.
    * @param {!Event} event The enable button event.
    * @private
    */
@@ -318,10 +407,14 @@ Polymer({
     } else if (this.shouldShowSubpage_(
                    this.deviceState, this.networkStateList)) {
       this.fire('show-networks', this.deviceState.type);
-    } else if (this.activeNetworkState.guid) {
-      this.fire('show-detail', this.activeNetworkState);
-    } else if (this.networkStateList.length > 0) {
-      this.fire('show-detail', this.networkStateList[0]);
+    } else if (this.showDetailsIsVisible_(
+                   this.activeNetworkState, this.deviceState,
+                   this.networkStateList)) {
+      if (this.activeNetworkState.guid) {
+        this.fire('show-detail', this.activeNetworkState);
+      } else if (this.networkStateList.length > 0) {
+        this.fire('show-detail', this.networkStateList[0]);
+      }
     }
     event.stopPropagation();
   },
@@ -346,8 +439,8 @@ Polymer({
     // Item is actionable if tapping should show either networks subpage or the
     // network details page.
     return this.shouldShowSubpage_(this.deviceState, this.networkStateList) ||
-        !!(this.activeNetworkState && this.activeNetworkState.guid) ||
-        this.networkStateList.length > 0;
+        this.showDetailsIsVisible_(
+            activeNetworkState, deviceState, networkStateList);
   },
 
   /**

chromeos/network/managed_network_configuration_handler_impl.cc

@@ -598,7 +598,7 @@ void ManagedNetworkConfigurationHandlerImpl::SetPolicy(
           ::onc::global_network_config::kDisableNetworkTypes,
           &prohibited_list) &&
       prohibited_technologies_handler_) {
-    // Prohibited technologies are only allowed in user policy.
+    // Prohibited technologies are only allowed in device policy.
     DCHECK_EQ(::onc::ONC_SOURCE_DEVICE_POLICY, onc_source);
 
     prohibited_technologies_handler_->SetProhibitedTechnologies(

chromeos/network/network_connection_handler_impl.cc

@@ -20,10 +20,13 @@
 #include "chromeos/network/managed_network_configuration_handler.h"
 #include "chromeos/network/network_configuration_handler.h"
 #include "chromeos/network/network_event_log.h"
+#include "chromeos/network/network_handler.h"
 #include "chromeos/network/network_profile_handler.h"
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
+#include "chromeos/network/network_type_pattern.h"
 #include "chromeos/network/network_util.h"
+#include "chromeos/network/prohibited_technologies_handler.h"
 #include "chromeos/network/shill_property_util.h"
 #include "dbus/object_path.h"
 #include "net/cert/x509_certificate.h"
@@ -137,6 +140,17 @@ std::string GetDefaultUserProfilePath(const NetworkState* network) {
                  : NetworkProfileHandler::GetSharedProfilePath();
 }
 
+bool IsVpnProhibited() {
+  if (!NetworkHandler::IsInitialized())
+    return false;
+
+  std::vector<std::string> prohibited_technologies =
+      NetworkHandler::Get()
+          ->prohibited_technologies_handler()
+          ->GetCurrentlyProhibitedTechnologies();
+  return base::Contains(prohibited_technologies, shill::kTypeVPN);
+}
+
 }  // namespace
 
 NetworkConnectionHandlerImpl::ConnectRequest::ConnectRequest(
@@ -295,6 +309,13 @@ void NetworkConnectionHandlerImpl::ConnectToNetwork(
       }
       return;
     }
+
+    if (NetworkTypePattern::VPN().MatchesType(network->type()) &&
+        IsVpnProhibited()) {
+      InvokeConnectErrorCallback(service_path, error_callback,
+                                 kErrorBlockedByPolicy);
+      return;
+    }
   }
 
   // If the network does not have a profile path, specify the correct default

chromeos/network/network_connection_handler_impl_unittest.cc

@@ -12,16 +12,19 @@
 #include "base/files/file_util.h"
 #include "base/json/json_reader.h"
 #include "base/macros.h"
+#include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/test/task_environment.h"
 #include "chromeos/network/managed_network_configuration_handler_impl.h"
 #include "chromeos/network/network_cert_loader.h"
 #include "chromeos/network/network_configuration_handler.h"
 #include "chromeos/network/network_connection_observer.h"
+#include "chromeos/network/network_handler.h"
 #include "chromeos/network/network_profile_handler.h"
 #include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/network_state_test_helper.h"
 #include "chromeos/network/onc/onc_utils.h"
+#include "chromeos/network/prohibited_technologies_handler.h"
 #include "components/onc/onc_constants.h"
 #include "crypto/scoped_nss_types.h"
 #include "crypto/scoped_test_nss_db.h"
@@ -32,6 +35,7 @@
 #include "net/test/test_data_directory.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
+#include "third_party/cros_system_api/dbus/shill/dbus-constants.h"
 
 namespace chromeos {
 
@@ -591,6 +595,35 @@ TEST_F(NetworkConnectionHandlerImplTest, ConnectToTetherNetwork_Failure) {
             network_connection_observer()->GetResult(kTetherGuid));
 }
 
+TEST_F(NetworkConnectionHandlerImplTest,
+       ConnectToVpnNetworkWhenProhibited_Failure) {
+  // This test tests a code that accesses NetworkHandler::Get() directly (when
+  // checking if VPN is disabled by policy when attempting to connect to a VPN
+  // network), so NetworkStateTestHelper can not be used here. That's because
+  // NetworkStateTestHelper initializes a NetworkStateHandler for testing, but
+  // NetworkHandler::Initialize() constructs its own NetworkStateHandler
+  // instance and NetworkHandler::Get() uses it.
+  NetworkHandler::Initialize();
+  NetworkHandler::Get()
+      ->prohibited_technologies_handler()
+      ->AddGloballyProhibitedTechnology(shill::kTypeVPN);
+
+  const std::string kVpnGuid = "vpn_guid";
+  const std::string kShillJsonStringTemplate =
+      R"({"GUID": "$1", "Type": "vpn", "State": "idle",
+            "Provider": {"Type": "l2tpipsec", "Host": "host"}})";
+
+  const std::string shill_json_string =
+      base::ReplaceStringPlaceholders(kShillJsonStringTemplate, {kVpnGuid},
+                                      /*offsets=*/nullptr);
+  const std::string vpn_service_path = ConfigureService(shill_json_string);
+  ASSERT_FALSE(vpn_service_path.empty());
+
+  Connect(/*service_path=*/vpn_service_path);
+  EXPECT_EQ(NetworkConnectionHandler::kErrorBlockedByPolicy,
+            GetResultAndReset());
+}
+
 TEST_F(NetworkConnectionHandlerImplTest,
        ConnectToTetherNetwork_NoTetherDelegate) {
   network_state_handler()->SetTetherTechnologyState(

chromeos/network/onc/onc_validator.cc

@@ -1028,8 +1028,9 @@ bool Validator::ValidateGlobalNetworkConfiguration(
   // Ensure the list contains only legitimate network type identifiers.
   const std::vector<const char*> valid_network_type_values = {
       ::onc::network_config::kCellular, ::onc::network_config::kEthernet,
-      ::onc::network_config::kWiFi, ::onc::network_config::kWimaxDeprecated,
-      ::onc::network_config::kTether};
+      ::onc::network_config::kTether,   ::onc::network_config::kWiFi,
+      ::onc::network_config::kVPN,      ::onc::network_config::kWimaxDeprecated,
+  };
   if (!ListFieldContainsValidValues(
           *result, ::onc::global_network_config::kDisableNetworkTypes,
           valid_network_type_values)) {

chromeos/network/prohibited_technologies_handler.h

@@ -34,7 +34,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) ProhibitedTechnologiesHandler
   void SetProhibitedTechnologies(const base::ListValue* prohibited_list);
   // Functions for updating the list of technologies that are prohibited
   // everywhere, including login screen
-  void AddGloballyProhibitedTechnology(const std::string& prohibited_list);
+  void AddGloballyProhibitedTechnology(const std::string& technology);
   void RemoveGloballyProhibitedTechnology(const std::string& technology);
   // Returns the currently active list of prohibited
   // technologies(session-dependent and globally-prohibited ones)

chromeos/services/network_config/cros_network_config.cc

@@ -4,6 +4,8 @@
 
 #include "chromeos/services/network_config/cros_network_config.h"
 
+#include <vector>
+
 #include "base/strings/string_util.h"
 #include "chromeos/components/sync_wifi/network_eligibility_checker.h"
 #include "chromeos/login/login_state/login_state.h"
@@ -18,6 +20,7 @@
 #include "chromeos/network/network_type_pattern.h"
 #include "chromeos/network/network_util.h"
 #include "chromeos/network/onc/onc_translation_tables.h"
+#include "chromeos/network/prohibited_technologies_handler.h"
 #include "chromeos/network/proxy/ui_proxy_config_service.h"
 #include "chromeos/services/network_config/public/cpp/cros_network_config_util.h"
 #include "chromeos/services/network_config/public/mojom/cros_network_config_mojom_traits.h"
@@ -26,6 +29,7 @@
 #include "components/user_manager/user_manager.h"
 #include "net/base/ip_address.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
+#include "third_party/cros_system_api/dbus/shill/dbus-constants.h"
 
 using user_manager::UserManager;
 
@@ -249,6 +253,24 @@ const std::string& GetVpnProviderName(
   return base::EmptyString();
 }
 
+mojom::DeviceStatePropertiesPtr GetVpnState() {
+  auto result = mojom::DeviceStateProperties::New();
+  result->type = mojom::NetworkType::kVPN;
+
+  bool vpn_disabled = false;
+  if (NetworkHandler::IsInitialized()) {
+    std::vector<std::string> prohibited_technologies =
+        NetworkHandler::Get()
+            ->prohibited_technologies_handler()
+            ->GetCurrentlyProhibitedTechnologies();
+    vpn_disabled = base::Contains(prohibited_technologies, shill::kTypeVPN);
+  }
+
+  result->device_state = vpn_disabled ? mojom::DeviceStateType::kProhibited
+                                      : mojom::DeviceStateType::kEnabled;
+  return result;
+}
+
 mojom::NetworkStatePropertiesPtr NetworkStateToMojo(
     NetworkStateHandler* network_state_handler,
     const std::vector<mojom::VpnProviderPtr>& vpn_providers,
@@ -1825,6 +1847,12 @@ void CrosNetworkConfig::GetDeviceStateList(
     if (mojo_device)
       result.emplace_back(std::move(mojo_device));
   }
+
+  // Handle VPN state separately because VPN is not considered a device by shill
+  // and thus will not be included in the |devices| list returned by network
+  // state handler. In the UI code, it is treated as a "device" for consistency.
+  result.emplace_back(GetVpnState());
+
   std::move(callback).Run(std::move(result));
 }
 

chromeos/services/network_config/cros_network_config_unittest.cc

@@ -601,7 +601,7 @@ TEST_F(CrosNetworkConfigTest, GetNetworkStateList) {
 
 TEST_F(CrosNetworkConfigTest, GetDeviceStateList) {
   std::vector<mojom::DeviceStatePropertiesPtr> devices = GetDeviceStateList();
-  ASSERT_EQ(3u, devices.size());
+  ASSERT_EQ(4u, devices.size());
   EXPECT_EQ(mojom::NetworkType::kWiFi, devices[0]->type);
   EXPECT_EQ(mojom::DeviceStateType::kEnabled, devices[0]->device_state);
 
@@ -627,12 +627,16 @@ TEST_F(CrosNetworkConfigTest, GetDeviceStateList) {
   EXPECT_EQ(shill::kSIMLockPin, cellular->sim_lock_status->lock_type);
   EXPECT_EQ(3, cellular->sim_lock_status->retries_left);
 
+  mojom::DeviceStateProperties* vpn = devices[3].get();
+  EXPECT_EQ(mojom::NetworkType::kVPN, vpn->type);
+  EXPECT_EQ(mojom::DeviceStateType::kEnabled, vpn->device_state);
+
   // Disable WiFi
   helper().network_state_handler()->SetTechnologyEnabled(
       NetworkTypePattern::WiFi(), false, network_handler::ErrorCallback());
   base::RunLoop().RunUntilIdle();
   devices = GetDeviceStateList();
-  ASSERT_EQ(3u, devices.size());
+  ASSERT_EQ(4u, devices.size());
   EXPECT_EQ(mojom::NetworkType::kWiFi, devices[0]->type);
   EXPECT_EQ(mojom::DeviceStateType::kDisabled, devices[0]->device_state);
 }
@@ -899,7 +903,7 @@ TEST_F(CrosNetworkConfigTest, ForgetNetwork) {
 
 TEST_F(CrosNetworkConfigTest, SetNetworkTypeEnabledState) {
   std::vector<mojom::DeviceStatePropertiesPtr> devices = GetDeviceStateList();
-  ASSERT_EQ(3u, devices.size());
+  ASSERT_EQ(4u, devices.size());
   EXPECT_EQ(mojom::NetworkType::kWiFi, devices[0]->type);
   EXPECT_EQ(mojom::DeviceStateType::kEnabled, devices[0]->device_state);
 
@@ -914,7 +918,7 @@ TEST_F(CrosNetworkConfigTest, SetNetworkTypeEnabledState) {
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(succeeded);
   devices = GetDeviceStateList();
-  ASSERT_EQ(3u, devices.size());
+  ASSERT_EQ(4u, devices.size());
   EXPECT_EQ(mojom::NetworkType::kWiFi, devices[0]->type);
   EXPECT_EQ(mojom::DeviceStateType::kDisabled, devices[0]->device_state);
 }

components/onc/docs/onc_spec.md

@@ -191,11 +191,13 @@ warns admins of the implications of mis-using this policy for Chrome OS.
 * **DisableNetworkTypes**
     * (optional) - **array of string**
     * Allowed values are:
-        * Cellular
-        * Ethernet
-        * WiFi
-        * Tether
+        * *Cellular*
+        * *Ethernet*
+        * *WiFi*
+        * *Tether*
+        * *VPN*
     * List of strings containing disabled network interfaces.
+    * Adding *VPN* to the list will only disable Chrome OS built-in VPN.
 
 ## Network Configuration
 

ui/webui/resources/cr_elements/policy/cr_policy_network_behavior_mojo.js

@@ -102,7 +102,7 @@ const CrPolicyNetworkBehaviorMojo = {
   /**
    * @param {!chromeos.networkConfig.mojom.OncSource} source
    * @return {!CrPolicyIndicatorType}
-   * @private
+   * @protected
    */
   getIndicatorTypeForSource(source) {
     if (source === chromeos.networkConfig.mojom.OncSource.kDevicePolicy) {