cros: Added a new function to quick unlock api for checking unfinished pins.

chrome/browser/chromeos/extensions/quick_unlock_private/quick_unlock_private_api_unittest.cc

Index: chrome/browser/chromeos/extensions/quick_unlock_private/quick_unlock_private_api_unittest.cc
diff --git a/chrome/browser/chromeos/extensions/quick_unlock_private/quick_unlock_private_api_unittest.cc b/chrome/browser/chromeos/extensions/quick_unlock_private/quick_unlock_private_api_unittest.cc
index b97b51a4116eaa15a80684c327af073db9df4918..cdebe0f9e670c9d6f9830b1b1e8d6f96af0d46d4 100644
--- a/chrome/browser/chromeos/extensions/quick_unlock_private/quick_unlock_private_api_unittest.cc
+++ b/chrome/browser/chromeos/extensions/quick_unlock_private/quick_unlock_private_api_unittest.cc
@@ -6,6 +6,8 @@
 
 #include "chrome/browser/chromeos/extensions/quick_unlock_private/quick_unlock_private_api.h"
 
+#include <algorithm>
+
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "chrome/browser/chromeos/login/quick_unlock/pin_storage.h"
@@ -14,12 +16,16 @@
 #include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
 #include "chrome/browser/chromeos/login/users/scoped_user_manager_enabler.h"
 #include "chrome/browser/extensions/extension_api_unittest.h"
+#include "chrome/common/pref_names.h"
 #include "chromeos/login/auth/fake_extended_authenticator.h"
 #include "extensions/browser/api_test_utils.h"
 #include "extensions/browser/extension_function_dispatcher.h"
 
 using namespace extensions;
 namespace quick_unlock_private = extensions::api::quick_unlock_private;
+using CredentialRequirement = quick_unlock_private::CredentialRequirement;
+using CredentialRequirementFailure =
+    quick_unlock_private::CredentialRequirementFailure;
 using QuickUnlockMode = quick_unlock_private::QuickUnlockMode;
 using QuickUnlockModeList = std::vector<QuickUnlockMode>;
 using CredentialList = std::vector<std::string>;
@@ -29,7 +35,7 @@ namespace {
 
 const char* kTestUserEmail = "testuser@gmail.com";
 const char* kTestUserEmailHash = "testuser@gmail.com-hash";
-const char* kValidPassword = "valid";
+const char* kValidPassword = "2016";
 const char* kInvalidPassword = "invalid";
 
 chromeos::ExtendedAuthenticator* CreateFakeAuthenticator(
@@ -129,6 +135,64 @@ class QuickUnlockPrivateUnitTest : public ExtensionApiUnittest {
     return modes;
   }
 
+  bool CredentialFailuresContains(
+      CredentialRequirementFailure failure_to_check,
+      const std::vector<CredentialRequirementFailure> list_of_failures) {
+    return std::find(list_of_failures.begin(), list_of_failures.end(),
+                     failure_to_check) != list_of_failures.end();
+  }
+
+  // Helper function for checking whether |GetUsablePinUsingPassword| will
+  // return the right message given a sets of organized pins.
+  void CheckPins(const std::vector<std::string>& good_pins,
+                 const std::vector<std::string>& short_pins,
+                 const std::vector<std::string>& long_pins,
+                 const std::vector<std::string>& digit_same_pins,
+                 const std::vector<std::string>& digit_increasing_pins) {
+    for (const std::string& pin : good_pins) {
+      EXPECT_TRUE(IsCredentialUsableUsingPin(pin).failures.empty());
+    }
+
+    for (const std::string& pin : short_pins) {
+      EXPECT_TRUE(CredentialFailuresContains(
+          CredentialRequirementFailure::
+              CREDENTIAL_REQUIREMENT_FAILURE_TOO_SHORT,
+          IsCredentialUsableUsingPin(pin).failures));
+    }
+
+    for (const std::string& pin : long_pins) {
+      EXPECT_TRUE(CredentialFailuresContains(
+          CredentialRequirementFailure::CREDENTIAL_REQUIREMENT_FAILURE_TOO_LONG,
+          IsCredentialUsableUsingPin(pin).failures));
+    }
+
+    for (const std::string& pin : digit_same_pins) {
+      EXPECT_TRUE(CredentialFailuresContains(
+          CredentialRequirementFailure::CREDENTIAL_REQUIREMENT_FAILURE_TOO_WEAK,
+          IsCredentialUsableUsingPin(pin).failures));
+    }
+
+    for (const std::string& pin : digit_increasing_pins) {
+      EXPECT_TRUE(CredentialFailuresContains(
+          CredentialRequirementFailure::CREDENTIAL_REQUIREMENT_FAILURE_TOO_WEAK,
+          IsCredentialUsableUsingPin(pin).failures));
+    }
+  }
+
+  CredentialRequirement IsCredentialUsableUsingPin(const std::string& pin) {
+    auto params = base::MakeUnique<base::ListValue>();

[jdufault, 2016/09/30 01:05:51]

auto* for pointers

[sammiequon, 2016/09/30 18:38:35]

I don't think auto* works for std::unique_ptr<T>.

+    auto pin_mode = QuickUnlockMode::QUICK_UNLOCK_MODE_PIN;
+    params->AppendString(ToString(pin_mode));

[jdufault, 2016/09/30 01:05:50]

Move pin_mode below so params_ initialization stays together.

[sammiequon, 2016/09/30 18:38:35]

Done.

+    params->AppendString(pin);
+
+    std::unique_ptr<base::Value> result = RunFunction(
+        new QuickUnlockPrivateIsCredentialUsableFunction(), std::move(params));
+
+    CredentialRequirement function_result;
+    EXPECT_TRUE(CredentialRequirement::Populate(*result, &function_result));
+    return function_result;
+  }
+
   // Wrapper for chrome.quickUnlockPrivate.setModes that automatically uses a
   // valid password.
   bool SetModes(const QuickUnlockModeList& modes,
@@ -245,7 +309,7 @@ TEST_F(QuickUnlockPrivateUnitTest, SetModesFailsWithInvalidPassword) {
   FailIfModesChanged();
   EXPECT_FALSE(SetModesUsingPassword(
       kInvalidPassword,
-      QuickUnlockModeList{QuickUnlockMode::QUICK_UNLOCK_MODE_PIN}, {"11"}));
+      QuickUnlockModeList{QuickUnlockMode::QUICK_UNLOCK_MODE_PIN}, {"1111"}));
   EXPECT_EQ(GetActiveModes(), QuickUnlockModeList{});
 }
 
@@ -263,12 +327,12 @@ TEST_F(QuickUnlockPrivateUnitTest, ModeChangeEventOnlyRaisedWhenModesChange) {
   ExpectModesChanged(
       QuickUnlockModeList{QuickUnlockMode::QUICK_UNLOCK_MODE_PIN});
   EXPECT_TRUE(SetModes(
-      QuickUnlockModeList{QuickUnlockMode::QUICK_UNLOCK_MODE_PIN}, {"11"}));
+      QuickUnlockModeList{QuickUnlockMode::QUICK_UNLOCK_MODE_PIN}, {"1111"}));

[jdufault, 2016/09/30 01:05:50]

By default policy should not be active, so these should still be allowed.

[sammiequon, 2016/09/30 18:38:36]

Done.

   FailIfModesChanged();
   EXPECT_TRUE(SetModes(
-      QuickUnlockModeList{QuickUnlockMode::QUICK_UNLOCK_MODE_PIN}, {"22"}));
+      QuickUnlockModeList{QuickUnlockMode::QUICK_UNLOCK_MODE_PIN}, {"2222"}));
   EXPECT_TRUE(SetModes(
-      QuickUnlockModeList{QuickUnlockMode::QUICK_UNLOCK_MODE_PIN}, {""}));
+      QuickUnlockModeList{QuickUnlockMode::QUICK_UNLOCK_MODE_PIN}, {"3333"}));
 }
 
 // Ensures that quick unlock can be enabled and disabled by checking the result
@@ -280,7 +344,7 @@ TEST_F(QuickUnlockPrivateUnitTest, SetModesAndGetActiveModes) {
   ExpectModesChanged(
       QuickUnlockModeList{QuickUnlockMode::QUICK_UNLOCK_MODE_PIN});
   EXPECT_TRUE(SetModes(
-      QuickUnlockModeList{QuickUnlockMode::QUICK_UNLOCK_MODE_PIN}, {"11"}));
+      QuickUnlockModeList{QuickUnlockMode::QUICK_UNLOCK_MODE_PIN}, {"1111"}));
   EXPECT_EQ(GetActiveModes(),
             QuickUnlockModeList{QuickUnlockMode::QUICK_UNLOCK_MODE_PIN});
   EXPECT_TRUE(pin_storage->IsPinSet());
@@ -300,13 +364,13 @@ TEST_F(QuickUnlockPrivateUnitTest, VerifyAuthenticationAgainstPIN) {
   EXPECT_FALSE(pin_storage->IsPinSet());
 
   EXPECT_TRUE(SetModes(
-      QuickUnlockModeList{QuickUnlockMode::QUICK_UNLOCK_MODE_PIN}, {"11"}));
+      QuickUnlockModeList{QuickUnlockMode::QUICK_UNLOCK_MODE_PIN}, {"1111"}));
   EXPECT_TRUE(pin_storage->IsPinSet());
 
   pin_storage->MarkStrongAuth();
   pin_storage->ResetUnlockAttemptCount();
-  EXPECT_TRUE(pin_storage->TryAuthenticatePin("11"));
-  EXPECT_FALSE(pin_storage->TryAuthenticatePin("00"));
+  EXPECT_TRUE(pin_storage->TryAuthenticatePin("1111"));
+  EXPECT_FALSE(pin_storage->TryAuthenticatePin("0000"));
 }
 
 // Verifies that the number of modes and the number of passwords given must be
@@ -316,4 +380,43 @@ TEST_F(QuickUnlockPrivateUnitTest, ThrowErrorOnMismatchedParameterCount) {
   EXPECT_FALSE(SetModesWithError("[\"valid\", [], [\"11\"]]").empty());
 }
 
+// Verifies that unsubmitted pins give the right message to users.
+TEST_F(QuickUnlockPrivateUnitTest, VerifyPinsAgainstPreferences) {
+  PrefService* pref_service = profile()->GetPrefs();
+
+  std::vector<std::string> empty;
+  // Verify the pin checks work with the default preferences which are minimum
+  // length of 4, maximum length of 0 (no maximum) and no easy to guess check.
+  CheckPins(std::vector<std::string>{"1243", "24569", "584934353422432"},

[jdufault, 2016/09/30 01:05:50]

These are extremely hard to read because it's just a long sequence of numbers. I
think the test would be significantly more readable if it was written like this

  EXPECT_EQ({}, CheckPin("1234"));
  EXPECT_EQ({TOO_SHORT, WEAK_PIN}, CheckPin("11"));

[sammiequon, 2016/09/30 18:38:36]

Done.

+            std::vector<std::string>{"1", "25", "236"}, empty, empty, empty);
+
+  // Verify that now if the minimum is set to 3, pins of length 3 are accepted.

[jdufault, 2016/09/30 01:05:51]

What happens if pin "asdf" is checked?

[sammiequon, 2016/09/30 18:38:35]

This should never be called. Should we add the digit checker into the function
and add a new failure?

[jdufault, 2016/10/04 22:57:27]

Up to you. We should at least do a sanity check here to make sure we don't
crash.

+  pref_service->SetInteger(prefs::kPinUnlockMinimumLength, 3);
+  CheckPins(std::vector<std::string>{"236", "1243", "24569", "584934353422432"},
+            std::vector<std::string>{"1", "25"}, empty, empty, empty);
+
+  // Verify that now if the maximum is set to 5, pins longer than 5 are
+  // considered as long pins.
+  pref_service->SetInteger(prefs::kPinUnlockMaximumLength, 5);
+  CheckPins(std::vector<std::string>{"236", "1243", "24569"},
+            std::vector<std::string>{"1", "25"},
+            std::vector<std::string>{"145253", "1353151414"}, empty, empty);
+
+  // Set the pins minimum/maximum lengths back to their defaults.
+  pref_service->SetInteger(prefs::kPinUnlockMinimumLength, 4);
+  pref_service->SetInteger(prefs::kPinUnlockMaximumLength, 0);
+  // Verify that pins that consists of the same character, or pins that are
+  // increasing are considered same digit and increasing pins respectively. A 9
+  // followed by a 0 is not considered increasing.
+  CheckPins(std::vector<std::string>{"7890"}, empty, empty,
+            std::vector<std::string>{"1111", "2222", "9999"},
+            std::vector<std::string>{"0123", "1234", "6789"});
+
+  // Verify that trying out pins under the minimum/maximum lengths will send the
+  // minimum/maximum lengths as additional information for display purposes.
+  pref_service->SetInteger(prefs::kPinUnlockMinimumLength, 6);
+  pref_service->SetInteger(prefs::kPinUnlockMaximumLength, 8);
+  EXPECT_EQ(6, *(IsCredentialUsableUsingPin("1456").min_length));
+  EXPECT_EQ(8, *(IsCredentialUsableUsingPin("141329556").max_length));
+}
 }  // namespace chromeos