Commit c452cff9 authored by Reda Tawfik's avatar Reda Tawfik Committed by Commit Bot

[Android][Mfill] Disable "Use other password" button on unsecured sites

This CL disables "Use other password" button on low level security sites

Bug: 1129026, 1104132
Change-Id: I7bb85d2179933420f8c13b3e1dce7d30d81ffb60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2470658Reviewed-by: default avatarIoana Pandele <ioanap@chromium.org>
Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Commit-Queue: Reda Tawfik <redatawfik@google.com>
Cr-Commit-Position: refs/heads/master@{#822151}
parent 98450034
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "chrome/browser/password_manager/android/password_manager_launcher_android.h" #include "chrome/browser/password_manager/android/password_manager_launcher_android.h"
#include "chrome/browser/password_manager/chrome_password_manager_client.h" #include "chrome/browser/password_manager/chrome_password_manager_client.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ssl/security_state_tab_helper.h"
#include "chrome/browser/ui/passwords/manage_passwords_view_utils.h" #include "chrome/browser/ui/passwords/manage_passwords_view_utils.h"
#include "chrome/browser/vr/vr_tab_helper.h" #include "chrome/browser/vr/vr_tab_helper.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
...@@ -271,10 +272,9 @@ void PasswordAccessoryControllerImpl::RefreshSuggestionsForField( ...@@ -271,10 +272,9 @@ void PasswordAccessoryControllerImpl::RefreshSuggestionsForField(
} }
} }
if (origin.GetURL().SchemeIsCryptographic() && if (IsSecureSite() && origin.GetURL().SchemeIsCryptographic() &&
base::FeatureList::IsEnabled( base::FeatureList::IsEnabled(
password_manager::features::kFillingPasswordsFromAnyOrigin)) { password_manager::features::kFillingPasswordsFromAnyOrigin)) {
// TODO(crbug.com/1104132): Disable the feature in insecure websites.
base::string16 button_title = base::string16 button_title =
is_password_field is_password_field
? l10n_util::GetStringUTF16( ? l10n_util::GetStringUTF16(
...@@ -441,4 +441,16 @@ void PasswordAccessoryControllerImpl::AllPasswordsSheetDismissed() { ...@@ -441,4 +441,16 @@ void PasswordAccessoryControllerImpl::AllPasswordsSheetDismissed() {
all_passords_bottom_sheet_controller_.reset(); all_passords_bottom_sheet_controller_.reset();
} }
bool PasswordAccessoryControllerImpl::IsSecureSite() {
if (security_level_for_testing_) {
return security_level_for_testing_ == security_state::SECURE;
}
SecurityStateTabHelper::CreateForWebContents(web_contents_);
SecurityStateTabHelper* helper =
SecurityStateTabHelper::FromWebContents(web_contents_);
return helper && helper->GetSecurityLevel() == security_state::SECURE;
}
WEB_CONTENTS_USER_DATA_KEY_IMPL(PasswordAccessoryControllerImpl) WEB_CONTENTS_USER_DATA_KEY_IMPL(PasswordAccessoryControllerImpl)
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "components/autofill/core/common/password_generation_util.h" #include "components/autofill/core/common/password_generation_util.h"
#include "components/password_manager/core/browser/credential_cache.h" #include "components/password_manager/core/browser/credential_cache.h"
#include "components/password_manager/core/browser/password_manager_client.h" #include "components/password_manager/core/browser/password_manager_client.h"
#include "components/security_state/core/security_state.h"
#include "content/public/browser/web_contents_user_data.h" #include "content/public/browser/web_contents_user_data.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -79,6 +80,18 @@ class PasswordAccessoryControllerImpl ...@@ -79,6 +80,18 @@ class PasswordAccessoryControllerImpl
password_manager::ContentPasswordManagerDriver* driver, password_manager::ContentPasswordManagerDriver* driver,
autofill::mojom::FocusedFieldType focused_field_type); autofill::mojom::FocusedFieldType focused_field_type);
// Returns true if the current site attached to `web_contents_` has a SECURE
// security level.
bool IsSecureSite();
#if defined(UNIT_TEST)
// Used for testing to set `security_level_for_testing_`.
void SetSecurityLevelForTesting(
security_state::SecurityLevel security_level) {
security_level_for_testing_ = security_level;
}
#endif
private: private:
friend class content::WebContentsUserData<PasswordAccessoryControllerImpl>; friend class content::WebContentsUserData<PasswordAccessoryControllerImpl>;
...@@ -144,6 +157,10 @@ class PasswordAccessoryControllerImpl ...@@ -144,6 +157,10 @@ class PasswordAccessoryControllerImpl
// called with. // called with.
autofill::mojom::FocusedFieldType last_focused_field_type_; autofill::mojom::FocusedFieldType last_focused_field_type_;
// Security level used for testing only.
security_state::SecurityLevel security_level_for_testing_ =
security_state::NONE;
WEB_CONTENTS_USER_DATA_KEY_DECL(); WEB_CONTENTS_USER_DATA_KEY_DECL();
DISALLOW_COPY_AND_ASSIGN(PasswordAccessoryControllerImpl); DISALLOW_COPY_AND_ASSIGN(PasswordAccessoryControllerImpl);
......
...@@ -34,6 +34,7 @@ ...@@ -34,6 +34,7 @@
#include "components/password_manager/core/browser/stub_password_manager_client.h" #include "components/password_manager/core/browser/stub_password_manager_client.h"
#include "components/password_manager/core/browser/stub_password_manager_driver.h" #include "components/password_manager/core/browser/stub_password_manager_driver.h"
#include "components/password_manager/core/common/password_manager_features.h" #include "components/password_manager/core/common/password_manager_features.h"
#include "components/security_state/core/security_state.h"
#include "components/strings/grit/components_strings.h" #include "components/strings/grit/components_strings.h"
#include "content/public/test/web_contents_tester.h" #include "content/public/test/web_contents_tester.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
...@@ -203,7 +204,7 @@ class PasswordAccessoryControllerTest : public ChromeRenderViewHostTestHarness { ...@@ -203,7 +204,7 @@ class PasswordAccessoryControllerTest : public ChromeRenderViewHostTestHarness {
void TearDown() override { mock_password_store_->ShutdownOnUIThread(); } void TearDown() override { mock_password_store_->ShutdownOnUIThread(); }
PasswordAccessoryController* controller() { PasswordAccessoryControllerImpl* controller() {
return PasswordAccessoryControllerImpl::FromWebContents(web_contents()); return PasswordAccessoryControllerImpl::FromWebContents(web_contents());
} }
...@@ -639,6 +640,7 @@ TEST_F(PasswordAccessoryControllerTest, AddsSaveToggleIfIsBlacklisted) { ...@@ -639,6 +640,7 @@ TEST_F(PasswordAccessoryControllerTest, AddsSaveToggleIfIsBlacklisted) {
} }
TEST_F(PasswordAccessoryControllerTest, AddsShowOtherPasswordsIfEnabled) { TEST_F(PasswordAccessoryControllerTest, AddsShowOtherPasswordsIfEnabled) {
controller()->SetSecurityLevelForTesting(security_state::SECURE);
base::test::ScopedFeatureList scoped_feature_list; base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature( scoped_feature_list.InitAndEnableFeature(
password_manager::features::kFillingPasswordsFromAnyOrigin); password_manager::features::kFillingPasswordsFromAnyOrigin);
...@@ -658,6 +660,7 @@ TEST_F(PasswordAccessoryControllerTest, AddsShowOtherPasswordsIfEnabled) { ...@@ -658,6 +660,7 @@ TEST_F(PasswordAccessoryControllerTest, AddsShowOtherPasswordsIfEnabled) {
} }
TEST_F(PasswordAccessoryControllerTest, AddsShowOtherUsername) { TEST_F(PasswordAccessoryControllerTest, AddsShowOtherUsername) {
controller()->SetSecurityLevelForTesting(security_state::SECURE);
base::test::ScopedFeatureList scoped_feature_list; base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature( scoped_feature_list.InitAndEnableFeature(
password_manager::features::kFillingPasswordsFromAnyOrigin); password_manager::features::kFillingPasswordsFromAnyOrigin);
...@@ -677,7 +680,8 @@ TEST_F(PasswordAccessoryControllerTest, AddsShowOtherUsername) { ...@@ -677,7 +680,8 @@ TEST_F(PasswordAccessoryControllerTest, AddsShowOtherUsername) {
} }
TEST_F(PasswordAccessoryControllerTest, TEST_F(PasswordAccessoryControllerTest,
AddsShowOtherPasswordForOnlySecuredSites) { AddsShowOtherPasswordForOnlyCryptographicSchemeSites) {
controller()->SetSecurityLevelForTesting(security_state::SECURE);
// `Setup` method sets the URL to https but http is required for this method. // `Setup` method sets the URL to https but http is required for this method.
NavigateAndCommit(GURL(kExampleHttpSite)); NavigateAndCommit(GURL(kExampleHttpSite));
FocusWebContentsOnMainFrame(); FocusWebContentsOnMainFrame();
...@@ -698,6 +702,7 @@ TEST_F(PasswordAccessoryControllerTest, ...@@ -698,6 +702,7 @@ TEST_F(PasswordAccessoryControllerTest,
} }
TEST_F(PasswordAccessoryControllerTest, HidesShowOtherPasswordsIfDisabled) { TEST_F(PasswordAccessoryControllerTest, HidesShowOtherPasswordsIfDisabled) {
controller()->SetSecurityLevelForTesting(security_state::SECURE);
base::test::ScopedFeatureList scoped_feature_list; base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature( scoped_feature_list.InitAndDisableFeature(
password_manager::features::kFillingPasswordsFromAnyOrigin); password_manager::features::kFillingPasswordsFromAnyOrigin);
...@@ -713,6 +718,24 @@ TEST_F(PasswordAccessoryControllerTest, HidesShowOtherPasswordsIfDisabled) { ...@@ -713,6 +718,24 @@ TEST_F(PasswordAccessoryControllerTest, HidesShowOtherPasswordsIfDisabled) {
/*is_manual_generation_available=*/false); /*is_manual_generation_available=*/false);
} }
TEST_F(PasswordAccessoryControllerTest,
HideShowOtherPasswordForLowSecurityLevelSites) {
controller()->SetSecurityLevelForTesting(security_state::WARNING);
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(
password_manager::features::kFillingPasswordsFromAnyOrigin);
AccessorySheetData::Builder data_builder(AccessoryTabType::PASSWORDS,
passwords_empty_str(kExampleDomain));
data_builder.AppendFooterCommand(manage_passwords_str(),
autofill::AccessoryAction::MANAGE_PASSWORDS);
EXPECT_CALL(mock_manual_filling_controller_,
RefreshSuggestions(std::move(data_builder).Build()));
controller()->RefreshSuggestionsForField(
FocusedFieldType::kFillablePasswordField,
/*is_manual_generation_available=*/false);
}
TEST_F(PasswordAccessoryControllerTest, TEST_F(PasswordAccessoryControllerTest,
NoSaveToggleIfIsBlacklistedAndSavingDisabled) { NoSaveToggleIfIsBlacklistedAndSavingDisabled) {
base::test::ScopedFeatureList scoped_feature_list; base::test::ScopedFeatureList scoped_feature_list;
......
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