diff --git a/atom/browser/net/url_request_context_getter.cc b/atom/browser/net/url_request_context_getter.cc index 9833616368fcc..4164a92e4db6f 100644 --- a/atom/browser/net/url_request_context_getter.cc +++ b/atom/browser/net/url_request_context_getter.cc @@ -188,6 +188,9 @@ URLRequestContextGetter::Handle::CreateNetworkContextParams() { base_path.Append(chrome::kChannelIDFilename); network_context_params->restore_old_session_cookies = false; network_context_params->persist_session_cookies = false; + // TODO(deepak1556): Matches the existing behavior https://git.io/fxHMl, + // enable encryption as a followup. + network_context_params->enable_encrypted_cookies = false; } // TODO(deepak1556): Decide the stand on chrome ct policy and @@ -259,6 +262,11 @@ void URLRequestContextGetter::NotifyContextShuttingDown( std::unique_ptr resource_context) { DCHECK_CURRENTLY_ON(BrowserThread::IO); + // todo(brenca): remove once C70 lands + if (url_request_context_ && url_request_context_->cookie_store()) { + url_request_context_->cookie_store()->FlushStore(base::NullCallback()); + } + context_shutting_down_ = true; resource_context.reset(); net::URLRequestContextGetter::NotifyContextShuttingDown(); diff --git a/patches/common/chromium/.patches b/patches/common/chromium/.patches index 6b5a62445a1b0..9322b6fbc4370 100644 --- a/patches/common/chromium/.patches +++ b/patches/common/chromium/.patches @@ -76,3 +76,4 @@ cross_site_document_resource_handler.patch content_allow_embedder_to_prevent_locking_scheme_registry.patch fix_trackpad_scrolling.patch mac_fix_form_control_rendering_on_10_14_mojave.patch +ensure_cookie_store.patch diff --git a/patches/common/chromium/ensure_cookie_store.patch b/patches/common/chromium/ensure_cookie_store.patch new file mode 100644 index 0000000000000..1bd5e5d3e1e47 --- /dev/null +++ b/patches/common/chromium/ensure_cookie_store.patch @@ -0,0 +1,375 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Matt Menke +Date: Fri, 27 Jul 2018 19:05:47 +0000 +Subject: Backports crrev.com/c/1152083 + +This backport ensures that the cookie store is always created. We use a +code path that didn't previously create the cookie store internally, +which caused all cookies to be deleted when the application was closed. + +This backport can be deleted when the C70 upgrade lands. + +diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc +index 55df34044af7bafb55521738a6581410877494c0..56da4a1012a6bcf7a500c0e600a08776b948ef03 100644 +--- a/chrome/browser/io_thread.cc ++++ b/chrome/browser/io_thread.cc +@@ -360,6 +360,11 @@ void IOThread::Init() { + #endif + + ConstructSystemRequestContext(); ++ ++ // Prevent DCHECK failures when a NetworkContext is created with an encrypted ++ // cookie store. ++ if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) ++ content::GetNetworkServiceImpl()->set_os_crypt_is_configured(); + } + + void IOThread::CleanUp() { +diff --git a/chrome/browser/profiles/off_the_record_profile_io_data.cc b/chrome/browser/profiles/off_the_record_profile_io_data.cc +index ebb7e95151156209aae7234de0e17ef9241335a7..de61d90917a40d0d64bb7c15daad0fd2c1147247 100644 +--- a/chrome/browser/profiles/off_the_record_profile_io_data.cc ++++ b/chrome/browser/profiles/off_the_record_profile_io_data.cc +@@ -215,14 +215,6 @@ void OffTheRecordProfileIOData::InitializeInternal( + std::make_unique( + new net::DefaultChannelIDStore(nullptr))); + +- using content::CookieStoreConfig; +- std::unique_ptr cookie_store(CreateCookieStore( +- CookieStoreConfig(base::FilePath(), false, false, nullptr))); +- cookie_store->SetChannelIDServiceID(channel_id_service->GetUniqueID()); +- +- builder->SetCookieAndChannelIdStores(std::move(cookie_store), +- std::move(channel_id_service)); +- + AddProtocolHandlersToBuilder(builder, protocol_handlers); + SetUpJobFactoryDefaultsForBuilder( + builder, std::move(request_interceptors), +diff --git a/chrome/browser/profiles/profile_impl_io_data.cc b/chrome/browser/profiles/profile_impl_io_data.cc +index 6dd54b7d045f38195c3858699dbd4ac5ad877277..c4038dc1e9cd5c13e946919ef5747357d347654f 100644 +--- a/chrome/browser/profiles/profile_impl_io_data.cc ++++ b/chrome/browser/profiles/profile_impl_io_data.cc +@@ -450,49 +450,6 @@ void ProfileImplIOData::InitializeInternal( + IOThread* const io_thread = profile_params->io_thread; + IOThread::Globals* const io_thread_globals = io_thread->globals(); + +- // This check is needed because with the network service the cookies are used +- // in a different process. See the bottom of +- // ProfileNetworkContextService::SetUpProfileIODataMainContext. +- if (profile_params->main_network_context_params->cookie_path) { +- // Create a single task runner to use with the CookieStore and +- // ChannelIDStore. +- scoped_refptr cookie_background_task_runner = +- base::CreateSequencedTaskRunnerWithTraits( +- {base::MayBlock(), base::TaskPriority::BACKGROUND, +- base::TaskShutdownBehavior::BLOCK_SHUTDOWN}); +- +- // Set up server bound cert service. +- DCHECK(!profile_params->main_network_context_params->channel_id_path.value() +- .empty()); +- scoped_refptr channel_id_db = +- new QuotaPolicyChannelIDStore( +- profile_params->main_network_context_params->channel_id_path +- .value(), +- cookie_background_task_runner, +- lazy_params_->special_storage_policy.get()); +- std::unique_ptr channel_id_service( +- std::make_unique( +- new net::DefaultChannelIDStore(channel_id_db.get()))); +- +- // Set up cookie store. +- content::CookieStoreConfig cookie_config( +- profile_params->main_network_context_params->cookie_path.value(), +- profile_params->main_network_context_params +- ->restore_old_session_cookies, +- profile_params->main_network_context_params->persist_session_cookies, +- lazy_params_->special_storage_policy.get()); +- cookie_config.crypto_delegate = cookie_config::GetCookieCryptoDelegate(); +- cookie_config.channel_id_service = channel_id_service.get(); +- cookie_config.background_task_runner = cookie_background_task_runner; +- std::unique_ptr cookie_store( +- content::CreateCookieStore(cookie_config)); +- +- cookie_store->SetChannelIDServiceID(channel_id_service->GetUniqueID()); +- +- builder->SetCookieAndChannelIdStores(std::move(cookie_store), +- std::move(channel_id_service)); +- } +- + AddProtocolHandlersToBuilder(builder, protocol_handlers); + + // Install the Offline Page Interceptor. +diff --git a/services/network/network_context.cc b/services/network/network_context.cc +index 50b4f60a47c046796f2452a7454e2ed821113d3c..ec799409ac3597c2437d38a8686efcb8448255b8 100644 +--- a/services/network/network_context.cc ++++ b/services/network/network_context.cc +@@ -64,6 +64,7 @@ + #include "net/url_request/static_http_user_agent_settings.h" + #include "net/url_request/url_request_context.h" + #include "net/url_request/url_request_context_builder.h" ++#include "services/network/cookie_manager.h" + #include "services/network/cors/cors_url_loader_factory.h" + #include "services/network/expect_ct_reporter.h" + #include "services/network/http_server_properties_pref_delegate.h" +@@ -308,10 +309,7 @@ NetworkContext::NetworkContext( + params_(std::move(params)), + on_connection_close_callback_(std::move(on_connection_close_callback)), + binding_(this, std::move(request)) { +- SessionCleanupCookieStore* session_cleanup_cookie_store = nullptr; +- SessionCleanupChannelIDStore* session_cleanup_channel_id_store = nullptr; +- url_request_context_owner_ = MakeURLRequestContext( +- &session_cleanup_cookie_store, &session_cleanup_channel_id_store); ++ url_request_context_owner_ = MakeURLRequestContext(); + url_request_context_ = url_request_context_owner_.url_request_context.get(); + + network_service_->RegisterNetworkContext(this); +@@ -323,10 +321,6 @@ NetworkContext::NetworkContext( + binding_.set_connection_error_handler(base::BindOnce( + &NetworkContext::OnConnectionError, base::Unretained(this))); + +- cookie_manager_ = std::make_unique( +- url_request_context_->cookie_store(), session_cleanup_cookie_store, +- session_cleanup_channel_id_store, +- std::move(params_->cookie_manager_params)); + socket_factory_ = std::make_unique(network_service_->net_log(), + url_request_context_); + resource_scheduler_ = +@@ -348,9 +342,6 @@ NetworkContext::NetworkContext( + url_request_context_ = url_request_context_owner_.url_request_context.get(); + + network_service_->RegisterNetworkContext(this); +- cookie_manager_ = std::make_unique( +- url_request_context_->cookie_store(), nullptr, nullptr, +- std::move(params_->cookie_manager_params)); + socket_factory_ = std::make_unique(network_service_->net_log(), + url_request_context_); + resource_scheduler_ = +@@ -819,6 +810,61 @@ URLRequestContextOwner NetworkContext::ApplyContextParamsToBuilder( + network_service_->network_quality_estimator()); + } + ++ scoped_refptr ++ session_cleanup_cookie_store; ++ scoped_refptr session_cleanup_channel_id_store; ++ if (params_->cookie_path) { ++ scoped_refptr client_task_runner = ++ base::MessageLoopCurrent::Get()->task_runner(); ++ scoped_refptr background_task_runner = ++ base::CreateSequencedTaskRunnerWithTraits( ++ {base::MayBlock(), base::TaskPriority::BEST_EFFORT, ++ base::TaskShutdownBehavior::BLOCK_SHUTDOWN}); ++ ++ std::unique_ptr channel_id_service; ++ if (params_->channel_id_path) { ++ session_cleanup_channel_id_store = ++ base::MakeRefCounted( ++ params_->channel_id_path.value(), background_task_runner); ++ channel_id_service = std::make_unique( ++ new net::DefaultChannelIDStore( ++ session_cleanup_channel_id_store.get())); ++ } ++ ++ net::CookieCryptoDelegate* crypto_delegate = nullptr; ++ if (params_->enable_encrypted_cookies) { ++#if defined(OS_LINUX) && !defined(OS_CHROMEOS) && !defined(IS_CHROMECAST) ++ DCHECK(network_service_->os_crypt_config_set()) ++ << "NetworkService::SetCryptConfig must be called before creating a " ++ "NetworkContext with encrypted cookies."; ++#endif ++ crypto_delegate = cookie_config::GetCookieCryptoDelegate(); ++ } ++ scoped_refptr sqlite_store( ++ new net::SQLitePersistentCookieStore( ++ params_->cookie_path.value(), client_task_runner, ++ background_task_runner, params_->restore_old_session_cookies, ++ crypto_delegate)); ++ ++ session_cleanup_cookie_store = ++ base::MakeRefCounted(sqlite_store); ++ ++ std::unique_ptr cookie_store = ++ std::make_unique(session_cleanup_cookie_store.get(), ++ channel_id_service.get()); ++ if (params_->persist_session_cookies) ++ cookie_store->SetPersistSessionCookies(true); ++ ++ if (channel_id_service) { ++ cookie_store->SetChannelIDServiceID(channel_id_service->GetUniqueID()); ++ } ++ builder->SetCookieAndChannelIdStores(std::move(cookie_store), ++ std::move(channel_id_service)); ++ } else { ++ DCHECK(!params_->restore_old_session_cookies); ++ DCHECK(!params_->persist_session_cookies); ++ } ++ + std::unique_ptr user_agent_settings = + std::make_unique( + params_->accept_language, params_->user_agent); +@@ -1065,6 +1111,12 @@ URLRequestContextOwner NetworkContext::ApplyContextParamsToBuilder( + #endif + } + ++ cookie_manager_ = std::make_unique( ++ result.url_request_context->cookie_store(), ++ std::move(session_cleanup_cookie_store), ++ std::move(session_cleanup_channel_id_store), ++ std::move(params_->cookie_manager_params)); ++ + return result; + } + +@@ -1098,71 +1150,11 @@ void NetworkContext::OnConnectionError() { + std::move(on_connection_close_callback_).Run(this); + } + +-URLRequestContextOwner NetworkContext::MakeURLRequestContext( +- SessionCleanupCookieStore** session_cleanup_cookie_store, +- SessionCleanupChannelIDStore** session_cleanup_channel_id_store) { ++URLRequestContextOwner NetworkContext::MakeURLRequestContext() { + URLRequestContextBuilderMojo builder; + const base::CommandLine* command_line = + base::CommandLine::ForCurrentProcess(); + +- // The cookie configuration is in this method, which is only used by the +- // network process, and not ApplyContextParamsToBuilder which is used by the +- // browser as well. This is because this code path doesn't handle encryption +- // and other configuration done in QuotaPolicyCookieStore yet (and we still +- // have to figure out which of the latter needs to move to the network +- // process). TODO: http://crbug.com/789644 +- if (params_->cookie_path) { +- scoped_refptr client_task_runner = +- base::MessageLoopCurrent::Get()->task_runner(); +- scoped_refptr background_task_runner = +- base::CreateSequencedTaskRunnerWithTraits( +- {base::MayBlock(), base::TaskPriority::BACKGROUND, +- base::TaskShutdownBehavior::BLOCK_SHUTDOWN}); +- +- std::unique_ptr channel_id_service; +- if (params_->channel_id_path) { +- auto channel_id_db = base::MakeRefCounted( +- params_->channel_id_path.value(), background_task_runner); +- *session_cleanup_channel_id_store = channel_id_db.get(); +- channel_id_service = std::make_unique( +- new net::DefaultChannelIDStore(channel_id_db.get())); +- } +- +- net::CookieCryptoDelegate* crypto_delegate = nullptr; +- if (params_->enable_encrypted_cookies) { +-#if defined(OS_LINUX) && !defined(OS_CHROMEOS) && !defined(IS_CHROMECAST) +- DCHECK(network_service_->os_crypt_config_set()) +- << "NetworkService::SetCryptConfig must be called before creating a " +- "NetworkContext with encrypted cookies."; +-#endif +- crypto_delegate = cookie_config::GetCookieCryptoDelegate(); +- } +- scoped_refptr sqlite_store( +- new net::SQLitePersistentCookieStore( +- params_->cookie_path.value(), client_task_runner, +- background_task_runner, params_->restore_old_session_cookies, +- crypto_delegate)); +- +- scoped_refptr cleanup_store( +- base::MakeRefCounted(sqlite_store)); +- *session_cleanup_cookie_store = cleanup_store.get(); +- +- std::unique_ptr cookie_store = +- std::make_unique(cleanup_store.get(), +- channel_id_service.get()); +- if (params_->persist_session_cookies) +- cookie_store->SetPersistSessionCookies(true); +- +- if (channel_id_service) { +- cookie_store->SetChannelIDServiceID(channel_id_service->GetUniqueID()); +- } +- builder.SetCookieAndChannelIdStores(std::move(cookie_store), +- std::move(channel_id_service)); +- } else { +- DCHECK(!params_->restore_old_session_cookies); +- DCHECK(!params_->persist_session_cookies); +- } +- + if (g_cert_verifier_for_testing) { + builder.SetCertVerifier(std::make_unique()); + } else { +diff --git a/services/network/network_context.h b/services/network/network_context.h +index 83459ab0f23ea0190bae410921391ca9d11c5aa9..4b25717b4be3a21249d177f0c7caca613e0aad89 100644 +--- a/services/network/network_context.h ++++ b/services/network/network_context.h +@@ -21,7 +21,6 @@ + #include "build/build_config.h" + #include "mojo/public/cpp/bindings/binding.h" + #include "mojo/public/cpp/bindings/strong_binding_set.h" +-#include "services/network/cookie_manager.h" + #include "services/network/http_cache_data_counter.h" + #include "services/network/http_cache_data_remover.h" + #include "services/network/public/mojom/network_context.mojom.h" +@@ -51,6 +50,7 @@ class TreeStateTracker; + } // namespace certificate_transparency + + namespace network { ++class CookieManager; + class ExpectCTReporter; + class NetworkService; + class ResourceScheduler; +@@ -233,9 +233,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext + // On connection errors the NetworkContext destroys itself. + void OnConnectionError(); + +- URLRequestContextOwner MakeURLRequestContext( +- SessionCleanupCookieStore** session_cleanup_cookie_store, +- SessionCleanupChannelIDStore** session_cleanup_channel_id_store); ++ URLRequestContextOwner MakeURLRequestContext(); + + NetworkService* const network_service_; + +diff --git a/services/network/network_context_unittest.cc b/services/network/network_context_unittest.cc +index c756789a39f3d265c536a69d899512e816708344..88cdcc0b5433087c15edad4e6a977bb2a127f834 100644 +--- a/services/network/network_context_unittest.cc ++++ b/services/network/network_context_unittest.cc +@@ -72,6 +72,7 @@ + #include "net/url_request/url_request_context.h" + #include "net/url_request/url_request_context_builder.h" + #include "net/url_request/url_request_job_factory.h" ++#include "services/network/cookie_manager.h" + #include "services/network/mojo_net_log.h" + #include "services/network/network_context.h" + #include "services/network/network_service.h" +diff --git a/services/network/network_service.cc b/services/network/network_service.cc +index c4df8eceaad173b580c8fa91aca6cfdced5f571b..a91a9f4b5866c11ae76e2d173a92579f0b7d12a1 100644 +--- a/services/network/network_service.cc ++++ b/services/network/network_service.cc +@@ -169,6 +169,10 @@ NetworkService::~NetworkService() { + DCHECK(network_contexts_.empty()); + } + ++void NetworkService::set_os_crypt_is_configured() { ++ os_crypt_config_set_ = true; ++} ++ + std::unique_ptr NetworkService::Create( + mojom::NetworkServiceRequest request, + net::NetLog* net_log) { +@@ -370,6 +374,7 @@ void NetworkService::UpdateSignedTreeHead(const net::ct::SignedTreeHead& sth) { + #if defined(OS_LINUX) && !defined(OS_CHROMEOS) + void NetworkService::SetCryptConfig(mojom::CryptConfigPtr crypt_config) { + #if !defined(IS_CHROMECAST) ++ DCHECK(!os_crypt_config_set_); + auto config = std::make_unique(); + config->store = crypt_config->store; + config->product_name = crypt_config->product_name; +diff --git a/services/network/network_service.h b/services/network/network_service.h +index 839a5da98be9adaa836a3881f5a42fde069c860c..a686fd84e0cf61219289d6f5ee9c700edb9bc96d 100644 +--- a/services/network/network_service.h ++++ b/services/network/network_service.h +@@ -62,6 +62,11 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService + + ~NetworkService() override; + ++ // Call to inform the NetworkService that OSCrypt::SetConfig() has already ++ // been invoked, so OSCrypt::SetConfig() does not need to be called before ++ // encrypted storage can be used. ++ void set_os_crypt_is_configured(); ++ + // Can be used to seed a NetworkContext with a consumer-configured + // URLRequestContextBuilder, which |params| will then be applied to. The + // results URLRequestContext will be written to |url_request_context|, which diff --git a/spec/api-session-spec.js b/spec/api-session-spec.js index a0f0066642a9c..deeb2eba2bcf5 100644 --- a/spec/api-session-spec.js +++ b/spec/api-session-spec.js @@ -6,6 +6,7 @@ const path = require('path') const fs = require('fs') const send = require('send') const auth = require('basic-auth') +const ChildProcess = require('child_process') const { closeWindow } = require('./window-helpers') const { ipcRenderer, remote } = require('electron') @@ -213,6 +214,33 @@ describe('session module', () => { }) }) }) + + it('should survive an app restart for persistent partition', async () => { + const appPath = path.join(__dirname, 'fixtures', 'api', 'cookie-app') + const electronPath = remote.getGlobal('process').execPath + + const test = (result, phase) => { + return new Promise((resolve, reject) => { + let output = '' + + const appProcess = ChildProcess.spawn( + electronPath, + [appPath], + { env: { PHASE: phase, ...process.env } } + ) + + appProcess.stdout.on('data', (data) => { output += data }) + appProcess.stdout.on('end', () => { + output = output.replace(/(\r\n|\n|\r)/gm, '') + assert.strictEqual(output, result) + resolve() + }) + }) + } + + await test('011', 'one') + await test('110', 'two') + }) }) describe('ses.clearStorageData(options)', () => { diff --git a/spec/fixtures/api/cookie-app/main.js b/spec/fixtures/api/cookie-app/main.js new file mode 100644 index 0000000000000..b9ba61d00efe0 --- /dev/null +++ b/spec/fixtures/api/cookie-app/main.js @@ -0,0 +1,70 @@ +const { app, session } = require('electron') + +app.on('ready', async function () { + const url = 'http://foo.bar' + const persistentSession = session.fromPartition('persist:ence-test') + + const set = () => { + return new Promise((resolve, reject) => { + persistentSession.cookies.set({ + url, + name: 'test', + value: 'true', + expirationDate: Date.now() + 60000 + }, error => { + if (error) { + reject(error) + } else { + resolve() + } + }) + }) + } + + const get = () => { + return new Promise((resolve, reject) => { + persistentSession.cookies.get({ url }, (error, list) => { + if (error) { + reject(error) + } else { + resolve(list) + } + }) + }) + } + + const maybeRemove = (pred) => { + return new Promise((resolve, reject) => { + if (pred()) { + persistentSession.cookies.remove(url, 'test', error => { + if (error) { + reject(error) + } else { + resolve() + } + }) + } else { + resolve() + } + }) + } + + try { + await maybeRemove(() => process.env.PHASE === 'one') + const one = await get() + await set() + const two = await get() + await maybeRemove(() => process.env.PHASE === 'two') + const three = await get() + + process.stdout.write(`${one.length}${two.length}${three.length}`) + } catch (e) { + process.stdout.write('ERROR') + } finally { + process.stdout.end() + + setImmediate(() => { + app.quit() + }) + } +}) diff --git a/spec/fixtures/api/cookie-app/package.json b/spec/fixtures/api/cookie-app/package.json new file mode 100644 index 0000000000000..bd21c272f5e2b --- /dev/null +++ b/spec/fixtures/api/cookie-app/package.json @@ -0,0 +1,4 @@ +{ + "name": "electron-cookie-app", + "main": "main.js" +}