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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update bedrock's cookie helper lib #8670

Closed
2 tasks done
alexgibson opened this issue Mar 17, 2020 · 2 comments
Closed
2 tasks done

Update bedrock's cookie helper lib #8670

alexgibson opened this issue Mar 17, 2020 · 2 comments
Labels
Papercuts 💸 Tech debt payoff

Comments

@alexgibson
Copy link
Member

alexgibson commented Mar 17, 2020

Description

Bedrock's cookie helper is a small custom library, copied from MDN, which was originally based on the source code of this github repo: https://github.com/madmurphy/cookies.js

The version in bedrock, whilst altered for some custom functionality, is now quite out of date. It does not support setting the sameSite property, for example: https://web.dev/samesite-cookies-explained/

We should consider updating our helper, so that we can be more explicit when it comes to setting cookie values relating to 1st / 3rd party contexts. We could either update our existing helper to add support, or alternatively deprecate it in favor of using the cookies.js open source library directly.


💛 Success Criteria 💛

  • Cookie helper is updated.
  • Cookies set using JS explicitly set the sameSite attribute (lax should be fine for front end needs where cookies are used to control display of content?).
@alexgibson alexgibson added the Papercuts 💸 Tech debt payoff label Mar 17, 2020
@mitchell-frost
Copy link

I am new to mozilla. Can I work on this?

@alexgibson
Copy link
Member Author

Changes to SameSite cookie behavior are rolling out in Chrome and soon also in Firefox. https://hacks.mozilla.org/2020/08/changes-to-samesite-cookie-behavior/

From what I can tell, I don't think any cookies in use on mozorg are effected directly by this change, but it may still be worthwhile updating this lib, so that we can set the SameSite attribute explicitly, instead of relay on the new implied default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Papercuts 💸 Tech debt payoff
Projects
None yet
Development

No branches or pull requests

2 participants