Title: [233387] trunk
Revision
233387
Author
dba...@webkit.org
Date
2018-06-29 19:11:01 -0700 (Fri, 29 Jun 2018)

Log Message

REGRESSION (r230921): Cannot log in to forums.swift.org using GitHub account
https://bugs.webkit.org/show_bug.cgi?id=187197
<rdar://problem/40420821>

Reviewed by Brent Fulgham.

Source/WebCore:

Fixes an issue where a Same-Site cookies are not sent with any child window load if the
load is cross-origin with respect to the window's opener. One example where this issue
manifest itself was in the GitHub sign in flow on forums.swift.org.

Currently we always consider the origin of the window's opener (if we have one) when
determining whether a frame load request is same-origin and hence should send Same-Site
cookies when performing the request. So, when page A.com opens a child window to B.com and
then a person clicks a hyperlink or submits a form to B.com/b2 then we do not send Same-
Site cookies with the request to B.com/b2 (because its origin, B.com, is cross-origin
with its opener, A.com). But we should send Same-Site cookies with the request to B.com/b2
because it is same-origin with the page that initiated the request, B.com. Instead of
always considering the origin the window's opener for every frame load we should only
consider it for the first non-empty document load.

Tests: http/tests/cookies/same-site/fetch-in-about-blank-popup.html
       http/tests/cookies/same-site/post-from-cross-site-popup.html

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::addExtraFieldsToRequest):

LayoutTests:

Add tests to ensure that Same-Site cookies are sent in a subsequent same-origin form submission
from a child window that is cross-origin with its opener. Also add a test to ensure that Same-Site
cookies are visible to an about:blank pop-up window (as about:blank is Same-Site with its opener
by definition of being same-origin with it).

* http/tests/cookies/same-site/fetch-in-about-blank-popup-expected.txt: Added.
* http/tests/cookies/same-site/fetch-in-about-blank-popup.html: Added.
* http/tests/cookies/same-site/post-from-cross-site-popup-expected.txt: Added.
* http/tests/cookies/same-site/post-from-cross-site-popup.html: Added.
* http/tests/cookies/same-site/resources/post-from-popup.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (233386 => 233387)


--- trunk/LayoutTests/ChangeLog	2018-06-30 02:06:23 UTC (rev 233386)
+++ trunk/LayoutTests/ChangeLog	2018-06-30 02:11:01 UTC (rev 233387)
@@ -1,3 +1,22 @@
+2018-06-29  Daniel Bates  <daba...@apple.com>
+
+        REGRESSION (r230921): Cannot log in to forums.swift.org using GitHub account
+        https://bugs.webkit.org/show_bug.cgi?id=187197
+        <rdar://problem/40420821>
+
+        Reviewed by Brent Fulgham.
+
+        Add tests to ensure that Same-Site cookies are sent in a subsequent same-origin form submission
+        from a child window that is cross-origin with its opener. Also add a test to ensure that Same-Site
+        cookies are visible to an about:blank pop-up window (as about:blank is Same-Site with its opener
+        by definition of being same-origin with it).
+
+        * http/tests/cookies/same-site/fetch-in-about-blank-popup-expected.txt: Added.
+        * http/tests/cookies/same-site/fetch-in-about-blank-popup.html: Added.
+        * http/tests/cookies/same-site/post-from-cross-site-popup-expected.txt: Added.
+        * http/tests/cookies/same-site/post-from-cross-site-popup.html: Added.
+        * http/tests/cookies/same-site/resources/post-from-popup.html: Added.
+
 2018-06-29  Ryan Haddad  <ryanhad...@apple.com>
 
         Skip media/picture-in-picture-interruption.html on iOS since it relies on 'runWithKeyDown'.

Added: trunk/LayoutTests/http/tests/cookies/same-site/fetch-in-about-blank-popup-expected.txt (0 => 233387)


--- trunk/LayoutTests/http/tests/cookies/same-site/fetch-in-about-blank-popup-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/cookies/same-site/fetch-in-about-blank-popup-expected.txt	2018-06-30 02:11:01 UTC (rev 233387)
@@ -0,0 +1,14 @@
+Tests that Same-Site DOM cookies for 127.0.0.1 are visible from about:blank pop-up.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Cookies visible in DOM:
+PASS Has DOM cookie "strict" with value 13.
+PASS Has DOM cookie "implicit-strict" with value 13.
+PASS Has DOM cookie "strict-because-invalid-SameSite-value" with value 13.
+PASS Has DOM cookie "lax" with value 13.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/http/tests/cookies/same-site/fetch-in-about-blank-popup.html (0 => 233387)


--- trunk/LayoutTests/http/tests/cookies/same-site/fetch-in-about-blank-popup.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/cookies/same-site/fetch-in-about-blank-popup.html	2018-06-30 02:11:01 UTC (rev 233387)
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<script src=""
+<body>
+<script>
+window.jsTestIsAsync = true;
+
+description("Tests that Same-Site DOM cookies for 127.0.0.1 are visible from about:blank pop-up.");
+
+async function runTest()
+{
+    await resetCookies();
+    await setCookie("strict", "13", {"SameSite": "Strict", "Max-Age": 100, "path": "/"});
+    await setCookie("implicit-strict", "13", {"SameSite": null, "Max-Age": 100, "path": "/"});
+    await setCookie("strict-because-invalid-SameSite-value", "13", {"SameSite": "invalid", "Max-Age": 100, "path": "/"});
+    await setCookie("lax", "13", {"SameSite": "Lax", "Max-Age": 100, "path": "/"});
+
+    let childWindow = window.open("about:blank"); // Loads synchronously.
+    setBaseDocumentWhenFetchingDOMCookies(childWindow.document);
+
+    debug("Cookies visible in DOM:");
+    shouldHaveDOMCookieWithValue("strict", "13");
+    shouldHaveDOMCookieWithValue("implicit-strict", "13");
+    shouldHaveDOMCookieWithValue("strict-because-invalid-SameSite-value", "13");
+    shouldHaveDOMCookieWithValue("lax", "13");
+
+    await resetCookies();
+    finishJSTest();
+}
+runTest();
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/http/tests/cookies/same-site/post-from-cross-site-popup-expected.txt (0 => 233387)


--- trunk/LayoutTests/http/tests/cookies/same-site/post-from-cross-site-popup-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/cookies/same-site/post-from-cross-site-popup-expected.txt	2018-06-30 02:11:01 UTC (rev 233387)
@@ -0,0 +1,3 @@
+
+PASS '127.0.0.1' is same-site with itself, so Same-Site cookies are sent. 
+

Added: trunk/LayoutTests/http/tests/cookies/same-site/post-from-cross-site-popup.html (0 => 233387)


--- trunk/LayoutTests/http/tests/cookies/same-site/post-from-cross-site-popup.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/cookies/same-site/post-from-cross-site-popup.html	2018-06-30 02:11:01 UTC (rev 233387)
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<script src=""
+<script src=""
+<script src=""
+<script>
+if (window.location.hostname == "127.0.0.1") {
+    document.cookie = STRICT_DOM + "=1; SameSite=Strict; Max-Age=100; path=/";
+    document.cookie = IMPLICIT_STRICT_DOM + "=1; SameSite; Max-Age=100; path=/";
+    document.cookie = STRICT_BECAUSE_INVALID_SAMESITE_VALUE + "=1; SameSite=invalid; Max-Age=100; path=/";
+    document.cookie = LAX_DOM + "=1; SameSite=Lax; Max-Age=100; path=/";
+    document.cookie = NORMAL_DOM + "=1; Max-Age=100; path=/";
+    window.location.hostname = "localhost";
+} else {
+    async_test(t => {
+        window.addEventListener("message", t.step_func_done(e => {
+            assert_equals(e.data.http[STRICT_DOM], "1", "strict");
+            assert_equals(e.data.http[IMPLICIT_STRICT_DOM], "1", "implicit-strict");
+            assert_equals(e.data.http[STRICT_BECAUSE_INVALID_SAMESITE_VALUE], "1", "strict-because-invalid-SameSite-value");
+            assert_equals(e.data.http[LAX_DOM], "1", "lax");
+            assert_equals(e.data.http[NORMAL_DOM], "1", "normal");
+            assert_equals(normalizeCookie(e.data.document), normalizeCookie(STRICT_BECAUSE_INVALID_SAMESITE_VALUE + "=1; " + IMPLICIT_STRICT_DOM + "=1; " + STRICT_DOM + "=1; " + LAX_DOM + "=1; " + NORMAL_DOM + "=1"));
+        }));
+        window.open("http://127.0.0.1:8000/cookies/same-site/resources/post-from-popup.html");
+    }, "'127.0.0.1' is same-site with itself, so Same-Site cookies are sent.");
+}
+</script>

Added: trunk/LayoutTests/http/tests/cookies/same-site/resources/post-from-popup.html (0 => 233387)


--- trunk/LayoutTests/http/tests/cookies/same-site/resources/post-from-popup.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/cookies/same-site/resources/post-from-popup.html	2018-06-30 02:11:01 UTC (rev 233387)
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+<body>
+<form action="" method="POST">
+    <input type="submit" name="Submit">
+</form>
+<script>
+document.forms[0].submit();
+</script>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (233386 => 233387)


--- trunk/Source/WebCore/ChangeLog	2018-06-30 02:06:23 UTC (rev 233386)
+++ trunk/Source/WebCore/ChangeLog	2018-06-30 02:11:01 UTC (rev 233387)
@@ -1,3 +1,31 @@
+2018-06-29  Daniel Bates  <daba...@apple.com>
+
+        REGRESSION (r230921): Cannot log in to forums.swift.org using GitHub account
+        https://bugs.webkit.org/show_bug.cgi?id=187197
+        <rdar://problem/40420821>
+
+        Reviewed by Brent Fulgham.
+
+        Fixes an issue where a Same-Site cookies are not sent with any child window load if the
+        load is cross-origin with respect to the window's opener. One example where this issue
+        manifest itself was in the GitHub sign in flow on forums.swift.org.
+
+        Currently we always consider the origin of the window's opener (if we have one) when
+        determining whether a frame load request is same-origin and hence should send Same-Site
+        cookies when performing the request. So, when page A.com opens a child window to B.com and
+        then a person clicks a hyperlink or submits a form to B.com/b2 then we do not send Same-
+        Site cookies with the request to B.com/b2 (because its origin, B.com, is cross-origin
+        with its opener, A.com). But we should send Same-Site cookies with the request to B.com/b2
+        because it is same-origin with the page that initiated the request, B.com. Instead of
+        always considering the origin the window's opener for every frame load we should only
+        consider it for the first non-empty document load.
+
+        Tests: http/tests/cookies/same-site/fetch-in-about-blank-popup.html
+               http/tests/cookies/same-site/post-from-cross-site-popup.html
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::addExtraFieldsToRequest):
+
 2018-06-29  Nan Wang  <n_w...@apple.com>
 
         AX: [iOS] VoiceOver scroll position is jumpy in frames

Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (233386 => 233387)


--- trunk/Source/WebCore/loader/FrameLoader.cpp	2018-06-30 02:06:23 UTC (rev 233386)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp	2018-06-30 02:11:01 UTC (rev 233387)
@@ -2776,7 +2776,7 @@
         auto* initiator = m_frame.document();
         if (isMainResource) {
             auto* ownerFrame = m_frame.tree().parent();
-            if (!ownerFrame)
+            if (!ownerFrame && m_stateMachine.isDisplayingInitialEmptyDocument())
                 ownerFrame = m_opener;
             if (ownerFrame)
                 initiator = ownerFrame->document();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to