Allow dynamic updating of authentication policies

chrome/browser/io_thread.cc

Index: chrome/browser/io_thread.cc
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index 14f405c0f0e0046875a805250df3b4ac7a832343..2ebe0128a0be9011aff165e004faa06651cd850c 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -458,16 +458,32 @@ IOThread::IOThread(
       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));
@@ -493,13 +509,11 @@ IOThread::IOThread(
                            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(
@@ -720,8 +734,7 @@ void IOThread::Init() {
 
   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 =
@@ -983,30 +996,54 @@ void IOThread::RegisterPrefs(PrefRegistrySimple* registry) {
   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() {