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

[feature] Desensitize sensitive information. #2848

Closed
wants to merge 12 commits into from

Conversation

ayu-v0
Copy link
Contributor

@ayu-v0 ayu-v0 commented Nov 30, 2024

What's changed?

#2792

This PR desensitizes the phone and email, while ensuring that the desensitized data cannot be submitted to the database.
The desensitization rule for a email is to replace it with a "." to make the email illegal and ensure that it cannot be submitted directly to the database.The same goes for cell phone numbers.

Here is a picture of the modification operation:
p1
p2

The following is the information of the database:
p3
p4

Checklist

  • I have read the Contributing Guide
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

Add or update API

  • I have added the necessary e2e tests and all cases have passed.

@zqr10159 zqr10159 requested review from tomsun28 and Ceilzcx December 1, 2024 14:19
@tomsun28
Copy link
Contributor

tomsun28 commented Dec 4, 2024

relate with #2800

@ayu-v0
Copy link
Contributor Author

ayu-v0 commented Dec 4, 2024

The merge error is caused by @Date and @description, I've fixed it @yuluo-yx

@tomsun28
Copy link
Contributor

tomsun28 commented Dec 4, 2024

hi @ayu-v0 thanks for this pr.
I reviewed and found that this implementation is not very reasonable. In case user modify the old data, eg: receiver name, phone. Its impact will make people feel confused and think it is a bug.
and another case like send test alarm, it will use the serialize data [email protected] to send email and failed.
Suggest suspend this issue, investigate the desensitization methods of other platforms later.

@tomsun28
Copy link
Contributor

tomsun28 commented Dec 4, 2024

Sorry for rejecting this PR you worked so hard on.

@ayu-v0
Copy link
Contributor Author

ayu-v0 commented Dec 4, 2024

Sorry for rejecting this PR you worked so hard on.

Thank you very much for your review. This PR may be because I considered that the desensitized data is the correct information that the user has verified, so I did not consider that the user might send the email with the desensitized information. Based on this scenario, I wonder if there are any solutions (if I can think of them, haha).

Aias00 and others added 3 commits December 5, 2024 08:50
…-branch

# Conflicts:
#	hertzbeat-common/src/main/java/org/apache/hertzbeat/common/serialize/EmailDesensitizationSerializer.java
#	hertzbeat-common/src/main/java/org/apache/hertzbeat/common/serialize/PhoneDesensitizationSerializer.java
@zqr10159 zqr10159 closed this Dec 6, 2024
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