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..0db87b8db847d94f2b7572d5348b5175b617d77f 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,15 @@ void FailIfCalled(const QuickUnlockModeList& modes) {
   FAIL();
 }
 
+enum ExpectedPinState {
+  PIN_GOOD = 1 << 0,
+  PIN_SHORT = 1 << 1,
+  PIN_LONG = 1 << 2,
+  PIN_WEAK = 1 << 3,
+  PIN_WEAK_BUT_OK = 1 << 4,
+  PIN_CONTAINS_NONDIGIT = 1 << 5
+};
+
 }  // namespace
 
 class QuickUnlockPrivateUnitTest : public ExtensionApiUnittest {
@@ -129,6 +143,56 @@ class QuickUnlockPrivateUnitTest : public ExtensionApiUnittest {
     return modes;
   }
 
+  // Returns true if |problem| is contained in |problems|.
+  bool HasProblem(CredentialProblem problem,
+                  const std::vector<CredentialProblem> problems) {
+    return std::find(problems.begin(), problems.end(), problem) !=
+           problems.end();
+  }
+
+  bool HasFlag(int outcome, int flag) { return (outcome & flag) != 0; }
+
+  // Helper function for checking whether |IsCredentialUsableUsingPin| will
+  // return the right message given a pin.
+  void CheckPin(int expected_outcome, const std::string& pin) {
+    CredentialCheck result = CheckCredentialUsingPin(pin);
+    const std::vector<CredentialProblem> errors(result.errors);
+    const std::vector<CredentialProblem> warnings(result.warnings);
+
+    // A pin is considered good if it emits no errors or warnings.
+    EXPECT_EQ(HasFlag(expected_outcome, PIN_GOOD),
+              errors.empty() && warnings.empty());
+    EXPECT_EQ(
+        HasFlag(expected_outcome, PIN_SHORT),
+        HasProblem(CredentialProblem::CREDENTIAL_PROBLEM_TOO_SHORT, errors));
+    EXPECT_EQ(
+        HasFlag(expected_outcome, PIN_LONG),
+        HasProblem(CredentialProblem::CREDENTIAL_PROBLEM_TOO_LONG, errors));
+    EXPECT_EQ(
+        HasFlag(expected_outcome, PIN_WEAK_BUT_OK),
+        HasProblem(CredentialProblem::CREDENTIAL_PROBLEM_TOO_WEAK, warnings));
+    EXPECT_EQ(
+        HasFlag(expected_outcome, PIN_WEAK),
+        HasProblem(CredentialProblem::CREDENTIAL_PROBLEM_TOO_WEAK, errors));
+    EXPECT_EQ(
+        HasFlag(expected_outcome, PIN_CONTAINS_NONDIGIT),
+        HasProblem(CredentialProblem::CREDENTIAL_PROBLEM_CONTAINS_NONDIGIT,
+                   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;
+    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 +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,10 +327,10 @@ 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}, {""}));
 }
@@ -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,93 @@ TEST_F(QuickUnlockPrivateUnitTest, ThrowErrorOnMismatchedParameterCount) {
   EXPECT_FALSE(SetModesWithError("[\"valid\", [], [\"11\"]]").empty());
 }
 
+// Validates PIN error checking in conjuction with policy-related prefs.
+TEST_F(QuickUnlockPrivateUnitTest, CheckCredentialProblemReporting) {
+  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, "11112");
+  CheckPin(PIN_GOOD, "1111111111111112");
+  CheckPin(PIN_WEAK_BUT_OK, "1111");
+  CheckPin(PIN_SHORT, "1");
+  CheckPin(PIN_SHORT, "11");
+  CheckPin(PIN_SHORT | PIN_WEAK_BUT_OK, "111");
+  CheckPin(PIN_SHORT | PIN_CONTAINS_NONDIGIT, "a");
+  CheckPin(PIN_CONTAINS_NONDIGIT, "aaab");
+  CheckPin(PIN_CONTAINS_NONDIGIT | PIN_WEAK_BUT_OK, "aaaa");
+  CheckPin(PIN_CONTAINS_NONDIGIT | PIN_WEAK_BUT_OK, "abcd");
+
+  // Verify that now if the minimum is set to 3, PINs of length 3 are accepted.

[jdufault, 2016/11/17 17:50:33]

nit: minimum => minimum length

[sammiequon, 2016/11/17 23:53:15]

Done.

+  pref_service->SetInteger(prefs::kPinUnlockMinimumLength, 3);
+  CheckPin(PIN_WEAK_BUT_OK, "111");

[jdufault, 2016/11/17 17:50:33]

What about renaming PIN_WEAK/PIN_WEAK_BUT_OK?

  PIN_WEAK => PIN_WEAK_ERROR
  PIN_WEAK_BUT_OK => PIN_WEAK_WARNING

I think it will make it clearer that weak can be either a warning or error.

[sammiequon, 2016/11/17 23:53:15]

Done.

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

[jdufault, 2016/11/17 17:50:33]

You should also test when min and max length are equal.

[sammiequon, 2016/11/17 23:53:15]

Done.

+  pref_service->SetInteger(prefs::kPinUnlockMinimumLength, 4);
+  CheckPin(PIN_GOOD, "1112");
+  CheckPin(PIN_SHORT, "112");

[jdufault, 2016/11/17 17:50:33]

PIN_TOO_SHORT, PIN_TOO_LONG will probably be easier to read as it makes it
clearer that they are errors.

[sammiequon, 2016/11/17 23:53:15]

Done.

+  CheckPin(PIN_LONG, "11112");
+
+  // Verify that now if the maximum is set to 5, PINs longer than 5 are
+  // considered as long PINs.

[jdufault, 2016/11/17 17:50:33]

nit:  are considered as long PINs => are considered too long and cannot be used.

[sammiequon, 2016/11/17 23:53:15]

Done.

+  pref_service->SetInteger(prefs::kPinUnlockMaximumLength, 5);
+  CheckPin(PIN_LONG | PIN_WEAK_BUT_OK, "111111");
+  CheckPin(PIN_LONG | PIN_WEAK_BUT_OK, "1111111");
+
+  // 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 are weak are flagged as such. A PIN is considered
+  // weak if it:
+  // a) has all the same digits
+  // b) each digit is one larger than the previous digit
+  // c) each digit is one smaller than the previous digit
+  // d) is on the top ten of this list;
+  // www.datagenetics.com/blog/september32012/
+  // Note: A 9 followed by a 0 is not considered increasing, and a 0 followed by
+  // a 9 is not considered decreasing.
+  pref_service->SetBoolean(prefs::kPinUnlockAllowWeakPins, false);
+  // Good.
+  CheckPin(PIN_GOOD, "1112");
+  CheckPin(PIN_GOOD, "7890");
+  CheckPin(PIN_GOOD, "0987");
+  // Same digits.
+  CheckPin(PIN_WEAK, "1111");

[jdufault, 2016/11/17 17:50:33]

I'm not sure the difference between the "1111" and the "11111111" test case. I
would remove the "11111111" test case.

[sammiequon, 2016/11/17 23:53:15]

Done.

+  CheckPin(PIN_WEAK, "1111111");
+  // Increasing.
+  CheckPin(PIN_WEAK, "0123");
+  CheckPin(PIN_WEAK, "3456789");
+  // Decreasing.
+  CheckPin(PIN_WEAK, "3210");
+  CheckPin(PIN_WEAK, "987654");
+  // Too common.
+  CheckPin(PIN_WEAK, "1212");

[jdufault, 2016/11/17 17:50:33]

I think you only need one of these cases (making sure it doesn't match any of
the other weak filters).

[sammiequon, 2016/11/17 23:53:15]

Done.

+  CheckPin(PIN_WEAK, "1004");
+  CheckPin(PIN_WEAK, "2000");
+  CheckPin(PIN_WEAK, "6969");
+
+  CheckPin(PIN_SHORT | PIN_WEAK, "111");

[jdufault, 2016/11/17 17:50:33]

Comment or remove these test cases.

[sammiequon, 2016/11/17 23:53:15]

Done.

+  CheckPin(PIN_SHORT | PIN_WEAK, "222");
+}
+
+TEST_F(QuickUnlockPrivateUnitTest, CheckCredentialGivesCorrectMinMaxPinLength) {
+  PrefService* pref_service = profile()->GetPrefs();
+
+  // 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, CheckCredentialUsingPin("1111").min_length);
+  EXPECT_EQ(8, CheckCredentialUsingPin("111111111").max_length);
+
+  // Verify that by setting a maximum length to be nonzero and smaller than the
+  // minimum length, the resulting maxium length will be equal to the minimum
+  // length pref.
+  pref_service->SetInteger(prefs::kPinUnlockMaximumLength, 4);
+  EXPECT_EQ(6, CheckCredentialUsingPin("1111").min_length);
+}
 }  // namespace chromeos