shopify-api-js icon indicating copy to clipboard operation
shopify-api-js copied to clipboard

Custom session storage generates multiple session IDs from single app install

Open g-hamilton opened this issue 4 years ago • 25 comments

I'm not 100% sure this is a bug, but it seems to be unexpected behaviour so I need to ask...

I built a Node shopify app using the shopify CLI (recent version).

In my server.js file, I'm initialising the Shopify context to use custom session storage, like so:

Shopify.Context.initialize({
  API_KEY: process.env.SHOPIFY_API_KEY,
  API_SECRET_KEY: process.env.SHOPIFY_API_SECRET,
  SCOPES: process.env.SCOPES.split(","),
  HOST_NAME: process.env.HOST.replace(/https:\/\//, ""),
  API_VERSION: ApiVersion.October20,
  IS_EMBEDDED_APP: true,
  SESSION_STORAGE: new Shopify.Session.CustomSessionStorage(
    storeCallback,
    loadCallback,
    deleteCallback
  ),
});

I used the guide to define the 3 required callback methods. I'm using Firebase for my custom storage solution, so my code looks like this:

// https://github.com/Shopify/shopify-node-api/blob/main/docs/usage/customsessions.md

/*
  The storeCallback takes in the Session, and sets it in Firestore
  This callback is used for BOTH saving new Sessions and updating existing Sessions
  Returns a Firebase write result if the session can be stored
*/
const storeCallback = async (session) => {
  console.log(
    `Custom session storage storeCallback fired with id [${session.id}]`
  );
  try {
    await db
      .doc(`app-sessions/${session.id}`)
      .set(JSON.parse(JSON.stringify(session)), { merge: true });
    return true;
  } catch (err) {
    throw new Error(err);
  }
};

/*
  The loadCallback takes in the id, and tries to retrieve the session data from Firestore
  If a stored session exists, it's returned
  Otherwise, return undefined
  */
const loadCallback = async (id) => {
  console.log(`Custom session storage loadCallback fired with id [${id}]`);
  try {
    const sessionSnapshot = await db
      .doc(`app-sessions/${id}`)
      .get();
    if (!sessionSnapshot.exists) {
      console.log(`Custom session storage session id [${id}] does not exist`);
      return undefined;
    }
    const session = sessionSnapshot.data();
    if (!session) {
      console.log(`Custom session storage session id [${id}] no data`);
      return undefined;
    }
    return session;
  } catch (err) {
    throw new Error(err);
  }
};

/*
    The deleteCallback takes in the id, and attempts to delete the session from Firestore
    If the session can be deleted, return true,
    otherwise, return false
  */
const deleteCallback = async (id) => {
  console.log(`Custom session storage deleteCallback fired with id [${id}]`);
  try {
    const sessionSnapshot = await db
      .doc(`app-sessions/${id}`)
      .get();
    if (!sessionSnapshot.exists) {
      console.log(`Custom session storage session id [${id}] does not exist`);
      return false;
    }
    await db.doc(`app-sessions/${id}`).delete();
    return true;
  } catch (err) {
    throw new Error(err);
  }
};

What I would expect to see when a user installs the app is a single session object being stored in the DB.

What I actually see is 2 session objects being stored, each with a different session ID.

Here is the relevant portion of my logs from a single app install on my development store at the point where the custom session methods are being executed:

2021-07-28T16:36:38.847238+00:00 app[web.1]: Custom session storage loadCallback fired with id [3b8ecc7b-2f8b-401e-9ef0-dc4dee1a0c6c]
2021-07-28T16:36:39.399084+00:00 app[web.1]: Custom session storage storeCallback fired with id [my-test-store.myshopify.com_75101241518]
2021-07-28T16:36:39.503989+00:00 app[web.1]: Custom session storage storeCallback fired with id [3b8ecc7b-2f8b-401e-9ef0-dc4dee1a0c6c]
2021-07-28T16:36:39.602216+00:00 app[web.1]: Custom session storage loadCallback fired with id [3b8ecc7b-2f8b-401e-9ef0-dc4dee1a0c6c]
2021-07-28T16:36:39.696272+00:00 app[web.1]: after auth...

Note:

  • Both session objects have an identical session.accessToken and session.onlineAccessInfo.associatedUser.session, but different session.id and session.expires value.

So my questions:

  1. Why is storeCallback firing twice, with 2 different session IDs even though as a user, I've only started one session from one app install?
  2. When a user loads the app, I'm planning on inspecting the session.expires to decide whether to send the user back into the oauth flow (I think that is the correct implementation but stop me if I'm wrong). With 2 different sessions for the same user, which one should I inspect? I've tested locally and ended up with 4 session objects from the same app/install, which is really odd! I'm guessing that if I answer question 1, question 2 becomes irrelevant.

Thanks in advance for the help :)

g-hamilton avatar Jul 28 '21 16:07 g-hamilton

Great question @g-hamilton! Sadly, I don't have any answers for you as I have the same issue and trying to figure how to fix it.

However, from what I've found for the moment, those 2 tokens have different concerns.

  • The one with UUID as ID is the user session token
  • The other one (my-test-store.myshopify.com_75101241518) is the store session token

The store token expire after 24h. The user token expire after 60 seconds.

From what I understand, the user session token should not be stored into your database but the store session should since it's the token you need when your app call Shopify API on WebHook events.

Keep tokens with online access in a user's temporary session storage, backed by a cookie in the user's browser, and make API requests using this access token in response to the user's requests.

(source: https://shopify.dev/apps/auth/access-modes#best-practices)

I think the user token should be an online access token and the store token should be an offline access token. Online token expires. Offline token don't.

So, from my understanding, we should have a way to generate and handle user/shop session tokens but I don't see it in the actual @shopify/shopify-api documentation.

arsnl avatar Jul 28 '21 19:07 arsnl

Hey folks! Thanks for raising this. This is a know issue we're currently working on. Sorry we don't have an immediate solution to suggest, but hopefully this bug will be gone soon.

thecodepixi avatar Jul 28 '21 20:07 thecodepixi

Thanks for the reply, @thecodepixi. :)

Are you referring to https://github.com/Shopify/shopify-node-api/pull/169 when you're saying that you're currently working on it?

arsnl avatar Jul 29 '21 00:07 arsnl

Thanks very much @arsnl for the helpful information and to @thecodepixi for your reply. It would be good to understand which thread to monitor so that we get updated on any potential fix.

Can I ask a very quick follow up question related to the quote above:

Keep tokens with online access in a user's temporary session storage, backed by a cookie in the user's browser, and make API requests using this access token in response to the user's requests.

In my server.js afterAuth method, I'm generating cookies like so in order to send back to the browser:

app.prepare().then(async () => {
    const server = new Koa();
    const router = new Router();
    server.keys = [Shopify.Context.API_SECRET_KEY];
    server.use(
      createShopifyAuth({
        async afterAuth(ctx) {
          ctx.cookies.set("shopOrigin", shop, {
            httpOnly: false,
            secure: true,
            sameSite: "none",
          });
          ... etc etc

Now I'm just trying to understand the docs which note:

Note These restrictions don't apply to first-party cookies. Apps that use session tokens can continue to use first-party cookies without incident. Source: https://shopify.dev/apps/auth/session-tokens#why-use-session-tokens

My question:

  • In this case, is the cookie being generated here a first party cookie (thus, safe to use regarding the browser changes)?

Thanks!

g-hamilton avatar Jul 29 '21 09:07 g-hamilton

Hi @g-hamilton! :)

I don't know if your app is embedded or not. But, if your app is embedded (inside an iframe in the Shopify Admin), your cookies are always third-party cookies since the url of your app don't match the url of the browser. If your app is not embedded, it should. It's better for the UX.

Because of that, it's not recommended to use cookies with your app.

From what I see, the way to make it work correctly is to;

  • Never use cookies.
  • Only persist the store session in your database.
  • The store session should be an offline session if you work with Webhooks.
  • For the client session (online session), store it on the client side (localStorage, for example) then pass it to your backend when you make your calls (check https://shopify.dev/apps/auth/session-tokens/axios).
  • Because of the previous point, make sure your server don't call API on the rendering if you are doing SSR. What you can do is to simply show a loader page, then when the client is "hydrated", show the app and make the API calls client side with the client session.

Authentication through session tokens doesn't rely on cookies for embedded apps to authenticate merchants. Instead, your app frontend sends the session token to its backend with every request. The backend then uses the session token to determine the user's identity.

(source: https://shopify.dev/apps/auth/session-tokens#third-party-cookies-vs-session-tokens)

arsnl avatar Jul 29 '21 15:07 arsnl

Hi, I'm also having the same issue that getting two sessions, When I save it in the MongoDB with NodeJS it will record 2 sessions. And someone can explain how deleteCallback and loadCallback work when those functions fire?

chamathdev avatar Aug 04 '21 16:08 chamathdev

I'm also having the same issue that getting two sessions

Ok. As you see, the issue is already acknowledged and a fix is in progress with this PR https://github.com/Shopify/shopify-node-api/pull/169

We hope it's gonna be fixed and release soon.

And someone can explain how deleteCallback and loadCallback work when those functions fire?

"How" they work is really depend on you since they are only handler functions you define yourself on your project. Basically, you can do anything inside these functions, but you have to respect their signature (what they accept, what they return).

For example, to fix the issue described here and avoid useless network calls, I did a utility function to detect if the session was a store or a client session. If it's a store session, I use Redis for all three handlers (storeCallback, loadCallback and deleteCallback). Else, I just use the memory. Also, to avoid keeping expired session and keep my Redis DB clean, I've added an expiration check in the storeCallback that extract the expiration from the session passed and set it in Redis. Then Redis know when to delete the entry (see: https://redis.io/commands/expire).

You can learn more about storeCallback, loadCallback and deleteCallback handlers and how to implement them in this document: https://github.com/Shopify/shopify-node-api/blob/main/docs/usage/customsessions.md

arsnl avatar Aug 04 '21 17:08 arsnl

I'm also seeing the same. Further more, the session data changes. Sometimes it's like this:

Session {
   id: '<uuid>',
   shop: 'mystore.myshopify.com',
   state: '123456789',
   isOnline: true
 }

And other times it looks like this:

Session {
  id: 'mystore.myshopify.com_69785485480',
  shop: 'mystore.myshopify.com',
  state: '12345678',
  scope: 'write_content',
  expires: 2021-10-15T08:12:06.481Z,
  isOnline: true,
  accessToken: 'shpat_...',
  onlineAccessInfo: {
    expires_in: 86398,
    associated_user_scope: 'write_content',
    session: '<big giant number>',
    account_number: 0,
    associated_user: {
      id: 69785485480,
      first_name: 'Gezim',
      last_name: 'H',
      email: '[email protected]',
      account_owner: true,
      locale: 'en-CA',
      collaborator: false,
      email_verified: true
    }
  }
}

What's causing this?

hgezim avatar Oct 14 '21 08:10 hgezim

FWIW, #169 didn't fix this issue above.

hgezim avatar Oct 15 '21 22:10 hgezim

Hey folks! So, the reason you're seeing two sessions is that we use one 1st party cookie-based session for OAuth, which then gets converted to a JWT-based session that can be used in an embedded app.

Previously, we were setting the OAuth session to expire a little bit after the process is completed, so that you could still call loadCurrentSession in your OAuth callback to e.g. set up webhooks. This wasn't ideal because that session was technically not deleted, so the app would still need to periodically clean them up.

With our upcoming v2, we're aiming to fix that by returning the session in the OAuth callback and deleting the OAuth session right away for the app. This is the PR that implements that: https://github.com/Shopify/shopify-node-api/pull/217.

Hope this helps!

paulomarg avatar Oct 18 '21 12:10 paulomarg

When will this be fixed? this a major problem I'm getting the same issue I keep seeing people saying to do redis but will that fix it? or is going to be the same issue?

codingphasedotcom avatar Oct 20 '21 04:10 codingphasedotcom

how does one check release timelines? (I.e when is v2 going to be released?) Is it in the repo somewhere?

govindrai avatar Oct 20 '21 23:10 govindrai

Does anyone have a proposed workaround or solution?

canllama avatar Jan 06 '22 12:01 canllama

Any updates @paulomarg @arsnl @thecodepixi ? Would love to get this fixed!

staadecker avatar Mar 17 '22 15:03 staadecker

Hey @staadecker ! :) Sadly, I cannot help you with that. I've stopped using shopify-node-api since it was not stable enough for my production requirements. This still not fixed bug is a good example. I was finding myself in a situation where I was blocked by bugs that I was trying to fix on the solution supposed to help me develop a Shopify app faster. This was really counter productive so I've just decided to drop it and call the API directly.

arsnl avatar Mar 17 '22 23:03 arsnl

Ok thank you! I might have to do the same, unfortunately.

staadecker avatar Mar 18 '22 01:03 staadecker

Question: does anyone know if the Session passed into the storeCallback method ever comes through w/ isOnline: false? Like, does this method fire on app install or anything? I need to know so that I can determine if I can keep handling "offline" tokens the way that I am for my background tasks, or if I need to move that logic into this?

If the answer is "no, this is only for 'online' session management" then why pass the isOnline flag at all?

RavenHursT avatar Mar 24 '22 23:03 RavenHursT

Yes, offline tokens are also stored using storeCallback. It works the same as online tokens in that aspect.

staadecker avatar Mar 25 '22 14:03 staadecker

Ok.. so if we're doing something special w/ offline tokens (like putting them in a place where a background serverless service worker would need to access them) we should be doing that persistence, based on the the isOnline flag, in the storeCallback as well, right?

RavenHursT avatar Mar 25 '22 18:03 RavenHursT

@RavenHursT

Whatever your 'persist Session / token in your database' logic is, it should live in storeCallback, yep!

bishpls avatar Mar 25 '22 18:03 bishpls

Thanks!

RavenHursT avatar Mar 25 '22 19:03 RavenHursT

Ok.. so if we're doing something special w/ offline tokens (like putting them in a place where a background serverless service worker would need to access them) we should be doing that persistence, based on the the isOnline flag, in the storeCallback as well, right?

@RavenHursT it sounds like you're thinking about offline tokens in the same way we do where they're not really "sessions" but rather "stores" which aren't saved in a session DB, but instead our application DB which can be read by other services running jobs in the background or on webhooks etc? If so, are you doing something like this, and how do you read it appropriately?

Shopify.Context.initialize({
  ...
  SESSION_STORAGE: new Shopify.Session.CustomSessionStorage(
    async function storeCallback(session) {
      if (session.isOnline) {
          // `sessionDB` is a DB designed for v. fast read / writes (potentially in exchange for reduced durability), e.g. redis
          return await sessionDB.sessions.save(session.id, session);
      }
      // `appDB` is a DB designed with stronger durability guarantees, e.g. PostGres, MySQL or MongoDB etc.
      return await appDB.stores.upsert({ shop: session.shop, accessToken: session.accessToken });
    },
    async function loadCallback(id) {
      // Where do you look up the 'session'?
    },
    async function deleteCallback(id) {
      // Where do you look up the 'session'?
    }
  )
});

richardscarrott avatar Jul 21 '22 14:07 richardscarrott

@richardscarrott correct. What you've got there, is similar to what we're doing, storing the "offline" tokens, on app install, into a postGres DB..

Then, for background processes (in our case, AWS Lambdas that run a few times each hour) we just query the PostGres DB as normal, based on the shop we're operating on, and use that offline token to interact w/ the Shopify API...

RavenHursT avatar Jul 21 '22 18:07 RavenHursT

@RavenHursT Thanks for the response. Do you additionally store the "online" uuid session in your postgres DB too; i.e. the one mentioned here https://github.com/Shopify/shopify-api-node/issues/224#issuecomment-943141486 which doesn't have an access token associated with it:

Session {
   id: '<uuid>',
   shop: 'mystore.myshopify.com',
   state: '123456789',
   isOnline: true
 }

richardscarrott avatar Jul 21 '22 18:07 richardscarrott

@richardscarrott yeah.. We dump everything into redis, keyed on id.. Not sure if those "incomplete" session objects are ever read again, but figured it doesn't hurt to have them in there 🤷

RavenHursT avatar Jul 21 '22 19:07 RavenHursT

This issue is stale because it has been open for 90 days with no activity. It will be closed if no further action occurs in 14 days.

github-actions[bot] avatar Oct 06 '22 02:10 github-actions[bot]

We are closing this issue because it has been inactive for a few months. This probably means that it is not reproducible or it has been fixed in a newer version. If it’s an enhancement and hasn’t been taken on since it was submitted, then it seems other issues have taken priority.

If you still encounter this issue with the latest stable version, please reopen using the issue template. You can also contribute directly by submitting a pull request– see the CONTRIBUTING.md file for guidelines

Thank you!

github-actions[bot] avatar Oct 20 '22 02:10 github-actions[bot]