add user_id field in session table

[mo]

When a user does a password reset / changes his password, it makes sense to drop all sessions for that user. Currently the user id is stored inside a json blob on the session, so it becomes very inefficient to query for all sessions that belong to a specific user.

In issue #53 ( #53 ), it was suggested that one way to workaround this issue is to, A) not use createDatabaseTable: true, B) add a user_id column, and C) right after login, update the session with the appropriate user_id.

However, this approach has the downside that "user_id" cannot be a "NOT NULL" column with a foreign key constraint pointing to the user table. This in turn would enable cascading deletes for sessions when a user is deleted.

      columnNames: {
        user_id: 'user_id',
        ...
     }

Do you agree that having such a column is a good idea, i.e. would you accept a PR to add one?

[chill117]

I don't like the idea of adding a user_id field to the sessions table. Most users of this module won't use it. I think there might be a way to do what you need without modifying the module. A few possibilities to explore:

• Set the default value on your user_id field to NULL; so that the session can be inserted into the database table without an error. I found this stackoverflow post which could help.
• Using a reference table (e.g user_sessions), but I am not sure if this could really work. I would have to try it.
• Use a trigger to pull the user ID from the JSON object in the data field and set the user_id field on the session before it is inserted into the sessions table.

Hope that helps!

[mo]

@chill117 What if the "user_id" property was optional and such a column was only created and set by express-mysql-session if it was defined in the columnNames dict?

[chill117]

@mo Maybe, but only if the other solutions are not workable. Even if a user of this module wanted to set a user_id column in their sessions table, maybe they don't want to use a foreign key? And how would the module know the name of their users table?

[chill117]

@mo How did this one end up?

[mo]

If express-mysql-session creates the table and the optional columnNames.user_id is defined, then it should create a column field without a foreign key. This would make it possible for me to pre-create the session table WITH the foreign key constraint, and then I would pass "columnNames.user_id" to express-mysql-session ensuring that all rows will always have a correct value in the user_id field. This way, express-mysql-session doesn't have to know about the user table at all. Does this sound OK for a PR @chill117 ?

For now, I'm doing a second query that writes the user_id field and I have set my foreign key constraint to be in effect only when the value is NOT NULL. However, I really don't want to allow sessions with a NULL value so I would be more much comfortable if my database schema enforced this.

[chill117]

Please tell me see if I understand correctly. When a new option flag is true, then the following happens:

• The createDatabaseTable function will add a user ID field to the sessions database table. This new field's name will also be configurable via the schema.columnNames option.
• The set(session_id, data, cb) function will set the value of this new user ID field equal to whatever is passed via data.user_id.

At least one issue I see with this:

• How to know what type to use for the user ID field on the sessions table if it is created by this module?

Instead of making this a one-off solution for user IDs, let's try a more general solution. A new option that defines keys in the session data object that should be written to distinct columns in each session row. For example:

var sessionStore = new MySQLStore({
    dataWithOwnColumns: [ 'user_id', 'some_other_field' ]
});

Where the database table looks like this:

CREATE TABLE IF NOT EXISTS `sessions` (
  `session_id` varchar(128) COLLATE utf8mb4_bin NOT NULL,
  `expires` int(11) unsigned NOT NULL,
  `data` text COLLATE utf8mb4_bin,
  `user_id` int(11) unsigned,
  `some_other_field` varchar(255),
  PRIMARY KEY (`session_id`)
) ENGINE=InnoDB

The module user (developer) would be responsible for creating their own sessions table with the correct columns.

Then whenever sessionStore.set(session_id, data, cb) is called, the columns for the new/existing session will be set from values provided in the data object.

[chill117]

Is there still interest in this issue?

[mo]

I'm still interested in this feature.

The field that passport puts into "data" for me is called "user" but the database column I have is called "user_id". Ideally, I think it's good if the session storage does not mandate certain column names so it would be nice to find a solution that ensures this, even for "dataWithOwnColumns" columns.

Did you mean that dataWithOwnColumns could be combined with columns, like this:

{
    createDatabaseTable: false,
    schema: {
      tableName: 'my_session_table',

      // note that I changed this to "user" instead of "user_id" since it refers to the data property
      dataWithOwnColumns: [ 'user' ],

      columnNames: {
        session_id: 'session_id',
        expires: 'expires',
        data: 'data',
        user: 'user_id'
      }
    }
  }
}

I suppose another option would be to simply omit the "dataWithOwnColumns" member, and instead make a rule that says:

If there is any property KEY:VAL inside columnNames dict where KEY is not "session_id", "expires" or "data", then the session store will create a column called VAL and assume that the data for it is in data[KEY]

{
    createDatabaseTable: false,
    schema: {
      tableName: 'my_session_table',
      columnNames: {
        session_id: 'session_id',
        expires: 'expires',
        data: 'data',
        user: 'user_id'
      }
    }
  }
}

[dimabolgov]

Hey guys, this function is still needed. I would like to write to this new column user_uuid value (binary16).
Have you come to any decision?

[chill117]

It's still not clear to me how to proceed with this one. I don't want to introduce foot-guns with a feature that most users of this module won't need. But if you have some suggestion for how to implement this, I am willing to listen.

[bcmyguest]

Using a reference table (e.g user_sessions), but I am not sure if this could really work. I would have to try it.

This solution appears to work fine @demjanich

[HoldYourWaffle]

I'm not sure I completely understand what @mo is suggesting here, but wouldn't an implementation of #91 fix the inefficiency problem?
I haven't used the JSON column type yet since it didn't exist the last time I did something with MySQL, but a quick google search tells me it allows for more efficient querying using something like JSON_EXTRACT.

[mo]

The things I would like are:

• fast queries (i.e no JSON lookup)
• foreign key from session table pointing to user table
• user_id on session should be NOT NULL

The two last points are important if you want your db schema to ensure that the data is consistent.

---

My proposed solution is; if express-mysql-session gets:

      columnNames: {
        ...
        user: 'user_id'
      }

...then it will insert the user id into a column called "user_id". This would make it possible to pre-create the session table with the foreign key / not null.

[mo]

My first workaround attempt was to run "res.end()" so that express-mysql-session creates the session in the db. Then I used an UPDATE query to fill in the user_id column. One drawback of this was that user_id could not be NOT NULL and after running this in prod I had a few junk sessions lying around with user_id=NULL. When I investigated why, I noticed that about 1 times out of 50 the express-mysql-session INSERT query would be done after the UPDATE query due to a race condition between the two. To reproduce this bug I had to run my E2E testsuite hundreds of times consecutively. This is why db schemas that enforce consistency are useful.

A slightly better solution (still hacky but it works) is to do something like:

await db.query(
      'INSERT INTO your_session_table(session_id, expires, user_id) VALUES (?, ?, ?) ON DUPLICATE KEY UPDATE user_id = VALUES(user_id)',
      [req.session.id, sessionExpiresSeconds, user.user_id])

...because then it doesn't matter if the INSERT or the UPDATE happens first. However, the best solution would be to fix express-mysql-session so that it allows users to have a strong consistent db schema.

[madacol]

I agree with HoldYourWaffle, this could be partially solved along with #91 by implementing an opt-in JSON data type for the data column. I say partially, because this still does not allow foreign key constraints for cascading deletes.

But has fast reads, no JSON lookups.
From MYSQL docs:

JSON documents stored in JSON columns are converted to an internal format that permits quick read access to document elements. When the server later must read a JSON value stored in this binary format, the value need not be parsed from a text representation. The binary format is structured to enable the server to look up subobjects or nested values directly by key or array index without reading all values before or after them in the document.

And for cascading deletes you could workaround with a trigger