Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
c5b7666
:sparkles: 이메일 인증 기능 의존성 추가
ohyuchan123 Jan 7, 2025
8a35e5b
:sparkles: Email 인증 번호 전송 및 체크 기능 구현
ohyuchan123 Jan 7, 2025
b4a5657
:recycle: 이메일 인증 번호 전달 방식 리팩토링(조금더 보안적으로)
ohyuchan123 Jan 8, 2025
92944df
:sparkles: 이메일 인증 번호 여러번 요청 제한 (1분당 최대 3번)
ohyuchan123 Jan 8, 2025
3915305
:recycle: 지금은 스케줄러 방식을 통해서 인메모리를 제거 추후 Redis를 통해서 개선 필요
ohyuchan123 Jan 8, 2025
d6f20ad
:recycle: 인증번호 만료 시간 지정
ohyuchan123 Jan 8, 2025
5fca75a
:recycle: 인증번호 요청 많은 시 발생하는 예외처리 상세화
ohyuchan123 Jan 8, 2025
b40a9e5
:recycle: VerificationData Record를 통해서 더 효율적으로 코드 관리
ohyuchan123 Jan 8, 2025
0bae0de
:recycle: 코드 구조 개선
ohyuchan123 Jan 8, 2025
0bb17dc
:fire: 불필요한 import 문 제거
ohyuchan123 Jan 8, 2025
13cd90f
:fire: 불필요한 gitkeep 제거
ohyuchan123 Jan 8, 2025
1da4aef
:recycle: ApiResponse 전송 방식 양식 변경
ohyuchan123 Jan 8, 2025
579d964
:recycle: 예외 처리 양식 변경
ohyuchan123 Jan 8, 2025
ed0d689
:recycle: 이메일 인증 방식 Request 와 Response Dto 방식으로 코드 개선
ohyuchan123 Jan 8, 2025
8aa701d
:fire: 불필요한 import 제거
ohyuchan123 Jan 8, 2025
9991a3d
:recycle: 예외처리 로직 개선
ohyuchan123 Jan 8, 2025
89a7f63
:recycle: 이메일 인증 로직 개선 Get -> Post
ohyuchan123 Jan 8, 2025
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
2 changes: 2 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,11 @@ dependencies {
// Security
implementation 'org.springframework.boot:spring-boot-starter-security'
testImplementation 'org.springframework.security:spring-security-test'
implementation 'org.springframework.boot:spring-boot-starter-mail'
implementation 'io.jsonwebtoken:jjwt-api:0.12.3'
implementation 'io.jsonwebtoken:jjwt-impl:0.12.3'
implementation 'io.jsonwebtoken:jjwt-jackson:0.12.3'

}

test {
Expand Down
24 changes: 20 additions & 4 deletions src/main/java/com/mycom/socket/auth/controller/AuthController.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

컨트롤러의 상태 저장 방식 개선 필요

인증 번호를 컨트롤러 인스턴스 변수로 저장하는 것은 다음과 같은 문제가 있습니다:

  1. 여러 사용자의 인증 요청을 처리할 수 없습니다.
  2. 서버가 여러 인스턴스로 실행될 경우 동작하지 않습니다.

인증 번호 저장소를 서비스 계층으로 이동하고 Redis와 같은 분산 캐시를 사용하는 것을 권장합니다.

-    private int number;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private final MailService mailService;
private int number;
private final MailService mailService;


@PostMapping("/login")
public LoginResponseDto login(@Valid @RequestBody LoginRequestDto request,
Expand All @@ -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());
}
}
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jan 7, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

이메일 전송 엔드포인트 개선 필요

다음과 같은 개선이 필요합니다:

  1. 응답 형식이 표준화되어 있지 않습니다.
  2. 입력값 검증이 부족합니다.
  3. 예외 처리가 구체적이지 않습니다.
     @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);
         }
     }

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


@GetMapping("/mail-check")
public Boolean mailCheck(@RequestParam(name = "userNumber") String userNumber) {
return userNumber.equals(String.valueOf(number));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

인증 번호 확인 엔드포인트 개선 필요

다음과 같은 보안 및 기능 개선이 필요합니다:

  1. 응답 형식이 표준화되어 있지 않습니다.
  2. 인증 시도 횟수 제한이 없습니다.
  3. 인증 만료 시간 설정이 없습니다.
     @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));
     }

Committable suggestion skipped: line range outside the PR's diff.


}
4 changes: 0 additions & 4 deletions src/main/java/com/mycom/socket/auth/service/AuthService.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ public LoginResponseDto login(LoginRequestDto request, HttpServletResponse respo
);
}

// 이메일 인증 코드 전송

// 이메일 인증 코드 만료

@Transactional
public Long register(RegisterRequestDto request) {
// 이메일 중복 검사
Expand Down
49 changes: 49 additions & 0 deletions src/main/java/com/mycom/socket/auth/service/MailService.java
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;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

정적 변수 사용 및 이메일 주소 하드코딩 문제

다음과 같은 심각한 문제점들이 있습니다:

  1. number를 정적 변수로 사용하면 동시성 문제가 발생할 수 있습니다.
  2. 이메일 주소가 코드에 하드코딩되어 있습니다.

다음과 같이 수정하는 것을 권장합니다:

-    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<>();

Committable suggestion skipped: line range outside the PR's diff.


// 랜덤으로 숫자 생성
public static void createNumber() {
number = (int)(Math.random() * (90000)) + 100000; //(int) Math.random() * (최댓값-최소값+1) + 최소값
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

인증 번호 생성 로직 개선 필요

인증 번호 생성 메서드에 다음과 같은 문제가 있습니다:

  1. 정적 메서드로 구현되어 테스트가 어렵습니다.
  2. 보안성이 낮은 난수 생성 방식을 사용합니다.

다음과 같이 개선하는 것을 권장합니다:

-    public static void createNumber() {
-        number = (int)(Math.random() * (90000)) + 100000;
+    private int createVerificationCode() {
+        SecureRandom secureRandom = new SecureRandom();
+        return 100000 + secureRandom.nextInt(900000);

Committable suggestion skipped: line range outside the PR's diff.


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
Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} catch (MessagingException e) {
throw new BaseException("이메일 생성 중 오류가 발생했습니다.", HttpStatus.BAD_REQUEST);
}
} catch (MessagingException e) {
throw new BaseException("이메일 생성 중 오류가 발생했습니다: " + e.getMessage(),
HttpStatus.BAD_REQUEST, e);
}


return message;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

이메일 생성 로직 개선 필요

다음과 같은 개선이 필요합니다:

  1. 예외 처리가 미흡합니다.
  2. 이메일 주소 유효성 검증이 없습니다.
  3. HTML 템플릿 분리가 필요합니다.
     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;
     }

Committable suggestion skipped: line range outside the PR's diff.


public int sendMail(String mail) {
MimeMessage message = CreateMail(mail);
javaMailSender.send(message);

return number;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

이메일 전송 메서드 개선 필요

다음과 같은 보안 및 안정성 개선이 필요합니다:

  1. 속도 제한(rate limiting) 구현이 필요합니다.
  2. 이메일 전송 실패 시 예외 처리가 미흡합니다.
     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;
     }

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +62 to +109
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

인증 코드 검증 로직 개선 필요

현재 구현에서는 인증 코드 만료 여부를 확인하지 않습니다. 또한, 검증 후에도 코드가 계속 유효한 상태로 남아있습니다.

다음과 같은 개선을 제안합니다:

 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;
 }

Committable suggestion skipped: line range outside the PR's diff.

}