Commit 49951bc3 authored by David Roger's avatar David Roger Committed by Commit Bot

[signin] Make CHROME_ID_CONSISTENCY_STATE a host cookie

The cookie was a domain cookie, but host cookie is preferred.

This allows simplifying and re-enabling the test.

The test was failing with net::ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN.
This CL removes the DNS rule and the SSL configuration (which were
required for the domain cookie, but can be removed for a host cookie),
which makes the test simpler and hopefully more robust.

Bug: 964264, 965993
Change-Id: I03412b9046b79a2cbb56ebdb95dd1661d67fb938
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1617360
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662203}
parent 80c83c2b
...@@ -13,8 +13,6 @@ ...@@ -13,8 +13,6 @@
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/account_reconcilor_factory.h" #include "chrome/browser/signin/account_reconcilor_factory.h"
#include "chrome/browser/signin/chrome_signin_client_factory.h" #include "chrome/browser/signin/chrome_signin_client_factory.h"
#include "chrome/browser/ssl/cert_verifier_browser_test.h"
#include "chrome/browser/ssl/chrome_mock_cert_verifier.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h" #include "chrome/test/base/ui_test_utils.h"
...@@ -26,7 +24,6 @@ ...@@ -26,7 +24,6 @@
#include "mojo/public/cpp/bindings/binding.h" #include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/cpp/bindings/interface_request.h" #include "mojo/public/cpp/bindings/interface_request.h"
#include "net/cookies/canonical_cookie.h" #include "net/cookies/canonical_cookie.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/default_handlers.h" #include "net/test/embedded_test_server/default_handlers.h"
#include "net/test/embedded_test_server/embedded_test_server.h" #include "net/test/embedded_test_server/embedded_test_server.h"
#include "services/network/public/mojom/cookie_manager.mojom.h" #include "services/network/public/mojom/cookie_manager.mojom.h"
...@@ -50,8 +47,7 @@ class TestConsistencyCookieManager ...@@ -50,8 +47,7 @@ class TestConsistencyCookieManager
// Listen to cookie changes. // Listen to cookie changes.
network::mojom::CookieChangeListenerPtr listener_ptr; network::mojom::CookieChangeListenerPtr listener_ptr;
cookie_listener_binding_.Bind(mojo::MakeRequest(&listener_ptr)); cookie_listener_binding_.Bind(mojo::MakeRequest(&listener_ptr));
client->GetCookieManager()->AddCookieChangeListener( client->GetCookieManager()->AddGlobalChangeListener(
GaiaUrls::GetInstance()->gaia_url(), kConsistencyCookieName,
std::move(listener_ptr)); std::move(listener_ptr));
// Subclasses have to call UpdateCookie() in the constructor. // Subclasses have to call UpdateCookie() in the constructor.
UpdateCookie(); UpdateCookie();
...@@ -77,7 +73,8 @@ class TestConsistencyCookieManager ...@@ -77,7 +73,8 @@ class TestConsistencyCookieManager
// CookieChangeListener: // CookieChangeListener:
void OnCookieChange(const net::CanonicalCookie& cookie, void OnCookieChange(const net::CanonicalCookie& cookie,
network::mojom::CookieChangeCause cause) override { network::mojom::CookieChangeCause cause) override {
ASSERT_EQ(kConsistencyCookieName, cookie.Name()); if (cookie.Name() != kConsistencyCookieName)
return;
if (!run_loop_quit_closure_.is_null()) if (!run_loop_quit_closure_.is_null())
std::move(run_loop_quit_closure_).Run(); std::move(run_loop_quit_closure_).Run();
} }
...@@ -92,7 +89,7 @@ class TestConsistencyCookieManager ...@@ -92,7 +89,7 @@ class TestConsistencyCookieManager
} // namespace } // namespace
class ConsistencyCookieBrowserTest : public CertVerifierBrowserTest { class ConsistencyCookieBrowserTest : public InProcessBrowserTest {
public: public:
ConsistencyCookieBrowserTest() ConsistencyCookieBrowserTest()
: https_server_(net::EmbeddedTestServer::TYPE_HTTPS) { : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {
...@@ -129,31 +126,23 @@ class ConsistencyCookieBrowserTest : public CertVerifierBrowserTest { ...@@ -129,31 +126,23 @@ class ConsistencyCookieBrowserTest : public CertVerifierBrowserTest {
} }
private: private:
// CertVerifierBrowserTest: // InProcessBrowserTest:
void SetUp() override { void SetUp() override {
ASSERT_TRUE(https_server_.InitializeAndListen()); ASSERT_TRUE(https_server_.InitializeAndListen());
CertVerifierBrowserTest::SetUp(); InProcessBrowserTest::SetUp();
}
void SetUpCommandLine(base::CommandLine* command_line) override {
const GURL& base_url = https_server_.base_url();
command_line->AppendSwitchASCII(switches::kGaiaUrl, base_url.spec());
} }
void SetUpOnMainThread() override { void SetUpOnMainThread() override {
CertVerifierBrowserTest::SetUpOnMainThread(); InProcessBrowserTest::SetUpOnMainThread();
// Configure the embedded test server.
// DNS rule for the Gaia URL.
host_resolver()->AddRule(GaiaUrls::GetInstance()->gaia_url().host(),
https_server_.base_url().host());
// Whitelist all certs for the HTTPS server to prevent SSL errors.
auto cert = https_server_.GetCertificate();
net::CertVerifyResult verify_result;
verify_result.cert_status = 0;
verify_result.is_issued_by_known_root = true;
verify_result.verified_cert = cert;
mock_cert_verifier()->AddResultForCert(cert.get(), verify_result, net::OK);
// Start the server.
https_server_.StartAcceptingConnections(); https_server_.StartAcceptingConnections();
Profile* profile = browser()->profile();
// Setup the CookieConsistencyCookieManager // Setup the CookieConsistencyCookieManager
Profile* profile = browser()->profile();
AccountReconcilor* reconcilor = AccountReconcilor* reconcilor =
AccountReconcilorFactory::GetForProfile(profile); AccountReconcilorFactory::GetForProfile(profile);
std::unique_ptr<TestConsistencyCookieManager> consistency_cookie_manager = std::unique_ptr<TestConsistencyCookieManager> consistency_cookie_manager =
...@@ -170,9 +159,7 @@ class ConsistencyCookieBrowserTest : public CertVerifierBrowserTest { ...@@ -170,9 +159,7 @@ class ConsistencyCookieBrowserTest : public CertVerifierBrowserTest {
// Tests that the ConsistencyCookieManager can set and change the cookie in HTTP // Tests that the ConsistencyCookieManager can set and change the cookie in HTTP
// and javascript. // and javascript.
// TODO(https://crbug.com/964264): Fails on some bots with error IN_PROC_BROWSER_TEST_F(ConsistencyCookieBrowserTest, Basic) {
// net::ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN.
IN_PROC_BROWSER_TEST_F(ConsistencyCookieBrowserTest, DISABLED_Basic) {
// Check the initial value. // Check the initial value.
CheckCookieValue(std::string(kConsistencyCookieName) + "=initial_value"); CheckCookieValue(std::string(kConsistencyCookieName) + "=initial_value");
// Change the cookie. // Change the cookie.
......
...@@ -67,11 +67,10 @@ void ConsistencyCookieManagerBase::UpdateCookie() { ...@@ -67,11 +67,10 @@ void ConsistencyCookieManagerBase::UpdateCookie() {
base::Time now = base::Time::Now(); base::Time now = base::Time::Now();
base::Time expiry = now + base::TimeDelta::FromDays(2 * 365); // Two years. base::Time expiry = now + base::TimeDelta::FromDays(2 * 365); // Two years.
net::CanonicalCookie cookie( net::CanonicalCookie cookie(
kCookieName, cookie_value, kCookieName, cookie_value, GaiaUrls::GetInstance()->gaia_url().host(),
"." + GaiaUrls::GetInstance()->gaia_url().host(), /*path=*/"/", /*path=*/"/", /*creation=*/now, /*expiration=*/expiry,
/*creation=*/now, /*expiration=*/expiry, /*last_access=*/now, /*last_access=*/now, /*secure=*/true, /*httponly=*/false,
/*secure=*/true, /*httponly=*/false, net::CookieSameSite::NO_RESTRICTION, net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_DEFAULT);
net::COOKIE_PRIORITY_DEFAULT);
cookie_manager->SetCanonicalCookie( cookie_manager->SetCanonicalCookie(
cookie, "https", net::CookieOptions(), cookie, "https", net::CookieOptions(),
network::mojom::CookieManager::SetCanonicalCookieCallback()); network::mojom::CookieManager::SetCanonicalCookieCallback());
......
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