0

How do I insert a value into a select statement using JavaScript, specifically when using express and postgres?

The createUser, and listAllUsers, is working (included below for reference). The try/catch is working and satisfying the request or throwing the error for those two as well.

Any help would be greatly appreciated!

When using Postman, the output that I receive when I send the get (localhost:4000/user/id with a x-www-formurlencoded key value user_id = 3) is…

{
    "name": "error",
    "length": 90,
    "severity": "ERROR",
    "code": "42601",
    "position": "37",
    "file": "scan.l",
    "line": "1134",
    "routine": "scanner_yyerror"
}

And in the terminal, it shows the following (trapped from my console.log).

3
QUERY:  SELECT * FROM users WHERE user_id = ${user_id}

When I user curl it says the same in the terminal. Here is the curl command and putput…

curl -X GET localhost:4000/user/3

{"name":"error","length":90,"severity":"ERROR","code":"42601","position":"37","file":"scan.l","line":"1134","routine":"scanner_yyerror"}ww10sc2353621:~ james.mcgreggor$ curl -X GET localhost:4000/user/3

Ultimately the 3 that I am passing as the user_id is not being substituted in the select statement. That is my problem. I cannot figure out how to correctly do this. Should I even be taking this approach, or should I try passing it as a parameter in the URL?

This is from my User class file (User.js)

const db = require('../connectors/db.js');
class User {

  constructor(id, user_id, first_name, middle_initial, last_name, email, type) {
    this.id = id;
    this.first_name = first_name;
    this.middle_initial = middle_initial;
    this.last_name = last_name;
    this.email = email;
    this.type = type;
    this.user_id = user_id;
  }

  static newUser(user_id, first_name, middle_initial, last_name, email, type) {
     return db.one(`
      INSERT INTO users ("user_id", "first_name", "middle_initial", "last_name", "email", "type")
      VALUES ('${user_id}', '${first_name}', '${middle_initial}', '${last_name}', '${email}', '${type}')
  returning id
      `)
  }

  static async allUsers() {
    const findAllQuery = 'SELECT * FROM users;';
    return db.query(findAllQuery)
  }

  static async selectUser(user_id) {
    console.log(user_id);
    const findOneQuery = 'SELECT * FROM users WHERE user_id = ${user_id}';
    return db.query(findOneQuery)
  }
}

module.exports = User;

This is from my Routes file (Routes.js)

const express = require('express');
const dataFunctions = require('./catalog.js');

const AppRouter = express.Router();

AppRouter.post('/user', dataFunctions.createUser);
AppRouter.get('/users', dataFunctions.listAllUsers);
AppRouter.get('/user/:id', dataFunctions.listUserByUserID);
AppRouter.delete('/user/:id', dataFunctions.deleteUserByUserID);

module.exports = AppRouter;

This is from my Catalog file (Routes.js)

const Users = require('../models/users.js')

// Create

async function createUser(req, res) {
  try {
  console.log(req.body);
  const userId = await Users.newUser(req.body.user_id, req.body.first_name, req.body.middle_initial, req.body.last_name, req.body.email, req.body.type)
 res.status(201).send(`User ID: ${userId.id}`);
  } catch(error) {
    res.status(400).send(error);
  }
}

// List all

async function listAllUsers(req, res) {
  try {
  const userList = await Users.allUsers();
  console.log(userList);
  res.status(200).send(userList);
  } catch(error) {
    res.status(400).send(error);
  }
}

// List by ID

async function listUserByUserID(req, res) {
  try {
  const userList = await Users.selectUser(req.body.user_id);
  console.log(userList);
  res.status(200).send(userList);
  } catch(error) {
    res.status(400).send(error);
  }
}

module.exports = {
  createUser,
  listAllUsers,
  listUserByUserID
}

3 Answers3

1

Never use string concatenation for querying, you already have mechanism called prepared statement, signature like

.query('SELECT * FROM `books` WHERE `author` = ?', ['David'])

It will sanitize input for you and partially prevent sql-injection attacks, also always do validation of input values. And if you are not want to use ORM like typeorm, Sequelize, you can use knex.js which can only create query strings and fully manage db interaction

VitoMakarevich
  • 439
  • 2
  • 5
  • Thanks for responding; I apologize for the late comment. I see what you are talking about with the risk of string concatenation. With my first try I just wanted to make sure that my basic query was working. Now I want to start cleaning it up and adding some security measures like you suggested. Once I feel comfortable with that, then I will probably try out Sequelize or typeorm, but i want to get the concepts down first before I rely a tool to show me. I will check out knex.js too! Thanks. – JMcGreggor Sep 04 '19 at 21:20
  • I did run into an issue with getting query to work though. For the life of me I could not get the id value to pass into the statement. I tried several variations but nothing seemed to make a difference. Do you have any idea what I may have been doing wrong? I will past my examples in the comment below. – JMcGreggor Sep 04 '19 at 21:22
  • `return db.query('SELECT * FROM users WHERE id = ?', id);` – JMcGreggor Sep 04 '19 at 21:32
  • `return db.query('SELECT * FROM users WHERE id = ?', [id]);` – JMcGreggor Sep 04 '19 at 21:32
  • `return db.query('SELECT * FROM users WHERE id = ?', ['id']);` – JMcGreggor Sep 04 '19 at 21:33
  • `return db.query('SELECT * FROM `users` WHERE `id` = ?', [id]);` – JMcGreggor Sep 04 '19 at 21:33
  • `var dbresult = db.query('SELECT * FROM `users` WHERE `id` = ?', [id]);` – JMcGreggor Sep 04 '19 at 21:34
  • `var dbresult = db.query('SELECT * FROM users WHERE id = ?', [id]);` – JMcGreggor Sep 04 '19 at 21:34
  • `var dbresult = db.query(`SELECT * FROM users WHERE id = ?`, [id]);` – JMcGreggor Sep 04 '19 at 21:34
  • Also, if you know of a better way to post the code snippets as comments please feel free to let me know ;) – JMcGreggor Sep 04 '19 at 21:35
  • Either way, the end result was that the id value was never passed into the ?. In the console I always saw the following... ` QUERY: SELECT * FROM users WHERE id = ? ` – JMcGreggor Sep 04 '19 at 21:36
  • FYI: I am checking that the id value exists in the function scope just before the query: ` console.log(id);` – JMcGreggor Sep 04 '19 at 21:37
0

You should never insert the values directly into your query like that. Consider the example:

db.query(`
  INSERT INTO messages (message, username)
  VALUES ('${message}', '${username}')
`);

If the username was my authenticated username, but the message was whatever value I typed into the UI, I could pretend to be someone else by sending a message like: I am stupid', 'someone_else') --

The SQL would then look like:

INSERT INTO messages (message, username)
VALUES ('I am stupid', 'someone_else') --', 'my_username')

The --', 'my_username') bit is treated as a comment, so it looks like someone_else said I am stupid. This is one of the most common and easily exploitable vulnerabilities in web applications.

Solution 1

You could parameterise your query:

db.query(`
  INSERT INTO messages (message, username)
  VALUES (?, ?)
`, [message, username]);

This is secure, but harder to read (in my opinion) and you have to be very careful to always do this consistently.

Solution 2

https://www.atdatabases.org provides database APIs that are relatively easy to use, and totally safe from this kind of attack. You would just do:

import connect, {sql} from '@databases/pg';

const db = connect();

db.query(sql`
  INSERT INTO messages (message, username)
  VALUES (${message}, ${username})
`);

to safely execute the same query. The sql tag ensures the data is properly handled/escaped and @databases/pg automatically enforces that you always add the sql tag. N.B. there are then no quotes around the parameters.

ForbesLindesay
  • 10,482
  • 3
  • 47
  • 74
-1

Instead of using single quotes in select query in static async selectUser use ``.

Varun Mehta
  • 111
  • 1
  • 7
  • Ah thank you for that, I was trying various options and overlooked the ``. The query still is not working. It may not be the query but my lack of how to handle URL queries/routes properly. – JMcGreggor Jul 05 '19 at 12:43
  • I have found a few things on how to handle queries and parameters in routes but can not seem to get it right. – JMcGreggor Jul 05 '19 at 12:54
  • I guess to start I should ask how do I correctly rewrite my route, so that I can get the url parameter? I have read through this post (https://stackoverflow.com/questions/20089582/how-to-get-a-url-parameter-in-express) but still have not had any luck – JMcGreggor Jul 05 '19 at 12:56
  • Hi JMcGreggor Main problem with your route is ` AppRouter.get('/user/:id', dataFunctions.listUserByUserID); ` your are making it a get request which passes the id in req.params not req.body. follow the below If route is a GET request like the below `AppRouter.get('/user/:id',` Access id with `req.params.id` If route is a post request like the below `AppRouter.post('/user/:id',` access id with `req.body.id` – Varun Mehta Jul 06 '19 at 14:17
  • Oh! Thank you very much. That was very helpful. I have it partially working now. It does not seem like it matters what I put in after the : it still passes in the value. I have req.body.id working, but not req.params.id. I still am working through how all of it ties together which is why I want to get both working - so i have a better understanding. Thanks again for your help! I will post another comment with the final results once it is all working. – JMcGreggor Jul 07 '19 at 06:49
  • Please do not use this solution, it is vulnerable to SQL Injection, which is very easy to exploit. – ForbesLindesay Jul 24 '19 at 11:09
  • Thanks ForbesLindesay. I agree, with you. This was just to get me started. Now I am working on sanitizing and protecting my queries. I will look at your example below. – JMcGreggor Sep 04 '19 at 21:38