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

#257 Refine database security rules and write more tests #633

Merged
merged 4 commits into from
Sep 13, 2021

Conversation

HalcyonJAC
Copy link
Contributor

Author checklist

  • Include primary ticket number in title - e.g. "Move uploaded references to Google Drive #123 New styling for widget" - and any additional tickets in description
  • Fill in the details below and delete as appropriate
  • Be proactive in getting your work approved 💪

What's included?

This work fixes the email verification issue with third party (specifically Microsoft) authenticated accounts to the admin platform. The security rules require for all users that an email address is verified, and there is no way of telling on firestore where the request originated from.

Microsoft accounts do not set the email verified flag to true. It is a requirements that these are set to true if logging in via the admin platform, as users from the JAC and other government departments logging into the platform should not have to verify their email address.

This pull request is the other half connected with jac-uk/admin#1476 and contains a re-instantiation of the email validation for security rules, fixes for some firestore rules, addition of field validation for some documents and fixing of all current firestore tests, as well as adding some new ones.

Who should test?

  • Product Owner
  • Developers
  • UTG

How to test?

Login using a microsoft account once merged, as per jac-uk/admin#1476.

To run the tests, follow the instructions in the README.md file. You will need to change the file referenced in the package.json file for npm run test:rules

Risk - how likely is this to impact other areas?

🟠 Medium risk - this does change code that is shared with other areas

Additional context

Include screen grabs, video demo, notes etc.


PREVIEW:DEVELOP
can be OFF, DEVELOP or STAGING

Copy link
Member

@warrensearle warrensearle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍

@@ -17,6 +17,33 @@ See [database/firestore.indexes.json](database/firestore.indexes.json) for our c

See [storage/storage.rules](storage/storage.rules) for our current rules.


To run the emulator on your local machine, create a `./data/firestore.json` file with the contents of `[]`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀 This is super helpful! - thanks!

@joy-ade joy-ade merged commit 75f6109 into develop Sep 13, 2021
@joy-ade joy-ade deleted the 257-security-rules-email-verification branch September 13, 2021 12:18
warrensearle added a commit that referenced this pull request Sep 14, 2021
warrensearle added a commit that referenced this pull request Sep 14, 2021
@warrensearle warrensearle restored the 257-security-rules-email-verification branch September 14, 2021 09:53
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

Successfully merging this pull request may close these issues.

5 participants