search

gnikyt / laravel-shopify

A full-featured Laravel package for aiding in Shopify App development
MIT License
1.24k stars 374 forks source link

Admin Links and Bulk Actions 404 not found #903

Closed mattias closed 3 years ago

mattias commented 3 years ago

Expected Behavior

When I use an admin link or bulk action, I should get a http code 200 and see the app page.

Current Behavior

http code 404 on every admin link and bulk action.

Failure Information

This seems to be related to the new v17 release and it goes as far as to do authenticate.token. There it strips some headers which the middleware verify.shopify needs, and because those then are missing on the next redirect, a 404 occurs instead of showing the page.

In more detail the flow seems to go like this:

  1. It arrives at the link I've specified but gets a 302 to authenticate.token, but this request has the following query string parameters: hmac, host, id, locale, session, shop, timestamp.
  2. authenticate.token gets a 200 and does it's thing, but it seems to remove a lot of headers required for verify.shopify, so the redirect back to the first link now gets a 404. This request had the following query string parameters: shop, target
  3. Now we only have the following query string parameters on the final 404 request: host, id and token.

Diving deeper I can see that the verify.shopify middleware does a tokenRedirect() when missing a token, and there it filters the query parameters. Is this necessary? Am I wrong in thinking this is the cause of the issue? Someone else must have used admin links/bulk actions by now?

Maybe there is another way to fix this instead of messing with filtering? I did try to comment out the exceptions in the filtering, but then I just get a http code 500 and a message about Unable to verify signature..

Steps to Reproduce

  1. Create an admin link or bulk action under extensions on the shopify app settings page.
  2. Go to the place you added the admin link or bulk action and use it.
  3. 404 not found will be shown.

Context

Failure Logs

No logs written.

gnikyt commented 3 years ago

Thanks for digging into this, I'll look at this when I get a chance.

mattias commented 3 years ago

I've looked into it further and after noticing other controllers of different types had their $request data different from before, I found out that these admin links all tries to look for the $request->shop variable, which no longer exists. And if the user and this variable doesn't match name, I had an abort(404);.

I've removed this check since it should already be verified it is the correct user.