Skip to content
This repository has been archived by the owner on Sep 25, 2024. It is now read-only.

set cookies to be secure #1255

Merged
merged 2 commits into from
Feb 3, 2020
Merged

set cookies to be secure #1255

merged 2 commits into from
Feb 3, 2020

Conversation

katiedavis
Copy link
Contributor

@katiedavis katiedavis commented Feb 3, 2020

Description

Fixes issue where request is not considered secure. See: https://github.com/Shopify/shopify-demo-app-node-react/issues/129#issuecomment-580820979

Type of change

  • koa-shopify-auth Patch: Bug/ Documentation fix (non-breaking change which fixes an issue or adds documentation)

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above

Copy link
Contributor

@marutypes marutypes left a comment

Choose a reason for hiding this comment

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

Seems legit to me.

@marutypes
Copy link
Contributor

Should we also change the spots we set cookies to manually use secure: true? That way we can still advise partners to use security everywhere but be sure our cookies always are.

@katiedavis
Copy link
Contributor Author

@TheMallen There's actually a great PR open here: #1251
I like the utility they built to check the browser etc. Should we review that and get it in?


- Package now forces cookies.secure to be true [1255](https://github.com/Shopify/quilt/pull/1255)

## 3.1.55 -2020-02-02
Copy link
Contributor

Choose a reason for hiding this comment

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

I would just release em under the same version

@katiedavis katiedavis merged commit 7ae87a7 into master Feb 3, 2020
@katiedavis katiedavis deleted the try-force-cookies branch February 3, 2020 18:06
@ismail-syed ismail-syed temporarily deployed to production February 3, 2020 19:02 Inactive
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants