Skip to content

handleLogIn could be cleaned up #39

Description

@finnhodgkin
  • Why is dbHash a let? You only assign it a value once and only use it in one place, so just declare it there:
      getHash(data[0], (err, result) => {
        if (err) console.log(err);
        else if (result.length === 0) {
          res.writeHead(200, {'Content-Type': 'text/html'});
          res.end('Username does not exist');
        }
        else {
          const dbHash = result[0].password;
          //Compare the retrieved hash with the one provided by user.
          bcrypt.compare(data[1], dbHash, (err, result) => {
  • You're not handling actual errors from your queries:
getHash(data[0], (err, result) => {
        if (err) {
          res.writeHead(500, {'Content-Type': 'text/html'});
          return res.end('Sorry there was an error on our end');
        }
        else if (result.length === 0) {
          res.writeHead(200, {'Content-Type': 'text/html'});
          return res.end('Username does not exist');
        }
        // ...The rest of your code
        // ...
  • You could make just one call to the database that gets the user's username and password at the same time, rather than having two nested calls.
req.on('end', ()=>{
      const [username, password] = data.split(',');
      getUsernameAndPassword(username, (err, result) => {
        if (err) {
          res.writeHead(500, {'Content-Type': 'text/html'});
          return res.end('Sorry there was an error on our end');
        }
        else if (result.length === 0) {
          res.writeHead(200, {'Content-Type': 'text/html'});
          return res.end('User does not exist');
        }
        const [usernameDb, hashDb] = result[0];
        //Compare the retrieved hash with the one provided by user.
        bcrypt.compare(password, hashDb, (err, result) => {
        //... The rest
        //...
  • Again, handle the errors in the bcrypt callback:
bcrypt.compare(password, hashDb, (err, result) => {
          if (err) // HANDLE ME
          else {
            if (result === false) {
              res.writeHead(200, {'Content-Type': 'text/html'});
              res.end('Wrong Password');
            } else {

Also there's some very weird nesting in that callback. Why is there if(err) {stuff;} else { if(result === false) { ... } rather than just an else if

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions