PalisadoesFoundation / talawa-api

API Backend for the Talawa Mobile App. Click on the link below to see our documentation
https://docs.talawa.io/
GNU General Public License v3.0
205 stars 683 forks source link

No encryption of user email addresses in the database #1756

Open palisadoes opened 5 months ago

palisadoes commented 5 months ago

Describe the bug

  1. User email addresses are not encrypted in the database
  2. This means a bad actor could SPAM Talawa users if they got access to the database

To Reproduce

  1. Dump the database
  2. View unencrypted email data in the User table

Expected behavior

A solution where:

  1. Email addresses are:
    1. encrypted for storage in the database
    2. decrypted for retrieval from the database
  2. The encryption key is not stored in the database
  3. Uses a random salt in addition to the encryption key
  4. The encryption key is configured with setup.ts
    1. If a value is found, then the default for changing the value must be No
    2. If a value is not found, then the encryption key must be randomly generated
  5. Uses a strong encryption algorithm
  6. The User email address data in the sample_data/users.json file must be updated with the encrypted addresses during the data importation process.
  7. The encryption / decryption key(s) are not stored in the:
    1. code base
    2. database

Actual behavior

  1. Email addresses are not encrypted

Screenshots

Additional details

  1. This is the first of many issues to encrypt PII stored in the database
  2. All tests must pass and be valid
  3. No other functionality must be affected

Potential internship candidates

Please read this if you are planning to apply for a Palisadoes Foundation internship

Anubhav-2003 commented 5 months ago

Respected sir, I can work in this issue, I am comfortable with our graphql schema, after the last two issues got merged. Thank You.

AVtheking commented 5 months ago

I would like to work on this issue

devsargam commented 5 months ago

Can I work on this?

Anubhav-2003 commented 5 months ago

@palisadoes Respected sir,

I wanted to clarify one question. Do you want a single encryption key, with a email specific random salt for every user email. Or a user specific unique encryption key, just like the user specific salt for each email. The latter option would involve creating an in-house KMS for handling the keys.

Thank You.

palisadoes commented 5 months ago
  1. A single encryption key, encrypting the field using a random salt.
  2. Here is an example that could be used.
  3. Use a long key, the token generation methodology used in the setup.ts file could be used
  4. Research other implementations as possible candidates and use them if better. The PR reviewers will ask about the rationale for your approach.
Anubhav-2003 commented 5 months ago

@palisadoes Ok sir, Thank You.

Anubhav-2003 commented 5 months ago

@palisadoes Respected sir,

There was a recent revert of a PR in the API that was causing error related to user signup. I had started my feature branch before the revert. So i had to merge the latest upstream to my feature branch. But as a result a lot of files were changed.

One thing I noticed is that for every file changed, eslint throws multiple linting errors that are already present in the code base. At the moment around a hundred linting errors are showing while I try to commit my changes. How can I disable those errors. Otherwise I am unable to commit. Every new line of code I write is passed through linting checks, but the errors shown are for hundreds of lines of code already present.

Thank You.

palisadoes commented 5 months ago

Please ask the talawa-api slack channel for assistance.

Anubhav-2003 commented 5 months ago

@palisadoes Respected sir,

The issue is almost done. But I am using an opensource key management service by HashIcorp, for an in-house secret management. As storing the encryption key as plaintext in the .env file is not secure, and industry standard. But this would require all future users of Talawa-api to install 'Vault' from 'HashICorp' , into their local systems and configure it before they can start contributing. Also when pushed to the main repo, the actual cloud instance that runs the API in production must also be updated with the latest software.

Should I proceed with this major addition of software. Or store the key in the .env file only. I feel that if we make the migration, then all current secrets in the .env file could be migrated to the service as well for better security.

Thank You.

palisadoes commented 5 months ago

At this time use the .env file for the simplicity of implementation.

Anubhav-2003 commented 5 months ago

@palisadoes ok sir. Thank You.

github-actions[bot] commented 5 months ago

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

Anubhav-2003 commented 4 months ago

This issue is active. I have already raised a PR, it is awaited approval. Thank you.

github-actions[bot] commented 4 months ago

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

Anubhav-2003 commented 4 months ago

I have already raised PR for this issue, but due to the merge of my recent PR #1896 and a few others there has been drastic changes in the setup. I will be updating the PR as soon as the new implementations are done. Thank You.

github-actions[bot] commented 4 months ago

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

jyotiv737 commented 4 months ago

@Anubhav-2003 Are you working on this?

Anubhav-2003 commented 4 months ago

@Anubhav-2003 Are you working on this?

Actually, I have already raised a PR for this, the feature is completely implemented, but due to recent pull request merges. Some tests are failing. Thank you.

github-actions[bot] commented 3 months ago

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

Anubhav-2003 commented 3 months ago

Update: working on the latest new PR for this issue after the userType-fix branch merge.

github-actions[bot] commented 2 months ago

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

Cioppolo14 commented 2 months ago

Unassigning due to no activity or open PR

github-actions[bot] commented 2 months ago

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.