-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
goog.window.open
with "no-referrer" should stop using document.write
, because of COOP:same-origin-allow-popup
.
#1137
Comments
Update test expectations to follow the current HTML specification. Bug:whatwg/html#6960 Bug:google/closure-library#1137 Bug:1216244 Change-Id: Ib8bd9e69a5a84de47577f5be1c211af9a35468be
Update test expectations to follow the current HTML specification. Bug: whatwg/html#6960 Bug: google/closure-library#1137 Bug: 1216244 Change-Id: Ib8bd9e69a5a84de47577f5be1c211af9a35468be Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3097699 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Commit-Queue: Camille Lamy <clamy@chromium.org> Auto-Submit: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Camille Lamy <clamy@chromium.org> Cr-Commit-Position: refs/heads/master@{#912551}
Update test expectations to follow the current HTML specification. Bug: whatwg/html#6960 Bug: google/closure-library#1137 Bug: 1216244 Change-Id: Ib8bd9e69a5a84de47577f5be1c211af9a35468be Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3097699 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Commit-Queue: Camille Lamy <clamy@chromium.org> Auto-Submit: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Camille Lamy <clamy@chromium.org> Cr-Commit-Position: refs/heads/master@{#912551}
Is there an alternative we can use to accomplish the same thing as calling |
@kjin I have the same question. I spent some time exploring the various options here and wasn't able to come up with any way of doing so. It seems like the best option may be to just drop support for this parameter and rely on the fact that modern browsers no longer include the full referrer. Internally, I've been advising services using this parameter to do exactly that unless it is critical to hide the referrer in older browsers. |
Within google, it seems that only 10% or so of the calls pass the noreferrer option, less than 100 total. Unfortunately, it's unlikely that any tests would break if we removed the feature and it was actually still necessary, so it's not clear how we'd actually communicate this change safely. FWIW, we're also looking into deprecating the entire method in favor of exposing a safer variant in https://github.com/google/safevalues. |
@ddworken Can you give some more details about this? I don't fully grok what's going on here. When did the behavior change? Can you point to when/where the spec changed that would not send the full referrer? Is this in reference to point 4 in the bug you linked? It sounds like that's only really a partial solution and can be opted out, which will cause privacy issues. It's possible we could change the behavior only for "modern browsers" and keep doing the same thing we've always done on older browsers - but what is the cutoff and how do you recommend detecting it? That said, even if we update Closure Library, we can't guarantee the new version is picked up ubiquitously enough - any given Google property will be rebuilt against head at worst every year (and usually much more frequently), but there are external properties still running against versions of the library from 2017 or earlier, and there's nothing we can do to push out any updates. Are we concerned about breaking them? My impression is that the spec itself is problematic - can we change it to better align with reality and not break the web? |
Referer-Policy is a new way for sites to restrict the data that is included in the But yes, you're right that a site could be setting a referer policy of
Yeah, that could work. It could in theory be done by user agent sniffing and including a list of the versions where each browser started hiding the full referer by default.
Personally, I'm not too concerned with this. My suspicion is that any sites using old versions of the library probably aren't deploying COOP since it is a very cutting edge security standard. But there is still a risk there. But at least by dropping support for no-referrer, we would make it so that sites deploying COOP with a modern version of closure are safe.
I personally agree with this. As I understand it, the issue here is more that it is hard to make the way COOP is specced allow for this, even though conceptually it doesn't seem problematic. @camillelamy would know more about this. |
The issue is really with how If you want to just inject content, using e.g. |
Yes, to be verified, I don't know if this will strip the referer and works across every web browsers. @domfarolino who know about referrer might be able to provide useful informations. If this works or another solution works great! Otherwise, I am wondering if the websites at Google that used this "noreferrer" option and want to keep the openee/opener access couldn't simply stop using this option? It feels weird for me wanting both at the same time. WDYT @ddworken ? |
The problem for changing the COOP spec is that there is no way for the browser to distinguish between "I used document.write to open a popup without a referrer" and "I used document.write to create a complete HTML page, which navigated 20mn later to another page", and we'd really like to trigger a browsing context group switch in the second case. Since your goal is not to actually create a new page but strip the referrer, using document.write might not be the thing you want here. |
So after some testing, it appears as though setting
Yeah, I think this is plausible (and is what I've been recommending to teams). The main caveats are that:
If we think this is the best path forward, I'm fine with just not changing closure and adding this to our list of things that we warn product teams about. But there is a non-insignificant risk to this. |
Right. This is because, unlike I suggest absolutizing the URL using |
Thanks Domenic! I tried implementing that (and adding logic to only do it for modern browsers that support What do people think we should do? Is our best solution just to document this as a limitation (and I can drive a change internally to minimize our use of |
IIUC, @ddworken is saying that absolutizing the URL is insufficient because Firefox won't trigger the redirect at all in the first place. Otherwise, it sounds like Assuming that's not feasible, here's the problematic combinations:
Situation 1 I think is entirely an education problem - we shouldn't recommend folks do this without understanding all the ramifications. Situation 2 is also primarily an education problem, but only if Closure decides to ignore Is there any way to determine in JS if a site has opted into COOP? (assuming that's even where it happens? I'm not as clear on how it's supposed to work) If so, then could we make it an error to use no-referrer at the same time? This would "fix" the breakage, but would introduce migration hurdles (or would make it harder to flip the switch by default - not sure if that's planned). Regardless of all this, driving down use of |
It should work, and I wonder if it might even fix the Firefox problem; maybe they have weird logic about parser-triggered navigations. (The spec has no such logic so this is just a Firefox bug...)
You can use |
I checked and sadly that doesn't work in Firefox either. :/
This is my understanding too. At least at Google, we have deployed COOP widely but haven't deployed COEP very much. We also generally are encouraging services to deploy COOP before COEP. So I don't think this will help too much.
Personally, this seems mostly okay to me. At least at Google, I think there is only 1 service that overrides the referrer policy to |
Unfortunately, this case is specific to COOP same-origin-allow-popups, which does not enable crossOriginIsolation. So we can't rely on checking if the document is crossOriginIsolated. To be clear, there is nothing preventing us from exposing the COOP status of a document, so we could add that to the platform if needed. |
…ations., a=testonly Automatic update from web-platform-tests COOP:SOAP vs document.write update expecations. Update test expectations to follow the current HTML specification. Bug: whatwg/html#6960 Bug: google/closure-library#1137 Bug: 1216244 Change-Id: Ib8bd9e69a5a84de47577f5be1c211af9a35468be Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3097699 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Commit-Queue: Camille Lamy <clamy@chromium.org> Auto-Submit: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Camille Lamy <clamy@chromium.org> Cr-Commit-Position: refs/heads/master@{#912551} -- wpt-commits: 18f738d6c0a314f7230c7f6c9f3cd943e5b5c009 wpt-pr: 30049
…ations., a=testonly Automatic update from web-platform-tests COOP:SOAP vs document.write update expecations. Update test expectations to follow the current HTML specification. Bug: whatwg/html#6960 Bug: google/closure-library#1137 Bug: 1216244 Change-Id: Ib8bd9e69a5a84de47577f5be1c211af9a35468be Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3097699 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Commit-Queue: Camille Lamy <clamy@chromium.org> Auto-Submit: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Camille Lamy <clamy@chromium.org> Cr-Commit-Position: refs/heads/master@{#912551} -- wpt-commits: 18f738d6c0a314f7230c7f6c9f3cd943e5b5c009 wpt-pr: 30049
We had a realtime chat about this and came up with the following plan:
@ddworken is going to prepare a change for this, and the Closure team will help if it's nontrivial to land. |
Fixed by f2c85d1. |
Update test expectations to follow the current HTML specification. Bug: whatwg/html#6960 Bug: google/closure-library#1137 Bug: 1216244 Change-Id: Ib8bd9e69a5a84de47577f5be1c211af9a35468be Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3097699 Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Commit-Queue: Camille Lamy <clamy@chromium.org> Auto-Submit: Arthur Sonzogni <arthursonzogni@chromium.org> Reviewed-by: Camille Lamy <clamy@chromium.org> Cr-Commit-Position: refs/heads/master@{#912551} NOKEYCHECK=True GitOrigin-RevId: b83a573bae82e84554a8b2ebe8636f56d940a782
To strip referrer in
goog.window.open()
with the ''no-referrer" option. The closure library is using document.write:closure-library/closure/goog/window/window.js
Line 235 in 6da97fe
document.write
taint the initial empty document. This preventsCOOP:same-origin-allow-popup
to allow the popup to stay within the same browsing context group. This breaks OAUTH flow of some websites.The
COOP:same-origin-allow-popup
anddocument.write
behavior is implemented by:It was implemented by Chrome, but reverted because it broke some Google users of the closure library. Chrome intends to apply the specification at some point after resolving this bug.
Would not using document.write be an option for you?
See related issues:
+CC @ddworken @camillelamy @cdumez
The text was updated successfully, but these errors were encountered: