Reauthenticate the user before launching Smart Lock setup app.

chrome/browser/signin/easy_unlock_service_regular.cc

Index: chrome/browser/signin/easy_unlock_service_regular.cc
diff --git a/chrome/browser/signin/easy_unlock_service_regular.cc b/chrome/browser/signin/easy_unlock_service_regular.cc
index 870d6636930e72c51c08b18969734a605375182e..b73f246bc3c245d4c45b5c5f3d6a2b9b8dbe744e 100644
--- a/chrome/browser/signin/easy_unlock_service_regular.cc
+++ b/chrome/browser/signin/easy_unlock_service_regular.cc
@@ -18,9 +18,15 @@
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/pref_names.h"
 #include "components/pref_registry/pref_registry_syncable.h"
+#include "content/public/browser/browser_thread.h"
 #include "extensions/browser/extension_system.h"
 
 #if defined(OS_CHROMEOS)
+#include "apps/app_lifetime_monitor_factory.h"
+#include "base/thread_task_runner_handle.h"
+#include "chrome/browser/chromeos/login/easy_unlock/easy_unlock_key_manager.h"
+#include "chrome/browser/chromeos/login/easy_unlock/easy_unlock_reauth.h"
+#include "chrome/browser/chromeos/login/session/user_session_manager.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "components/user_manager/user_manager.h"
 #endif
@@ -40,7 +46,8 @@ const char kKeyPhoneId[] = "permitRecord.id";
 
 EasyUnlockServiceRegular::EasyUnlockServiceRegular(Profile* profile)
     : EasyUnlockService(profile),
-      turn_off_flow_status_(EasyUnlockService::IDLE) {
+      turn_off_flow_status_(EasyUnlockService::IDLE),
+      weak_ptr_factory_(this) {
 }
 
 EasyUnlockServiceRegular::~EasyUnlockServiceRegular() {
@@ -55,6 +62,53 @@ std::string EasyUnlockServiceRegular::GetUserEmail() const {
 }
 
 void EasyUnlockServiceRegular::LaunchSetup() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+#if defined(OS_CHROMEOS)
+  // Force the user to reauthenticate by showing a modal overlay (similar to the
+  // lock screen). The password obtained from the reauth is cached for a short
+  // period of time and used to create the cryptohome keys for sign-in.
+  if (short_lived_user_context_ && short_lived_user_context_->user_context()) {

[tbarzic, 2014/11/01 23:21:45]

maybe we could also check that the credentials are not just about to get expired
(or maybe reset the deletion timeout); not necessarily in this cl

[tbarzic, 2014/11/01 23:40:51]

you can ignore this (we should not be keeping user context from a stale setup
attempt, right).

[Tim Song, 2014/11/03 19:10:43]

Acknowledged

+    OpenSetupApp();
+  } else {
+    bool reauth_success = chromeos::EasyUnlockReauth::ReauthForUserContext(
+        base::Bind(&EasyUnlockServiceRegular::OnUserContextFromReauth,
+                   weak_ptr_factory_.GetWeakPtr()));
+    if (!reauth_success)
+      OpenSetupApp();
+  }
+#else
+  OpenSetupApp();
+#endif
+}
+
+#if defined(OS_CHROMEOS)
+void EasyUnlockServiceRegular::OnUserContextFromReauth(
+    const chromeos::UserContext& user_context) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  short_lived_user_context_.reset(new chromeos::ShortLivedUserContext(
+      user_context,
+      apps::AppLifetimeMonitorFactory::GetForProfile(profile()),
+      base::ThreadTaskRunnerHandle::Get().get()));
+
+  OpenSetupApp();
+}
+
+void EasyUnlockServiceRegular::OnKeysRefreshedForSetDevices(bool success) {
+  // If the keys were refreshed successfully, the hardlock state should be
+  // cleared, so Smart Lock can be used normally. Otherwise, we fall back to
+  // a hardlock state to force the user to type in their credentials again.
+  if (success) {
+    SetHardlockStateForUser(GetUserEmail(),
+                            EasyUnlockScreenlockStateHandler::NO_HARDLOCK);
+  }
+
+  // Even if the keys refresh suceeded, we still fetch the cryptohome keys as a
+  // sanity check.
+  CheckCryptohomeKeysAndMaybeHardlock();
+}
+#endif
+
+void EasyUnlockServiceRegular::OpenSetupApp() {
   ExtensionService* service =
       extensions::ExtensionSystem::Get(profile())->extension_service();
   const extensions::Extension* extension =
@@ -103,7 +157,31 @@ void EasyUnlockServiceRegular::SetRemoteDevices(
   DictionaryPrefUpdate pairing_update(profile()->GetPrefs(),
                                       prefs::kEasyUnlockPairing);
   pairing_update->SetWithoutPathExpansion(kKeyDevices, devices.DeepCopy());
+
+#if defined(OS_CHROMEOS)
+  if (short_lived_user_context_ && short_lived_user_context_->user_context() &&
+      !devices.empty()) {
+    // We may already have the password cached, so proceed to create the

[tbarzic, 2014/11/01 23:21:45]

my main concern here is that SetRemoteDevices could get called by sync logic
while the app window is still open (In particular if the user does not finish
the setup). We'd treat it the same as if the user set up those devices during
the setup.
I'm not sure that we can handle this without more context from the app, though.
Can you at least open a bug for this and fix it in a follow up?

[tbarzic, 2014/11/01 23:29:05]

actually, a workaround could be to check whether SetRemoteDevice was called from
the background page (which would mean that it's set by sync).
I think this should work for determining if the function was called from the
background page (in EasyUnlockPrivateFunction implementation):
content::WebContents::FromRenderViewHost(render_view_host())->GetDelegate()->IsNeverVisible().

I'm not completely sure this would work, though :)

[tbarzic, 2014/11/03 22:50:13]

any update on this? (as I said, at least a TODO would be nice)

[Tim Song, 2014/11/04 00:23:08]

Sorry, missed these comments.

I'm not sure if this is actually a problem. I'll put a TODO to look into this,
but I don't think the privacy/security concerns really apply in this case.

+    // cryptohome keys for sign-in or the system will be hardlocked.
+    chromeos::UserContext* user_context =
+        short_lived_user_context_->user_context();
+    chromeos::EasyUnlockKeyManager* key_manager =
+        chromeos::UserSessionManager::GetInstance()->GetEasyUnlockKeyManager();
+
+    // Set the hardlock state to pairing_changed here
+    SetHardlockStateForUser(GetUserEmail(),

[tbarzic, 2014/11/01 23:21:45]

why is this needed here?

[tbarzic, 2014/11/03 22:50:13]

how about this one?

[Tim Song, 2014/11/04 00:23:08]

Done. Sorry that was a left over.

+                            EasyUnlockScreenlockStateHandler::PAIRING_CHANGED);
+    key_manager->RefreshKeys(
+        *user_context,
+        devices,
+        base::Bind(&EasyUnlockServiceRegular::OnKeysRefreshedForSetDevices,
+                   weak_ptr_factory_.GetWeakPtr()));
+  } else {
+    CheckCryptohomeKeysAndMaybeHardlock();
+  }
+#else
   CheckCryptohomeKeysAndMaybeHardlock();
+#endif
 }
 
 void EasyUnlockServiceRegular::ClearRemoteDevices() {
@@ -184,6 +262,10 @@ void EasyUnlockServiceRegular::InitializeInternal() {
 }
 
 void EasyUnlockServiceRegular::ShutdownInternal() {
+#if defined(OS_CHROMEOS)
+  short_lived_user_context_.reset();
+#endif
+
   turn_off_flow_.reset();
   turn_off_flow_status_ = EasyUnlockService::IDLE;
   registrar_.RemoveAll();