Firestore security rules and arithmetic in paths

78 views
Skip to first unread message

Bartholomew Furrow

unread,
Apr 16, 2018, 11:37:50 PM4/16/18
to Firebase Google Group
The following formulation used to work:

      function friendshipExists(userA, userB) {
        return userA == userB ||
               exists(/databases/$(database)/documents/friendships/$(userA + '-' + userB)) ||
               exists(/databases/$(database)/documents/friendships/$(userB + '-' + userA));
      }

It no longer seems to. Of course I might have another problem, but this isn't my only use of exists() -- just the only one with arithmetic in the path. Is it possible that this no longer works?

By the way, figuring this out is a huge pain point. Here's why:

1. I can't tell what rule is failing without commenting out rules.
2. I can't even really tell what this rule is doing. Is it checking the path I think it's checking?
3. I can't tell when my new rules have been published, so I just sort of have to wait until I'm pretty sure. It's supposed to be within 10 minutes, and that's all I know.
4. My iOS app seems to cache rule failures, so it behaves differently after an app restart (this one took me a while to figure out).

Cheers,
Bartholomew

Bartholomew Furrow

unread,
Apr 17, 2018, 12:43:13 PM4/17/18
to Firebase Google Group
This appears to be a bug, but not the one I thought. exists(A) || exists(B) returns false (or possibly error) while exists(B) || exists(A) returns true. As it happens, B is the thing that exists. To wit, when I pass (AAA, BBB) into the following functions, only the second one returns true, when /friendships/BBB-AAA exists:

// First version:
      function friendshipExists(userA, userB) {
        return userA == userB ||
                (exists(/databases/$(database)/documents/friendships/$(userA + '-' + userB))) ||
               (exists(/databases/$(database)/documents/friendships/$(userB + '-' + userA)));
      }

// Second version:
      function friendshipExists(userA, userB) {
        return userA == userB ||
                (exists(/databases/$(database)/documents/friendships/$(userB + '-' + userA))) ||
               (exists(/databases/$(database)/documents/friendships/$(userA + '-' + userB)));
      }

I am only 90% sure about this. Possible reasons why it could be wrong:
- If some commented-out stuff was secretly in action, which I rate highly unlikely.
- If somehow I was testing against the wrong version of the pushed rules. I did switch between these two and observe them going back and forth between (add allowed) and (add not allowed), but who knows.
- I could of course have screwed something up.

Bartholomew

--
You received this message because you are subscribed to a topic in the Google Groups "Firebase Google Group" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/firebase-talk/EWevDQpg9NE/unsubscribe.
To unsubscribe from this group and all its topics, send an email to firebase-tal...@googlegroups.com.
To post to this group, send email to fireba...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/firebase-talk/b64e6061-12e6-49c8-972d-29eb5738b52b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Bartholomew Furrow

unread,
Apr 17, 2018, 12:53:16 PM4/17/18
to Firebase Google Group
Further confirmed by:
- Switching userA and userB by changing whom I logged in as in my app.
- Changing just the 'A' and 'B' characters to swap the order of the rule in case a special character snuck in somewhere.

It's consistent that exists(X) || exists(Y) returns true only if X exists. 

Bartholomew Furrow

unread,
Apr 17, 2018, 1:54:15 PM4/17/18
to Firebase Google Group
OK, for completeness I'm including a simple repro here. I've filed bug 2-7678000021717, and won't comment further on the mailing list unless a team member wants to have the discussion here.

Cheers,
Bartholomew

1. Set these as your Firestore rules:
service cloud.firestore {
  match /databases/{database}/documents {
    match /bar/{id} {
      allow read, write: if exists(/databases/$(database)/documents/test/a) || exists(/databases/$(database)/documents/test/b);
    }
    match /foo/{id} {
      allow read, write: if exists(/databases/$(database)/documents/test/b) || exists(/databases/$(database)/documents/test/a);
    }
  }
}

2. In the console, create /test/a with anything in it.

3. Take the iOS quickstart FriendlyEats, changing these two functions in RestaurantsTableViewController.swift as follows:
  @IBAction func didTapClearButton(_ sender: Any) {
    Firestore.firestore().document("/foo/a").setData(["a": "b"])
  }

  @IBAction func didTapFilterButton(_ sender: Any) {
    Firestore.firestore().document("/bar/a").setData(["a": "b"])
  }

4. Tap clear and filter.

5. Observe that "bar/a" was created, and "foo/a" was not, and gives a "Missing or insufficient permissions" error.

Bartholomew Furrow

unread,
Apr 17, 2018, 10:40:56 PM4/17/18
to Firebase Google Group
Update: I was wrong. 90% was clearly too high, and I screwed up on my repro (I was, in a blunder on the same scale as going up against a Sicilian when death is on the line, pressing the wrong button to call didTapClearButton. Spoiler: it did not say "clear"). In my defense:

1. The documentation for Firestore security rules doesn't mention that there's a limit to how many times you can call exists() and get(). You have to go to the documentation for quotas and limits for that.

2. I'm not even exceeding the limits described in that document: in my app that's experiencing problems, if the first clause of friendshipExists doesn't return, I end up with 1 get() and 3 exists() calls. Nevertheless, exists() #3 seems to cause an error.

3. I have spent the last two years building up an immunity to iocaine powder.

As a bonus for wasting your time, here's an iOS code snippet for figuring out whether your security rules are updated:
    for i in 0...10 {
      Firestore.firestore().document("/versiontest/\(i)").getDocument() {
        if $1 == nil { NSLog("Rule version \(i).") }
      }
    }

And the corresponding security rule, which should be changed whenever you update:
    match /versiontest/{versionId} {
      allow read: if versionId == "3"
    }

I probably should have made something like that for myself earlier!
Bartholomew

Samuel Stern

unread,
Apr 18, 2018, 1:06:35 PM4/18/18
to fireba...@googlegroups.com
Hi Bartholomew,

Thank you for having this conversation with yourself on the mailing list!  I learned a lot by reading it, and you have certainly not wasted anyone's time.  All of the issues you had reflect improvements we need to make on the Rules developer experience.  And we're definitely working on it!

As for the limits on get and exists: we are working on raising those so that you're much, much less likely to have to care about them.  Your feedback is one more reason we think that's a good idea.

- Sam

Security rules keeps using that word "deployed".  I do not think it means what it thinks it means.

You received this message because you are subscribed to the Google Groups "Firebase Google Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to firebase-tal...@googlegroups.com.

To post to this group, send email to fireba...@googlegroups.com.

Bartholomew Furrow

unread,
Apr 18, 2018, 2:25:11 PM4/18/18
to fireba...@googlegroups.com
Thanks for the response, Sam! I'm glad those limits are headed up. They feel a little tight as-is, whether they're working as described or not!

I can understand why it's hard to develop user-friendly tools for these security rules. They're a lot more powerful than the RTDB ones, and correspondingly potentially more complex.

Bartholomew

Reply all
Reply to author
Forward
0 new messages