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..18b38cf95f644806d61920d44f020483ce227f0f 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,15 @@
 #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 CredentialCheck = quick_unlock_private::CredentialCheck;
+using CredentialProblem = quick_unlock_private::CredentialProblem;
 using QuickUnlockMode = quick_unlock_private::QuickUnlockMode;
 using QuickUnlockModeList = std::vector<QuickUnlockMode>;
 using CredentialList = std::vector<std::string>;
@@ -50,6 +55,14 @@ void FailIfCalled(const QuickUnlockModeList& modes) {
   FAIL();
 }
 
+enum TestPinState {
+  PIN_GOOD = 1 << 0,
+  PIN_SHORT = 1 << 1,
+  PIN_LONG = 1 << 2,
+  PIN_WEAK = 1 << 3,
+  PIN_WEAK_BUT_OK = 1 << 4
+};
+
 }  // namespace
 
 class QuickUnlockPrivateUnitTest : public ExtensionApiUnittest {
@@ -129,6 +142,50 @@ class QuickUnlockPrivateUnitTest : public ExtensionApiUnittest {
     return modes;
   }
 
+  bool CredentialChecksContains(
+      CredentialProblem problem_to_check,
+      const std::vector<CredentialProblem> list_of_problems) {
+    return std::find(list_of_problems.begin(), list_of_problems.end(),
+                     problem_to_check) != list_of_problems.end();
+  }
+
+  // Helper function for checking whether |IsCredentialUsableUsingPin| will
+  // return the right message given a pin.
+  void CheckPin(int expected_outcome, const std::string& pin) {
+    auto result = CheckCredentialUsingPin(pin);

[jdufault, 2016/10/31 22:03:08]

Only use auto when type is evident from the initializer expression.

[sammiequon, 2016/11/01 18:08:32]

Done.

+    auto errors = result.errors;

[jdufault, 2016/10/31 22:03:08]

Specify init type

[sammiequon, 2016/11/01 18:08:32]

Done.

+    auto warnings = result.warnings;

[jdufault, 2016/10/31 22:03:08]

Specify init type

[sammiequon, 2016/11/01 18:08:32]

Done.

+
+    // A pin is considered good if it emits no errors or warnings.
+    EXPECT_EQ(expected_outcome & PIN_GOOD ? true : false,

[jdufault, 2016/10/31 22:03:08]

HasFlag(expected_outcome, PIN_GOOD)?

[sammiequon, 2016/11/01 18:08:32]

Done.

+              errors.empty() && warnings.empty());
+    EXPECT_EQ(expected_outcome & PIN_SHORT ? true : false,
+              CredentialChecksContains(
+                  CredentialProblem::CREDENTIAL_PROBLEM_TOO_SHORT, errors));
+    EXPECT_EQ(expected_outcome & PIN_LONG ? true : false,
+              CredentialChecksContains(
+                  CredentialProblem::CREDENTIAL_PROBLEM_TOO_LONG, errors));
+    EXPECT_EQ(expected_outcome & PIN_WEAK_BUT_OK ? true : false,
+              CredentialChecksContains(
+                  CredentialProblem::CREDENTIAL_PROBLEM_TOO_WEAK, warnings));
+    EXPECT_EQ(expected_outcome & PIN_WEAK ? true : false,
+              CredentialChecksContains(
+                  CredentialProblem::CREDENTIAL_PROBLEM_TOO_WEAK, errors));
+  }
+
+  CredentialCheck CheckCredentialUsingPin(const std::string& pin) {
+    auto params = base::MakeUnique<base::ListValue>();
+    params->AppendString(ToString(QuickUnlockMode::QUICK_UNLOCK_MODE_PIN));
+    params->AppendString(pin);
+
+    std::unique_ptr<base::Value> result = RunFunction(
+        new QuickUnlockPrivateCheckCredentialFunction(), std::move(params));
+
+    CredentialCheck function_result;

[jdufault, 2016/10/31 22:03:08]

What about just result?

[sammiequon, 2016/11/01 18:08:32]

result is used in line 181.

+    EXPECT_TRUE(CredentialCheck::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 +302,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 +320,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"}));
   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}, {"0000"}));

[jdufault, 2016/10/31 22:03:08]

Revert, "" means that the mode is not changed.

For example, if there are two quick unlock modes, PIN and SWIPE, then

  SetModes({ PIN, SWIPE }, { "", "1234" })

will leave PIN with the current value and set SWIPE to 1234.

[sammiequon, 2016/11/01 18:08:32]

Done.

 }
 
 // Ensures that quick unlock can be enabled and disabled by checking the result
@@ -280,7 +337,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 +357,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 +373,61 @@ 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();
+
+  // 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.
+  CheckPin(PIN_GOOD, "1112");
+  CheckPin(PIN_GOOD, "22223");
+  CheckPin(PIN_GOOD, "3333333333333334");
+  CheckPin(PIN_WEAK_BUT_OK, "3333");
+  CheckPin(PIN_SHORT, "4");
+  CheckPin(PIN_SHORT, "55");

[jdufault, 2016/10/31 22:03:08]

Try to reuse the same numeric value when you can - only make them different when
it is important to the test.

[sammiequon, 2016/11/01 18:08:32]

Done.

+  CheckPin(PIN_SHORT | PIN_WEAK_BUT_OK, "777");
+
+  // Verify that now if the minimum is set to 3, pins of length 3 are accepted.
+  pref_service->SetInteger(prefs::kPinUnlockMinimumLength, 3);
+  CheckPin(PIN_WEAK_BUT_OK, "777");
+
+  // Verify setting a nonzero maximum length that is less than the minimum
+  // length results in the pin only accepting pins of length minimum length.
+  pref_service->SetInteger(prefs::kPinUnlockMaximumLength, 2);
+  pref_service->SetInteger(prefs::kPinUnlockMinimumLength, 4);
+  CheckPin(PIN_GOOD, "1112");
+  CheckPin(PIN_GOOD, "2112");
+  CheckPin(PIN_SHORT, "112");
+  CheckPin(PIN_LONG, "11211");
+
+  // 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);
+  CheckPin(PIN_LONG | PIN_WEAK_BUT_OK, "888888");
+  CheckPin(PIN_LONG | PIN_WEAK_BUT_OK, "9999999");
+
+  // 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. Decreasing pins are
+  // considered weak as well.
+  pref_service->SetBoolean(prefs::kPinUnlockAllowWeakPins, false);
+  CheckPin(PIN_GOOD, "7890");
+  CheckPin(PIN_WEAK, "1111");
+  CheckPin(PIN_WEAK, "2222222");
+  CheckPin(PIN_WEAK, "0123");
+  CheckPin(PIN_WEAK, "3456789");
+  CheckPin(PIN_WEAK, "3210");
+  CheckPin(PIN_WEAK, "9876");
+  CheckPin(PIN_SHORT | PIN_WEAK, "111");

[jdufault, 2016/10/31 22:03:08]

Just to verify, if you have just CheckPin(PIN_SHORT, "111") then the test will
fail?

[sammiequon, 2016/11/01 18:08:31]

That is correct. This test checks that all problems are reported.

+
+  // 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);

[jdufault, 2016/10/31 22:03:08]

This seems like a separate test.

[sammiequon, 2016/11/01 18:08:32]

Done.

+  pref_service->SetInteger(prefs::kPinUnlockMaximumLength, 8);
+  EXPECT_EQ(6, *(CheckCredentialUsingPin("1456").min_length));
+  EXPECT_EQ(8, *(CheckCredentialUsingPin("141329556").max_length));
+}
 }  // namespace chromeos