CODEWITHSUNDEEP
javascriptnode.jsmongodbapimongoose
Rron Jakupi
Rron Jakupi

Reputation: 15

Node JS preventin users from deleting other user's products

I have a REST API built with Node JS and I'm currently using MongoDB as my database. I want to prevent the users from deleting another user's products and for this I checked if the userId from the decoded token is the same as the product userId.

Product schema

const mongoose = require("mongoose");

const productSchema = mongoose.Schema(
  {
    _id: mongoose.Schema.Types.ObjectId,
    userId: mongoose.Schema.Types.ObjectId,
    name: { type: String, required: true },
    price: { type: Number, required: true },
    productImage: { type: String, required: false },
    category: {
      type: mongoose.Schema.Types.ObjectId,
      ref: "Category",
      required: true
    },
    gender: { type: String, required: true }
  },
  { timestamps: { createdAt: "created_at" } }
);

module.exports = mongoose.model("Product", productSchema);

The delete product method: 

  const id = req.params.productId;
  Product.findById({ _id: id }).then((product) => {
    if (product.userId != req.user._id) {
      return res.status(401).json("Not authorized");
    } else {
      Product.deleteOne({ _id: id })
        .exec()
        .then(() => {
          return res.status(200).json({
            message: "Product deleted succesfully",
          });
        })
        .catch((err) => {
          console.log(err);
          return res.status(500).json({
            error: err,
          });
        });
    }
  });
};

As you guys see first I'm searching executing the findByID method to access the userId property of the product, then I'm comparing the userId from the response with the userId from the decoded token.

I don't think my method is very efficient since it's running both findById and deleteOne methods.

Can you help me with finding a better solution for this?

Upvotes: 1

Views: 697

Answers (1)

typicallearner
typicallearner

Reputation: 256

as Guy Incognito mentioned, what you are trying to do is an OK thing and you may want to keep it this way in case you want to send a 404 status stating the product they are trying to remove does not exist.

however, if you are trying to do it with only one request

Product.deleteOne({ _id: id, userId: req.user._id })

hope it helps!

Upvotes: 1

Related Questions