Bug 187197 - REGRESSION (r230921): Cannot log in to forums.swift.org using GitHub account
Summary: REGRESSION (r230921): Cannot log in to forums.swift.org using GitHub account
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P1 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar, Regression
Depends on: 159464
Blocks:
  Show dependency treegraph
 
Reported: 2018-06-29 15:01 PDT by Daniel Bates
Modified: 2018-06-30 00:41 PDT (History)
6 users (show)

See Also:


Attachments
[Screenshot] GitHub error page on Mojave (51.95 KB, image/png)
2018-06-29 15:46 PDT, Daniel Bates
no flags Details
Patch and layout tests (9.62 KB, patch)
2018-06-29 16:13 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-sierra-wk2 (3.32 MB, application/zip)
2018-06-29 17:20 PDT, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2018-06-29 15:01:57 PDT
Seen with top-of-tree WebKit on macOS Mojave.

Steps to reproduce:

1. Visit <https://forums.swift.org>.
2. Click Log In.
3. In the Log In overlay, click the "with GitHub" button.
4. In the GitHub pop up window, sign in with your Github credentials, if applicable.
5. Click the "Authorize apple" button.

Then you should see an "Authorization complete" message. But I see a Github error page with text "What? \\ Your browser did something unexpected. Please contact us if the problem persists."
Comment 1 Daniel Bates 2018-06-29 15:02:21 PDT
<rdar://problem/40420821>
Comment 2 Daniel Bates 2018-06-29 15:44:30 PDT
The issue is that we consider the origin of the pop-up window opener when determining whether to send Same-Site cookies for a request to be loaded in the pop-up regardless of whether the request was initiated by the opener. We should only consider the opener's origin for the first non-empty document load in the pop-up window. (An about:blank pop-up is same-origin with its opener; => it is Same-Site with its opener).

With regards to the sign in flow for forums.swift.org using a GitHub account, subsequent navigations/form submissions in the GitHub pop-window after initial load are considered cross-origin (because they are compared against the opener, forums.swift.org). But they should be considered same-origin because all the subsequent navigations/form submissions are to https://github.com pages.

Additional remarks:

In <https://trac.webkit.org/changeset/230921/> (bug #159464) we added support for Same-Site cookies on Mac when running on macOS Mojave or later. And GitHub is making use of Same-Site cookies. This issue does not occur on earlier version of macOS as treat Same-Site cookies equivalent to non-Same-Site cookies.
Comment 3 Daniel Bates 2018-06-29 15:46:42 PDT
Created attachment 343955 [details]
[Screenshot] GitHub error page on Mojave

Screenshot of the error page seen on macOS Mojave after following the reproduction steps.
Comment 4 Daniel Bates 2018-06-29 16:13:13 PDT
Created attachment 343963 [details]
Patch and layout tests
Comment 5 Brent Fulgham 2018-06-29 16:21:05 PDT
Comment on attachment 343963 [details]
Patch and layout tests

View in context: https://bugs.webkit.org/attachment.cgi?id=343963&action=review

r=me, but please be sure EWS is happy before landing.

> Source/WebCore/ChangeLog:9
> +        Fixes an issue where a Same-Site cookies are not sent with any child window load if the

"... where Same-Site cookies"
Comment 6 EWS Watchlist 2018-06-29 17:20:54 PDT
Comment on attachment 343963 [details]
Patch and layout tests

Attachment 343963 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8389065

New failing tests:
css3/filters/backdrop/add-remove-add-backdrop-filter.html
Comment 7 EWS Watchlist 2018-06-29 17:20:56 PDT
Created attachment 343980 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 8 Daniel Bates 2018-06-29 19:07:05 PDT
(In reply to Build Bot from comment #6)
> Comment on attachment 343963 [details]
> Patch and layout tests
> 
> Attachment 343963 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: https://webkit-queues.webkit.org/results/8389065
> 
> New failing tests:
> css3/filters/backdrop/add-remove-add-backdrop-filter.html

As far as I can tell this failure is not related to the change made in the proposed patch.
Comment 9 Daniel Bates 2018-06-29 19:09:24 PDT
(In reply to Daniel Bates from comment #8)
> (In reply to Build Bot from comment #6)
> > Comment on attachment 343963 [details]
> > Patch and layout tests
> > 
> > Attachment 343963 [details] did not pass mac-wk2-ews (mac-wk2):
> > Output: https://webkit-queues.webkit.org/results/8389065
> > 
> > New failing tests:
> > css3/filters/backdrop/add-remove-add-backdrop-filter.html
> 
> As far as I can tell this failure is not related to the change made in the
> proposed patch.

Additionally, I am unable to reproduce this failure on my machine with the patch applied.
Comment 10 Daniel Bates 2018-06-29 19:11:10 PDT
Comment on attachment 343963 [details]
Patch and layout tests

Clearing flags on attachment: 343963

Committed r233387: <https://trac.webkit.org/changeset/233387>
Comment 11 Daniel Bates 2018-06-29 19:11:12 PDT
All reviewed patches have been landed.  Closing bug.