Get rid of net::DO_NOT_PROMPT_FOR_LOGIN

content/browser/loader/resource_dispatcher_host_impl.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.
 
 // See http://dev.chromium.org/developers/design-documents/multi-process-resource-loading
 
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 
 #include <algorithm>
 #include <set>
@@ skipping 577 matching lines @@
 }
 
 DownloadInterruptReason ResourceDispatcherHostImpl::BeginDownload(
     scoped_ptr<net::URLRequest> request,
     const Referrer& referrer,
     bool is_content_initiated,
     ResourceContext* context,
     int child_id,
     int route_id,
     bool prefer_cache,
+    bool do_not_prompt_for_login,
     scoped_ptr<DownloadSaveInfo> save_info,
     uint32 download_id,
     const DownloadStartedCallback& started_callback) {
   if (is_shutdown_)
     return CallbackAndReturn(started_callback,
                              DOWNLOAD_INTERRUPT_REASON_USER_SHUTDOWN);
 
   const GURL& url = request->original_url();
 
   // http://crbug.com/90971
@@ skipping 42 matching lines @@
   const net::URLRequestContext* request_context = context->GetRequestContext();
   if (!request_context->job_factory()->IsHandledURL(url)) {
     VLOG(1) << "Download request for unsupported protocol: "
             << url.possibly_invalid_spec();
     return CallbackAndReturn(started_callback,
                              DOWNLOAD_INTERRUPT_REASON_NETWORK_INVALID_REQUEST);
   }
 
   ResourceRequestInfoImpl* extra_info =
       CreateRequestInfo(child_id, route_id, true, context);
+  extra_info->set_do_not_prompt_for_login(do_not_prompt_for_login);
   extra_info->AssociateWithRequest(request.get());  // Request takes ownership.
 
   if (request->url().SchemeIs(url::kBlobScheme)) {
     ChromeBlobStorageContext* blob_context =
         GetChromeBlobStorageContextForResourceContext(context);
     storage::BlobProtocolHandler::SetRequestedBlobDataHandle(
         request.get(),
         blob_context->context()->GetBlobDataFromPublicURL(request->url()));
   }
 
@@ skipping 515 matching lines @@
         net::URLRequest::UPDATE_FIRST_PARTY_URL_ON_REDIRECT);
   }
 
   const Referrer referrer(request_data.referrer, request_data.referrer_policy);
   SetReferrerForRequest(new_request.get(), referrer);
 
   net::HttpRequestHeaders headers;
   headers.AddHeadersFromString(request_data.headers);
   new_request->SetExtraRequestHeaders(headers);
 
-  new_request->SetLoadFlags(load_flags);
-
   storage::BlobStorageContext* blob_context =
       GetBlobStorageContext(filter_->blob_storage_context());
   // Resolve elements from request_body and prepare upload data.
   if (request_data.request_body.get()) {
     // |blob_context| could be null when the request is from the plugins because
     // ResourceMessageFilters created in PluginProcessHost don't have the blob
     // context.
     if (blob_context) {
       // Attaches the BlobDataHandles to request_body not to free the blobs and
       // any attached shareable files until upload completion. These data will
       // be used in UploadDataStream and ServiceWorkerURLRequestJob.
       AttachRequestBodyBlobDataHandles(
           request_data.request_body.get(),
           blob_context);
     }
     new_request->set_upload(UploadDataStreamBuilder::Build(
         request_data.request_body.get(),
         blob_context,
         filter_->file_system_context(),
         BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE)
             .get()));
   }
 
   bool allow_download = request_data.allow_download &&
       IsResourceTypeFrame(request_data.resource_type);

[mmenke, 2014/12/10 19:28:30]

Suggest moving the load_flags and is_sync_load stuff down here, too, to keep
related code together.  Also, the DCHECKs on what flags are set should probably
be just before the SetLoadFlags line, now that we're modifying flags here, too.

[baranovich, 2014/12/10 22:24:38]

Done.

+  bool do_not_prompt_for_login =
+      request_data.resource_type == RESOURCE_TYPE_PREFETCH ||
+      request_data.resource_type == RESOURCE_TYPE_FAVICON;

[mmenke, 2014/12/10 19:28:30]

BUG:  You need request_data.do_not_prompt_for_login in the mix, too.  The fact
this didn't make any tests fail is a bit concerning.

I think this may be a little clearer, than just adding another || clause:

bool do_not_prompt_for_login = request_data.do_not_prompt_for_login;
if (... || ...)
  do_not_prompt_for_login = true;

[baranovich, 2014/12/10 22:24:38]

I didn't found any tests (at least in RDH browsertests/unittests) on that
prompting/non-prompting issue btw.

[baranovich, 2014/12/10 22:24:38]

Done.

+  if (request_data.resource_type == RESOURCE_TYPE_IMAGE &&
+      HTTP_AUTH_RELATION_BLOCKED_CROSS ==
+          HttpAuthRelationTypeOf(request_data.url,
+                                 request_data.first_party_for_cookies)) {
+    // Prevent third-party image content from prompting for login, as this
+    // is often a scam to extract credentials for another domain from the user.
+    // Only block image loads, as the attack applies largely to the "src"
+    // property of the <img> tag. It is common for web properties to allow
+    // untrusted values for <img src>; this is considered a fair thing for an
+    // HTML sanitizer to do. Conversely, any HTML sanitizer that didn't
+    // filter sources for <script>, <link>, <embed>, <object>, <iframe> tags
+    // would be considered vulnerable in and of itself.
+    do_not_prompt_for_login = true;
+    load_flags |= net::LOAD_DO_NOT_USE_EMBEDDED_IDENTITY;

[baranovich, 2014/12/09 23:33:49]

not sure if it's the best decision. On the one hand, I want to keep the things
together, since they're together:) on the other hand, there is
BuildLoadFlagsForRequest() ti build request flags.

[mmenke, 2014/12/10 19:28:30]

It's certainly a bit ugly, but I don't have a better suggestion.

+  }
+
+  new_request->SetLoadFlags(load_flags);
 
   // Make extra info and read footer (contains request ID).
   ResourceRequestInfoImpl* extra_info =
       new ResourceRequestInfoImpl(
           process_type,
           child_id,
           route_id,
           request_data.origin_pid,
           request_id,
           request_data.render_frame_id,
           request_data.is_main_frame,
           request_data.parent_is_main_frame,
           request_data.parent_render_frame_id,
           request_data.resource_type,
           request_data.transition_type,
           request_data.should_replace_current_entry,
           false,  // is download
           false,  // is stream
           allow_download,
           request_data.has_user_gesture,
           request_data.enable_load_timing,
           request_data.enable_upload_progress,
+          do_not_prompt_for_login,
           request_data.referrer_policy,
           request_data.visiblity_state,
           resource_context,
           filter_->GetWeakPtr(),
           !is_sync_load);
   // Request takes ownership.
   extra_info->AssociateWithRequest(new_request.get());
 
   if (new_request->url().SchemeIs(url::kBlobScheme)) {
     // Hang on to a reference to ensure the blob is not released prior
@@ skipping 225 matching lines @@
     int route_id,
     bool download,
     ResourceContext* context) {
   return new ResourceRequestInfoImpl(
       PROCESS_TYPE_RENDERER,
       child_id,
       route_id,
       0,
       request_id_,
       MSG_ROUTING_NONE,  // render_frame_id
-      false,     // is_main_frame
-      false,     // parent_is_main_frame
-      -1,        // parent_render_frame_id
+      false,             // is_main_frame
+      false,             // parent_is_main_frame
+      -1,                // parent_render_frame_id
       RESOURCE_TYPE_SUB_RESOURCE,
       ui::PAGE_TRANSITION_LINK,
       false,     // should_replace_current_entry
       download,  // is_download
       false,     // is_stream
       download,  // allow_download
       false,     // has_user_gesture
       false,     // enable_load_timing
       false,     // enable_upload_progress
+      false,     // do_not_prompt_for_login
       blink::WebReferrerPolicyDefault,
       blink::WebPageVisibilityStateVisible,
       context,
       base::WeakPtr<ResourceMessageFilter>(),  // filter
       true);     // is_async
 }
 
 void ResourceDispatcherHostImpl::OnRenderViewHostCreated(int child_id,
                                                          int route_id,
                                                          bool is_visible,
@@ skipping 434 matching lines @@
           params.transition,
           // should_replace_current_entry. This was only maintained at layer for
           // request transfers and isn't needed for browser-side navigations.
           false,
           false,  // is download
           false,  // is stream
           params.allow_download,
           info.navigation_params.has_user_gesture,
           true,   // enable_load_timing
           false,  // enable_upload_progress
+          false,  // do_not_prompt_for_login
           params.referrer.policy,
           // TODO(davidben): This is only used for prerenders. Replace
           // is_showing with something for that. Or maybe it just comes from the
           // same mechanism as the cookie one.
           blink::WebPageVisibilityStateVisible,
           resource_context,
           base::WeakPtr<ResourceMessageFilter>(),  // filter
           true);
   // Request takes ownership.
   extra_info->AssociateWithRequest(new_request.get());
@@ skipping 354 matching lines @@
   // Although EV status is irrelevant to sub-frames and sub-resources, we have
   // to perform EV certificate verification on all resources because an HTTP
   // keep-alive connection created to load a sub-frame or a sub-resource could
   // be reused to load a main frame.
   load_flags |= net::LOAD_VERIFY_EV_CERT;
   if (request_data.resource_type == RESOURCE_TYPE_MAIN_FRAME) {
     load_flags |= net::LOAD_MAIN_FRAME;
   } else if (request_data.resource_type == RESOURCE_TYPE_SUB_FRAME) {
     load_flags |= net::LOAD_SUB_FRAME;
   } else if (request_data.resource_type == RESOURCE_TYPE_PREFETCH) {
-    load_flags |= (net::LOAD_PREFETCH | net::LOAD_DO_NOT_PROMPT_FOR_LOGIN);
-  } else if (request_data.resource_type == RESOURCE_TYPE_FAVICON) {
-    load_flags |= net::LOAD_DO_NOT_PROMPT_FOR_LOGIN;
-  } else if (request_data.resource_type == RESOURCE_TYPE_IMAGE) {
-    // Prevent third-party image content from prompting for login, as this
-    // is often a scam to extract credentials for another domain from the user.
-    // Only block image loads, as the attack applies largely to the "src"
-    // property of the <img> tag. It is common for web properties to allow
-    // untrusted values for <img src>; this is considered a fair thing for an
-    // HTML sanitizer to do. Conversely, any HTML sanitizer that didn't
-    // filter sources for <script>, <link>, <embed>, <object>, <iframe> tags
-    // would be considered vulnerable in and of itself.
-    HttpAuthRelationType relation_type = HttpAuthRelationTypeOf(
-        request_data.url, request_data.first_party_for_cookies);
-    if (relation_type == HTTP_AUTH_RELATION_BLOCKED_CROSS) {
-      load_flags |= (net::LOAD_DO_NOT_USE_EMBEDDED_IDENTITY |
-                     net::LOAD_DO_NOT_PROMPT_FOR_LOGIN);
-    }
+    load_flags |= net::LOAD_PREFETCH;
   }
 
   if (is_sync_load)
     load_flags |= net::LOAD_IGNORE_LIMITS;
 
   ChildProcessSecurityPolicyImpl* policy =
       ChildProcessSecurityPolicyImpl::GetInstance();
   if (!policy->CanSendCookiesForOrigin(child_id, request_data.url)) {
     load_flags |= (net::LOAD_DO_NOT_SEND_COOKIES |
                    net::LOAD_DO_NOT_SEND_AUTH_DATA |
@@ skipping 10 matching lines @@
 
   // Add a flag to selectively bypass the data reduction proxy if the resource
   // type is not an image.
   if (request_data.resource_type != RESOURCE_TYPE_IMAGE)
     load_flags |= net::LOAD_BYPASS_DATA_REDUCTION_PROXY;
 
   return load_flags;
 }
 
 }  // namespace content