Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not use third-party cookies in embedded videos #5548

Merged
merged 9 commits into from
Jun 7, 2024
Merged

Conversation

javierm
Copy link
Member

@javierm javierm commented May 19, 2024

References

Objectives

  • Don't store third-party cookies when people watch a YouTube/Vimeo video embedded in a Consul Democracy installation
  • Refactor the code rendering embedded videos

Notes

Even with these changes, Vimeo still stores cookies that we cannot avoid; quoting from Vimeo player parameters overview:

When DNT is enabled, Vimeo deploys one essential cookie via the embeddable player:

  • The __cf_bm cookie, which is part of Cloudflare's Bot Management service and helps mitigate risk associated with spam and bot traffic.

Not sure whether Google stores a NID cookie when playing YouTube videos 馃. I haven't been able to reproduce it.

Sponsored

Functionality developed by

@javierm javierm self-assigned this May 19, 2024
@javierm javierm added this to Reviewing in Consul Democracy May 19, 2024
@javierm javierm changed the title Video cookies Do not load third-party cookies in embedded videos May 19, 2024
@javierm javierm added the security Pull requests that address a security vulnerability label May 19, 2024
@javierm javierm changed the title Do not load third-party cookies in embedded videos Do not use third-party cookies in embedded videos May 19, 2024
@taitus taitus self-assigned this Jun 6, 2024
We were using the same code, and the same regular expressions, in two
places. To do so, we were including a helper inside a model, which is
something we don't usually do.
No that it's no longer a helper, we can extract method without fearing
they will have the same name as other helper methods.
When using `<%= `, `nil` is converted to an empty string, so there's no
need to explicitely return an empty string.
We were using `reg_exp` as the method name, when it returned
`VIMEO_REGEX` or `YOUTUBE_REGEX`.

So using `regex` as the method name is less confusing.
Now we're also testing that there's an iframe with the URL; before this
change, the test would pass even if the JavaScript generating the iframe
wouldn't work.
When embedding a video in our site YouTube stores cookies in the user's
computer that aren't necessary to watch the video, so we'd have to make
people accept those cookies before letting them watch the video.

Using a URL that doesn't use cookies, like mentioned in YouTube Help
[1], is easier, though, and respects people's privacy without affecting
the user experience.

That I've found some references saying that youtube does store cookies
once you hit the "play" button even when using the nocookie server [2].
Not sure whether that's an old behavior or I'm doing something wrong,
but I don't see this is the case; even after playing the video, cookies
aren't stored on my browser.

[1] https://support.google.com/youtube/answer/171780#zippy=%2Cturn-on-privacy-enhanced-mode
[2] https://www.cnet.com/news/privacy/youtubes-new-nocookie-feature-continues-to-serve-cookies/
Consul Democracy automation moved this from Reviewing to Testing Jun 7, 2024
With this parameter, Vimeo no longer uses cookies that identifies users
browsing our site.

They do still store some cookies, though; quoting from Vimeo player
parameters overview:

> When DNT is enabled, Vimeo deploys one essential cookie via the
> embeddable player:
> The __cf_bm cookie, which is part of Cloudflare's Bot Management
> service and helps mitigate risk associated with spam and bot traffic.

Not sure whether this counts as essential cookies in our case; they're
essential for Vimeo, but for us, they're third-party cookies, after all.

[1] https://help.vimeo.com/hc/en-us/articles/12426260232977-Player-parameters-overview
@javierm
Copy link
Member Author

javierm commented Jun 7, 2024

The failing test is not related to this pull request.

@javierm javierm merged commit 9bef791 into master Jun 7, 2024
12 of 13 checks passed
Consul Democracy automation moved this from Testing to Release 2.2.0 Jun 7, 2024
@javierm javierm deleted the video_cookies branch June 7, 2024 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security Pull requests that address a security vulnerability
Projects
Consul Democracy
  
Release 2.2.0
Development

Successfully merging this pull request may close these issues.

None yet

2 participants