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

No encryption of user email addresses in the database #1756

Open
palisadoes opened this issue Jan 29, 2024 · 32 comments
Open

No encryption of user email addresses in the database #1756

palisadoes opened this issue Jan 29, 2024 · 32 comments
Assignees
Labels
bug Something isn't working no-issue-activity No issue activity security Security fix

Comments

@palisadoes
Copy link
Contributor

palisadoes commented Jan 29, 2024

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

  • N/A

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

@palisadoes palisadoes added bug Something isn't working security Security fix labels Jan 29, 2024
@github-actions github-actions bot added the unapproved Unapproved for Pull Request label Jan 29, 2024
@Anubhav-2003
Copy link
Contributor

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
Copy link

I would like to work on this issue

@devsargam
Copy link

Can I work on this?

@Cioppolo14 Cioppolo14 removed the unapproved Unapproved for Pull Request label Jan 29, 2024
@Anubhav-2003
Copy link
Contributor

Anubhav-2003 commented Jan 30, 2024

@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
Copy link
Contributor Author

palisadoes commented Jan 30, 2024

  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
Copy link
Contributor

@palisadoes Ok sir, Thank You.

@Anubhav-2003
Copy link
Contributor

@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
Copy link
Contributor Author

Please ask the talawa-api slack channel for assistance.

@Anubhav-2003
Copy link
Contributor

@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
Copy link
Contributor Author

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

@Anubhav-2003
Copy link
Contributor

@palisadoes ok sir. Thank You.

Copy link

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.

@github-actions github-actions bot added the no-issue-activity No issue activity label Feb 21, 2024
@Anubhav-2003
Copy link
Contributor

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

@github-actions github-actions bot removed the no-issue-activity No issue activity label Feb 22, 2024
Copy link

github-actions bot commented Mar 3, 2024

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.

@github-actions github-actions bot added the no-issue-activity No issue activity label Mar 3, 2024
@Anubhav-2003
Copy link
Contributor

Anubhav-2003 commented Mar 3, 2024

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 github-actions bot removed the no-issue-activity No issue activity label Mar 4, 2024
Copy link

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.

@github-actions github-actions bot added the no-issue-activity No issue activity label Mar 15, 2024
@jyotiv737
Copy link

@Anubhav-2003 Are you working on this?

@Anubhav-2003
Copy link
Contributor

@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 github-actions bot removed the no-issue-activity No issue activity label Mar 19, 2024
Copy link

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.

@github-actions github-actions bot added the no-issue-activity No issue activity label Mar 29, 2024
@Anubhav-2003
Copy link
Contributor

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

@github-actions github-actions bot removed the no-issue-activity No issue activity label Apr 13, 2024
Copy link

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.

@github-actions github-actions bot added the no-issue-activity No issue activity label Apr 23, 2024
@Cioppolo14 Cioppolo14 removed the no-issue-activity No issue activity label Apr 23, 2024
@Cioppolo14
Copy link
Contributor

Unassigning due to no activity or open PR

Copy link

github-actions bot commented May 4, 2024

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.

@github-actions github-actions bot added the no-issue-activity No issue activity label May 4, 2024
@Suyash878
Copy link

Suyash878 commented Aug 11, 2024

I would like to work on this issue. I will proceed with the preferred AES-256-GCM Encryption/Decryption algorithm.

@palisadoes
Copy link
Contributor Author

You will need to ensure that all the encryption / decryption will work:

  1. maintaining existing functionality
  2. across the entire test suite
  3. with the setup script where sample data is imported.

@Suyash878
Copy link

Suyash878 commented Aug 12, 2024

I was planning on implementing function for the encryption key variable in the setup.ts.
So my question was, Is there a specific preference for where exactly should this function be implemented? as in for instance after the mongodb URL function and or before the recaptcha function.
Or should I just proceed with what I think would be the most suitable place.

@github-actions github-actions bot removed the no-issue-activity No issue activity label Aug 13, 2024
@Suyash878
Copy link

Suyash878 commented Aug 16, 2024

According to this error, we must switch to eslint.config.js from .eslintrc.json, and moreover .eslintignore is no longer supported either. This could be a potential issue in my opinion.
If there's something that needs to be done on my end, I would like to know.
image

@palisadoes
Copy link
Contributor Author

According to this error, we must switch to eslint.config.js from .eslintrc.json, and moreover .eslintignore is no longer supported either. This could be a potential issue in my opinion. If there's something that needs to be done on my end, I would like to know. image

Please open an issue to fix this for both Admin and the API

Copy link

github-actions bot commented Sep 2, 2024

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.

@github-actions github-actions bot added the no-issue-activity No issue activity label Sep 2, 2024
@AnshulKahar2729
Copy link

Suyash are you working on this.

@Suyash878
Copy link

Yes, I am working on this.

@github-actions github-actions bot removed the no-issue-activity No issue activity label Sep 5, 2024
Copy link

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.

@github-actions github-actions bot added the no-issue-activity No issue activity label Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working no-issue-activity No issue activity security Security fix
Projects
None yet
Development

No branches or pull requests

8 participants