Skip to content

GitLab

T
tangled
Commit 3b9b2cca authored by Alexey Khoroshilov's avatar Alexey Khoroshilov Committed by Commit Bot
Browse files
Options

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: default avatarVadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575239}
parent 199e61ec
Hide whitespace changes
Inline Side-by-side
Showing with 155 additions and 1 deletion
chrome/browser/password_manager/password_manager_interactive_uitest.cc
View file @ 3b9b2cca
......@@ -316,6 +316,57 @@ 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 0 → 100644
View file @ 3b9b2cca
<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 0 → 100644
View file @ 3b9b2cca
<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 0 → 100644
View file @ 3b9b2cca
<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 0 → 100644
View file @ 3b9b2cca
<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 @ 3b9b2cca
......@@ -186,7 +186,11 @@ 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 @ 3b9b2cca
......@@ -1076,6 +1076,12 @@ 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 */);
}
......@@ -1321,7 +1327,8 @@ void PasswordAutofillAgent::OnWillSubmitForm(
PasswordForm::SubmissionIndicatorEvent::HTML_FORM_SUBMISSION;
}
if (FrameCanAccessPasswordManager()) {
if (FrameCanAccessPasswordManagerWithoutAboutBlankCheck() &&
!submitted_form->origin.IsAboutBlank()) {
// 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 @ 3b9b2cca
......@@ -186,6 +186,12 @@ 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 @ 3b9b2cca
......@@ -17,4 +17,9 @@ bool TestPasswordAutofillAgent::FrameCanAccessPasswordManager() {
return true;
}
bool TestPasswordAutofillAgent::
FrameCanAccessPasswordManagerWithoutAboutBlankCheck() {
return true;
}
} // namespace autofill
components/autofill/content/renderer/test_password_autofill_agent.h
View file @ 3b9b2cca
......@@ -20,6 +20,8 @@ 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