Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tighten Firestore security rules #276

Open
tamsingreen opened this issue Jun 7, 2021 · 2 comments
Open

Tighten Firestore security rules #276

tamsingreen opened this issue Jun 7, 2021 · 2 comments

Comments

@tamsingreen
Copy link
Contributor

As alerted by Pietro, in firestore,rules:
match /projects/{document=**} {
allow read, write
}
Allows any authenticated user to read/write to a project.

We need to lock this down to admin users only and check where this rule is in use in the app & rewrite where this would prevent legitimate access.

@pietrop
Copy link
Contributor

pietrop commented Jul 7, 2021

Describe the bug

I think on your end the data is is also organized as follows(?):
120530971-16d85680-c3ac-11eb-9213-cebffaca15f9

  • projects collection where the project document contains the transcripts collection.
  • the project document also contains a roles attributes as a hash map where the key is the user email and the value is the role type : owner, write, read.

At the moment client side we query the /projects collection and only display to the users the once they are associated with.

by using list for projects In the current security rules and locking down further other operations and nested routes.

In some situations, it's useful to break down read and write into more granular operations. For example, your app may want to enforce different conditions on document creation than on document deletion. Or you may want to allow single document reads but deny large queries.

A read rule can be broken into get and list, while a write rule can be broken into create, update, and delete

From firestore docs Granular operations

This is because a collection, especially at the top level, cannot have tighter security without disallowing the list operation.

All match statements should point to documents, not collections.

From firestore Basic read/write rules

The problem is that when we list a project, "anyone" can theoretically see the project title, description but most importantly the emails of the users associated with that project and their corresponding roles.

Expected behavior

Ideally we'd want to be able to lock down the project list or restructure the data to be more secure.

Additional context

looking at the firestore doc Securely query data & Secure data access for users and groups there's another pattern, we can consider where the /projects/ collection could be locked down, and a /users/{userId} collection/document could contain the info to match users to projects.

@emettely
Copy link
Contributor

emettely commented Jul 8, 2021

We did need the read write initially for users to be able to create the projects collection.
The reason for this is because of this flow (see https://github.com/bbc/digital-paper-edit-firebase/blob/master/firestore.rules) also:
Users login -> users authenticated using the users collection.
Users need to be allowed to read projects collection to check if their project exists & write if it doesn't the user needs to create a project.
... etc.

      match /projects/{document=**} {    
	      allow read, write
      }
  match /projects/{pId} {    
        allow read, write: if isOnProject(pId) || isAdmin();        
      }

      match /projects/{pId}/{document=**} {
        allow read, write: if isOnProject(pId) || isAdmin();        
			} 
  	}
}
}

The get / create operation seem like a good evolution / only using create rather than an entire read and write :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants