Add stricter tests for RapporService::LoadSecret

components/rappor/rappor_prefs_unittest.cc

Index: components/rappor/rappor_prefs_unittest.cc
diff --git a/components/rappor/rappor_prefs_unittest.cc b/components/rappor/rappor_prefs_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..764484469d4e97d357091475ab9252e4d5c4c8f1
--- /dev/null
+++ b/components/rappor/rappor_prefs_unittest.cc
@@ -0,0 +1,144 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.

[Alexei Svitkine (slow), 2015/01/14 17:29:06]

2015 now.

[Steven Holte, 2015/01/14 21:12:16]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/rappor/rappor_prefs.h"
+
+#include "base/base64.h"
+#include "base/prefs/testing_pref_service.h"
+#include "base/test/histogram_tester.h"
+#include "components/rappor/byte_vector_utils.h"
+#include "components/rappor/proto/rappor_metric.pb.h"
+#include "components/rappor/rappor_pref_names.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace rappor {
+
+namespace internal {
+
+namespace {
+
+// Convert a secret to base 64 and store it in preferences.
+void StoreSecret(TestingPrefServiceSimple* test_prefs,

[Alexei Svitkine (slow), 2015/01/14 17:29:05]

Nit: Modifiable params (i.e. test_prefs) should be last.

[Steven Holte, 2015/01/14 21:12:16]

Done.

+                 const std::string& secret) {
+  std::string secret_base64;
+  base::Base64Encode(secret, &secret_base64);
+  test_prefs->SetString(prefs::kRapporSecret, secret_base64);
+}
+
+// Verify that the current value of the secret pref matches the loaded secret.
+void ExpectConsistentSecret(const TestingPrefServiceSimple& test_prefs,
+                            const std::string& loaded_secret) {
+  std::string pref = test_prefs.GetString(prefs::kRapporSecret);
+  std::string decoded_pref;
+  EXPECT_TRUE(base::Base64Decode(pref, &decoded_pref));
+  EXPECT_EQ(loaded_secret, decoded_pref);
+}
+
+}  // namespace
+
+TEST(RapporPrefsTest, EmptyCohort) {
+  base::HistogramTester histogram;

[Alexei Svitkine (slow), 2015/01/14 17:29:05]

Nit: tester or histogram_tester, it's not a histogram.

[Steven Holte, 2015/01/14 21:12:16]

Done.

+  TestingPrefServiceSimple test_prefs;
+  RegisterPrefs(test_prefs.registry());

[Alexei Svitkine (slow), 2015/01/14 17:29:05]

All these tests begin the same. Please make a test harness class so you can do
make the tester and prefs instance variables of it and the RegisterPrefs() call
in the ctor.

[Steven Holte, 2015/01/14 21:12:16]

Done.

+  test_prefs.ClearPref(prefs::kRapporCohortSeed);
+  // Loaded cohort should have been rerolled into a valid number.
+  int32_t cohort = LoadCohort(&test_prefs);
+  histogram.ExpectUniqueSample(kLoadCohortHistogramName, LOAD_EMPTY_VALUE, 1);
+  EXPECT_GE(cohort, 0);
+  EXPECT_LT(cohort, RapporParameters::kMaxCohorts);
+  // The preferences should be consistent with the loaded value.
+  int32_t pref = test_prefs.GetInteger(prefs::kRapporCohortSeed);
+  EXPECT_EQ(pref, cohort);
+}
+
+TEST(RapporPrefsTest, LoadCohort) {
+  base::HistogramTester histogram;
+  TestingPrefServiceSimple test_prefs;
+  RegisterPrefs(test_prefs.registry());
+  test_prefs.SetInteger(prefs::kRapporCohortSeed, 1);
+  // Loading the valid cohort should just retrieve it.
+  int32_t cohort = LoadCohort(&test_prefs);
+  histogram.ExpectUniqueSample(kLoadCohortHistogramName, LOAD_SUCCESS, 1);
+  EXPECT_EQ(1, cohort);
+  // The preferences should be consistent with the loaded value.
+  int32_t pref = test_prefs.GetInteger(prefs::kRapporCohortSeed);
+  EXPECT_EQ(pref, cohort);
+}
+
+TEST(RapporPrefsTest, CorruptCohort) {
+  base::HistogramTester histogram;
+  TestingPrefServiceSimple test_prefs;
+  RegisterPrefs(test_prefs.registry());
+  // Set an invalid cohort value in the preference.
+  test_prefs.SetInteger(prefs::kRapporCohortSeed, -10);
+  // Loaded cohort should have been rerolled into a valid number.
+  int32_t cohort = LoadCohort(&test_prefs);
+  histogram.ExpectUniqueSample(kLoadCohortHistogramName, LOAD_CORRUPT_VALUE, 1);
+  EXPECT_GE(cohort, 0);
+  EXPECT_LT(cohort, RapporParameters::kMaxCohorts);
+  // The preferences should be consistent with the loaded value.
+  int32_t pref = test_prefs.GetInteger(prefs::kRapporCohortSeed);
+  EXPECT_EQ(pref, cohort);
+}
+
+TEST(RapporPrefsTest, EmptySecret) {
+  base::HistogramTester histogram;
+  TestingPrefServiceSimple test_prefs;
+  RegisterPrefs(test_prefs.registry());
+  test_prefs.ClearPref(prefs::kRapporSecret);
+  // Loaded secret should be rerolled from empty.
+  std::string secret2 = LoadSecret(&test_prefs);
+  histogram.ExpectUniqueSample(kLoadSecretHistogramName, LOAD_EMPTY_VALUE, 1);
+  EXPECT_EQ(HmacByteVectorGenerator::kEntropyInputSize, secret2.size());
+  // The stored preference should also be updated.
+  ExpectConsistentSecret(test_prefs, secret2);
+}
+
+TEST(RapporPrefsTest, LoadSecret) {
+  base::HistogramTester histogram;
+  TestingPrefServiceSimple test_prefs;
+  RegisterPrefs(test_prefs.registry());
+  std::string secret1 = HmacByteVectorGenerator::GenerateEntropyInput();
+  StoreSecret(&test_prefs, secret1);
+  // Secret should load successfully.
+  std::string secret2 = LoadSecret(&test_prefs);
+  histogram.ExpectUniqueSample(kLoadSecretHistogramName, LOAD_SUCCESS, 1);
+  EXPECT_EQ(secret1, secret2);
+  // The stored preference should also be unchanged.
+  ExpectConsistentSecret(test_prefs, secret2);
+}
+
+TEST(RapporPrefsTest, CorruptSecret) {
+  base::HistogramTester histogram;
+  TestingPrefServiceSimple test_prefs;
+  RegisterPrefs(test_prefs.registry());
+  // Store an invalid secret in the preferences that won't decode as base64.
+  test_prefs.SetString(prefs::kRapporSecret, "!!INVALID!!");
+  // We should have rerolled a new secret.
+  std::string secret2 = LoadSecret(&test_prefs);
+  histogram.ExpectUniqueSample(kLoadSecretHistogramName, LOAD_CORRUPT_VALUE, 1);
+  EXPECT_EQ(HmacByteVectorGenerator::kEntropyInputSize, secret2.size());
+  // The stored preference should also be updated.
+  ExpectConsistentSecret(test_prefs, secret2);
+}
+
+TEST(RapporPrefsTest, DecodableCorruptSecret) {
+  base::HistogramTester histogram;
+  TestingPrefServiceSimple test_prefs;
+  RegisterPrefs(test_prefs.registry());
+  // Store an invalid secret in the preferences that will decode as base64.
+  std::string secret1 = "!!INVALID!!";
+  StoreSecret(&test_prefs, secret1);
+  // We should have rerolled a new secret.
+  std::string secret2 = LoadSecret(&test_prefs);
+  histogram.ExpectUniqueSample(kLoadSecretHistogramName, LOAD_CORRUPT_VALUE, 1);
+  EXPECT_NE(secret1, secret2);
+  EXPECT_EQ(HmacByteVectorGenerator::kEntropyInputSize, secret2.size());
+  // The stored preference should also be updated.
+  ExpectConsistentSecret(test_prefs, secret2);
+}
+
+}  // namespace internal
+
+}  // namespace rappor