Shopify / shopify_app

A Rails Engine for building Shopify Apps
MIT License
1.77k stars 688 forks source link

Cannot download files using `send_data` #1910

Open hirowatari opened 1 month ago

hirowatari commented 1 month ago

Issue summary

Before opening this issue, I have:

Given a rails app with

# app/controllers/failed_rows_csv_controller.rb
class FailedRowsCSVController < ApplicationController
  include ShopifyApp::EnsureHasSession
  # skip_around_action :activate_shopify_session
  def show
    send_data 'foo,bar', filename: 'del.csv'
  end
end

and a link to that route: <a href="/location_import_files/5/failed_rows_csv?session=whatever&amp;shop=prolo-dev.myshopify.com">download</a>

Expected behavior

What do you think should happen?

The file should be downloaded

Actual behavior

What actually happens?

The file is shown in the browser

Steps to reproduce the problem

I think I've detailed it above, but please let me know if you would like more details.

Debug logs

logs where the issue is shown

Started GET "/location_import_files/5/failed_rows_csv?session=67ebebfce25c7668a6ced116994464711faae739888b3c3c773989cc3c10b728&shop=prolo-dev.myshopify.com" for 108.180.124.202 at 2024-09-20 21:13:30 -0700
Cannot render console from 108.180.124.202! Allowed networks: 127.0.0.0/127.255.255.255, ::1
Processing by FailedRowsCSVController#show as HTML
  Parameters: {"session"=>"67ebebfce25c7668a6ced116994464711faae739888b3c3c773989cc3c10b728", "shop"=>"prolo-dev.myshopify.com", "location_import_file_id"=>"5"}
[ ShopifyApp | DEBUG | Shop Not Found ] Responding to invalid Shopify ID token: Missing Shopify ID Token
[ ShopifyApp | DEBUG | Shop Not Found ] Redirecting to bounce page for patching Shopify ID token
Redirected to https://prolo.ngrok.app//patch_shopify_id_token?session=67ebebfce25c7668a6ced116994464711faae739888b3c3c773989cc3c10b728&shop=prolo-dev.myshopify.com&shopify-reload=%2Flocation_import_files%2F5%2Ffailed_rows_csv%3Fsession%3D67ebebfce25c7668a6ced116994464711faae739888b3c3c773989cc3c10b728%26shop%3Dprolo-dev.myshopify.com
[ ShopifyApp | DEBUG | Shop Not Found ] Deactivating session
Completed 302 Found in 2ms (ActiveRecord: 0.2ms | Allocations: 914)

Started GET "//patch_shopify_id_token?session=67ebebfce25c7668a6ced116994464711faae739888b3c3c773989cc3c10b728&shop=prolo-dev.myshopify.com&shopify-reload=%2Flocation_import_files%2F5%2Ffailed_rows_csv%3Fsession%3D67ebebfce25c7668a6ced116994464711faae739888b3c3c773989cc3c10b728%26shop%3Dprolo-dev.myshopify.com" for 108.180.124.202 at 2024-09-20 21:13:30 -0700
Cannot render console from 108.180.124.202! Allowed networks: 127.0.0.0/127.255.255.255, ::1
Processing by ShopifyApp::SessionsController#patch_shopify_id_token as HTML
  Parameters: {"session"=>"67ebebfce25c7668a6ced116994464711faae739888b3c3c773989cc3c10b728", "shop"=>"prolo-dev.myshopify.com", "shopify-reload"=>"/location_import_files/5/failed_rows_csv?session=67ebebfce25c7668a6ced116994464711faae739888b3c3c773989cc3c10b728&shop=prolo-dev.myshopify.com"}
  Rendering layout vendor/bundle/ruby/3.2.0/gems/shopify_app-22.4.0/app/views/shopify_app/layouts/app_bridge.html.erb
  Rendering vendor/bundle/ruby/3.2.0/gems/shopify_app-22.4.0/app/views/shopify_app/sessions/patch_shopify_id_token.html.erb within shopify_app/layouts/app_bridge
  Rendered vendor/bundle/ruby/3.2.0/gems/shopify_app-22.4.0/app/views/shopify_app/sessions/patch_shopify_id_token.html.erb within shopify_app/layouts/app_bridge (Duration: 0.2ms | Allocations: 84)
  Rendered layout vendor/bundle/ruby/3.2.0/gems/shopify_app-22.4.0/app/views/shopify_app/layouts/app_bridge.html.erb (Duration: 2.0ms | Allocations: 653)
Completed 200 OK in 4ms (Views: 3.2ms | ActiveRecord: 0.3ms | Allocations: 1717)

Finished "/cable" [WebSocket] for 108.180.124.202 at 2024-09-20 21:13:30 -0700
Hotwire::Livereload::ReloadChannel stopped streaming from hotwire-reload
Started GET "/location_import_files/5/failed_rows_csv?session=67ebebfce25c7668a6ced116994464711faae739888b3c3c773989cc3c10b728&shop=prolo-dev.myshopify.com" for 108.180.124.202 at 2024-09-20 21:13:30 -0700
Cannot render console from 108.180.124.202! Allowed networks: 127.0.0.0/127.255.255.255, ::1
Processing by FailedRowsCSVController#show as HTML
  Parameters: {"session"=>"67ebebfce25c7668a6ced116994464711faae739888b3c3c773989cc3c10b728", "shop"=>"prolo-dev.myshopify.com", "location_import_file_id"=>"5"}
[ ShopifyApp | DEBUG | Shop Not Found ] Loading session by domain - domain: prolo-dev.myshopify.com
  Shop Load (0.1ms)  SELECT "shops".* FROM "shops" WHERE "shops"."shopify_domain" = $1 LIMIT $2  [["shopify_domain", "prolo-dev.myshopify.com"], ["LIMIT", 1]]
  ↳ app/middlewares/shopify_session_middleware.rb:16:in `call'
[ ShopifyApp | DEBUG | Shop Not Found ] Activating Shopify session
  Rendering text template
  Rendered text template (Duration: 0.0ms | Allocations: 1)
Sent data del.csv (0.4ms)
[ ShopifyApp | DEBUG | prolo-dev.myshopify.com ] Deactivating session
Completed 200 OK in 4ms (Views: 0.3ms | ActiveRecord: 0.4ms | Allocations: 1546)

However if I uncomment skip_around_action :activate_shopify_session then file successfully downloads with the following logs


Started GET "/location_import_files/5/failed_rows_csv?session=67ebebfce25c7668a6ced116994464711faae739888b3c3c773989cc3c10b728&shop=prolo-dev.myshopify.com" for 108.180.124.202 at 2024-09-20 21:14:54 -0700
Cannot render console from 108.180.124.202! Allowed networks: 127.0.0.0/127.255.255.255, ::1
Processing by FailedRowsCSVController#show as HTML
  Parameters: {"session"=>"67ebebfce25c7668a6ced116994464711faae739888b3c3c773989cc3c10b728", "shop"=>"prolo-dev.myshopify.com", "location_import_file_id"=>"5"}
  Rendering text template
  Rendered text template (Duration: 0.0ms | Allocations: 1)
Sent data del.csv (0.3ms)
Completed 200 OK in 2ms (Views: 0.2ms | ActiveRecord: 0.2ms | Allocations: 551)
matteodepalo commented 1 month ago

Hi @hirowatari thank you for opening this issue. Have you tried adding type: 'text/csv' and disposition: 'attachment'? Also I don't see how this is an issue with the library, could you help me understand that?

hirowatari commented 1 month ago

Hi @matteodepalo,

Thanks for looking at this.

Have you tried adding type: 'text/csv' and disposition: 'attachment'?

Yes, I've tried that. I've tried quite a few other things as well and would be happy to list those if that's helpful

Also I don't see how this is an issue with the library, could you help me understand that?

Perhaps I should back up and ask a question in return. What I'm trying to accomplish is to have a route protected by the shopify_app gem. In other places I used ShopifyApp::EnsureHasSession to do this. Should I expect ShopifyApp::EnsureHasSession to work when using send_data? If so, that's why I thought it was a bug in shopify_app. If not, perhaps I'm missing a more obvious way to accomplish this.

Thanks again for your attention to this. I've always appreciated Shopify support going back many years.

matteodepalo commented 1 month ago

I haven't seen this issue with send_data before, so I'm not sure wether ShopifyApp::EnsureHasSession is creating problems with that. Have you tried removing the line include ShopifyApp::EnsureHasSession and seeing if it correctly downloads the document?

hirowatari commented 1 month ago

Have you tried removing the line include ShopifyApp::EnsureHasSession and seeing if it correctly downloads the document?

Yes.

Sorry I wasn't clearer. In the first post I tried to explain that ShopifyApp::EnsureHasSession caused this, specifically, :activate_shopify_session. In fact, adding skip_around_action :activate_shopify_session allows the file to download as expected.

matteodepalo commented 1 month ago

Thank you for your reply @hirowatari , I've flagged this internally so that the team can look at it.

zzooeeyy commented 1 month ago

Hey @hirowatari, I noticed in your log that the id_token is not available

[ ShopifyApp | DEBUG | Shop Not Found ] Responding to invalid Shopify ID token: Missing Shopify ID Token

The id_token is used by EnsureSession to authenticate the user to ensure this request is coming from a user from the Shopify admin embedded app. Are you using app bridge to make authenticated fetch to your server? That should include the id token as a part of the request header.

hirowatari commented 1 month ago

Are you using app bridge to make authenticated fetch to your server? That should include the id token as a part of the request header. Yes

I'm not mistaken, the session is there and shown in the first line of the logs.

Started GET "/location_import_files/5/failed_rows_csv?session=67ebebfce25c7668a6ced116994464711faae739888b3c3c773989cc3c10b728&shop=prolo-dev.myshopify.com" for 108.180.124.202 at 2024-09-20 21:13:30 -0700

I believed, the session would need to be there and valid otherwise the request would fail (403). I'm not having an issue with the request failing. The issue is with the request is shown in the browser when it should be downloaded.

I'm concerned that we're not making any progress here. Is there a way I could show you the issue that you would believe? Perhaps with a sample Rails application? Do you have a template or any guidance here?

zzooeeyy commented 1 month ago

session= isn't the parameter the ShopifyApp gem uses to authenticate the user.

The JWT id token that the Gem uses is either in:

Here are some references for authentication


hirowatari commented 1 month ago

Sorry for the confusion about where the token is stored. The authorization header is there as well (see attached) Image

It does not change between the first request that redirects to patch_shopify_id_token and the request that eventually works but shows the file instead of downloading it.

I'm concerned that we're not making any progress here. Is there a way I could show you the issue that you would believe? Perhaps with a sample Rails application? Do you have a template or any guidance here?

zzooeeyy commented 1 month ago

Hmm yea that is quite strange.. If you could provide a sample Rails application that'd be really helpful. Thanks

hirowatari commented 1 month ago

Hopefully, this repo will show the issue: https://github.com/hirowatari/bug-report-shopify-send-file

zzooeeyy commented 1 month ago

Thank you @hirowatari, I can see the issue, I'll bring this to the team and see what's the best solution.

hirowatari commented 3 weeks ago

Thanks @zzooeeyy. Was there are progress on this? Or any advice on how to workaround the issue?