-
Notifications
You must be signed in to change notification settings - Fork 0
🔀 Email Send #27
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
🔀 Email Send #27
Changes from 2 commits
c5b7666
8a35e5b
b4a5657
92944df
3915305
d6f20ad
5fca75a
b40a9e5
0bae0de
0bb17dc
13cd90f
1da4aef
579d964
ed0d689
8aa701d
9991a3d
89a7f63
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,20 +4,20 @@ | |
| import com.mycom.socket.auth.dto.request.RegisterRequestDto; | ||
| import com.mycom.socket.auth.dto.response.LoginResponseDto; | ||
| import com.mycom.socket.auth.service.AuthService; | ||
| import com.mycom.socket.auth.service.MailService; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import jakarta.validation.Valid; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.web.bind.annotation.PostMapping; | ||
| import org.springframework.web.bind.annotation.RequestBody; | ||
| import org.springframework.web.bind.annotation.RequestMapping; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
| import org.springframework.web.bind.annotation.*; | ||
|
|
||
| @RestController | ||
| @RequestMapping("/api/auth") | ||
| @RequiredArgsConstructor | ||
| public class AuthController { | ||
|
|
||
| private final AuthService authService; | ||
| private final MailService mailService; | ||
| private int number; | ||
|
|
||
| @PostMapping("/login") | ||
| public LoginResponseDto login(@Valid @RequestBody LoginRequestDto request, | ||
|
|
@@ -34,4 +34,20 @@ public void logout(HttpServletResponse response) { | |
| public Long register(@Valid @RequestBody RegisterRequestDto request) { | ||
| return authService.register(request); | ||
| } | ||
|
|
||
| @PostMapping("/mail-send") | ||
| public Integer mailSend(@RequestParam(name = "mail") String mail) { | ||
| try { | ||
| number = mailService.sendMail(mail); | ||
| return number; | ||
| } catch (Exception e) { | ||
| throw new RuntimeException("Failed to send mail: " + e.getMessage()); | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 이메일 전송 엔드포인트 개선 필요 다음과 같은 개선이 필요합니다:
@PostMapping("/mail-send")
- public Integer mailSend(@RequestParam(name = "mail") String mail) {
+ public ApiResponse<VerificationResponse> mailSend(
+ @RequestParam(name = "mail") @Email(message = "유효한 이메일 주소를 입력해주세요") String mail) {
try {
- number = mailService.sendMail(mail);
- return number;
+ int verificationCode = mailService.sendMail(mail);
+ return ApiResponse.success(new VerificationResponse(verificationCode));
} catch (Exception e) {
- throw new RuntimeException("Failed to send mail: " + e.getMessage());
+ throw new EmailServiceException("이메일 전송에 실패했습니다", e);
}
}
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| @GetMapping("/mail-check") | ||
| public Boolean mailCheck(@RequestParam(name = "userNumber") String userNumber) { | ||
| return userNumber.equals(String.valueOf(number)); | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 인증 번호 확인 엔드포인트 개선 필요 다음과 같은 보안 및 기능 개선이 필요합니다:
@GetMapping("/mail-check")
- public Boolean mailCheck(@RequestParam(name = "userNumber") String userNumber) {
- return userNumber.equals(String.valueOf(number));
+ public ApiResponse<VerificationResult> verifyCode(
+ @RequestParam(name = "mail") @Email String mail,
+ @RequestParam(name = "code") @Pattern(regexp = "^[0-9]{6}$") String code) {
+ boolean isValid = mailService.verifyCode(mail, code);
+ return ApiResponse.success(new VerificationResult(isValid));
}
|
||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,49 @@ | ||||||||||||||||
| package com.mycom.socket.auth.service; | ||||||||||||||||
|
|
||||||||||||||||
| import jakarta.mail.MessagingException; | ||||||||||||||||
| import jakarta.mail.internet.MimeMessage; | ||||||||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||||||||
| import org.springframework.mail.javamail.JavaMailSender; | ||||||||||||||||
| import org.springframework.stereotype.Service; | ||||||||||||||||
|
|
||||||||||||||||
| @Service | ||||||||||||||||
| @RequiredArgsConstructor | ||||||||||||||||
| public class MailService { | ||||||||||||||||
|
|
||||||||||||||||
| private final JavaMailSender javaMailSender; | ||||||||||||||||
| private static final String senderEmail= "oyuchan50@gmail.com"; | ||||||||||||||||
| private static int number; | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 정적 변수 사용 및 이메일 주소 하드코딩 문제 다음과 같은 심각한 문제점들이 있습니다:
다음과 같이 수정하는 것을 권장합니다: - private static final String senderEmail= "oyuchan50@gmail.com";
- private static int number;
+ @Value("${mail.sender.address}")
+ private String senderEmail;
+ private final Map<String, Integer> verificationCodes = new ConcurrentHashMap<>();
|
||||||||||||||||
|
|
||||||||||||||||
| // 랜덤으로 숫자 생성 | ||||||||||||||||
| public static void createNumber() { | ||||||||||||||||
| number = (int)(Math.random() * (90000)) + 100000; //(int) Math.random() * (최댓값-최소값+1) + 최소값 | ||||||||||||||||
| } | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 인증 번호 생성 로직 개선 필요 인증 번호 생성 메서드에 다음과 같은 문제가 있습니다:
다음과 같이 개선하는 것을 권장합니다: - public static void createNumber() {
- number = (int)(Math.random() * (90000)) + 100000;
+ private int createVerificationCode() {
+ SecureRandom secureRandom = new SecureRandom();
+ return 100000 + secureRandom.nextInt(900000);
|
||||||||||||||||
|
|
||||||||||||||||
| public MimeMessage CreateMail(String mail) { | ||||||||||||||||
| createNumber(); | ||||||||||||||||
| MimeMessage message = javaMailSender.createMimeMessage(); | ||||||||||||||||
|
|
||||||||||||||||
| try { | ||||||||||||||||
| message.setFrom(senderEmail); | ||||||||||||||||
| message.setRecipients(MimeMessage.RecipientType.TO, mail); | ||||||||||||||||
| message.setSubject("이메일 인증"); | ||||||||||||||||
| String body = ""; | ||||||||||||||||
| body += "<h3>" + "요청하신 인증 번호입니다." + "</h3>"; | ||||||||||||||||
| body += "<h1>" + number + "</h1>"; | ||||||||||||||||
| body += "<h3>" + "감사합니다." + "</h3>"; | ||||||||||||||||
| message.setText(body,"UTF-8", "html"); | ||||||||||||||||
| } catch (MessagingException e) { | ||||||||||||||||
| e.printStackTrace(); | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+64
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 예외 처리 개선 필요 원인 예외를 포함하여 더 자세한 디버깅이 가능하도록 해야 합니다. - throw new BaseException("이메일 생성 중 오류가 발생했습니다.", HttpStatus.BAD_REQUEST);
+ throw new BaseException("이메일 생성 중 오류가 발생했습니다: " + e.getMessage(),
+ HttpStatus.BAD_REQUEST, e);📝 Committable suggestion
Suggested change
|
||||||||||||||||
|
|
||||||||||||||||
| return message; | ||||||||||||||||
| } | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 이메일 생성 로직 개선 필요 다음과 같은 개선이 필요합니다:
public MimeMessage CreateMail(String mail) {
+ if (!isValidEmail(mail)) {
+ throw new InvalidEmailException("유효하지 않은 이메일 주소입니다.");
+ }
- createNumber();
+ int verificationCode = createVerificationCode();
+ verificationCodes.put(mail, verificationCode);
MimeMessage message = javaMailSender.createMimeMessage();
try {
message.setFrom(senderEmail);
message.setRecipients(MimeMessage.RecipientType.TO, mail);
message.setSubject("이메일 인증");
- String body = "";
- body += "<h3>" + "요청하신 인증 번호입니다." + "</h3>";
- body += "<h1>" + number + "</h1>";
- body += "<h3>" + "감사합니다." + "</h3>";
+ String body = emailTemplateService.getVerificationEmailTemplate(verificationCode);
message.setText(body,"UTF-8", "html");
} catch (MessagingException e) {
- e.printStackTrace();
+ throw new EmailSendException("이메일 생성 중 오류가 발생했습니다.", e);
}
return message;
}
|
||||||||||||||||
|
|
||||||||||||||||
| public int sendMail(String mail) { | ||||||||||||||||
| MimeMessage message = CreateMail(mail); | ||||||||||||||||
| javaMailSender.send(message); | ||||||||||||||||
|
|
||||||||||||||||
| return number; | ||||||||||||||||
| } | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 이메일 전송 메서드 개선 필요 다음과 같은 보안 및 안정성 개선이 필요합니다:
public int sendMail(String mail) {
+ rateLimiter.checkRateLimit(mail);
MimeMessage message = CreateMail(mail);
- javaMailSender.send(message);
+ try {
+ javaMailSender.send(message);
+ return verificationCodes.get(mail);
+ } catch (MailException e) {
+ throw new EmailSendException("이메일 전송 중 오류가 발생했습니다.", e);
+ }
- return number;
}
Comment on lines
+62
to
+109
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 인증 코드 검증 로직 개선 필요 현재 구현에서는 인증 코드 만료 여부를 확인하지 않습니다. 또한, 검증 후에도 코드가 계속 유효한 상태로 남아있습니다. 다음과 같은 개선을 제안합니다: public boolean verifyCode(String email, String code) {
- Integer savedCode = verificationCodes.get(email);
- return savedCode != null && String.valueOf(savedCode).equals(code);
+ VerificationData data = verificationCodes.get(email);
+ if (data == null || LocalDateTime.now().isAfter(data.expiryTime)) {
+ return false;
+ }
+ boolean isValid = String.valueOf(data.code).equals(code);
+ if (isValid) {
+ verificationCodes.remove(email); // 사용된 코드 제거
+ }
+ return isValid;
}
|
||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
컨트롤러의 상태 저장 방식 개선 필요
인증 번호를 컨트롤러 인스턴스 변수로 저장하는 것은 다음과 같은 문제가 있습니다:
인증 번호 저장소를 서비스 계층으로 이동하고 Redis와 같은 분산 캐시를 사용하는 것을 권장합니다.
- private int number;📝 Committable suggestion