Skip to content

Commit

Permalink
Replacing CompromisedCredentials with InsecureScredential
Browse files Browse the repository at this point in the history 
This is the last CL in a sequence to rename CompromisedCredentials into
InsecureCredential.

Bug: 1175743
Change-Id: I0e4eab286e62a5d860dca5f833ea4ebffea910f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2699888
Commit-Queue: Viktor Semeniuk <vsemeniuk@google.com>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Sean Harrison <harrisonsean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#858534}
  • Loading branch information
Viktor Semeniuk authored and Chromium LUCI CQ committed Mar 1, 2021
1 parent 950e08e commit 0b58d9c
Show file tree
Hide file tree
Showing 17 changed files with 250 additions and 251 deletions.
179 changes: 95 additions & 84 deletions chrome/browser/extensions/api/passwords_private/password_check_delegate_unittest.cc
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc
View file
Expand Up @@ -609,7 +609,7 @@ TEST_F(PasswordsPrivateDelegateImplTest, TestReauthOnGetPlaintextCompPassword) {
PasswordsPrivateDelegateImpl delegate(&profile_);

password_manager::PasswordForm form = CreateSampleForm();
password_manager::CompromisedCredentials compromised_credentials;
password_manager::InsecureCredential compromised_credentials;
compromised_credentials.signon_realm = form.signon_realm;
compromised_credentials.username = form.username_value;
store_->AddLogin(form);
Expand Down
36 changes: 21 additions & 15 deletions chrome/browser/password_check/android/password_check_manager_unittest.cc
View file
Expand Up @@ -38,7 +38,7 @@
#include "testing/gtest/include/gtest/gtest.h"

using password_manager::BulkLeakCheckService;
using password_manager::CompromisedCredentials;
using password_manager::InsecureCredential;
using password_manager::InsecureCredentialTypeFlags;
using password_manager::InsecureType;
using password_manager::PasswordCheckUIStatus;
Expand Down Expand Up @@ -163,15 +163,15 @@ PasswordForm MakeSavedAndroidPassword(
return form;
}

CompromisedCredentials MakeCompromised(
InsecureCredential MakeInsecureCredential(
base::StringPiece signon_realm,
base::StringPiece username,
base::TimeDelta time_since_creation = base::TimeDelta(),
InsecureType compromise_type = InsecureType::kLeaked) {
return CompromisedCredentials(
std::string(signon_realm), base::ASCIIToUTF16(username),
base::Time::Now() - time_since_creation, compromise_type,
password_manager::IsMuted(false));
return InsecureCredential(std::string(signon_realm),
base::ASCIIToUTF16(username),
base::Time::Now() - time_since_creation,
compromise_type, password_manager::IsMuted(false));
}

// Creates matcher for a given compromised credential
Expand Down Expand Up @@ -275,7 +275,8 @@ TEST_F(PasswordCheckManagerTest, OnCompromisedCredentialsChanged) {
RunUntilIdle();

EXPECT_CALL(mock_observer(), OnCompromisedCredentialsChanged(1));
store().AddInsecureCredential(MakeCompromised(kExampleCom, kUsername1));
store().AddInsecureCredential(
MakeInsecureCredential(kExampleCom, kUsername1));
RunUntilIdle();
}

Expand Down Expand Up @@ -341,7 +342,8 @@ TEST_F(PasswordCheckManagerTest,
TEST_F(PasswordCheckManagerTest, CorrectlyCreatesUIStructForSiteCredential) {
InitializeManager();
store().AddLogin(MakeSavedPassword(kExampleCom, kUsername1));
store().AddInsecureCredential(MakeCompromised(kExampleCom, kUsername1));
store().AddInsecureCredential(
MakeInsecureCredential(kExampleCom, kUsername1));
RunUntilIdle();
EXPECT_THAT(
manager().GetCompromisedCredentials(),
Expand All @@ -362,9 +364,9 @@ TEST_F(PasswordCheckManagerTest, CorrectlyCreatesUIStructForAppCredentials) {
store().AddLogin(MakeSavedAndroidPassword(kExampleApp, kUsername2,
"Example App", kExampleCom));
store().AddInsecureCredential(
MakeCompromised(MakeAndroidRealm(kExampleApp), kUsername1));
MakeInsecureCredential(MakeAndroidRealm(kExampleApp), kUsername1));
store().AddInsecureCredential(
MakeCompromised(MakeAndroidRealm(kExampleApp), kUsername2));
MakeInsecureCredential(MakeAndroidRealm(kExampleApp), kUsername2));

RunUntilIdle();

Expand Down Expand Up @@ -424,7 +426,8 @@ TEST_F(PasswordCheckManagerTest,
password_manager::features::kPasswordChangeInSettings},
{});
store().AddLogin(MakeSavedPassword(kExampleCom, kUsername1));
store().AddInsecureCredential(MakeCompromised(kExampleCom, kUsername1));
store().AddInsecureCredential(
MakeInsecureCredential(kExampleCom, kUsername1));

RunUntilIdle();
// To have precise metrics, scripts are not requested for users who cannot
Expand Down Expand Up @@ -455,7 +458,8 @@ TEST_F(PasswordCheckManagerTest,
password_manager::features::kPasswordChangeInSettings},
{});
store().AddLogin(MakeSavedPassword(kExampleCom, kUsername1));
store().AddInsecureCredential(MakeCompromised(kExampleCom, kUsername1));
store().AddInsecureCredential(
MakeInsecureCredential(kExampleCom, kUsername1));

RunUntilIdle();
EXPECT_CALL(fetcher(), RefreshScriptsIfNecessary)
Expand Down Expand Up @@ -486,7 +490,7 @@ TEST_F(PasswordCheckManagerTest,
password_manager::features::kPasswordChangeInSettings},
{});
store().AddLogin(MakeSavedPassword(kExampleCom, ""));
store().AddInsecureCredential(MakeCompromised(kExampleCom, ""));
store().AddInsecureCredential(MakeInsecureCredential(kExampleCom, ""));

RunUntilIdle();
EXPECT_CALL(fetcher(), RefreshScriptsIfNecessary)
Expand Down Expand Up @@ -520,7 +524,8 @@ TEST_F(PasswordCheckManagerTest,
/*disabled_features=*/{
password_manager::features::kPasswordChangeInSettings});
store().AddLogin(MakeSavedPassword(kExampleCom, kUsername1));
store().AddInsecureCredential(MakeCompromised(kExampleCom, kUsername1));
store().AddInsecureCredential(
MakeInsecureCredential(kExampleCom, kUsername1));

RunUntilIdle();
EXPECT_CALL(fetcher(), RefreshScriptsIfNecessary)
Expand Down Expand Up @@ -553,7 +558,8 @@ TEST_F(PasswordCheckManagerTest,
password_manager::features::kPasswordChangeInSettings},
{});
store().AddLogin(MakeSavedPassword(kExampleCom, kUsername1));
store().AddInsecureCredential(MakeCompromised(kExampleCom, kUsername1));
store().AddInsecureCredential(
MakeInsecureCredential(kExampleCom, kUsername1));

RunUntilIdle();
EXPECT_CALL(fetcher(), RefreshScriptsIfNecessary)
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/webui/settings/safety_check_handler_unittest.cc
View file
Expand Up @@ -176,7 +176,7 @@ class TestPasswordsDelegate : public extensions::TestPasswordsPrivateDelegate {
store_->AddLogin(form);
base::RunLoop().RunUntilIdle();

store_->AddInsecureCredential(password_manager::CompromisedCredentials(
store_->AddInsecureCredential(password_manager::InsecureCredential(
form.signon_realm, form.username_value, base::Time(),
password_manager::InsecureType::kLeaked,
password_manager::IsMuted(false)));
Expand Down
58 changes: 27 additions & 31 deletions components/password_manager/core/browser/insecure_credentials_table.cc
View file
Expand Up @@ -15,9 +15,9 @@ namespace password_manager {
namespace {

// Returns a compromised credentials vector from the SQL statement.
std::vector<CompromisedCredentials> StatementToCompromisedCredentials(
std::vector<InsecureCredential> StatementToInsecureCredential(
sql::Statement* s) {
std::vector<CompromisedCredentials> results;
std::vector<InsecureCredential> results;
while (s->Step()) {
int parent_key = s->ColumnInt64(0);
std::string signon_realm = s->ColumnString(1);
Expand All @@ -26,9 +26,8 @@ std::vector<CompromisedCredentials> StatementToCompromisedCredentials(
base::Time create_time = base::Time::FromDeltaSinceWindowsEpoch(
(base::TimeDelta::FromMicroseconds(s->ColumnInt64(4))));
bool is_muted = !!s->ColumnInt64(5);
CompromisedCredentials issue(std::move(signon_realm), std::move(username),
create_time, insecurity_type,
IsMuted(is_muted));
InsecureCredential issue(std::move(signon_realm), std::move(username),
create_time, insecurity_type, IsMuted(is_muted));
issue.parent_key = FormPrimaryKey(parent_key);
results.emplace_back(std::move(issue));
}
Expand All @@ -37,35 +36,32 @@ std::vector<CompromisedCredentials> StatementToCompromisedCredentials(

} // namespace

CompromisedCredentials::CompromisedCredentials() = default;
InsecureCredential::InsecureCredential() = default;

CompromisedCredentials::CompromisedCredentials(std::string signon_realm,
base::string16 username,
base::Time create_time,
InsecureType insecurity_type,
IsMuted is_muted)
InsecureCredential::InsecureCredential(std::string signon_realm,
base::string16 username,
base::Time create_time,
InsecureType insecurity_type,
IsMuted is_muted)
: signon_realm(std::move(signon_realm)),
username(std::move(username)),
create_time(create_time),
insecure_type(insecurity_type),
is_muted(is_muted) {}

CompromisedCredentials::CompromisedCredentials(
const CompromisedCredentials& rhs) = default;
InsecureCredential::InsecureCredential(const InsecureCredential& rhs) = default;

CompromisedCredentials::CompromisedCredentials(CompromisedCredentials&& rhs) =
default;
InsecureCredential::InsecureCredential(InsecureCredential&& rhs) = default;

CompromisedCredentials& CompromisedCredentials::operator=(
const CompromisedCredentials& rhs) = default;
InsecureCredential& InsecureCredential::operator=(
const InsecureCredential& rhs) = default;

CompromisedCredentials& CompromisedCredentials::operator=(
CompromisedCredentials&& rhs) = default;
InsecureCredential& InsecureCredential::operator=(InsecureCredential&& rhs) =
default;

CompromisedCredentials::~CompromisedCredentials() = default;
InsecureCredential::~InsecureCredential() = default;

bool operator==(const CompromisedCredentials& lhs,
const CompromisedCredentials& rhs) {
bool operator==(const InsecureCredential& lhs, const InsecureCredential& rhs) {
return lhs.signon_realm == rhs.signon_realm && lhs.username == rhs.username &&
lhs.create_time == rhs.create_time &&
lhs.insecure_type == rhs.insecure_type &&
Expand All @@ -79,7 +75,7 @@ void InsecureCredentialsTable::Init(sql::Database* db) {
}

bool InsecureCredentialsTable::AddRow(
const CompromisedCredentials& compromised_credentials) {
const InsecureCredential& compromised_credentials) {
DCHECK(db_);
if (compromised_credentials.signon_realm.empty())
return false;
Expand Down Expand Up @@ -130,7 +126,7 @@ bool InsecureCredentialsTable::RemoveRow(
DCHECK(db_->DoesTableExist(kTableName));

// Retrieve the rows that are to be removed to log.
const std::vector<CompromisedCredentials> compromised_credentials =
const std::vector<InsecureCredential> compromised_credentials =
GetRows(signon_realm);
if (compromised_credentials.empty())
return false;
Expand All @@ -156,11 +152,11 @@ bool InsecureCredentialsTable::RemoveRow(
return s.Run();
}

std::vector<CompromisedCredentials> InsecureCredentialsTable::GetRows(
std::vector<InsecureCredential> InsecureCredentialsTable::GetRows(
const std::string& signon_realm) const {
DCHECK(db_);
if (signon_realm.empty())
return std::vector<CompromisedCredentials>{};
return std::vector<InsecureCredential>{};

DCHECK(db_->DoesTableExist(kTableName));

Expand All @@ -173,10 +169,10 @@ std::vector<CompromisedCredentials> InsecureCredentialsTable::GetRows(
kTableName)
.c_str()));
s.BindString(0, signon_realm);
return StatementToCompromisedCredentials(&s);
return StatementToInsecureCredential(&s);
}

std::vector<CompromisedCredentials> InsecureCredentialsTable::GetRows(
std::vector<InsecureCredential> InsecureCredentialsTable::GetRows(
FormPrimaryKey parent_key) const {
DCHECK(db_);
DCHECK(db_->DoesTableExist(kTableName));
Expand All @@ -190,10 +186,10 @@ std::vector<CompromisedCredentials> InsecureCredentialsTable::GetRows(
kTableName)
.c_str()));
s.BindInt(0, *parent_key);
return StatementToCompromisedCredentials(&s);
return StatementToInsecureCredential(&s);
}

std::vector<CompromisedCredentials> InsecureCredentialsTable::GetAllRows() {
std::vector<InsecureCredential> InsecureCredentialsTable::GetAllRows() {
DCHECK(db_);
DCHECK(db_->DoesTableExist(kTableName));

Expand All @@ -204,7 +200,7 @@ std::vector<CompromisedCredentials> InsecureCredentialsTable::GetAllRows() {
"INNER JOIN logins ON parent_id = logins.id",
kTableName)
.c_str()));
return StatementToCompromisedCredentials(&s);
return StatementToInsecureCredential(&s);
}

void InsecureCredentialsTable::ReportMetrics(BulkCheckDone bulk_check_done) {
Expand Down
35 changes: 17 additions & 18 deletions components/password_manager/core/browser/insecure_credentials_table.h
View file
Expand Up @@ -49,18 +49,18 @@ enum class RemoveInsecureCredentialsReason {
};

// Represents information about the particular compromised credentials.
struct CompromisedCredentials {
CompromisedCredentials();
CompromisedCredentials(std::string signon_realm,
base::string16 username,
base::Time create_time,
InsecureType insecure_type,
IsMuted is_muted);
CompromisedCredentials(const CompromisedCredentials& rhs);
CompromisedCredentials(CompromisedCredentials&& rhs);
CompromisedCredentials& operator=(const CompromisedCredentials& rhs);
CompromisedCredentials& operator=(CompromisedCredentials&& rhs);
~CompromisedCredentials();
struct InsecureCredential {
InsecureCredential();
InsecureCredential(std::string signon_realm,
base::string16 username,
base::Time create_time,
InsecureType insecure_type,
IsMuted is_muted);
InsecureCredential(const InsecureCredential& rhs);
InsecureCredential(InsecureCredential&& rhs);
InsecureCredential& operator=(const InsecureCredential& rhs);
InsecureCredential& operator=(InsecureCredential&& rhs);
~InsecureCredential();

// The primary key of an affected Login.
FormPrimaryKey parent_key{-1};
Expand All @@ -78,8 +78,7 @@ struct CompromisedCredentials {
PasswordForm::Store in_store = PasswordForm::Store::kNotSet;
};

bool operator==(const CompromisedCredentials& lhs,
const CompromisedCredentials& rhs);
bool operator==(const InsecureCredential& lhs, const InsecureCredential& rhs);

// Represents the 'compromised credentials' table in the Login Database.
class InsecureCredentialsTable {
Expand All @@ -94,7 +93,7 @@ class InsecureCredentialsTable {

// Adds information about the compromised credentials. Returns true
// if the SQL completed successfully and an item was created.
bool AddRow(const CompromisedCredentials& compromised_credentials);
bool AddRow(const InsecureCredential& compromised_credentials);

// Removes information about the credentials compromised for |username| on
// |signon_realm|. |reason| is the reason why the credentials is removed from
Expand All @@ -105,14 +104,14 @@ class InsecureCredentialsTable {
RemoveInsecureCredentialsReason reason);

// Gets all the rows in the database for |signon_realm|.
std::vector<CompromisedCredentials> GetRows(
std::vector<InsecureCredential> GetRows(
const std::string& signon_realm) const;

// Gets all the rows in the database for |parent_key|.
std::vector<CompromisedCredentials> GetRows(FormPrimaryKey parent_key) const;
std::vector<InsecureCredential> GetRows(FormPrimaryKey parent_key) const;

// Returns all compromised credentials from the database.
std::vector<CompromisedCredentials> GetAllRows();
std::vector<InsecureCredential> GetAllRows();

// Reports UMA metrics about the table. |bulk_check_done| means that the
// password bulk leak check was executed in the past.
Expand Down

0 comments on commit 0b58d9c

Please sign in to comment.