[Firestore] Security rules return permission denied when reading immediately after writing new document

[SUPERCILEX]

[REQUIRED] Step 2: Describe your environment

• Firebase Component: Firestore
• Component version: 17.1.5

[REQUIRED] Step 3: Describe the problem

Suppose I'm writing this document:

field1: true
...
owners:
  uid1: 5

With these security rules:

match /teams/{teamId} {
    allow get: if resource.data.owners[request.auth.uid] is int;
    allow write: true;
}

And this code:

ref.set(obj)
ref.get() // This fails!

The get request will fail with a PERMISSION_DENIED error. But it works fine if I wait a few seconds.

[mikelehen]

@SUPERCILEX Can you try waiting for the set to complete before calling get()? E.g.: ref.set(obj).addOnSuccessListener(() -> { ref.get(); ... }

What may be happening is that the get() is reaching the server before the set() has completed and so the document doesn't exist yet, and your rules don't permit reading nonexistent documents, so the get() fails. Once the set() completes, then you can read it.

[SUPERCILEX]

@mikelehen I'm sure that would work, but that callback won't be triggered if I'm offline.

1. Is there an offline solution?
2. Is there a way for the Firestore SDK to keep the order of requests for specific paths? So x/y and y/x reads and writes can be in any order, but x/y and x/y reads and writes must happen in the order called by the developer. Or if it's not an issue with the SDK, the server should handle this correctly.
3. Can security rules let a read go through even though the document doesn't exist?

[mikelehen]

1. Can you explain your high-level use case? That might help me come up with an offline solution for you. One option might be to do ref.get(Source.CACHE) if you know that you just called .set() and just want to read the data back.

2. Unfortunately reads and writes are completely independent on the backend (but all writes will happen in order and all reads will happen in order). So to do this, we'd have to do it in the client and I think it would be problematic to implement and would cause undesirable latency in many cases.

3. Sure. Just do allow get: if resource == null || ...

Sorry I don't have a better answer.

[samtstern]

This made me see a bug in our docs:
https://firebase.google.com/docs/reference/rules/rules.firestore

It says that resource is a non-null field but it's null if the resource does not exist. Will update.

[SUPERCILEX]

@samtstern Yeah, I was confused by that. Thanks! ❤️

@mikelehen

1. I create a doc while a snapshot listener is active. In said listener, I periodically fetch some data from the network to update the doc. However, since the current device might have gone offline while another device deleted the doc, I do a GET request to make sure the doc isn't trashed or completely deleted (!exists()). That means this is the flow: write -> snapshot update -> read. Does that make sense?
2. Yeah, I thought that might be an issue. 😢
3. Whoa, so here's something super interesting! If I add resource == null || ... to my security rules, the GET succeeds with the full data. That means the resource should not be null. So there's your bug right there. 😁

[mikelehen]

@SUPERCILEX Thanks! Your use case mostly makes sense to me, but I have a couple thoughts:

1. If you're actually guarding against the doc being deleted or trashed, you should be doing the read / update inside a transaction, else your code is still race-y and you might as well forego the "GET request to make sure the doc isn't trashed or completed deleted" and just use the data from the snapshot instead.
2. If you can tolerate the updates only happening while you're online, perhaps you could just make your listener ignore documents with snapshot.getMetadata().hasPendingWrites() == true ? That way you don't try to read/modify the document until it's been committed to the server. (NOTE: You likely will want to pass MetadataChanges.INCLUDE to your addSnapshotListener call)

Can you elaborate on 3? I'm not quite clear on what happened and what you were expecting. Are you saying you believe there's a bug with security rules or something?

[SUPERCILEX]

@mikelehen

1. That's actually a pretty darn good point. 👍 I'll look into using transactions for my background updates.
2. No, the snapshot listener must work offline. It's the background work that can assume it's online.
3. Yes, let me try to be clearer. Here's what I'm seeing:
   1. I make the write -> read request.
   2. It fails in the security rules because resource is null.
   3. BUT! If I set the security rules to resource == null || realCondition(), the request succeeds and returns the full data. I would expect it to return a "document doesn't exist" error, right? Because the security rule succeeds only because I added resource == null to a read rule, thus implying the document doesn't exist. So why on the client am I seeing the document's data instead of a "doesn't exist" error? Does that make sense?

[samtstern]

Now that you mention it ... resource == null || in a read rule is pretty nonsensical. It can never be statically verified from the query constraints, so it only applies to single-document reads. And then what does it even mean to be allowed to read a document that doesn't exist?

[mikelehen]

@SUPERCILEX

1. Just to be clear, using a transaction is a good idea but probably won't help any with this permission denied error. :-)
2. I don't fully understand the scenario, but my thinking was that maybe you could process the snapshots as normal but only kick off the background work for snapshots with hasPendingWrites==false.
3. Oh! I see. Heh. So this happens because of "latency compensation" in the SDK. We always overlay your local writes on top of what the server sends back, and so in this case since the write is still outstanding, we overlay it on top of the nonexistent document we get from the server.

@samtstern Being able to read a nonexistent document means your read will succeed with a snapshot with .exists() returning false rather than failing with a permission denied error. I think this is probably useful in some cases, e.g. so you can display a "This xyz doesn't exist or was deleted" message vs a "You don't have access to this xyz." or whatever.

[SUPERCILEX]

@mikelehen

1. 👍
2. Well the other issue is that I do want the job to be scheduled even if the device is currently offline. Thanks for the options though! 😊
3. @mikelehen @samtstern Ohhhhhh, a "doesn't exist" error instead of the "perm denied" one is exactly what I want! Guess I'll add resource == null to my rules.