RomainVialard / Google-Plus-Community-Migrator

https://docs.google.com/document/d/1UGhxaN5AiRXXL0Ki0DlVWLYJo_YYiEYhM2w1caRhljU/edit
12 stars 5 forks source link

Add more secured rules for +1s #11

Open RomainVialard opened 5 years ago

RomainVialard commented 5 years ago

Currently it's possible to write anything in the +1 part of the database. Best to secure with validation, following this example: https://stackoverflow.com/questions/37954217/is-the-way-the-firebase-database-quickstart-handles-counts-secure/37956590#37956590

RomainVialard commented 5 years ago

When a user +1 a post, we must update 2 different locations in Firebase:

The goal is to create security rules that prevent users to update one location but not the other (eg: someone update the total number of +1 for a post without adding a new entry under the plusoners path or someone update the plusoners path but not the total number of +1 recorded under the posts path).

Note that we could also use a Firebase function to secure this. ie: we would forbid people to update the sum of +1 under the posts path, letting them only add or remove a +1 linked to their own Google ID under the plusoners path. And a Firebase function would be triggered every time someone updates the plusoners path, to sum again the number of +1 and update the posts path from server side.

RomainVialard commented 5 years ago

Under the plusoners path, we want to let users add a new record (a new +1) only if they haven't already +1 the same post in the past (you can't +1 the same post twice). Thus we can begin the validation with:

"plusoners": {
  "$post_id": { // The ID of an activity.
    "$google_id": { // The Google ID of the person who +1'd this activity.
      ".write": "auth != null && auth.token.firebase.identities['google.com'][0] == $google_id",
      ".validate": "!data.exists()",
    }
  }
}

We should also let users cancel a previous +1, ie remove (set to null) their +1 on a post if it exists. Thus: ".validate": "!data.exists() || data.exists() && newData.val() == null"

RomainVialard commented 5 years ago

We also want to ensure that when they add new data under the plusoners path, they also increment the sum under the posts path (if a new person +1 a post, the total number of +1 for the post should be incremented) - ie the new value for totalItems should be equal to the old value + 1.

"plusoners": {
  "$post_id": { // The ID of an activity.
    "$google_id": { // The Google ID of the person who +1'd this activity.
      ".write": "auth != null && auth.token.firebase.identities['google.com'][0] == $google_id",
      ".validate": "!data.exists() && newData.parent().parent().parent().child('posts/' + $post_id + '/object/plusoners/totalItems').val() == data.parent().parent().parent().child('posts/' + $post_id + '/object/plusoners/totalItems').val() + 1",
    }
  }
}
RomainVialard commented 5 years ago

Same thing when removing a +1, the new value for totalItems should be equal to the old value - 1. When combining both (validation on adding and removing a +1), we get:

"plusoners": {
  "$post_id": { // The ID of an activity.
    "$google_id": { // The Google ID of the person who +1'd this activity.
      ".write": "auth != null && auth.token.firebase.identities['google.com'][0] == $google_id",
      ".validate": "(!data.exists() && newData.parent().parent().parent().child('posts/' + $post_id + '/object/plusoners/totalItems').val() == data.parent().parent().parent().child('posts/' + $post_id + '/object/plusoners/totalItems').val() + 1) || (data.exists() && newData.val() == null && newData.parent().parent().parent().child('posts/' + $post_id + '/object/plusoners/totalItems').val() == data.parent().parent().parent().child('posts/' + $post_id + '/object/plusoners/totalItems').val() - 1)",
    }
  }
}
RomainVialard commented 5 years ago

Then we need to add similar validation rules under the posts path of the database, to make sure it's not possible to increment or decrement the total number of +1s for a post without updating the plusoners path.

"posts": {
  ".indexOn": ["published"],
  "$post_id": {
    "object": { // The object of this activity.
      "plusoners": { // People who +1'd this activity.
        "totalItems": { // Total number of people who +1'd this activity.
          ".write": "auth != null",
          ".validate": ""
        }
      } 
    }
  }
}

First we can make sure that the totalItems value can only be incremented or decremented one by one: ".validate": "newData.val() == data.val() + 1 || (newData.val() == data.val() - 1"

RomainVialard commented 5 years ago

And, when user wants to increment the value for totalItems (ie: when he wants to +1 a post), we can make sure that:

  1. he is adding at the same time a new entry, in his own name (using his Google ID) under the plusoners path.
  2. there was no existing +1 for this user on this post

Giving us quite a long rule: newData.val() == data.val() + 1 && newData.parent().parent().parent().parent().parent().child('plusoners/' + $post_id + '/' + auth.token.firebase.identities['google.com'][0]).exists() && !data.parent().parent().parent().parent().parent().child('plusoners/' + $post_id + '/' + auth.token.firebase.identities['google.com'][0]).exists()

Especially when you add its counterpart (when a user wants to decrement the number of +1, an entry with his own Google ID, for this post, should exists in the database under the plusoners path and it should be removed):

"posts": {
  ".indexOn": ["published"],

  "$post_id": {
    "object": { // The object of this activity.

      "plusoners": { // People who +1'd this activity.
        "selfLink": {},
        "totalItems": { // Total number of people who +1'd this activity.
          ".write": "auth != null",
          ".validate": "(newData.val() == data.val() + 1 && newData.parent().parent().parent().parent().parent().child('plusoners/' + $post_id + '/' + auth.token.firebase.identities['google.com'][0]).exists() && !data.parent().parent().parent().parent().parent().child('plusoners/' + $post_id + '/' + auth.token.firebase.identities['google.com'][0]).exists()) || (newData.val() == data.val() - 1  && !newData.parent().parent().parent().parent().parent().child('plusoners/' + $post_id + '/' + auth.token.firebase.identities['google.com'][0]).exists() && data.parent().parent().parent().parent().parent().child('plusoners/' + $post_id + '/' + auth.token.firebase.identities['google.com'][0]).exists())"
        }
      } 
    }
  }
}