2

So I currently have two roles for all users: isAdmin and isReader.

An Admin is allowed to read and write data and an Reader is allowed to read data.

When someone creates an account he has no rights. Not even isReader. Only an Admin can change rules.

This is how I planned to do it:

Once someone creates an account I create an Document in the Users Collection like this:

uid: user.uid,
email: user.email,
role: {
    isAdmin: false,
    isReader: false,
}

On each login I update 'email' and uid but keep role untouched. To secure this behaviour I have these rules:

match /Users/{userId} {
  allow read: if isOwner(userId) || isAdmin();
  allow create: if request.resource.data.hasAll(['uid', 'email', 'role']) && request.resource.data.role.isAdmin == false && request.resource.data.role.isReader == false;
  allow update: if resource.data.role == null || isAdmin();
}

function isAdmin() {
  return getUserData().role.isAdmin == true;
}

I think I have 2 errors:

  1. for some reason the data.hasAll(['uid', 'email', 'role']) does not work. When I remove this part the create rule works as planned.

  2. resource.data.role == null does not work. I intend to check if the data contains any updates for role because I can't allow it is it doesn't come from an Admin. But for some reason it does not work.

Any Ideas what I'm doing wrong? Also is my strategy save or is there a way someone could "hack" himself Reader or Admin rights?

Doug Stevenson
  • 297,357
  • 32
  • 422
  • 441
Jonas
  • 7,089
  • 15
  • 49
  • 110

2 Answers2

3

This looks like it may be a good use case for custom auth claims. You can set specific roles on a user within a secured environment, as shown in this codelab. Below is an example of setting a custom claim in your server. You can even use Cloud Functions for this. I recommend you check out the full code of the Codelab so you can see how to ensure not just anyone can request custom claims be added to their user.

admin.auth().setCustomUserClaims(uid, {Admin: true}).then(() => {
// The new custom claims will propagate to the user's ID token the
// next time a new one is issued.
});

Then you can check for those roles on the user in your security rules.

service cloud.firestore {
  match /databases/{database}/documents {
    match /Users/{userId} {
      allow read: if request.auth.token.Owner == true || request.auth.token.Admin == true;
      allow create: request.auth.uid == userId && 
      request.resource.data.uid == request.auth.uid &&
      request.resource.data.email != null;
      allow update: request.auth.uid == userId || request.auth.token.Admin == true;
    }
  }
}

Notice all the rules about "role" have been removed because they're no longer needed. Let me know if you have questions about implementation because I'm trying to make some more content around this since it's such a common problem.

Jen Person
  • 7,356
  • 22
  • 30
  • Thank you Jen! I will check that out. I haven't really done anything with cloud functions yet and neither with admin sdk but I will experiment with it when I have setup my basic features. – Jonas Sep 10 '18 at 20:19
  • This is a great solution. It reduces lookups in rules since the roles are sourced from the Auth Token (i.e. `getUserData()` is not needed), and it allows roles to be used across services (RTDB, Firestore, Storage, etc.). – Bryan Massoth Sep 11 '18 at 00:13
1

request.resource.data.hasAll(['uid', 'email', 'role']) does not work, because request.resource.data is a Map and not a List. You should use keys() to create a List from the Map and ensure certain keys exist.

In regards to your second issue, you should just check whether there is a write to roles: allow update: if !('roles' in request.writeFields) || isAdmin();. This will ensure that any updates to roles will fail unless a user is an Admin.

About your security question; I see a couple issues. The first is anyone can create unlimited users which also means that any Admin can create unlimited other Admin accounts. To stop this from happening, I would add another section to the allow create that restricts creation to the user:

allow create: if userId == request.resource.data.uid
  && request.auth.uid == request.resource.data.uid
  && request.resource.data.hasAll(['uid', 'email', 'role'])
  && request.resource.data.role.isAdmin == false
  && request.resource.data.role.isReader == false;`

The second is anyone can change their uid and try to impersonate someone else. Obviously this doesn't change the uid associated to their Auth Token, but depending on how you write the rest of your rules, the backend security, or even the frontend display, someone could use that flaw to exploit your code or another user (potentially an Admin). You can ensure no one changes their uid by checking whether it is in the writeFields (you will also need the previous security solution to also ensure they don't impersonate during creation).

allow update: if !('uid' in request.writeFields)
  && (!('roles' in request.writeFields) || isAdmin());

I hope this helps.

Bryan Massoth
  • 1,109
  • 6
  • 7
  • Thanks for the help! Adding `keys()`solved the first problem for me but `if !('role' in request.writeFields);` doesn't seem to work for me. It allows to edit the `role` field. I also tried `if !('role' in request.resource.writeFields);` but this throws an error everytime (so also when I update the user data without the role field). – Jonas Sep 10 '18 at 19:59
  • For the security question: Generally I think its fine when I allow any admin to promote unlimited others to admin because the program is for a business where I can trust the admins. I'm not sure if I understand your 1st suggestion 100% but I think the additional rules won't forbid anything I need particularly so I'will just add them... – Jonas Sep 10 '18 at 20:12
  • Also I'm not sure if the 2nd rule applies to me. As you said the uid of the AuthToken is still the same and even if they have admin rights at "client side" the firestore rules should secure the rights with the auth uid. I'm currently updating the uid and the email of the userdata on every login. This won't work with the 2. suggestion. However I don't think I need to update uid and email on every login so I will test your suggestion too. – Jonas Sep 10 '18 at 20:13
  • 1
    @Jonas I see why it is not working for you. It is checking for exactly 'role' instead of 'role', 'role.isAdmin', and 'role.isReader'. Try `!request.writeFields.hasAny(['role','role.isAdmin','role.isReader'])`. Regarding your second comment, not only can admins (and anyone else; **including people who are not signed in**) promote anyone, but they can create fake users at will. However, only admins can then give them admin privileges which can then be used to skirt around any action logging system you might have (i.e. a way to blame an admin for abusive behavior). – Bryan Massoth Sep 11 '18 at 00:05