Bug 187197

Summary: REGRESSION (r230921): Cannot log in to forums.swift.org using GitHub account
Product: WebKit Reporter: Daniel Bates <dbates>
Component: WebCore Misc.Assignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cdumez, ews-watchlist, japhet, rniwa, webkit-bug-importer
Priority: P1 Keywords: InRadar, Regression
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=185780
Bug Depends on: 159464    
Bug Blocks:    
Attachments:
Description Flags
[Screenshot] GitHub error page on Mojave
none
Patch and layout tests
none
Archive of layout-test-results from ews107 for mac-sierra-wk2 none

Daniel Bates
Reported 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."
Attachments
[Screenshot] GitHub error page on Mojave (51.95 KB, image/png)
2018-06-29 15:46 PDT, Daniel Bates
no flags
Patch and layout tests (9.62 KB, patch)
2018-06-29 16:13 PDT, Daniel Bates
no flags
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
Daniel Bates
Comment 1 2018-06-29 15:02:21 PDT
Daniel Bates
Comment 2 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.
Daniel Bates
Comment 3 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.
Daniel Bates
Comment 4 2018-06-29 16:13:13 PDT
Created attachment 343963 [details] Patch and layout tests
Brent Fulgham
Comment 5 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"
EWS Watchlist
Comment 6 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
EWS Watchlist
Comment 7 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
Daniel Bates
Comment 8 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.
Daniel Bates
Comment 9 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.
Daniel Bates
Comment 10 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>
Daniel Bates
Comment 11 2018-06-29 19:11:12 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.