XMLHttpRequest without credentials fails with CORS error on redirections

https://bugs.webkit.org/show_bug.cgi?id=276364

Reviewed by NOBODY (OOPS!).

XMLHTTPRequest without credentials to the same-origin, which redirects to a cross-origin and
then back to the same-origin with Access-Control-Allow-Origin=*, fails with error CORS policy:
"Cannot use wildcard in Access-Control-Allow-Origin when credentials flag is true."

This change fixes the problem. It allows to make a cross-origin XMLHTTPRequest without credentials to
different origin with response Access-Control-Allow-Origin=*. The specification:
https://fetch.spec.whatwg.org/#cors-protocol-and-credentials says that only if credentials mode is "include",
then `Access-Control-Allow-Origin` cannot be `*`.

Added test case which tests this case.

* LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-expected.txt:
* LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects.html:
* LayoutTests/platform/mac-wk1/http/tests/xmlhttprequest/access-control-and-redirects-expected.txt:
* Source/WebCore/loader/CrossOriginAccessControl.cpp:
(WebCore::passesAccessControlCheck):
(WebCore::validatePreflightResponse):
* Source/WebCore/loader/CrossOriginAccessControl.h:
* Source/WebCore/loader/CrossOriginPreflightChecker.cpp:
(WebCore::CrossOriginPreflightChecker::validatePreflightResponse):
* Source/WebCore/loader/DocumentThreadableLoader.cpp:
(WebCore::DocumentThreadableLoader::loadRequest):
* Source/WebCore/loader/SubresourceLoader.cpp:
(WebCore::SubresourceLoader::checkResponseCrossOriginAccessControl):
(WebCore::SubresourceLoader::checkRedirectionCrossOriginAccessControl):
* Source/WebCore/loader/cache/CachedResource.cpp:

Author: pgorszkowski-igalia

LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-expected.txt

@@ -30,4 +30,12 @@ PASS: NetworkError:  A network error occurred.
 Testing http://localhost:8000/resources/redirect.py?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi(async)
 Expecting success: false
 PASS: 0
+Testing resources/redirect-cors.py?access-control-allow-origin=*&url=http://localhost:8000/xmlhttprequest/resources/redirect-cors.py?access-control-allow-origin=*%26url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi (sync)
+Expecting success: true
+PASS: PASS: Cross-domain access allowed.
+
+Testing resources/redirect-cors.py?access-control-allow-origin=*&url=http://localhost:8000/xmlhttprequest/resources/redirect-cors.py?access-control-allow-origin=*%26url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi(async)
+Expecting success: true
+PASS: PASS: Cross-domain access allowed.
+
 

LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects.html

@@ -47,7 +47,8 @@
 var tests = [
     ["/resources/redirect.py?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi", true, true],
     ["http://localhost:8000/resources/redirect.py?url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow.cgi", false, false],
-    ["http://localhost:8000/resources/redirect.py?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi", false, false]
+    ["http://localhost:8000/resources/redirect.py?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi", false, false],
+    ["resources/redirect-cors.py?access-control-allow-origin=*&url=http://localhost:8000/xmlhttprequest/resources/redirect-cors.py?access-control-allow-origin=*%26url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi", true, true]
 ]
 
 var currentTest = 0;

LayoutTests/platform/mac-wk1/http/tests/xmlhttprequest/access-control-and-redirects-expected.txt

@@ -27,4 +27,11 @@ PASS: NetworkError:  A network error occurred.
 Testing http://localhost:8000/resources/redirect.py?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi(async)
 Expecting success: false
 PASS: 0
+Testing resources/redirect-cors.py?access-control-allow-origin=*&url=http://localhost:8000/xmlhttprequest/resources/redirect-cors.py?access-control-allow-origin=*%26url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi (sync)
+Expecting success: true
+FAIL: NetworkError:  A network error occurred.
+Testing resources/redirect-cors.py?access-control-allow-origin=*&url=http://localhost:8000/xmlhttprequest/resources/redirect-cors.py?access-control-allow-origin=*%26url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi(async)
+Expecting success: true
+PASS: PASS: Cross-domain access allowed.
+
 

Source/WebCore/loader/CrossOriginAccessControl.cpp

@@ -240,41 +240,43 @@ bool CrossOriginAccessControlCheckDisabler::crossOriginAccessControlCheckEnabled
     return m_accessControlCheckEnabled;
 }
 
-Expected<void, String> passesAccessControlCheck(const ResourceResponse& response, StoredCredentialsPolicy storedCredentialsPolicy, const SecurityOrigin& securityOrigin, const CrossOriginAccessControlCheckDisabler* checkDisabler)
+Expected<void, String> passesAccessControlCheck(const ResourceResponse& response, FetchOptions::Credentials fetchOptionsCredentials, const SecurityOrigin& securityOrigin, const CrossOriginAccessControlCheckDisabler* checkDisabler)
 {
-    // A wildcard Access-Control-Allow-Origin can not be used if credentials are to be sent,
-    // even with Access-Control-Allow-Credentials set to true.
     const String& accessControlOriginString = response.httpHeaderField(HTTPHeaderName::AccessControlAllowOrigin);
-    bool starAllowed = storedCredentialsPolicy == StoredCredentialsPolicy::DoNotUse;
-    if (!starAllowed)
-        starAllowed = checkDisabler && !checkDisabler->crossOriginAccessControlCheckEnabled();
-    if (accessControlOriginString == "*"_s && starAllowed)
-        return { };
-
-    String securityOriginString = securityOrigin.toString();
-    if (accessControlOriginString != securityOriginString) {
-        if (accessControlOriginString == "*"_s)
+    const String securityOriginString = securityOrigin.toString();
+    if (accessControlOriginString == "*"_s) {
+        if (checkDisabler && !checkDisabler->crossOriginAccessControlCheckEnabled())
+            return { };
+        // A wildcard Access-Control-Allow-Origin can not be used if credentials are to be sent,
+        // even with Access-Control-Allow-Credentials set to true.
+        // https://fetch.spec.whatwg.org/#cors-protocol-and-credentials.
+        if (fetchOptionsCredentials == FetchOptions::Credentials::Include)
             return makeUnexpected("Cannot use wildcard in Access-Control-Allow-Origin when credentials flag is true."_s);
+    } else if (accessControlOriginString != securityOriginString) {
+        // Multiple origins are not allowed in the allow origin header.
+        // https://fetch.spec.whatwg.org/#http-access-control-allow-origin.
         if (accessControlOriginString.find(',') != notFound)
             return makeUnexpected("Access-Control-Allow-Origin cannot contain more than one origin."_s);
+
         return makeUnexpected(makeString("Origin ", securityOriginString, " is not allowed by Access-Control-Allow-Origin.", " Status code: ", response.httpStatusCode()));
     }
 
-    if (storedCredentialsPolicy == StoredCredentialsPolicy::Use) {
+    if (fetchOptionsCredentials == FetchOptions::Credentials::Include) {
         const String& accessControlCredentialsString = response.httpHeaderField(HTTPHeaderName::AccessControlAllowCredentials);
+        // https://fetch.spec.whatwg.org/#http-access-control-allow-credentials.
         if (accessControlCredentialsString != "true"_s)
             return makeUnexpected("Credentials flag is true, but Access-Control-Allow-Credentials is not \"true\"."_s);
     }
 
     return { };
 }
 
-Expected<void, String> validatePreflightResponse(PAL::SessionID sessionID, const ResourceRequest& request, const ResourceResponse& response, StoredCredentialsPolicy storedCredentialsPolicy, const SecurityOrigin& securityOrigin, const CrossOriginAccessControlCheckDisabler* checkDisabler)
+Expected<void, String> validatePreflightResponse(PAL::SessionID sessionID, const ResourceRequest& request, const ResourceResponse& response, FetchOptions::Credentials fetchOptionsCredentials, StoredCredentialsPolicy storedCredentialsPolicy, const SecurityOrigin& securityOrigin, const CrossOriginAccessControlCheckDisabler* checkDisabler)
 {
     if (!response.isSuccessful())
         return makeUnexpected(makeString("Preflight response is not successful. Status code: ", response.httpStatusCode()));
 
-    auto accessControlCheckResult = passesAccessControlCheck(response, storedCredentialsPolicy, securityOrigin, checkDisabler);
+    auto accessControlCheckResult = passesAccessControlCheck(response, fetchOptionsCredentials, securityOrigin, checkDisabler);
     if (!accessControlCheckResult)
         return accessControlCheckResult;
 

Source/WebCore/loader/CrossOriginAccessControl.h

@@ -26,6 +26,7 @@
 
 #pragma once
 
+#include "FetchOptions.h"
 #include "HTTPHeaderNames.h"
 #include "ReferrerPolicy.h"
 #include "StoredCredentialsPolicy.h"
@@ -84,8 +85,8 @@ class WEBCORE_EXPORT CrossOriginAccessControlCheckDisabler {
     bool m_accessControlCheckEnabled { true };
 };
 
-WEBCORE_EXPORT Expected<void, String> passesAccessControlCheck(const ResourceResponse&, StoredCredentialsPolicy, const SecurityOrigin&, const CrossOriginAccessControlCheckDisabler*);
-WEBCORE_EXPORT Expected<void, String> validatePreflightResponse(PAL::SessionID, const ResourceRequest&, const ResourceResponse&, StoredCredentialsPolicy, const SecurityOrigin&, const CrossOriginAccessControlCheckDisabler*);
+WEBCORE_EXPORT Expected<void, String> passesAccessControlCheck(const ResourceResponse&, FetchOptions::Credentials, const SecurityOrigin&, const CrossOriginAccessControlCheckDisabler*);
+WEBCORE_EXPORT Expected<void, String> validatePreflightResponse(PAL::SessionID, const ResourceRequest&, const ResourceResponse&, FetchOptions::Credentials, StoredCredentialsPolicy, const SecurityOrigin&, const CrossOriginAccessControlCheckDisabler*);
 
 enum class ForNavigation : bool { No, Yes };
 WEBCORE_EXPORT std::optional<ResourceError> validateCrossOriginResourcePolicy(CrossOriginEmbedderPolicyValue, const SecurityOrigin&, const URL&, const ResourceResponse&, ForNavigation);

Source/WebCore/loader/CrossOriginPreflightChecker.cpp

@@ -72,7 +72,7 @@ void CrossOriginPreflightChecker::validatePreflightResponse(DocumentThreadableLo
         return;
     }
 
-    auto result = WebCore::validatePreflightResponse(page->sessionID(), request, response, loader.options().storedCredentialsPolicy, loader.securityOrigin(), &CrossOriginAccessControlCheckDisabler::singleton());
+    auto result = WebCore::validatePreflightResponse(page->sessionID(), request, response, loader.options().credentials, loader.options().storedCredentialsPolicy, loader.securityOrigin(), &CrossOriginAccessControlCheckDisabler::singleton());
     if (!result) {
         loader.document().addConsoleMessage(MessageSource::Security, MessageLevel::Error, result.error());
         loader.preflightFailure(identifier, ResourceError(errorDomainWebKitInternal, 0, request.url(), result.error(), ResourceError::Type::AccessControl));

Source/WebCore/loader/DocumentThreadableLoader.cpp

@@ -658,7 +658,7 @@ void DocumentThreadableLoader::loadRequest(ResourceRequest&& request, SecurityCh
             else {
                 ASSERT(m_options.mode == FetchOptions::Mode::Cors);
                 response.setTainting(ResourceResponse::Tainting::Cors);
-                auto accessControlCheckResult = passesAccessControlCheck(response, m_options.storedCredentialsPolicy, securityOrigin(), &CrossOriginAccessControlCheckDisabler::singleton());
+                auto accessControlCheckResult = passesAccessControlCheck(response, m_options.credentials, securityOrigin(), &CrossOriginAccessControlCheckDisabler::singleton());
                 if (!accessControlCheckResult) {
                     logErrorAndFail(ResourceError(errorDomainWebKitInternal, 0, response.url(), accessControlCheckResult.error(), ResourceError::Type::AccessControl));
                     return;

Source/WebCore/loader/SubresourceLoader.cpp

@@ -639,7 +639,7 @@ Expected<void, String> SubresourceLoader::checkResponseCrossOriginAccessControl(
 
     ASSERT(m_origin);
 
-    return passesAccessControlCheck(response, options().credentials == FetchOptions::Credentials::Include ? StoredCredentialsPolicy::Use : StoredCredentialsPolicy::DoNotUse, *m_origin, &CrossOriginAccessControlCheckDisabler::singleton());
+    return passesAccessControlCheck(response, options().credentials, *m_origin, &CrossOriginAccessControlCheckDisabler::singleton());
 }
 
 Expected<void, String> SubresourceLoader::checkRedirectionCrossOriginAccessControl(const ResourceRequest& previousRequest, const ResourceResponse& redirectResponse, ResourceRequest& newRequest)
@@ -663,7 +663,7 @@ Expected<void, String> SubresourceLoader::checkRedirectionCrossOriginAccessContr
 
         ASSERT(m_origin);
         if (crossOriginFlag) {
-            auto accessControlCheckResult = passesAccessControlCheck(redirectResponse, options().storedCredentialsPolicy, *m_origin, &CrossOriginAccessControlCheckDisabler::singleton());
+            auto accessControlCheckResult = passesAccessControlCheck(redirectResponse, options().credentials, *m_origin, &CrossOriginAccessControlCheckDisabler::singleton());
             if (!accessControlCheckResult)
                 return accessControlCheckResult;
         }

Source/WebCore/loader/cache/CachedResource.cpp

@@ -305,7 +305,7 @@ void CachedResource::loadFrom(const CachedResource& resource)
 
     if (isCrossOrigin() && m_options.mode == FetchOptions::Mode::Cors) {
         ASSERT(m_origin);
-        auto accessControlCheckResult = WebCore::passesAccessControlCheck(resource.response(), m_options.storedCredentialsPolicy, *m_origin, &CrossOriginAccessControlCheckDisabler::singleton());
+        auto accessControlCheckResult = WebCore::passesAccessControlCheck(resource.response(), m_options.credentials, *m_origin, &CrossOriginAccessControlCheckDisabler::singleton());
         if (!accessControlCheckResult) {
             setResourceError(ResourceError(String(), 0, url(), accessControlCheckResult.error(), ResourceError::Type::AccessControl));
             return;

Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp

@@ -142,7 +142,7 @@ void NetworkCORSPreflightChecker::didCompleteWithError(const WebCore::ResourceEr
 
     CORS_CHECKER_RELEASE_LOG("didComplete http_status_code=%d", m_response.httpStatusCode());
 
-    auto result = validatePreflightResponse(m_parameters.sessionID, m_parameters.originalRequest, m_response, m_parameters.storedCredentialsPolicy, m_parameters.sourceOrigin, m_networkResourceLoader.get());
+    auto result = validatePreflightResponse(m_parameters.sessionID, m_parameters.originalRequest, m_response, m_parameters.fetchOptionsCredentials, m_parameters.storedCredentialsPolicy, m_parameters.sourceOrigin, m_networkResourceLoader.get());
     if (!result) {
         CORS_CHECKER_RELEASE_LOG("didComplete, AccessControl error: %s", result.error().utf8().data());
         m_completionCallback(ResourceError { errorDomainWebKitInternal, 0, m_parameters.originalRequest.url(), result.error(), ResourceError::Type::AccessControl });