CODEWITHSUNDEEP
firebasegoogle-cloud-firestorefirebase-security
dsl101
dsl101

Reputation: 1784

Firestore rules allow access to docs, but not via collection query

Moving from firebase to firestore, and hoping this is just something I haven't understood about firestore rules. I have a collection of docs, and a user record which stores which docs that user is allowed to read. My docs look something like this:

/eventDetails/abc123
  name: "Event name"
  description: "Event description"
  id: "abc123"

and my rules look like this:

    match /eventDetails/{eventId} {
        allow read: if isEventMember(eventId);
    }

    function isEventMember(eventId) {
      return eventId in get(/databases/$(database)/documents/users/$(request.auth.uid)).data.events.keys();
    }

In code, I can access individual docs in /eventDetails by id, but accessing them via a collection query fails with insufficient permissions:

  for (const eventId of eventIds) {
    await fsFirestore().collection('eventDetails').doc(eventId).get()
      .then(snap => { console.log('got doc snap:', snap)})
      .catch(err => { console.log('caught error'); console.error(err) })
  }
  await fsFirestore().collection('eventDetails').where('id', 'in', eventIds).get()
    .then(snap => { console.log('got collection snap:', snap)})
    .catch(err => { console.log('caught error'); console.error(err) })

The for() loop all works fine (assuming I pass in a list of eventIds that the user should be able to access), and I also see an error there if I pass in an eventId that they shouldn't be able to access. All correct so far, and so I'm pretty sure the isEventMember() function in the rules is doing what it should.

But accessing the very same docs via the collection query (all docs have an id property with matching value) fails with Missing or insufficient permissions. This seems counter to the example on this page.

Is it possible to get a collection of documents using a query in this way, whilst still preventing a user from access other documents?

Upvotes: 1

Views: 927

Answers (2)

dsl101
dsl101

Reputation: 1784

Reading this section of the documentation, I was convinced what I was trying to do was possible. And then this sentence gave me a clue:

In contrast, the following query succeeds, because it includes the same constraint on the author field as the security rules:

Since my query restricts the collection result to documents in eventDetails where the id field of the document is in the set of events the user is allowed to access, I changed my security rules function to:

function isEventMember() {
  return resource.data.id in get(/databases/$(database)/documents/users/$(request.auth.uid)).data.events.keys();
}

By accessing the event id as resource.data.id instead of the path matching wildcard {eventId}, the rules appear to work exactly as I would expect. It's still not ideal because if, for some reason, the id field in an event is changed, corrupted, or otherwise set incorrectly, it could allow access to a document the user shouldn't have access to. For example, if the user's events field was set so they were only allowed to access doc abc123, but the data was set like this:

/eventDetails/abc123
{
  name: "Accessible event",
  id: "abc123"
},

/eventDetails/secret234
{
  name: "Private event",
  id: "abc123"
}

Then the user would be able to access /eventDetails/secret234. But since the only place in my code where the id field is set is in a firebase function triggered on event creation, I can be pretty confident this wouldn't happen.

Upvotes: 1

Doug Stevenson
Doug Stevenson

Reputation: 317487

The problem is that Firebase security rules are not filters. Please read that documentation. Rules will not take the results of a query and remove items that would not satisfy the rule. Queries are all-or-nothing. If anything in the results set might not pass a rule, the entire query is rejected.

What you should do instead is simply get() each event ID separately by iterating eventIds, instead of using an "in" query. This allows security rules to know exactly which pair of event ID and UID to check for each query so it can pass or fail each query individually. I know it seems like a hassle, but this is required because of the way rules work.

Upvotes: 3

Related Questions