Allow dynamic updating of authentication policies

chrome/browser/io_thread.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/io_thread.h"
 
 #include <vector>
 
 #include "base/base64.h"
 #include "base/bind.h"
@@ skipping 440 matching lines @@
     : net_log_(net_log),
 #if defined(ENABLE_EXTENSIONS)
       extension_event_router_forwarder_(extension_event_router_forwarder),
 #endif
       globals_(NULL),
       is_spdy_disabled_by_policy_(false),
       is_quic_allowed_by_policy_(true),
       creation_time_(base::TimeTicks::Now()),
       weak_factory_(this) {
   auth_schemes_ = local_state->GetString(prefs::kAuthSchemes);
-  negotiate_disable_cname_lookup_ = local_state->GetBoolean(
-      prefs::kDisableAuthNegotiateCnameLookup);
-  negotiate_enable_port_ = local_state->GetBoolean(
-      prefs::kEnableAuthNegotiatePort);
-  auth_server_whitelist_ = local_state->GetString(prefs::kAuthServerWhitelist);
-  auth_delegate_whitelist_ = local_state->GetString(
-      prefs::kAuthNegotiateDelegateWhitelist);
+  negotiate_disable_cname_lookup_.Init(
+      prefs::kDisableAuthNegotiateCnameLookup, local_state,
+      base::Bind(&IOThread::UpdateNegotiateDisableCnameLookup,
+                 weak_factory_.GetWeakPtr()));
+  scoped_refptr<base::SingleThreadTaskRunner> io_thread_proxy =
+      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO);
+  negotiate_disable_cname_lookup_.MoveToThread(io_thread_proxy);
+  negotiate_enable_port_.Init(prefs::kEnableAuthNegotiatePort, local_state,
+                              base::Bind(&IOThread::UpdateNegotiateEnablePort,
+                                         weak_factory_.GetWeakPtr()));
+  negotiate_enable_port_.MoveToThread(io_thread_proxy);
+  auth_server_whitelist_.Init(
+      prefs::kAuthServerWhitelist, local_state,
+      base::Bind(&IOThread::UpdateServerWhitelist, weak_factory_.GetWeakPtr()));
+  auth_server_whitelist_.MoveToThread(io_thread_proxy);
+  auth_delegate_whitelist_.Init(prefs::kAuthNegotiateDelegateWhitelist,
+                                local_state,
+                                base::Bind(&IOThread::UpdateDelegateWhitelist,
+                                           weak_factory_.GetWeakPtr()));
+  auth_delegate_whitelist_.MoveToThread(io_thread_proxy);
   gssapi_library_name_ = local_state->GetString(prefs::kGSSAPILibraryName);
-  auth_android_negotiate_account_type_ =
-      local_state->GetString(prefs::kAuthAndroidNegotiateAccountType);
+  auth_android_negotiate_account_type_.Init(

[asanka, 2015/10/28 03:27:43]

Can we condition out the Android code?

[aberent, 2015/11/02 18:52:50]

Not sure about this; should we also condition out the non-Android, Posix
specific code (gssapi_library_name is only used on non-Android Posix systems)?
In fact you could argue that much of this should be within an #if
defined(USE_KERBEROS).

I don't like conditioning out code excessively. If you leave it in
unconditionally then, if people make a change they at least compile it locally,
and find the compilation errors locally (even if it can't be tested locally)
whereas if it is only compiled on other platforms they are likely to not to find
the compilation errors until they send it to the trybots.

+      prefs::kAuthAndroidNegotiateAccountType, local_state,
+      base::Bind(&IOThread::UpdateAndroidAuthNegotiateAccount,
+                 weak_factory_.GetWeakPtr()));
+  auth_android_negotiate_account_type_.MoveToThread(io_thread_proxy);
   pref_proxy_config_tracker_.reset(
       ProxyServiceFactory::CreatePrefProxyConfigTrackerOfLocalState(
           local_state));
   ChromeNetworkDelegate::InitializePrefsOnUIThread(
       &system_enable_referrers_,
       NULL,
       NULL,
       NULL,
       local_state);
   ssl_config_service_manager_.reset(
       ssl_config::SSLConfigServiceManager::CreateDefaultManager(
           local_state,
           BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO)));
 
   base::Value* dns_client_enabled_default = new base::FundamentalValue(
       chrome_browser_net::ConfigureAsyncDnsFieldTrial());
   local_state->SetDefaultPrefValue(prefs::kBuiltInDnsClientEnabled,
                                    dns_client_enabled_default);
   chrome_browser_net::LogAsyncDnsPrefSource(
       local_state->FindPreference(prefs::kBuiltInDnsClientEnabled));
 
   dns_client_enabled_.Init(prefs::kBuiltInDnsClientEnabled,
                            local_state,
                            base::Bind(&IOThread::UpdateDnsClientEnabled,
                                       base::Unretained(this)));
-  dns_client_enabled_.MoveToThread(
-      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO));
+  dns_client_enabled_.MoveToThread(io_thread_proxy);
 
   quick_check_enabled_.Init(prefs::kQuickCheckEnabled,
                             local_state);
-  quick_check_enabled_.MoveToThread(
-      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO));
+  quick_check_enabled_.MoveToThread(io_thread_proxy);
 
 #if defined(ENABLE_CONFIGURATION_POLICY)
   is_spdy_disabled_by_policy_ = policy_service->GetPolicies(
       policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME, std::string())).Get(
           policy::key::kDisableSpdy) != NULL;
 
   const base::Value* value = policy_service->GetPolicies(
       policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME,
       std::string())).GetValue(policy::key::kQuicAllowed);
   if (value)
@@ skipping 200 matching lines @@
   // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/466432
   // is fixed.
   tracked_objects::ScopedTracker tracking_profile10(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "466432 IOThread::InitAsync::CertPolicyEnforcer"));
   net::CertPolicyEnforcer* policy_enforcer = new net::CertPolicyEnforcer;
   globals_->cert_policy_enforcer.reset(policy_enforcer);
 
   globals_->ssl_config_service = GetSSLConfigService();
 
-  globals_->http_auth_handler_factory.reset(CreateDefaultAuthHandlerFactory(
-      globals_->host_resolver.get()));
+  CreateDefaultAuthHandlerFactory();
   globals_->http_server_properties.reset(new net::HttpServerPropertiesImpl());
   // For the ProxyScriptFetcher, we use a direct ProxyService.
   globals_->proxy_script_fetcher_proxy_service =
       net::ProxyService::CreateDirectWithNetLog(net_log_);
   // In-memory cookie store.
   // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/466432
   // is fixed.
   tracked_objects::ScopedTracker tracking_profile11(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "466432 IOThread::InitAsync::CreateCookieStore::Start"));
@@ skipping 241 matching lines @@
   registry->RegisterStringPref(prefs::kAuthAndroidNegotiateAccountType,
                                std::string());
   registry->RegisterStringPref(
       data_reduction_proxy::prefs::kDataReductionProxy, std::string());
   registry->RegisterBooleanPref(prefs::kEnableReferrers, true);
   data_reduction_proxy::RegisterPrefs(registry);
   registry->RegisterBooleanPref(prefs::kBuiltInDnsClientEnabled, true);
   registry->RegisterBooleanPref(prefs::kQuickCheckEnabled, true);
 }
 
-net::HttpAuthHandlerFactory* IOThread::CreateDefaultAuthHandlerFactory(
-    net::HostResolver* resolver) {
-  net::HttpAuthFilterWhitelist* auth_filter_default_credentials = NULL;
-  if (!auth_server_whitelist_.empty()) {
-    auth_filter_default_credentials =
-        new net::HttpAuthFilterWhitelist(auth_server_whitelist_);
+void IOThread::UpdateServerWhitelist() {
+  std::string server_whitelist = auth_server_whitelist_.GetValue();
+  if (!server_whitelist.empty()) {

[asanka, 2015/10/28 03:27:43]

It should be possible to set the whitelist to an empty string (effectively
removing any entries that were there before).

[aberent, 2015/11/02 18:52:50]

Done.

+    globals_->url_security_manager->SetDefaultWhitelist(
+        new net::HttpAuthFilterWhitelist(server_whitelist));

[asanka, 2015/10/28 03:27:43]

Let's use scoped_ptrs<> to transfer ownership like you've used elsewhere.

[aberent, 2015/11/02 18:52:50]

Done.

   }
-  net::HttpAuthFilterWhitelist* auth_filter_delegate = NULL;
-  if (!auth_delegate_whitelist_.empty()) {
-    auth_filter_delegate =
-        new net::HttpAuthFilterWhitelist(auth_delegate_whitelist_);
+}
+
+void IOThread::UpdateDelegateWhitelist() {
+  std::string delegate_whitelist = auth_delegate_whitelist_.GetValue();
+  if (!delegate_whitelist.empty()) {

[asanka, 2015/10/28 03:27:43]

Ditto. It's valid to set the whitelist to an empty string.

[aberent, 2015/11/02 18:52:50]

Done.

+    globals_->url_security_manager->SetDelegateWhitelist(
+        new net::HttpAuthFilterWhitelist(delegate_whitelist));
   }
-  globals_->url_security_manager.reset(
-      net::URLSecurityManager::Create(auth_filter_default_credentials,
-                                      auth_filter_delegate));
+}
+
+void IOThread::UpdateAndroidAuthNegotiateAccount() {
+  globals_->http_auth_handler_factory->SetAndroidAuthNegotiateAccountType(
+      make_scoped_ptr(
+          new std::string(auth_android_negotiate_account_type_.GetValue())));
+}
+
+void IOThread::UpdateNegotiateDisableCnameLookup() {
+  globals_->http_auth_handler_factory->SetNegotiateDisableCnameLookup(
+      negotiate_disable_cname_lookup_.GetValue());
+}
+
+void IOThread::UpdateNegotiateEnablePort() {
+  globals_->http_auth_handler_factory->SetNegotiateEnablePort(
+      negotiate_enable_port_.GetValue());
+}
+
+void IOThread::CreateDefaultAuthHandlerFactory() {
   std::vector<std::string> supported_schemes = base::SplitString(
       auth_schemes_, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);

[asanka, 2015/10/28 03:27:43]

Pre-existing, but change this to specify base::SPLIT_WANT_NONEMPTY. An empty
string is not a valid scheme.

[aberent, 2015/11/02 18:52:50]

Done.

-
-  scoped_ptr<net::HttpAuthHandlerRegistryFactory> registry_factory(
-      net::HttpAuthHandlerRegistryFactory::Create(
-          supported_schemes, globals_->url_security_manager.get(), resolver,
-          gssapi_library_name_, auth_android_negotiate_account_type_,
-          negotiate_disable_cname_lookup_, negotiate_enable_port_));
-  return registry_factory.release();
+  globals_->http_auth_handler_factory.reset(
+      net::HttpAuthHandlerRegistryFactory::Create(supported_schemes,
+                                                  globals_->host_resolver.get(),
+                                                  gssapi_library_name_));
+  globals_->url_security_manager.reset(
+      net::URLSecurityManager::Create(nullptr, nullptr));
+  UpdateServerWhitelist();
+  UpdateDelegateWhitelist();
+  globals_->http_auth_handler_factory->SetSecurityManager(

[asanka, 2015/10/28 03:27:43]

Let's move this up so that the Update*s are all in a group together. It should
be safe to do so.

[aberent, 2015/11/02 18:52:50]

Actually no longer applies, since now moved back into the constructor, as you
suggested elsewhere.

+      globals_->url_security_manager.get());
+  UpdateAndroidAuthNegotiateAccount();
+  UpdateNegotiateDisableCnameLookup();
+  UpdateNegotiateEnablePort();
 }
 
 void IOThread::ClearHostCache() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   net::HostCache* host_cache = globals_->host_resolver->GetHostCache();
   if (host_cache)
     host_cache->clear();
 }
 
@@ skipping 610 matching lines @@
   globals->proxy_script_fetcher_url_request_job_factory = job_factory.Pass();
 
   context->set_job_factory(
       globals->proxy_script_fetcher_url_request_job_factory.get());
 
   // TODO(rtenneti): We should probably use HttpServerPropertiesManager for the
   // system URLRequestContext too. There's no reason this should be tied to a
   // profile.
   return context;
 }