Skip to content

GitLab

T
tangled
Commit f6d0d007 authored by Owen Min's avatar Owen Min Committed by Commit Bot
Browse files
Options

Revert "Fix invalid password save offer in some cases." 

This reverts commit 3b9b2cca.

Reason for revert: The test is flaky on Win 10 and Win 7

Original change's description:
> Fix invalid password save offer in some cases.
> 
> Here we fix few password save flaws:
> 1. Invalid password save offer when a password form was filled and then a non
> password form was submitted.
> 2. A proper password form handling when a form is submitted into about:blank
> iframe using "target" attribute. This is a valid flow which some websites use to send private information via iframe.
> 
> The iframe test is implemented exactly for the #2 flaw.
> 
> Bug: 854171
> Change-Id: I39aec0d9b1f5fcbda551efb145f0c3be2456c0da
> Reviewed-on: https://chromium-review.googlesource.com/1106137
> Commit-Queue: Alexey Khoroshilov <sense@yandex-team.ru>
> Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575239}

TBR=dvadym@chromium.org,kolos@chromium.org,sense@yandex-team.ru

Change-Id: Ic89f7474943072def85f97f75033fa6ebe2ba736
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 854171
Reviewed-on: https://chromium-review.googlesource.com/1138518Reviewed-by: default avatarOwen Min <zmin@chromium.org>
Commit-Queue: Owen Min <zmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575341}
parent ebc96c64
Hide whitespace changes
Inline Side-by-side
Showing with 1 addition and 155 deletions
chrome/browser/password_manager/password_manager_interactive_uitest.cc
View file @ f6d0d007
......@@ -316,57 +316,6 @@ IN_PROC_BROWSER_TEST_P(PasswordManagerBrowserTestWithConditionalPopupViews,
"username_field", "password_field", submit);
}
IN_PROC_BROWSER_TEST_P(
PasswordManagerBrowserTestWithConditionalPopupViews,
SavedAfterTargetIframeSubmitFollowingNonPasswordFormSubmit) {
std::string submit =
"document.getElementById('input_submit_button').click();";
VerifyPasswordIsSavedAndFilled("/password/iframe_target.html",
"username_field", "password_field", submit);
}
IN_PROC_BROWSER_TEST_P(PasswordManagerBrowserTestWithConditionalPopupViews,
SaveNotTriggeredOnOtherFormSubmit) {
NavigateToFile("/password/password_form_with_simple_form.html");
// Fill password form.
FillElementWithValue("username_field", "user");
FillElementWithValue("password_field", "1234");
NavigationObserver observer(WebContents());
std::unique_ptr<BubbleObserver> prompt_observer(
new BubbleObserver(WebContents()));
// Submit OTHER form.
std::string submit =
"document.getElementById('submit_search_button').click();";
ASSERT_TRUE(content::ExecuteScript(WebContents(), submit));
observer.Wait();
WaitForPasswordStore();
EXPECT_FALSE(prompt_observer->IsSavePromptShownAutomatically());
}
IN_PROC_BROWSER_TEST_P(PasswordManagerBrowserTestWithConditionalPopupViews,
SaveNotTriggeredOnOtherFormSubmitWithUnownedForm) {
NavigateToFile("/password/no_form_elements_with_additional_form.html");
// Fill in unowned password form inputs.
FillElementWithValue("username", "user");
FillElementWithValue("password", "1234");
NavigationObserver observer(WebContents());
std::unique_ptr<BubbleObserver> prompt_observer(
new BubbleObserver(WebContents()));
// Submit OTHER form.
std::string submit =
"document.getElementById('submit_search_button').click();";
ASSERT_TRUE(content::ExecuteScript(WebContents(), submit));
observer.Wait();
WaitForPasswordStore();
EXPECT_FALSE(prompt_observer->IsSavePromptShownAutomatically());
}
INSTANTIATE_TEST_CASE_P(All,
PasswordManagerBrowserTestWithConditionalPopupViews,
/*popup_views_enabled=*/::testing::Bool());
......
chrome/test/data/password/done_redirect_parent.html deleted 100644 → 0
View file @ ebc96c64
<html>
<body>
<script>
if (!window.location.origin) {
window.location.origin = window.location.protocol + "//" + window.location.hostname;
}
var loc = window.location;
var locOrigin = loc.origin;
loc.href='about:blank';
window.parent.postMessage("done", locOrigin);
</script>
This html posts a message to window.parent.
</body>
</html>
chrome/test/data/password/iframe_target.html deleted 100644 → 0
View file @ ebc96c64
<html>
<body>
<script>
function receiveMessage(event){
if (event.data == "done") {
// Modify the form so it will POST as usual.
form = document.getElementById("testform");
form.action = "done.html";
form.target = "";
// Remove the password field so Password Manager will ignore the form.
form.removeChild(document.getElementById("password_field"));
// Submit the form.
document.getElementById("testform").submit();
}
}
window.addEventListener("message", receiveMessage, false);
</script>
<form method="POST" action="done_redirect_parent.html" id="testform" target="iframe">
<input type="text" id="username_field" name="username_field">
<input type="password" id="password_field" name="password_field">
<input type="submit" id="input_submit_button" name="input_submit_button">
</form>
<iframe name="iframe" src="about:blank"></iframe>
</body>
</html>
chrome/test/data/password/no_form_elements_with_additional_form.html deleted 100644 → 0
View file @ ebc96c64
<html>
<body>
<input type="text" id="username">
<input type="password" id="password">
<form method="POST" action="done.html" id="search_form">
<input type="text" id="search_field">
<input type="submit" id="submit_search_button">
</form>
</body>
</html>
chrome/test/data/password/password_form_with_simple_form.html deleted 100644 → 0
View file @ ebc96c64
<html>
<body>
<form method="POST" action="done.html" id="testform">
<input type="text" id="username_field" name="username_field">
<input type="password" id="password_field" name="password_field">
<input type="submit" id="input_submit_button" name="input_submit_button">
</form>
<form method="POST" action="done.html" id="search_form">
<input type="text" id="search_field">
<input type="submit" id="submit_search_button">
</form>
</body>
</html>
components/autofill/content/renderer/form_tracker.cc
View file @ f6d0d007
......@@ -186,11 +186,7 @@ void FormTracker::DidStartProvisionalLoad(WebDocumentLoader* document_loader) {
// (i.e. DidStartProvisionalLoad is called twice in this case). The check for
// kWebNavigationTypeLinkClicked is reliable only for content initiated
// navigations.
// Don't handle kWebNavigationTypeFormSubmitted here, it is handled in
// WillSubmitForm().
if (navigation_state->IsContentInitiated() &&
document_loader->GetNavigationType() !=
blink::kWebNavigationTypeFormSubmitted &&
document_loader->GetNavigationType() !=
blink::kWebNavigationTypeLinkClicked) {
FireProbablyFormSubmitted();
......
components/autofill/content/renderer/password_autofill_agent.cc
View file @ f6d0d007
......@@ -1077,12 +1077,6 @@ bool PasswordAutofillAgent::FrameCanAccessPasswordManager() {
return frame->GetSecurityOrigin().CanAccessPasswordManager();
}
bool PasswordAutofillAgent::
FrameCanAccessPasswordManagerWithoutAboutBlankCheck() {
blink::WebLocalFrame* frame = render_frame()->GetWebFrame();
return frame->GetSecurityOrigin().CanAccessPasswordManager();
}
void PasswordAutofillAgent::OnDynamicFormsSeen() {
SendPasswordForms(false /* only_visible */);
}
......@@ -1328,8 +1322,7 @@ void PasswordAutofillAgent::OnWillSubmitForm(
PasswordForm::SubmissionIndicatorEvent::HTML_FORM_SUBMISSION;
}
if (FrameCanAccessPasswordManagerWithoutAboutBlankCheck() &&
!submitted_form->origin.IsAboutBlank()) {
if (FrameCanAccessPasswordManager()) {
// Some observers depend on sending this information now instead of when
// the frame starts loading. If there are redirects that cause a new
// RenderView to be instantiated (such as redirects to the WebStore)
......
components/autofill/content/renderer/password_autofill_agent.h
View file @ f6d0d007
......@@ -186,12 +186,6 @@ class PasswordAutofillAgent : public content::RenderFrameObserver,
// unique origins aren't allowed access.
virtual bool FrameCanAccessPasswordManager();
// Determine whether the current frame is allowed to access the password
// manager without an additional check for about:blank. There is a valid
// usecase for about:blank iframe where a website POSTs a form to the empty
// iframe from the main frame.
virtual bool FrameCanAccessPasswordManagerWithoutAboutBlankCheck();
private:
// Ways to restrict which passwords are saved in ProvisionallySavePassword.
enum ProvisionallySaveRestriction {
......
components/autofill/content/renderer/test_password_autofill_agent.cc
View file @ f6d0d007
......@@ -17,9 +17,4 @@ bool TestPasswordAutofillAgent::FrameCanAccessPasswordManager() {
return true;
}
bool TestPasswordAutofillAgent::
FrameCanAccessPasswordManagerWithoutAboutBlankCheck() {
return true;
}
} // namespace autofill
components/autofill/content/renderer/test_password_autofill_agent.h
View file @ f6d0d007
......@@ -20,8 +20,6 @@ class TestPasswordAutofillAgent : public PasswordAutofillAgent {
// work with the password manager.
// PasswordAutofillAgent:
bool FrameCanAccessPasswordManager() override;
bool FrameCanAccessPasswordManagerWithoutAboutBlankCheck() override;
};
} // namespace autofill
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment