Add SameSite=None and secure to App Session

[jenshenny]

Background

In Chrome 80, the default value of the SameSite cookie property will switch from None to Lax (more details here). This means that all iframe'd and cross-origin non-GET requests will not send cookies, meaning that it will break embedded app functionality (you can try enabling the SameSite None flag and accessing an embedded app in your admin)

Additionally, SameSite=None (which we do not use today) is unfortunately interpreted incorrectly by specific browsers, so this PR handles such situations as well.

Changes

Added a middleware that sets all app sessions to SameSite=None and secure if the browser is compatible and the app is an embedded app.

Decided to add a middleware as setting the session option to same_site: :none isn't applied to set-cookie headers for shopify_app assets and /auth/shopify (omniauth)

Tophatting

Tophatted with a new shopify app (where _example_session has secure and SameSite None.

Also tophatted with one of our first party embedded apps Exchange (though I removed the ShopifyApp.configuration.embedded_app == true check to test as Exchange runs both as an embedded app and non embedded app). Also tophatted with the user agent Mozilla/5.0 (iPad; CPU OS 11_3 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/11.0 Tablet/15E148 Safari/604.1.

If the session already has secure and SameSite set, a secure and SameSite will still be appended to the end of the set-cookie header. I personally don't think it's a huge problem as the browser ignores the previous secure and SameSite in the header, but would love to get more opinions on it.

[tanema]

Hey @jenshenny Is this work needed after the additional work being added to Rack and Rails?

[jenshenny]

This work can do without the bump to Rack 2.1.0 and the additional work in Rails. Originally, I was trying to change the session option of same_site to :none in the app which then would need the bump in Rack, but there were many unusual areas where the Set-Cookie header didn't apply the session option change. I figured it would be much simpler to manually change the Set-Cookie header myself.

[rafaelfranca]

Decided to add a middleware as setting the session option to same_site: :none isn't applied to set-cookie headers for shopify_app assets and /auth/shopify (omniauth)

Could you expand on that? Why it doesn't work for auth and the assets?

[JackMc]

This approach looks good for now. In the future I think it would be better if we were able to set this on the session, but given the timeline (Feb 4), we should do this ASAP so app developers have as much time as possible to fix their apps.

[JackMc]

One additional comment, building off Rafael's observation.

I don't believe that this is a blocker on this PR being merged and think that if it will take significant time to implement we should ship a version so developers can start adopting it.

[dmcvittie]

👍 + 🎩'ed on the Launchpad app.

SameSite = None & Secure is set as expected

[tanema]

This is good work! However I just have one touchup I would like to see. I appreciate the added configurability.

[netwire88]

Is this affecting redirects to confirm_recurring_application_charge URL? Currently, it works if the redirect is handled in a non-embedded app browser window, but in the embedded app browser window, redirects don't work.

[tanema]

@netwire88 Please open an issue.

[RichardBlair]

@jenshenny just wanted to thank you for this PR. It's well written and the tests do a good job of documenting what I needed to do. Our app (Parcelify.co) has been around a while, and as such it was less obvious for me were to hook things up. Your PR pointed me to the solution. 👏