Shopify / shopify_app

A Rails Engine for building Shopify Apps
MIT License
1.74k stars 685 forks source link

Flash messages not working after updating the gem from 18.1 to the latest version #1796

Open CaioFML opened 4 months ago

CaioFML commented 4 months ago

Issue summary shopify_api version: 12.4.0 shopify_app version: 21.10.0 Ruby version: 3.2.3 Rails 6.1.7.7

Expected behavior Redirect with flash messages set on cookies should be displayed by the Toast function shared by the gem templates in _flash_messages layout and js, also in the embedded_app.html.erb layout

Actual behavior I have updated my shopify_app and shopify_api from 18.1 and 9.4 versions to the latest ones, with that I found the issue related to the flash messages, which I saw the same problem in this issue https://github.com/Shopify/shopify_app/issues/1641

It appears the SameSiteCookieMiddleware was deleted with the release of ShopifyApp 19.0.0. I couldn't find any reference to this in the documentation. This makes Chrome ignore the cookie session, so flash messages stop working. The session_store.rb template in ShopifyApp does not include the .SameSite='None' and Secure options.

Using the suggested fix it works:

Rails.application.config.session_store(:cookie_store, key: 'some_key', expire_after: some_days, secure: true, same_site: 'None')

Which works but are highlighted by Chrome that will be deprecated soon

I can see that the content used by shopify to read the flash messages does not update at all:

image

Steps to reproduce the problem: Create a brand new project with ShopifyApp 21.4.0 and Rails 6.1 + Add a flash message and redirect to any page inside the app. In Chrome, check the response headers for cookies and check the options.

CaioFML commented 4 months ago

I'm also using the session_store as:

Rails.application.config.session_store(:active_record_store)

With that gem, because in my previous version the cookies were being set exceeding the size of the app and raising an error

matteodepalo commented 4 months ago

Hi @CaioFML, thank you for opening this issue, I'm going to put it in your backlog so that we can look at it.

CaioFML commented 2 months ago

@matteodepalo Any updates on this?