[UPLUS-138] 이메일, SMS 전송 로직 구현, usage-noti request 복호화/마스킹#5
Conversation
…eat/UPLUS-138 # Conflicts: # src/main/java/com/project/notification/infra/entity/enums/Grade.java
github-actions
Bot
added
feat
Summary of ChangesHello @k0081915, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 알림 서비스의 핵심 기능을 확장하여 이메일 및 SMS 전송 로직을 구현하고, Thymeleaf를 이용한 동적 템플릿 렌더링을 지원합니다. 또한, 민감한 사용자 정보를 안전하게 처리하기 위해 복호화 및 마스킹 기능을 도입했으며, 알림 전송 실패 시나리오에 대비한 폴백 메커니즘과 테스트 용이성을 위한 HTTP 기반 트리거를 추가하여 시스템의 안정성과 유연성을 향상시켰습니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 이메일 및 SMS 전송 로직, Thymeleaf를 사용한 템플릿 파싱, 요청 데이터의 복호화 및 마스킹 기능을 구현합니다. 전반적으로 기능 구현의 방향은 좋지만, 몇 가지 중요한 개선점이 필요합니다. 테스트 컨트롤러의 불안전한 타입 캐스팅, MaskingUtil의 null 반환으로 인한 잠재적 NullPointerException, 운영 코드에 포함된 테스트용 실패 시뮬레이션 로직, 그리고 SMS 템플릿 렌더링 시 마스킹되지 않은 변수를 사용하는 심각한 버그가 발견되었습니다. 이러한 문제들은 시스템의 안정성과 보안에 영향을 줄 수 있으므로 수정이 필요합니다.
| if (ThreadLocalRandom.current().nextInt(100) == 0) { | ||
| throw new RuntimeException("Simulated 1% Email Failure"); | ||
| } |
There was a problem hiding this comment.
1% 확률로 이메일 전송을 실패시키는 테스트용 로직이 포함되어 있습니다. 이러한 코드는 테스트 환경에서는 유용할 수 있으나, 운영 환경에 배포될 경우 실제 사용자에게 예기치 않은 오류를 발생시킬 수 있어 매우 위험합니다. 이 로직은 제거하거나, test와 같은 특정 프로필에서만 활성화되도록 조건부로 실행해야 합니다.
| event.templateGroupId()); | ||
|
|
||
| String body = templateEngine.render(template.getBody(), event.variables()); | ||
| String body = messageTemplateEngine.renderText(template.getBody(), event.variables()); |
There was a problem hiding this comment.
SMS 본문을 렌더링할 때 마스킹되지 않은 원본 변수(event.variables())를 사용하고 있습니다. 이는 이메일 전송 로직에서 maskedVariables를 사용하는 것과 일치하지 않으며, 민감한 정보가 마스킹 없이 노출될 수 있는 잠재적인 보안 문제입니다. maskedVariables를 사용하여 템플릿을 렌더링해야 합니다.
| String body = messageTemplateEngine.renderText(template.getBody(), event.variables()); | |
| String body = messageTemplateEngine.renderText(template.getBody(), maskedVariables); |
| Long subId = Long.valueOf((Integer) request.get("subId")); | ||
| String email = (String) request.get("email"); | ||
| String phoneNumber = (String) request.get("phoneNumber"); | ||
| Long templateGroupId = Long.valueOf((Integer) request.get("templateGroupId")); |
There was a problem hiding this comment.
request.get("...")의 결과를 Integer로 캐스팅한 후 Long.valueOf()를 호출하는 것은 ClassCastException을 유발할 수 있습니다. JSON 파서가 숫자를 Integer, Long, Double 등 다양한 타입으로 해석할 수 있기 때문입니다. 더 안전한 방법은 Number 타입으로 캐스팅한 후 longValue()를 호출하는 것입니다.
| Long subId = Long.valueOf((Integer) request.get("subId")); | |
| String email = (String) request.get("email"); | |
| String phoneNumber = (String) request.get("phoneNumber"); | |
| Long templateGroupId = Long.valueOf((Integer) request.get("templateGroupId")); | |
| Long subId = ((Number) request.get("subId")).longValue(); | |
| String email = (String) request.get("email"); | |
| String phoneNumber = (String) request.get("phoneNumber"); | |
| Long templateGroupId = ((Number) request.get("templateGroupId")).longValue(); |
|
|
||
| int at = email.indexOf('@'); | ||
| if (at < 0) { | ||
| return null; |
There was a problem hiding this comment.
maskEmail 메소드는 이메일 주소에 '@' 문자가 없을 경우 null을 반환합니다. 이는 호출하는 쪽에서 null 처리를 해주지 않으면 NullPointerException을 유발할 수 있는 위험한 방식입니다. 다른 유효하지 않은 입력 케이스처럼 ***와 같은 마스킹된 문자열을 반환하도록 수정하는 것이 더 안전합니다.
| return null; | |
| return "***"; |
|
|
||
| /** [테스트용] Kafka 없이 HTTP 요청으로 알림 발송 로직 직접 트리거 */ | ||
| @PostMapping("/test/send-notification") | ||
| public String sendTest(@RequestBody Map<String, Object> request) { |
There was a problem hiding this comment.
요청 본문을 Map<String, Object>로 받는 것은 타입 안전성을 보장하지 않으며, 런타임에 ClassCastException과 같은 예외를 발생시킬 수 있습니다. 또한 API의 명세를 코드만으로 파악하기 어렵게 만듭니다. 별도의 DTO(Data Transfer Object) 클래스를 정의하여 사용하면 코드가 더 견고해지고 가독성이 향상되며, API 명세가 명확해집니다.
| String replacement = valueObj != null ? String.valueOf(valueObj) : ""; | ||
|
|
||
| if (replacement == null) { | ||
| log.warn( | ||
| "Template variable '{}' not found in variables map, replacing with empty" | ||
| + " string", | ||
| variableName); | ||
| replacement = ""; | ||
| } |
There was a problem hiding this comment.
템플릿 변수가 누락되었을 때 처리하는 로직에 결함이 있습니다. replacement 변수는 null이 될 수 없으므로 if (replacement == null) 조건문은 항상 false가 되어, 변수가 누락되어도 경고 로그가 기록되지 않습니다. 변수 조회 결과인 valueObj가 null인지 직접 확인하여 로그를 남기도록 수정해야 합니다.
| String replacement = valueObj != null ? String.valueOf(valueObj) : ""; | |
| if (replacement == null) { | |
| log.warn( | |
| "Template variable '{}' not found in variables map, replacing with empty" | |
| + " string", | |
| variableName); | |
| replacement = ""; | |
| } | |
| String replacement = ""; | |
| if (valueObj != null) { | |
| replacement = String.valueOf(valueObj); | |
| } else { | |
| log.warn( | |
| "Template variable '{}' not found in variables map, replacing with empty" | |
| + " string", | |
| variableName); | |
| } |
2 participants
🍀 이슈 번호
UPLUS-138
✅ 작업 사항
📋 체크리스트
⌨ 기타