FirebaseExtended / bolt

Bolt Compiler (Firebase Security and Modeling)
Apache License 2.0
896 stars 108 forks source link

Should Bolt allow partial edits by default? #242

Closed lukepighetti closed 5 years ago

lukepighetti commented 5 years ago

Consider this simple type and path.

path /private/{uid}/{id} is SecretMessage {
  read() { auth.uid == uid }
  write() { auth.uid == uid }
}

type SecretMessage {
    uid: String,
    message: String,
    timestamp: Number,
    isModerator: Boolean
}

This generates

{
  "rules": {
    "private": {
      "$uid": {
        "$id": {
          ".validate": "newData.hasChildren(['uid', 'message', 'timestamp', 'isModerator'])",
          "uid": {
            ".validate": "newData.isString()"
          },
          "message": {
            ".validate": "newData.isString()"
          },
          "timestamp": {
            ".validate": "newData.isNumber()"
          },
          "isModerator": {
            ".validate": "newData.isBoolean()"
          },
          "$other": {
            ".validate": "false"
          },
          ".read": "auth.uid == $uid",
          ".write": "auth.uid == $uid"
        }
      }
    }
  }
}

This requires that any create or update operation include ALL of those children. This means you cannot do partial updates. I would argue that this is too strict. The alternative is to make all those Type|Null which is too loose.

I suggest that we generate the offending .validate line as such:

 ".validate": "newData.hasChildren(['uid', 'message', 'timestamp', 'isModerator']) ||
 data.hasChildren(['uid', 'message', 'timestamp', 'isModerator']",
lukepighetti commented 5 years ago

Nevermind, I think I'm doing something wrong.

danielkcz commented 5 years ago

@lukepighetti How did you solve this? Partial updates feel legit to me and that validation prevents it.

lukepighetti commented 5 years ago

What I found is that partial updates work just fine. Not sure what I was doing wrong, it was a while back. If you have more info maybe it will jog my memory?

danielkcz commented 5 years ago

Well, it's kinda obvious when there is a validation that expects newData to have all listed children, it will surely fail if running an update with only one of them. Your suggestion makes much more sense to have validate existing data as well.

lukepighetti commented 5 years ago

What I found is that even though it reads like it would fail a partial update in practice it does not. Can you confirm that you've tried this?

danielkcz commented 5 years ago

I tried it in the provided rules simulator and it was indeed failing validation with a partial update. After making it of Any type, it was working just fine.

Well, it doesn't matter that much, Bolt is kinda unmaintained anyway, I was merely curious if there is something I am missing out. Thanks for your responses.

lukepighetti commented 5 years ago

That's really strange because I was definitely able to do partial updates but that was a while back and I could be mistaken.