how to stop users from viewing and updating another user's data in node.js?

Admin 1
Viewed 39

I am storing a parking detail with a merchant id in the mongoose schema since a parking belongs to a certain merchant user and it cannot be empty or null.

Here is the model:

const parkingSchema = new mongoose.Schema({
  merchantId: {
    type: mongoose.Schema.Types.ObjectId,
    required: true,
    ref: "Merchant",
  },
  //other details
})

merchant model is something like this:

const merchantSchema = new mongoose.Schema({
  merchantId: {
    type: mongoose.Schema.Types.ObjectId,
    ref: "Auth",
    },
  //other details
 })

And finally the auth schema:

const authSchema = new mongoose.Schema({
    accountType: {
        type: String,
        required: true,
        trim: true,
        default: "user",
        enum: ["merchant", "user", "provider"],
      },
    //other details
  })

If the original user wishes it, I simply want to update the parking data; otherwise, I want to throw an error.

I am using jsonwebtoken to authenticate users.

Here is the query to update the data:

exports.updateParking = async (req, res) => {
  try {
    const { parkingName, price, address, name, phoneNumber, about } = req.body;
    const { parkingImage } = req.files;
    const check_exist = await Auth.findById(req.data.id);
    if (!check_exist) return res.status(404).json({ error: "User not found" });
    console.log(req.data.id);

    const updateData = await Parking.findByIdAndUpdate(
      { _id: req.params.id, merchantId: req.data.id }, // I think here is the problem
      {
        $set: {
          parkingName,
          price,
          address,
          ...
        },
      }
    );
    return res.status(200).json({
      success: true,
      msg: "Parking has updated successfully",
    });
    } catch (error) {
    return error.message;
    }
    };

However, the issue is that other users can now update another user's data which I want to stop

below is the query of middleware:

routing.patch("/parking/update/:id", middleware.authenticateToken, merchant.updateParking)
mongoose express node.js mongodb
2 Answers

You should be showing each user only their parkings that they have created or belong to them.

const myParkings = async (req, res) => {
  // always await code in try/catch block
  const merchants = await Parkings.find({ user: req.user._id })
  .. then populate the fields that you want to show

  res.status(200).json({
    success: true,
    bookings,
  });
};

you have to set this req.user._id when user logins. You could create a session.

user_102628050

I think what you're looking for is something like CASL Mongoose (or a similar package), and more specifically, the "conditions" section of the CASL docs.

What you're dealing with here is the distinction between 2 concepts:

  1. AuthN (authentication) - determines who someone is and whether they are "authenticated" to make an API request
  2. AuthZ (authorization) - determines what the authenticated user is allowed to do

In your app, middleware.authenticateToken is responsible for the AuthN piece of the equation. It makes sure that only users that have created an account are able to make requests to your API routes.

What you still need to solve for is the AuthZ piece, which can be done in a bunch of different ways, but one popular one is to use CASL, which is a Node AuthZ library that allows you to utilize your ORM's native query syntax to limit actions based on the authenticated (AuthN) user's attributes.

In other words, you can do something like, "Only allow user with ID 1 to update Parking entities that he/she owns". Below is generally what you're looking for (not tested for your use case, but the general idea is here):

const casl = require('@casl/ability');

// Define what a `Auth` (user) can do based on their database ID
function defineMerchantAbilities(merchantUser) {
    const abilities = casl.defineAbility((allow, deny) => {
       
       // Allow merchant to update a parking record that they own 
       allow('update', 'Parking', { merchantId: merchantUser.id })
    })

    return abilities
}

exports.updateParking = async (req, res) => {

  const userId = req.data.id
  const parkingId = req.params.id

  // Find your merchant user in DB (see my comments at end of post)
  const merchantUser = await Auth.findById(userId)

  // Find your parking record
  const parking = await Parking.findById(parkingId)

  // Pass user to your ability function
  const ability = defineMerchantAbilities(merchantUser)

  // This will throw an error if a user who does not own this Parking record
  // tries to update it
  casl.ForbiddenError
      .from(ability)
      .throwUnlessCan('update', casl.subject('Parking', parking))

  // If you make it here, you know this user is authorized to make the change
  Parking.findByIdAndUpdate( ...your code here )
}

Additional comments/notes:

  • I would recommend removing your try/catch handler and using an Express default error handler as it will reduce the boilerplate you have to write for each route.
  • I would also recommend writing a middleware that finds a user by ID in the database and attaches it to a custom property called req.user so you always have req.user available to you in your authenticated routes.
user_74377370
Related