Closed CetoBasilius closed 2 years ago
CetoBasilius
commented
2 years ago @Kyon147 Seems removing the host and billing (Line 96, 97) from the 'route_names.home' redirect, from src/Traits/BillingController.php in your last commit 695f968859be1bf041c5b1df8d67640502e5ce1a Fixes this,
I imagine these were put in to fix something else, why were these put in?
CetoBasilius
commented
2 years ago @Kyon147 Saw that your changes are important and relevant for the new flag frontend_engine so i used the Util::useNativeAppBridge() function to determine if host and billing params are sent.
Separated the test case in 2 different scenarios
Don't know if this is the optimal solution but it works for both cases. @osiset what would be your comments?
CetoBasilius
commented
2 years ago Just saw the discussion at #1250 so this might not be added after all from what i see
Kyon147
commented
2 years ago Hi @CetoBasilius
Thanks for the detailed bug report.
I have seen Shopify uses the url that you enter in your app partner config as the url to target for auth etc, can you confirm you have added https and not http.
I can't see any reason why adding the host param on the billing route would break the BLADE route, as the existing param is still there shop and one was just an addition.
The param was added because it is needed for AppBridge once you are redirected back from Shopify's bulling screen otherwise the app won't load correctly because AppBridge can't initialise again. This happens when using SPA more than Blade as Blade templates use the auth.token middleware.
Kyon147
commented
2 years ago I do remember someone else mentioning this last week with a potential fix, I just can't find the comment.
Will do a little digging to see if that fix worked for them.
d-shannon
commented
2 years ago If anyone needs a quick hacky fix, I am catching the exception in Handler.php and redirecting to home route.
CetoBasilius
commented
2 years ago Hi @Kyon147, i can confirm it's https and that was not the problem, using valet + ngrok. Debugged yesterday, the hmac calculation breaks when it receives the host and billing params when billing redirects to home.
I added a small detection on the BillingController trait so that it checks if the frontend_engine flag is set to REACT, if so, it does add the params. I added another test case modifying the flag so that your changes continue to work on PR #1263
Reading the "Outdated App" issue seems we will all be headed towards a SPA anyway so I'm not sure if this PR will be merged
nonghinh
commented
2 years ago Same issue
Expected Behavior
User approves recurring billing charge and should be redirected to home route
Current Behavior
I approve recurring billing charge, error page is presented with message "Unable to verify signature." If i navigate to the homepage/onboarding/settings of the app AFTER the error, everything continues working, just the billing approve redirect fails
Failure Information
Everything was working fine before i upgraded from 17.3.0 (Custom fork with Laravel9 Support) to 17.3.2 Api key & secret is OK, hmac check fails as it is different from what it expects when debugging If i remove the billable middleware everything works fine
shopify-app config notes:
I have tried removing the prefix, setting default routes & more
Steps to Reproduce
Context
Downgraded to PHP 8.0.25 and the error persists. Using xDebug to see exactly whats going on, hmac verification fails on BasicShopifyAPI.php on line 376, hmacs are different.
Request on which it crashed is (Removed some data with asterisks and my ngrok address)
Seems like it's trying to use http instead of https?
Attached here is part of query string
target query string has
&billing=successFailure Logs
[2022-11-14 15:19:08] local.ERROR: Unable to verify signature. {"exception":"[object] (Osiset\ShopifyApp\Exceptions\SignatureVerificationException(code: 0): Unable to verify signature. at (REDACTED)/vendor/osiset/laravel-shopify/src/Http/Middleware/VerifyShopify.php:96)