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

Session's "isActive()" method is lost when custom storage is used like Redis

Open rkumorek opened this issue 3 years ago • 11 comments

Hey,

I recently started playing with Shopify app looking at the in progress example in "Shopify App Node" repo. I noticed it in "verifyRequest" here which doesn't work (throws error) if used with Redis store.

Issue summary

When custom session storage is used like Redis in the example in storeCallback we're stringifying the session which removes the "isActive" method as it's not valid json.

It works with MemorySessionStorage as we're storing objects.

Steps to reproduce the problem

const session = new Shopify.Auth.Session(...);
console.log(session.isActive()) // boolean

const session2 = JSON.parse(JSON.stringify(session));
console.log(session2.isActive())) // error: is not a function

Checklist

  • [x] I have described this issue in a way that is actionable (if possible)

Solution

Maybe move it inside Utils? Something like:

Shopify.Utils.isSessionActive(session) // boolean

If it's me doing something wrong, apologies.

rkumorek avatar Feb 23 '22 20:02 rkumorek

Yep I had the same problem when saving the session into my MongoDB store, because I didn't realize until later that methods aren't serialized into the database. I second this request!

limeforadime avatar Mar 05 '22 04:03 limeforadime

Just incase it's of any use to anyone. I'm doing this for now:

class RedisStore {
  constructor () {
    this.client = new Redis(process.env.REDIS_URL)
  }

  async storeCallback (session) {
    try {
      return await this.client.set(session.id, JSON.stringify(session))
    } catch (err) {
      throw new Error(err)
    }
  }

  async loadCallback (id) {
    try {
      const reply = await this.client.get(id)

      if (reply) {
        return JSON.parse(reply)
      } else {
        return undefined
      }
    } catch (error) {
      throw new Error(error)
    }
  }

  async deleteCallback (id) {
    try {
      return await this.client.del(id)
    } catch (error) {
      throw new Error(error)
    }
  }

  isActive (session) {
    return (
      Shopify.Context.SCOPES.equals(session.scope) &&
      session.accessToken &&
      (!session.expires || session.expires >= new Date())
    )
  }
}

export const sessionStorage = new RedisStore()

const session = await Shopify.Utils.loadCurrentSession(req, res, isOnline)

if (sessionStorage.isActive(session)) {
  ..
}

Not ideal but emulates what is there

dan-gamble avatar Mar 05 '22 11:03 dan-gamble

See #333 for another workaround that's a bit cleaner (imo) as well as the exact line of code that causes this bug.

staadecker avatar Mar 18 '22 00:03 staadecker

Yep I had the same problem when saving the session into my MongoDB store, because I didn't realize until later that methods aren't serialized into the database. I second this request!

@limeforadime David THANK YOU for posting your solution in the comments here and helping me get auth working!: https://jsrepos.com/lib/TheSecurityDev-simple-koa-shopify-auth

buildpath-ian avatar May 17 '22 03:05 buildpath-ian

Yep I had the same problem when saving the session into my MongoDB store, because I didn't realize until later that methods aren't serialized into the database. I second this request!

@limeforadime David THANK YOU for posting your solution in the comments here and helping me get auth working!: https://jsrepos.com/lib/TheSecurityDev-simple-koa-shopify-auth

Np 👍

limeforadime avatar May 17 '22 04:05 limeforadime

Besides isActive() function gone, I've also encountered an issue with session.expires. session.expires is a Date but it will get returned by Redis as a String.

Here is my workaround but it has to be something more elegant:

class RedisStore {
  constructor() {
    ...
    async loadCallback(id) {
      try {
        const reply = await this.client.get(id);
  
        if (reply) {
          var session = JSON.parse(reply);
          session.expires = new Date(session.expires);
  
          session.isActive = function() {
            const scopesUnchanged = Shopify.Context.SCOPES.equals(session.scope);
  
            if (
              scopesUnchanged && 
              session.accessToken && 
              (!session.expires || session.expires >= new Date())) {
              return true;
            }
            return false;
          }
  
          return session;
        } else {
          return undefined;
        }
      } catch (err) {
        throw new Error(err);
      }
    }

    ...
  }
}

There are some session utils and https://github.com/Shopify/shopify-node-api/blob/e69a4ab3a7e35645b7109977d97dee30b99d3dc6/src/auth/session/session-utils.ts and a Redis implementation https://github.com/Shopify/shopify-node-api/blob/e69a4ab3a7e35645b7109977d97dee30b99d3dc6/src/auth/session/storage/redis.ts. This may provide a better solution

mirceapiturca avatar May 19 '22 18:05 mirceapiturca

@mirceapiturca Yeah I'm doing the exact same thing, converting it into a new Date() first. Works just fine after that.

limeforadime avatar May 19 '22 23:05 limeforadime

Hello everyone, you know that I have the same problem as you, I am working with the most recent version of shopify cli, and when trying to work with new Shopify.Session.PostgreSQLSessionStorage() I am presented with two types of errors, the first is that when reloading the page for the first time gives me the error "TypeError: session?.isActive is not a function" I go to pgadmin and the table is created, I make some changes to try to solve the previous error and a new error message appears: relation '"shopify_sessions" already exists', to continue testing I must be deleting the table!!! I'm really very disoriented, and frustrated. I leave parts of my code to see can you help me...

is this all that is needed in index js?

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.April22, IS_EMBEDDED_APP: true, // This should be replaced with your preferred storage strategy SESSION_STORAGE: new Shopify.Session.PostgreSQLSessionStorage( new URL(process.env.DATABASE_URL), ), });

and this is the original postgresql.js file:

`"use strict"; Object.defineProperty(exports, "__esModule", { value: true }); exports.PostgreSQLSessionStorage = void 0; var tslib_1 = require("tslib"); var pg_1 = tslib_1.__importDefault(require("pg")); var session_utils_1 = require("../session-utils"); var defaultPostgreSQLSessionStorageOptions = { sessionTableName: 'shopify_sessions', port: 3211, };

var PostgreSQLSessionStorage = /** @class */ (function () { function PostgreSQLSessionStorage(dbUrl, opts) { if (opts === void 0) { opts = {}; } this.dbUrl = dbUrl; if (typeof this.dbUrl === 'string') { this.dbUrl = new URL(this.dbUrl); } this.options = tslib_1.__assign(tslib_1.__assign({}, defaultPostgreSQLSessionStorageOptions), opts); this.ready = this.init(); } PostgreSQLSessionStorage.withCredentials = function (host, dbName, username, password, opts) { return new PostgreSQLSessionStorage(new URL("postgres://".concat(encodeURIComponent(username), ":").concat(encodeURIComponent(password), "@").concat(host, "/").concat(encodeURIComponent(dbName))), opts); }; PostgreSQLSessionStorage.prototype.storeSession = function (session) { return tslib_1.__awaiter(this, void 0, void 0, function () { var entries, query; return tslib_1.__generator(this, function (_a) { switch (_a.label) { case 0: return [4 /yield/, this.ready]; case 1: _a.sent(); entries = (0, session_utils_1.sessionEntries)(session); query = "\n INSERT INTO ".concat(this.options.sessionTableName, "\n (").concat(entries.map(function (_a) { var _b = tslib_1.__read(_a, 1), key = b[0]; return key; }).join(', '), ")\n VALUES (").concat(entries.map(function (, i) { return "$".concat(i + 1); }).join(', '), ")\n ON CONFLICT (id) DO UPDATE SET ").concat(entries .map(function (_a) { var _b = tslib_1.__read(_a, 1), key = _b[0]; return "".concat(key, " = Excluded.").concat(key); }) .join(', '), ";\n "); return [4 /yield/, this.query(query, entries.map(function (_a) { var _b = tslib_1.__read(_a, 2), _key = _b[0], value = _b[1]; return value; }))]; case 2: _a.sent(); return [2 /return/, true]; } }); }); }; PostgreSQLSessionStorage.prototype.loadSession = function (id) { return tslib_1.__awaiter(this, void 0, void 0, function () { var query, rows, rawResult; return tslib_1.__generator(this, function (_a) { switch (_a.label) { case 0: return [4 /yield/, this.ready]; case 1: _a.sent(); query = "\n SELECT * FROM ".concat(this.options.sessionTableName, "\n WHERE id = $1;\n "); return [4 /yield/, this.query(query, [id])]; case 2: rows = _a.sent(); if (!Array.isArray(rows) || (rows === null || rows === void 0 ? void 0 : rows.length) !== 1) return [2 /return/, undefined]; rawResult = rows[0]; return [2 /return/, (0, session_utils_1.sessionFromEntries)(Object.entries(rawResult))]; } }); }); }; PostgreSQLSessionStorage.prototype.deleteSession = function (id) { return tslib_1.__awaiter(this, void 0, void 0, function () { var query; return tslib_1.__generator(this, function (_a) { switch (_a.label) { case 0: return [4 /yield/, this.ready]; case 1: _a.sent(); query = "\n DELETE FROM ".concat(this.options.sessionTableName, "\n WHERE id = $1;\n "); return [4 /yield/, this.query(query, [id])]; case 2: _a.sent(); return [2 /return/, true]; } }); }); }; PostgreSQLSessionStorage.prototype.disconnect = function () { return this.client.end(); }; PostgreSQLSessionStorage.prototype.init = function () { return tslib_1.__awaiter(this, void 0, void 0, function () { return tslib_1.__generator(this, function (_a) { switch (_a.label) { case 0: this.client = new pg_1.default.Client({ connectionString: this.dbUrl.toString() }); return [4 /yield/, this.connectClient()]; case 1: _a.sent(); return [4 /yield/, this.createTable()]; case 2: _a.sent(); return [2 /return/]; } }); }); }; PostgreSQLSessionStorage.prototype.connectClient = function () { return tslib_1.__awaiter(this, void 0, void 0, function () { return tslib_1.__generator(this, function (_a) { switch (_a.label) { case 0: return [4 /yield/, this.client.connect()]; case 1: _a.sent(); return [2 /return/]; } }); }); }; PostgreSQLSessionStorage.prototype.hasSessionTable = function () { return tslib_1.__awaiter(this, void 0, void 0, function () { var query, _a, rows; return tslib_1.__generator(this, function (_b) { switch (_b.label) { case 0: query = "\n SELECT * FROM pg_catalog.pg_tables WHERE tablename = $1\n "; return [4 /yield/, this.query(query, [this.options.sessionTableName])]; case 1: _a = tslib_1.__read.apply(void 0, [_b.sent(), 1]), rows = _a[0]; return [2 /return/, Array.isArray(rows) && rows.length === 1]; } }); }); }; PostgreSQLSessionStorage.prototype.createTable = function () { return tslib_1.__awaiter(this, void 0, void 0, function () { var hasSessionTable, query; return tslib_1.__generator(this, function (_a) { switch (_a.label) { case 0: return [4 /yield/, this.hasSessionTable()]; case 1: hasSessionTable = _a.sent(); if (!!hasSessionTable) return [3 /break/, 3]; query = "\n CREATE TABLE ".concat(this.options.sessionTableName, " (\n id varchar(255) NOT NULL PRIMARY KEY,\n shop varchar(255) NOT NULL,\n state varchar(255) NOT NULL,\n isOnline boolean NOT NULL,\n scope varchar(255),\n accessToken varchar(255)\n )\n "); return [4 /yield/, this.query(query)]; case 2: _a.sent(); _a.label = 3; case 3: return [2 /return/]; } }); }); }; PostgreSQLSessionStorage.prototype.query = function (sql, params) { if (params === void 0) { params = []; } return tslib_1.__awaiter(this, void 0, void 0, function () { var result; return tslib_1.__generator(this, function (_a) { switch (_a.label) { case 0: return [4 /yield/, this.client.query(sql, params)]; case 1: result = _a.sent(); return [2 /return/, result.rows]; } }); }); }; return PostgreSQLSessionStorage; }()); exports.PostgreSQLSessionStorage = PostgreSQLSessionStorage;`

dto1902 avatar May 20 '22 04:05 dto1902

I'm facing a similar problem where the server throws an error if the JWT token is expired.

It seems that there's no built in logic for redirecting the customer to the oAuth flow when that happens.

It just throws an error and eventually crashes my whole nodeJS server.

daviareias avatar Jul 13 '22 07:07 daviareias

Eventually I found that this was only happening to me locally, and the expired JWT issue did not occur in production.

On Wed, Jul 13, 2022 at 2:57 AM daviareias @.***> wrote:

I'm facing a similar problem where the server throws an error if the JWT token is expired.

It seems that there's no built in logic for redirecting the customer to the oAuth flow when that happens.

It just throws an error and eventually crashes my whole nodeJS server.

— Reply to this email directly, view it on GitHub https://github.com/Shopify/shopify-api-node/issues/307#issuecomment-1182892454, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEFIQNDHACYXRTGR6O37VLVTZZFRANCNFSM5PFOPCJQ . You are receiving this because you commented.Message ID: @.***>

buildpath-ian avatar Jul 13 '22 14:07 buildpath-ian

I think the example code is wrong. The loadCallback function is supposed to return an instance of SessionInterface, not just a parsed JSON object. (Not sure why TypeScript doesn't catch this.)

If you look at the "real" implementations of SessionStorage, like redis.ts, you can see that they use functions from session-utils.ts called sessionFromEntries and sessionEntries. I'm not sure if those functions can be imported by an application, but they appear to be the right way to serialize and deserialize SessionInterface.

#428 would help in situations like this...

adelespinasse avatar Aug 01 '22 22:08 adelespinasse

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]