Commit 69e13c2a authored by Jan Wilken Dörrie's avatar Jan Wilken Dörrie Committed by Commit Bot

[Passwords] Use a StrongAlias for CredentialPair::IsPublicSuffixMatch

This change changes the type of CredentialPair::is_public_suffix_match
from bool to a strong bool alias and updates callers.

Bug: 945300
Change-Id: Ibea7fda806f07575635e66dce933b0fb218945cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1849389Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704589}
parent 2ad1dbeb
...@@ -23,6 +23,8 @@ using ::testing::Eq; ...@@ -23,6 +23,8 @@ using ::testing::Eq;
using ::testing::ReturnRefOfCopy; using ::testing::ReturnRefOfCopy;
using ::testing::WithArg; using ::testing::WithArg;
using IsPublicSuffixMatch = CredentialPair::IsPublicSuffixMatch;
constexpr char kExampleCom[] = "https://example.com/"; constexpr char kExampleCom[] = "https://example.com/";
struct MockPasswordManagerDriver : password_manager::StubPasswordManagerDriver { struct MockPasswordManagerDriver : password_manager::StubPasswordManagerDriver {
...@@ -69,7 +71,7 @@ class TouchToFillControllerTest : public testing::Test { ...@@ -69,7 +71,7 @@ class TouchToFillControllerTest : public testing::Test {
TEST_F(TouchToFillControllerTest, Show_And_Fill) { TEST_F(TouchToFillControllerTest, Show_And_Fill) {
CredentialPair credentials[] = { CredentialPair credentials[] = {
{base::ASCIIToUTF16("alice"), base::ASCIIToUTF16("p4ssw0rd"), {base::ASCIIToUTF16("alice"), base::ASCIIToUTF16("p4ssw0rd"),
GURL(kExampleCom), /*is_public_suffix_match=*/false}}; GURL(kExampleCom), IsPublicSuffixMatch(false)}};
EXPECT_CALL(view(), Show(Eq(base::ASCIIToUTF16("example.com")), EXPECT_CALL(view(), Show(Eq(base::ASCIIToUTF16("example.com")),
ElementsAreArray(credentials))); ElementsAreArray(credentials)));
...@@ -89,11 +91,9 @@ TEST_F(TouchToFillControllerTest, Show_And_Fill_Android_Credential) { ...@@ -89,11 +91,9 @@ TEST_F(TouchToFillControllerTest, Show_And_Fill_Android_Credential) {
// Test multiple credentials with one of them being an Android credential. // Test multiple credentials with one of them being an Android credential.
CredentialPair credentials[] = { CredentialPair credentials[] = {
{base::ASCIIToUTF16("alice"), base::ASCIIToUTF16("p4ssw0rd"), {base::ASCIIToUTF16("alice"), base::ASCIIToUTF16("p4ssw0rd"),
GURL(kExampleCom), GURL(kExampleCom), IsPublicSuffixMatch(false)},
/*is_public_suffix_match=*/false},
{base::ASCIIToUTF16("bob"), base::ASCIIToUTF16("s3cr3t"), {base::ASCIIToUTF16("bob"), base::ASCIIToUTF16("s3cr3t"),
GURL("android://hash@com.example.my"), GURL("android://hash@com.example.my"), IsPublicSuffixMatch(false)}};
/*is_public_suffix_match=*/false}};
EXPECT_CALL(view(), Show(Eq(base::ASCIIToUTF16("example.com")), EXPECT_CALL(view(), Show(Eq(base::ASCIIToUTF16("example.com")),
ElementsAreArray(credentials))); ElementsAreArray(credentials)));
...@@ -112,7 +112,7 @@ TEST_F(TouchToFillControllerTest, Show_And_Fill_Android_Credential) { ...@@ -112,7 +112,7 @@ TEST_F(TouchToFillControllerTest, Show_And_Fill_Android_Credential) {
TEST_F(TouchToFillControllerTest, Dismiss) { TEST_F(TouchToFillControllerTest, Dismiss) {
CredentialPair credentials[] = { CredentialPair credentials[] = {
{base::ASCIIToUTF16("alice"), base::ASCIIToUTF16("p4ssw0rd"), {base::ASCIIToUTF16("alice"), base::ASCIIToUTF16("p4ssw0rd"),
GURL(kExampleCom), /*is_public_suffix_match=*/false}}; GURL(kExampleCom), IsPublicSuffixMatch(false)}};
EXPECT_CALL(view(), Show(Eq(base::ASCIIToUTF16("example.com")), EXPECT_CALL(view(), Show(Eq(base::ASCIIToUTF16("example.com")),
ElementsAreArray(credentials))); ElementsAreArray(credentials)));
......
...@@ -38,7 +38,8 @@ CredentialPair ConvertJavaCredential(JNIEnv* env, ...@@ -38,7 +38,8 @@ CredentialPair ConvertJavaCredential(JNIEnv* env,
Java_Credential_getPassword(env, credential)), Java_Credential_getPassword(env, credential)),
GURL(ConvertJavaStringToUTF8( GURL(ConvertJavaStringToUTF8(
env, Java_Credential_getOriginUrl(env, credential))), env, Java_Credential_getOriginUrl(env, credential))),
Java_Credential_isPublicSuffixMatch(env, credential)); CredentialPair::IsPublicSuffixMatch(
Java_Credential_isPublicSuffixMatch(env, credential)));
} }
} // namespace } // namespace
...@@ -70,7 +71,7 @@ void TouchToFillViewImpl::Show( ...@@ -70,7 +71,7 @@ void TouchToFillViewImpl::Show(
ConvertUTF16ToJavaString(env, credential.password), ConvertUTF16ToJavaString(env, credential.password),
ConvertUTF16ToJavaString(env, GetDisplayUsername(credential)), ConvertUTF16ToJavaString(env, GetDisplayUsername(credential)),
ConvertUTF8ToJavaString(env, credential.origin_url.spec()), ConvertUTF8ToJavaString(env, credential.origin_url.spec()),
credential.is_public_suffix_match); credential.is_public_suffix_match.value());
} }
Java_TouchToFillBridge_showCredentials( Java_TouchToFillBridge_showCredentials(
......
...@@ -27,8 +27,9 @@ void CredentialCache::SaveCredentialsForOrigin( ...@@ -27,8 +27,9 @@ void CredentialCache::SaveCredentialsForOrigin(
std::vector<CredentialPair> credentials; std::vector<CredentialPair> credentials;
credentials.reserve(best_matches.size()); credentials.reserve(best_matches.size());
for (const PasswordForm* form : best_matches) { for (const PasswordForm* form : best_matches) {
credentials.emplace_back(form->username_value, form->password_value, credentials.emplace_back(
form->origin, form->is_public_suffix_match); form->username_value, form->password_value, form->origin,
CredentialPair::IsPublicSuffixMatch(form->is_public_suffix_match));
} }
// Sort by origin, then username. // Sort by origin, then username.
std::sort(credentials.begin(), credentials.end(), std::sort(credentials.begin(), credentials.end(),
......
...@@ -19,6 +19,8 @@ using autofill::PasswordForm; ...@@ -19,6 +19,8 @@ using autofill::PasswordForm;
using base::ASCIIToUTF16; using base::ASCIIToUTF16;
using url::Origin; using url::Origin;
using IsPublicSuffixMatch = CredentialPair::IsPublicSuffixMatch;
constexpr char kExampleSiteAndroidApp[] = "android://3x4mpl3@com.example.app/"; constexpr char kExampleSiteAndroidApp[] = "android://3x4mpl3@com.example.app/";
constexpr char kExampleSite[] = "https://example.com"; constexpr char kExampleSite[] = "https://example.com";
constexpr char kExampleSite2[] = "https://example.two.com"; constexpr char kExampleSite2[] = "https://example.two.com";
...@@ -62,29 +64,32 @@ TEST_F(CredentialCacheTest, StoresCredentialsSortedByAplhabetAndOrigins) { ...@@ -62,29 +64,32 @@ TEST_F(CredentialCacheTest, StoresCredentialsSortedByAplhabetAndOrigins) {
// Alphabetical entries of the exactly matching https://example.com: // Alphabetical entries of the exactly matching https://example.com:
CredentialPair(ASCIIToUTF16("Adam"), ASCIIToUTF16("Pas83B"), CredentialPair(ASCIIToUTF16("Adam"), ASCIIToUTF16("Pas83B"),
GURL(kExampleSite), /*is_psl_match*/ false), GURL(kExampleSite), IsPublicSuffixMatch(false)),
CredentialPair(ASCIIToUTF16("Berta"), ASCIIToUTF16("30948"), CredentialPair(ASCIIToUTF16("Berta"), ASCIIToUTF16("30948"),
GURL(kExampleSite), /*is_psl_match*/ false), GURL(kExampleSite), IsPublicSuffixMatch(false)),
CredentialPair(ASCIIToUTF16("Carl"), ASCIIToUTF16("P1238C"), CredentialPair(ASCIIToUTF16("Carl"), ASCIIToUTF16("P1238C"),
GURL(kExampleSite), /*is_psl_match*/ false), GURL(kExampleSite), IsPublicSuffixMatch(false)),
CredentialPair(ASCIIToUTF16("Dora"), ASCIIToUTF16("PakudC"), CredentialPair(ASCIIToUTF16("Dora"), ASCIIToUTF16("PakudC"),
GURL(kExampleSite), /*is_psl_match*/ false), GURL(kExampleSite), IsPublicSuffixMatch(false)),
// Entry for PSL-match android://3x4mpl3@com.example.app: // Entry for PSL-match android://3x4mpl3@com.example.app:
CredentialPair(ASCIIToUTF16("Cesar"), ASCIIToUTF16("V3V1V"), CredentialPair(ASCIIToUTF16("Cesar"), ASCIIToUTF16("V3V1V"),
GURL(kExampleSiteAndroidApp), /*is_psl_match*/ false), GURL(kExampleSiteAndroidApp),
IsPublicSuffixMatch(false)),
// Alphabetical entries of PSL-match https://accounts.example.com: // Alphabetical entries of PSL-match https://accounts.example.com:
CredentialPair(ASCIIToUTF16("Elfi"), ASCIIToUTF16("a65ddm"), CredentialPair(ASCIIToUTF16("Elfi"), ASCIIToUTF16("a65ddm"),
GURL(kExampleSiteSubdomain), /*is_psl_match*/ true), GURL(kExampleSiteSubdomain),
IsPublicSuffixMatch(true)),
CredentialPair(ASCIIToUTF16("Greg"), ASCIIToUTF16("5fnd1m"), CredentialPair(ASCIIToUTF16("Greg"), ASCIIToUTF16("5fnd1m"),
GURL(kExampleSiteSubdomain), /*is_psl_match*/ true), GURL(kExampleSiteSubdomain),
IsPublicSuffixMatch(true)),
// Alphabetical entries of PSL-match https://m.example.com: // Alphabetical entries of PSL-match https://m.example.com:
CredentialPair(ASCIIToUTF16("Alf"), ASCIIToUTF16("R4nd50m"), CredentialPair(ASCIIToUTF16("Alf"), ASCIIToUTF16("R4nd50m"),
GURL(kExampleSiteMobile), /*is_psl_match*/ true), GURL(kExampleSiteMobile), IsPublicSuffixMatch(true)),
CredentialPair(ASCIIToUTF16("Rolf"), ASCIIToUTF16("A4nd0m"), CredentialPair(ASCIIToUTF16("Rolf"), ASCIIToUTF16("A4nd0m"),
GURL(kExampleSiteMobile), /*is_psl_match*/ true))); GURL(kExampleSiteMobile), IsPublicSuffixMatch(true))));
} }
TEST_F(CredentialCacheTest, StoredCredentialsForIndependentOrigins) { TEST_F(CredentialCacheTest, StoredCredentialsForIndependentOrigins) {
...@@ -97,13 +102,13 @@ TEST_F(CredentialCacheTest, StoredCredentialsForIndependentOrigins) { ...@@ -97,13 +102,13 @@ TEST_F(CredentialCacheTest, StoredCredentialsForIndependentOrigins) {
{CreateEntry("Abe", "B4dPW", GURL(kExampleSite2), false).first}, origin2); {CreateEntry("Abe", "B4dPW", GURL(kExampleSite2), false).first}, origin2);
EXPECT_THAT(cache()->GetCredentialStore(origin).GetCredentials(), EXPECT_THAT(cache()->GetCredentialStore(origin).GetCredentials(),
testing::ElementsAre( testing::ElementsAre(CredentialPair(
CredentialPair(ASCIIToUTF16("Ben"), ASCIIToUTF16("S3cur3"), ASCIIToUTF16("Ben"), ASCIIToUTF16("S3cur3"),
GURL(kExampleSite), /*is_psl_match*/ false))); GURL(kExampleSite), IsPublicSuffixMatch(false))));
EXPECT_THAT(cache()->GetCredentialStore(origin2).GetCredentials(), EXPECT_THAT(cache()->GetCredentialStore(origin2).GetCredentials(),
testing::ElementsAre( testing::ElementsAre(CredentialPair(
CredentialPair(ASCIIToUTF16("Abe"), ASCIIToUTF16("B4dPW"), ASCIIToUTF16("Abe"), ASCIIToUTF16("B4dPW"),
GURL(kExampleSite2), /*is_psl_match*/ false))); GURL(kExampleSite2), IsPublicSuffixMatch(false))));
} }
TEST_F(CredentialCacheTest, ClearsCredentials) { TEST_F(CredentialCacheTest, ClearsCredentials) {
...@@ -112,9 +117,9 @@ TEST_F(CredentialCacheTest, ClearsCredentials) { ...@@ -112,9 +117,9 @@ TEST_F(CredentialCacheTest, ClearsCredentials) {
{CreateEntry("Ben", "S3cur3", GURL(kExampleSite), false).first}, {CreateEntry("Ben", "S3cur3", GURL(kExampleSite), false).first},
Origin::Create(GURL(kExampleSite))); Origin::Create(GURL(kExampleSite)));
ASSERT_THAT(cache()->GetCredentialStore(origin).GetCredentials(), ASSERT_THAT(cache()->GetCredentialStore(origin).GetCredentials(),
testing::ElementsAre( testing::ElementsAre(CredentialPair(
CredentialPair(ASCIIToUTF16("Ben"), ASCIIToUTF16("S3cur3"), ASCIIToUTF16("Ben"), ASCIIToUTF16("S3cur3"),
GURL(kExampleSite), /*is_psl_match*/ false))); GURL(kExampleSite), IsPublicSuffixMatch(false))));
cache()->ClearCredentials(); cache()->ClearCredentials();
EXPECT_EQ(cache()->GetCredentialStore(origin).GetCredentials().size(), 0u); EXPECT_EQ(cache()->GetCredentialStore(origin).GetCredentials().size(), 0u);
......
...@@ -23,7 +23,7 @@ GURL GetAndroidOrOriginURL(const GURL& url) { ...@@ -23,7 +23,7 @@ GURL GetAndroidOrOriginURL(const GURL& url) {
CredentialPair::CredentialPair(base::string16 username, CredentialPair::CredentialPair(base::string16 username,
base::string16 password, base::string16 password,
const GURL& origin_url, const GURL& origin_url,
bool is_public_suffix_match) IsPublicSuffixMatch is_public_suffix_match)
: username(std::move(username)), : username(std::move(username)),
password(std::move(password)), password(std::move(password)),
origin_url(GetAndroidOrOriginURL(origin_url)), origin_url(GetAndroidOrOriginURL(origin_url)),
...@@ -32,6 +32,7 @@ CredentialPair::CredentialPair(CredentialPair&&) = default; ...@@ -32,6 +32,7 @@ CredentialPair::CredentialPair(CredentialPair&&) = default;
CredentialPair::CredentialPair(const CredentialPair&) = default; CredentialPair::CredentialPair(const CredentialPair&) = default;
CredentialPair& CredentialPair::operator=(CredentialPair&&) = default; CredentialPair& CredentialPair::operator=(CredentialPair&&) = default;
CredentialPair& CredentialPair::operator=(const CredentialPair&) = default; CredentialPair& CredentialPair::operator=(const CredentialPair&) = default;
CredentialPair::~CredentialPair() = default;
bool operator==(const CredentialPair& lhs, const CredentialPair& rhs) { bool operator==(const CredentialPair& lhs, const CredentialPair& rhs) {
return lhs.username == rhs.username && lhs.password == rhs.password && return lhs.username == rhs.username && lhs.password == rhs.password &&
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/containers/span.h" #include "base/containers/span.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/util/type_safety/strong_alias.h"
#include "url/gurl.h" #include "url/gurl.h"
#include "url/origin.h" #include "url/origin.h"
...@@ -16,19 +17,23 @@ namespace password_manager { ...@@ -16,19 +17,23 @@ namespace password_manager {
// Encapsulates the data from the password manager backend as used by the UI. // Encapsulates the data from the password manager backend as used by the UI.
struct CredentialPair { struct CredentialPair {
using IsPublicSuffixMatch =
util::StrongAlias<class IsPublicSuffixMatchTag, bool>;
CredentialPair(base::string16 username, CredentialPair(base::string16 username,
base::string16 password, base::string16 password,
const GURL& origin_url, const GURL& origin_url,
bool is_public_suffix_match); IsPublicSuffixMatch is_public_suffix_match);
CredentialPair(CredentialPair&&); CredentialPair(CredentialPair&&);
CredentialPair(const CredentialPair&); CredentialPair(const CredentialPair&);
CredentialPair& operator=(CredentialPair&&); CredentialPair& operator=(CredentialPair&&);
CredentialPair& operator=(const CredentialPair&); CredentialPair& operator=(const CredentialPair&);
~CredentialPair();
base::string16 username; base::string16 username;
base::string16 password; base::string16 password;
GURL origin_url; // Could be android:// which url::Origin doesn't support. GURL origin_url; // Could be android:// which url::Origin doesn't support.
bool is_public_suffix_match = false; IsPublicSuffixMatch is_public_suffix_match{false};
}; };
bool operator==(const CredentialPair& lhs, const CredentialPair& rhs); bool operator==(const CredentialPair& lhs, const CredentialPair& rhs);
......
...@@ -18,13 +18,15 @@ namespace { ...@@ -18,13 +18,15 @@ namespace {
using base::ASCIIToUTF16; using base::ASCIIToUTF16;
using testing::ElementsAre; using testing::ElementsAre;
using IsPublicSuffixMatch = CredentialPair::IsPublicSuffixMatch;
constexpr char kExampleSite[] = "https://example.com"; constexpr char kExampleSite[] = "https://example.com";
constexpr char kExampleSiteAndroidApp[] = "android://3x4mpl3@com.example.app/"; constexpr char kExampleSiteAndroidApp[] = "android://3x4mpl3@com.example.app/";
CredentialPair CreateCredentials(std::string username, std::string password) { CredentialPair CreateCredentials(std::string username, std::string password) {
return CredentialPair(base::ASCIIToUTF16(std::move(username)), return CredentialPair(base::ASCIIToUTF16(std::move(username)),
base::ASCIIToUTF16(std::move(password)), base::ASCIIToUTF16(std::move(password)),
GURL(kExampleSite), /*is_psl_match=*/false); GURL(kExampleSite), IsPublicSuffixMatch(false));
} }
} // namespace } // namespace
...@@ -53,30 +55,32 @@ TEST_F(OriginCredentialStoreTest, StoresCredentials) { ...@@ -53,30 +55,32 @@ TEST_F(OriginCredentialStoreTest, StoresCredentials) {
TEST_F(OriginCredentialStoreTest, StoresOnlyNormalizedOrigins) { TEST_F(OriginCredentialStoreTest, StoresOnlyNormalizedOrigins) {
store()->SaveCredentials( store()->SaveCredentials(
{CredentialPair(base::ASCIIToUTF16("Berta"), base::ASCIIToUTF16("30948"), {CredentialPair(base::ASCIIToUTF16("Berta"), base::ASCIIToUTF16("30948"),
GURL(kExampleSite), /*is_psl_match=*/false), GURL(kExampleSite), IsPublicSuffixMatch(false)),
CredentialPair(base::ASCIIToUTF16("Adam"), base::ASCIIToUTF16("Pas83B"), CredentialPair(base::ASCIIToUTF16("Adam"), base::ASCIIToUTF16("Pas83B"),
GURL(kExampleSite).Resolve("/agbs"), GURL(kExampleSite).Resolve("/agbs"),
/*is_psl_match=*/false), IsPublicSuffixMatch(false)),
CredentialPair(base::ASCIIToUTF16("Dora"), base::ASCIIToUTF16("PakudC"), CredentialPair(base::ASCIIToUTF16("Dora"), base::ASCIIToUTF16("PakudC"),
GURL(kExampleSiteAndroidApp), /*is_psl_match=*/false)}); GURL(kExampleSiteAndroidApp),
IsPublicSuffixMatch(false))});
EXPECT_THAT(store()->GetCredentials(), EXPECT_THAT(
store()->GetCredentials(),
ElementsAre( ElementsAre(
// The URL that equals an origin stays untouched. // The URL that equals an origin stays untouched.
CredentialPair(base::ASCIIToUTF16("Berta"), CredentialPair(base::ASCIIToUTF16("Berta"),
base::ASCIIToUTF16("30948"), base::ASCIIToUTF16("30948"), GURL(kExampleSite),
GURL(kExampleSite), /*is_psl_match=*/false), IsPublicSuffixMatch(false)),
// The longer URL is reduced to an origin. // The longer URL is reduced to an origin.
CredentialPair(base::ASCIIToUTF16("Adam"), CredentialPair(base::ASCIIToUTF16("Adam"),
base::ASCIIToUTF16("Pas83B"), base::ASCIIToUTF16("Pas83B"), GURL(kExampleSite),
GURL(kExampleSite), /*is_psl_match=*/false), IsPublicSuffixMatch(false)),
// The android origin stays untouched. // The android origin stays untouched.
CredentialPair( CredentialPair(
base::ASCIIToUTF16("Dora"), base::ASCIIToUTF16("PakudC"), base::ASCIIToUTF16("Dora"), base::ASCIIToUTF16("PakudC"),
GURL(kExampleSiteAndroidApp), /*is_psl_match=*/false))); GURL(kExampleSiteAndroidApp), IsPublicSuffixMatch(false))));
} }
TEST_F(OriginCredentialStoreTest, ReplacesCredentials) { TEST_F(OriginCredentialStoreTest, ReplacesCredentials) {
......
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