Skip to content

[feature] Desensitize sensitive information(new) #13

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

[feature] Desensitize sensitive information(new) #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c9adee8
[feature] Desensitize sensitive information(new)
Dec 6, 2024
ab96101
[chore] Added phone desensitization serializer test
Dec 9, 2024
578337c
[chore] Added email desensitization serializer test
Dec 9, 2024
d2a8725
Merge branch 'master' into e-branch
Aias00 Dec 14, 2024
5b34623
Merge branch 'master' into e-branch
Aias00 Dec 16, 2024
8ade3d3
Merge branch 'master' into e-branch
ayu-v0 Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions hertzbeat-common/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,20 @@
<version>${poi.version}</version>
<scope>compile</scope>
</dependency>

<dependency>
<groupId>org.apache.poi</groupId>
<artifactId>poi-ooxml</artifactId>
<version>${poi.version}</version>
<scope>compile</scope>
</dependency>

<!-- hutool -->
<dependency>
<groupId>cn.hutool</groupId>
<artifactId>hutool-all</artifactId>
<version>${hutool-all.version}</version>
</dependency>
</dependencies>

</project>
9 changes: 9 additions & 0 deletions hertzbeat-common/src/main/java/org/apache/hertzbeat/common/cache/CacheFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ private CacheFactory() {}

private static final CommonCacheService<String, Object> ALERT_CONVERGE_CACHE =
new CaffeineCacheServiceImpl<>(10, 1000, Duration.ofDays(1), false);

private static final CommonCacheService<String, Object> DESENSITIZATION_MAP_CACHE =
new CaffeineCacheServiceImpl<>(10, 1000, Duration.ofDays(1), false);
Copy link

Choose a reason for hiding this comment

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

Cache Expiration Risk

Sensitive data stored in cache for 1 day creates unnecessary exposure risk. Long-lived sensitive data cache increases potential compromise window if application is breached.

Suggested change
new CaffeineCacheServiceImpl<>(10, 1000, Duration.ofDays(1), false);
private static final CommonCacheService<String, Object> DESENSITIZATION_MAP_CACHE =
new CaffeineCacheServiceImpl<>(10, 1000, Duration.ofMinutes(30), false);
Standards
  • CWE-922
  • OWASP-A02

Copy link

Choose a reason for hiding this comment

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

Cache Expiration Risk

Desensitization cache uses a long expiration time (1 day) which increases sensitive data exposure risk. Attackers with access to the application could access sensitive data longer than necessary.

    private static final CommonCacheService<String, Object> DESENSITIZATION_MAP_CACHE =
            new CaffeineCacheServiceImpl<>(10, 1000, Duration.ofMinutes(30), false);
Standards
  • CWE-539
  • OWASP-A02

Comment on lines +37 to +38
Copy link

Choose a reason for hiding this comment

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

Cache Size Limitation

The DESENSITIZATION_MAP_CACHE is configured with a maximum size of 1000 entries and TTL of 1 day. For large systems with many users viewing sensitive data, this could lead to cache evictions and reduced performance. The cache size should be adjusted based on expected user load and sensitive data volume.

    private static final CommonCacheService<String, Object> DESENSITIZATION_MAP_CACHE =
            new CaffeineCacheServiceImpl<>(10, 5000, Duration.ofHours(12), false);
Standards
  • ISO-IEC-25010-Performance-Resource-Utilization
  • Netflix-Multi-Layer-Caching

Comment on lines +37 to +38
Copy link

Choose a reason for hiding this comment

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

Cache Cleanup Missing

The desensitization cache stores sensitive data for 1 day without explicit cleanup mechanism. This can lead to sensitive data persisting longer than necessary, creating potential data exposure risks.

private static final CommonCacheService<String, Object> DESENSITIZATION_MAP_CACHE =
        new CaffeineCacheServiceImpl<>(10, 1000, Duration.ofMinutes(30), true);
Standards
  • CWE-212
  • CWE-400
  • ISO-IEC-25010-Reliability-Security

Copy link

Choose a reason for hiding this comment

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

Missing Cache Expiration

Sensitive data cache retains information for 1 day without explicit expiration control. Extended cache retention increases exposure window of sensitive information. Shorter TTL reduces risk window.

private static final CommonCacheService<String, Object> DESENSITIZATION_MAP_CACHE =
            new CaffeineCacheServiceImpl<>(10, 1000, Duration.ofMinutes(30), true);
Standards
  • CWE-524
  • OWASP-A04

Copy link

Choose a reason for hiding this comment

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

Cache Eviction Strategy

Desensitization cache uses fixed 1-day expiration without explicit eviction strategy. This could lead to memory pressure if many records are processed. Consider implementing a more aggressive expiration policy or size-based eviction.

Standards
  • ISO-IEC-25010-Reliability-Resource-Utilization
  • SRE-Resource-Management


/**
* get notice cache
Expand All @@ -57,4 +60,10 @@ public static CommonCacheService<String, Object> getAlertSilenceCache() {
public static CommonCacheService<String, Object> getAlertConvergeCache() {
return ALERT_CONVERGE_CACHE;
}

/**
* get desensitizationMap cache
* @return desensitizationMap cache
*/
public static CommonCacheService<String, Object> getDesensitizationMapCache(){return DESENSITIZATION_MAP_CACHE;};
Copy link

Choose a reason for hiding this comment

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

medium

The formatting of this method is inconsistent with the other getter methods in this class. For better readability and maintainability, please format it to follow the same style.

    public static CommonCacheService<String, Object> getDesensitizationMapCache() {
        return DESENSITIZATION_MAP_CACHE;
    }

}
242 changes: 242 additions & 0 deletions ...beat-common/src/main/java/org/apache/hertzbeat/common/entity/dto/vo/NoticeReceiverVO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,242 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hertzbeat.common.entity.dto.vo;

import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.persistence.Column;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.validation.constraints.Min;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Size;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.NoArgsConstructor;
import org.apache.hertzbeat.common.serialize.EmailDesensitizationSerializer;
import org.apache.hertzbeat.common.serialize.PhoneDesensitizationSerializer;
import org.springframework.data.annotation.CreatedBy;
import org.springframework.data.annotation.CreatedDate;
import org.springframework.data.annotation.LastModifiedBy;
import org.springframework.data.annotation.LastModifiedDate;

import java.time.LocalDateTime;

import static io.swagger.v3.oas.annotations.media.Schema.AccessMode.READ_ONLY;
import static io.swagger.v3.oas.annotations.media.Schema.AccessMode.READ_WRITE;

/**
* 2024-12-06
*/
@Data
@AllArgsConstructor
@NoArgsConstructor
@Builder
public class NoticeReceiverVO {

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
@Schema(title = "Recipient entity primary key index ID", description = "Recipient entity primary key index ID",
example = "87584674384", accessMode = READ_ONLY)
private Long id;

@Schema(title = "Recipient name", description = "Recipient name",
example = "tom", accessMode = READ_WRITE)
@Size(max = 100)
@NotBlank(message = "name can not null")
private String name;

@Schema(title = "Notification information method: 0-SMS 1-Email 2-webhook 3-WeChat Official Account 4-Enterprise WeChat Robot "
+ "5-DingTalk Robot 6-FeiShu Robot 7-Telegram Bot 8-SlackWebHook 9-Discord Bot 10-Enterprise WeChat app message",
description = "Notification information method: "
+ "0-SMS 1-Email 2-webhook 3-WeChat Official Account "
+ "4-Enterprise WeChat Robot 5-DingTalk Robot 6-FeiShu Robot "
+ "7-Telegram Bot 8-SlackWebHook 9-Discord Bot 10-Enterprise "
+ "WeChat app message",
accessMode = READ_WRITE)
@Min(0)
@NotNull(message = "type can not null")
private Byte type;

@Schema(title = "Mobile number: Valid when the notification method is SMS",
description = "Mobile number: Valid when the notification method is SMS",
example = "18923435643", accessMode = READ_WRITE)
@Size(max = 100)
@JsonSerialize(using = PhoneDesensitizationSerializer.class)
private String phone;

@Schema(title = "Email account: Valid when the notification method is email",
description = "Email account: Valid when the notification method is email",
example = "[email protected]", accessMode = READ_WRITE)
@Size(max = 100)
@JsonSerialize(using = EmailDesensitizationSerializer.class)
private String email;

@Schema(title = "URL address: The notification method is valid for webhook",
description = "URL address: The notification method is valid for webhook",
example = "https://www.tancloud.cn", accessMode = READ_WRITE)
@Size(max = 300)
@Column(length = 300)
private String hookUrl;

@Schema(title = "openId : The notification method is valid for WeChat official account, enterprise WeChat robot or FlyBook robot",
description = "openId : The notification method is valid for WeChat official account, enterprise WeChat robot or FlyBook robot",
example = "343432", accessMode = READ_WRITE)
@Size(max = 300)
@Column(length = 300)
private String wechatId;

@Schema(title = "Access token : The notification method is valid for DingTalk robot",
description = "Access token : The notification method is valid for DingTalk robot",
example = "34823984635647", accessMode = READ_WRITE)
@Size(max = 300)
@Column(length = 300)
private String accessToken;

@Schema(title = "Telegram bot token : The notification method is valid for Telegram Bot",
description = "Telegram bot token : The notification method is valid for Telegram Bot",
example = "1499012345:AAEOB_wEYS-DZyPM3h5NzI8voJMXXXXXX", accessMode = READ_WRITE)
private String tgBotToken;

@Schema(title = "Telegram user id: The notification method is valid for Telegram Bot",
description = "Telegram user id: The notification method is valid for Telegram Bot",
example = "779294123", accessMode = READ_WRITE)
private String tgUserId;

@Schema(title = "DingTalk,FeiShu,WeWork user id: The notification method is valid for DingTalk,FeiShu,WeWork Bot",
description = "DingTalk,FeiShu,WeWork user id: The notification method is valid for DingTalk,FeiShu,WeWork Bot",
example = "779294123", accessMode = READ_WRITE)
private String userId;

@Schema(title = "URL address: The notification method is valid for Slack",
description = "URL address: The notification method is valid for Slack",
example = "https://hooks.slack.com/services/XXXX/XXXX/XXXX", accessMode = READ_WRITE)
@Size(max = 300)
@Column(length = 300)
private String slackWebHookUrl;

@Schema(title = "Enterprise weChat message: The notification method is valid for Enterprise WeChat app message",
description = "Enterprise weChat message: The notification method is valid for Enterprise WeChat app message",
example = "ww1a603432123d0dc1", accessMode = READ_WRITE)
private String corpId;

@Schema(title = "Enterprise weChat appId: The notification method is valid for Enterprise WeChat app message",
description = "Enterprise weChat appId: The notification method is valid for Enterprise WeChat app message",
example = "1000001", accessMode = READ_WRITE)
private Integer agentId;

@Schema(title = "Enterprise weChat secret: The notification method is valid for Enterprise WeChat app message",
description = "Enterprise weChat secret: The notification method is valid for Enterprise WeChat app message",
example = "oUydwn92ey0lnuY02MixNa57eNK-20dJn5NEOG-u2uE", accessMode = READ_WRITE)
private String appSecret;

@Schema(title = "Enterprise weChat party id: The notification method is valid for Enterprise WeChat app message",
description = "Enterprise weChat party id: The notification method is valid for Enterprise WeChat app message",
example = "779294123", accessMode = READ_WRITE)
private String partyId;

@Schema(title = "Enterprise weChat tag id: The notification method is valid for Enterprise WeChat app message",
description = "Enterprise weChat tag id: The notification method is valid for Enterprise WeChat app message",
example = "779294123", accessMode = READ_WRITE)
private String tagId;

@Schema(title = "Discord channel id: The notification method is valid for Discord",
description = "Discord channel id: The notification method is valid for Discord",
example = "1065303416030642266", accessMode = READ_WRITE)
@Size(max = 300)
@Column(length = 300)
private String discordChannelId;

@Schema(title = "Discord bot token: The notification method is valid for Discord",
description = "Discord bot token: The notification method is valid for Discord",
example = "MTA2NTMwMzU0ODY4Mzg4MjUzNw.xxxxx.xxxxxxx", accessMode = READ_WRITE)
@Size(max = 300)
@Column(length = 300)
private String discordBotToken;

@Schema(title = "huawei cloud SMN ak: If the notification method is valid for huawei cloud SMN",
description = "huawei cloud SMN ak: If the notification method is valid for huawei cloud SMN",
example = "NCVBODJOEYHSW3VNXXXX", accessMode = READ_WRITE)
@Size(max = 22)
@Column(length = 22)
private String smnAk;

@Schema(title = "huawei cloud SMN sk: If the notification method is valid for huawei cloud SMN",
description = "huawei cloud SMN sk: If the notification method is valid for huawei cloud SMN",
example = "nmSNhUJN9MlpPl8lfCsgdA0KvHCL9JXXXX", accessMode = READ_WRITE)
@Size(max = 42)
@Column(length = 42)
private String smnSk;

@Schema(title = "huawei cloud SMN projectId: If the notification method is valid for huawei cloud SMN",
description = "huawei cloud SMN projectId: If the notification method is valid for huawei cloud SMN",
example = "320c2fb11edb47a481c299c1XXXXXX", accessMode = READ_WRITE)
@Size(max = 32)
@Column(length = 32)
private String smnProjectId;

@Schema(title = "huawei cloud SMN region: If the notification method is valid for huawei cloud SMN",
description = "huawei cloud SMN region: If the notification method is valid for huawei cloud SMN",
example = "cn-east-3", accessMode = READ_WRITE)
@Size(max = 32)
@Column(length = 32)
private String smnRegion;

@Schema(title = "huawei cloud SMN TopicUrn: If the notification method is valid for huawei cloud SMN",
description = "huawei cloud SMN TopicUrn: If the notification method is valid for huawei cloud SMN",
example = "urn:smn:cn-east-3:xxx:hertzbeat_test", accessMode = READ_WRITE)
@Size(max = 300)
@Column(length = 300)
private String smnTopicUrn;

@Schema(title = "serverChanToken : The notification method is valid for ServerChan",
description = "serverChanToken : The notification method is valid for ServerChan",
example = "SCT193569TSNm6xIabdjqeZPtOGOWcvU1e", accessMode = READ_WRITE)
@Size(max = 300)
@Column(length = 300)
private String serverChanToken;

@Schema(title = "Gotify token : The notification method is valid for Gotify",
description = "Gotify token : The notification method is valid for Gotify",
example = "A845h__ZMqDxZlO", accessMode = READ_WRITE)
@Size(max = 300)
@Column(length = 300)
private String gotifyToken;

@Schema(title = "The creator of this record", example = "tom",
accessMode = READ_ONLY)
@CreatedBy
private String creator;

@Schema(title = "This record was last modified by", example = "tom", accessMode = READ_ONLY)
@LastModifiedBy
private String modifier;

@Schema(title = "Record creation time (millisecond timestamp)",
example = "1612198922000", accessMode = READ_ONLY)
@CreatedDate
private LocalDateTime gmtCreate;

@Schema(title = "Record the latest modification time (timestamp in milliseconds)",
example = "1612198444000", accessMode = READ_ONLY)
@LastModifiedDate
private LocalDateTime gmtUpdate;
}
49 changes: 49 additions & 0 deletions ...n/src/main/java/org/apache/hertzbeat/common/serialize/EmailDesensitizationSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hertzbeat.common.serialize;

import cn.hutool.core.util.StrUtil;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.JsonSerializer;
import com.fasterxml.jackson.databind.SerializerProvider;
import org.apache.hertzbeat.common.cache.CacheFactory;
import org.apache.hertzbeat.common.cache.CommonCacheService;
import org.apache.hertzbeat.common.entity.dto.vo.NoticeReceiverVO;

import java.io.IOException;

/**
* 2024-12-06
* Email Desensitizing serializer
*/
public class EmailDesensitizationSerializer extends JsonSerializer<String> {

@Override
public void serialize(String email, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) throws IOException {
Comment on lines +34 to +37
Copy link

Choose a reason for hiding this comment

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

Missing Javadoc Comments

Class and method lack Javadoc comments explaining purpose and behavior. Missing documentation reduces maintainability by making code intent and usage less clear for future developers.

/**
 * Custom serializer that desensitizes email addresses by replacing characters with '*'.
 * Stores original value in cache for retrieval when needed.
 */
public class EmailDesensitizationSerializer extends JsonSerializer<String> {

    /**
     * Serializes email by replacing characters between first character and '@' with '*'.
     * Stores the mapping between desensitized and original email in cache.
     *
     * @param email Email address to desensitize
     * @param jsonGenerator JSON generator
     * @param serializerProvider Serializer provider
     * @throws IOException If an I/O error occurs
     */
    @Override
    public void serialize(String email, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) throws IOException {
Standards
  • Clean-Code-Documentation
  • Clean-Code-Readability

String emailDesensitization = "";
CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache();
if (StrUtil.isNotBlank(email)) {
int index = StrUtil.indexOf(email, '@');
emailDesensitization = index <= 1 ? email :
StrUtil.replace(email, 1, index, '*');
Comment on lines +41 to +43
Copy link

Choose a reason for hiding this comment

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

Inefficient Email Desensitization

The current implementation uses StrUtil.replace() which creates a new string for each desensitization operation. For high-volume systems processing many emails, this creates unnecessary object allocations and GC pressure. The string replacement operation has O(n) complexity where n is the length of the email string.

int index = StrUtil.indexOf(email, '@');
if (index <= 1) {
    emailDesensitization = email;
} else {
    StringBuilder sb = new StringBuilder(email.length());
    sb.append(email.charAt(0));
    for (int i = 1; i < index; i++) {
        sb.append('*');
    }
    sb.append(email.substring(index));
    emailDesensitization = sb.toString();
}
Standards
  • ISO-IEC-25010-Performance-Resource-Utilization
  • Algorithm-Opt-String-Builder

Comment on lines +41 to +43
Copy link

Choose a reason for hiding this comment

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

Email Desensitization Logic

The email desensitization logic has an edge case where emails with only one character before @ (e.g., [email protected]) won't be desensitized at all. This contradicts the desensitization purpose and creates inconsistent behavior between similar email addresses.

Standards
  • Algorithm-Correctness-Edge-Cases
  • Business-Rule-Consistency
  • Logic-Verification-Boundary-Conditions

NoticeReceiverVO currentValue = (NoticeReceiverVO) jsonGenerator.getOutputContext().getCurrentValue();
Copy link

Choose a reason for hiding this comment

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

medium

Casting jsonGenerator.getOutputContext().getCurrentValue() to NoticeReceiverVO creates a tight coupling between this serializer and the NoticeReceiverVO class. This makes the serializer less reusable and could lead to a ClassCastException if it's ever used on a property of a different class. While this might be acceptable for now, consider if a more decoupled approach is possible for future maintainability.

desensitizationMapCache.put(currentValue.getId() + "_" + emailDesensitization, email);
Comment on lines +44 to +45
Copy link

Choose a reason for hiding this comment

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

Cache Key Collision

Cache key construction using desensitized value creates potential collisions when different emails produce identical desensitized patterns. This can cause incorrect data retrieval during restoration.

Suggested change
NoticeReceiverVO currentValue = (NoticeReceiverVO) jsonGenerator.getOutputContext().getCurrentValue();
desensitizationMapCache.put(currentValue.getId() + "_" + emailDesensitization, email);
NoticeReceiverVO currentValue = (NoticeReceiverVO) jsonGenerator.getOutputContext().getCurrentValue();
desensitizationMapCache.put(currentValue.getId() + "_email", email);
Standards
  • ISO-IEC-25010-Performance-Resource-Utilization
  • Algorithm-Opt-Hash-Map
  • Cache-Key-Uniqueness

Comment on lines +44 to +45
Copy link

Choose a reason for hiding this comment

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

Cache Entry Leakage

Cache entries are created but never removed, potentially causing memory leaks over time. Long-lived cached sensitive data increases exposure risk and memory consumption.

// Add cache entry with expiration time
NoticeReceiverVO currentValue = (NoticeReceiverVO) jsonGenerator.getOutputContext().getCurrentValue();
if (currentValue.getId() != null) {
    desensitizationMapCache.put(currentValue.getId() + "_" + emailDesensitization, email);
}
Standards
  • ISO-IEC-25010-Reliability-Resource-Utilization
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Comment on lines +44 to +45
Copy link

Choose a reason for hiding this comment

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

Cache Key Collision

Cache key uses desensitized email which creates collision risk when multiple receivers have similar emails like '[email protected]' and '[email protected]' (both masked as '*@example.com'). This would overwrite cache entries causing data retrieval errors.

NoticeReceiverVO currentValue = (NoticeReceiverVO) jsonGenerator.getOutputContext().getCurrentValue();
// Use a more unique key pattern to prevent collisions
desensitizationMapCache.put(currentValue.getId() + "_email", email);
Standards
  • Logic-Verification-Data-Integrity
  • Business-Rule-Unique-Key-Generation
  • Algorithm-Correctness-Cache-Management

Copy link

Choose a reason for hiding this comment

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

Cache Key Collision

Cache key uses desensitized email which could cause key collisions when different emails desensitize to the same pattern. This creates a risk of retrieving incorrect original emails during restoration.

// Use a more unique key that includes both ID and a type prefix
desensitizationMapCache.put(currentValue.getId() + "_email_" + emailDesensitization, email);
Standards
  • CWE-664
  • CWE-212
  • Business-Rule-Data-Privacy

Copy link

Choose a reason for hiding this comment

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

Potential Cache Leakage

Cache entries are created but never expired or cleaned up, potentially causing memory leaks as the cache grows unbounded over time. Without TTL or eviction policy, sensitive data remains in memory indefinitely.

// Add cache entry with TTL to prevent memory leaks
desensitizationMapCache.put(currentValue.getId() + "_" + emailDesensitization, email, Duration.ofMinutes(30));
Standards
  • Logic-Verification-Resource-Management
  • Algorithm-Correctness-Memory-Management
  • Business-Rule-Data-Protection

Copy link

Choose a reason for hiding this comment

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

Cache Key Collision

Cache key construction using desensitized value creates potential collision risk. Multiple different emails could produce identical desensitized values, causing cache overwrite and incorrect data retrieval. This would lead to incorrect information being restored during edit operations.

            if (currentValue.getId() != null) {
                // Use original email in key to prevent collisions from different emails with same desensitized pattern
                desensitizationMapCache.put(currentValue.getId() + "_email_" + email, email);
            }
Commitable Suggestion
Suggested change
desensitizationMapCache.put(currentValue.getId() + "_" + emailDesensitization, email);
if (currentValue.getId() != null) {
// Use original email in key to prevent collisions from different emails with same desensitized pattern
desensitizationMapCache.put(currentValue.getId() + "_email_" + email, email);
}
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness
  • DbC-Data-Integrity

Comment on lines +44 to +45
Copy link

Choose a reason for hiding this comment

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

Null ID Handling

The code assumes currentValue.getId() is non-null when constructing the cache key, but there's no null check. If ID is null during serialization (e.g., for new unsaved entities), this will cause a NullPointerException when concatenating with the string.

            NoticeReceiverVO currentValue = (NoticeReceiverVO) jsonGenerator.getOutputContext().getCurrentValue();
            if (currentValue.getId() != null) {
                // Use a prefix and original email to ensure key uniqueness
                desensitizationMapCache.put(currentValue.getId() + "_email_" + email, email);
            }
Commitable Suggestion
Suggested change
NoticeReceiverVO currentValue = (NoticeReceiverVO) jsonGenerator.getOutputContext().getCurrentValue();
desensitizationMapCache.put(currentValue.getId() + "_" + emailDesensitization, email);
NoticeReceiverVO currentValue = (NoticeReceiverVO) jsonGenerator.getOutputContext().getCurrentValue();
if (currentValue.getId() != null) {
// Use a prefix and original email to ensure key uniqueness
desensitizationMapCache.put(currentValue.getId() + "_email_" + email, email);
}
Standards
  • Algorithm-Correctness-Null-Safety
  • Business-Rule-Error-Prevention
  • Logic-Verification-Defensive-Programming

Comment on lines +43 to +45
Copy link

Choose a reason for hiding this comment

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

Cache Security Risk

Sensitive email data is stored in a cache without expiration controls or access restrictions. This creates a potential data exposure risk if the cache is compromised or accessed by unauthorized code. The original sensitive data remains accessible through the cache indefinitely.

Standards
  • CWE-524
  • OWASP-A04
  • NIST-SSDF-PW.1

}
jsonGenerator.writeString(emailDesensitization);
Comment on lines +36 to +47
Copy link

Choose a reason for hiding this comment

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

Null Safety Improvement

The code assumes currentValue is always a NoticeReceiverVO and never null. If the serializer is used with a different context, it could cause NullPointerException during desensitization.

@Override
public void serialize(String email, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) throws IOException {
    String emailDesensitization = "";
    if (StrUtil.isNotBlank(email)) {
        int index = StrUtil.indexOf(email, '@');
        emailDesensitization = index <= 1 ? email :
                StrUtil.replace(email, 1, index, '*');
        Object currentObj = jsonGenerator.getOutputContext().getCurrentValue();
        if (currentObj instanceof NoticeReceiverVO) {
            NoticeReceiverVO currentValue = (NoticeReceiverVO) currentObj;
            if (currentValue.getId() != null) {
                CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache();
                desensitizationMapCache.put(currentValue.getId() + "_" + emailDesensitization, email);
            }
        }
    }
    jsonGenerator.writeString(emailDesensitization);
}
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • CWE-704
  • CWE-755

Comment on lines +38 to +47
Copy link

Choose a reason for hiding this comment

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

Inconsistent Null Handling

The code handles null/blank emails by setting emailDesensitization to an empty string, but doesn't add anything to the cache. This creates inconsistent behavior where desensitized empty values can't be reversed through the cache lookup mechanism used elsewhere.

Standards
  • Algorithm-Correctness-Null-Handling
  • Business-Rule-Consistency
  • Logic-Verification-Edge-Cases

}
}
49 changes: 49 additions & 0 deletions ...n/src/main/java/org/apache/hertzbeat/common/serialize/PhoneDesensitizationSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hertzbeat.common.serialize;

import cn.hutool.core.util.DesensitizedUtil;
import cn.hutool.core.util.StrUtil;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.JsonSerializer;
import com.fasterxml.jackson.databind.SerializerProvider;
import org.apache.hertzbeat.common.cache.CacheFactory;
import org.apache.hertzbeat.common.cache.CommonCacheService;
import org.apache.hertzbeat.common.entity.dto.vo.NoticeReceiverVO;

import java.io.IOException;

/**
* 2024-12-06
* Phone Desensitizing serializer
*/
public class PhoneDesensitizationSerializer extends JsonSerializer<String> {

@Override
public void serialize(String phone, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) throws IOException {
String phoneDesensitization = "";
CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache();
if (StrUtil.isNotBlank(phone)){
phoneDesensitization = DesensitizedUtil.mobilePhone(phone);
NoticeReceiverVO currentValue = (NoticeReceiverVO)jsonGenerator.getOutputContext().getCurrentValue();
Copy link

Choose a reason for hiding this comment

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

medium

Casting jsonGenerator.getOutputContext().getCurrentValue() to NoticeReceiverVO creates a tight coupling between this serializer and the NoticeReceiverVO class. This makes the serializer less reusable and could lead to a ClassCastException if it's ever used on a property of a different class. While this might be acceptable for now, consider if a more decoupled approach is possible for future maintainability.

desensitizationMapCache.put(currentValue.getId()+"_"+phoneDesensitization, phone);
Comment on lines +42 to +44
Copy link

Choose a reason for hiding this comment

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

Null ID Risk

No null check on currentValue.getId() before cache key creation. NullPointerException risk if serializing object with null ID, causing serialization failures.

phoneDesensitization = DesensitizedUtil.mobilePhone(phone);
NoticeReceiverVO currentValue = (NoticeReceiverVO)jsonGenerator.getOutputContext().getCurrentValue();
if (currentValue.getId() != null) {
    desensitizationMapCache.put(currentValue.getId()+"_"+phoneDesensitization, phone);
}
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Robustness

Comment on lines +43 to +44
Copy link

Choose a reason for hiding this comment

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

Phone Desensitization Collision

Cache key uses desensitized phone which creates collision risk when multiple receivers have similar phone numbers that produce identical masked values. This would overwrite cache entries causing data retrieval errors.

NoticeReceiverVO currentValue = (NoticeReceiverVO)jsonGenerator.getOutputContext().getCurrentValue();
// Use a more unique key pattern to prevent collisions
desensitizationMapCache.put(currentValue.getId()+"_phone", phone);
Standards
  • Logic-Verification-Data-Integrity
  • Business-Rule-Unique-Key-Generation
  • Algorithm-Correctness-Cache-Management

Comment on lines +42 to +44
Copy link

Choose a reason for hiding this comment

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

Cache Key Collision

Phone serializer uses the same cache key pattern as email serializer without type distinction. This creates potential key collisions when phone and email desensitize to similar patterns, causing data restoration errors.

phoneDesensitization = DesensitizedUtil.mobilePhone(phone);
NoticeReceiverVO currentValue = (NoticeReceiverVO)jsonGenerator.getOutputContext().getCurrentValue();
// Use more unique key format to prevent collisions
String cacheKey = "phone_" + currentValue.getId() + "_" + phoneDesensitization;
desensitizationMapCache.put(cacheKey, phone);
Standards
  • CWE-664
  • CWE-212
  • Business-Rule-Data-Privacy

Copy link

Choose a reason for hiding this comment

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

Cache Key Collision

Cache key construction using desensitized value creates potential collision risk. Different phone numbers with same desensitized pattern could overwrite each other in cache. This would lead to incorrect information being restored during edit operations.

            if (currentValue.getId() != null) {
                // Use original phone in key to prevent collisions from different phones with same desensitized pattern
                desensitizationMapCache.put(currentValue.getId() + "_phone_" + phone, phone);
            }
Commitable Suggestion
Suggested change
desensitizationMapCache.put(currentValue.getId()+"_"+phoneDesensitization, phone);
if (currentValue.getId() != null) {
// Use original phone in key to prevent collisions from different phones with same desensitized pattern
desensitizationMapCache.put(currentValue.getId() + "_phone_" + phone, phone);
}
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness
  • DbC-Data-Integrity

Comment on lines +43 to +44
Copy link

Choose a reason for hiding this comment

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

Missing Null Check

No null check on currentValue or its getId() method before using in cache key. NullPointerException could occur if the context value is null or doesn't have a valid ID, causing serialization failure and potential data loss.

Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness
  • DbC-Defensive-Programming

Comment on lines +42 to +44
Copy link

Choose a reason for hiding this comment

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

Cache Security Risk

Sensitive phone number data is stored in a cache without expiration controls or access restrictions. This creates a potential data exposure risk if the cache is compromised or accessed by unauthorized code. The original sensitive data remains accessible through the cache indefinitely.

Standards
  • CWE-524
  • OWASP-A04
  • NIST-SSDF-PW.1

}

jsonGenerator.writeString(phoneDesensitization);
Comment on lines +36 to +47
Copy link

Choose a reason for hiding this comment

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

Null Safety Improvement

Similar to the email serializer, this code assumes currentValue is always a NoticeReceiverVO and never null. Missing null checks could cause NullPointerException during phone desensitization.

@Override
public void serialize(String phone, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) throws IOException {
    String phoneDesensitization = "";
    if (StrUtil.isNotBlank(phone)) {
        phoneDesensitization = DesensitizedUtil.mobilePhone(phone);
        Object currentObj = jsonGenerator.getOutputContext().getCurrentValue();
        if (currentObj instanceof NoticeReceiverVO) {
            NoticeReceiverVO currentValue = (NoticeReceiverVO) currentObj;
            if (currentValue.getId() != null) {
                CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache();
                desensitizationMapCache.put(currentValue.getId() + "_" + phoneDesensitization, phone);
            }
        }
    }
    jsonGenerator.writeString(phoneDesensitization);
}
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • CWE-704
  • CWE-755

}
}
Comment on lines +35 to +49
Copy link

Choose a reason for hiding this comment

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

Duplicate Cache Logic

Similar caching logic is duplicated across Email and Phone serializers. This violates DRY principle and makes future changes error-prone as modifications must be synchronized across multiple classes.

public abstract class AbstractDesensitizationSerializer<T> extends JsonSerializer<String> {

    protected abstract String desensitize(String value);

    @Override
    public void serialize(String value, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) throws IOException {
        String desensitized = "";
        CommonCacheService<String, Object> desensitizationMapCache = CacheFactory.getDesensitizationMapCache();
        if (StrUtil.isNotBlank(value)) {
            desensitized = desensitize(value);
            NoticeReceiverVO currentValue = (NoticeReceiverVO) jsonGenerator.getOutputContext().getCurrentValue();
            desensitizationMapCache.put(currentValue.getId() + "_" + desensitized, value);
        }
        jsonGenerator.writeString(desensitized);
    }
}

public class EmailDesensitizationSerializer extends AbstractDesensitizationSerializer<String> {
    @Override
    protected String desensitize(String email) {
        int index = StrUtil.indexOf(email, '@');
        return index <= 1 ? email : StrUtil.replace(email, 1, index, '*');
    }
}

public class PhoneDesensitizationSerializer extends AbstractDesensitizationSerializer<String> {
    @Override
    protected String desensitize(String phone) {
        return DesensitizedUtil.mobilePhone(phone);
    }
}
Standards
  • SOLID-DRY
  • Clean-Code-Duplication
  • Design-Pattern-Template-Method

Loading