Enable the PIN keyboard on the lockscreen.

chrome/browser/chromeos/login/lock/screen_locker.cc

 // Copyright 2014 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/login/lock/screen_locker.h"
 
 #include <string>
 #include <vector>
 
 #include "ash/ash_switches.h"
 #include "ash/audio/sounds.h"
 #include "ash/desktop_background/desktop_background_controller.h"
 #include "ash/shell.h"
 #include "ash/wm/common/window_state.h"
 #include "ash/wm/common/wm_event.h"
 #include "ash/wm/lock_state_controller.h"
 #include "ash/wm/window_state_aura.h"
 #include "ash/wm/window_util.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/login/lock/webui_screen_locker.h"
+#include "chrome/browser/chromeos/login/quick_unlock/pin_storage.h"
+#include "chrome/browser/chromeos/login/quick_unlock/pin_storage_factory.h"
 #include "chrome/browser/chromeos/login/session/user_session_manager.h"
 #include "chrome/browser/chromeos/login/supervised/supervised_user_authentication.h"
 #include "chrome/browser/chromeos/login/ui/user_adding_screen.h"
 #include "chrome/browser/chromeos/login/users/chrome_user_manager.h"
 #include "chrome/browser/chromeos/login/users/supervised_user_manager.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/signin/easy_unlock_service.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/ui/webui/chromeos/login/screenlock_icon_provider.h"
 #include "chrome/browser/ui/webui/chromeos/login/screenlock_icon_source.h"
@@ skipping 72 matching lines @@
     ScreenLocker::HandleLockScreenRequest();
   }
 
  private:
   bool session_started_;
   content::NotificationRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(ScreenLockObserver);
 };
 
+bool IsLikelyPIN(const std::string& input) {
+  // Check if |input| looks like a number; the actual numeric value doesn't
+  // matter.
+  int num;
+  return base::StringToInt(input, &num);
+}
+
 ScreenLockObserver* g_screen_lock_observer = NULL;
 
 }  // namespace
 
 // static
 ScreenLocker* ScreenLocker::screen_locker_ = NULL;
 
 //////////////////////////////////////////////////////////////////////////////
 // ScreenLocker, public:
 
@@ skipping 76 matching lines @@
   }
 
   const user_manager::User* user =
       user_manager::UserManager::Get()->FindUser(user_context.GetAccountId());
   if (user) {
     if (!user->is_active()) {
       saved_ime_state_ = NULL;
       user_manager::UserManager::Get()->SwitchActiveUser(
           user_context.GetAccountId());
     }
+
+    // Reset the number of PIN attempts available to the user. We always do this
+    // because:
+    // 1. If the user signed in with a PIN, that means they should be able to
+    //    continue signing in with a PIN.
+    // 2. If the user signed in with cryptohome keys, then the PIN timeout is
+    //    going to be reset as well, so it is safe to reset the unlock attempt
+    //    count.
+    PinStorage* pin_storage = PinStorageFactory::GetForUser(user);
+    if (pin_storage) {
+      pin_storage->ResetUnlockAttemptCount();
+    } else {
+      NOTREACHED() << "PinStorage not available for user";
+    }
+
     UserSessionManager::GetInstance()->UpdateEasyUnlockKeys(user_context);
   } else {
     NOTREACHED() << "Logged in user not found.";
   }
 
   authentication_capture_.reset(new AuthenticationParametersCapture());
   authentication_capture_->user_context = user_context;
 
   // Add guard for case when something get broken in call chain to unlock
   // for sure.
   base::MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
       base::Bind(&ScreenLocker::UnlockOnLoginSuccess,
           weak_factory_.GetWeakPtr()),
       base::TimeDelta::FromMilliseconds(kUnlockGuardTimeoutMs));
   delegate_->AnimateAuthenticationSuccess();
 }
 
+void ScreenLocker::OnPasswordAuthSuccess(const UserContext& user_context) {
+  // The user has signed in using their password, so reset the PIN timeout.
+  PinStorage* pin_storage =
+      PinStorageFactory::GetForAccountId(user_context.GetAccountId());
+
+  if (pin_storage) {
+    pin_storage->MarkStrongAuth();
+  } else {
+    NOTREACHED() << "Missing PinStorage for account";
+  }
+}
+
 void ScreenLocker::UnlockOnLoginSuccess() {
   DCHECK(base::MessageLoopForUI::IsCurrent());
   if (!authentication_capture_.get()) {
     LOG(WARNING) << "Call to UnlockOnLoginSuccess without previous " <<
       "authentication success.";
     return;
   }
 
   if (auth_status_consumer_) {
     auth_status_consumer_->OnAuthSuccess(authentication_capture_->user_context);
   }
   authentication_capture_.reset();
   weak_factory_.InvalidateWeakPtrs();
 
   VLOG(1) << "Hiding the lock screen.";
   chromeos::ScreenLocker::Hide();
 }
 
 void ScreenLocker::Authenticate(const UserContext& user_context) {
-  LOG_ASSERT(IsUserLoggedIn(user_context.GetAccountId().GetUserEmail()))
+  LOG_ASSERT(IsUserLoggedIn(user_context.GetAccountId()))
       << "Invalid user trying to unlock.";
 
   authentication_start_time_ = base::Time::Now();
   delegate_->SetInputEnabled(false);
   delegate_->OnAuthenticate();
 
-  // Special case: supervised users. Use special authenticator.
-  if (const user_manager::User* user =
-          FindUnlockUser(user_context.GetAccountId().GetUserEmail())) {
+  const user_manager::User* user = FindUnlockUser(user_context.GetAccountId());
+  if (user) {
+    // Check to see if the user submitted a PIN and it is valid.
+    std::string pin = user_context.GetKey()->GetSecret();

[achuithb, 2016/06/02 23:34:45]

const

[jdufault, 2016/06/03 23:39:49]

Done.

+    if (IsLikelyPIN(pin)) {

[achuithb, 2016/06/02 23:34:45]

Not sure if this deserves a separate function - it's only 2 lines, right?

[jdufault, 2016/06/03 23:39:49]

Done. Yea, it's iffy. Inlining introduces an extra unused variable.

+      chromeos::PinStorage* pin_storage =
+          chromeos::PinStorageFactory::GetForUser(user);
+      if (!pin_storage) {
+        NOTREACHED() << "Missing PinStorage when trying to authenticate PIN";

[achuithb, 2016/06/02 23:34:45]

Is this logging important? Why not 
if (pin_storage && pin_storage->TryAuthenticatePin(pin)) { 

[jdufault, 2016/06/03 23:39:49]

Done.

+      } else if (pin_storage->TryAuthenticatePin(pin)) {
+        OnAuthSuccess(user_context);
+        return;
+      }
+    }
+
+    // Special case: supervised users. Use special authenticator.
     if (user->GetType() == user_manager::USER_TYPE_SUPERVISED) {
       UserContext updated_context = ChromeUserManager::Get()
                                         ->GetSupervisedUserManager()
                                         ->GetAuthentication()
                                         ->TransformKey(user_context);
-      // TODO(antrim) : replace empty closure with explicit method.
-      // http://crbug.com/351268
       BrowserThread::PostTask(
-          BrowserThread::UI,
-          FROM_HERE,
+          BrowserThread::UI, FROM_HERE,
           base::Bind(&ExtendedAuthenticator::AuthenticateToCheck,
-                     extended_authenticator_.get(),
-                     updated_context,
-                     base::Closure()));
+                     extended_authenticator_.get(), updated_context,
+                     base::Bind(&ScreenLocker::OnPasswordAuthSuccess,
+                                base::Unretained(this), user_context)));

[achuithb, 2016/06/02 23:34:45]

Why base::Unretained? Also, should this be updated_context?

[jdufault, 2016/06/03 23:39:49]

Done on updated_context.

       return;
     }
   }
 
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&ExtendedAuthenticator::AuthenticateToCheck,
-                 extended_authenticator_.get(),
-                 user_context,
-                 base::Closure()));
+                 extended_authenticator_.get(), user_context,
+                 base::Bind(&ScreenLocker::OnPasswordAuthSuccess,
+                            base::Unretained(this), user_context)));

[achuithb, 2016/06/02 23:34:45]

base::Unretained?

[jdufault, 2016/06/03 23:39:49]

I've converted this to use a weak_ptr, but I believe this is safe because
AuthenticateToCheck already calls back into the object, and the newly added
callback will get invoked in the same call hierarchy.

If OnPasswordAuthSuccess doesn't get called but OnAuthSuccess does on a password
authentication, PIN will be permanently disabled after it times out, so this
feels like an error that should be fixed (base::Unretained) instead of silently
ignored (weak_ptr).

[achuithb, 2016/06/04 00:32:00]

If you feel strongly that base::Unretained is the right construct, you should
add a comment.

In general, I think we try to avoid use of base::Unretained because of the
security risk of accessing freed memory, especially if this code gets
re-factored to behave differently in the future, such as posting on the message
loop, etc.

[jdufault, 2016/06/08 18:40:50]

sg; the current CL is using the weak_ptr approach.

 }
 
 const user_manager::User* ScreenLocker::FindUnlockUser(
-    const std::string& user_id) {
-  const user_manager::User* unlock_user = NULL;
-  for (user_manager::UserList::const_iterator it = users_.begin();
-       it != users_.end();
-       ++it) {
-    if ((*it)->email() == user_id) {
-      unlock_user = *it;
-      break;
-    }
+    const AccountId& account_id) {
+  for (const user_manager::User* user : users_) {
+    if (user->GetAccountId() == account_id)
+      return user;
   }
-  return unlock_user;
+  return nullptr;
 }
 
 void ScreenLocker::ClearErrors() {
   delegate_->ClearErrors();
 }
 
 void ScreenLocker::Signout() {
   delegate_->ClearErrors();
   content::RecordAction(UserMetricsAction("ScreenLocker_Signout"));
   // We expect that this call will not wait for any user input.
@@ skipping 183 matching lines @@
 
   input_method::InputMethodManager::Get()
       ->GetActiveIMEState()
       ->EnableLockScreenLayouts();
 }
 
 content::WebUI* ScreenLocker::GetAssociatedWebUI() {
   return delegate_->GetAssociatedWebUI();
 }
 
-bool ScreenLocker::IsUserLoggedIn(const std::string& username) {
-  for (user_manager::UserList::const_iterator it = users_.begin();
-       it != users_.end();
-       ++it) {
-    if ((*it)->email() == username)
+bool ScreenLocker::IsUserLoggedIn(const AccountId& account_id) {

[achuithb, 2016/06/02 23:34:45]

is it painful to make this function const?

[jdufault, 2016/06/03 23:39:49]

Done.

+  for (user_manager::User* user : users_) {
+    if (user->GetAccountId() == account_id)
       return true;
   }
   return false;
 }
 
 }  // namespace chromeos