cros: Added a fake fingerprint storage class.

chromeos/dbus/biod/fake_biod_client_unittest.cc

+// Copyright 2017 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 "chromeos/dbus/biod/fake_biod_client.h"
+
+#include "base/bind.h"
+#include "base/macros.h"
+#include "base/strings/string_util.h"
+#include "base/test/test_simple_task_runner.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "chromeos/dbus/biod/biod_test_utils.h"
+#include "dbus/object_path.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+
+namespace {
+
+const char kTestUserId[] = "testuser@gmail.com";
+const char kTestLabel[] = "testLabel";
+// Template of a scan string to be used in GenerateTestFingerprint. The # and $
+// are two wildcards that will be replaced by numbers to ensure unique scans.
+const char kTestScan[] = "finger#scan$";
+
+}  // namepsace
+
+class FakeBiodClientTest : public testing::Test {
+ public:
+  FakeBiodClientTest()
+      : task_runner_(new base::TestSimpleTaskRunner),
+        task_runner_handle_(task_runner_) {}
+  ~FakeBiodClientTest() override {}
+
+  int NumRecords() {
+    return static_cast<int>(fake_biod_client_.records_.size());

[Daniel Erat, 2017/04/07 23:40:56]

this feels like it's testing the class's internals more than necessary. why do
you care how many records it has internally? it'd probably be better to check
that GetRecordsForUser returns the expected records for each user; that's what
you actually care about, right?

[sammiequon, 2017/04/08 00:43:32]

Yeah, think this was a temporary that i forgot to replace. Done.

+  }
+
+  int NumRecords(const std::string& user_id) {

[Daniel Erat, 2017/04/07 23:40:56]

it'd probably be cleaner to call this GetRecordsForUser() and return the vector
of enrollments

[sammiequon, 2017/04/08 00:43:33]

Done.

+    std::vector<dbus::ObjectPath> enrollment_paths;
+    fake_biod_client_.GetRecordsForUser(
+        kTestUserId,
+        base::Bind(&test_utils::CopyObjectPathArray, &enrollment_paths));
+    task_runner_->RunUntilIdle();
+    return static_cast<int>(enrollment_paths.size());
+  }
+
+  void SendAuthScanDone(const std::string& scan_data) {

[Daniel Erat, 2017/04/07 23:40:56]

this is a single call to a public method. can the tests just do this themselves
instead?

[sammiequon, 2017/04/08 00:43:32]

I thought it would be nice to make a helper since it was pretty long. Done.

[Daniel Erat, 2017/04/08 00:57:21]

i agree that it's long! :-)

biod::ScanResult::SUCCESS would be a better name -- you don't need a redundant
SCAN_RESULT_ prefix in the value name if you're using an enum class, since it's
already namespaced.

note also that you can do "using ScanResult = biod::ScanResult" near the top of
this file to avoid needing to type the namespace everywhere.

[sammiequon, 2017/04/11 17:11:37]

Done.

+    fake_biod_client_.SendAuthScanDone(scan_data,
+                                       biod::ScanResult::SCAN_RESULT_SUCCESS);
+  }
+
+  // Helper function which enrolls a fingerprint. Each element in
+  // |fingerprint_data| corresponds to a finger tap.
+  void EnrollFingerprint(const std::string& id,
+                         const std::string& label,
+                         const std::vector<std::string>& fingerprint_data) {
+    if (fingerprint_data.size() < 1)

[Daniel Erat, 2017/04/07 23:40:56]

you can just use fingerprint_data.empty() instead, but it's better to
ASSERT_FALSE or CHECK here if you want to make sure that test code never calls
this with an empty vector.

[sammiequon, 2017/04/08 00:43:32]

Done.

+      return;
+
+    dbus::ObjectPath returned_path;
+    fake_biod_client_.StartEnrollSession(
+        id, label, base::Bind(&test_utils::CopyObjectPath, &returned_path));
+    task_runner_->RunUntilIdle();
+    EXPECT_NE(returned_path, dbus::ObjectPath());

[Daniel Erat, 2017/04/07 23:40:56]

EXPECT_ and ASSERT_ expect parameters to be (expected, actual) rather than
(actual, expected); you'll get confusing failure methods otherwise.

(it's confusing how this is backwards from how CHECK_EQ expressions are usually
written, but blame junit :-P)

[sammiequon, 2017/04/08 00:43:32]

Done.

+
+    // Send |fingerprint_data| size - 1 incomplete scans.
+    for (int i = 0; i < static_cast<int>(fingerprint_data.size()) - 1; ++i) {

[Daniel Erat, 2017/04/07 23:40:56]

instead of casting, just make i be a size_t

[sammiequon, 2017/04/08 00:43:32]

Done.

+      fake_biod_client_.SendEnrollScanDone(
+          fingerprint_data[i], biod::ScanResult::SCAN_RESULT_SUCCESS,
+          false /* is_complete*/);

[Daniel Erat, 2017/04/07 23:40:56]

/* is_complete */ (spaces on both sides)

[sammiequon, 2017/04/08 00:43:32]

Done.

+    }
+
+    // Finish the enrollment by sending a complete scan signal.
+    fake_biod_client_.SendEnrollScanDone(fingerprint_data.back(),
+                                         biod::ScanResult::SCAN_RESULT_SUCCESS,
+                                         true /*is_complete*/);

[Daniel Erat, 2017/04/07 23:40:56]

just move this into the above for loop and pass (i == fingerprint_data.size() -
1) as is_complete

[sammiequon, 2017/04/08 00:43:32]

Done.

+  }
+
+  // Helper function which enrolls |n| fingerprints with the same |id|, |label|
+  // and |fingerprint_data|.
+  void EnrollNTestFingerprints(const std::string& id,
+                               const std::string& label,
+                               const std::vector<std::string>& fingerprint_data,
+                               int n) {
+    for (int i = 0; i < n; ++i)
+      EnrollFingerprint(id, label, fingerprint_data);
+  }
+
+  // Creates a new unique fingerprint consisting of unique scans.
+  std::vector<std::string> GenerateTestFingerprint(int scans) {
+    DCHECK_GE(scans, 0);
+    num_test_fingerprints_++;
+
+    std::vector<std::string> fingerprint;
+    for (int i = 0; i < scans; ++i) {
+      std::string scan = kTestScan;
+      base::ReplaceSubstringsAfterOffset(
+          &scan, 0, "#", std::to_string(num_test_fingerprints_));
+      base::ReplaceSubstringsAfterOffset(&scan, 0, "$", std::to_string(i));
+      fingerprint.push_back(scan);
+    }
+    return fingerprint;
+  }
+
+ protected:
+  FakeBiodClient fake_biod_client_;
+  scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
+  base::ThreadTaskRunnerHandle task_runner_handle_;
+
+  // This number is incremented each time GenerateTestFingerprint is called to
+  // ensure each fingerprint is unique.
+  int num_test_fingerprints_ = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FakeBiodClientTest);
+};
+
+TEST_F(FakeBiodClientTest, TestEnrollSessionWorkflow) {
+  test_utils::TestBiodObserver observer;
+  fake_biod_client_.AddObserver(&observer);
+
+  auto kTestFingerprint = GenerateTestFingerprint(2);

[Daniel Erat, 2017/04/07 23:40:56]

i think you need to write the type here instead of 'auto', since it isn't
obvious from the function call

[sammiequon, 2017/04/08 00:43:32]

Done.

+  // Verify that successful enrollments gets stored as expected. A fingerprint

[Daniel Erat, 2017/04/07 23:40:56]

nit: s/gets/get/

[sammiequon, 2017/04/08 00:43:33]

Done.

+  // that was created with 2 scans should have 1 incomplete scans and 1 complete
+  // scans.
+  EnrollFingerprint(kTestUserId, kTestLabel, kTestFingerprint);
+  EXPECT_EQ(1, NumRecords());
+  EXPECT_EQ(1, observer.num_incomplete_enroll_scans_received());
+  EXPECT_EQ(1, observer.num_complete_enroll_scans_received());
+
+  // Verify that the enroll session worflow can be used repeatedly by enrolling
+  // 3 more fingerprints (each with 1 incomplete and 1 complete scans).
+  EnrollNTestFingerprints(kTestUserId, kTestLabel, kTestFingerprint, 3);
+  EXPECT_EQ(4, NumRecords());
+  EXPECT_EQ(4, observer.num_incomplete_enroll_scans_received());
+  EXPECT_EQ(4, observer.num_complete_enroll_scans_received());
+}
+
+// Test authentication when one user has one or more fingerprints registered.
+// This should be the normal scenario.
+TEST_F(FakeBiodClientTest, TestAuthSessionWorkflowSingleUser) {
+  test_utils::TestBiodObserver observer;
+  fake_biod_client_.AddObserver(&observer);
+  EXPECT_EQ(0, NumRecords(kTestUserId));
+
+  auto kTestFingerprint1 = GenerateTestFingerprint(2);
+  auto kTestFingerprint2 = GenerateTestFingerprint(2);
+  auto kTestFingerprint3 = GenerateTestFingerprint(2);
+  auto kTestFingerprint4 = GenerateTestFingerprint(2);
+
+  // Add two fingerprints a and b and start an auth session.

[Daniel Erat, 2017/04/07 23:40:56]

is this comment outdated? i don't see 'a' or 'b'

[sammiequon, 2017/04/08 00:43:32]

Done.

+  EnrollFingerprint(kTestUserId, kTestLabel, kTestFingerprint1);
+  EnrollFingerprint(kTestUserId, kTestLabel, kTestFingerprint2);
+  dbus::ObjectPath returned_path;
+  fake_biod_client_.StartAuthSession(
+      base::Bind(&test_utils::CopyObjectPath, &returned_path));
+  task_runner_->RunUntilIdle();
+  EXPECT_NE(returned_path, dbus::ObjectPath());
+
+  // Verify that by sending two attempts signals of fingerprints that have been
+  // enrolled, the observer should received two matches and zero unmatches.

[Daniel Erat, 2017/04/07 23:40:56]

s/received/receive/, s/unmatches/non-matches/

[sammiequon, 2017/04/08 00:43:32]

Done.

+  SendAuthScanDone(kTestFingerprint1[0]);
+  SendAuthScanDone(kTestFingerprint2[0]);
+  EXPECT_EQ(2, observer.num_matched_auth_scans_received());
+  EXPECT_EQ(0, observer.num_unmatched_auth_scans_received());
+
+  // Verify that by sending two attempts signals of fingerprints that have not

[Daniel Erat, 2017/04/07 23:40:57]

s/attempts/attempt/

[sammiequon, 2017/04/08 00:43:33]

Done.

+  // been enrolled, the observer should received two unmatches and zero

[Daniel Erat, 2017/04/07 23:40:56]

s/received/receive/, s/unmatches/non-matches/

[sammiequon, 2017/04/08 00:43:33]

Done.

+  // matches.
+  SendAuthScanDone(kTestFingerprint3[0]);
+  SendAuthScanDone(kTestFingerprint4[0]);
+  EXPECT_EQ(2, observer.num_matched_auth_scans_received());
+  EXPECT_EQ(2, observer.num_unmatched_auth_scans_received());
+}
+
+// Test authentication when multiple users have fingerprints registered. Cover
+// cases such as when both users use the same labels, a user had registered the
+// same fingerprint multiple times, or two users use the same fingerprint.
+TEST_F(FakeBiodClientTest, TestAuthenticateWorkflowMultipleUsers) {
+  test_utils::TestBiodObserver observer;
+  fake_biod_client_.AddObserver(&observer);
+  EXPECT_EQ(0, NumRecords(kTestUserId));
+
+  // Add two users, who have scanned three fingers between the two of them.
+  const std::string kUserOne = std::string(kTestUserId) + "1";
+  const std::string kUserTwo = std::string(kTestUserId) + "2";
+
+  const std::string kLabelOne = std::string(kTestLabel) + "1";
+  const std::string kLabelTwo = std::string(kTestLabel) + "2";
+  const std::string kLabelThree = std::string(kTestLabel) + "3";
+
+  // Generate 2 test fingerprints per user.
+  auto kUser1Finger1 = GenerateTestFingerprint(2);
+  auto kUser1Finger2 = GenerateTestFingerprint(2);
+  auto kUser2Finger1 = GenerateTestFingerprint(2);
+  auto kUser2Finger2 = GenerateTestFingerprint(2);
+
+  EnrollFingerprint(kUserOne, kLabelOne, kUser1Finger1);
+  EnrollFingerprint(kUserOne, kLabelTwo, kUser1Finger2);
+  // User one has registered finger two twice.
+  EnrollFingerprint(kUserOne, kLabelThree, kUser1Finger2);
+  EnrollFingerprint(kUserTwo, kLabelOne, kUser2Finger1);
+  EnrollFingerprint(kUserTwo, kLabelTwo, kUser2Finger2);
+  // User two has allowed user one to unlock his/her account with his/her
+  // first finger.
+  EnrollFingerprint(kUserTwo, kLabelThree, kUser1Finger1);
+
+  dbus::ObjectPath returned_path;
+  fake_biod_client_.StartAuthSession(
+      base::Bind(&test_utils::CopyObjectPath, &returned_path));
+  task_runner_->RunUntilIdle();
+  EXPECT_NE(returned_path, dbus::ObjectPath());
+
+  // Verify that if a user registers the same finger to two different labels,
+  // both labels are returned as matches.
+  AuthScanMatches expected_auth_scans_matches;
+  expected_auth_scans_matches[kUserOne] = {kLabelTwo, kLabelThree};
+  SendAuthScanDone(kUser1Finger2[0]);
+  EXPECT_TRUE(
+      observer.CheckExpectedLastAttemptMatches(expected_auth_scans_matches));
+
+  // Verify that a fingerprint associated with one user and one label returns a
+  // match with one user and one label.
+  expected_auth_scans_matches.clear();
+  expected_auth_scans_matches[kUserTwo] = {kLabelOne};
+  SendAuthScanDone(kUser2Finger1[0]);
+  EXPECT_TRUE(
+      observer.CheckExpectedLastAttemptMatches(expected_auth_scans_matches));
+
+  // Verify if two users register the same fingerprint, the matches contain
+  // both users.
+  expected_auth_scans_matches.clear();
+  expected_auth_scans_matches[kUserOne] = {kLabelOne};
+  expected_auth_scans_matches[kUserTwo] = {kLabelThree};
+  SendAuthScanDone(kUser1Finger1[0]);
+  EXPECT_TRUE(
+      observer.CheckExpectedLastAttemptMatches(expected_auth_scans_matches));
+
+  // Verify if a unregistered finger is scanned, the matches are empty.
+  expected_auth_scans_matches.clear();
+  SendAuthScanDone("Unregistered");
+  EXPECT_TRUE(
+      observer.CheckExpectedLastAttemptMatches(expected_auth_scans_matches));
+}
+
+TEST_F(FakeBiodClientTest, TestGetAllRecords) {
+  // Verify that initially |kTestUserId| will have no fingerprints.
+  int num_fingerprints = -1;
+  fake_biod_client_.GetRecordsForUser(
+      kTestUserId, base::Bind(&test_utils::CopyNumRecords, &num_fingerprints));

[Daniel Erat, 2017/04/07 23:40:56]

why copy the number of records here instead of just copying the actual records
and comparing .size()? it'd be cleaner to delete CopyNumRecords if you don't
actually need it.

[sammiequon, 2017/04/08 00:43:32]

Done.

+  task_runner_->RunUntilIdle();
+  EXPECT_EQ(0, num_fingerprints);
+
+  // Verify that after enrolling 2 fingerprints, a GetRecords call return 2
+  // items.
+  EnrollNTestFingerprints(kTestUserId, kTestLabel, GenerateTestFingerprint(2),
+                          2);
+  num_fingerprints = -1;
+  fake_biod_client_.GetRecordsForUser(
+      kTestUserId, base::Bind(&test_utils::CopyNumRecords, &num_fingerprints));
+  task_runner_->RunUntilIdle();
+  EXPECT_EQ(2, num_fingerprints);
+
+  // Verify that GetRecords call for a user with no registered fingerprints
+  // should return 0 items.
+  num_fingerprints = -1;
+  fake_biod_client_.GetRecordsForUser(
+      "noregisteredginerprints@gmail.com",
+      base::Bind(&test_utils::CopyNumRecords, &num_fingerprints));
+  task_runner_->RunUntilIdle();
+  EXPECT_EQ(0, num_fingerprints);
+}
+
+TEST_F(FakeBiodClientTest, TestDestroyingRecords) {
+  // Verify that after enrolling 2 fingerprints and destroying them, 0
+  // fingerprints will remain.
+  EnrollNTestFingerprints(kTestUserId, kTestLabel, GenerateTestFingerprint(2),
+                          2);
+  EXPECT_EQ(2, NumRecords(kTestUserId));
+  fake_biod_client_.DestroyAllRecords();
+  EXPECT_EQ(0, NumRecords(kTestUserId));
+}
+
+TEST_F(FakeBiodClientTest, TestGetAndSetRecordLabels) {
+  const std::string& kLabelOne = "Finger 1";

[Daniel Erat, 2017/04/07 23:40:56]

const std::string (no reference since this isn't actually a reference to some
other std::string object)

[sammiequon, 2017/04/08 00:43:33]

Done.

+  const std::string& kLabelTwo = "Finger 2";
+
+  EnrollFingerprint(kTestUserId, kLabelOne, GenerateTestFingerprint(2));
+  EnrollFingerprint(kTestUserId, kLabelTwo, GenerateTestFingerprint(2));
+  EXPECT_EQ(2, NumRecords(kTestUserId));
+  std::vector<dbus::ObjectPath> enrollment_paths;
+  fake_biod_client_.GetRecordsForUser(
+      kTestUserId,
+      base::Bind(&test_utils::CopyObjectPathArray, &enrollment_paths));
+  task_runner_->RunUntilIdle();
+  EXPECT_EQ(2u, enrollment_paths.size());
+
+  // Verify the labels we get using GetLabel are the same as the one we
+  // originally set.
+  std::string returned_label;
+  fake_biod_client_.RequestRecordLabel(
+      enrollment_paths[0], base::Bind(&test_utils::CopyLabel, &returned_label));
+  task_runner_->RunUntilIdle();
+  EXPECT_EQ(kLabelOne, returned_label);
+
+  returned_label = "";
+  fake_biod_client_.RequestRecordLabel(
+      enrollment_paths[1], base::Bind(&test_utils::CopyLabel, &returned_label));
+  task_runner_->RunUntilIdle();
+  EXPECT_EQ(kLabelTwo, returned_label);
+
+  // Verify that by setting a new label, getting the label will return the value
+  // of the new label.
+  const std::string& kNewLabelTwo = "Finger 2 New";
+  fake_biod_client_.SetRecordLabel(enrollment_paths[1], kNewLabelTwo);
+  fake_biod_client_.RequestRecordLabel(
+      enrollment_paths[1], base::Bind(&test_utils::CopyLabel, &returned_label));
+  task_runner_->RunUntilIdle();
+  EXPECT_EQ(kNewLabelTwo, returned_label);
+}
+
+}  // namespace chromeos