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
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package com.daramg.server.auth.application;

import com.daramg.server.auth.domain.MailMessages;
import com.daramg.server.auth.util.MailContentBuilder;
import com.daramg.server.auth.util.MimeMessageGenerator;
import jakarta.mail.internet.MimeMessage;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.mail.javamail.JavaMailSender;
import org.springframework.scheduling.annotation.Async;
import org.springframework.stereotype.Component;

@Slf4j
@Component
@RequiredArgsConstructor
public class AsyncMailSender {

private final JavaMailSender javaMailSender;
private final MimeMessageGenerator mimeMessageGenerator;
private final MailContentBuilder mailContentBuilder;

@Async("mailTaskExecutor")
public void sendVerificationCode(String email, String verificationCode) {
try {
String htmlContent = mailContentBuilder.buildVerificationEmail(verificationCode);
MimeMessage mimeMessage = mimeMessageGenerator.generate(
email,
MailMessages.MAIL_VERIFICATION_SUBJECT,
htmlContent
);
javaMailSender.send(mimeMessage);
} catch (Exception e) {
log.error("이메일 발송 실패 - email: {}, error: {}", email, e.getMessage());

Choose a reason for hiding this comment

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

security-medium medium

A critical security concern exists here: the user's email address is logged in plain text when email sending fails. Email addresses are Personally Identifiable Information (PII) and should not be logged to avoid privacy violations and compliance issues (e.g., GDPR).

Additionally, while asynchronous processing improves API response time, the current implementation logs exceptions on email sending failure without proper handling. This means the API might return a 200 OK even if the email wasn't sent, potentially confusing users. Furthermore, the current logging only captures e.getMessage(), missing the full stack trace needed for debugging. It's recommended to log the full Throwable directly for better error context.

Suggested change
log.error("이메일 발송 실패 - email: {}, error: {}", email, e.getMessage());
log.error("이메일 발송 실패 - error: {}", e.getMessage());

}
}
}
Original file line number Diff line number Diff line change
@@ -1,34 +1,26 @@
package com.daramg.server.auth.application;

import com.daramg.server.auth.domain.MailMessages;
import com.daramg.server.auth.dto.EmailVerificationRequestDto;
import com.daramg.server.auth.dto.CodeVerificationRequestDto;
import com.daramg.server.auth.exception.AuthErrorStatus;
import com.daramg.server.auth.repository.VerificationCodeRepository;
import com.daramg.server.auth.util.MailContentBuilder;
import com.daramg.server.auth.util.MimeMessageGenerator;
import com.daramg.server.auth.util.VerificationCodeGenerator;
import com.daramg.server.common.exception.BusinessException;
import com.daramg.server.user.exception.UserErrorStatus;
import com.daramg.server.user.repository.UserRepository;
import jakarta.mail.MessagingException;
import jakarta.mail.internet.MimeMessage;

import com.daramg.server.auth.repository.RateLimitRepository;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.data.redis.RedisConnectionFailureException;
import org.springframework.mail.MailException;
import org.springframework.mail.javamail.JavaMailSender;
import org.springframework.stereotype.Service;

@Slf4j
@Service
@RequiredArgsConstructor
public class MailVerificationServiceImpl implements MailVerificationService{

private final MimeMessageGenerator mimeMessageGenerator;
private final MailContentBuilder mailContentBuilder;
private final JavaMailSender javaMailSender;
private final AsyncMailSender asyncMailSender;
private final VerificationCodeRepository verificationCodeRepository;
private final RateLimitRepository rateLimitRepository;
private final UserRepository userRepository;
Expand Down Expand Up @@ -77,19 +69,7 @@ private void sendVerificationCode(EmailVerificationRequestDto request) {
verificationCodeRepository.save(request.getEmail(), verificationCode)
);

try {
String htmlContent = mailContentBuilder.buildVerificationEmail(verificationCode);
MimeMessage mimeMessage = mimeMessageGenerator.generate(
request.getEmail(),
MailMessages.MAIL_VERIFICATION_SUBJECT,
htmlContent
);

javaMailSender.send(mimeMessage);
} catch (MessagingException | MailException | java.io.UnsupportedEncodingException e) {
log.error("이메일 발송 실패 - email: {}, error: {}", request.getEmail(), e.getMessage());
throw new BusinessException(AuthErrorStatus.SEND_VERIFICATION_EMAIL_FAILED);
}
asyncMailSender.sendVerificationCode(request.getEmail(), verificationCode);
}

public void verifyEmailWithCode(CodeVerificationRequestDto request) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
.requestMatchers("/auth/verify-email").permitAll()
.requestMatchers("/auth/signup").permitAll()
.requestMatchers("/auth/login").permitAll()
.requestMatchers(HttpMethod.PUT, "/auth/password-reset").permitAll()
.requestMatchers("/composers").permitAll()
.requestMatchers(HttpMethod.GET, "/posts/**").permitAll()
.requestMatchers("/users/check-nickname").permitAll()
Expand Down
24 changes: 24 additions & 0 deletions src/main/java/com/daramg/server/common/config/AsyncConfig.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.daramg.server.common.config;

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.scheduling.annotation.EnableAsync;
import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor;

import java.util.concurrent.Executor;

@Configuration
@EnableAsync
public class AsyncConfig {

@Bean(name = "mailTaskExecutor")
public Executor mailTaskExecutor() {
ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
executor.setCorePoolSize(2);
executor.setMaxPoolSize(5);
executor.setQueueCapacity(100);
executor.setThreadNamePrefix("mail-");
executor.initialize();
return executor;
}
}
Original file line number Diff line number Diff line change
@@ -1,38 +1,32 @@
package com.daramg.server.auth.application;

import com.daramg.server.auth.domain.EmailPurpose;
import com.daramg.server.auth.dto.CodeVerificationRequestDto;
import com.daramg.server.auth.dto.EmailVerificationRequestDto;
import com.daramg.server.auth.exception.AuthErrorStatus;
import com.daramg.server.auth.repository.RateLimitRepository;
import com.daramg.server.auth.repository.VerificationCodeRepository;
import com.daramg.server.auth.util.MailContentBuilder;
import com.daramg.server.auth.util.MimeMessageGenerator;
import com.daramg.server.common.exception.BusinessException;
import com.daramg.server.user.repository.UserRepository;
import jakarta.mail.internet.MimeMessage;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.mail.javamail.JavaMailSender;

import java.util.Optional;

import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.*;

@ExtendWith(MockitoExtension.class)
class MailVerificationServiceImplTest {

@Mock private MimeMessageGenerator mimeMessageGenerator;
@Mock private MailContentBuilder mailContentBuilder;
@Mock private JavaMailSender javaMailSender;
@Mock private AsyncMailSender asyncMailSender;
@Mock private VerificationCodeRepository verificationCodeRepository;
@Mock private RateLimitRepository rateLimitRepository;
@Mock private UserRepository userRepository;
Expand All @@ -48,16 +42,14 @@ class SendVerificationEmail {

@Test
@DisplayName("새 코드 발급 시 시도 횟수를 초기화하지 않는다")
void 새_코드_발급_시_시도_횟수_초기화_안됨() throws Exception {
void 새_코드_발급_시_시도_횟수_초기화_안됨() {
given(userRepository.existsByEmail(TEST_EMAIL)).willReturn(false);
given(rateLimitRepository.isRateLimited(TEST_EMAIL)).willReturn(false);
given(mailContentBuilder.buildVerificationEmail(anyString())).willReturn("<html>code</html>");
given(mimeMessageGenerator.generate(anyString(), anyString(), anyString()))
.willReturn(mock(MimeMessage.class));

EmailVerificationRequestDto request = new EmailVerificationRequestDto(null, TEST_EMAIL, EmailPurpose.SIGNUP);
mailVerificationService.sendVerificationEmail(request);

verify(asyncMailSender).sendVerificationCode(eq(TEST_EMAIL), anyString());
verify(rateLimitRepository, never()).resetAttempts(TEST_EMAIL);
}

Expand All @@ -72,6 +64,8 @@ class SendVerificationEmail {
assertThatThrownBy(() -> mailVerificationService.sendVerificationEmail(request))
.isInstanceOf(BusinessException.class)
.hasFieldOrPropertyWithValue("errorCode", AuthErrorStatus.EMAIL_RATE_LIMIT_EXCEEDED);

verify(asyncMailSender, never()).sendVerificationCode(anyString(), anyString());
}

@Test
Expand All @@ -84,6 +78,8 @@ class SendVerificationEmail {
assertThatThrownBy(() -> mailVerificationService.sendVerificationEmail(request))
.isInstanceOf(BusinessException.class)
.hasFieldOrPropertyWithValue("errorCode", AuthErrorStatus.DUPLICATE_EMAIL);

verify(asyncMailSender, never()).sendVerificationCode(anyString(), anyString());
}

@Test
Expand All @@ -96,6 +92,8 @@ class SendVerificationEmail {
assertThatThrownBy(() -> mailVerificationService.sendVerificationEmail(request))
.isInstanceOf(BusinessException.class)
.hasFieldOrPropertyWithValue("errorCode", AuthErrorStatus.EMAIL_NOT_REGISTERED);

verify(asyncMailSender, never()).sendVerificationCode(anyString(), anyString());
}
}

Expand All @@ -108,11 +106,8 @@ class VerifyEmailWithCode {
void 인증_성공_시_시도_횟수_초기화() {
given(rateLimitRepository.isAttemptExceeded(TEST_EMAIL)).willReturn(false);
given(verificationCodeRepository.findByEmail(TEST_EMAIL)).willReturn(Optional.of("123456"));
doNothing().when(verificationCodeRepository).deleteByEmail(TEST_EMAIL);
doNothing().when(rateLimitRepository).resetAttempts(TEST_EMAIL);

com.daramg.server.auth.dto.CodeVerificationRequestDto request =
new com.daramg.server.auth.dto.CodeVerificationRequestDto(TEST_EMAIL, "123456");
CodeVerificationRequestDto request = new CodeVerificationRequestDto(TEST_EMAIL, "123456");
mailVerificationService.verifyEmailWithCode(request);

verify(rateLimitRepository).resetAttempts(TEST_EMAIL);
Expand All @@ -123,8 +118,7 @@ class VerifyEmailWithCode {
void 시도_횟수_초과_시_예외_발생() {
given(rateLimitRepository.isAttemptExceeded(TEST_EMAIL)).willReturn(true);

com.daramg.server.auth.dto.CodeVerificationRequestDto request =
new com.daramg.server.auth.dto.CodeVerificationRequestDto(TEST_EMAIL, "123456");
CodeVerificationRequestDto request = new CodeVerificationRequestDto(TEST_EMAIL, "123456");

assertThatThrownBy(() -> mailVerificationService.verifyEmailWithCode(request))
.isInstanceOf(BusinessException.class)
Expand All @@ -137,8 +131,7 @@ class VerifyEmailWithCode {
given(rateLimitRepository.isAttemptExceeded(TEST_EMAIL)).willReturn(false);
given(verificationCodeRepository.findByEmail(TEST_EMAIL)).willReturn(Optional.of("123456"));

com.daramg.server.auth.dto.CodeVerificationRequestDto request =
new com.daramg.server.auth.dto.CodeVerificationRequestDto(TEST_EMAIL, "999999");
CodeVerificationRequestDto request = new CodeVerificationRequestDto(TEST_EMAIL, "999999");

assertThatThrownBy(() -> mailVerificationService.verifyEmailWithCode(request))
.isInstanceOf(BusinessException.class)
Expand Down
Loading