Enable client certificate patterns in device ONC policy

chrome/browser/chromeos/enrollment_dialog_view.cc

Index: chrome/browser/chromeos/enrollment_dialog_view.cc
diff --git a/chrome/browser/chromeos/enrollment_dialog_view.cc b/chrome/browser/chromeos/enrollment_dialog_view.cc
index 33f3af5a4bfdb2e073d95eb3517c9a7d5c812f09..96f342fc65dbbe2e39750acafeb0c5b4857f1690 100644
--- a/chrome/browser/chromeos/enrollment_dialog_view.cc
+++ b/chrome/browser/chromeos/enrollment_dialog_view.cc
@@ -16,6 +16,7 @@
 #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"
@@ -251,6 +252,37 @@ 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
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -270,6 +302,8 @@ bool CreateEnrollmentDialog(const std::string& network_id,
   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;
@@ -278,14 +312,11 @@ bool CreateEnrollmentDialog(const std::string& network_id,
           ->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;