talawa-api
talawa-api copied to clipboard
No encryption of user email addresses in the database
Describe the bug
- User email addresses are not encrypted in the database
- This means a bad actor could SPAM Talawa users if they got access to the database
To Reproduce
- Dump the database
- View unencrypted email data in the User table
Expected behavior
A solution where:
- Email addresses are:
- encrypted for storage in the database
- decrypted for retrieval from the database
- The encryption key is not stored in the database
- Uses a random salt in addition to the encryption key
- The encryption key is configured with
setup.ts
- If a value is found, then the default for changing the value must be
No
- If a value is not found, then the encryption key must be randomly generated
- If a value is found, then the default for changing the value must be
- Uses a strong encryption algorithm
- The User email address data in the
sample_data/users.json
file must be updated with the encrypted addresses during the data importation process. - The encryption / decryption key(s) are not stored in the:
- code base
- database
Actual behavior
- Email addresses are not encrypted
Screenshots
- N/A
Additional details
- This is the first of many issues to encrypt PII stored in the database
- All tests must pass and be valid
- No other functionality must be affected
Potential internship candidates
Please read this if you are planning to apply for a Palisadoes Foundation internship
- https://github.com/PalisadoesFoundation/talawa/issues/359
Respected sir, I can work in this issue, I am comfortable with our graphql schema, after the last two issues got merged. Thank You.
I would like to work on this issue
Can I work on this?
@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.
- A single encryption key, encrypting the field using a random salt.
- Here is an example that could be used.
- https://gist.github.com/AndiDittrich/4629e7db04819244e843
- Use a long key, the token generation methodology used in the
setup.ts
file could be used - Research other implementations as possible candidates and use them if better. The PR reviewers will ask about the rationale for your approach.
@palisadoes Ok sir, Thank You.
@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.
Please ask the talawa-api slack channel for assistance.
@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.
At this time use the .env
file for the simplicity of implementation.
@palisadoes ok sir. Thank You.
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.
This issue is active. I have already raised a PR, it is awaited approval. Thank you.
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.
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.
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 Are you working on this?
@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.
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.
Update: working on the latest new PR for this issue after the userType-fix branch merge.
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.
Unassigning due to no activity or open PR
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.
I would like to work on this issue. I will proceed with the preferred AES-256-GCM Encryption/Decryption algorithm.
You will need to ensure that all the encryption / decryption will work:
- maintaining existing functionality
- across the entire test suite
- with the setup script where sample data is imported.
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.
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.
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.
Please open an issue to fix this for both Admin and the API
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.
Suyash are you working on this.