Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

Custom session storage generates multiple session IDs from single app install #224

Closed
g-hamilton opened this issue Jul 28, 2021 · 27 comments
Labels

Comments

@g-hamilton
Copy link

g-hamilton commented Jul 28, 2021

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 :)

@arsnl
Copy link

arsnl commented Jul 28, 2021

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.

@thecodepixi
Copy link
Contributor

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.

@arsnl
Copy link

arsnl commented Jul 29, 2021

Thanks for the reply, @thecodepixi. :)

Are you referring to #169 when you're saying that you're currently working on it?

@g-hamilton
Copy link
Author

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!

@arsnl
Copy link

arsnl commented Jul 29, 2021

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)

@chamathdev
Copy link

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?

@arsnl
Copy link

arsnl commented Aug 4, 2021

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 #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

@hgezim
Copy link

hgezim commented Oct 14, 2021

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
Copy link

hgezim commented Oct 15, 2021

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

@paulomarg
Copy link
Contributor

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: #217.

Hope this helps!

@codingphasedotcom
Copy link

codingphasedotcom commented Oct 20, 2021

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?

@govindrai
Copy link

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

@canllama
Copy link

canllama commented Jan 6, 2022

Does anyone have a proposed workaround or solution?

@staadecker
Copy link

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

@arsnl
Copy link

arsnl commented Mar 17, 2022

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.

@staadecker
Copy link

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

@RavenHursT
Copy link

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?

@staadecker
Copy link

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

@RavenHursT
Copy link

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?

@bishpls
Copy link

bishpls commented Mar 25, 2022

@RavenHursT

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

@RavenHursT
Copy link

Thanks!

@richardscarrott
Copy link

richardscarrott commented Jul 21, 2022

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'?
    }
  )
});

@RavenHursT
Copy link

@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...

@richardscarrott
Copy link

richardscarrott commented Jul 21, 2022

@RavenHursT Thanks for the response. Do you additionally store the "online" uuid session in your postgres DB too; i.e. the one mentioned here #224 (comment) which doesn't have an access token associated with it:

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

@RavenHursT
Copy link

@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 🤷

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

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 github-actions bot added the Stale label Oct 6, 2022
@github-actions
Copy link
Contributor

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!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests