Save password without an associated username.

chrome/browser/password_manager/password_form_manager_unittest.cc

 // Copyright (c) 2011 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 "testing/gtest/include/gtest/gtest.h"
 
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/password_manager/password_form_manager.h"
 #include "chrome/browser/password_manager/password_manager.h"
@@ skipping 15 matching lines @@
     observed_form_.submit_element = ASCIIToUTF16("signIn");
     observed_form_.signon_realm = "http://www.google.com";
 
     saved_match_ = observed_form_;
     saved_match_.origin = GURL("http://www.google.com/a/ServiceLoginAuth");
     saved_match_.action = GURL("http://www.google.com/a/ServiceLogin");
     saved_match_.preferred = true;
     saved_match_.username_value = ASCIIToUTF16("test@gmail.com");
     saved_match_.password_value = ASCIIToUTF16("test1");
     profile_ = new TestingProfile();
+
+    // A form for testing a case there's no username (there's only password).
+    observed_form2_.origin = GURL("http://www.google.com/b/LoginAuth");
+    observed_form2_.action = GURL("http://www.google.com/b/Login");
+    observed_form2_.password_element = ASCIIToUTF16("Passwd");
+    observed_form2_.submit_element = ASCIIToUTF16("signIn");
+    observed_form2_.signon_realm = "http://www.google.com";
+
+    saved_match2_ = observed_form_;
+    saved_match2_.origin = GURL("http://www.google.com/a/ServiceLoginAuth");
+    saved_match2_.action = GURL("http://www.google.com/a/ServiceLogin");
+    saved_match2_.preferred = true;
+    saved_match2_.password_value = ASCIIToUTF16("test2");
+    profile2_ = new TestingProfile();
   }
 
   virtual void TearDown() {
     delete profile_;
+    delete profile2_;
   }
 
   PasswordForm* GetPendingCredentials(PasswordFormManager* p) {
     return &p->pending_credentials_;
   }
 
   void SimulateMatchingPhase(PasswordFormManager* p, bool find_match) {
     // Roll up the state to mock out the matching phase.
     p->state_ = PasswordFormManager::POST_MATCHING_PHASE;
     if (!find_match)
       return;
 
     PasswordForm* match = new PasswordForm(saved_match_);
     // Heap-allocated form is owned by p.
     p->best_matches_[match->username_value] = match;
     p->preferred_match_ = match;
   }
 
   bool IgnoredResult(PasswordFormManager* p, PasswordForm* form) {
     return p->IgnoreResult(*form);
   }
 
   Profile* profile() { return profile_; }
+  Profile* profile2() { return profile2_; }
 
   PasswordForm* observed_form() { return &observed_form_; }
   PasswordForm* saved_match() { return &saved_match_; }
+  PasswordForm* observed_form2() { return &observed_form2_; }
+  PasswordForm* saved_match2() { return &saved_match2_; }
+
 
  private:
   PasswordForm observed_form_;
   PasswordForm saved_match_;
   Profile* profile_;
+  PasswordForm observed_form2_;
+  PasswordForm saved_match2_;
+  Profile* profile2_;
 };
 
 TEST_F(PasswordFormManagerTest, TestNewLogin) {
   PasswordFormManager* manager = new PasswordFormManager(
       profile(), NULL, *observed_form(), false);
   SimulateMatchingPhase(manager, false);
   // User submits credentials for the observed form.
   PasswordForm credentials = *observed_form();
   credentials.username_value = saved_match()->username_value;
   credentials.password_value = saved_match()->password_value;
@@ skipping 35 matching lines @@
             GetPendingCredentials(manager)->signon_realm);
   EXPECT_TRUE(GetPendingCredentials(manager)->preferred);
   EXPECT_EQ(new_pass,
             GetPendingCredentials(manager)->password_value);
   EXPECT_EQ(new_user,
             GetPendingCredentials(manager)->username_value);
   // Done.
   delete manager;
 }
 
+TEST_F(PasswordFormManagerTest, TestNewLoginAndUpdateWithoutUsername) {
+  PasswordFormManager* manager = new PasswordFormManager(
+      profile(), NULL, *observed_form2(), false);

[tim (not reviewing), 2012/04/03 16:40:27]

Why do we need observed_form2 It seems tests use either version, so it should be
a way to set this up so that we don't need the extra members or Profile in the
test class, and instead do an extra configuration step in the tests that want
'2'.

Also, this appears to be mostly copy/paste from the test above.  Can you move
the similar code into a function (perhaps parameterized by your form1/2
variables) that is called once with a non-empty username form and once with an
empty username?  This will prevent the tests from diverging in the future.

[Yumikiyo Osanai, 2012/04/05 00:19:50]

I basically agree with your idea.

I've removed the '2' members and added SetUpForNoUsernameTests() function.
And, I've took refactoring the code to delete copy/paste tests as possible.

On 2012/04/03 16:40:27, timsteele wrote:

+  SimulateMatchingPhase(manager, false);
+
+  // User submits credentials for the observed form.
+  PasswordForm credentials = *observed_form2();
+  credentials.username_value = saved_match2()->username_value;
+  credentials.password_value = saved_match2()->password_value;
+  credentials.preferred = true;
+  manager->ProvisionallySave(credentials);
+
+  // Successful login. The PasswordManager would instruct PasswordFormManager
+  // to save, which should know this is a new login.
+  EXPECT_TRUE(manager->IsNewLogin());
+  // Make sure the credentials that would be submitted on successful login
+  // are going to match the stored entry in the db.
+  EXPECT_EQ(observed_form2()->origin.spec(),
+            GetPendingCredentials(manager)->origin.spec());
+  EXPECT_EQ(observed_form2()->signon_realm,
+            GetPendingCredentials(manager)->signon_realm);
+  EXPECT_TRUE(GetPendingCredentials(manager)->preferred);
+  EXPECT_EQ(saved_match2()->password_value,
+            GetPendingCredentials(manager)->password_value);
+  EXPECT_EQ(saved_match2()->username_value,
+            GetPendingCredentials(manager)->username_value);
+
+  // Now, suppose the user re-visits the site and wants to save an additional
+  // login for the site with a new password. In this case, the matching phase
+  // will yield the previously saved login.
+  SimulateMatchingPhase(manager, true);
+
+  // Set up the new login.
+  string16 new_pass = ASCIIToUTF16("newpass2");
+  credentials.password_value = new_pass;
+  manager->ProvisionallySave(credentials);
+
+  // Again, the PasswordFormManager should know this is still a new login.
+  EXPECT_TRUE(manager->IsNewLogin());
+
+  // And make sure everything squares up again.
+  EXPECT_EQ(observed_form2()->origin.spec(),
+            GetPendingCredentials(manager)->origin.spec());
+  EXPECT_EQ(observed_form2()->signon_realm,
+            GetPendingCredentials(manager)->signon_realm);
+  EXPECT_TRUE(GetPendingCredentials(manager)->preferred);
+  EXPECT_EQ(new_pass,
+            GetPendingCredentials(manager)->password_value);
+
+  // Done.
+  delete manager;
+}
+
+
 TEST_F(PasswordFormManagerTest, TestUpdatePassword) {
   // Create a PasswordFormManager with observed_form, as if we just
   // saw this form and need to find matching logins.
   PasswordFormManager* manager = new PasswordFormManager(
       profile(), NULL, *observed_form(), false);
   SimulateMatchingPhase(manager, true);
 
   // User submits credentials for the observed form using a username previously
   // stored, but a new password. Note that the observed form may have different
   // origin URL (as it does in this case) than the saved_match, but we want to
@@ skipping 54 matching lines @@
   login.username_value = saved_match()->username_value;
   login.password_value = saved_match()->password_value;
   manager->ProvisionallySave(login);
   EXPECT_FALSE(manager->IsNewLogin());
   // We bless our saved PasswordForm entry with the action URL of the
   // observed form.
   EXPECT_EQ(observed_form()->action,
             GetPendingCredentials(manager.get())->action);
 }
 
+TEST_F(PasswordFormManagerTest, TestEmptyActionWithoutUsername) {
+  scoped_ptr<PasswordFormManager> manager(new PasswordFormManager(
+      profile(), NULL, *observed_form2(), false));
+
+  saved_match2()->action = GURL();
+  SimulateMatchingPhase(manager.get(), true);
+  // User logs in with the autofilled username / password from saved_match.
+  PasswordForm login = *observed_form2();
+  login.password_value = saved_match2()->password_value;
+  manager->ProvisionallySave(login);
+
+  // If there's no username, we supporse it's the new login.
+  EXPECT_TRUE(manager->IsNewLogin());
+  // We bless our saved PasswordForm entry with the action URL of the
+  // observed form.
+  EXPECT_EQ(observed_form2()->action,
+            GetPendingCredentials(manager.get())->action);
+}
+
 TEST_F(PasswordFormManagerTest, TestValidForms) {
   // User submits credentials for the observed form.
   PasswordForm credentials = *observed_form();
   credentials.scheme = PasswordForm::SCHEME_HTML;
   credentials.username_value = saved_match()->username_value;
   credentials.password_value = saved_match()->password_value;
 
   // Form with both username_element and password_element.
   PasswordFormManager manager1(profile(), NULL, credentials, false);
   SimulateMatchingPhase(&manager1, false);
   EXPECT_TRUE(manager1.HasValidPasswordForm());
 
   // Form without a username_element but with a password_element.
+  // It should be true, because we should save and autofill a password
+  // if there isn't any username.
   credentials.username_element.clear();
   PasswordFormManager manager2(profile(), NULL, credentials, false);
   SimulateMatchingPhase(&manager2, false);
-  EXPECT_FALSE(manager2.HasValidPasswordForm());
+  EXPECT_TRUE(manager2.HasValidPasswordForm());
 
   // Form without a password_element but with a username_element.
   credentials.username_element = saved_match()->username_element;
   credentials.password_element.clear();
   PasswordFormManager manager3(profile(), NULL, credentials, false);
   SimulateMatchingPhase(&manager3, false);
   EXPECT_FALSE(manager3.HasValidPasswordForm());
 
   // Form with neither a password_element nor a username_element.
   credentials.username_element.clear();
   credentials.password_element.clear();
   PasswordFormManager manager4(profile(), NULL, credentials, false);
   SimulateMatchingPhase(&manager4, false);
   EXPECT_FALSE(manager4.HasValidPasswordForm());
 }
 
+TEST_F(PasswordFormManagerTest, TestValidFormsWithNoUsername) {
+  // User submits credentials for the observed form.
+  PasswordForm credentials = *observed_form2();
+  credentials.scheme = PasswordForm::SCHEME_HTML;
+  credentials.password_value = saved_match2()->password_value;
+
+  // Form with a password_element.
+  PasswordFormManager manager1(profile2(), NULL, credentials, false);
+  SimulateMatchingPhase(&manager1, false);
+  EXPECT_TRUE(manager1.HasValidPasswordForm());
+
+  // Form with no a password_element
+  credentials.password_element.clear();
+  PasswordFormManager manager2(profile2(), NULL, credentials, false);
+  SimulateMatchingPhase(&manager2, false);
+  EXPECT_FALSE(manager2.HasValidPasswordForm());
+}
+
 TEST_F(PasswordFormManagerTest, TestValidFormsBasic) {
   // User submits credentials for the observed form.
   PasswordForm credentials = *observed_form();
   credentials.scheme = PasswordForm::SCHEME_BASIC;
   credentials.username_value = saved_match()->username_value;
   credentials.password_value = saved_match()->password_value;
 
   // Form with both username_element and password_element.
   PasswordFormManager manager1(profile(), NULL, credentials, false);
   SimulateMatchingPhase(&manager1, false);
@@ skipping 12 matching lines @@
   SimulateMatchingPhase(&manager3, false);
   EXPECT_TRUE(manager3.HasValidPasswordForm());
 
   // Form with neither a password_element nor a username_element.
   credentials.username_element.clear();
   credentials.password_element.clear();
   PasswordFormManager manager4(profile(), NULL, credentials, false);
   SimulateMatchingPhase(&manager4, false);
   EXPECT_TRUE(manager4.HasValidPasswordForm());
 }
+
+TEST_F(PasswordFormManagerTest, TestValidFormsBasicWithoutUsername) {
+  // User submits credentials for the observed form.
+  PasswordForm credentials = *observed_form2();
+  credentials.scheme = PasswordForm::SCHEME_BASIC;
+  credentials.password_value = saved_match2()->password_value;
+
+  // Form with a password_element.
+  PasswordFormManager manager1(profile2(), NULL, credentials, false);
+  SimulateMatchingPhase(&manager1, false);
+  EXPECT_TRUE(manager1.HasValidPasswordForm());
+
+  // Form with no a password_element
+  credentials.password_element.clear();
+  PasswordFormManager manager2(profile2(), NULL, credentials, false);
+  SimulateMatchingPhase(&manager2, false);
+  EXPECT_TRUE(manager2.HasValidPasswordForm());
+}

[tim (not reviewing), 2012/04/03 16:40:27]

What happens if
- a user saves a password for a form with no username element on foo.com
- the user visits foo.com/a, which has a password form *with* [and next,
without] a username element. 

[Yumikiyo Osanai, 2012/04/05 00:19:50]

Umm... I don't understand what you want to test...
Please write the detail.

And I've tried to create the test as PasswordFormManagerTest(), but I haven't
found the way to express "visit foo.com/a".
I know manager->ProvisionallySave() can save the password, but I haven't found
the funciton that offers "visit a URL".
Please see the  TEST_F(PasswordFormManagerTest, TestVisitOtherForm), and if you
have any idea, please let me know.

On 2012/04/03 16:40:27, timsteele wrote:

+