0

got this function to check if the username is an admin.

module.exports = {
 checkAdmin: function(username){
  var _this = this;
  var admin = null;
  sql.execute(sql.format('SELECT * FROM tbl_admins'), (err, result, fields) => {
    if (err) {
        logger.error(err.message);
        return;
    }
    adminArray = []
    result.forEach(element => {
        if(element.Username == username){
            adminArray.push(element.Username)
        }
    });
    
    if (adminArray.includes(username)){
      _this.admin = true;
    }else{
      _this.admin = false;
    }
})
return admin;

} }

And this is in the Express file.

var check = Admin.checkAdmin(req.body.username);
if (check == false) {
  console.log("Wrong")
  res.send({
    message: 'Access denied'
  }); //bad request
  return;
}

The SQL is correct. The problem is to set the variable "admin" inside the SQL function. The function returns "".

Thx

isherwood
  • 58,414
  • 16
  • 114
  • 157
CDOGamer
  • 33
  • 4
  • 1
    Does this answer your question? [How to return the response from an asynchronous call](https://stackoverflow.com/questions/14220321/how-to-return-the-response-from-an-asynchronous-call) – Mickael B. Oct 07 '21 at 13:43
  • 1
    Pass a callback function to your `checkAdmin` which is passed the value of check inside the SQL query handler. Right now you are just returning the value null since the query handler has not even executed. – Abrar Hossain Oct 07 '21 at 13:51
  • If i done it right, the error appeard: callback is not a function callback(false); @AbrarHossain – CDOGamer Oct 07 '21 at 14:03
  • 2
    Looks like your **return admin;** is located outside your function looking at your code. And your **_this.admin = true;** and **_this.admin = false;** should just be **admin = true;** since what your returning is admin – slashroot Oct 07 '21 at 14:12
  • @slashroot No nothing change – CDOGamer Oct 07 '21 at 14:16

1 Answers1

2

I can't help it but to notice that your whole checkAdmin function has a style I haven't seen before. Let me try to simplify it a bit, maybe it helps with the issue.

// Admin.js

// Return a Promise, so it's easier
// to deal with the asynchronous SQL Call

function checkAdmin (username) {
  return new Promise((resolve, reject) => {

    sql.execute(
      sql.format('SELECT * FROM tbl_admins'),
      (err, result, fields) => {

      if (err) { return reject(err); }

      let foundUser = false;
      result.map((element) => {
        if (element.Username == username) { foundUser = true; }
      });

      return resolve(foundUser);

    });

  });
}

module.exports = { checkAdmin };

First we return a Promise. Then we stay inside the callback function from the SQL call. If there is an error err we reject the Promise. If there is no error we resolve the Promise with a boolean foundUser. If we find the wanted username inside the table foundUser will be true, otherwise it will be false.

Now keep in mind that checkAdmin returns a Promise now.

// Router Controller

const Admin = require('./Admin');

app.post('/some-route-only-admins-can-use', (req, res) => {
   
  const username = req.body.username;

  Admin.checkAdmin(username)
  .then((isAdmin) => {
  
    if (isAdmin) {
      return res.status(200).send({
        message: 'Access granted'
      });
    } else {
      return res.status(401).send({
        message: 'Access denied'
      });
    }

  })
  .catch((err) => {
    logger.error(err.message);
  });

});

This might differ from your code but it should give your the right idea hopefully.

Problem with your code

The main issue with your original checkAdmin method is that the SQL call is asynchronous. In your code you don't wait for it to finish, but you instantly return admin which at that point will always be null. And null == false.

Don't think about this too much. You normally don't need to alter it or use it.

turbopasi
  • 3,327
  • 2
  • 16
  • 43