shop is getting undefined when redirecting to /api/auth?shop=undefined

[yashsony]

Issue summary

when session verification fails then it redirects to /api/auth and passes shop in query parameters but here shop is undefined
/api/auth?shop=undefined

it doesn't happen on development server but it happens on prod server
followed official doc for fly.io deployment but this error exist

• @shopify/shopify-app-* package and version:
• Node version: v18.12.1
• Operating system: don't know (fly.io server)

Expected behavior

it should redirect to /api/auth?shop=mystore.myshopify.com

Actual behavior

/api/auth?shop=undefined

Steps to reproduce the problem

1. clone this repo https://github.com/yashsony/deploymenttesting
2. and deploy it on fly.io through cli link

[alexissel]

I'm having the same issue on the latest version v1.1.0 but v1.0.1 seams to work fine.

| backend  | Webhook for app uninstall was successfully handled
| backend  | [shopify-app/INFO] Webhook processed, returned status code 200
| backend  | [shopify-app/INFO] Running validateAuthenticatedSession
| backend  | [shopify-app/INFO] Session was not valid. Redirecting to /api/auth?shop=undefined | {shop: undefined}
| backend  | [shopify-app/ERROR] No shop provided to redirect to auth

Steps to reproduce:

1. Open the app in two tabs
2. Uninstall the app in the first tab
3. In the second tab click on something that makes a back-end call
4. The app should redirect you back to the installation page /api/auth?shop=shop

[yashsony]

yes older version v1.0.1 works fine @alexissel

[TudorBNG]

Hi. I'm having the same issue as @alexissel:

2023-01-25 15:01:20 | backend    | [shopify-app/INFO] Running validateAuthenticatedSession
2023-01-25 15:01:20 | backend    | [shopify-app/INFO] Running validateAuthenticatedSession
2023-01-25 15:01:20 | backend    | [shopify-app/INFO] Session was not valid. Redirecting to /api/auth?shop=undefined | {shop: undefined}
2023-01-25 15:01:20 | backend    | [shopify-app/INFO] Session was not valid. Redirecting to /api/auth?shop=undefined | {shop: undefined}
2023-01-25 15:01:20 | backend    | [shopify-app/ERROR] No shop provided to redirect to auth

Node version: 16.18.1

Packages:

• @shopify/app: 3.30.1
• @shopify/cli: 3.30.1
• @shopify/shopify-app-express: 1.1.0

It's great that it works on v1.0.1, but I believe that there are some other gaps there, which have been covered in v1.1.0. So it would be great to have a fix for this redirect after uninstall issue in v1.1.1 as it is a requirement in the current app approval process 😄

[JeaneC]

+1 started to see this as well

2023-01-29T06:18:34.841 app[8814583e] mia [info] [shopify-app/INFO] Running ensureInstalledOnShop
2023-01-29T06:18:34.897 app[8814583e] mia [info] [shopify-app/INFO] Running ensureInstalledOnShop
2023-01-29T06:18:34.899 app[8814583e] mia [info] [shopify-app/INFO] App is installed and ready to load | {shop: 126teststorejc.myshopify.com}
2023-01-29T06:18:35.212 app[8814583e] mia [info] [shopify-api/INFO] Beginning OAuth | {shop: 126teststorejc.myshopify.com, isOnline: false, callbackPath: /api/auth/callback}
2023-01-29T06:18:35.356 app[8814583e] mia [info] [shopify-app/INFO] Running validateAuthenticatedSession
2023-01-29T06:18:35.358 app[8814583e] mia [info] [shopify-app/INFO] Session was not valid. Redirecting to /api/auth?shop=undefined | {shop: undefined}

[FrankHeijden]

It seems the backend is only checking for offline tokens when requesting a page which makes use of the shopify. ensureInstalledOnShop() middleware, and not checking for a valid online access token (link).

In my case (pretty similar to the node app template), the ensureInstalledOnShop middleware is called for frontend pages. Once a frontend page is loaded, most of the pages do a backend request to /api/*, where the middleware validateAuthenticatedSession is called. This middleware finally detects that the session is not valid using online tokens, and will attempt to redirect to /api/auth, but because no valid session is found or shop query parameter, it will fill in undefined (link).

So I see two ways to fix this, either make all backend requests to /api/* (or wherever validateAuthenticatedSession is used) and add the query parameter with the shop (not ideal, since the frontend page has already been loaded) or redirect to create a new access token when the ensureInstalledOnShop middleware is called. The latter is trickier and I am not sure how to achieve it since the request does not contain the online session data.

[JeaneC]

Nice find @FrankHeijden . I tried to debug that before but hard a hard time. From that info, I tried to intercept the middleware for /api/* since my pages, like most, request it immediately

app.use(
  "/api/*",
  async (req, res, next) => {
    try {
      const sessionId = await shopify.api.session.getCurrentId({
        isOnline: shopify.config.useOnlineTokens,
        rawRequest: req,
        rawResponse: res,
      });
      const session = await shopify.config.sessionStorage.loadSession(
        sessionId
      );
      const shop = req.query.shop || session?.shop;

      if (!shop) {
        return undefined;
      }
    } catch (e) {
      console.error(e);
    }

    next();
  },
  shopify.validateAuthenticatedSession()
);

[granteagon]

I put the above solution in place and it worked, but this was working before. I'm not sure why it suddenly doesn't work. I didn't upgrade my packages and it went from a working status to a non-working status.

[jaret32]

I am having the same issue. It is happening when the app (using both offline and online access tokens) has already been installed for a shop, and a new user with no session then tries to access the app. Because the app has a valid offline session token, the ensureInstalledOnShop() middleware never sends the new user through the authentication flow, so when the validateAuthenticatedSession() middleware is called, the shop parameter is undefined because it is not included in the request to the api and the user does not have a session.

[walkingbrad]

+1 same issue

I've seen it occur in both dev (using sqlite session storage) and in prod (using redis session storage). I haven't uninstalled my app on the stores that have the issue, so the cause cannot be related to uninstallation. But the issue does seem to occur when I simply wipe the sessions from session storage (e.g. delete my database.sqlite file in dev or clear all entries from Redis).

By referencing the logs that Shopify spits out, the issue does seem to occur somewhere in validateAuthenticatedSession. Since some say this wasn't an issue in 1.0.1, I took a look at the old implementation of that method. If we compare that to the latest code in 1.1.0, we see that the old code has an additional branch in which it can determine the value of the shop parameter. Presumably, reintroducing this logic would solve the issue:

else if (api.config.isEmbeddedApp) {
  const payload = await api.session.decodeSessionToken(token);
  shop = payload.dest.replace('https://', '');
}

[alexissel]

Why is this still not fixed in v1.2.0?

[rycastaneda]

+1
Having this issue, I went with the old implementation on returnTopLevelRedirection

[mkevinosullivan]

Fixed in @shopify/shopify-app-express@1.2.1

[alexissel]

Thanks Kevin!

[olafghanizadeh]

This issue still persists in @shopify/shopify-app-express@1.2.2

[AleksandrBy]

Yes. In @shopify/shopify-app-express@2.0.0 too

[AleksandrBy]

Dear Developers, @mkevinosullivan @cquemin @paulomarg @refactor-this @teddyhwang @ajshepley

I am writing to bring to your attention a critical issue that is affecting the performance of the Shopify apps developed using your library. As you may already be aware, there is a bug in your code that needs immediate attention.

This issue has caused inconvenience and loss of revenue for many businesses, including small companies and individual developers like myself, who rely on your library to build applications. It is not only frustrating but also damages the reputation of our companies and the trust of our users.

I understand that fixing bugs can be a challenging and time-consuming task. However, I implore you to prioritize this issue and provide a resolution as soon as possible. If a quick fix is not feasible, it would be appreciated if you could suggest a temporary solution or workaround to mitigate the impact of the bug on our applications.

I am confident that you will address this problem promptly and efficiently, and I appreciate your efforts in ensuring that your library provides a reliable and stable foundation for the development of Shopify apps.

Thank you for your attention to this matter.

Sincerely,

@AleksandrBy

[vitordiniz22]

Oh, I just had this issue, and I realized that I was not making the frontend request with the hook useAuthenticatedFetch. Using the hook to make the request solved my problem.

[pschoffer]

Same issue here. My fail was that when implementing session storage strategy I would return empty session object if session didn't exist instead of undefined:

Good

    async loadSession(id) {
        const session = await admin.firestore().collection(COLLECTIONS.sessions).doc(id).get();
        if (!session.exists) {
            return undefined;
        }
        return new Session(session.data());
    }

Bad

    async loadSession(id) {
        const session = await admin.firestore().collection(COLLECTIONS.sessions).doc(id).get();
         return new Session(session.data());
    }