firebase / firebase-tools

The Firebase Command Line Tools
MIT License
4.01k stars 925 forks source link

resource.data is missing data when evaluating firestore rules in emulator #4420

Open evelant opened 2 years ago

evelant commented 2 years ago

[REQUIRED] Environment info

firebase-tools: 10.6.0

Platform: macOS

[REQUIRED] Test case

The following function in security rule function will return true even when the document in question definitely contains the userId field

function doesntExistOrMissingUserId(){
      return resource == null || !resource.data.keys().hasAll(['userId']);
}

[REQUIRED] Steps to reproduce

Add the above function to a read rule. Issue a query from a client for a doc that contains a userId field. The rule will pass when it should be rejected.

[REQUIRED] Expected behavior

The rule should be rejected

[REQUIRED] Actual behavior

The rule evaluates to true

joehan commented 2 years ago

Hey @evelant - could you provide some sample documents where this rule misbehaves? We have a good guess as to what is causing this, but having a concrete example would be very helpful

mcapodici commented 2 years ago

I just had the same issue.

Using local emulator.

Looking in the emulator portal, the item has 4 fields, but in the request trace it shows Detailed Information -> Resource with only a single field.

This is on a read operation, and I was subscribing using a query snapshot.

bkendall commented 2 years ago

@mcapodici (or @evelant) Could either of you help us out and give us a minimal reproducible example for this issue? We're still needing a little help to debug exactly what's happening and a specific example will help a lot

google-oss-bot commented 2 years ago

Hey @evelant. We need more information to resolve this issue but there hasn't been an update in 7 weekdays. I'm marking the issue as stale and if there are no new updates in the next 3 days I will close it automatically.

If you have more information that will help us get to the bottom of this, just add a comment!

google-oss-bot commented 2 years ago

Since there haven't been any recent updates here, I am going to close this issue.

@evelant if you're still experiencing this problem and want to continue the discussion just leave a comment here and we are happy to re-open this.

mcapodici commented 2 years ago

What I have found is that the resource data seems to match the query, not the resource, at least as shown in http://localhost:4000/firestore/requests/ view, as I was trying a hack to get around this:

image

mcapodici commented 2 years ago

It is a pain to write minimal reproducible steps as I would need to develop a complete app just do to this. It seems to happen a lot - when doing a query over all documents on a where clause. It is probably not hard for you to find out why based on the information especially that that constraint ended up in the resource.

Here is my example that lead to the query above.

  const q = query(collection(firestore, "users") as CollectionReference<UserData>, 
    where("email", "==", email.toLowerCase()),
google-oss-bot commented 2 years ago

Since there haven't been any recent updates here, I am going to close this issue.

@evelant if you're still experiencing this problem and want to continue the discussion just leave a comment here and we are happy to re-open this.

christhompsongoogle commented 2 years ago

Could be related : https://github.com/firebase/firebase-js-sdk/issues/5229

Includes a possible workaround

Google internal tracking bug: b/129293104

mcapodici commented 2 years ago

I suspect that other issue is the same thing basically.

The workaround mentioned there is:

if your security rule checks for admin == uid you need to include .where("admin", "==", uid) even though all your documents may have admin == uid

Or more generally, include everything your rule checks for in a where clause.

Yes this is my experience too. It means you might need 'dummy' clauses, but this is good enough workaround to unblock me I think. I will post back if I find a situation where this doesn't work.

All this kind of makes me think: unless it is a super simple security rule, then use a function as the gatekeeper rather than rules.

Steven-Douglass commented 1 year ago

I have experienced a similar issue with the Cloud Firestore Emulator where the request.resource.data property is not 'hydrated' as I thought it would be with a doc().set() request on an existing document.


Expected: The request.resource will be 'The new resource value, present on write requests only.'   Actual: The request.resource only contains the fields that are being updated, not all fields of the document.   I've reproduced this using the a Mocha test with the Cloud Firestore Emulator running locally. I am building an appointment booking application with a rule that after an appointment has been created, the customer cannot be changed. This is represented with the following rule:   match /appointments/{appointmentId} {   allow update: if request.auth != null &&     resource.data.user.uid == request.resource.data.user.uid; }   This is the test I've written to reproduce this issue:       it.only('Appointments can be updated if the user does not change', async () => {         // Data Setup         const user_abc = { uid: 'user_abc', email: 'user_abc@gmail.com' }         // Create the document         const admin = firebase.initializeAdminApp({ projectId: MY_PROJECT_ID }).firestore();         const testDoc = admin.collection('appointments').doc(randomUid);         await testDoc.set({ user: { uid: user_abc.uid }, serviceProvider: { uid: owner_abc.uid } });            // Update the document         const db = getFirestore(user_abc);         const testDoc2 = db.collection('appointments').doc(randomUid);         // await firebase.assertSucceeds(testDoc2.set({ startTime: 'Feb 22, 2023', })); // This actually fails         await firebase.assertSucceeds(testDoc2.update({ startTime: 'Feb 22, 2023', })); // This succeeds     });   What I have found is that calling doc().update() results in a properly 'hydrated' request.resource.data object that contains all the fields of the document.   However, if  I use doc().set() on an existing document which subsequently makes a doc().update() call, the request.resource.data object only contains the fields being updated. This causes the security rule to produce an error because the request.resource.data does not contain the expected user property:

FirebaseError: 7 PERMISSION_DENIED: Property user is undefined on object. for 'update' @ L69   Results of unit test: If I uncomment the testDoc2.set() line in the above test, it results in a create request which fails because the document exists. Then an update request occurs, but the update() request contains the same request.resource object as the create()  request.

image image

Now contrast that with the testDoc2.update() call which has the 'hydrated' request.resource object as expected:

image image

Hope this helps. Steve

christhompsongoogle commented 1 year ago

Hi Steven-Douglass, thank you for the detailed repro this helps narrow down the issue a lot. I took a good look at your example with some colleagues and I think this is WAI; let me tell you why:

testDoc2.set({ startTime: 'Feb 22, 2023', }) without the merge option actually overwrites the data in testDoc2. Afterwards, when the rules evaluation checks resource.data.user.uid == request.resource.data.user.uid the request.resource.data.user portion doesn't exist and thus the check fails. Can you try with the merge option and let me know if you still are encountering the issue?

https://firebase.google.com/docs/firestore/manage-data/add-data#set_a_document

adriank commented 7 months ago

Actual data in collection:

{
        inviterId: TEAM_OWNER_ID,
        teamId: TEAM_A_ID,
        inviteeId: USER_NOT_IN_MY_TEAM_ID,
 }

Test case:

    it('Inviter can read their invitations to a team', async () => {
      const db = getFirestore( { uid: 'TEAM_OWNER_ID' });
      const testDoc = db.collection('TEAM_INVITATIONS_COLLECTION').where('inviterId', '==', 'TEAM_OWNER_ID');
      await firebase.assertSucceeds(await testDoc.get());
    });

Rules:

match /team_invitations/{invitationId} {
  allow read: if debug(resource.data.inviteeId) == request.auth.uid;
}

Error: Property inviteeId is undefined on object. for 'list' @ L40

Output of debug is correct. It prints 'USER_NOT_IN_MY_TEAM_ID'.

So it fails every time when we check other field than mentioned in query. The correct behavior would be that resource.data has all fields that are visible in "Data" tab in emulator UI.

ASE55471 commented 6 months ago

I got similar issue where when i use resource.data.members == null && request.auth.uid in resource.data.members, And the rule passed which is make no sense.

scrowley-Datirium commented 5 months ago

Running into same issue where the goal is to allow reading of "group" docs if request comes from user that is listed as part of that group.

A "group" collection exists where documents have a map of userId:public_data for the members of that group.

Unless I'm wrong, this kind of implementation is essentially exactly what's suggested here https://firebase.google.com/docs/firestore/solutions/role-based-access

example group doc:

{
  name: "group A",
  ownerID: "user1_ID",
  members: {
    user1_id: { name: "user1 name", otherPublicData: ...},
    user2_id: { name: "user2 name", otherPublicData: ...},
    ...
}

and firestore rules as such

function isGroupMember(req, res) {
   return debug(req.auth != null && req.auth.token.email_verified) && debug(req.auth.uid in debug(res.data.members.keys()));
}
match /groups/{groupId} {
  allow read: if isGroupMember(request, resource);
}

and the query being made is utilizing rxFire in angular like so

import { Firestore, collection, query, orderBy, documentId, where, limit, startAfter } from '@angular/fire/firestore';
import { collection as collect } from 'rxfire/firestore';

// in service
private firestore: Firestore = inject(Firestore);
// ...

// in constructor of service (user object comes from auth service. it works)
const q: any = query(collection(this.firestore, 'groups'), where('ownerID', '==', user.uid));
this.myGroup$ =  collect(q).pipe(
    map((doc: any) => {
        //...
    }),
);

Firestore emulator.requests shows a failure on a query to "groups" where that failed query has a resource value of: data:{ownerID: "some_user_id"}

I've tried accessing resource.data in different ways, but also run into an issue where im unable to use "get()" in order to check that doc