Commit 965bd52b authored by Jan Wilken Dörrie's avatar Jan Wilken Dörrie Committed by Commit Bot

[Passwords] Add found leaks to the Password Store

This change implements PasswordCheckDelegate::OnCredentialDone,
resulting in adding found leaked credentials to the password store
if there is a matching saved password.

Bug: 1047726, 1049193
Change-Id: I5c4b75d2a4cf1b46374d83adfc87da3a1f8eabdb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2087682
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747623}
parent b9b614aa
...@@ -832,6 +832,7 @@ jumbo_static_library("extensions") { ...@@ -832,6 +832,7 @@ jumbo_static_library("extensions") {
"//components/onc", "//components/onc",
"//components/password_manager/core/browser", "//components/password_manager/core/browser",
"//components/password_manager/core/browser:affiliation", "//components/password_manager/core/browser:affiliation",
"//components/password_manager/core/browser/leak_detection",
"//components/payments/core", "//components/payments/core",
"//components/pdf/browser", "//components/pdf/browser",
"//components/performance_manager", "//components/performance_manager",
......
...@@ -10,9 +10,11 @@ ...@@ -10,9 +10,11 @@
#include "base/containers/flat_set.h" #include "base/containers/flat_set.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/time/time.h"
#include "chrome/browser/extensions/api/passwords_private/passwords_private_event_router.h" #include "chrome/browser/extensions/api/passwords_private/passwords_private_event_router.h"
#include "chrome/browser/extensions/api/passwords_private/passwords_private_event_router_factory.h" #include "chrome/browser/extensions/api/passwords_private/passwords_private_event_router_factory.h"
#include "chrome/browser/extensions/api/passwords_private/passwords_private_utils.h" #include "chrome/browser/extensions/api/passwords_private/passwords_private_utils.h"
#include "chrome/browser/password_manager/bulk_leak_check_service_factory.h"
#include "chrome/browser/password_manager/password_store_factory.h" #include "chrome/browser/password_manager/password_store_factory.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/common/extensions/api/passwords_private.h" #include "chrome/common/extensions/api/passwords_private.h"
...@@ -20,7 +22,11 @@ ...@@ -20,7 +22,11 @@
#include "components/autofill/core/common/password_form.h" #include "components/autofill/core/common/password_form.h"
#include "components/keyed_service/core/service_access_type.h" #include "components/keyed_service/core/service_access_type.h"
#include "components/password_manager/core/browser/android_affiliation/affiliation_utils.h" #include "components/password_manager/core/browser/android_affiliation/affiliation_utils.h"
#include "components/password_manager/core/browser/bulk_leak_check_service.h"
#include "components/password_manager/core/browser/compromised_credentials_table.h"
#include "components/password_manager/core/browser/leak_detection/encryption_utils.h"
#include "components/password_manager/core/browser/ui/compromised_credentials_provider.h" #include "components/password_manager/core/browser/ui/compromised_credentials_provider.h"
#include "components/password_manager/core/browser/ui/credential_utils.h"
#include "components/password_manager/core/browser/ui/saved_passwords_presenter.h" #include "components/password_manager/core/browser/ui/saved_passwords_presenter.h"
#include "components/url_formatter/url_formatter.h" #include "components/url_formatter/url_formatter.h"
#include "net/base/escape.h" #include "net/base/escape.h"
...@@ -32,6 +38,8 @@ namespace extensions { ...@@ -32,6 +38,8 @@ namespace extensions {
namespace { namespace {
using autofill::PasswordForm;
using password_manager::CanonicalizeUsername;
using password_manager::CredentialWithPassword; using password_manager::CredentialWithPassword;
using ui::TimeFormat; using ui::TimeFormat;
using CompromisedCredentialsView = using CompromisedCredentialsView =
...@@ -95,6 +103,8 @@ PasswordCheckDelegate::PasswordCheckDelegate(Profile* profile) ...@@ -95,6 +103,8 @@ PasswordCheckDelegate::PasswordCheckDelegate(Profile* profile)
&saved_passwords_presenter_) { &saved_passwords_presenter_) {
observed_compromised_credentials_provider_.Add( observed_compromised_credentials_provider_.Add(
&compromised_credentials_provider_); &compromised_credentials_provider_);
observed_bulk_leak_check_service_.Add(
BulkLeakCheckServiceFactory::GetForProfile(profile_));
// Instructs the presenter and provider to initialize and built their caches. // Instructs the presenter and provider to initialize and built their caches.
// This will soon after invoke OnCompromisedCredentialsChanged(), which then // This will soon after invoke OnCompromisedCredentialsChanged(), which then
...@@ -132,7 +142,7 @@ PasswordCheckDelegate::GetCompromisedCredentialsInfo() { ...@@ -132,7 +142,7 @@ PasswordCheckDelegate::GetCompromisedCredentialsInfo() {
// |formatted_orgin| and |change_password_url| need special handling for // |formatted_orgin| and |change_password_url| need special handling for
// Android. Here we use affiliation information instead of the // Android. Here we use affiliation information instead of the
// signon_realm. // signon_realm.
const autofill::PasswordForm& android_form = const PasswordForm& android_form =
credentials_to_forms_.at(credential).at(0); credentials_to_forms_.at(credential).at(0);
api_credential.formatted_origin = android_form.app_display_name; api_credential.formatted_origin = android_form.app_display_name;
api_credential.change_password_url = api_credential.change_password_url =
...@@ -233,7 +243,7 @@ bool PasswordCheckDelegate::RemoveCompromisedCredential( ...@@ -233,7 +243,7 @@ bool PasswordCheckDelegate::RemoveCompromisedCredential(
// Erase all matching credentials from the store. Return whether any // Erase all matching credentials from the store. Return whether any
// credentials were deleted. // credentials were deleted.
SavedPasswordsView saved_passwords = it->second; SavedPasswordsView saved_passwords = it->second;
for (const autofill::PasswordForm& saved_password : saved_passwords) for (const PasswordForm& saved_password : saved_passwords)
password_store_->RemoveLogin(saved_password); password_store_->RemoveLogin(saved_password);
return !saved_passwords.empty(); return !saved_passwords.empty();
...@@ -250,6 +260,35 @@ void PasswordCheckDelegate::OnCompromisedCredentialsChanged( ...@@ -250,6 +260,35 @@ void PasswordCheckDelegate::OnCompromisedCredentialsChanged(
} }
} }
void PasswordCheckDelegate::OnStateChanged(
password_manager::BulkLeakCheckService::State state) {
NOTIMPLEMENTED();
// TODO(https://crbug.com/1047726): Implement.
}
void PasswordCheckDelegate::OnCredentialDone(
const password_manager::LeakCheckCredential& credential,
password_manager::IsLeaked is_leaked) {
if (!is_leaked)
return;
const base::string16 canocalized_username =
CanonicalizeUsername(credential.username());
for (const PasswordForm& saved_password :
saved_passwords_presenter_.GetSavedPasswords()) {
if (saved_password.password_value == credential.password() &&
CanonicalizeUsername(saved_password.username_value) ==
canocalized_username) {
password_store_->AddCompromisedCredentials({
.signon_realm = saved_password.signon_realm,
.username = saved_password.username_value,
.create_time = base::Time::Now(),
.compromise_type = password_manager::CompromiseType::kLeaked,
});
}
}
}
const CredentialWithPassword* const CredentialWithPassword*
PasswordCheckDelegate::FindMatchingCompromisedCredential( PasswordCheckDelegate::FindMatchingCompromisedCredential(
const api::passwords_private::CompromisedCredential& credential) const { const api::passwords_private::CompromisedCredential& credential) const {
......
...@@ -9,6 +9,9 @@ ...@@ -9,6 +9,9 @@
#include "base/scoped_observer.h" #include "base/scoped_observer.h"
#include "chrome/browser/extensions/api/passwords_private/passwords_private_utils.h" #include "chrome/browser/extensions/api/passwords_private/passwords_private_utils.h"
#include "chrome/common/extensions/api/passwords_private.h" #include "chrome/common/extensions/api/passwords_private.h"
#include "components/password_manager/core/browser/bulk_leak_check_service.h"
#include "components/password_manager/core/browser/leak_detection/bulk_leak_check.h"
#include "components/password_manager/core/browser/leak_detection/leak_detection_delegate_interface.h"
#include "components/password_manager/core/browser/password_store.h" #include "components/password_manager/core/browser/password_store.h"
#include "components/password_manager/core/browser/ui/compromised_credentials_provider.h" #include "components/password_manager/core/browser/ui/compromised_credentials_provider.h"
#include "components/password_manager/core/browser/ui/credential_utils.h" #include "components/password_manager/core/browser/ui/credential_utils.h"
...@@ -25,7 +28,8 @@ namespace extensions { ...@@ -25,7 +28,8 @@ namespace extensions {
// This class handles the part of the passwordsPrivate extension API that deals // This class handles the part of the passwordsPrivate extension API that deals
// with the bulk password check feature. // with the bulk password check feature.
class PasswordCheckDelegate class PasswordCheckDelegate
: public password_manager::CompromisedCredentialsProvider::Observer { : public password_manager::CompromisedCredentialsProvider::Observer,
public password_manager::BulkLeakCheckService::Observer {
public: public:
using CredentialPasswordsMap = using CredentialPasswordsMap =
std::map<password_manager::CredentialWithPassword, std::map<password_manager::CredentialWithPassword,
...@@ -69,6 +73,12 @@ class PasswordCheckDelegate ...@@ -69,6 +73,12 @@ class PasswordCheckDelegate
password_manager::CompromisedCredentialsProvider::CredentialsView password_manager::CompromisedCredentialsProvider::CredentialsView
credentials) override; credentials) override;
// password_manager::BulkLeakCheckService::Observer:
void OnStateChanged(
password_manager::BulkLeakCheckService::State state) override;
void OnCredentialDone(const password_manager::LeakCheckCredential& credential,
password_manager::IsLeaked is_leaked) override;
const password_manager::CredentialWithPassword* const password_manager::CredentialWithPassword*
FindMatchingCompromisedCredential( FindMatchingCompromisedCredential(
const api::passwords_private::CompromisedCredential& credential) const; const api::passwords_private::CompromisedCredential& credential) const;
...@@ -93,6 +103,11 @@ class PasswordCheckDelegate ...@@ -93,6 +103,11 @@ class PasswordCheckDelegate
password_manager::CompromisedCredentialsProvider::Observer> password_manager::CompromisedCredentialsProvider::Observer>
observed_compromised_credentials_provider_{this}; observed_compromised_credentials_provider_{this};
// A scoped observer for the BulkLeakCheckService.
ScopedObserver<password_manager::BulkLeakCheckService,
password_manager::BulkLeakCheckService::Observer>
observed_bulk_leak_check_service_{this};
// A map that matches CredentialWithPasswords to corresponding PasswordForms. // A map that matches CredentialWithPasswords to corresponding PasswordForms.
// This is required to inject affiliation information into Android // This is required to inject affiliation information into Android
// credentials, as well as being able to reflect edits and removals of // credentials, as well as being able to reflect edits and removals of
......
...@@ -14,23 +14,33 @@ ...@@ -14,23 +14,33 @@
#include "base/stl_util.h" #include "base/stl_util.h"
#include "base/strings/strcat.h" #include "base/strings/strcat.h"
#include "base/strings/string_piece.h" #include "base/strings/string_piece.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/bind_test_util.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "chrome/browser/extensions/api/passwords_private/passwords_private_event_router.h" #include "chrome/browser/extensions/api/passwords_private/passwords_private_event_router.h"
#include "chrome/browser/extensions/api/passwords_private/passwords_private_event_router_factory.h" #include "chrome/browser/extensions/api/passwords_private/passwords_private_event_router_factory.h"
#include "chrome/browser/password_manager/bulk_leak_check_service_factory.h"
#include "chrome/browser/password_manager/password_store_factory.h" #include "chrome/browser/password_manager/password_store_factory.h"
#include "chrome/common/extensions/api/passwords_private.h" #include "chrome/common/extensions/api/passwords_private.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
#include "components/keyed_service/core/keyed_service.h" #include "components/keyed_service/core/keyed_service.h"
#include "components/password_manager/core/browser/bulk_leak_check_service.h"
#include "components/password_manager/core/browser/compromised_credentials_table.h" #include "components/password_manager/core/browser/compromised_credentials_table.h"
#include "components/password_manager/core/browser/leak_detection/bulk_leak_check.h"
#include "components/password_manager/core/browser/leak_detection/leak_detection_delegate_interface.h"
#include "components/password_manager/core/browser/password_manager_test_utils.h" #include "components/password_manager/core/browser/password_manager_test_utils.h"
#include "components/password_manager/core/browser/test_password_store.h" #include "components/password_manager/core/browser/test_password_store.h"
#include "components/password_manager/core/common/password_manager_features.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "components/signin/public/identity_manager/identity_test_environment.h"
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
#include "content/public/test/browser_task_environment.h" #include "content/public/test/browser_task_environment.h"
#include "extensions/browser/event_router.h" #include "extensions/browser/event_router.h"
#include "extensions/browser/extension_event_histogram_value.h" #include "extensions/browser/extension_event_histogram_value.h"
#include "extensions/browser/test_event_router.h" #include "extensions/browser/test_event_router.h"
#include "extensions/browser/test_event_router_observer.h" #include "extensions/browser/test_event_router_observer.h"
#include "services/network/test/test_shared_url_loader_factory.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -51,13 +61,20 @@ constexpr char kPassword2[] = "f00b4r"; ...@@ -51,13 +61,20 @@ constexpr char kPassword2[] = "f00b4r";
using api::passwords_private::CompromisedCredential; using api::passwords_private::CompromisedCredential;
using api::passwords_private::CompromisedCredentialsInfo; using api::passwords_private::CompromisedCredentialsInfo;
using autofill::PasswordForm; using autofill::PasswordForm;
using password_manager::BulkLeakCheckDelegateInterface;
using password_manager::BulkLeakCheckService;
using password_manager::CompromisedCredentials;
using password_manager::CompromiseType; using password_manager::CompromiseType;
using password_manager::IsLeaked;
using password_manager::LeakCheckCredential;
using password_manager::TestPasswordStore; using password_manager::TestPasswordStore;
using ::testing::AllOf; using ::testing::AllOf;
using ::testing::AtLeast; using ::testing::AtLeast;
using ::testing::ElementsAre; using ::testing::ElementsAre;
using ::testing::Field; using ::testing::Field;
using ::testing::IsEmpty;
using ::testing::Pointee; using ::testing::Pointee;
using ::testing::UnorderedElementsAre;
PasswordsPrivateEventRouter* CreateAndUsePasswordsPrivateEventRouter( PasswordsPrivateEventRouter* CreateAndUsePasswordsPrivateEventRouter(
Profile* profile) { Profile* profile) {
...@@ -93,7 +110,21 @@ scoped_refptr<TestPasswordStore> CreateAndUseTestPasswordStore( ...@@ -93,7 +110,21 @@ scoped_refptr<TestPasswordStore> CreateAndUseTestPasswordStore(
.get())); .get()));
} }
password_manager::CompromisedCredentials MakeCompromised( BulkLeakCheckService* CreateAndUseBulkLeakCheckService(
signin::IdentityManager* identity_manager,
Profile* profile) {
return static_cast<BulkLeakCheckService*>(
BulkLeakCheckServiceFactory::GetInstance()->SetTestingFactoryAndUse(
profile, base::BindLambdaForTesting([identity_manager](
content::BrowserContext*) {
return std::unique_ptr<
KeyedService>(std::make_unique<BulkLeakCheckService>(
identity_manager,
base::MakeRefCounted<network::TestSharedURLLoaderFactory>()));
})));
}
CompromisedCredentials MakeCompromised(
base::StringPiece signon_realm, base::StringPiece signon_realm,
base::StringPiece username, base::StringPiece username,
base::TimeDelta time_since_creation = base::TimeDelta(), base::TimeDelta time_since_creation = base::TimeDelta(),
...@@ -108,7 +139,7 @@ password_manager::CompromisedCredentials MakeCompromised( ...@@ -108,7 +139,7 @@ password_manager::CompromisedCredentials MakeCompromised(
PasswordForm MakeSavedPassword(base::StringPiece signon_realm, PasswordForm MakeSavedPassword(base::StringPiece signon_realm,
base::StringPiece username, base::StringPiece username,
base::StringPiece password = "", base::StringPiece password = kPassword1,
base::StringPiece username_element = "") { base::StringPiece username_element = "") {
PasswordForm form; PasswordForm form;
form.signon_realm = std::string(signon_realm); form.signon_realm = std::string(signon_realm);
...@@ -159,15 +190,21 @@ class PasswordCheckDelegateTest : public ::testing::Test { ...@@ -159,15 +190,21 @@ class PasswordCheckDelegateTest : public ::testing::Test {
return event_router_observer_; return event_router_observer_;
} }
TestPasswordStore& store() { return *store_; } TestPasswordStore& store() { return *store_; }
BulkLeakCheckService* service() { return bulk_leak_check_service_; }
PasswordCheckDelegate& delegate() { return delegate_; } PasswordCheckDelegate& delegate() { return delegate_; }
private: private:
content::BrowserTaskEnvironment task_env_; content::BrowserTaskEnvironment task_env_{
base::test::TaskEnvironment::TimeSource::MOCK_TIME};
signin::IdentityTestEnvironment identity_test_env_;
TestingProfile profile_; TestingProfile profile_;
EventRouter* event_router_ = CreateAndUseEventRouter(&profile_); EventRouter* event_router_ = CreateAndUseEventRouter(&profile_);
PasswordsPrivateEventRouter* password_router_ = PasswordsPrivateEventRouter* password_router_ =
CreateAndUsePasswordsPrivateEventRouter(&profile_); CreateAndUsePasswordsPrivateEventRouter(&profile_);
TestEventRouterObserver event_router_observer_{event_router_}; TestEventRouterObserver event_router_observer_{event_router_};
BulkLeakCheckService* bulk_leak_check_service_ =
CreateAndUseBulkLeakCheckService(identity_test_env_.identity_manager(),
&profile_);
scoped_refptr<TestPasswordStore> store_ = scoped_refptr<TestPasswordStore> store_ =
CreateAndUseTestPasswordStore(&profile_); CreateAndUseTestPasswordStore(&profile_);
PasswordCheckDelegate delegate_{&profile_}; PasswordCheckDelegate delegate_{&profile_};
...@@ -471,4 +508,102 @@ TEST_F(PasswordCheckDelegateTest, RemoveCompromisedCredentialSuccess) { ...@@ -471,4 +508,102 @@ TEST_F(PasswordCheckDelegateTest, RemoveCompromisedCredentialSuccess) {
EXPECT_FALSE(delegate().RemoveCompromisedCredential(credential)); EXPECT_FALSE(delegate().RemoveCompromisedCredential(credential));
} }
// Tests that we don't create an entry in the database if there is no matching
// saved password.
TEST_F(PasswordCheckDelegateTest, OnLeakFoundDoesNotCreateCredential) {
static_cast<BulkLeakCheckDelegateInterface*>(service())->OnFinishedCredential(
LeakCheckCredential(base::ASCIIToUTF16(kUsername1),
base::ASCIIToUTF16(kPassword1)),
IsLeaked(true));
RunUntilIdle();
EXPECT_THAT(store().compromised_credentials(), IsEmpty());
}
// Test that we don't create an entry in the password store if IsLeaked is
// false.
TEST_F(PasswordCheckDelegateTest, NoLeakedFound) {
store().AddLogin(MakeSavedPassword(kExampleCom, kUsername1, kPassword1));
RunUntilIdle();
static_cast<BulkLeakCheckDelegateInterface*>(service())->OnFinishedCredential(
LeakCheckCredential(base::ASCIIToUTF16(kUsername1),
base::ASCIIToUTF16(kPassword1)),
IsLeaked(false));
RunUntilIdle();
EXPECT_THAT(store().compromised_credentials(), IsEmpty());
}
// Test that a found leak creates a compromised credential in the password
// store.
TEST_F(PasswordCheckDelegateTest, OnLeakFoundCreatesCredential) {
store().AddLogin(MakeSavedPassword(kExampleCom, kUsername1, kPassword1));
RunUntilIdle();
static_cast<BulkLeakCheckDelegateInterface*>(service())->OnFinishedCredential(
LeakCheckCredential(base::ASCIIToUTF16(kUsername1),
base::ASCIIToUTF16(kPassword1)),
IsLeaked(true));
RunUntilIdle();
EXPECT_THAT(store().compromised_credentials(),
ElementsAre(CompromisedCredentials{
.signon_realm = kExampleCom,
.username = base::ASCIIToUTF16(kUsername1),
.create_time = base::Time::Now(),
.compromise_type = CompromiseType::kLeaked,
}));
}
// Test that a found leak creates a compromised credential in the password
// store for each combination of the same canonicalized username and password.
TEST_F(PasswordCheckDelegateTest, OnLeakFoundCreatesMultipleCredential) {
const std::string kUsername2Upper = base::ToUpperASCII(kUsername2);
const std::string kUsername2Email = base::StrCat({kUsername2, "@email.com"});
store().AddLogin(MakeSavedPassword(kExampleCom, kUsername1, kPassword1));
store().AddLogin(MakeSavedPassword(kExampleOrg, kUsername1, kPassword1));
store().AddLogin(MakeSavedPassword(kExampleCom, kUsername2Upper, kPassword2));
store().AddLogin(MakeSavedPassword(kExampleOrg, kUsername2Email, kPassword2));
RunUntilIdle();
static_cast<BulkLeakCheckDelegateInterface*>(service())->OnFinishedCredential(
LeakCheckCredential(base::ASCIIToUTF16(kUsername1),
base::ASCIIToUTF16(kPassword1)),
IsLeaked(true));
static_cast<BulkLeakCheckDelegateInterface*>(service())->OnFinishedCredential(
LeakCheckCredential(base::ASCIIToUTF16(kUsername2Email),
base::ASCIIToUTF16(kPassword2)),
IsLeaked(true));
RunUntilIdle();
EXPECT_THAT(store().compromised_credentials(),
UnorderedElementsAre(
CompromisedCredentials{
.signon_realm = kExampleCom,
.username = base::ASCIIToUTF16(kUsername1),
.create_time = base::Time::Now(),
.compromise_type = CompromiseType::kLeaked,
},
CompromisedCredentials{
.signon_realm = kExampleOrg,
.username = base::ASCIIToUTF16(kUsername1),
.create_time = base::Time::Now(),
.compromise_type = CompromiseType::kLeaked,
},
CompromisedCredentials{
.signon_realm = kExampleCom,
.username = base::ASCIIToUTF16(kUsername2Upper),
.create_time = base::Time::Now(),
.compromise_type = CompromiseType::kLeaked,
},
CompromisedCredentials{
.signon_realm = kExampleOrg,
.username = base::ASCIIToUTF16(kUsername2Email),
.create_time = base::Time::Now(),
.compromise_type = CompromiseType::kLeaked,
}));
}
} // namespace extensions } // namespace extensions
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