voxpelli / node-connect-pg-simple

A simple, minimal PostgreSQL session store for Express
https://www.npmjs.com/package/connect-pg-simple
MIT License
233 stars 74 forks source link

Allow customization of the column names #236

Closed Moebits closed 2 years ago

Moebits commented 2 years ago

I want to use my own column names, but this module uses hardcoded column names.

With this modification, you should be able to pass a custom sidColumnName, sessColumnName, or expireColumnName.

No one should be passing in column names from unsanitized input, so I don't think that SQL injections are an issue.

I only modified some of the tests because PostgreSQL seems to be returning the numbers as strings without an explicit type cast.

Code Example:

app.use(session({
  store: new pgSession({
    pool: pgPool,
    sidColumnName: "sessionID",
    sessColumnName: "session",
    expireColumnName: "expires"
  }),
  secret: process.env.COOKIE_SECRET,
  cookie: {maxAge: 1000 * 60 * 60 * 24},
  resave: false,
  saveUninitialized: false
}))
voxpelli commented 2 years ago

Why do you want your own column names? Making things too configurable will make the module needlessly complex. I'm not sure I'm convinced of the benefit of adding this.

No one should be passing in column names from unsanitized input, so I don't think that SQL injections are an issue.

Well, #151 begs to differ, which caused https://github.com/voxpelli/node-connect-pg-simple/security/advisories/GHSA-xqh8-5j36-4556

Moebits commented 2 years ago

Why do you want your own column names?

For naming consistency sake, I don't use abbreviations in my column names which is why I prefer using "sessionID" and "session".

Well, #151 begs to differ, which caused GHSA-xqh8-5j36-4556

You solved this by adding the function escapePgIdentifier() to escape double quotes, which I am also using for the column names.

SQL injection is possible if they pass in unsanitized input for the column name, and this is already possible for the table name too. You should warn people to only pass in a hardcoded string for the column/table names in the readme.

voxpelli commented 2 years ago

Why do you want your own column names?

For naming consistency sake, I don't use abbreviations in my column names which is why I prefer using "sessionID" and "session".

But this is internally within the dedicated session table? And shouldn't be used by anyone else? So it won't really matter which names they have? Or where are you getting exposed to them?

Moebits commented 2 years ago

it won't really matter which names they have?

It’s purely a matter of personal preference, changing the column names doesn’t have a functional difference. I’d just rather use names which aren’t abbreviated.