Commit 46ba6769 authored by Jan Wilken Dörrie's avatar Jan Wilken Dörrie Committed by Commit Bot

[Passwords] Add CreateChangePasswordUrl Utility

This change adds a CreateChangePasswordUrl utility for creating a
change password URL depending on the state of the
kWellKnownChangePassword feature flag. Furthermore, it updates the
Password Check callsites for all platforms to use this method.

Bug: 927473
Change-Id: I1019c92e15ee8357751fd0a166df8a830759e3f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2367060
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Auto-Submit: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: default avatarSergio Collazos <sczs@chromium.org>
Reviewed-by: default avatarIoana Pandele <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800409}
parent f5c73a9c
...@@ -67,19 +67,9 @@ using SavedPasswordsView = ...@@ -67,19 +67,9 @@ using SavedPasswordsView =
password_manager::SavedPasswordsPresenter::SavedPasswordsView; password_manager::SavedPasswordsPresenter::SavedPasswordsView;
using State = password_manager::BulkLeakCheckService::State; using State = password_manager::BulkLeakCheckService::State;
std::unique_ptr<std::string> GetChangePasswordUrl(const std::string& url) { std::unique_ptr<std::string> GetChangePasswordUrl(const GURL& url) {
// If the WellKnownChangePassword flag is enabled, replace the
// change_password_url to the well-known path. The
// WellKnownChangePasswordNavigationThrottle will continue process it.
if (!base::FeatureList::IsEnabled(
password_manager::features::kWellKnownChangePassword)) {
return std::make_unique<std::string>(url);
}
GURL origin = GURL(url).GetOrigin();
GURL::Replacements replacements;
replacements.SetPathStr(password_manager::kWellKnownChangePasswordPath);
return std::make_unique<std::string>( return std::make_unique<std::string>(
origin.ReplaceComponents(replacements).spec()); password_manager::CreateChangePasswordUrl(url).spec());
} }
} // namespace } // namespace
...@@ -283,7 +273,7 @@ PasswordCheckDelegate::GetCompromisedCredentials() { ...@@ -283,7 +273,7 @@ PasswordCheckDelegate::GetCompromisedCredentials() {
api_credential.formatted_origin = android_form.app_display_name; api_credential.formatted_origin = android_form.app_display_name;
api_credential.detailed_origin = android_form.app_display_name; api_credential.detailed_origin = android_form.app_display_name;
api_credential.change_password_url = api_credential.change_password_url =
GetChangePasswordUrl(android_form.affiliated_web_realm); GetChangePasswordUrl(GURL(android_form.affiliated_web_realm));
} else { } else {
// In case no affiliation information could be obtained show the // In case no affiliation information could be obtained show the
// formatted package name to the user. An empty change_password_url will // formatted package name to the user. An empty change_password_url will
...@@ -306,8 +296,7 @@ PasswordCheckDelegate::GetCompromisedCredentials() { ...@@ -306,8 +296,7 @@ PasswordCheckDelegate::GetCompromisedCredentials() {
api_credential.detailed_origin = api_credential.detailed_origin =
base::UTF16ToUTF8(url_formatter::FormatUrlForSecurityDisplay( base::UTF16ToUTF8(url_formatter::FormatUrlForSecurityDisplay(
credential.url.GetOrigin())); credential.url.GetOrigin()));
api_credential.change_password_url = api_credential.change_password_url = GetChangePasswordUrl(credential.url);
GetChangePasswordUrl(credential.url.GetOrigin().spec());
} }
api_credential.id = api_credential.id =
......
...@@ -56,8 +56,8 @@ namespace extensions { ...@@ -56,8 +56,8 @@ namespace extensions {
namespace { namespace {
constexpr char kExampleCom[] = "https://example.com"; constexpr char kExampleCom[] = "https://example.com/";
constexpr char kExampleOrg[] = "http://www.example.org"; constexpr char kExampleOrg[] = "http://www.example.org/";
constexpr char kExampleApp[] = "com.example.app"; constexpr char kExampleApp[] = "com.example.app";
constexpr char kTestEmail[] = "user@gmail.com"; constexpr char kTestEmail[] = "user@gmail.com";
...@@ -452,7 +452,7 @@ TEST_F(PasswordCheckDelegateTest, GetCompromisedCredentialsInjectsAndroid) { ...@@ -452,7 +452,7 @@ TEST_F(PasswordCheckDelegateTest, GetCompromisedCredentialsInjectsAndroid) {
delegate().GetCompromisedCredentials(), delegate().GetCompromisedCredentials(),
ElementsAre( ElementsAre(
ExpectCompromisedCredential( ExpectCompromisedCredential(
"Example App", "Example App", "https://example.com", kUsername2, "Example App", "Example App", "https://example.com/", kUsername2,
base::TimeDelta::FromDays(3), "3 days ago", base::TimeDelta::FromDays(3), "3 days ago",
api::passwords_private::COMPROMISE_TYPE_PHISHED), api::passwords_private::COMPROMISE_TYPE_PHISHED),
ExpectCompromisedCredential( ExpectCompromisedCredential(
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "components/password_manager/core/browser/password_manager_client.h" #include "components/password_manager/core/browser/password_manager_client.h"
#include "components/password_manager/core/browser/password_manager_util.h" #include "components/password_manager/core/browser/password_manager_util.h"
#include "components/password_manager/core/browser/ui/compromised_credentials_manager.h" #include "components/password_manager/core/browser/ui/compromised_credentials_manager.h"
#include "components/password_manager/core/browser/well_known_change_password_util.h"
#include "components/password_manager/core/common/password_manager_features.h" #include "components/password_manager/core/common/password_manager_features.h"
#include "components/password_manager/core/common/password_manager_pref_names.h" #include "components/password_manager/core/common/password_manager_pref_names.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
...@@ -25,22 +26,12 @@ ...@@ -25,22 +26,12 @@
namespace { namespace {
constexpr char kWellKnownUrlPath[] = ".well-known/change-password";
base::string16 GetDisplayUsername(const base::string16& username) { base::string16 GetDisplayUsername(const base::string16& username) {
return username.empty() return username.empty()
? l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_EMPTY_LOGIN) ? l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_EMPTY_LOGIN)
: username; : username;
} }
std::string CreateChangeUrl(const GURL& url) {
if (base::FeatureList::IsEnabled(
password_manager::features::kWellKnownChangePassword)) {
return url.GetOrigin().spec() + kWellKnownUrlPath;
}
return url.GetOrigin().spec();
}
} // namespace } // namespace
using autofill::PasswordForm; using autofill::PasswordForm;
...@@ -263,7 +254,8 @@ CompromisedCredentialForUI PasswordCheckManager::MakeUICredential( ...@@ -263,7 +254,8 @@ CompromisedCredentialForUI PasswordCheckManager::MakeUICredential(
url_formatter::kFormatUrlOmitTrivialSubdomains | url_formatter::kFormatUrlOmitTrivialSubdomains |
url_formatter::kFormatUrlTrimAfterHost, url_formatter::kFormatUrlTrimAfterHost,
net::UnescapeRule::SPACES, nullptr, nullptr, nullptr); net::UnescapeRule::SPACES, nullptr, nullptr, nullptr);
ui_credential.change_password_url = CreateChangeUrl(ui_credential.url); ui_credential.change_password_url =
password_manager::CreateChangePasswordUrl(ui_credential.url).spec();
} }
return ui_credential; return ui_credential;
......
...@@ -4,8 +4,11 @@ ...@@ -4,8 +4,11 @@
#include "components/password_manager/core/browser/well_known_change_password_util.h" #include "components/password_manager/core/browser/well_known_change_password_util.h"
#include "base/check.h"
#include "base/feature_list.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "components/password_manager/core/common/password_manager_features.h"
#include "url/gurl.h" #include "url/gurl.h"
namespace password_manager { namespace password_manager {
...@@ -28,6 +31,14 @@ bool IsWellKnownChangePasswordUrl(const GURL& url) { ...@@ -28,6 +31,14 @@ bool IsWellKnownChangePasswordUrl(const GURL& url) {
return path == kWellKnownChangePasswordPath; return path == kWellKnownChangePasswordPath;
} }
GURL CreateChangePasswordUrl(const GURL& url) {
DCHECK(url.is_valid());
GURL::Replacements replacements;
if (base::FeatureList::IsEnabled(features::kWellKnownChangePassword))
replacements.SetPathStr(password_manager::kWellKnownChangePasswordPath);
return url.GetOrigin().ReplaceComponents(replacements);
}
GURL CreateWellKnownNonExistingResourceURL(const GURL& url) { GURL CreateWellKnownNonExistingResourceURL(const GURL& url) {
GURL::Replacements replacement; GURL::Replacements replacement;
replacement.SetPathStr(kWellKnownNotExistingResourcePath); replacement.SetPathStr(kWellKnownNotExistingResourcePath);
......
...@@ -25,6 +25,11 @@ extern const char kWellKnownNotExistingResourcePath[]; ...@@ -25,6 +25,11 @@ extern const char kWellKnownNotExistingResourcePath[];
// https://wicg.github.io/change-password-url/ // https://wicg.github.io/change-password-url/
bool IsWellKnownChangePasswordUrl(const GURL& url); bool IsWellKnownChangePasswordUrl(const GURL& url);
// Creates a change password URL from `url`. In case the WellKnownChangePassword
// feature is active this returns the origin + `WellKnownChangePasswordPath`,
// otherwise it returns just the origin.
GURL CreateChangePasswordUrl(const GURL& url);
// Creates a GURL for a given origin with |kWellKnownNotExistingResourcePath| as // Creates a GURL for a given origin with |kWellKnownNotExistingResourcePath| as
// path. // path.
GURL CreateWellKnownNonExistingResourceURL(const GURL& url); GURL CreateWellKnownNonExistingResourceURL(const GURL& url);
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
#include "components/password_manager/core/browser/well_known_change_password_util.h" #include "components/password_manager/core/browser/well_known_change_password_util.h"
#include "base/test/scoped_feature_list.h"
#include "components/password_manager/core/common/password_manager_features.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h" #include "url/gurl.h"
#include "url/origin.h" #include "url/origin.h"
...@@ -27,6 +29,22 @@ TEST(WellKnownChangePasswordUtilTest, IsWellKnownChangePasswordUrl) { ...@@ -27,6 +29,22 @@ TEST(WellKnownChangePasswordUtilTest, IsWellKnownChangePasswordUrl) {
EXPECT_FALSE(IsWellKnownChangePasswordUrl(GURL("mailto:?subject=test"))); EXPECT_FALSE(IsWellKnownChangePasswordUrl(GURL("mailto:?subject=test")));
} }
TEST(WellKnownChangePasswordUtilTest, CreateChangePasswordUrlWithoutFeature) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(features::kWellKnownChangePassword);
EXPECT_EQ((GURL("https://example.com/")),
CreateChangePasswordUrl(GURL("https://example.com/some-path")));
}
TEST(WellKnownChangePasswordUtilTest, CreateChangePasswordUrlWithFeature) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(features::kWellKnownChangePassword);
EXPECT_EQ((GURL("https://example.com/.well-known/change-password")),
CreateChangePasswordUrl(GURL("https://example.com/some-path")));
}
TEST(WellKnownChangePasswordUtilTest, CreateWellKnownNonExistingResourceURL) { TEST(WellKnownChangePasswordUtilTest, CreateWellKnownNonExistingResourceURL) {
EXPECT_EQ(CreateWellKnownNonExistingResourceURL(GURL("https://google.com")), EXPECT_EQ(CreateWellKnownNonExistingResourceURL(GURL("https://google.com")),
GURL("https://google.com/.well-known/" GURL("https://google.com/.well-known/"
......
...@@ -52,7 +52,6 @@ source_set("password_details_ui") { ...@@ -52,7 +52,6 @@ source_set("password_details_ui") {
"//components/autofill/core/common", "//components/autofill/core/common",
"//components/password_manager/core/browser:affiliation", "//components/password_manager/core/browser:affiliation",
"//components/password_manager/core/browser:browser", "//components/password_manager/core/browser:browser",
"//components/password_manager/core/common",
"//components/strings", "//components/strings",
"//ios/chrome/app/strings:ios_chromium_strings_grit", "//ios/chrome/app/strings:ios_chromium_strings_grit",
"//ios/chrome/app/strings:ios_strings_grit", "//ios/chrome/app/strings:ios_strings_grit",
......
...@@ -9,7 +9,6 @@ ...@@ -9,7 +9,6 @@
#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/password_ui_utils.h" #include "components/password_manager/core/browser/password_ui_utils.h"
#include "components/password_manager/core/browser/well_known_change_password_util.h" #include "components/password_manager/core/browser/well_known_change_password_util.h"
#include "components/password_manager/core/common/password_manager_features.h"
#if !defined(__has_feature) || !__has_feature(objc_arc) #if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support." #error "This file requires ARC support."
...@@ -24,8 +23,8 @@ ...@@ -24,8 +23,8 @@
form.signon_realm); form.signon_realm);
if (facetUri.IsValidAndroidFacetURI()) { if (facetUri.IsValidAndroidFacetURI()) {
if (!form.app_display_name.empty()) { if (!form.app_display_name.empty()) {
_changePasswordURL = _changePasswordURL = password_manager::CreateChangePasswordUrl(
[self changePasswordUrlFor:GURL(form.affiliated_web_realm)]; GURL(form.affiliated_web_realm));
_origin = base::SysUTF8ToNSString(form.app_display_name); _origin = base::SysUTF8ToNSString(form.app_display_name);
_website = base::SysUTF8ToNSString(form.app_display_name); _website = base::SysUTF8ToNSString(form.app_display_name);
} else { } else {
...@@ -36,7 +35,7 @@ ...@@ -36,7 +35,7 @@
auto nameWithLink = password_manager::GetShownOriginAndLinkUrl(form); auto nameWithLink = password_manager::GetShownOriginAndLinkUrl(form);
_origin = base::SysUTF8ToNSString(nameWithLink.first); _origin = base::SysUTF8ToNSString(nameWithLink.first);
_website = base::SysUTF8ToNSString(nameWithLink.second.spec()); _website = base::SysUTF8ToNSString(nameWithLink.second.spec());
_changePasswordURL = [self changePasswordUrlFor:form.url]; _changePasswordURL = password_manager::CreateChangePasswordUrl(form.url);
} }
_username = base::SysUTF16ToNSString(form.username_value); _username = base::SysUTF16ToNSString(form.username_value);
_password = base::SysUTF16ToNSString(form.password_value); _password = base::SysUTF16ToNSString(form.password_value);
...@@ -44,14 +43,4 @@ ...@@ -44,14 +43,4 @@
return self; return self;
} }
- (GURL)changePasswordUrlFor:(const GURL&)url {
if (!base::FeatureList::IsEnabled(
password_manager::features::kWellKnownChangePassword)) {
return url.GetOrigin();
}
return password_manager::CreateWellKnownNonExistingResourceURL(
url.GetOrigin());
}
@end @end
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