Connection String Failing on User Id

zzidante commented 1 year ago

Cannot connect with a db user when using a connection string. RE: ConnectionError: Login failed for user ''.

Expected behaviour:

When passing User Id (as well as: UserId, User ID, UserID, user id, userid) in a connection string; MSSQL should parse the string via the Tedious Connection String Parser and internally match its results to its own config. The user id should be ferried to tedious connect and the connection should work.

Actual behaviour:

When passingUser Id (as well as: UserId, User ID, UserID, user id, userid) in a connection string; MSSQL successfully uses tedious's Connection String Parser (CSP) to parse the attributes from the connection string, however in the next step where it internally matches those results to its (MSSQL's) own config, there is no matcher for the value returned from the CSP.

This means user (and domain) never get set and the connection fails.

CSP returns userid whereas MSSQL has a matcher of user id. Thwarted by a space.

Notes:

Connecting via configuration object does not have this same issue; only the connection string.

Configuration:

const cxnStr = `Server=${serverName},${port || 1433};Database=${dbName};UserID=${userName};Password=${password};Encrypt=${true};trustServerCertificate=${true};ApplicationName=${appName};`

mssql.connect(cxnStr);

We end up with the following trace:

Error trace:
ConnectionError: Login failed for user ''.
    at C:\...\node_modules\mssql\lib\tedious\connection-pool.js:70:17
    at Connection.onConnect (C:\...\node_modules\tedious\lib\connection.js:10
12:9)
    at Object.onceWrapper (node:events:652:26)
    at Connection.emit (node:events:537:28)
    at Connection.emit (C:\...\node_modules\tedious\lib\connection.js:1040:18
)
    at C:\...\node_modules\tedious\lib\connection.js:2519:18
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  code: 'ELOGIN',
  originalError: ConnectionError: Login failed for user ''.
      at Login7TokenHandler.onErrorMessage (C:\...\node_modules\tedious\lib\t
oken\handler.js:245:19)
      at Readable.<anonymous> (C:\...\node_modules\tedious\lib\token\token-st
ream-parser.js:26:33)
      at Readable.emit (node:events:537:28)
      at addChunk (node:internal/streams/readable:324:12)
      at readableAddChunk (node:internal/streams/readable:297:9)
      at Readable.push (node:internal/streams/readable:234:10)
      at next (node:internal/streams/from:98:31)
      at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
    code: 'ELOGIN',
    isTransient: undefined

Proposed Solution

I tested adding an additional case to the following clause (in mssql\lib\base\connection-pool.js), and it worked:

case 'userid': // added
case 'user id':{
  let user = value
  let domain
  if (/^(.*)\\(.*)$/.exec(user)) {
    domain = RegExp.$1
    user = RegExp.$2
    }
    Object.assign(config, {
      domain,
       user
    })
    break
}

This could be the solution, however it may be better suited in more specific tedious connection-pool.

Software versions

NodeJS: 18.5
node-mssql: 9.01
SQL Server: 18
Tediousjs/connection-string: 0.4.1

dhensby commented 1 year ago

Are you 100% sure this is the problem? I'm using connection strings in production and do not have this problem (though granted we may be using a space).

The connection string library returns normalised keys (or should) for user ID, so there should be no need to handle with and without spaces when handling the parsed object...

dhensby commented 1 year ago

Ok, I just had a look. The connection string library does not support an alias without a space for user id and that's because it is not part of the spec (see https://learn.microsoft.com/en-us/dotnet/api/system.data.sqlclient.sqlconnection.connectionstring).

It's either with a space or UID (but also user, which isn't supported for some reason).

I'd suggest you update your connection string so it complies with the standard.

zzidante commented 1 year ago

Thank you for reviewing, you are correct and I can see what happened with fresh eyes. The build cache wasn't updating correctly and so the various attempts of diff forms of User Id was not what was being sent, only my first one, which was wrong and lacked a space. I appreciate the response and hope this at least helps someone else in the future.

tediousjs / node-mssql