Enable client certificate patterns in device ONC policy

chrome/browser/chromeos/enrollment_dialog_view.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/chromeos/enrollment_dialog_view.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/ash/system_tray_client.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_navigator_params.h"
 #include "chrome/grit/generated_resources.h"
+#include "chromeos/login/login_state.h"
 #include "chromeos/network/client_cert_util.h"
 #include "chromeos/network/managed_network_configuration_handler.h"
 #include "chromeos/network/network_event_log.h"
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
 #include "extensions/browser/extension_host.h"
 #include "extensions/common/constants.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/page_transition_types.h"
 #include "ui/views/controls/label.h"
@@ skipping 215 matching lines @@
 
   // No appropriate scheme was found.
   NET_LOG_ERROR("No usable enrollment URI", network_name_);
   return false;
 }
 
 void EnrollmentComplete(const std::string& network_id) {
   NET_LOG_USER("Enrollment Complete", network_id);
 }
 
+// Decides if the enrollment dialog is allowed in the current login state.
+bool EnrollmentDialogAllowed() {
+  chromeos::LoginState::LoggedInUserType user_type =
+      LoginState::Get()->GetLoggedInUserType();
+  switch (user_type) {
+    case LoginState::LOGGED_IN_USER_NONE:
+      // Enrollment on the sign-in screen would not work anyway because we have
+      // no extensions there yet and no PKCS11 token is loaded.

[emaxx, 2017/04/25 15:15:58]

nit: The part "because we have no extensions there yet" will change soon, but
this comment will likely stay here unnoticed. Also, maybe something else may
prevent us from enabling this on the login screen - like the UI restrictions.
So I'd rather recommend not to have this detailed comment here. If you want to
document the reasoning here against the current state of things, the crbug or
the Design Doc will be a better place for this, IMO.

[pmarko, 2017/04/25 16:59:57]

Done. (You're right - I've dropped the comments here. They're just waiting to be
outdated/incorrect).

+      return false;
+    case LoginState::LOGGED_IN_USER_REGULAR:
+      return true;

[emaxx, 2017/04/25 15:15:58]

I'm afraid this potentially opens the possibility for showing the enrollment
dialog on the lock screen and maybe in some other scenarios.
So I think that the check against IsSigninProfile(browser) is still required.

[pmarko, 2017/04/25 16:59:57]

Done. (Added back IsSigninProfile check in the beginning of this function)

+    case LoginState::LOGGED_IN_USER_OWNER:
+      return true;
+    case LoginState::LOGGED_IN_USER_GUEST:
+      return true;
+    case LoginState::LOGGED_IN_USER_PUBLIC_ACCOUNT:
+      // Not allowed for now because we haven't tested this.

[emaxx, 2017/04/25 15:15:58]

nit: I think it's better not to track in the code what is tested and what is
not; moreover, this may also be influenced by a PM decision. So crbugs work
better for tracking this stuff (and you won't have to file another CL if the
resulting decision is to keep "false" :) ).
So I'd recommend to drop this comment unless you can come up with a more neutral
form.

[pmarko, 2017/04/25 16:59:57]

Done.

+      return false;
+    case LoginState::LOGGED_IN_USER_SUPERVISED:
+      return true;
+    case LoginState::LOGGED_IN_USER_KIOSK_APP:
+      // We don't want to show dialogs on kiosk.
+      return false;
+    case LoginState::LOGGED_IN_USER_ARC_KIOSK_APP:
+      // We don't want to show dialogs on kiosk.
+      return false;
+  }
+  NOTREACHED();
+  return false;
+}
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 // Factory function.
 
 namespace enrollment {
 
 bool CreateEnrollmentDialog(const std::string& network_id,
                             gfx::NativeWindow owning_window) {
   const NetworkState* network =
       NetworkHandler::Get()->network_state_handler()->GetNetworkStateFromGuid(
           network_id);
   if (!network) {
     NET_LOG_ERROR("Enrolling Unknown network", network_id);
     return false;
   }
   Browser* browser = chrome::FindBrowserWithWindow(owning_window);
   Profile* profile =
       browser ? browser->profile() : ProfileManager::GetPrimaryUserProfile();
+  if (!EnrollmentDialogAllowed())
+    return false;
   std::string username_hash = ProfileHelper::GetUserIdHashFromProfile(profile);
 
   onc::ONCSource onc_source = onc::ONC_SOURCE_NONE;
   const base::DictionaryValue* policy =
       NetworkHandler::Get()
           ->managed_network_configuration_handler()
           ->FindPolicyByGUID(username_hash, network_id, &onc_source);
 
-  // We skip certificate patterns for device policy ONC so that an unmanaged
-  // user can't get to the place where a cert is presented for them
-  // involuntarily.
-  if (!policy || onc_source == onc::ONC_SOURCE_DEVICE_POLICY)
+  if (!policy)
     return false;
 
   client_cert::ClientCertConfig cert_config;
-  OncToClientCertConfig(*policy, &cert_config);
+  OncToClientCertConfig(onc_source, *policy, &cert_config);
 
   if (cert_config.client_cert_type != onc::client_cert::kPattern)
     return false;
 
   if (cert_config.pattern.Empty())
     NET_LOG_ERROR("Certificate pattern is empty", network_id);
 
   if (cert_config.pattern.enrollment_uri_list().empty()) {
     NET_LOG_EVENT("No enrollment URIs", network_id);
     return false;
   }
 
   NET_LOG_USER("Enrolling", network_id);
 
   DialogEnrollmentDelegate* enrollment =
       new DialogEnrollmentDelegate(owning_window, network->name(), profile);
   return enrollment->Enroll(cert_config.pattern.enrollment_uri_list(),
                             base::Bind(&EnrollmentComplete, network_id));
 }
 
 }  // namespace enrollment
 
 }  // namespace chromeos