Security Rules

[rwieruch]

We have deployed security rules for the Firebase Realtime Database version over here: the-road-to-react-with-firebase/react-firebase-authentication#26 Has anyone recommendations how these rules would look like for Firestore?

[iryhor]

service cloud.firestore {
match /databases/{database}/documents {
// Custom functions
function signedIn() {
return request.auth != null;
}

function isAdmin() {
		return signedIn() &&         
    	'ADMIN'in get(/databases/$(database)/documents/users/$(request.auth.uid)).data.roles.values();
}

function ownsMessage() {
	return signedIn() && request.auth.uid == resource.data.userId;
}

function isSelf() {
	return signedIn() && request.auth.uid == resource.id;
}

// Rules
match /users/{userId} {
	allow get: if isSelf() || isAdmin();
  allow list: if isAdmin();
  allow write:  if isSelf() || isAdmin();
}

  
match /messages/{messageId} {
  allow read: if signedIn();
  allow create: if signedIn();
  allow update, delete: if signedIn() && ownsMessage();
}
}
}

[iryhor]

Apologies for being a hot mess right now trying to respond with the code. I tested these with the Simulator in the firebase console and I'm fairly confident they fully recapitulate to the realtime database set.

[rwieruch]

No worries @iryhor Maybe you want to check out GitHub's markdown rules: https://guides.github.com/features/mastering-markdown/ There you see how to do code blocks. Would be great to see your rule set as code here! Perfect that it works for you 👍

EDIT: I edited your comment. Only the indentation is missing!

I pointed our Slack Community https://slack-the-road-to-learn-react.wieruch.com/ to your solution. I think this will help lots of people :)

[azizakgul]

One addition to @iryhor 's code can be to check if the user creating the message is creating with his own id:

match /messages/{messageId} {
  allow read: if signedIn();
  allow create: if signedIn() && request.resource.data.userId == request.auth.uid ;
  allow update, delete: if signedIn() && ownsMessage();
}

[PrivatePuffin]

One slight issue with this code:
Not every implementation works with this. In some cases (for example the REST API), this leads to a 403 on create.
This is due to resource.data point towards the existing document which (obviously) does not exist yet.

Beter solution would be:

// Rules
match /users/{userId} {
        allow list: if isAdmin();
    	allow get, update, delete: if isSelf() || isAdmin();
    	allow create: if signedIn();
    }

This ensures compatibility with other API's and expansion, without compromising too much security. I am in doubt if it's needed to also add the code @azizakgul used in the messageId rule though.

[azizakgul]

One slight issue with this code:
Not every implementation works with this. In some cases (for example the REST API), this leads to a 403 on create.
This is due to resource.data point towards the existing document which (obviously) does not exist yet.

Beter solution would be:

// Rules
match /users/{userId} {
        allow list: if isAdmin();
    	allow get, update, delete: if isSelf() || isAdmin();
    	allow create: if signedIn();
    }

This ensures compatibility with other API's and expansion, without compromising too much security. I am in doubt if it's needed to also add the code @azizakgul used in the messageId rule though.

@Ornias1993 I think adding the following rule is necessary, otherwise a signedIn user can create a message as another user.

allow create: if signedIn() && request.resource.data.userId == request.auth.uid

[PrivatePuffin]

Only for the messages then. Got it!

What do you think of my change to the {userId} rules?

[azizakgul]

Only for the messages then. Got it!

What do you think of my change to the {userId} rules?

@Ornias1993 You're correct that checking if a user is Admin before there's user data will return null. So you can separate "create" rule to just "signedIn" like you have suggested.

On the other hand if for some reason the developer wants Admin to be able to create user data for other users, then isAdmin() can check for "null" in requesting user data before checking if "Admin" is included in user roles.

[PrivatePuffin]

@azizakgul
Good point I totally forgot to add isAdmin() to the create rule. Ofc an admin should be able to create users!

But the point is more that doing this:
allow create: if isSelf();

Freaks out at times, because (i Guess):
request.auth.uid == resource.id;
Leads to comparing the auth.uid with null, before creation.

anyhow adding admin into the mix:

// Rules
match /users/{userId} {
        allow list: if isAdmin();
    	allow get, update, delete: if isSelf() || isAdmin();
    	allow create: if signedIn();
    }

[jonathan-irvin]

👍 Getting the same issues, trying with new rules.