search

Shopify / shopify_app

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

Navigating from Shopify Admin into App page causes immediate reload #1849

Closed MauroVentosa closed 3 weeks ago

MauroVentosa commented 1 month ago

Issue summary

Before opening this issue, I have:

Expected behavior

Navigating to an App page should load the target page and not redirect to home. If there is an authentication error or any other error, it should be displayed either on the browser's console, or in the App logs.

Actual behavior

Navigating to an App page from the Admin (Create Discounts menu) loads the desired view, then immediately gets redirected to home without a clear error.

This happens specifically after upgrading shopify_app to 22.2.0 or greater. The error goes away with version 22.1.0 or lower

Steps to reproduce the problem

  1. Upgrade shopify_app to 22.2.0 or 22.2.1
  2. Navigate to Shopify Admin (Discounts admin)
  3. Create new discount type that redirects into App page
  4. Expected page loads and is visible, immediately gets redirected

Debug logs

11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | Started GET "/discounts/01cd81b7-e2c2-4621-8674-2173ff4db756/free_shipping
_on_monday_discounts/new?embedded=1&hmac=[REDACTED]&host=[REDACTED]&id_token=[FILTERED]&locale=en-US&session=[REDACTED]&shop=mejuri-staging-inc.myshopify.com&timestamp=1716213772" for 127.0.0.1 at 2024-05-20 11:02:53 -0300
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | Processing by Discounts::FreeShippingOnMondayDiscountsController#new as 
HTML
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Parameters: {"embedded"=>"1", 
"hmac"=>"[REDACTED]", 
"host"=>"[REDACTED]", "id_token"=>"[FILTERED]", "locale"=>"en-US", 
"session"=>"[REDACTED]", "shop"=>"mejuri-staging-inc.myshopify.com", 
"timestamp"=>"1716213772", "discount_id"=>"01cd81b7-e2c2-4621-8674-2173ff4db756"}
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Shop Load (2.4ms)  SELECT "shops".* FROM "shops" WHERE 
"shops"."shopify_domain" = $1 LIMIT $2  [["shopify_domain", "mejuri-staging-inc.myshopify.com"], ["LIMIT", 1]]
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | [ ShopifyApp | INFO | mejuri-staging-inc.myshopify.com ] Installed store  
- mejuri-staging-inc.myshopify.com deduced from user session
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   CACHE Shop Load (0.4ms)  SELECT "shops".* FROM "shops" WHERE 
"shops"."shopify_domain" = $1 LIMIT $2  [["shopify_domain", "mejuri-staging-inc.myshopify.com"], ["LIMIT", 1]]
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   ↳ app/controllers/concerns/current_shop.rb:17:in `current_shop'
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | [ ShopifyApp | INFO | mejuri-staging-inc.myshopify.com ] Installed store  
- mejuri-staging-inc.myshopify.com deduced from user session
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendering layout layouts/embedded_app.html.erb
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendering discounts/free_shipping_on_monday_discounts/new.html.erb 
within layouts/embedded_app
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendered discounts/_form.html.erb (Duration: 37.5ms | Allocations: 
17952)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendered discounts/free_shipping_on_monday_discounts/new.html.erb within
 layouts/embedded_app (Duration: 67.9ms | Allocations: 29619)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendered /Users/mauro.ventosa/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/
gems/hotwire-livereload-1.4.0/app/views/hotwire/livereload/_head_action_cable.html.erb (Duration: 0.3ms | Allocations: 222)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendered layouts/_head.html.erb (Duration: 3.1ms | Allocations: 5887)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendered layouts/_navigation_menu.html.erb (Duration: 0.6ms | 
Allocations: 596)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendered layout layouts/embedded_app.html.erb (Duration: 76.1ms | 
Allocations: 38316)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | Completed 200 OK in 247ms (Views: 78.1ms | ActiveRecord: 46.5ms | 
Allocations: 148445)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | 
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | 
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | [ ShopifyApp | INFO | Shop Not Found ] Installed store  - 
mejuri-staging-inc.myshopify.com deduced from user session
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | Started GET "/cable" for 127.0.0.1 at 2024-05-20 11:02:53 -0300
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | Started GET "/home?shop=mejuri-staging-inc.myshopify.com" for 127.0.0.1 at
 2024-05-20 11:02:53 -0300
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | Started GET "/cable" [WebSocket] for 127.0.0.1 at 2024-05-20 11:02:53 
-0300
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | Successfully upgraded to WebSocket (REQUEST_METHOD: GET, HTTP_CONNECTION: 
keep-alive, Upgrade, HTTP_UPGRADE: websocket)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | Hotwire::Livereload::ReloadChannel is transmitting the subscription 
confirmation
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | Hotwire::Livereload::ReloadChannel is streaming from hotwire-reload
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | Processing by HomeController#show as HTML
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Parameters: {"shop"=>"mejuri-staging-inc.myshopify.com"}
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Shop Load (1.0ms)  SELECT "shops".* FROM "shops" WHERE 
"shops"."shopify_domain" = $1 LIMIT $2  [["shopify_domain", "mejuri-staging-inc.myshopify.com"], ["LIMIT", 1]]
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | [ ShopifyApp | INFO | mejuri-staging-inc.myshopify.com ] Installed store  
- mejuri-staging-inc.myshopify.com deduced from user session
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   CACHE Shop Load (0.4ms)  SELECT "shops".* FROM "shops" WHERE 
"shops"."shopify_domain" = $1 LIMIT $2  [["shopify_domain", "mejuri-staging-inc.myshopify.com"], ["LIMIT", 1]]
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   ↳ app/controllers/concerns/current_shop.rb:17:in `current_shop'
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | [ ShopifyApp | INFO | mejuri-staging-inc.myshopify.com ] Installed store  
- mejuri-staging-inc.myshopify.com deduced from user session
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendering layout layouts/embedded_app.html.erb
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendering home/show.html.erb within layouts/embedded_app
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendered home/show.html.erb within layouts/embedded_app (Duration: 
42.0ms | Allocations: 14375)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendered /Users/mauro.ventosa/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/
gems/hotwire-livereload-1.4.0/app/views/hotwire/livereload/_head_action_cable.html.erb (Duration: 0.0ms | Allocations: 96)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendered layouts/_head.html.erb (Duration: 0.5ms | Allocations: 829)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendered layouts/_navigation_menu.html.erb (Duration: 0.2ms | 
Allocations: 249)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendered layout layouts/embedded_app.html.erb (Duration: 44.3ms | 
Allocations: 16405)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | Completed 200 OK in 51ms (Views: 44.5ms | ActiveRecord: 0.5ms | 
Allocations: 19431)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | 
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | 
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  | [ ShopifyApp | INFO | Shop Not Found ] Installed store  - 
mejuri-staging-inc.myshopify.com deduced from user session

Additional Info

I suspect either this redirect introduced in 22.2.0 or the new token exchange functionality, although it should be opt-in, and I've not enabled it.

matteodepalo commented 1 month ago

@MauroVentosa thank you for opening this issue, we're taking a look.

resistorsoftware commented 1 month ago

I updated to Shopify App 22.2.1 and can confirm. For no apparently clear reason, when an ADMIN LINK from Shopify hits the App, the router executes the controller and completes fine, but the App then redirects to the HOME PAGE!!!

WOW. I am trying to debug this, but with the logs all clean, it'll take a few debug steps to isolate where this is happening!

As per the above pointing to a certain new redirect, my Admin Link is hitting my controller with a host and a shop parameter, so that should not be the issue for me. 

 Parameters: {"embedded"=>"1", "hmac"=>"70e2dd3711ba4a5203ddc786ae59ce66fdca51df825744e7d55709651342383b", "host"=>"YWRtaW4uc2hvcGlmeS5jb20vc3RvcmUvaG90d2lyZS1yZXNpc4rvcg",
"id"=>"7670628483327", "id_token"=>"[FILTERED]", "locale"=>"en", "session"=>"d520f279c4cec223aa30846cf8c4a36f897c1a218d4abeab234f7b8994fec86e", "shop"=>"hotwire-vodka.myshopify.com", "timestamp"=>"1716306044"}
MauroVentosa commented 1 month ago

Hi @matteodepalo any updates on this issue??

uurcank commented 1 month ago

@resistorsoftware can you show what your splash page controller and authenticated controller looks like?

uurcank commented 1 month ago

Can you delete skip_before_action and try again?

With token exchange it should not be necessary.

uurcank commented 1 month ago

I just remember i removed mine because helpers no longer have that. It was causing a redirect in my case.

uurcank commented 1 month ago

Also in token exchange we should not need a splash controller, the app route should be authenticated root.

resistorsoftware commented 1 month ago

Hi @matteodepalo any updates on this issue??

Been a couple of weeks on this, no progress in finding the culprit?

zzooeeyy commented 1 month ago

Hey everyone! I took a look to try and find out what changed in the code that might have caused this and found that the difference between 22.2 and 22.1 is that 22.2 can validate and decode the Shopify session token from the id_token URL param without redirecting to splash page to get the session token from app bridge in the form of the HTTP_AUTHORIZATION header. This means that the app might never call redirect_to_splash_page because "missing_exepcted_jwt?" might not be true.

This means if your app depends on return_to param to reload a path, it will not be set from the EnsureAuthenticatedLinks controller and thus not work.

The following is a huge assumption to try and help debug your app, please provide your implementation of embedded_app.html.erb if my assumption is wrong -

I noticed that you're using another gem hotwire-livereload, this makes me assume that you MIGHT be using their hotwire sample app. 

11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendered /Users/mauro.ventosa/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/
gems/hotwire-livereload-1.4.0/app/views/hotwire/livereload/_head_action_cable.html.erb (Duration: 0.3ms | Allocations: 222)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendered layouts/_head.html.erb (Duration: 3.1ms | Allocations: 5887)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendered layouts/_navigation_menu.html.erb (Duration: 0.6ms | 
Allocations: 596)
11:02:53 │ web-frontend-backend │ 11:02:53 web.1  |   Rendered layout layouts/embedded_app.html.erb (Duration: 76.1ms |

If your embedded_app.html.erb resembles this, then the load_path will always be home_path since params[:return_to] will be nil from shopify_app gem due to not having to redirect to splash page.. This could be why you're seeing the "immediate reload" to your app's home path.

You could try load_path: request.fullpath to see if that helps, although I'm not sure what other consequences this might trigger for your app since I can't see your implementation.

I think this issue stems from how your app handles re-loading paths and not from shopify_app gem redirecting your app to home. It just no longer redirects to the splash page since it's able to validate the session token without the splash page.

resistorsoftware commented 1 month ago

Thanks @zzooeeyy for your effort! You point out the flaw perfectly. The load path will have to be figured out.

github-actions[bot] commented 3 weeks ago

We are closing this issue because we did not hear back regarding additional details we needed to resolve this issue. If the issue persists and you are able to provide the missing clarification we need, feel free to respond and reopen this issue.

We appreciate your understanding as we try to manage our number of open issues.