1

I am trying to validate an API key to run an API endpoint function.

server.js

let db = require("./sql/rest_functions")            // Custom modules
let session = require("./sql/session_management")

app.get("/node/api/mssql/test", function(req, res) {
   if(session.validateKey(req)) {
     db.mssqlQuery(req, res)
   }
 })

session.js

exports.validateKey = function(req) {
  return sql.connect(config.properties)
    .then(pool => {
      return pool.request()
        .query("SELECT CASE WHEN EXISTS (SELECT * FROM Login WHERE (username = '" + req.header("username") + "' AND apiKey = '" + req.header("apiKey") + "')) THEN CAST(1 AS BIT) ELSE CAST(0 AS BIT) END")
        .then(response => {
          console.log(typeof response[0]['']) // returns boolean
          return response[0]['']              // return nested value to get true / false response
        })
    })
}

The validateKey function is returning true and false (I think as plain text), and I would like it to resolve to a boolean that can be passed into the API test function.

I have tried this JSON.parse method and the session.validateKey(req) == 'true' design pattern, but neither case resolves to an expected behavior.

How can I resolve the returned value to verify the user's credentials?

Matthew
  • 1,461
  • 3
  • 23
  • 49
  • 1
    You should address that [SQL injection vulnerability](https://www.owasp.org/index.php/SQL_Injection). – msanford Jan 04 '18 at 16:47
  • 1
    Also, that seems like a somewhat convoluted query to validate username/key combination: is there a reason not to simply `SELECT COUNT(*) WHERE...` and then `return response[0]['COUNT(*)'] > 0` (which will give you a Boolean)? – msanford Jan 04 '18 at 16:50
  • @msanford I am working through upgrading security measures when interacting with this DB, the idea is to move towards stored procedures, but I am using the query function to test retrieval from the database during pre-production. Thank you for the OWASP link. I will read through it today. I was thinking that forcing an API key match would prevent writing to the DB without proper credentials. Am I incorrect in this assumption? – Matthew Jan 04 '18 at 17:03
  • @msanford I am trying to use an apiKey like a session token and the user needs a validated apiKey to read or write. – Matthew Jan 04 '18 at 17:04
  • @msanford I would like to run the ```db.mssqlQuery``` command if the user has a valid apiKey, as an added layer of protection. – Matthew Jan 04 '18 at 17:08
  • @msantord Also, the docs for the mssql package for Node states that "All values are automatically sanitized against sql injection." -- https://www.npmjs.com/package/mssql – Matthew Jan 04 '18 at 17:31
  • And sure, my "convoluted" comment was about the SQL query _itself:_ it's a (1) select, then (2) sub-select then (3) a cast where a simple count would suffice. It's probably not really going to be a performance bottleneck, just seems like an odd thing to do. Your general pattern of checking that the key is valid for every request is good. – msanford Jan 04 '18 at 18:16
  • 1
    @msanford OK. Thank you for the link and the guidance. I am new to SQL, and am working through best practices for security. – Matthew Jan 04 '18 at 18:32

1 Answers1

1

I would suggest simply returning the count of rows from the database and validating that there exists 1 entry (or more than one, depending on how your data are structured).

Note that to mitigate against SQL Injection, you must use parameter binding, illustrated below; the library does not "automatically" protect against injection, as per the documentation if you do not use parameter binding. Change VarChar(100) to whatever the column type is for those fields.

exports.validateKey = async function(req) {
  return await sql.connect(config.properties)
    .then(pool => pool.request()
        .input('user', sql.VarChar(100), req.header("username"))
        .input('key', sql.VarChar(100), req.header("apiKey"))
        .query('SELECT COUNT(*) AS valid FROM Login WHERE username = @user AND apiKey = @key')
        .then(response => result.recordset[0].valid === 1)
    )
}

Note that validateKey will return a Boolean Promise, so we've added async/await to save modifying the route controller.

Note that I've removed the braces from most of the fat arrow functions: they're all one statement so they're redundant.

Caveat: I can't actually try this, it's an educated guess from reading the documentation. I hope it helps.

msanford
  • 11,803
  • 11
  • 66
  • 93