IMPORTANT: Deploy Security Rules

[rwieruch]

Ref.: #23 CC @edguerrade

The PR addresses the main issue of having users full control of reading and writing from/to the database. The following security rules that you can set up on your Firebase Dashboard should work:

{
  "rules": {
    "users": {
      "$uid": {
        ".read": "$uid === auth.uid || root.child('users/'+auth.uid).child('roles').hasChildren(['ADMIN'])",
        ".write": "$uid === auth.uid || root.child('users/'+auth.uid).child('roles').hasChildren(['ADMIN'])"
      },
      ".read": "root.child('users/'+auth.uid).child('roles').hasChildren(['ADMIN'])",
      ".write": "root.child('users/'+auth.uid).child('roles').hasChildren(['ADMIN'])"
    },
    "messages": {
      ".indexOn": ["createdAt"],
      "$uid": {
        ".write": "data.exists() ? data.child('userId').val() === auth.uid : newData.child('userId').val() === auth.uid"
      },
      ".read": "auth != null",
    },
  }
}

• only admin users can read/write to ALL and INDIVIDUAL users
• a user can only read/write its OWN user data
• messages can be read only by authenticated users
• a message can only be created and updated by a message owner

Basically that's what the first commit (2a895f9) of this PR is doing, because we need a roles object instead of array in the database to apply the shown rules. With this change comes the restriction that users cannot fetch all users on the home page anymore to associate them with messages (52d24d0). The third commit gives only message owners the ability to edit/delete messages in the UI, otherwise there would be an error due to the new security rules (8cb594e).

Let me know your thoughts!

If anyone wants to help me out with this security issue, all of the changes need to be applied to the other Firebase repositories as well:

• https://github.com/the-road-to-react-with-firebase/react-semantic-ui-firebase-authentication
• https://github.com/the-road-to-react-with-firebase/react-gatsby-firebase-authentication
• https://github.com/the-road-to-react-with-firebase/react-firestore-authentication
• https://github.com/the-road-to-react-with-firebase/react-mobx-firebase-authentication
• https://github.com/the-road-to-react-with-firebase/react-redux-firebase-authentication

Note for myself:

• update book
• update blog posts
• write another blog post/chapter about security rules

[edguerrade]

I think that's a good start for now. I would add rules to prevent creating unwanted entities (read/write not allowed by default) so nobody can create "unwantedmessage" doc on our db. Forcing us to check all permissions on all new entities preventing any security lack.

{
  "rules": {
    ".read": false,
    ".write": false,
    "users": {
      "$uid": {
        ".read": "$uid === auth.uid || root.child('users/'+auth.uid).child('roles').hasChildren(['ADMIN'])",
        ".write": "$uid === auth.uid || root.child('users/'+auth.uid).child('roles').hasChildren(['ADMIN'])"
      },
      ".read": "root.child('users/'+auth.uid).child('roles').hasChildren(['ADMIN'])",
      ".write": "root.child('users/'+auth.uid).child('roles').hasChildren(['ADMIN'])"
    },
    "messages": {
      ".indexOn": ["createdAt"],
      "$uid": {
        ".write": "data.exists() ? data.child('userId').val() === auth.uid : newData.child('userId').val() === auth.uid"
      },
      ".read": "auth != null",
    },
  }
}

[rwieruch]

You are right @edguerrade I extended the rule set a bit more, because otherwise users wouldn't be able to read messages anymore (line 18), because of the cascading nature of the rule set. Let me know what you think about it:

{
  "rules": {
    ".read": false,
    ".write": false,
    "users": {
      "$uid": {
        ".read": "$uid === auth.uid || root.child('users/'+auth.uid).child('roles').hasChildren(['ADMIN'])",
        ".write": "$uid === auth.uid || root.child('users/'+auth.uid).child('roles').hasChildren(['ADMIN'])"
      },
      ".read": "root.child('users/'+auth.uid).child('roles').hasChildren(['ADMIN'])",
      ".write": "root.child('users/'+auth.uid).child('roles').hasChildren(['ADMIN'])"
    },
    "messages": {
      ".indexOn": ["createdAt"],
      "$uid": {
        ".write": "data.exists() ? data.child('userId').val() === auth.uid : newData.child('userId').val() === auth.uid"
      },
      ".read": "auth != null",
      ".write": "auth != null",
    },
  }
}

Reading on message/[id] could be made more explicit as well. But if no rule for read is specified there, the read rule for messages should take care of it. Maybe keeping it this way shows the cascade effect again.

Do you think that's fine then? I went through the simulator once to check whether everything is alright, but would be great if someone else would try it as well :)

[Param-Harrison]

Hey @rwieruch
I tried these rules, the only problem I saw is,

• Message names aren't displaying, instead, it shows the uid.

[rwieruch]

@Param-Harrison are you sure your problem is related to the security rules?

[Param-Harrison]

@rwieruch I will test it again and tweak a bit to check it.
But yes, when I tried in the morning, everything works fine only message name disappear and show uid. When I revert back, it shows the name.

[Param-Harrison]

I am able to reproduce the issue. The message details got vanished when updating these rules.

With Rules

Without Rules

[Param-Harrison]

so basically this line in MessageItem.js only gets the userId not other info in the user object.

{message.user.username || message.user.userId}

[rwieruch]

Ah correct, that's because a user isn't able anymore to retrieve all users from the database to associate them with the messages. The security rules forbid it. That's why I removed this features from the code as well. Check the PR again to see the changes.

If you want to get the associated users to a message, I would recommend to save minimal user information to the message entity (e.g. message owner's name). As Alternative, you could also query every single user associated to a message. Since this could become a bottleneck eventually, you may want to deduplicate doubled users to minimize the queries (e.g. two messages are written by one user id, fetch this user only once from Firebase to associate the user with the message).

[Param-Harrison]

Got it 👍

[rwieruch]

Merged! And merged changes into the other projects as well.

In case of the Firestore project, any help is super appreciated to convert the security rules over to their syntax: the-road-to-react-with-firebase/react-firestore-authentication#10