Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 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
2 changes: 2 additions & 0 deletions src/main/java/com/mycom/socket/GoSocketBeApplication.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.scheduling.annotation.EnableScheduling;

@EnableScheduling
@SpringBootApplication
public class GoSocketBeApplication {

Expand Down
32 changes: 28 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,23 @@
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 com.mycom.socket.auth.service.RateLimiter;
import com.mycom.socket.global.exception.BaseException;
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.http.HttpStatus;
import org.springframework.web.bind.annotation.*;

@RestController
@RequestMapping("/api/auth")
@RequiredArgsConstructor
public class AuthController {

private final AuthService authService;
private final MailService mailService;
private final RateLimiter rateLimiter;

@PostMapping("/login")
public LoginResponseDto login(@Valid @RequestBody LoginRequestDto request,
Expand All @@ -34,4 +37,25 @@ public void logout(HttpServletResponse response) {
public Long register(@Valid @RequestBody RegisterRequestDto request) {
return authService.register(request);
}

@PostMapping("/email/verification")
public Boolean mailSend(@RequestParam(name = "mail") String mail) {
try {
rateLimiter.checkRateLimit(mail); // ์š”์ฒญ ์ œํ•œ ์ฒดํฌ
return mailService.sendMail(mail);
} catch (Exception e) {
throw new BaseException("์ด๋ฉ”์ผ ์ „์†ก์— ์‹คํŒจํ–ˆ์Šต๋‹ˆ๋‹ค.", HttpStatus.BAD_REQUEST);
}
}
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. @Email ์–ด๋…ธํ…Œ์ด์…˜์œผ๋กœ ์ด๋ฉ”์ผ ํ˜•์‹ ๊ฒ€์ฆ
  2. ํ‘œ์ค€ํ™”๋œ ์‘๋‹ต ํ˜•์‹ ์‚ฌ์šฉ
     @PostMapping("/email/verification")
-    public Boolean mailSend(@RequestParam(name = "mail") String mail) {
+    public ApiResponse<EmailVerificationResponse> mailSend(
+            @RequestParam(name = "mail") @Email(message = "์œ ํšจํ•œ ์ด๋ฉ”์ผ ์ฃผ์†Œ๋ฅผ ์ž…๋ ฅํ•ด์ฃผ์„ธ์š”") String mail) {
         try {
             rateLimiter.checkRateLimit(mail);  // ์š”์ฒญ ์ œํ•œ ์ฒดํฌ
-            return mailService.sendMail(mail);
+            boolean sent = mailService.sendMail(mail);
+            return ApiResponse.success(new EmailVerificationResponse(sent));
         } catch (Exception e) {
             throw new BaseException("์ด๋ฉ”์ผ ์ „์†ก์— ์‹คํŒจํ–ˆ์Šต๋‹ˆ๋‹ค.", HttpStatus.BAD_REQUEST);
         }
     }

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


@GetMapping("/email/verify")
public Boolean mailCheck(@RequestParam(name = "mail") String mail,
@RequestParam(name = "code") String code) {
try {
return mailService.verifyCode(mail, code);
} catch (Exception e) {
throw new BaseException("์ธ์ฆ์ฝ”๋“œ ๊ฒ€์ฆ์— ์‹คํŒจํ–ˆ์Šต๋‹ˆ๋‹ค.", HttpStatus.BAD_REQUEST);
}
}

}
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
131 changes: 131 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,131 @@
package com.mycom.socket.auth.service;

import com.mycom.socket.auth.service.data.VerificationData;
import com.mycom.socket.global.exception.BaseException;
import jakarta.mail.MessagingException;
import jakarta.mail.internet.MimeMessage;
import lombok.RequiredArgsConstructor;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.http.HttpStatus;
import org.springframework.mail.javamail.JavaMailSender;
import org.springframework.stereotype.Service;
import org.springframework.util.StringUtils;

import java.security.SecureRandom;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.regex.Pattern;

@Service
@RequiredArgsConstructor
public class MailService {

private final JavaMailSender javaMailSender;
private final RateLimiter rateLimiter; // ์ธ์ฆ ๋ฒˆํ˜ธ ์š”์ฒญ ์ œํ•œ

private final Map<String, VerificationData> verificationDataMap = new ConcurrentHashMap<>();
private static final String EMAIL_REGEX = "^[a-zA-Z0-9_+&*-]+(?:\\.[a-zA-Z0-9_+&*-]+)*@(?:[a-zA-Z0-9-]+\\.)+[a-zA-Z]{2,7}$";

@Value("${spring.mail.username}")
private String senderEmail;
Comment on lines +22 to +28
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

ํ•„๋“œ ๊ตฌ์กฐ ๊ฐœ์„  ํ•„์š”

๋‘ ๊ฐœ์˜ Map์„ ์‚ฌ์šฉํ•˜๋Š” ๋Œ€์‹  ๋‹จ์ผ Map๊ณผ ๋ณตํ•ฉ ๊ฐ์ฒด๋ฅผ ์‚ฌ์šฉํ•˜๋Š” ๊ฒƒ์ด ๋” ํšจ์œจ์ ์ž…๋‹ˆ๋‹ค.

-    private final Map<String, Integer> verificationCodes = new ConcurrentHashMap<>();
-    private final Map<String, LocalDateTime> expiryTimes = new ConcurrentHashMap<>();
+    private final Map<String, VerificationData> verificationData = new ConcurrentHashMap<>();
+
+    @Value("${mail.verification.expiry-minutes:5}")
+    private int expiryMinutes;
+
+    private record VerificationData(
+        int code,
+        LocalDateTime expiryTime
+    ) {}
๐Ÿ“ 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 JavaMailSender javaMailSender;
private final RateLimiter rateLimiter; // ์ธ์ฆ ๋ฒˆํ˜ธ ์š”์ฒญ ์ œํ•œ
private final Map<String, Integer> verificationCodes = new ConcurrentHashMap<>(); // ์ด๋ฉ”์ผ๋ณ„ ์ธ์ฆ๋ฒˆํ˜ธ ์ €์žฅ
private final Map<String, LocalDateTime> expiryTimes = new ConcurrentHashMap<>(); // ์ด๋ฉ”์ผ๋ณ„ ์ธ์ฆ๋ฒˆํ˜ธ ๋งŒ๋ฃŒ ์‹œ๊ฐ„ ์ €์žฅ
private static final Duration CODE_VALID_DURATION = Duration.ofMinutes(5);
@Value("${spring.mail.username}")
private String senderEmail;
private final JavaMailSender javaMailSender;
private final RateLimiter rateLimiter; // ์ธ์ฆ ๋ฒˆํ˜ธ ์š”์ฒญ ์ œํ•œ
private final Map<String, VerificationData> verificationData = new ConcurrentHashMap<>();
@Value("${mail.verification.expiry-minutes:5}")
private int expiryMinutes;
private record VerificationData(
int code,
LocalDateTime expiryTime
) {}
@Value("${spring.mail.username}")
private String senderEmail;

Comment on lines +23 to +28
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

์ด๋ฉ”์ผ ์œ ํšจ์„ฑ ๊ฒ€์ฆ ๋กœ์ง ์ถ”๊ฐ€ ํ•„์š”

์ด๋ฉ”์ผ ์ฃผ์†Œ์˜ ์œ ํšจ์„ฑ์„ ๊ฒ€์ฆํ•˜๋Š” ๋กœ์ง์ด ์—†์Šต๋‹ˆ๋‹ค. ์ž˜๋ชป๋œ ์ด๋ฉ”์ผ ์ฃผ์†Œ๋กœ ์ธํ•œ ์˜ค๋ฅ˜๋ฅผ ๋ฐฉ์ง€ํ•˜๊ธฐ ์œ„ํ•ด ์œ ํšจ์„ฑ ๊ฒ€์ฆ์ด ํ•„์š”ํ•ฉ๋‹ˆ๋‹ค.

๋‹ค์Œ๊ณผ ๊ฐ™์€ ์œ ํ‹ธ๋ฆฌํ‹ฐ ๋ฉ”์„œ๋“œ๋ฅผ ์ถ”๊ฐ€ํ•˜๋Š” ๊ฒƒ์„ ๊ถŒ์žฅํ•ฉ๋‹ˆ๋‹ค:

private boolean isValidEmail(String email) {
    String emailRegex = "^[A-Za-z0-9+_.-]+@(.+)$";
    return email != null && email.matches(emailRegex);
}


/**
* 6์ž๋ฆฌ ๋‚œ์ˆ˜ ์ธ์ฆ๋ฒˆํ˜ธ ์ƒ์„ฑ
* SecureRandom ์‚ฌ์šฉํ•˜์—ฌ ๋ณด์•ˆ์„ฑ ํ–ฅ์ƒ
* @return 100000~999999 ๋ฒ”์œ„์˜ ์ธ์ฆ๋ฒˆํ˜ธ
*/
private String createVerificationCode() {
// Math.random()์€ ์˜ˆ์ธก ๊ฐ€๋Šฅํ•œ ๋‚œ์ˆ˜๋ฅผ ์ƒ์„ฑํ•  ์ˆ˜ ์žˆ์–ด ๋ณด์•ˆ์— ์ทจ์•ฝ
// SecureRandom์€ ์•”ํ˜ธํ•™์ ์œผ๋กœ ์•ˆ์ „ํ•œ ๋‚œ์ˆ˜๋ฅผ ์ƒ์„ฑํ•˜๋ฏ€๋กœ ์ธ์ฆ๋ฒˆํ˜ธ ์ƒ์„ฑ์— ๋” ์ ํ•ฉ
SecureRandom secureRandom = new SecureRandom();
return String.format("%06d", secureRandom.nextInt(1000000));
}

/**
* ์ธ์ฆ๋ฉ”์ผ ์ƒ์„ฑ
* @param mail ์ˆ˜์‹ ์ž ์ด๋ฉ”์ผ ์ฃผ์†Œ
* @return ์ƒ์„ฑ๋œ ์ธ์ฆ๋ฉ”์ผ
*/
public MimeMessage createMail(String mail, String verificationCode) {
MimeMessage message = javaMailSender.createMimeMessage();
try {
message.setFrom(senderEmail);
message.setRecipients(MimeMessage.RecipientType.TO, mail);
message.setSubject("์ด๋ฉ”์ผ ์ธ์ฆ");
String body = String.format("""
<h3>์š”์ฒญํ•˜์‹  ์ธ์ฆ ๋ฒˆํ˜ธ์ž…๋‹ˆ๋‹ค.</h3>
<h1>%s</h1>
<h3>๊ฐ์‚ฌํ•ฉ๋‹ˆ๋‹ค.</h3>
""", verificationCode);
message.setText(body, "UTF-8", "html");
} catch (MessagingException e) {
throw new BaseException("์ด๋ฉ”์ผ ์ƒ์„ฑ ์ค‘ ์˜ค๋ฅ˜๊ฐ€ ๋ฐœ์ƒํ–ˆ์Šต๋‹ˆ๋‹ค: " + e.getMessage(),
HttpStatus.BAD_REQUEST);
}
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;
}

/**
* ์ด๋ฉ”์ผ ์œ ํšจ์„ฑ ๊ฒ€์‚ฌ
*
* @param email ๊ฒ€์‚ฌํ•  ์ด๋ฉ”์ผ ์ฃผ์†Œ
* @return ์œ ํšจํ•œ ์ด๋ฉ”์ผ ์ฃผ์†Œ์ธ์ง€ ์—ฌ๋ถ€
*/
private boolean isValidEmail(String email) {
return StringUtils.hasText(email) && Pattern.matches(EMAIL_REGEX, email);
}

/**
* ์ธ์ฆ๋ฉ”์ผ ๋ฐœ์†ก ๋ฐ ์ธ์ฆ๋ฒˆํ˜ธ ๋ฐ˜ํ™˜
* @param mail ์ˆ˜์‹ ์ž ์ด๋ฉ”์ผ ์ฃผ์†Œ
* @return ์ƒ์„ฑ๋œ ์ธ์ฆ๋ฒˆํ˜ธ
*/
public boolean sendMail(String mail) {
if (!isValidEmail(mail)) {
throw new BaseException("์œ ํšจํ•˜์ง€ ์•Š์€ ์ด๋ฉ”์ผ ํ˜•์‹์ž…๋‹ˆ๋‹ค.", HttpStatus.BAD_REQUEST);
}

rateLimiter.checkRateLimit(mail);
String verificationCode = createVerificationCode();
verificationDataMap.put(mail, new VerificationData(verificationCode));

MimeMessage message = createMail(mail, verificationCode);
try{
javaMailSender.send(message);
return true;
}catch (Exception e) {
throw new BaseException("์ด๋ฉ”์ผ ๋ฐœ์†ก ์ค‘ ์˜ค๋ฅ˜๊ฐ€ ๋ฐœ์ƒํ–ˆ์Šต๋‹ˆ๋‹ค: " + e.getMessage(), HttpStatus.INTERNAL_SERVER_ERROR);
}
}

/**
* ์ธ์ฆ๋ฒˆํ˜ธ ๊ฒ€์ฆ
* @param email ์ˆ˜์‹ ์ž ์ด๋ฉ”์ผ ์ฃผ์†Œ
* @param code ์‚ฌ์šฉ์ž๊ฐ€ ์ž…๋ ฅํ•œ ์ธ์ฆ๋ฒˆํ˜ธ
* @return ์ธ์ฆ๋ฒˆํ˜ธ ์ผ์น˜ ์—ฌ๋ถ€
*/
public boolean verifyCode(String email, String code) {
if (!isValidEmail(email)){
throw new BaseException("์œ ํšจํ•˜์ง€ ์•Š์€ ์ด๋ฉ”์ผ ํ˜•์‹์ž…๋‹ˆ๋‹ค.", HttpStatus.BAD_REQUEST);
}

if (!StringUtils.hasText(code) || !code.matches("\\d{6}")) {
return false;
}

VerificationData data = verificationDataMap.get(email);

if (data == null || data.isExpired()) {
return false;
}

boolean isVerified = data.code().equals(code);

if (isVerified){
verificationDataMap.remove(email);
}

Comment on lines +93 to +107
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

์ž…๋ ฅ๊ฐ’ ๊ฒ€์ฆ ๊ฐ•ํ™” ํ•„์š”

code ํŒŒ๋ผ๋ฏธํ„ฐ์— ๋Œ€ํ•œ null ์ฒดํฌ๊ฐ€ ๋ˆ„๋ฝ๋˜์—ˆ์Šต๋‹ˆ๋‹ค. ๋˜ํ•œ ์ฝ”๋“œ ํ˜•์‹(์ˆซ์ž 6์ž๋ฆฌ)์— ๋Œ€ํ•œ ๊ฒ€์ฆ๋„ ํ•„์š”ํ•ฉ๋‹ˆ๋‹ค.

๋‹ค์Œ๊ณผ ๊ฐ™์ด ์ˆ˜์ •ํ•˜๋Š” ๊ฒƒ์„ ๊ถŒ์žฅํ•ฉ๋‹ˆ๋‹ค:

     public boolean verifyCode(String email, String code) {
+        if (code == null || !code.matches("\\d{6}")) {
+            return false;
+        }
         VerificationData data = verificationDataMap.get(email);
๐Ÿ“ 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
public boolean verifyCode(String email, String code) {
VerificationData data = verificationDataMap.get(email);
if (data == null || LocalDateTime.now().isAfter(data.expiryTime())) {
return false;
}
boolean isValid = String.valueOf(data.code()).equals(code);
if (isValid) {
verificationDataMap.remove(email);
}
return isValid;
public boolean verifyCode(String email, String code) {
if (code == null || !code.matches("\\d{6}")) {
return false;
}
VerificationData data = verificationDataMap.get(email);
if (data == null || LocalDateTime.now().isAfter(data.expiryTime())) {
return false;
}
boolean isValid = String.valueOf(data.code()).equals(code);
if (isValid) {
verificationDataMap.remove(email);
}
return isValid;

return isVerified;
}
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.

}

47 changes: 47 additions & 0 deletions src/main/java/com/mycom/socket/auth/service/RateLimiter.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package com.mycom.socket.auth.service;

import com.mycom.socket.global.exception.BaseException;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
import org.springframework.scheduling.annotation.Scheduled;
import org.springframework.stereotype.Component;

import java.time.Duration;
import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

@Component
@RequiredArgsConstructor
public class RateLimiter {
private final Map<String, List<LocalDateTime>> requestMap = new ConcurrentHashMap<>();
private static final int MAX_REQUESTS = 3; // 1๋ถ„๋‹น ์ตœ๋Œ€ 3๋ฒˆ
private static final Duration WINDOW_SIZE = Duration.ofMinutes(1); // 1๋ถ„์˜ ์‹œ๊ฐ„ ๊ฐ„๊ฒฉ
Comment on lines +19 to +21
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

๋ฉ”๋ชจ๋ฆฌ ๋ˆ„์ˆ˜ ๊ฐ€๋Šฅ์„ฑ ๊ฐœ์„  ํ•„์š”

requestMap์— ์ €์žฅ๋œ ์ด๋ฉ”์ผ ์ฃผ์†Œ์™€ ์š”์ฒญ ์‹œ๊ฐ„์ด ์ง€์†์ ์œผ๋กœ ๋ˆ„์ ๋˜์–ด ๋ฉ”๋ชจ๋ฆฌ ๋ˆ„์ˆ˜๊ฐ€ ๋ฐœ์ƒํ•  ์ˆ˜ ์žˆ์Šต๋‹ˆ๋‹ค. ์ฃผ๊ธฐ์ ์œผ๋กœ ์˜ค๋ž˜๋œ ๋ฐ์ดํ„ฐ๋ฅผ ์ •๋ฆฌํ•˜๋Š” ๋ฉ”์ปค๋‹ˆ์ฆ˜์ด ํ•„์š”ํ•ฉ๋‹ˆ๋‹ค.

๋‹ค์Œ๊ณผ ๊ฐ™์€ ๊ฐœ์„ ์„ ์ œ์•ˆํ•ฉ๋‹ˆ๋‹ค:

 private final Map<String, List<LocalDateTime>> requestMap = new ConcurrentHashMap<>();
 private static final int MAX_REQUESTS = 3;  // 1๋ถ„๋‹น ์ตœ๋Œ€ 3๋ฒˆ
 private static final Duration WINDOW_SIZE = Duration.ofMinutes(1);  // 1๋ถ„์˜ ์‹œ๊ฐ„ ๊ฐ„๊ฒฉ
+private static final Duration CLEANUP_INTERVAL = Duration.ofHours(1);
+
+@Scheduled(fixedRate = 3600000) // 1์‹œ๊ฐ„๋งˆ๋‹ค ์‹คํ–‰
+public void cleanup() {
+    LocalDateTime threshold = LocalDateTime.now().minus(WINDOW_SIZE);
+    requestMap.entrySet().removeIf(entry ->
+        entry.getValue().stream().allMatch(time -> time.isBefore(threshold)));
+}

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


@Scheduled(fixedRate = 3600000) // 1์‹œ๊ฐ„๋งˆ๋‹ค ์‹คํ–‰
public void cleanup() {
LocalDateTime threshold = LocalDateTime.now().minus(WINDOW_SIZE);
requestMap.entrySet().removeIf(entry ->
entry.getValue().stream().allMatch(time -> time.isBefore(threshold)));
}

public void checkRateLimit(String email) {
List<LocalDateTime> requests = requestMap.computeIfAbsent(email, k -> new ArrayList<>());
LocalDateTime now = LocalDateTime.now();

requests.removeIf(requestTime ->
requestTime.plus(WINDOW_SIZE).isBefore(now));

if (requests.size() >= MAX_REQUESTS) {
LocalDateTime oldestRequest = requests.get(0);
Duration waitTime = WINDOW_SIZE.minus(Duration.between(oldestRequest, now));
throw new BaseException(
String.format("๋„ˆ๋ฌด ๋งŽ์€ ์š”์ฒญ์ž…๋‹ˆ๋‹ค. %d์ดˆ ํ›„์— ๋‹ค์‹œ ์‹œ๋„ํ•ด์ฃผ์„ธ์š”.",waitTime.getSeconds()),
HttpStatus.TOO_MANY_REQUESTS);
}

requests.add(now);
}
Comment on lines +30 to +46
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. ๋‚จ์€ ๋Œ€๊ธฐ ์‹œ๊ฐ„ ์ •๋ณด ์ œ๊ณต
     if (requests.size() >= MAX_REQUESTS) {
-        throw new BaseException("๋„ˆ๋ฌด ๋งŽ์€ ์š”์ฒญ์ž…๋‹ˆ๋‹ค. ์ž ์‹œ ํ›„ ๋‹ค์‹œ ์‹œ๋„ํ•ด์ฃผ์„ธ์š”.", HttpStatus.TOO_MANY_REQUESTS);
+        LocalDateTime oldestRequest = requests.get(0);
+        Duration waitTime = WINDOW_SIZE.minus(Duration.between(oldestRequest, now));
+        throw new BaseException(
+            String.format("๋„ˆ๋ฌด ๋งŽ์€ ์š”์ฒญ์ž…๋‹ˆ๋‹ค. %d์ดˆ ํ›„์— ๋‹ค์‹œ ์‹œ๋„ํ•ด์ฃผ์„ธ์š”.", 
+                waitTime.getSeconds()),
+            HttpStatus.TOO_MANY_REQUESTS);
     }
๐Ÿ“ 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
public void checkRateLimit(String email) {
List<LocalDateTime> requests = requestMap.computeIfAbsent(email, k -> new ArrayList<>());
LocalDateTime now = LocalDateTime.now();
requests.removeIf(requestTime ->
requestTime.plus(WINDOW_SIZE).isBefore(now));
if (requests.size() >= MAX_REQUESTS) {
throw new BaseException("๋„ˆ๋ฌด ๋งŽ์€ ์š”์ฒญ์ž…๋‹ˆ๋‹ค. ์ž ์‹œ ํ›„ ๋‹ค์‹œ ์‹œ๋„ํ•ด์ฃผ์„ธ์š”.", HttpStatus.TOO_MANY_REQUESTS);
}
requests.add(now);
}
public void checkRateLimit(String email) {
List<LocalDateTime> requests = requestMap.computeIfAbsent(email, k -> new ArrayList<>());
LocalDateTime now = LocalDateTime.now();
requests.removeIf(requestTime ->
requestTime.plus(WINDOW_SIZE).isBefore(now));
if (requests.size() >= MAX_REQUESTS) {
LocalDateTime oldestRequest = requests.get(0);
Duration waitTime = WINDOW_SIZE.minus(Duration.between(oldestRequest, now));
throw new BaseException(
String.format("๋„ˆ๋ฌด ๋งŽ์€ ์š”์ฒญ์ž…๋‹ˆ๋‹ค. %d์ดˆ ํ›„์— ๋‹ค์‹œ ์‹œ๋„ํ•ด์ฃผ์„ธ์š”.",
waitTime.getSeconds()),
HttpStatus.TOO_MANY_REQUESTS);
}
requests.add(now);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.mycom.socket.auth.service.data;

import java.time.LocalDateTime;

import java.time.Duration;

public record VerificationData(String code, LocalDateTime expiryTime) {

private static final Duration CODE_VALID_DURATION = Duration.ofMinutes(5);

public VerificationData(String code) {
this(code, LocalDateTime.now().plus(CODE_VALID_DURATION));
}

public boolean isExpired() {
return LocalDateTime.now().isAfter(expiryTime);
}
}