Skip to content

Feat: Add sameSite property to CookieStorage #5726

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.

Sign up for GitHub

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

Jump to bottom
Merged
ericclemmons merged 5 commits into from
Aug 11, 2020
Merged

Feat: Add sameSite property to CookieStorage #5726

ericclemmons merged 5 commits into from
Aug 11, 2020

Conversation

@michaelmerrill
Copy link
Contributor

@michaelmerrill michaelmerrill commented May 8, 2020

Issue: #4958

Add the sameSite attribute to CookieStorage.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov
Copy link

codecov bot commented May 8, 2020 •
edited
Loading

Codecov Report

Merging #5726 into main will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #5726   +/-   ##
=======================================
  Coverage   73.26%   73.26%           
=======================================
  Files         208      208           
  Lines       12920    12920           
  Branches     2432     2432           
=======================================
  Hits         9466     9466           
  Misses       3291     3291           
  Partials      163      163

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb97194...65c0bc7. Read the comment docs.

@amhinson amhinson added the needs-discussion Used for internal discussions label Jun 16, 2020
@ismyrnow ismyrnow mentioned this pull request Jun 16, 2020
Closed
@amhinson amhinson changed the base branch from master to main June 18, 2020 18:49
madStitcher
madStitcher approved these changes Aug 3, 2020
View reviewed changes
Copy link

@madStitcher madStitcher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fairly straight forward

@ghost
Copy link

ghost commented Aug 6, 2020

@amhinson What do you think about that?

@amhinson amhinson self-assigned this Aug 10, 2020
@ericclemmons ericclemmons added Reviewing and removed needs-discussion Used for internal discussions labels Aug 11, 2020
ericclemmons
ericclemmons reviewed Aug 11, 2020
View reviewed changes
packages/amazon-cognito-identity-js/package.json
"buffer": "4.9.1",
"crypto-js": "^3.3.0",
"js-cookie": "^2.1.4"
"js-cookie": "^2.2.1"
Copy link
Contributor

@ericclemmons ericclemmons Aug 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving a note that SameSite was added in:

js-cookie/js-cookie#276 (comment)

amhinson
amhinson approved these changes Aug 11, 2020
View reviewed changes
Copy link
Contributor

@amhinson amhinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @michaelmerrill! 👍

@amhinson amhinson requested a review from elorzafe August 11, 2020 17:21
elorzafe
elorzafe approved these changes Aug 11, 2020
View reviewed changes
Copy link
Contributor

@elorzafe elorzafe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelmerrill thanks for your contribution! 🥇

@ericclemmons ericclemmons merged commit 1934941 into aws-amplify:main Aug 11, 2020
ericclemmons added a commit to aws-amplify/docs that referenced this pull request Aug 11, 2020
@ericclemmons
Update example with CookieStorage's sameSite
b98e3bf 
aws-amplify/amplify-js#4958
aws-amplify/amplify-js#5726

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite
@ericclemmons ericclemmons mentioned this pull request Aug 11, 2020
Merged
This was referenced Aug 28, 2020
Closed
Merged
nubpro pushed a commit to nubpro/amplify-js that referenced this pull request Oct 2, 2020
@michaelmerrill @amhinson
@ericclemmons @nubpro
Feat: Add sameSite property to CookieStorage (aws-amplify#5726)
39db4ff 
Co-authored-by: Alex Hinson <[email protected]>
Co-authored-by: Eric Clemmons <[email protected]>
iartemiev pushed a commit that referenced this pull request Oct 13, 2020
@michaelmerrill @amhinson
@ericclemmons @iartemiev
Feat: Add sameSite property to CookieStorage (#5726)
7de3e20 
Co-authored-by: Alex Hinson <[email protected]>
Co-authored-by: Eric Clemmons <[email protected]>
@github-actions
Copy link

github-actions bot commented Aug 11, 2021

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

5 more reviewers

@ericclemmons ericclemmons ericclemmons left review comments

@amhinson amhinson amhinson approved these changes

@madStitcher madStitcher madStitcher approved these changes

@elorzafe elorzafe elorzafe approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@amhinson amhinson

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@michaelmerrill @ericclemmons @amhinson @madStitcher @elorzafe