Conversation
|
""" 워크스루이 풀 리퀘스트는 애플리케이션의 이메일 검증 및 인증 시스템을 Redis 기반으로 재설계하는 주요 변경 사항을 포함합니다. 기존의 메모리 내 접근 방식에서 Redis를 활용한 확장 가능하고 영구적인 솔루션으로 전환하여 이메일 검증, 속도 제한, 토큰 관리 등의 기능을 개선했습니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant Client
participant AuthController
participant MailService
participant RedisService
Client->>AuthController: 이메일 검증 요청
AuthController->>MailService: 검증 코드 생성 및 전송
MailService->>RedisService: 검증 코드 저장
RedisService-->>MailService: 코드 저장 확인
MailService->>Client: 검증 코드 전송
Client->>AuthController: 검증 코드 확인
AuthController->>MailService: 코드 검증 요청
MailService->>RedisService: 코드 유효성 확인
RedisService-->>MailService: 코드 유효성 결과
MailService-->>AuthController: 검증 결과
AuthController-->>Client: 검증 응답
관련 가능성 있는 이슈
관련 가능성 있는 PR
토끼의 시
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (3)
src/main/java/com/mycom/socket/auth/jwt/JWTUtil.java (1)
Line range hint
66-74: 이메일 추출 시 예외 처리가 필요합니다.토큰이 유효하지 않을 경우 발생할 수 있는 예외에 대한 처리가 필요합니다.
public String getEmail(String token) { + try { return Jwts.parser() .verifyWith(secretKey) .requireIssuer(jwtProperties.getIssuer()) .build() .parseSignedClaims(token) .getPayload() .getSubject(); + } catch (Exception e) { + log.error("토큰에서 이메일을 추출하는 중 오류 발생", e); + throw new IllegalArgumentException("유효하지 않은 토큰입니다.", e); + } }src/main/java/com/mycom/socket/auth/jwt/JWTFilter.java (2)
Line range hint
52-65: 사용자 상태 검증 로직 추가 필요현재 구현에서는 사용자의 활성 상태나 계정 만료 여부 등을 확인하지 않고 있습니다. 다음 검증들을 추가하는 것이 좋습니다:
- 계정 활성화 상태 확인
- 계정 잠금 상태 확인
- 비밀번호 만료 여부 확인
private void setAuthentication(String token) { String email = jwtUtil.getEmail(token); UserDetails userDetails = memberDetailsService.loadUserByUsername(email); + if (!userDetails.isEnabled() || !userDetails.isAccountNonLocked() + || !userDetails.isAccountNonExpired() || !userDetails.isCredentialsNonExpired()) { + throw new IllegalStateException("계정 상태가 유효하지 않습니다"); + } + UsernamePasswordAuthenticationToken authentication = new UsernamePasswordAuthenticationToken( userDetails,
쿠키 보안 설정이 일부 누락되어 있습니다
현재 구현된 보안 설정:
- ✅ HttpOnly 플래그 설정됨
- ✅ Secure 플래그 설정됨 (properties 통해 구성 가능)
- ✅ Path가 "/" 로 설정됨
보완이 필요한 설정:
- SameSite 속성이 설정되어 있지 않습니다 (CSRF 공격 방지를 위해 'Strict' 또는 'Lax' 설정 권장)
- 쿠키의 도메인 제한이 설정되어 있지 않습니다 (의도하지 않은 도메인과의 쿠키 공유 방지 필요)
🔗 Analysis chain
Line range hint
41-50: 쿠키 보안 설정을 확인해주세요JWT 토큰을 쿠키에 저장할 때 다음 보안 설정들을 고려해야 합니다:
- HttpOnly 플래그 설정
- Secure 플래그 설정 (HTTPS 환경)
- SameSite 속성 설정
- 적절한 도메인 및 경로 설정
쿠키 생성 부분의 보안 설정을 확인하기 위해 다음 스크립트를 실행해주세요:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check cookie security settings in the codebase # Search for cookie creation/configuration rg -l 'new Cookie|cookie\.set' # Search for specific security settings rg 'setHttpOnly|setSecure|setSameSite'Length of output: 793
Script:
#!/bin/bash # Check complete cookie configuration rg -A 5 'new Cookie' src/main/java/com/mycom/socket/auth/security/CookieUtil.java # Check JWT properties configuration fd -e java -e yml -e properties -e yaml | xargs rg -l 'cookie|Cookie'Length of output: 1121
🧹 Nitpick comments (8)
src/main/java/com/mycom/socket/auth/jwt/JWTUtil.java (3)
Line range hint
20-25: 생성자에 유효성 검사를 추가하는 것이 좋겠습니다.JWTProperties와 secret 값이 null이 아닌지 확인하는 검증 로직을 추가하면 좋겠습니다.
public JWTUtil(JWTProperties jwtProperties) { + if (jwtProperties == null || jwtProperties.getSecret() == null) { + throw new IllegalArgumentException("JWTProperties와 secret은 null이 될 수 없습니다."); + } this.jwtProperties = jwtProperties; this.secretKey = Keys.hmacShaKeyFor( jwtProperties.getSecret().getBytes(StandardCharsets.UTF_8) ); }
Line range hint
29-42: 토큰 생성 로깅을 추가하면 좋겠습니다.디버깅과 모니터링을 위해 토큰 생성 시점에 로그를 추가하는 것이 좋겠습니다.
public String createToken(String email) { + log.debug("JWT 토큰 생성 시작 - 이메일: {}", email); Date now = new Date(); Date validity = new Date(now.getTime() + (jwtProperties.getAccessTokenValidityInSeconds() * 1000)); - return Jwts.builder() + String token = Jwts.builder() .issuer(jwtProperties.getIssuer()) .subject(email) .issuedAt(now) .expiration(validity) .signWith(secretKey) .compact(); + log.debug("JWT 토큰 생성 완료 - 만료시간: {}", validity); + return token; }
Line range hint
47-61: 예외 처리를 더 구체화하면 좋겠습니다.현재는 모든 예외를 하나로 처리하고 있는데, 구체적인 예외 유형별로 처리하면 디버깅에 도움이 될 것 같습니다.
public boolean validateToken(String token) { try { if (!StringUtils.hasText(token)) { + log.warn("토큰이 비어있거나 null입니다."); return false; } Jwts.parser() .verifyWith(secretKey) .requireIssuer(jwtProperties.getIssuer()) .build() .parseSignedClaims(token); return true; - } catch (Exception e) { - log.warn("JWT 토큰 검증 실패", e); + } catch (ExpiredJwtException e) { + log.warn("만료된 JWT 토큰입니다.", e); + return false; + } catch (MalformedJwtException e) { + log.warn("잘못된 형식의 JWT 토큰입니다.", e); + return false; + } catch (Exception e) { + log.warn("JWT 토큰 검증 중 예상치 못한 오류 발생", e); return false; } }src/main/java/com/mycom/socket/auth/jwt/JWTFilter.java (1)
Line range hint
33-37: 로깅 개선이 필요합니다현재 로그 메시지("인증 처리 실패")는 너무 간단합니다. 다음 정보들을 포함하여 로깅을 개선해주세요:
- 실패한 요청의 URI
- 실패 원인 구분 (토큰 누락, 토큰 만료, 검증 실패 등)
- 요청 식별자 (request ID)
- log.warn("인증 처리 실패", e); + log.warn("인증 처리 실패 - URI: {}, 원인: {}, Request ID: {}", + request.getRequestURI(), + e.getMessage(), + request.getAttribute("requestId"));src/main/java/com/mycom/socket/auth/service/MailService.java (1)
30-30: SecureRandom 인스턴스 재사용 권장
createVerificationCode()메서드에서new SecureRandom()를 매번 생성하고 있습니다. 성능 향상을 위해 SecureRandom 인스턴스를 클래스 변수로 선언하여 재사용하는 것이 좋습니다.+private static final SecureRandom secureRandom = new SecureRandom(); private String createVerificationCode() { - return String.format("%06d", new SecureRandom().nextInt(1000000)); + return String.format("%06d", secureRandom.nextInt(1000000)); }src/main/java/com/mycom/socket/global/config/RedisConfig.java (1)
23-30: RedisTemplate 직렬화 설정 개선 필요현재 값 직렬화가 StringRedisSerializer로 설정되어 있어 문자열만 저장 가능합니다. 객체 저장을 위해 Jackson2JsonRedisSerializer 사용을 고려해보세요.
@Bean public RedisTemplate<?, ?> redisTemplate() { RedisTemplate<String, Object> redisTemplate = new RedisTemplate<>(); redisTemplate.setConnectionFactory(redisConnectionFactory()); redisTemplate.setKeySerializer(new StringRedisSerializer()); - redisTemplate.setValueSerializer(new StringRedisSerializer()); + redisTemplate.setValueSerializer(new Jackson2JsonRedisSerializer<>(Object.class)); return redisTemplate; }src/main/java/com/mycom/socket/global/service/RedisService.java (1)
82-85: 이메일 인증 확인 로직 개선 필요문자열 비교 대신 Boolean 타입을 사용하는 것이 더 적절합니다.
public boolean isEmailVerified(String email) { Object verified = redisTemplate.opsForValue().get(VERIFIED_EMAIL_PREFIX + email); - return "true".equals(verified); + return Boolean.TRUE.equals(verified); }build.gradle (1)
49-49: Redis 의존성 버전 명시 권장Spring Boot parent에서 상속받은 버전을 사용하고 있습니다. 명시적인 버전 관리를 위해 버전을 지정하는 것이 좋습니다.
- implementation 'org.springframework.boot:spring-boot-starter-data-redis' + implementation 'org.springframework.boot:spring-boot-starter-data-redis:3.3.4'
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
build.gradle(1 hunks)src/main/java/com/mycom/socket/auth/config/JWTProperties.java(1 hunks)src/main/java/com/mycom/socket/auth/config/MailProperties.java(1 hunks)src/main/java/com/mycom/socket/auth/config/SecurityConfig.java(0 hunks)src/main/java/com/mycom/socket/auth/controller/AuthController.java(1 hunks)src/main/java/com/mycom/socket/auth/jwt/JWTFilter.java(1 hunks)src/main/java/com/mycom/socket/auth/jwt/JWTUtil.java(1 hunks)src/main/java/com/mycom/socket/auth/security/CookieUtil.java(1 hunks)src/main/java/com/mycom/socket/auth/service/MailService.java(4 hunks)src/main/java/com/mycom/socket/auth/service/RateLimiter.java(0 hunks)src/main/java/com/mycom/socket/global/config/RedisConfig.java(1 hunks)src/main/java/com/mycom/socket/global/config/RedisProperties.java(1 hunks)src/main/java/com/mycom/socket/global/service/RedisService.java(1 hunks)src/test/java/com/mycom/socket/member/controller/AuthControllerTest.java(1 hunks)src/test/java/com/mycom/socket/member/service/LoginIntegrationTest.java(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/com/mycom/socket/auth/config/SecurityConfig.java
- src/main/java/com/mycom/socket/auth/service/RateLimiter.java
✅ Files skipped from review due to trivial changes (4)
- src/main/java/com/mycom/socket/auth/config/JWTProperties.java
- src/test/java/com/mycom/socket/member/controller/AuthControllerTest.java
- src/main/java/com/mycom/socket/auth/security/CookieUtil.java
- src/test/java/com/mycom/socket/member/service/LoginIntegrationTest.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-test
🔇 Additional comments (5)
src/main/java/com/mycom/socket/auth/jwt/JWTUtil.java (1)
3-3: 구성이 잘 정리되었습니다!JWTProperties를 config 패키지로 이동하고 final 필드로 선언한 것이 좋은 설계 결정입니다.
Also applies to: 17-18
src/main/java/com/mycom/socket/auth/controller/AuthController.java (1)
46-49: Redis 통합 검증이 필요합니다.엔드포인트 이름이
/verification-code로 변경된 것은 좋은 개선입니다. 하지만 Redis 통합과 관련된 부분을 검증해야 합니다.다음 사항들을 확인해주세요:
- Redis 연동이 올바르게 구현되었는지
- 이메일 인증 코드의 만료 시간이 설정되었는지
- 동시성 처리가 적절히 되어있는지
src/main/java/com/mycom/socket/auth/jwt/JWTFilter.java (1)
3-3: 패키지 구조 개선이 잘 되었습니다!JWTProperties를 config 패키지로 이동한 것은 관심사의 분리 원칙을 잘 따른 좋은 변경입니다.
src/main/java/com/mycom/socket/global/config/RedisProperties.java (1)
1-16: RedisProperties 구성 확인
RedisProperties클래스가 Redis 설정을 위한 필요한 속성을 적절히 매핑하고 있습니다.@ConfigurationProperties와@Component애너테이션을 사용하여 설정 파일로부터 값을 주입받는 방식이 올바르게 적용되었습니다.src/main/java/com/mycom/socket/auth/config/MailProperties.java (1)
1-22: MailProperties 구성 확인
MailProperties클래스는 메일 전송에 필요한 다양한 속성들을 포함하고 있으며,@ConfigurationProperties를 통해spring.mail프리픽스의 설정을 올바르게 매핑하고 있습니다. 메일 발송에 필요한 정보를 효율적으로 관리할 수 있을 것으로 보입니다.
| @PostMapping("/verify-email") | ||
| public EmailVerificationResponse sendVerificationEmail(@Valid @RequestBody EmailRequest request) { | ||
| return mailService.sendMail(request.email()); | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
API 문서화가 필요합니다
엔드포인트 변경사항(/verify-email)이 문서화되어 있지 않은 것으로 보입니다. 다음 사항들을 추가해주시기 바랍니다:
- API 엔드포인트에 대한 명세 문서 작성
- 클라이언트 코드에서 새로운 엔드포인트 사용 여부 확인
- README.md에 API 사용 방법 추가
🔗 Analysis chain
엔드포인트 이름이 개선되었습니다!
새로운 엔드포인트 이름 /verify-email이 기능을 더 명확하게 설명합니다.
API 문서가 업데이트되었는지 확인하기 위해 다음 스크립트를 실행하세요:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# API 문서에서 이전 엔드포인트 참조 확인
rg -i "/verification" --type md
rg -i "/verify-email" --type md
Length of output: 63
Script:
#!/bin/bash
# Search for API documentation files
echo "=== Documentation files ==="
fd -e yaml -e yml -e json . | grep -i "api\|swagger\|openapi"
echo -e "\n=== Searching for endpoint references ==="
# Search in all text files for both old and new endpoints
rg -t txt -t yaml -t yml -t json -t java "/verification|/verify-email"
echo -e "\n=== README files ==="
fd README
Length of output: 407
| if (redisService.incrementCount(email) > 3) { | ||
| throw new BaseException("너무 많은 요청입니다. 1분 후에 다시 시도해주세요.", | ||
| HttpStatus.TOO_MANY_REQUESTS); | ||
| } |
There was a problem hiding this comment.
요청 횟수 제한에 대한 만료 시간 설정 필요
redisService.incrementCount(email)를 통해 이메일별 요청 횟수를 증가시키지만, 현재 요청 횟수를 초기화하거나 만료시키는 로직이 없습니다. 이로 인해 사용자가 영구적으로 이메일 전송을 할 수 없게 될 수 있습니다. 요청 횟수에 대한 Redis 키에 TTL(만료 시간)을 설정하여 일정 시간 후에 자동으로 초기화되도록 수정해야 합니다.
| message.setFrom(mailProperties.getUsername()); | ||
| message.setRecipients(MimeMessage.RecipientType.TO, email); | ||
| message.setSubject("이메일 인증"); | ||
| String body = String.format(""" | ||
| <h3>요청하신 인증 번호입니다.</h3> | ||
| <h1>%s</h1> | ||
| <h3>감사합니다.</h3> | ||
| """, verificationCode); | ||
| String body = String.format(mailProperties.getBodyTemplate(), verificationCode); |
There was a problem hiding this comment.
발신 이메일 주소 설정 수정 필요
현재 message.setFrom(mailProperties.getUsername());로 설정되어 있는데, 이는 인증에 사용되는 사용자 이름입니다. 실제 발신자 이메일 주소를 설정하려면 mailProperties.getSenderEmail()을 사용하는 것이 적절합니다.
-message.setFrom(mailProperties.getUsername());
+message.setFrom(mailProperties.getSenderEmail());Committable suggestion skipped: line range outside the PR's diff.
| try { | ||
| redisService.getCode(code); // 인증코드 검증 | ||
| redisService.saveVerifiedEmail(email); // 인증된 이메일 저장 | ||
| return EmailVerificationResponse.of("이메일 인증이 완료되었습니다."); | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
인증 코드 검증 로직 보완 필요
인증 코드 검증 시 redisService.getCode(code);로 코드만을 이용하여 검증하고 있습니다. 이는 인증 코드와 이메일이 매핑되지 않아, 다른 사용자의 인증 코드를 악용할 수 있는 보안 문제가 발생할 수 있습니다. 인증 코드를 저장할 때 이메일과 함께 저장하고, 검증 시에도 해당 이메일에 저장된 코드를 가져와 비교하도록 수정해야 합니다.
-public EmailVerificationResponse verifyCode(String email, String code) {
if (!code.matches("\\d{6}")) {
throw new BaseException("유효하지 않은 인증 코드 형식입니다.", HttpStatus.BAD_REQUEST);
}
try {
- redisService.getCode(code); // 인증코드 검증
+ String savedCode = redisService.getCode(email); // 이메일로부터 코드 조회
+ if (!code.equals(savedCode)) {
+ throw new BaseException("인증 코드가 일치하지 않습니다.", HttpStatus.BAD_REQUEST);
+ }
redisService.saveVerifiedEmail(email); // 인증된 이메일 저장
return EmailVerificationResponse.of("이메일 인증이 완료되었습니다.");
} catch (Exception e) {
throw new BaseException("인증 코드가 일치하지 않습니다.", HttpStatus.BAD_REQUEST);
}
}Committable suggestion skipped: line range outside the PR's diff.
| public LettuceConnectionFactory redisConnectionFactory() { | ||
| return new LettuceConnectionFactory(redisProperties.getHost(), redisProperties.getPort()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Redis 연결 설정 개선 필요
연결 시간 초과 및 풀 설정이 누락되어 있습니다. 프로덕션 환경에서는 다음 설정들을 추가하는 것이 좋습니다:
- 연결 시간 초과 설정
- 연결 풀 설정
@Bean
public LettuceConnectionFactory redisConnectionFactory() {
- return new LettuceConnectionFactory(redisProperties.getHost(), redisProperties.getPort());
+ LettuceClientConfiguration clientConfig = LettuceClientConfiguration.builder()
+ .commandTimeout(Duration.ofSeconds(1))
+ .build();
+ RedisStandaloneConfiguration serverConfig = new RedisStandaloneConfiguration(
+ redisProperties.getHost(),
+ redisProperties.getPort()
+ );
+ return new LettuceConnectionFactory(serverConfig, clientConfig);
}📝 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.
| public LettuceConnectionFactory redisConnectionFactory() { | |
| return new LettuceConnectionFactory(redisProperties.getHost(), redisProperties.getPort()); | |
| } | |
| public LettuceConnectionFactory redisConnectionFactory() { | |
| LettuceClientConfiguration clientConfig = LettuceClientConfiguration.builder() | |
| .commandTimeout(Duration.ofSeconds(1)) | |
| .build(); | |
| RedisStandaloneConfiguration serverConfig = new RedisStandaloneConfiguration( | |
| redisProperties.getHost(), | |
| redisProperties.getPort() | |
| ); | |
| return new LettuceConnectionFactory(serverConfig, clientConfig); | |
| } |
| private static final String VERIFIED_EMAIL_PREFIX = "verified:email:"; | ||
| private static final String RATE_LIMIT_PREFIX = "rate-limit:"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Redis 키 접두사 체계 개선 필요
인증 코드에 대한 키 접두사가 누락되어 있습니다. 키 충돌을 방지하기 위해 모든 키에 접두사를 추가하는 것이 좋습니다.
private static final String VERIFIED_EMAIL_PREFIX = "verified:email:";
private static final String RATE_LIMIT_PREFIX = "rate-limit:";
+private static final String VERIFICATION_CODE_PREFIX = "verification:code:";📝 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.
| private static final String VERIFIED_EMAIL_PREFIX = "verified:email:"; | |
| private static final String RATE_LIMIT_PREFIX = "rate-limit:"; | |
| private static final String VERIFIED_EMAIL_PREFIX = "verified:email:"; | |
| private static final String RATE_LIMIT_PREFIX = "rate-limit:"; | |
| private static final String VERIFICATION_CODE_PREFIX = "verification:code:"; |
| public void saveCode(String code) { | ||
| redisTemplate.opsForValue().set(code, code, VERIFICATION_TTL); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
인증 코드 저장 로직 개선 필요
인증 코드 저장 시 접두사를 사용하지 않아 키 충돌이 발생할 수 있습니다.
public void saveCode(String code) {
- redisTemplate.opsForValue().set(code, code, VERIFICATION_TTL);
+ redisTemplate.opsForValue().set(VERIFICATION_CODE_PREFIX + code, code, VERIFICATION_TTL);
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/main/java/com/mycom/socket/global/service/RedisService.java (2)
19-28: Redis 관련 상수값들의 구성 개선 필요상수값들을 코드에서 직접 정의하는 대신 설정 파일로 분리하는 것이 좋습니다. 이를 통해 환경별로 다른 값을 적용하거나 운영 중에 값을 변경하기 쉬워집니다.
+@Value("${redis.ttl.verification:180}") // 3분 +private int verificationTtlSeconds; +@Value("${redis.ttl.verified-email:1800}") // 30분 +private int verifiedEmailTtlSeconds; +@Value("${redis.ttl.rate-limit:60}") // 1분 +private int rateLimitTtlSeconds; -private static final Duration VERIFICATION_TTL = Duration.ofMinutes(3); -private static final Duration VERIFIED_EMAIL_TTL = Duration.ofMinutes(30); -private static final Duration RATE_LIMIT_TTL = Duration.ofMinutes(1); +private Duration getVerificationTtl() { + return Duration.ofSeconds(verificationTtlSeconds); +}
71-86: 이메일 인증 상태 저장 방식 개선 필요Redis에 문자열로 boolean 값을 저장하는 것보다 Redis의 기능을 활용하는 것이 좋습니다.
public void saveVerifiedEmail(String email) { - redisTemplate.opsForValue().set( - VERIFIED_EMAIL_PREFIX + email, - "true", - VERIFIED_EMAIL_TTL - ); + redisTemplate.opsForValue().set(VERIFIED_EMAIL_PREFIX + email, "", VERIFIED_EMAIL_TTL); } public boolean isEmailVerified(String email) { - Object verified = redisTemplate.opsForValue().get(VERIFIED_EMAIL_PREFIX + email); - return "true".equals(verified); + return Boolean.TRUE.equals(redisTemplate.hasKey(VERIFIED_EMAIL_PREFIX + email)); }src/main/java/com/mycom/socket/auth/service/MailService.java (1)
28-31: 인증 코드 생성 로직 개선 필요SecureRandom 인스턴스를 매번 생성하는 것은 비효율적입니다. 또한 현재 구현은 1,000,000으로 나누지 않아 7자리 숫자가 생성될 수 있습니다.
+private static final SecureRandom secureRandom = new SecureRandom(); private String createVerificationCode() { - return String.format("%06d", new SecureRandom().nextInt(1000000)); + return String.format("%06d", secureRandom.nextInt(900000) + 100000); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/mycom/socket/auth/service/MailService.java(4 hunks)src/main/java/com/mycom/socket/global/config/RedisConfig.java(1 hunks)src/main/java/com/mycom/socket/global/service/RedisService.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/mycom/socket/global/config/RedisConfig.java
🔇 Additional comments (3)
src/main/java/com/mycom/socket/global/service/RedisService.java (1)
58-65: 요청 횟수 제한 로직 보완 필요최대 요청 횟수 제한이 설정되어 있지 않아 무한정 요청이 가능합니다. 또한 race condition이 발생할 수 있습니다.
public Long incrementCount(String email) { String key = RATE_LIMIT_PREFIX + email; Long count = redisTemplate.opsForValue().increment(key); if (count == 1) { redisTemplate.expire(key, RATE_LIMIT_TTL); } + if (count > 3) { // PR 목표: 1분당 3회로 제한 + throw new BaseException("너무 많은 요청이 발생했습니다. 잠시 후 다시 시도해주세요.", HttpStatus.TOO_MANY_REQUESTS); + } return count; }src/main/java/com/mycom/socket/auth/service/MailService.java (2)
63-70:⚠️ Potential issue이메일 발송 로직 개선 필요
인증 코드 저장 시 이메일 정보가 누락되어 있어 보안 문제가 발생할 수 있습니다.
if (redisService.incrementCount(email) > 3) { throw new BaseException("너무 많은 요청입니다. 1분 후에 다시 시도해주세요.", HttpStatus.TOO_MANY_REQUESTS); } String verificationCode = createVerificationCode(); -redisService.saveCode(verificationCode); +redisService.saveCode(email, verificationCode);Likely invalid or redundant comment.
89-99:⚠️ Potential issue인증 코드 검증 로직 개선 필요
이메일과 인증 코드의 매핑이 누락되어 있어 보안 문제가 발생할 수 있습니다. 또한 인증 성공 시 이메일 인증 상태를 저장하는 로직이 누락되어 있습니다.
try { - String saveCode = redisService.getCode(code); // 인증코드 검증 - if(!saveCode.equals(code)) { + String savedCode = redisService.getCode(email); // 이메일로 인증코드 조회 + if(!savedCode.equals(code)) { throw new BaseException("인증 코드가 일치하지 않습니다.", HttpStatus.BAD_REQUEST); } + redisService.saveVerifiedEmail(email); // 이메일 인증 완료 처리 return EmailVerificationResponse.of("이메일 인증이 완료되었습니다."); } catch (Exception e) { throw new BaseException("인증 코드가 일치하지 않습니다.", HttpStatus.BAD_REQUEST); }Likely invalid or redundant comment.
| public void saveCode(String code) { | ||
| redisTemplate.opsForValue().set(VERIFICATION_CODE_PREFIX +code, code, VERIFICATION_TTL); | ||
| } |
There was a problem hiding this comment.
인증 코드 저장 시 이메일 연동 필요
현재 구현은 인증 코드만을 저장하고 있어, 다른 사용자의 인증 코드를 사용할 수 있는 보안 취약점이 있습니다. 인증 코드를 이메일과 함께 저장하여 검증 시 특정 이메일에 대한 인증 코드인지 확인해야 합니다.
-public void saveCode(String code) {
- redisTemplate.opsForValue().set(VERIFICATION_CODE_PREFIX + code, code, VERIFICATION_TTL);
+public void saveCode(String email, String code) {
+ redisTemplate.opsForValue().set(VERIFICATION_CODE_PREFIX + email, code, VERIFICATION_TTL);
}📝 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.
| public void saveCode(String code) { | |
| redisTemplate.opsForValue().set(VERIFICATION_CODE_PREFIX +code, code, VERIFICATION_TTL); | |
| } | |
| public void saveCode(String email, String code) { | |
| redisTemplate.opsForValue().set(VERIFICATION_CODE_PREFIX + email, code, VERIFICATION_TTL); | |
| } |
| public String getCode(String code) { | ||
| Object savedCode = redisTemplate.opsForValue().get(code); | ||
| if (savedCode == null) { | ||
| throw new BaseException("인증 코드가 만료되었거나 존재하지 않습니다.", HttpStatus.BAD_REQUEST); | ||
| } | ||
| return savedCode.toString(); | ||
| } |
There was a problem hiding this comment.
인증 코드 조회 로직 개선 필요
이메일과 코드의 매핑을 위해 조회 메서드의 파라미터와 로직을 수정해야 합니다.
-public String getCode(String code) {
- Object savedCode = redisTemplate.opsForValue().get(code);
+public String getCode(String email) {
+ Object savedCode = redisTemplate.opsForValue().get(VERIFICATION_CODE_PREFIX + email);
if (savedCode == null) {
throw new BaseException("인증 코드가 만료되었거나 존재하지 않습니다.", HttpStatus.BAD_REQUEST);
}
return savedCode.toString();
}📝 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.
| public String getCode(String code) { | |
| Object savedCode = redisTemplate.opsForValue().get(code); | |
| if (savedCode == null) { | |
| throw new BaseException("인증 코드가 만료되었거나 존재하지 않습니다.", HttpStatus.BAD_REQUEST); | |
| } | |
| return savedCode.toString(); | |
| } | |
| public String getCode(String email) { | |
| Object savedCode = redisTemplate.opsForValue().get(VERIFICATION_CODE_PREFIX + email); | |
| if (savedCode == null) { | |
| throw new BaseException("인증 코드가 만료되었거나 존재하지 않습니다.", HttpStatus.BAD_REQUEST); | |
| } | |
| return savedCode.toString(); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/com/mycom/socket/global/service/RedisService.java (1)
36-38: 이미 인증된 이메일에 대한 인증 코드 발송 방지
saveCode메서드에서 이미 인증된 이메일에 대해 인증 코드를 재발송하지 않도록 이메일의 인증 상태를 확인하고, 인증된 경우 예외를 발생시키는 로직을 추가하는 것이 좋습니다.아래와 같이 코드를 수정할 수 있습니다:
public void saveCode(String email, String code) { if (isEmailVerified(email)) { throw new BaseException("이미 인증된 이메일입니다.", HttpStatus.BAD_REQUEST); } redisTemplate.opsForValue().set(VERIFICATION_CODE_PREFIX + email, code, VERIFICATION_TTL); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/mycom/socket/global/service/RedisService.java(1 hunks)
🔇 Additional comments (1)
src/main/java/com/mycom/socket/global/service/RedisService.java (1)
58-65: 요청 횟수 제한 로직 보완 필요
incrementCount메서드에서 최대 요청 횟수 제한이 적용되지 않아 사용자가 무제한으로 요청을 보낼 수 있습니다. PR 목표에 명시된 "1분당 최대 3회의 요청 제한"을 구현하기 위해, 요청 횟수가 제한을 초과하면 예외를 발생시키는 로직을 추가해야 합니다.이전 리뷰에서 제안한 코드를 참고하여 아래와 같이 수정할 수 있습니다:
public Long incrementCount(String email) { String key = RATE_LIMIT_PREFIX + email; Long count = redisTemplate.opsForValue().increment(key); if (count == 1) { redisTemplate.expire(key, RATE_LIMIT_TTL); } if (count > 3) { throw new BaseException("요청 가능 횟수를 초과했습니다. 잠시 후 다시 시도해주세요.", HttpStatus.TOO_MANY_REQUESTS); } return count; }
| public Long incrementCount(String email) { | ||
| String key = RATE_LIMIT_PREFIX + email; | ||
| Long count = redisTemplate.opsForValue().increment(key); | ||
| if (count == 1) { | ||
| redisTemplate.expire(key, RATE_LIMIT_TTL); | ||
| } | ||
| return count; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
동시성 문제 해결을 위한 원자적 연산 사용 권장
incrementCount 메서드에서 여러 스레드가 동시에 접근할 경우, race condition이 발생하여 요청 횟수 제한 로직이 올바르게 작동하지 않을 수 있습니다. Redis의 원자적 명령어 또는 Lua 스크립트를 활용하여 동시성 문제를 해결하는 것이 좋습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/com/mycom/socket/global/service/RedisService.java (2)
54-73: Rate Limiting 로직 개선이 필요합니다.현재 구현은 기본적인 동시성 처리를 포함하고 있지만, Lua 스크립트를 사용하면 더 안전한 원자적 연산이 가능합니다.
다음과 같이 Lua 스크립트를 사용하여 개선할 수 있습니다:
public Long incrementCount(String email) { String key = RATE_LIMIT_PREFIX + email; - return redisTemplate.execute( - (RedisConnection connection) -> { - Long count = connection.incr(key.getBytes()); - if (count == 1) { - connection.expire(key.getBytes(), RATE_LIMIT_TTL.getSeconds()); - } - return count; - } - ); + String script = "local count = redis.call('incr', KEYS[1]) " + + "if count == 1 then " + + " redis.call('expire', KEYS[1], ARGV[1]) " + + "end " + + "return count"; + return redisTemplate.execute( + new DefaultRedisScript<>(script, Long.class), + Collections.singletonList(key), + RATE_LIMIT_TTL.getSeconds() + ); }
75-95: 이메일 인증 상태 관리 기능 확장이 필요합니다.현재 구현은 기본적인 기능을 잘 수행하고 있지만, 인증 상태를 제거하는 기능이 누락되어 있습니다.
다음 메소드 추가를 제안합니다:
/** * 이메일 인증 상태 제거 * 사용자가 재인증을 요청하거나 인증을 취소할 때 사용 */ public void removeVerifiedEmail(String email) { redisTemplate.delete(VERIFIED_EMAIL_PREFIX + email); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/mycom/socket/global/service/RedisService.java(1 hunks)
🔇 Additional comments (3)
src/main/java/com/mycom/socket/global/service/RedisService.java (3)
1-16: 클래스 구조가 적절합니다!Spring과 Lombok 어노테이션을 올바르게 사용하고 있으며, 생성자 주입 패턴이 잘 적용되어 있습니다.
17-30: 상수 정의가 개선되었습니다!이전 리뷰에서 지적된 키 접두사 문제가 해결되었으며, TTL 설정도 적절합니다.
31-52: 인증 코드 저장 및 조회 로직이 개선되었습니다!이전 리뷰에서 지적된 이메일-코드 매핑 문제가 해결되었으며, 예외 처리도 적절합니다.
1 participant
📌 관련 이슈
#26 이메일 인증 기능 Redis 도입
✨ 과제 내용
📸 스크린샷(선택)
📚 레퍼런스 (또는 새로 알게 된 내용) 혹은 궁금한 사항들
Summary by CodeRabbit
Summary by CodeRabbit
새로운 기능
버그 수정
리팩토링