Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ jobs:
AWS_SECRET_KEY: dummy
AWS_REGION: ap-northeast-2
AWS_S3_BUCKET: dummy-bucket
SPRING_JWT_PRIVATE_KEY: file:/tmp/test-keys/private_key.pem
SPRING_JWT_PUBLIC_KEY: file:/tmp/test-keys/public_key.pem
run: ./gradlew test

- name: Upload test report
Expand Down
14 changes: 14 additions & 0 deletions src/main/java/chainsawman/gesture/controller/AuthController.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,20 @@ public ResponseEntity<ApiResponse<Void>> deleteRefreshToken(@RequestBody @Valid
return ResponseEntity.ok(ApiResponse.ok("삭제되었습니다."));
}

@PostMapping("/email-send")
@Operation(summary = "이메일 인증 발송", description = "이메일 인증 코드를 발송합니다.")
public ResponseEntity<ApiResponse<Void>> sendEmailVerification(@RequestBody @Valid EmailSendRequest request) {
authService.sendEmailVerification(request);
return ResponseEntity.ok(ApiResponse.ok("인증 코드가 발송되었습니다."));
}

@PostMapping("/email-verification")
@Operation(summary = "이메일 인증 확인", description = "이메일 인증 코드를 검증합니다.")
public ResponseEntity<ApiResponse<EmailVerificationResponse>> verifyEmail(@RequestBody @Valid EmailVerificationRequest request) {
EmailVerificationResponse result = authService.verifyEmail(request);
return ResponseEntity.ok(ApiResponse.ok(result));
}




Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package chainsawman.gesture.dto.user.request;

import jakarta.validation.constraints.Email;
import jakarta.validation.constraints.NotBlank;
import lombok.Getter;

@Getter
public class EmailSendRequest {

@NotBlank
@Email
private String email;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package chainsawman.gesture.dto.user.request;

import jakarta.validation.constraints.Email;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.Pattern;
import lombok.Getter;

@Getter
public class EmailVerificationRequest {

@NotBlank
@Email
private String email;

@NotBlank
@Pattern(regexp = "\\d{6}")
private String code;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package chainsawman.gesture.dto.user.response;

import com.fasterxml.jackson.annotation.JsonFormat;
import lombok.Builder;
import lombok.Getter;

import java.time.LocalDateTime;

@Getter
@Builder
public class EmailVerificationResponse {

private boolean verified;
private String email;

@JsonFormat(pattern = "yyyy-MM-dd'T'HH:mm:ss")
private LocalDateTime verifiedAt;
}
41 changes: 41 additions & 0 deletions src/main/java/chainsawman/gesture/service/AuthService.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
import chainsawman.gesture.entity.user.RefreshToken;
import chainsawman.gesture.entity.user.User;
import chainsawman.gesture.enums.ProviderType;
import chainsawman.gesture.exceptions.auth.ExpiredVerificationCodeException;
import chainsawman.gesture.exceptions.auth.InvalidRefreshTokenException;
import chainsawman.gesture.exceptions.auth.InvalidVerificationCodeException;
import chainsawman.gesture.exceptions.user.DeactivatedUserException;
import chainsawman.gesture.exceptions.user.DuplicateEmailException;
import chainsawman.gesture.exceptions.user.DuplicateIdException;
Expand All @@ -16,23 +18,32 @@
import chainsawman.gesture.global.TokenProvider;
import chainsawman.gesture.repository.user.RefreshTokenRepository;
import chainsawman.gesture.repository.user.UserRepository;
import org.springframework.data.redis.core.StringRedisTemplate;
import org.springframework.transaction.annotation.Transactional;
import lombok.RequiredArgsConstructor;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.stereotype.Service;

import java.security.SecureRandom;
import java.time.LocalDateTime;
import java.util.Optional;
import java.util.concurrent.TimeUnit;

@Service
@RequiredArgsConstructor
@Transactional
public class AuthService {
private static final String EMAIL_VERIFICATION_PREFIX = "email:verification:";
private static final long VERIFICATION_TTL_MINUTES = 5;
private static final SecureRandom SECURE_RANDOM = new SecureRandom();

private final UserRepository userRepository;
private final PasswordEncoder passwordEncoder;
private final TokenProvider tokenProvider;
private final MediaService mediaService;
private final RefreshTokenRepository refreshTokenRepository;
private final StringRedisTemplate redisTemplate;
private final EmailService emailService;


// 로그인
Expand Down Expand Up @@ -205,6 +216,36 @@ public void deleteRefreshToken(RefreshTokenRequest request) {



// 이메일 인증 코드 발송
public void sendEmailVerification(EmailSendRequest request) {
String code = String.format("%06d", SECURE_RANDOM.nextInt(1_000_000));
String key = EMAIL_VERIFICATION_PREFIX + request.getEmail();
redisTemplate.opsForValue().set(key, code, VERIFICATION_TTL_MINUTES, TimeUnit.MINUTES);
emailService.sendVerificationCode(request.getEmail(), code);
Comment on lines +223 to +224

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 | 🟠 Major | ⚡ Quick win

Avoid leaving verification codes in Redis when email delivery fails.

At Line 223-224, code is persisted before the external send. If send fails, a valid code remains without user delivery.

Proposed fix
     public void sendEmailVerification(EmailSendRequest request) {
         String code = String.format("%06d", SECURE_RANDOM.nextInt(1_000_000));
         String key = EMAIL_VERIFICATION_PREFIX + request.getEmail();
-        redisTemplate.opsForValue().set(key, code, VERIFICATION_TTL_MINUTES, TimeUnit.MINUTES);
-        emailService.sendVerificationCode(request.getEmail(), code);
+        redisTemplate.opsForValue().set(key, code, VERIFICATION_TTL_MINUTES, TimeUnit.MINUTES);
+        try {
+            emailService.sendVerificationCode(request.getEmail(), code);
+        } catch (RuntimeException e) {
+            redisTemplate.delete(key);
+            throw e;
+        }
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/chainsawman/gesture/service/AuthService.java` around lines 223
- 224, The code stores the verification code in Redis via
redisTemplate.opsForValue().set(key, code, VERIFICATION_TTL_MINUTES,
TimeUnit.MINUTES) before calling
emailService.sendVerificationCode(request.getEmail(), code), which can leave a
valid code in Redis if email delivery fails; change the flow so the email is
sent first and only on successful return persist the code, or if you must set
before send, wrap emailService.sendVerificationCode(...) in a try/catch and
remove the key (redisTemplate.delete(key)) on any failure (or propagate the
error) so a usable code is not left in Redis; update the logic around key, code,
VERIFICATION_TTL_MINUTES, and request.getEmail() accordingly.

}

// 이메일 인증 코드 확인
public EmailVerificationResponse verifyEmail(EmailVerificationRequest request) {
String key = EMAIL_VERIFICATION_PREFIX + request.getEmail();
String stored = redisTemplate.opsForValue().get(key);

if (stored == null) {
throw new ExpiredVerificationCodeException();
}
if (!stored.equals(request.getCode())) {
throw new InvalidVerificationCodeException();
}
Comment on lines +232 to +237

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 | 🟠 Major | 🏗️ Heavy lift

Add attempt limiting to prevent OTP brute force.

Line 232-237 allows unlimited verification attempts for a 6-digit code during TTL. This is a security gap on an unauthenticated endpoint.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/chainsawman/gesture/service/AuthService.java` around lines 232
- 237, The verification block in AuthService allows unlimited OTP guesses; add
attempt limiting by tracking and incrementing a per-code attempt counter stored
alongside the OTP (e.g., in the same Redis/kv store) before validating. Define a
MAX_ATTEMPTS constant (e.g., 5), increment the counter key (e.g.,
verificationAttemptsKey for the same request identifier) atomically, and if the
count exceeds MAX_ATTEMPTS throw a new TooManyAttemptsException and
invalidate/delete the stored code; otherwise proceed to compare
stored.equals(request.getCode()) and reset/delete both the code and attempts on
successful verification or when expired. Ensure the new counter respects the
original TTL (set its expiry to the OTP TTL) and use the AuthService method
handling verification to locate and implement these changes.


redisTemplate.delete(key);

LocalDateTime verifiedAt = LocalDateTime.now();
return EmailVerificationResponse.builder()
.verified(true)
.email(request.getEmail())
.verifiedAt(verifiedAt)
.build();
}

// 리프레시 토큰 저장 (있으면 업데이트 없으면 생성)
private void saveRefreshToken(User user, String refreshToken) {
LocalDateTime expiresAt = LocalDateTime.now().plusDays(7);
Expand Down
38 changes: 38 additions & 0 deletions src/main/java/chainsawman/gesture/service/EmailService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package chainsawman.gesture.service;

import org.springframework.beans.factory.annotation.Value;
import org.springframework.http.*;
import org.springframework.stereotype.Service;
import org.springframework.web.client.RestTemplate;

import java.util.List;
import java.util.Map;

@Service
public class EmailService {

private static final String BREVO_API_URL = "https://api.brevo.com/v3/smtp/email";

@Value("${brevo.api.key}")
private String apiKey;

@Value("${brevo.template.verification-id}")
private int templateId;

private final RestTemplate restTemplate = new RestTemplate();

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 | 🟠 Major | ⚡ Quick win

Configure HTTP timeouts for Brevo calls to avoid request-thread hangs.

Line 22 and Line 36 currently use a default RestTemplate without explicit connect/read timeouts. A slow/unresponsive Brevo call can block auth requests for too long.

Proposed fix
 `@Service`
 public class EmailService {
@@
-    private final RestTemplate restTemplate = new RestTemplate();
+    private final RestTemplate restTemplate;
+
+    public EmailService(RestTemplateBuilder restTemplateBuilder) {
+        this.restTemplate = restTemplateBuilder
+                .setConnectTimeout(Duration.ofSeconds(3))
+                .setReadTimeout(Duration.ofSeconds(5))
+                .build();
+    }
@@
-        restTemplate.postForEntity(BREVO_API_URL, new HttpEntity<>(body, headers), String.class);
+        restTemplate.postForEntity(BREVO_API_URL, new HttpEntity<>(body, headers), String.class);

Also applies to: 36-36

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/chainsawman/gesture/service/EmailService.java` at line 22, The
RestTemplate in EmailService is created without timeouts causing potential
thread hangs; replace the direct new RestTemplate() (field restTemplate and any
other plain RestTemplate uses around line 36) with a RestTemplate configured via
a ClientHttpRequestFactory that sets explicit connect and read timeouts (e.g.,
using HttpComponentsClientHttpRequestFactory or SimpleClientHttpRequestFactory),
and initialize restTemplate with that factory so Brevo HTTP calls have bounded
connect/read timeouts (choose reasonable timeout values like a few seconds).


public void sendVerificationCode(String toEmail, String code) {
HttpHeaders headers = new HttpHeaders();
headers.set("api-key", apiKey);
headers.set("accept", "application/json");
headers.setContentType(MediaType.APPLICATION_JSON);

Map<String, Object> body = Map.of(
"to", List.of(Map.of("email", toEmail)),
"templateId", templateId,
"params", Map.of("code", code)
);

restTemplate.postForEntity(BREVO_API_URL, new HttpEntity<>(body, headers), String.class);
}
Comment on lines +24 to +37

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 | 🟠 Major | ⚡ Quick win

Wrap Brevo client failures into a domain exception.

Line 36 can throw transport/4xx/5xx exceptions directly. Convert them to a consistent domain error so API clients get stable error contracts.

Proposed fix
+import org.springframework.web.client.RestClientException;
@@
     public void sendVerificationCode(String toEmail, String code) {
@@
-        restTemplate.postForEntity(BREVO_API_URL, new HttpEntity<>(body, headers), String.class);
+        try {
+            restTemplate.postForEntity(BREVO_API_URL, new HttpEntity<>(body, headers), String.class);
+        } catch (RestClientException e) {
+            throw new RuntimeException("이메일 전송에 실패했습니다."); // replace with project DomainException
+        }
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/chainsawman/gesture/service/EmailService.java` around lines 24
- 37, The sendVerificationCode method currently lets RestTemplate transport and
HTTP errors bubble up; wrap the call to restTemplate.postForEntity inside a
try/catch that catches HttpStatusCodeException and RestClientException and
rethrows a domain-specific unchecked exception (e.g., EmailSendException or
EmailServiceException) from EmailService with a clear message, the HTTP
status/body when available, and the original exception as the cause so API
clients receive a stable error contract; add the new exception class if missing
and update sendVerificationCode to throw it on failures.

}
4 changes: 4 additions & 0 deletions src/main/resources/application-prod.properties
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,7 @@ springdoc.swagger-ui.enabled=false

# redis
spring.data.redis.host=gesture-redis

# mail (Brevo API)
brevo.api.key=${BREVO_API_KEY}
brevo.template.verification-id=${BREVO_TEMPLATE_VERIFICATION_ID}
3 changes: 2 additions & 1 deletion src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,5 @@ aws.s3.bucket=${AWS_S3_BUCKET}
aws.region=${AWS_REGION}

# redis
spring.data.redis.port=6379
spring.data.redis.port=6379

20 changes: 20 additions & 0 deletions src/test/resources/application.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
spring.datasource.url=jdbc:h2:mem:testdb;MODE=MySQL
spring.datasource.driver-class-name=org.h2.Driver
spring.datasource.username=sa
spring.datasource.password=

spring.jpa.hibernate.ddl-auto=create-drop
spring.jpa.properties.hibernate.dialect=org.hibernate.dialect.H2Dialect

spring.jwt.private-key=classpath:keys/private_key.pem
spring.jwt.public-key=classpath:keys/public_key.pem

aws.region=ap-northeast-2
aws.s3.bucket=test-bucket
cloud.aws.stack.auto=false
aws.disable-ec2-metadata=true

spring.data.redis.host=localhost

brevo.api.key=test-dummy-key
brevo.template.verification-id=1
Loading