1654064 - Unable to sign in with Google on app.plex.tv with ETP Standard

Status: RESOLVED FIXED

(Core :: Privacy: Anti-Tracking, defect, P1)

Description

[Ksenia Berezina [:ksenia]]

Attachment: Screen Shot 2020-07-20 at 3.46.23 PM.png

Steps to reproduce:

1. Open https://app.plex.tv/desktop in recent Nightly desktop
2. Click "Sign in" on the top right.
3. After "Sign in" page is displayed, click on "Continue with Google".
4. Sign in to your Google account and after returning to the site observe the behaviour

Expected Behavior:
Signed in to the site

Actual Behavior:
Button is stuck in loading state

The issue is not reproducible if I turn off ETP. Also reproducible on mobile.

Comment 1

[Steven Englehardt [:englehardt]]

I can reproduce the breakage with cookieBehavior 4 and 5. The breakage seems to be related to cookie blocking on accounts.google.com. You see a message similar to: Request to access cookie or storage on “https://accounts.google.com/o/oauth2/iframe#origin=https%3A%2F%2Fapp.plex.tv&rpcToken=486012004.5689947” was blocked because it came from a tracker and content blocking is enabled. which prints a few times a second. This likely means the Google script is repeatedly trying to access storage.

When the "Continue with Google" button is clicked, a google window with the host accounts.google.com is opened, and the popup has opener access back to the plex document. However, accounts.google.com is not granted storage access by our opener heuristic. I noticed that the button is embedded in a first-party iframe, the opener points to https://app.plex.tv/auth-form/#!?clientID=.

Gary, would you mind to dig a bit further into this? Is the reason accounts.google.com isn't automatically granted storage access due to the fact that it's embedded in an iframe rather than the main context? If so, do you see any reason to prefer that restriction?

Comment 2

[Gary Chen [:xeonchen]]

OK!

Comment 3

[Gary Chen [:xeonchen]]

(In reply to Steven Englehardt [:englehardt] from comment #1)

I can reproduce the breakage with cookieBehavior 4 and 5. The breakage seems to be related to cookie blocking on accounts.google.com. You see a message similar to: Request to access cookie or storage on “https://accounts.google.com/o/oauth2/iframe#origin=https%3A%2F%2Fapp.plex.tv&rpcToken=486012004.5689947” was blocked because it came from a tracker and content blocking is enabled. which prints a few times a second. This likely means the Google script is repeatedly trying to access storage.

When the "Continue with Google" button is clicked, a google window with the host accounts.google.com is opened, and the popup has opener access back to the plex document. However, accounts.google.com is not granted storage access by our opener heuristic. I noticed that the button is embedded in a first-party iframe, the opener points to https://app.plex.tv/auth-form/#!?clientID=.

Gary, would you mind to dig a bit further into this? Is the reason accounts.google.com isn't automatically granted storage access due to the fact that it's embedded in an iframe rather than the main context? If so, do you see any reason to prefer that restriction?

[Child 91055: Main Thread]: D/AntiTracking Adding a first-party storage exception for https://accounts.google.com, triggered by opener
[Child 91055: Main Thread]: D/AntiTracking The current resource is third-party
[Child 91055: Main Thread]: D/AntiTracking Our window isn't a third-party window

According to the AntiTracking log, it shows the permission was not saved because the parent window context is not a 3rd-party window.

Dimi or Baku may have some comments?

Comment 4

[Dimi Lee [:dimi]]

(In reply to Gary Chen [:xeonchen] from comment #3)

According to the AntiTracking log, it shows the permission was not saved because the parent window context is not a 3rd-party window.

Dimi or Baku may have some comments?

Right, I think this is because window.open is called in a first-party iframe, but our heuristic uses 3rd-party logic code path because the parent context is not a top-level.
I guess the solution could be replacing top-level check here with first-party check.

Comment 5

[Andrea Marchesini [:baku]]

I'm OK with it. Maybe we should check if this is related to what we discussed yesterday about the 3rd-party check window vs channel.
Do you have time to work on it?

Comment 6

[Dimi Lee [:dimi]]

(In reply to Andrea Marchesini [:baku] from comment #5)

I'm OK with it. Maybe we should check if this is related to what we discussed yesterday about the 3rd-party check window vs channel.
Do you have time to work on it?

I'll talk PTO next week, if we can wait, I can work on this when I get back.

Comment 7

[Dimi Lee [:dimi]]

Attachment: Bug 1654064 - P1. Use first-party check instead of top-level check while applying storage access heuristic r=baku

Comment 8

[Dimi Lee [:dimi]]

Attachment: Bug 1654064 - P2. Add a testcase for applying window.open heuristic on a first-party iframe. r=baku

Depends on D85826

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/90bf9f23c414
P1. Use first-party check instead of top-level check while applying storage access heuristic r=baku
https://hg.mozilla.org/integration/autoland/rev/23e5e026f179
P2. Add a testcase for applying window.open heuristic on a first-party iframe. r=baku

Comment 10

[Andreea Pavel [:apavel]]

Backed out for android failures at test_third_party_iframes.html

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=23e5e026f179a129f2d89cfb4d737041a8e2415d&selectedTaskRun=GjUEEVGVQtmXY0UqEjzmbA.0

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=312189887&repo=autoland&lineNumber=3447

Backout: https://hg.mozilla.org/integration/autoland/rev/d2386f5c1be2f13b68d6c2c0f297bea08496808c

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/db37f6df1877
P1. Use first-party check instead of top-level check while applying storage access heuristic r=baku
https://hg.mozilla.org/integration/autoland/rev/cfeb2f1cae26
P2. Add a testcase for applying window.open heuristic on a first-party iframe. r=baku

Comment 12

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/db37f6df1877
https://hg.mozilla.org/mozilla-central/rev/cfeb2f1cae26