Commit ab76e2b5 authored by Vadym Doroshenko's avatar Vadym Doroshenko Committed by Commit Bot

Check safe browsing reputation only when the user clicks on password field.

Previously check reputation was when the user clicks on username field.
After removal of the old parser detection of username field is not working
anymore. And having check of username field is not so important, since
check is done on password field anyway. This CL is just a clean-up.

Bug: 949519
Change-Id: I9e5d3fdfffe1db9cfb21aa222f7d6b050d335d90
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1835505
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: default avatarMohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704156}
parent 45d6949f
...@@ -3286,11 +3286,9 @@ TEST_F(PasswordAutofillAgentTest, ...@@ -3286,11 +3286,9 @@ TEST_F(PasswordAutofillAgentTest,
#if BUILDFLAG(SAFE_BROWSING_DB_LOCAL) #if BUILDFLAG(SAFE_BROWSING_DB_LOCAL)
// Verify CheckSafeBrowsingReputation() is called when user starts filling // Verify CheckSafeBrowsingReputation() is called when user starts filling
// username or password field, and that this function is only called once. // a password field, and that this function is only called once.
// TODO(crbug.com/1008402): Enable this test after fixing the linked bug. TEST_F(PasswordAutofillAgentTest,
TEST_F( CheckSafeBrowsingReputationWhenUserStartsFillingUsernamePassword) {
PasswordAutofillAgentTest,
DISABLED_CheckSafeBrowsingReputationWhenUserStartsFillingUsernamePassword) {
ASSERT_EQ(0, fake_driver_.called_check_safe_browsing_reputation_cnt()); ASSERT_EQ(0, fake_driver_.called_check_safe_browsing_reputation_cnt());
// Simulate a click on password field to set its on focus, // Simulate a click on password field to set its on focus,
// CheckSafeBrowsingReputation() should be called. // CheckSafeBrowsingReputation() should be called.
...@@ -3302,14 +3300,20 @@ TEST_F( ...@@ -3302,14 +3300,20 @@ TEST_F(
SimulatePasswordTyping("modify"); SimulatePasswordTyping("modify");
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, fake_driver_.called_check_safe_browsing_reputation_cnt()); EXPECT_EQ(1, fake_driver_.called_check_safe_browsing_reputation_cnt());
// No CheckSafeBrowsingReputation() call on username field click.
SimulateElementClick(kUsernameName); SimulateElementClick(kUsernameName);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, fake_driver_.called_check_safe_browsing_reputation_cnt()); EXPECT_EQ(1, fake_driver_.called_check_safe_browsing_reputation_cnt());
// Navigate to another page and click on username field, SimulateElementClick(kPasswordName);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, fake_driver_.called_check_safe_browsing_reputation_cnt());
// Navigate to another page and click on password field,
// CheckSafeBrowsingReputation() should be triggered again. // CheckSafeBrowsingReputation() should be triggered again.
LoadHTML(kFormHTML); LoadHTML(kFormHTML);
SimulateElementClick(kUsernameName); SimulateElementClick(kPasswordName);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_EQ(2, fake_driver_.called_check_safe_browsing_reputation_cnt()); EXPECT_EQ(2, fake_driver_.called_check_safe_browsing_reputation_cnt());
} }
......
...@@ -736,33 +736,27 @@ bool PasswordAutofillAgent::FindPasswordInfoForElement( ...@@ -736,33 +736,27 @@ bool PasswordAutofillAgent::FindPasswordInfoForElement(
return true; return true;
} }
bool PasswordAutofillAgent::IsUsernameOrPasswordField( void PasswordAutofillAgent::MaybeCheckSafeBrowsingReputation(
const WebInputElement& element) { const WebInputElement& element) {
// Enabled on desktop and Android
#if BUILDFLAG(FULL_SAFE_BROWSING) || BUILDFLAG(SAFE_BROWSING_DB_REMOTE)
// Note: A site may use a Password field to collect a CVV or a Credit Card // Note: A site may use a Password field to collect a CVV or a Credit Card
// number, but showing a slightly misleading warning here is better than // number, but showing a slightly misleading warning here is better than
// showing no warning at all. // showing no warning at all.
if (element.IsPasswordFieldForAutofill()) if (!element.IsPasswordFieldForAutofill())
return true; return;
if (checked_safe_browsing_reputation_)
// If a field declares itself a username input, show the warning. return;
if (AutocompleteFlagForElement(element) == AutocompleteFlag::USERNAME)
return true;
// Otherwise, analyze the form and return true if this input element seems
// to be the username field.
std::unique_ptr<PasswordForm> password_form;
if (element.Form().IsNull()) {
// To double check that element's frame and |render_frame()->GetWebFrame()|
// which is used in |GetPasswordFormFromUnownedInputElements| are identical.
DCHECK_EQ(element.GetDocument().GetFrame(), render_frame()->GetWebFrame());
password_form = GetPasswordFormFromUnownedInputElements();
} else {
password_form = GetPasswordFormFromWebForm(element.Form());
}
if (!password_form) checked_safe_browsing_reputation_ = true;
return false; WebLocalFrame* frame = render_frame()->GetWebFrame();
return (password_form->username_element == element.NameForAutofill().Utf16()); GURL frame_url = GURL(frame->GetDocument().Url());
GURL action_url = element.Form().IsNull()
? GURL()
: form_util::GetCanonicalActionForForm(element.Form());
GetPasswordManagerDriver()->CheckSafeBrowsingReputation(action_url,
frame_url);
#endif
} }
bool PasswordAutofillAgent::TryToShowTouchToFill( bool PasswordAutofillAgent::TryToShowTouchToFill(
...@@ -793,22 +787,7 @@ bool PasswordAutofillAgent::ShowSuggestions(const WebInputElement& element, ...@@ -793,22 +787,7 @@ bool PasswordAutofillAgent::ShowSuggestions(const WebInputElement& element,
if (!FindPasswordInfoForElement(element, &username_element, &password_element, if (!FindPasswordInfoForElement(element, &username_element, &password_element,
&password_info)) { &password_info)) {
if (IsUsernameOrPasswordField(element)) { MaybeCheckSafeBrowsingReputation(element);
WebLocalFrame* frame = render_frame()->GetWebFrame();
GURL frame_url = GURL(frame->GetDocument().Url());
// Enabled on desktop and Android
#if BUILDFLAG(FULL_SAFE_BROWSING) || BUILDFLAG(SAFE_BROWSING_DB_REMOTE)
if (!checked_safe_browsing_reputation_) {
checked_safe_browsing_reputation_ = true;
GURL action_url =
element.Form().IsNull()
? GURL()
: form_util::GetCanonicalActionForForm(element.Form());
GetPasswordManagerDriver()->CheckSafeBrowsingReputation(action_url,
frame_url);
}
#endif
}
return false; return false;
} }
......
...@@ -172,8 +172,9 @@ class PasswordAutofillAgent : public content::RenderFrameObserver, ...@@ -172,8 +172,9 @@ class PasswordAutofillAgent : public content::RenderFrameObserver,
bool DidClearAutofillSelection( bool DidClearAutofillSelection(
const blink::WebFormControlElement& control_element); const blink::WebFormControlElement& control_element);
// Returns whether the element is a username or password textfield. // Sends a reputation check request in case if |element| has type password and
bool IsUsernameOrPasswordField(const blink::WebInputElement& element); // no check request were sent from this frame load.
void MaybeCheckSafeBrowsingReputation(const blink::WebInputElement& element);
// Asks the agent to show the touch to fill UI for |control_element|. Returns // Asks the agent to show the touch to fill UI for |control_element|. Returns
// whether the agent was able to do so. // whether the agent was able to do so.
......
...@@ -33,83 +33,6 @@ namespace autofill { ...@@ -33,83 +33,6 @@ namespace autofill {
namespace { namespace {
constexpr char kAutocompleteUsername[] = "username";
constexpr char kAutocompleteCurrentPassword[] = "current-password";
constexpr char kAutocompleteNewPassword[] = "new-password";
constexpr char kAutocompleteCreditCardPrefix[] = "cc-";
// Parses the string with the value of an autocomplete attribute. If any of the
// tokens "username", "current-password" or "new-password" are present, returns
// an appropriate enum value, picking an arbitrary one if more are applicable.
// Otherwise, it returns CREDIT_CARD if a token with a "cc-" prefix is found.
// Otherwise, returns NONE.
AutocompleteFlag ExtractAutocompleteFlag(const std::string& attribute) {
std::vector<base::StringPiece> tokens =
base::SplitStringPiece(attribute, base::kWhitespaceASCII,
base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
bool cc_seen = false;
for (base::StringPiece token : tokens) {
if (base::LowerCaseEqualsASCII(token, kAutocompleteUsername))
return AutocompleteFlag::USERNAME;
if (base::LowerCaseEqualsASCII(token, kAutocompleteCurrentPassword))
return AutocompleteFlag::CURRENT_PASSWORD;
if (base::LowerCaseEqualsASCII(token, kAutocompleteNewPassword))
return AutocompleteFlag::NEW_PASSWORD;
if (!cc_seen) {
cc_seen = base::StartsWith(token, kAutocompleteCreditCardPrefix,
base::CompareCase::SENSITIVE);
}
}
return cc_seen ? AutocompleteFlag::CREDIT_CARD : AutocompleteFlag::NONE;
}
// Helper to spare map::find boilerplate when caching field's autocomplete
// attributes.
class AutocompleteCache {
public:
AutocompleteCache();
~AutocompleteCache();
// Computes and stores the AutocompleteFlag for |field| based on its
// autocomplete attribute. Note that this cannot be done on-demand during
// RetrieveFor, because the cache spares space and look-up time by not storing
// AutocompleteFlag::NONE values, hence for all elements without an
// autocomplete attribute, every retrieval would result in a new computation.
void Store(const FormFieldData* field);
// Retrieves the value previously stored for |field|.
AutocompleteFlag RetrieveFor(const FormFieldData* field) const;
private:
std::map<const FormFieldData*, AutocompleteFlag> cache_;
DISALLOW_COPY_AND_ASSIGN(AutocompleteCache);
};
AutocompleteCache::AutocompleteCache() = default;
AutocompleteCache::~AutocompleteCache() = default;
void AutocompleteCache::Store(const FormFieldData* field) {
const AutocompleteFlag flag =
ExtractAutocompleteFlag(field->autocomplete_attribute);
// Only store non-trivial flags. Most of the elements will have the NONE
// value, so spare storage and lookup time by assuming anything not stored in
// |cache_| has the NONE flag.
if (flag != AutocompleteFlag::NONE)
cache_[field] = flag;
}
AutocompleteFlag AutocompleteCache::RetrieveFor(
const FormFieldData* field) const {
auto it = cache_.find(field);
if (it == cache_.end())
return AutocompleteFlag::NONE;
return it->second;
}
const char kPasswordSiteUrlRegex[] = const char kPasswordSiteUrlRegex[] =
"passwords(?:-[a-z-]+\\.corp)?\\.google\\.com"; "passwords(?:-[a-z-]+\\.corp)?\\.google\\.com";
...@@ -151,13 +74,6 @@ bool HasGaiaSchemeAndHost(const WebFormElement& form) { ...@@ -151,13 +74,6 @@ bool HasGaiaSchemeAndHost(const WebFormElement& form) {
} // namespace } // namespace
AutocompleteFlag AutocompleteFlagForElement(const WebInputElement& element) {
static const base::NoDestructor<WebString> kAutocomplete(("autocomplete"));
return ExtractAutocompleteFlag(
element.GetAttribute(*kAutocomplete)
.Utf8(WebString::UTF8ConversionMode::kStrictReplacingErrorsWithFFFD));
}
re2::RE2* CreateMatcher(void* instance, const char* pattern) { re2::RE2* CreateMatcher(void* instance, const char* pattern) {
re2::RE2::Options options; re2::RE2::Options options;
options.set_case_sensitive(false); options.set_case_sensitive(false);
......
...@@ -19,7 +19,6 @@ ...@@ -19,7 +19,6 @@
namespace blink { namespace blink {
class WebFormElement; class WebFormElement;
class WebInputElement;
class WebLocalFrame; class WebLocalFrame;
} }
...@@ -30,23 +29,8 @@ class RE2; ...@@ -30,23 +29,8 @@ class RE2;
namespace autofill { namespace autofill {
struct PasswordForm; struct PasswordForm;
class FieldDataManager; class FieldDataManager;
// The susbset of autocomplete flags related to passwords.
enum class AutocompleteFlag {
NONE,
USERNAME,
CURRENT_PASSWORD,
NEW_PASSWORD,
// Represents the whole family of cc-* flags.
CREDIT_CARD
};
// Returns the AutocompleteFlag derived from |element|'s autocomplete attribute.
AutocompleteFlag AutocompleteFlagForElement(
const blink::WebInputElement& element);
// The caller of this function is responsible for deleting the returned object. // The caller of this function is responsible for deleting the returned object.
re2::RE2* CreateMatcher(void* instance, const char* pattern); re2::RE2* CreateMatcher(void* instance, const char* pattern);
......
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