Supplimentary identifier for passwords specific

components/sync/core_impl/sync_manager_impl_unittest.cc

 // Copyright 2012 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.
 
 // Unit tests for the SyncApi. Note that a lot of the underlying
 // functionality is provided by the Syncable layer, which has its own
 // unit tests. We'll test SyncApi specific things in this harness.
 
 #include "components/sync/core_impl/sync_manager_impl.h"
 
@@ skipping 204 matching lines @@
   return entry.GetMetahandle();
 }
 
 int GetTotalNodeCount(UserShare* share, int64_t root) {
   ReadTransaction trans(FROM_HERE, share);
   ReadNode node(&trans);
   EXPECT_EQ(BaseNode::INIT_OK, node.InitByIdLookup(root));
   return node.GetTotalNodeCount();
 }
 
+const char kUrl[] = "example.com";
+const char kPasswordValue[] = "secret";
+const char kClientTag[] = "tag";
+
 }  // namespace
 
 class SyncApiTest : public testing::Test {
  public:
   void SetUp() override { test_user_share_.SetUp(); }
 
   void TearDown() override { test_user_share_.TearDown(); }
 
  protected:
   // Create an entry with the given |model_type|, |client_tag| and
@@ skipping 276 matching lines @@
     ReadTransaction trans(FROM_HERE, user_share());
     trans.GetCryptographer()->AddKey(params);
   }
   {
     WriteTransaction trans(FROM_HERE, user_share());
     ReadNode root_node(&trans);
     root_node.InitByRootLookup();
 
     WriteNode password_node(&trans);
     WriteNode::InitUniqueByCreationResult result =
-        password_node.InitUniqueByCreation(PASSWORDS, root_node, "foo");
+        password_node.InitUniqueByCreation(PASSWORDS, root_node, kClientTag);
     EXPECT_EQ(WriteNode::INIT_SUCCESS, result);
     sync_pb::PasswordSpecificsData data;
-    data.set_password_value("secret");
+    data.set_password_value(kPasswordValue);
     password_node.SetPasswordSpecifics(data);
   }
   {
     ReadTransaction trans(FROM_HERE, user_share());
 
     ReadNode password_node(&trans);
     EXPECT_EQ(BaseNode::INIT_OK,
-              password_node.InitByClientTagLookup(PASSWORDS, "foo"));
+              password_node.InitByClientTagLookup(PASSWORDS, kClientTag));
     const sync_pb::PasswordSpecificsData& data =
         password_node.GetPasswordSpecifics();
-    EXPECT_EQ("secret", data.password_value());
+    EXPECT_EQ(kPasswordValue, data.password_value());
+    // Check that nothing has appeared in the unencrypted field.
+    EXPECT_FALSE(password_node.GetEntitySpecifics()
+                     .password()
+                     .has_unencrypted_metadata());
   }
 }
 
+TEST_F(SyncApiTest, WriteDoesntChangeUnencryptedPasswordMetadata) {

[Nicolas Zea, 2016/08/26 23:47:01]

What's the purpose of this test? Just that the metadata gets persisted? It seems
like the test above could do that too (once the population of the metadata logic
is added)

[melandory, 2016/08/26 23:59:47]

Fixed. My idea was to have test for persisting of metadata and test above to
check that it is not currently populated.

+  KeyParams params = {"localhost", "username", "passphrase"};
+  {
+    ReadTransaction trans(FROM_HERE, user_share());
+    trans.GetCryptographer()->AddKey(params);
+  }
+  {
+    WriteTransaction trans(FROM_HERE, user_share());
+    ReadNode root_node(&trans);
+    root_node.InitByRootLookup();
+
+    WriteNode password_node(&trans);
+    WriteNode::InitUniqueByCreationResult result =
+        password_node.InitUniqueByCreation(PASSWORDS, root_node, kClientTag);
+    EXPECT_EQ(WriteNode::INIT_SUCCESS, result);
+    sync_pb::EntitySpecifics entity_specifics;
+    sync_pb::PasswordSpecifics* password_specifics =
+        entity_specifics.mutable_password();
+    password_specifics->mutable_unencrypted_metadata()->set_url(kUrl);
+    password_node.SetEntitySpecifics(entity_specifics);
+    sync_pb::PasswordSpecificsData data;
+    data.set_password_value(kPasswordValue);
+    password_node.SetPasswordSpecifics(data);
+  }
+  {
+    ReadTransaction trans(FROM_HERE, user_share());
+
+    ReadNode password_node(&trans);
+    EXPECT_EQ(BaseNode::INIT_OK,
+              password_node.InitByClientTagLookup(PASSWORDS, kClientTag));
+    const sync_pb::PasswordSpecificsData& data =
+        password_node.GetPasswordSpecifics();
+    EXPECT_EQ(kUrl, password_node.GetEntitySpecifics()
+                        .password()
+                        .unencrypted_metadata()
+                        .url());
+    EXPECT_EQ(kPasswordValue, data.password_value());
+  }
+}
+
 TEST_F(SyncApiTest, WriteEncryptedTitle) {
   KeyParams params = {"localhost", "username", "passphrase"};
   {
     ReadTransaction trans(FROM_HERE, user_share());
     trans.GetCryptographer()->AddKey(params);
   }
   encryption_handler()->EnableEncryptEverything();
   int bookmark_id;
   {
     WriteTransaction trans(FROM_HERE, user_share());
@@ skipping 1506 matching lines @@
     data.set_password_value("secret2");
     cryptographer->Encrypt(
         data, entity_specifics.mutable_password()->mutable_encrypted());
     node.SetPasswordSpecifics(data);
     const syncable::Entry* node_entry = node.GetEntry();
     EXPECT_TRUE(node_entry->GetIsUnsynced());
   }
 }
 
 // Passwords have their own handling for encryption. Verify setting a new
 // passphrase updates the data.

[Nicolas Zea, 2016/08/26 23:47:01]

nit: mention that it also clears the metadata

[melandory, 2016/08/26 23:59:47]

Done.

 TEST_F(SyncManagerTest, UpdatePasswordNewPassphrase) {
-  std::string client_tag = "title";
   EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION));
   sync_pb::EntitySpecifics entity_specifics;
   {
     ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
     Cryptographer* cryptographer = trans.GetCryptographer();
     sync_pb::PasswordSpecificsData data;
-    data.set_password_value("secret");
+    data.set_password_value(kPasswordValue);
+    entity_specifics.mutable_password()
+        ->mutable_unencrypted_metadata()
+        ->set_url(kUrl);
     cryptographer->Encrypt(
         data, entity_specifics.mutable_password()->mutable_encrypted());
   }
-  MakeServerNode(sync_manager_.GetUserShare(), PASSWORDS, client_tag,
-                 syncable::GenerateSyncableHash(PASSWORDS, client_tag),
+  EXPECT_TRUE(entity_specifics.password().has_unencrypted_metadata());
+  MakeServerNode(sync_manager_.GetUserShare(), PASSWORDS, kClientTag,
+                 syncable::GenerateSyncableHash(PASSWORDS, kClientTag),
                  entity_specifics);
   // New node shouldn't start off unsynced.
-  EXPECT_FALSE(ResetUnsyncedEntry(PASSWORDS, client_tag));
+  EXPECT_FALSE(ResetUnsyncedEntry(PASSWORDS, kClientTag));
 
   // Set a new passphrase. Should set is_unsynced.
   testing::Mock::VerifyAndClearExpectations(&encryption_observer_);
   EXPECT_CALL(encryption_observer_,
               OnBootstrapTokenUpdated(_, PASSPHRASE_BOOTSTRAP_TOKEN));
   ExpectPassphraseAcceptance();
   SetCustomPassphraseAndCheck("new_passphrase");
-  EXPECT_TRUE(ResetUnsyncedEntry(PASSWORDS, client_tag));
+  {
+    ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
+    Cryptographer* cryptographer = trans.GetCryptographer();
+    EXPECT_TRUE(cryptographer->is_ready());
+    ReadNode password_node(&trans);
+    EXPECT_EQ(BaseNode::INIT_OK,
+              password_node.InitByClientTagLookup(PASSWORDS, kClientTag));
+    const sync_pb::PasswordSpecificsData& data =
+        password_node.GetPasswordSpecifics();
+    EXPECT_EQ(kPasswordValue, data.password_value());
+    EXPECT_FALSE(password_node.GetEntitySpecifics()
+                     .password()
+                     .has_unencrypted_metadata());
+  }
+  EXPECT_TRUE(ResetUnsyncedEntry(PASSWORDS, kClientTag));
 }
 
 // Passwords have their own handling for encryption. Verify it does not result
 // in unnecessary writes via ReencryptEverything.
 TEST_F(SyncManagerTest, UpdatePasswordReencryptEverything) {
   std::string client_tag = "title";
   EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION));
   sync_pb::EntitySpecifics entity_specifics;
   {
     ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
@@ skipping 1252 matching lines @@
 // SyncManagerInitInvalidStorageTest::GetFactory will return
 // DirectoryBackingStore that ensures that SyncManagerImpl::OpenDirectory fails.
 // SyncManagerImpl initialization is done in SyncManagerTest::SetUp. This test's
 // task is to ensure that SyncManagerImpl reported initialization failure in
 // OnInitializationComplete callback.
 TEST_F(SyncManagerInitInvalidStorageTest, FailToOpenDatabase) {
   EXPECT_FALSE(initialization_succeeded_);
 }
 
 }  // namespace syncer