shopify-nextjs-toolbox

shopify-nextjs-toolbox copied to clipboard

Bug Description:

Shopify throws an "Invalid Signature" error on App Subscription Charge acceptance for the merchant and they are unable to continue forward.

To Reproduce:

in pages/api/auth/callback.js:

Create a custom afterAuth function to create a subscription. Retrieve the confirmationUrl from the appSubscriptionCreate response return confirmationUrl from function

          //  const recurringApplicationCharge = ....                    

         //fetch confirmationURL
            const redirectUrl =
                recurringApplicationCharge.body.data.appSubscriptionCreate
                    .confirmationUrl;

            // Redirect to billing url
            return redirectUrl;

Upon returning control to handleAuthCallback.js, the following code block executes:

      res.redirect(
        `${redirectPath || process.env.HOME_PATH}?${querystring.stringify(
          req.query
        )}`

The resulting redirectURL is set to

https://neutrl-test.myshopify.com/admin/charges/6343291/22336962698/RecurringApplicationCharge/confirm_recurring_application_charge?signature=BAh7BzoHaWRsKwiKAGMzBQA6EmF1dG9fYWN0aXZhdGVU--f798ca9351ce0b009fb468ec84047546fe1bc748?code=574ffe545a8611d071742065c1ef679e&hmac=9092dcabfa0b802eb11bfc0cb40e834245ed466c407c20f0beee2636f6d1cec1&host=bmV1dHJsLXRlc3QubXlzaG9waWZ5LmNvbS9hZG1pbg&shop=neutrl-test.myshopify.com&state=f18b54560a470f7f76a03b426918db27&timestamp=1642099550

The issue is that the redirect string is adding in an additional ? whereas in the overridden scenario, it should append an & as there is already a query parameter present in the URL string.

@melissadonohue thanks, the library wasn't written with redirecting to a subscription consent URL immediately in the auth handler in mind.

I assume that's the problem specifically you were running into.

Sounds like you were trying to :

1. Start OAuth
2. On OAuth callback, generate a subscription confirmation URL
3. Return this URL to the frontend to redirect the user to

In general the flow this lib supports out of the box is:

1. Start OAuth
2. Handle OAuth callback
3. Home page is loaded
4. Then allow the user to use the app in free mode, or redirect immediately to a subscription consent URL from a different API route on your app

Hey @ctrlaltdylan - Understood, I think we need to get the README.md updated in that case, as you call out the ability to do this here:

@ctrlaltdylan @melissadonohue this problem will be fixed if we change the ? to & here: https://github.com/ctrlaltdylan/shopify-nextjs-toolbox/blob/6f6d48621f48552e4b087a14a1159babd75d2d0d/src/middleware/oauth/handleAuthCallback.js#L42

in better words, this line assumed it will take the return url and append 1 param while in the scenario above there is already a param which is the signature from the charge callback.

@ctrlaltdylan is it ok if i open a PR for this? I have the same issue and it would be great if we improve the library for all

Thanks for the call out @melissadonohue , I started with a fix months ago and didn't commit it because I was having trouble blending the query params from the subscription redirect against the query params from the oauth callback.

I must have committed the content as if I fixed it, my bad.

But I guess the OAuth params don't need to be blended after all if you're redirecting the user directly back to Shopify for the subscription confirmation.

My main concern was that the frontend would render a 401/error page because the signature failed and app bridge couldn't instantiate.

@ahmed-adly-khalil I would accept either PR, but I could feel a lot better tagging a release if there was a basic test included too. The handleAuthCallback.test.js should include a test case with this scenario.

@ctrlaltdylan sounds great, I'm having even a more complex scenario, my app is not embedded, so I will try to cover that in the PR as well, not sure how exactly but I will work on it

@ctrlaltdylan I guess you also got the error Validation failed: Return url is too long (maximum is 255 characters) when merging oauth params to the charge return url, this makes it a bit harder, maybe the solution now is a custom session?

Building off of @melissadonohue 's solution using query string detection, this might solve for most cases:

// if a redirect path or URL is returned from `handleAuthCallback` use it
const newPath = redirectPath || process.env.HOME_PATH;

// if this new path contains a `?`, it's containing a query string and we should not attempt to double query string'ify
const newQueryString = !newPath.includes('?') && `?${querystring.stringify(req.query)}`;

// redirect to the Shopify Subscription page, or the normal internal path with the AppBridge params
res.redirect(newPath + newQueryString);