[WK2] Authentication dialog is displayed for cross-origin XHR
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Sep 2014 17:20:18 +0000 (17:20 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Sep 2014 17:20:18 +0000 (17:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=131349

Patch by Youenn Fablet <youenn.fablet@crf.canon.fr> on 2014-09-11
Reviewed by Alexey Proskuryakov.

Source/WebCore:

* WebCore.exp.in: Export of isAllowedToAskUserForCredentials.
* loader/ResourceLoader.cpp:
(WebCore::ResourceLoader::isAllowedToAskUserForCredentials): Replacing clientCredentialPolicy method. Returns true if credentials can be requested to the user.
(WebCore::ResourceLoader::didReceiveAuthenticationChallenge): Updated to use isAllowedToAskUserForCredentials.
* loader/ResourceLoader.h: Removing clientCredentialPolicy method and adding isAllowedToAskUserForCredentials method.

Source/WebKit2:

Precomputing client credential policy in the Web Process before sending the resource load task to the Network Process.

* NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::didReceiveAuthenticationChallenge): Added an ASSERT to ensure that credential policy is never set to DoNotAskClientForCrossOriginCredentials.
* WebProcess/Network/WebResourceLoadScheduler.cpp:
(WebKit::WebResourceLoadScheduler::scheduleLoad): Precomputing client credential policy to handle the case of cross-origin requests.
* WebProcess/Network/WebResourceLoader.cpp:
(WebKit::WebResourceLoader::willSendRequest): Added a TODO to check whether redirections need a specific handling.

LayoutTests:

* platform/mac-wk2/TestExpectations: Unskipped tests.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@173516 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/loader/ResourceLoader.cpp
Source/WebCore/loader/ResourceLoader.h
Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp
Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp
Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp

index 93e23db..b41bcca 100644 (file)
@@ -1,3 +1,12 @@
+2014-09-11  Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
+        [WK2] Authentication dialog is displayed for cross-origin XHR
+        https://bugs.webkit.org/show_bug.cgi?id=131349
+
+        Reviewed by Alexey Proskuryakov.
+
+        * platform/mac-wk2/TestExpectations: Unskipped tests.
+
 2014-09-11  Chris Fleizach  <cfleizach@apple.com>
 
         AX: Children inside a <legend> are not accessible
index ac76c09..98e970e 100644 (file)
@@ -341,10 +341,6 @@ webkit.org/b/125996 platform/mac/accessibility/search-when-element-starts-in-tab
 
 webkit.org/b/127960 [ MountainLion ] http/tests/security/cross-origin-plugin-private-browsing-toggled.html [ Pass Failure ]
 
-webkit.org/b/131349 http/tests/xmlhttprequest/access-control-preflight-credential-async.html [ Failure ]
-webkit.org/b/131349 http/tests/xmlhttprequest/cross-origin-no-authorization.html [ Failure ]
-webkit.org/b/131349 http/tests/xmlhttprequest/cross-origin-no-credential-prompt.html [ Failure ]
-
 webkit.org/b/134550 [ Mavericks ] http/tests/cache/iframe-304-crash.html [ Pass Failure ]
 
 # Subpixel wrong cliprect on WK2
index 1652974..e5962c6 100644 (file)
@@ -1,3 +1,16 @@
+2014-09-11  Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
+        [WK2] Authentication dialog is displayed for cross-origin XHR
+        https://bugs.webkit.org/show_bug.cgi?id=131349
+
+        Reviewed by Alexey Proskuryakov.
+
+        * WebCore.exp.in: Export of isAllowedToAskUserForCredentials.
+        * loader/ResourceLoader.cpp:
+        (WebCore::ResourceLoader::isAllowedToAskUserForCredentials): Replacing clientCredentialPolicy method. Returns true if credentials can be requested to the user.
+        (WebCore::ResourceLoader::didReceiveAuthenticationChallenge): Updated to use isAllowedToAskUserForCredentials.
+        * loader/ResourceLoader.h: Removing clientCredentialPolicy method and adding isAllowedToAskUserForCredentials method.
+
 2014-09-11  Chris Fleizach  <cfleizach@apple.com>
 
         AX: Children inside a <legend> are not accessible
index 7262e73..8d1fd40 100644 (file)
@@ -1693,6 +1693,7 @@ __ZNK7WebCore14ResourceBuffer4sizeEv
 __ZNK7WebCore14ResourceBuffer7isEmptyEv
 __ZNK7WebCore14ResourceHandle10connectionEv
 __ZNK7WebCore14ResourceLoader11frameLoaderEv
+__ZNK7WebCore14ResourceLoader32isAllowedToAskUserForCredentialsEv
 __ZNK7WebCore14ScrollableArea13scrolledToTopEv
 __ZNK7WebCore14ScrollableArea14scrollAnimatorEv
 __ZNK7WebCore14ScrollableArea14scrolledToLeftEv
index 7aa4d87..2c81fa3 100644 (file)
@@ -538,6 +538,11 @@ bool ResourceLoader::shouldUseCredentialStorage()
     return frameLoader()->client().shouldUseCredentialStorage(documentLoader(), identifier());
 }
 
+bool ResourceLoader::isAllowedToAskUserForCredentials() const
+{
+    return m_options.clientCredentialPolicy() == AskClientForAllCredentials || (m_options.clientCredentialPolicy() == DoNotAskClientForCrossOriginCredentials && m_frame->document()->securityOrigin()->canRequest(originalRequest().url()));
+}
+
 void ResourceLoader::didReceiveAuthenticationChallenge(const AuthenticationChallenge& challenge)
 {
     ASSERT(m_handle->hasAuthenticationChallenge());
@@ -547,7 +552,7 @@ void ResourceLoader::didReceiveAuthenticationChallenge(const AuthenticationChall
     Ref<ResourceLoader> protect(*this);
 
     if (m_options.allowCredentials() == AllowStoredCredentials) {
-        if (m_options.clientCredentialPolicy() == AskClientForAllCredentials || (m_options.clientCredentialPolicy() == DoNotAskClientForCrossOriginCredentials && m_frame->document()->securityOrigin()->canRequest(originalRequest().url()))) {
+        if (isAllowedToAskUserForCredentials()) {
             frameLoader()->notifier().didReceiveAuthenticationChallenge(this, challenge);
             return;
         }
index 2bf7537..e37f0c2 100644 (file)
@@ -122,10 +122,11 @@ public:
     bool shouldSendResourceLoadCallbacks() const { return m_options.sendLoadCallbacks() == SendCallbacks; }
     void setSendCallbackPolicy(SendCallbackPolicy sendLoadCallbacks) { m_options.setSendLoadCallbacks(sendLoadCallbacks); }
     bool shouldSniffContent() const { return m_options.sniffContent() == SniffContent; }
-    ClientCredentialPolicy clientCredentialPolicy() const { return m_options.clientCredentialPolicy(); }
+    WEBCORE_EXPORT bool isAllowedToAskUserForCredentials() const;
 
     bool reachedTerminalState() const { return m_reachedTerminalState; }
 
+
     const ResourceRequest& request() const { return m_request; }
 
     void setDataBufferingPolicy(DataBufferingPolicy);
index 19dc3cf..6c65139 100644 (file)
@@ -1,3 +1,19 @@
+2014-09-11  Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
+        [WK2] Authentication dialog is displayed for cross-origin XHR
+        https://bugs.webkit.org/show_bug.cgi?id=131349
+
+        Reviewed by Alexey Proskuryakov.
+
+        Precomputing client credential policy in the Web Process before sending the resource load task to the Network Process.
+
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::didReceiveAuthenticationChallenge): Added an ASSERT to ensure that credential policy is never set to DoNotAskClientForCrossOriginCredentials.
+        * WebProcess/Network/WebResourceLoadScheduler.cpp:
+        (WebKit::WebResourceLoadScheduler::scheduleLoad): Precomputing client credential policy to handle the case of cross-origin requests.
+        * WebProcess/Network/WebResourceLoader.cpp:
+        (WebKit::WebResourceLoader::willSendRequest): Added a TODO to check whether redirections need a specific handling.
+
 2014-09-11  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] Merge WebKitAuthenticationWidget into WebKitAuthenticationDialog
index 6b87a05..2edbdf9 100644 (file)
@@ -357,10 +357,9 @@ bool NetworkResourceLoader::shouldUseCredentialStorage(ResourceHandle* handle)
 void NetworkResourceLoader::didReceiveAuthenticationChallenge(ResourceHandle* handle, const AuthenticationChallenge& challenge)
 {
     ASSERT_UNUSED(handle, handle == m_handle);
+    // NetworkResourceLoader does not know whether the request is cross origin, so Web process computes an applicable credential policy for it.
+    ASSERT(m_parameters.clientCredentialPolicy != DoNotAskClientForCrossOriginCredentials);
 
-    // FIXME (http://webkit.org/b/115291): Since we go straight to the UI process for authentication we don't get WebCore's
-    // cross-origin check before asking the client for credentials.
-    // Therefore we are too permissive in the case where the ClientCredentialPolicy is DoNotAskClientForCrossOriginCredentials.
     if (m_parameters.clientCredentialPolicy == DoNotAskClientForAnyCredentials) {
         challenge.authenticationClient()->receivedRequestToContinueWithoutCredential(challenge);
         return;
index c55802f..71c8889 100644 (file)
@@ -169,7 +169,7 @@ void WebResourceLoadScheduler::scheduleLoad(ResourceLoader* resourceLoader, Cach
     loadParameters.contentSniffingPolicy = contentSniffingPolicy;
     loadParameters.allowStoredCredentials = allowStoredCredentials;
     // If there is no WebFrame then this resource cannot be authenticated with the client.
-    loadParameters.clientCredentialPolicy = (webFrame && webPage) ? resourceLoader->clientCredentialPolicy() : DoNotAskClientForAnyCredentials;
+    loadParameters.clientCredentialPolicy = (webFrame && webPage && resourceLoader->isAllowedToAskUserForCredentials()) ? AskClientForAllCredentials : DoNotAskClientForAnyCredentials;
     loadParameters.shouldClearReferrerOnHTTPSToHTTPRedirect = shouldClearReferrerOnHTTPSToHTTPRedirect;
     loadParameters.isMainResource = resource && resource->type() == CachedResource::MainResource;
     loadParameters.defersLoading = resourceLoader->defersLoading();
index 47045a8..bcc8dab 100644 (file)
@@ -89,6 +89,7 @@ void WebResourceLoader::willSendRequest(const ResourceRequest& proposedRequest,
     ResourceRequest newRequest = proposedRequest;
     if (m_coreLoader->documentLoader()->applicationCacheHost()->maybeLoadFallbackForRedirect(m_coreLoader.get(), newRequest, redirectResponse))
         return;
+    // FIXME: Do we need to update NetworkResourceLoader clientCredentialPolicy in case loader policy is DoNotAskClientForCrossOriginCredentials?
     m_coreLoader->willSendRequest(newRequest, redirectResponse);
     
     if (!m_coreLoader)