2
\$\begingroup\$

I'm currently learning Express.js and I wrote this authentication code. I'm unsure if what i have is correct, how to improve it and if it's secure. For user and session data storage I'm using PostgreSQL database. For session management I'm using express-session and connect-pg-simple libraries.

Any and all feedback is appreciated!

auth.js

const login = async (req, res) => {
  try {
    const { username, password } = req.body;
    if (!username || !password) {
      return res
        .status(400)
        .json({ message: "Username and password are required." });
    }
    const user = await db.query("SELECT * FROM users WHERE username = $1", [
      username,
    ]);
    if (user.rows.length === 0) {
      return res.status(401).json({ message: "Invalid credentials." });
    }
    const validPassword = await bcrypt.compare(password, user.rows[0].password);
    if (validPassword) {
      req.session.userId = user.rows[0].user_id;
      res.status(200).json({ message: "Login successful" });
    } else {
      return res.status(401).json({ message: "Invalid credentials." });
    }
  } catch (error) {
    console.log(error);
    res.status(500).json({ message: "An error occurred during login" });
  }
};

const registration = async (req, res) => {
  try {
    const { username, password } = req.body;

    if (!username || !password) {
      return res
        .status(400)
        .json({ message: "Username and password are required." });
    }
    const user = await db.query("SELECT * FROM users WHERE username = $1", [
      username,
    ]);

    if (user.rowCount !== 0) {
      return res.status(409).json({ message: "Username already exists" });
    }

    const hashedPassword = await bcrypt.hash(password, 10);
    await db.query("INSERT INTO users (username, password) VALUES ($1, $2)", [
      username,
      hashedPassword,
    ]);
    res.status(201).json({ message: "User registered successfully" });
  } catch (error) {
    console.log(error);
    res.status(500).json({ message: "An error occurred during registration" });
  }
};

const logout = async (req, res) => {
  try {
    req.session.destroy();
    res.clearCookie("connect.sid");
    res.status(201).json({ message: "User logged out successfully" });
  } catch (error) {
    console.log(error);
    res.status(500).json({ message: "An error occurred" });
  }
};

isAuthenticated.js

(Middleware to check if user is authenticated)

const isAuthenticated = (req, res, next) => {
  if (req.session.userId) {
    next();
  } else {
    return res.sendStatus(401);
  }
};
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Welcome! A few suggestions:

  1. Run the server locally so you can test. I guarantee you will find some issue during testing that you didn't expect. Here's an example (may need to do a bit of package installing or further configuration):

    const port = process.env.PORT || 8000;
    const app = express();
    app.listen(port, () => {
        console.log(`server is running on ${port}`);
    })
    
  2. Make sure your queries are not vulnerable to SQL injection. For example:

    username = "JOHN OR 1 = 1"
    
  3. Your users might be lazy. Think of ways to ensure secure credentials (keep in mind that many existing battle-tested packages can do this):

    username = "ADMIN"
    password = "1234"
    

    Of course, it's also possible to modularize and address these things in other parts of the codebase - if you've done the above, try to actually deploy using the free tier of a service like Render. Expect to go down a rabbit hole of proxies / fingerprinting / cookies / session configuration / cookie configuration.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.