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\$

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.