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

Firestore Security #36

Open
limyifan1 opened this issue Apr 29, 2020 · 14 comments
Open

Firestore Security #36

limyifan1 opened this issue Apr 29, 2020 · 14 comments
Labels
help wanted Extra attention is needed

Comments

@limyifan1
Copy link
Owner

Currently, our Firestore security allows anyone to access it... we will appreciate help configuring firestore.rules to step up security but at the same time allow Github developers to run locally on their machine.

@limyifan1 limyifan1 added the help wanted Extra attention is needed label Apr 29, 2020
@magicalbeacon
Copy link
Contributor

magicalbeacon commented May 5, 2020

Without authentication, it'll be difficult to write rules that'll be effective, but what we can do for now to step up security is:

  1. To validate data going into the database by writing validation rules
  2. Removing delete access by setting allow delete: if false, to prevent malicious users from deleting any listing
  3. Prevent duplicate listings from being added #58 This validation can also be done in rules, to prevent duplicates from being added by adding in validation.

What I'll suggest is most important now is to add some form of validation for creation of listings. For the issue of allowing GitHub developers to run locally on their machine, consider adding a development environment for developers instead of as mentioned in #24.

Meanwhile, if you can provide the database structure, along with a data dictionary, that'll help immensely, thanks

@limyifan1
Copy link
Owner Author

Thanks for the amazing tips! Implementing 2 right now. How can the validation be done? :) Do you mean through Firebase rules or through frontend checks? Thank you!

@limyifan1
Copy link
Owner Author

Also added a brief database structure with data dictionary in the README. If more is needed do let me know :)

@magicalbeacon
Copy link
Contributor

magicalbeacon commented May 6, 2020

You can write validations in the rules itself, for such operations you can refer to here (i.e. We can check if price is indeed a number by allowing them write access if math.isNaN(input) or even check if the lat, lng they provided is within a certain range). Ideally, there should be both frontend and backend checks, such that malicious users are not
able to write directly to the backend. Frontend checking acts as a first level of checks.

For point 3, it depends on how your data is stored, from a brief read through of your forms, it seems that you're relying on Firestore to generate a unique document ID. My proposal is to create a Cloud Function that checks for uniqueness before allowing the form to be submitted and providing more information and flexibility to the frontend. I think this will be the most prudent solution as writing these validations is rules requires workarounds, which we want to avoid.

If you need help with this, I'll be happy to help you with the rules and the cloud function.

@limyifan1
Copy link
Owner Author

Your help will be great :) Actually I've not had a good experience with Cloud Functions as sometimes it can take a while to load... perhaps I'm using it wrongly.

@magicalbeacon
Copy link
Contributor

By default, all Cloud Functions run in us-central1, which is in Iowa. You can change the region where your cloud functions run, which will reduce latency for your clients, take a look at this documentation. Personally, I use asia-east2 which is located in Hong Kong since it is the closest to Singapore.

Also, there is a concept of a 'cold boot' time for Cloud Functions, which means that it'll take time for it to spin-up when it hasn't been used for a while, once it becomes 'warm', it gets significantly faster.

@limyifan1
Copy link
Owner Author

Oh wow didn't know all those things, thanks so much! Added in the regions :)

@magicalbeacon
Copy link
Contributor

magicalbeacon commented May 6, 2020

I'll help with writing the function for #58 and some validation rules for Firestore.
I think that checking for duplicates should have multiple layers, (i.e. checking for latitude and longitude), since addresses can have different formats but fundamentally, they're at the same location. Not sure about the best approach for this, since in the issue description, there are edge cases where users key in block numbers into the unit number description, perhaps this can be more streamlined by trimming spaces or separating the level and unit nos. But for now, I'll create a simple cloud function for this.

For data validation, I'll try to validate fields that can be validated, as Firestore rules' syntax is not a full-fledged programming language (ideally, these tasks should be delegated to cloud functions, so you have more control), but I'll see what I can do.

@limyifan1
Copy link
Owner Author

Thanks so much for agreeing to help!! :) that sounds greattt

@magicalbeacon
Copy link
Contributor

Just a quick update, I've written some rudimentary rules and unit tests for them, will make a PR when I'm done.
A few points to consider:

  1. Data set is large for a single document with many optional fields, makes testing and validation difficult on the rules, consider removing or moving common data to a different collection or subcollection.

  2. Read/Writes to the Firestore are still available publicly, this means that anyone with the Firestore address will be able to make reads/writes to it via means of the Firestore REST API, or simply using this repo, consider moving all operations to the cloud functions or adding authentication.

  3. Image filenames/strings in database, it seems you can refactor this to an array of strings instead of a field for every single image (even if they're null), this will reduce the amount of fields on the document and help with validation.

@limyifan1
Copy link
Owner Author

Thanks for the update :) Responses below

  1. Great suggestion, so would you suggest we have another Firestore collection to store the less relevant data?
  2. For the Firestore security that's been a big issue on our mind but I'm still new so I'm still trying to figure out how to do that. Someone suggested building an admin console, what do you think? I used Firebase cloud functions before but somehow it loaded rather slow, but it might be because we were using the US server. Could try doing that in the future!
  3. Agree! We've done this for our menus now, so images is up next :)

@magicalbeacon
Copy link
Contributor

magicalbeacon commented May 11, 2020

For 1), I think it's the same as 3, just try to streamline the document, (i.e. you can create folders in Firebase Storage, so you can name the folder such as to the document ID). Need to brainstorm about this.

For 2), if you guys are considering adding authentication, you can have a look a Firebase authentication for very quick integration into your web app, you don't have to manage the storing of usernames/passwords, all you have to do is create a /Users/ collection to tracker other user information/access controls, if you need help with this, I will be glad to with the integration and on the backend as well.

To be clear, it is very difficult to add security rules/validation to an open database, because there is no concept of ownership/identification, this makes it difficult to limit who can view/edit documents. This solves the problems of ownership/identification, but anyone with an account can create documents, but that's a problem for the validation side. We'll have to think of the best way to do this.

@limyifan1
Copy link
Owner Author

Thanks so much! I've no prior experience with authentication so that'll be great :) I've created a Users collection in case you wish to further build on this.

On a side note, I'll take a look at your PR and merge it!

@magicalbeacon
Copy link
Contributor

magicalbeacon commented May 11, 2020

No worries! Always happy to help, when authentication comes in, you'll definitely need to use Cloud Functions to create user documents, and restrict any write access to them, since you do not want any User modifiable fields in there (for obvious security reasons).

Anyway, if you have any questions about the PR let me know (sorry for the botched commit messages!).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants