Skip to content

GitLab

T
tangled
Commit 18671723 authored by Jan Wilken Dörrie's avatar Jan Wilken Dörrie Committed by Commit Bot
Browse files
Options

[Passwords] Support x-origin redirects in WellKnown Throttle 

This change modifies the WellKnownChangePasswordNavigationThrottle to
use the URL used in the beginning of the navigation for the fallback
logic in case the server does not support .well-known/change-password
URLs.

So far it has been using `navigation_handle()->GetURL()`, which could
change in the presence of server-side redirects and thus could point
to a different origin, for which the fallback logic would be wrong.

Fixed: 1130519
Change-Id: I8177f7a8f794de91d2cf81d92aab11179280f73c
Bug: 1130519
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2421512Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809306}
parent 0fcaf722
Hide whitespace changes
Inline Side-by-side
Showing with 48 additions and 34 deletions
chrome/browser/ui/passwords/well_known_change_password_navigation_throttle.cc
View file @ 18671723
......@@ -109,6 +109,7 @@ WellKnownChangePasswordNavigationThrottle::MaybeCreateThrottleFor(
WellKnownChangePasswordNavigationThrottle::
WellKnownChangePasswordNavigationThrottle(NavigationHandle* handle)
: NavigationThrottle(handle),
request_url_(handle->GetURL()),
source_id_(
ukm::GetSourceIdForWebContentsDocument(handle->GetWebContents())) {
if (base::FeatureList::IsEnabled(
......@@ -117,7 +118,7 @@ WellKnownChangePasswordNavigationThrottle::
AffiliationServiceFactory::GetForProfile(Profile::FromBrowserContext(
handle->GetWebContents()->GetBrowserContext()));
well_known_change_password_state_.PrefetchChangePasswordURLs(
affiliation_service_, {navigation_handle()->GetURL()});
affiliation_service_, {request_url_});
} else {
change_password_url_service_ =
ChangePasswordUrlServiceFactory::GetForBrowserContext(
......@@ -146,7 +147,7 @@ WellKnownChangePasswordNavigationThrottle::WillStartRequest() {
net::IsolationInfo::RedirectMode::kUpdateNothing,
navigation_handle()->GetIsolationInfo().network_isolation_key());
well_known_change_password_state_.FetchNonExistingResource(
url_loader_factory.get(), navigation_handle()->GetURL(),
url_loader_factory.get(), request_url_,
navigation_handle()->GetInitiatorOrigin(), std::move(trusted_params));
return NavigationThrottle::PROCEED;
}
......@@ -183,20 +184,20 @@ void WellKnownChangePasswordNavigationThrottle::OnProcessingFinished(
Resume();
return;
}
GURL url = navigation_handle()->GetURL();
GURL redirect_url;
if (base::FeatureList::IsEnabled(
password_manager::features::kChangePasswordAffiliationInfo)) {
redirect_url = affiliation_service_->GetChangePasswordURL(url);
redirect_url = affiliation_service_->GetChangePasswordURL(request_url_);
} else {
redirect_url = change_password_url_service_->GetChangePasswordUrl(url);
redirect_url =
change_password_url_service_->GetChangePasswordUrl(request_url_);
}
if (redirect_url.is_valid()) {
RecordMetric(WellKnownChangePasswordResult::kFallbackToOverrideUrl);
Redirect(redirect_url);
} else {
RecordMetric(WellKnownChangePasswordResult::kFallbackToOriginUrl);
Redirect(url.GetOrigin());
Redirect(request_url_.GetOrigin());
}
CancelDeferredNavigation(NavigationThrottle::CANCEL);
}
......
chrome/browser/ui/passwords/well_known_change_password_navigation_throttle.h
View file @ 18671723
......@@ -57,6 +57,10 @@ class WellKnownChangePasswordNavigationThrottle
// Records the given UKM metric.
void RecordMetric(password_manager::WellKnownChangePasswordResult result);
// Stores `navigation_handle()->GetURL()` if the first navigation was to
// .well-known/change-password. It is later used to derive the URL for the
// non-existing resource, and to provide fallback logic.
const GURL request_url_;
password_manager::WellKnownChangePasswordState
well_known_change_password_state_{this};
ukm::SourceId source_id_ = ukm::kInvalidSourceId;
......
chrome/browser/ui/passwords/well_known_change_password_navigation_throttle_browsertest.cc
View file @ 18671723
......@@ -57,6 +57,7 @@ using net::test_server::HttpResponse;
using password_manager::kWellKnownChangePasswordPath;
using password_manager::kWellKnownNotExistingResourcePath;
using password_manager::WellKnownChangePasswordResult;
using testing::Return;
constexpr char kMockChangePasswordPath[] = "/change-password-override";
......@@ -77,24 +78,11 @@ struct ResponseDelayParams {
} // namespace
class TestChangePasswordUrlService
class MockChangePasswordUrlService
: public password_manager::ChangePasswordUrlService {
public:
void PrefetchURLs() override {}
GURL GetChangePasswordUrl(const GURL& url) override {
if (override_available_) {
GURL::Replacements replacement;
replacement.SetPathStr(kMockChangePasswordPath);
return url.ReplaceComponents(replacement);
}
return GURL();
}
void SetOverrideAvailable(bool available) { override_available_ = available; }
private:
bool override_available_ = false;
MOCK_METHOD(GURL, GetChangePasswordUrl, (const GURL&), (override));
};
class ChangePasswordNavigationThrottleBrowserTestBase
......@@ -111,6 +99,7 @@ class ChangePasswordNavigationThrottleBrowserTestBase
}
void Initialize() {
host_resolver()->AddRule("*", "127.0.0.1");
ASSERT_TRUE(test_server_->InitializeAndListen());
test_server_->StartAcceptingConnections();
test_recorder_ = std::make_unique<ukm::TestAutoSetUkmRecorder>();
......@@ -211,7 +200,7 @@ class WellKnownChangePasswordNavigationThrottleBrowserTest
void SetUpOnMainThread() override;
void TestNavigationThrottleForLocalhost(const std::string& expected_path);
TestChangePasswordUrlService* url_service_ = nullptr;
MockChangePasswordUrlService* url_service_ = nullptr;
private:
base::test::ScopedFeatureList feature_list_;
......@@ -224,7 +213,8 @@ void WellKnownChangePasswordNavigationThrottleBrowserTest::SetUpOnMainThread() {
->SetTestingSubclassFactoryAndUse(
browser()->profile(),
base::BindRepeating([](content::BrowserContext*) {
return std::make_unique<TestChangePasswordUrlService>();
return std::make_unique<
testing::StrictMock<MockChangePasswordUrlService>>();
}));
}
......@@ -336,11 +326,14 @@ IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
}
}
IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
SupportForChangePassword_WithRedirectToNotFoundPage) {
IN_PROC_BROWSER_TEST_P(
WellKnownChangePasswordNavigationThrottleBrowserTest,
SupportForChangePassword_WithXOriginRedirectToNotFoundPage) {
GURL change_password_url =
test_server_->GetURL("example.com", "/change-password");
path_response_map_[kWellKnownChangePasswordPath] = {
net::HTTP_PERMANENT_REDIRECT,
{std::make_pair("Location", "/change-password")},
{std::make_pair("Location", change_password_url.spec())},
response_delays().change_password_delay};
path_response_map_[kWellKnownNotExistingResourcePath] = {
net::HTTP_PERMANENT_REDIRECT,
......@@ -349,7 +342,8 @@ IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
path_response_map_["/change-password"] = {net::HTTP_OK, {}, 0};
path_response_map_["/not-found"] = {net::HTTP_NOT_FOUND, {}, 0};
TestNavigationThrottleForLocalhost(/*expected_path=*/"/change-password");
TestNavigationThrottle(test_server_->GetURL(kWellKnownChangePasswordPath),
change_password_url);
if (page_transition() & ui::PAGE_TRANSITION_FROM_API) {
ExpectUkmMetric(
WellKnownChangePasswordResult::kUsedWellKnownChangePassword);
......@@ -367,6 +361,9 @@ IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
net::HTTP_NOT_FOUND, {}, response_delays().not_exist_delay};
if (page_transition() & ui::PAGE_TRANSITION_FROM_API) {
EXPECT_CALL(*url_service_, GetChangePasswordUrl(test_server_->GetURL(
kWellKnownChangePasswordPath)))
.WillOnce(Return(GURL()));
TestNavigationThrottleForLocalhost(/*expected_path=*/"/");
ExpectUkmMetric(WellKnownChangePasswordResult::kFallbackToOriginUrl);
} else {
......@@ -379,13 +376,15 @@ IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
NoSupportForChangePassword_WithUrlOverride) {
url_service_->SetOverrideAvailable(true);
path_response_map_[kWellKnownChangePasswordPath] = {
net::HTTP_NOT_FOUND, {}, response_delays().change_password_delay};
path_response_map_[kWellKnownNotExistingResourcePath] = {
net::HTTP_NOT_FOUND, {}, response_delays().not_exist_delay};
if (page_transition() & ui::PAGE_TRANSITION_FROM_API) {
EXPECT_CALL(*url_service_, GetChangePasswordUrl(test_server_->GetURL(
kWellKnownChangePasswordPath)))
.WillOnce(Return(test_server_->GetURL(kMockChangePasswordPath)));
TestNavigationThrottleForLocalhost(
/*expected_path=*/kMockChangePasswordPath);
ExpectUkmMetric(WellKnownChangePasswordResult::kFallbackToOverrideUrl);
......@@ -406,6 +405,9 @@ IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
net::HTTP_OK, {}, response_delays().not_exist_delay};
if (page_transition() & ui::PAGE_TRANSITION_FROM_API) {
EXPECT_CALL(*url_service_, GetChangePasswordUrl(test_server_->GetURL(
kWellKnownChangePasswordPath)))
.WillOnce(Return(GURL()));
TestNavigationThrottleForLocalhost(/*expected_path=*/"/");
ExpectUkmMetric(WellKnownChangePasswordResult::kFallbackToOriginUrl);
} else {
......@@ -416,23 +418,31 @@ IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
}
}
IN_PROC_BROWSER_TEST_P(WellKnownChangePasswordNavigationThrottleBrowserTest,
NoSupportForChangePassword_WithRedirectToNotFoundPage) {
IN_PROC_BROWSER_TEST_P(
WellKnownChangePasswordNavigationThrottleBrowserTest,
NoSupportForChangePassword_WithXOriginRedirectToNotFoundPage) {
// Test a cross-origin redirect to a 404 page. Ensure that we try to obtain
// the ChangePasswordUrl for the original origin.
GURL not_found_url = test_server_->GetURL("example.com", "/not-found");
path_response_map_[kWellKnownChangePasswordPath] = {
net::HTTP_PERMANENT_REDIRECT,
{std::make_pair("Location", "/not-found")},
{std::make_pair("Location", not_found_url.spec())},
response_delays().change_password_delay};
path_response_map_[kWellKnownNotExistingResourcePath] = {
net::HTTP_PERMANENT_REDIRECT,
{std::make_pair("Location", "/not-found")},
{std::make_pair("Location", not_found_url.spec())},
response_delays().not_exist_delay};
path_response_map_["/not-found"] = {net::HTTP_NOT_FOUND, {}, 0};
if (page_transition() & ui::PAGE_TRANSITION_FROM_API) {
EXPECT_CALL(*url_service_, GetChangePasswordUrl(test_server_->GetURL(
kWellKnownChangePasswordPath)))
.WillOnce(Return(GURL()));
TestNavigationThrottleForLocalhost(/*expected_path=*/"/");
ExpectUkmMetric(WellKnownChangePasswordResult::kFallbackToOriginUrl);
} else {
TestNavigationThrottleForLocalhost(/*expected_path=*/"/not-found");
TestNavigationThrottle(test_server_->GetURL(kWellKnownChangePasswordPath),
not_found_url);
EXPECT_TRUE(
test_recorder()->GetEntriesByName(UkmBuilder::kEntryName).empty());
}
......@@ -506,7 +516,6 @@ class AffiliationChangePasswordNavigationThrottleBrowserTest
void AffiliationChangePasswordNavigationThrottleBrowserTest::
SetUpOnMainThread() {
Initialize();
host_resolver()->AddRule("*", "127.0.0.1");
scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory =
base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
......
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