Closed Bug 1505568 Opened 6 years ago Closed 6 years ago

Cookie activation heuristic does not activate for a pop-up window that has opener access.

Categories

(Firefox :: Protections UI, defect)

defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1505571

People

(Reporter: englehardt, Unassigned)

References

(Blocks 1 open bug)

Details

Look at the following test page: https://senglehardt.com/test/identity_providers/google_custom.html.

STR:
1. Open a new profile (or one on which google doesn't already have storage access on senglehardt.com)
2. Click the "Google" button
3. A pop-up with opener access is triggered by the click (and does not trigger the pop-up blocker).
4. Interact with the pop-up and close it. Just click around, no sign in necessary.

Actual results:
No domains are automatically granted storage access on `senglehardt.com`. Refresh the page and you will still see console logs related to blocked storage access for `accounts.google.com` resources.

Expected results:
Either the window.open or pop-up window interaction heuristic should trigger storage access for `accounts.google.com`.

To add to the weirdness:
Now click the Google button again. You should see storage access granted on the immediate `window.open` call. Also note that this ONLY happens if you interacted with the pop-up window in step (4). If you skip step (4), access is not granted on this step.
> Also note that this ONLY happens if you interacted with the pop-up window in step (4). If you skip step (4), access is not granted on this step.

I think this is intentional. The window.open heuristic requires the opened window to have had user interaction at some point. That was the original intention of the design, but we just didn't have the interaction flag to do it earlier. Ehsan, can you confirm? If so, I'll update the MDN docs.

Note there is still a bug here. The interaction that happens as part of step (4) should be all that's needed to grant access. Re-triggering the window.open call should not be necessary.
Flags: needinfo?(ehsan)
I see this is already captured in the MDN docs.
Flags: needinfo?(ehsan)
If you run with this environment variable: MOZ_LOG=AntiTracking:5 you will see the following in the console after step 4 in comment 0:

[Child 6428: Main Thread]: D/AntiTracking Asking the parent process to save the user-interaction for us: https://accounts.google.com/signin/oauth?client_id=902141776049-sjne3e0c48hsej7fuf71lgkq2bis8i7g.apps.googleusercontent.com&as=e-HFqTsW3_URGaoZVMfe_A&destination=https://senglehardt.com&approval_state=foo
[Child 6428: Main Thread]: D/AntiTracking Adding a first-party storage exception for https://accounts.google.com...
[Child 6428: Main Thread]: D/AntiTracking Deciding whether the user has overridden content blocking for https://senglehardt.com/test/identity_providers/google_custom.html
[Child 6428: Main Thread]: D/AntiTracking No user override found
[Child 6428: Main Thread]: D/AntiTracking The current resource is first-party
[Child 6428: Main Thread]: D/AntiTracking Tracking principal (https://accounts.google.com/signin/oauth?client_id=902141776049-sjne3e0c48hsej7fuf71lgkq2bis8i7g.apps.googleusercontent.com&as=e-HFqTsW3_URGaoZVMfe_A&destination=https://senglehardt.com&approval_state=foo) hasn't been interacted with before, refusing to add a first-party storage permission to access it

Note the last line.  As you've noted, this behaviour is intentional.

And immediately after that:

[Parent 6351: Main Thread]: D/AntiTracking Saving the userInteraction for https://accounts.google.com/signin/oauth?client_id=902141776049-sjne3e0c48hsej7fuf71lgkq2bis8i7g.apps.googleusercontent.com&as=e-HFqTsW3_URGaoZVMfe_A&destination=https://senglehardt.com&approval_state=foo

This is the parent process responding to the child in the first line.  This is recording our  first-party interaction with accounts.google.com.  But this happens *after* we check for this permission because we don't want the first time interaction to count as "interaction".

With this in mind, if you read the "To add to the weirdness" section, the behavior should make perfect sense now.  I agree that it is weird, but that is also a contrived scenario that is unlikely to happen in real life and hence I'd argue not something to optimize for.  IOW this heuristic tries to achieve the right UX in the common usage case.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INVALID
I don't think this scenario is unlikely to be hit in practice. Consider the following:

1. A user is using federated login from social.example to authenticate to news.example.
2. They clear their history / cookies / permissions and visit news.example to log in.
3. They click "Login with social.example", which triggers a pop-up with opener access and complete the login.
4. The login fails because social.example is not given storage access on news.example
5. The user is confused, tries again, and the login works.

Note that (5) is the ideal scenario. We know that in some cases, (4) will cause the user to have a totally broken account on the first-party site if this breakage occurs during registration.

This scenario will be experienced by all users who clear browser state unless they first visit and interact with social.example.
Flags: needinfo?(ehsan)
Thanks for disagreeing.  I think you're right.  The use case you're bringing up deserves a solution.

I'd like to know what Andrea thinks about this.  Do you think there is a way that we can help address the use case in comment 4 without compromising on the change we made in bug 1494476?
Status: RESOLVED → REOPENED
Flags: needinfo?(ehsan) → needinfo?(amarchesini)
Resolution: INVALID → ---
See Also: → 1494476
I think this is fixed by bug 1505571. I also add a test. Ehsan, can you confirm it's the same scenario?
Flags: needinfo?(amarchesini) → needinfo?(ehsan)
(In reply to Andrea Marchesini [:baku] from comment #6)
> I think this is fixed by bug 1505571. I also add a test. Ehsan, can you
> confirm it's the same scenario?

Well, I think from your explanation, that bug is the first sentence of this comment: https://bugzilla.mozilla.org/show_bug.cgi?id=1505571#c12

But I think your fix there is adding an intentional regression.  I'd like to know there is no other way to fix this bug before accepting this regression...
Flags: needinfo?(ehsan)
See Also: → 1505571
> But I think your fix there is adding an intentional regression.  I'd like to know there is no other way to fix this bug before accepting this regression...

IMO the scenario you described in Bug 1505571 Comment 15 isn't a regression. We don't know how the pop-up will look, and having a broken login experience for some popups is a much worse user experience than a couple accidental (and temporary) storage access grants. If we see the simple single-click heuristic being abused, that's a different story.

In that comment you suggested restricting user interaction to form controls as a possibility, but I fear this may break some login scenarios. Consider the following:

A user has Facebook login cookies, but hasn't interacted with Facebook recently enough as a first party to have an interaction flag. Let's say they strictly use FB for federated login.

1. On a new site, the user clicks "Login with Facebook"
2. Since they're already logged in, the pop-up doesn't have login forms on it. Instead, it simply asks for sharing permissions.
3. The user clicks a button to grant permission to Facebook to share data with the first party (or perhaps multiple buttons).
4. The pop-up closes

It seems like we'll break this is we add in additional interaction requirements.

Also note that an extension of this style of breakage is the case where the user has already granted Facebook data sharing permissions in the past for this first party. In that case, the pop-up may simply open and automatically close.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.