User is added to mongoDB database after registration despite "username" chosen already existing

Question

I am developping a web app with the MERN stack.

On Postman or on my front-end form, when I register a user with an existing email, the user is not added to database. I use the same logic to check if the username picked by the user is already taken. If it's taken, the error message is dispatched but the user is still added to the database.

In the User model, the field Username is unique, just like the field Email.

My register route:

const express = require("express");
const router = express.Router();
const User = require("../../models/User");
const { check, validationResult } = require("express-validator");
const bcrypt = require("bcryptjs");
const jwt = require("jsonwebtoken");
const config = require("config");

// @route GET register
// @desc Register
// @access Public
router.get("/", (req, res) => {
    res.send("Create an account");
});

// @route POST register
// @desc Register
// @access Public
router.post(
    "/",
    [
        check("email", "Please, include a valid email.").isEmail(),
        check(
            "password",
            "Please, enter a password with 6 or more characters"
        ).isLength({ min: 6 }),
        check("username", "Please, include a valid username.")
            .not()
            .isEmpty()
    ],
    async (req, res) => {
        const errors = validationResult(req);
        if (!errors.isEmpty()) {
            return res.status(400).json({ errors: errors.array() });
        }

        const {
            email,
            password,
            birthdate,
            birthplace,
            sun,
            username,
            date,
            is_paying
        } = req.body;

        try {
            // See if user exists
            let user = await User.findOne({ $or: [{ username }, { email }] });
            if (user) {
                res.status(400).json({ errors: [{ msg: "User already exists" }] });
            }

            // Create new user from User Model
            user = new User({
                email,
                password,
                birthdate,
                birthplace,
                sun,
                username,
                date,
                is_paying
            });

            // Encrypt password
            const salt = await bcrypt.genSalt(10);
            user.password = await bcrypt.hash(password, salt);

            // Add user to database
            await user.save();

            // Return JWT
            const payload = {
                user: {
                    id: user.id
                }
            };

            jwt.sign(
                payload,
                config.get("jwtSecret"),
                { expiresIn: 360000 },
                (err, token) => {
                    if (err) throw err;
                    res.json({ token });
                }
            );
        } catch (err) {
            console.error(err.message);
            res.status(500).send("Server Error");
        }
    }
);

module.exports = router;

My register action :

// REGISTER USER
export const register = ({
    email,
    password,
    birthdate,
    gender,
    sun,
    username
}) => async dispatch => {
    const config = {
        headers: {
            "Content-Type": "application/json"
        }
    };

    const body = JSON.stringify({
        email,
        password,
        birthdate,
        gender,
        sun,
        username
    });

    try {
        const res = await axios.post("/register", body, config);
        dispatch({
            type: REGISTER_SUCCESS,
            payload: res.data
        });
        dispatch(loadUser());
    } catch (err) {
        const errors = err.response.data.errors;
        if (errors) {
            errors.forEach(error => dispatch(setAlert(error.msg, "danger")));
        }

        dispatch({
            type: REGISTER_FAIL
        });
    }
};

Zlatko · Accepted Answer

You need to stop processing the request if the user exists:

        // See if user exists
        let user = await User.findOne({ $or: [{ username }, { email }] });
        if (user) {
            res.status(400).json({ errors: [{ msg: "User already exists" }] });
        }

        // Create new user from User Model
        user = new User({
            email,
            password,
            birthdate,
            birthplace,
            sun,
            username,
            date,
            is_paying
        });

See that? if (user) { res.json() } but you still go on. Make that return res.json() for a quick fix.

A bit better fix is you should set a unique index on the username and on the email fields. In mongoose it looks something like:

const userSchema = new Schema({
  name: {
    type: String,
    index: {
      unique: true
    }
  }
});

Or if you want to allow same usernames but the uniqueness to be the username-email combo, make a compound index.

And even better - move the whole thing to another file. Call the file user-registration.service.js. Make the file export one function:

async function registerUser(username, email, password) {
  // check uniqueness here. if it fails, _throw an error!_
  // if all is ok, save the user, return the details.
}

module.exports = {
  registerUser
}

That way your route controller can just say:

router.post(
    "/",
    [
        check("email", "Please, include a valid email.").isEmail(),
        check(
            "password",
            "Please, enter a password with 6 or more characters"
        ).isLength({ min: 6 }),
        check("username", "Please, include a valid username.")
            .not()
            .isEmpty()
    ],
    async (req, res) => {
        const errors = validationResult(req);
        if (!errors.isEmpty()) {
            return res.status(400).json({ errors: errors.array() });
        }

        const {
            email,
            password,
            birthdate,
            birthplace,
            sun,
            username,
            date,
            is_paying
        } = req.body;

    try {
      const user = await registerUser();
      return res.json(user); // or token or whatnot.
    } catch (err) {
      // see here? only one place to deal with that error.
      return res.status(400)
        .json(err);
    }
    }

Now you only have that one place to deal with errors. You could even push it further and simply omit that try/catch and let a global error handler deal with it.

User is added to mongoDB database after registration despite "username" chosen already existing

Answers (2)

Related Questions

User is added to mongoDB database after registration despite &quot;username&quot; chosen already existing

Answers (2)

Related Questions

User is added to mongoDB database after registration despite "username" chosen already existing