zooniverse / vox

Voting app
Apache License 2.0
0 stars 1 forks source link

Fix firebase security rules for private data #15

Closed rogerhutchings closed 8 years ago

rogerhutchings commented 8 years ago

Currently, anyone logged in can change anything; auth rules should be restricted so users can only change their own votes.

Ref #12

simoneduca commented 8 years ago

Change what exactly? I have noticed your votes, but I couldn't overwrite/remove them.

rogerhutchings commented 8 years ago

A bug in the code could overwrite votes, however, as it only checks for someone to be logged in, not the owner of the vote. So that should be changed in https://console.firebase.google.com/project/project-6243802502502885389/database/rules so that only the user can edit their own votes

simoneduca commented 8 years ago

I'm trying to change the rules according to the FB docs (user tab). I've tried this

{
  "rules": {
    "issues": {
      "$uid": {
        ".read": "true",
        ".write": "$uid === auth.uid"
      }
    },
    "users": {
      "$uid": {
        ".read": "auth != null",
        ".write": "$uid === auth.uid"
      }
    }
  }
}

But now I get permission denied every time I vote. Any thoughts?

rogerhutchings commented 8 years ago

Well, issues isn't keyed by auth.uid for one thing

simoneduca commented 8 years ago

@rogerhutchings how does this look? I changed the rules in the console so users are keyed by Firebase uid variable. The behaviour is as expected.

{
  "rules": {
    "issues": {
      ".read": "true",
        ".write": "auth != null"
    },
    "users": {
      "$uid": {
        ".read": "auth != null",
        ".write": "auth != null",
      }
    }
  }
}
rogerhutchings commented 8 years ago

A bug could still cause a user to overwrite another user's data, as anyone can read / write to any user as long as they're logged in. The rules were correct before, but I think the problem lies in that nodes under users are keyed by GitHub ID, not Firebase uid. So if you use the uid Firebase auth provides, this...

{
  "rules": {
    "issues": {
      ".read": "true",
      ".write": "auth != null"
    },
    "users": {
      "$uid": {
        ".read": "$uid === auth.uid",
        ".write": "$uid === auth.uid"
      }
    }
  }
}

...should work

simoneduca commented 8 years ago

Ah that makes sense thanks! Just made a PR.