인증 코드 메일 전송 및 검증 기능 구현 #32

youngh0 · 2024-02-21T09:35:27Z

👍관련 이슈

close: feat: 인증 메일 전송 기능을 구현한다. #30

🤔세부 내용

아직 api 명세가 안정해져서 로컬에서 포스트맨으로 테스트 했을 때 정상 동작합니다.
인증 코드 생성 시 해당 인증 코드가 어떤 행위 (회원 가입, 비밀번호 찾기) 인지 구분하기 위한 AuthCodeCategory enum 을 생성했습니다.
추후 메일 뿐만 아니라 문자 메시지로 보낼 수도 있기 때문에 이를 구분하기 위한 AuthCodePlatform enum 을 생성했습니다.
인증 코드가 유효한지 확인하는 조건을 아래와 같습니다.
- 인증 코드의 검증을 요청한 사람(destination), AuthCodeCategory, AuthCodePlatform 이 일치하는 것 중 가장 최근의 인증 코드를 조회합니다. -> 없으면 예외 발생 (AuthCodeRepository.findRecentlyAuthCodeBy()) 로직
- 조회된 인증코드와 요청으로 들어온 인증 코드 값이 같은지 확인합니다. -> (AuthCode.certificate()) 로직
- 조회된 인증코드가 사용됐는지 검증합니다. -> (AuthCode.certificate()) 로직
- 위 조건이 모두 통과되면 isUsed 를 true 로 바꿔 사용한 인증코드로 표시하고 검증 완료합니다.

🫵리뷰 중점사항

인증 코드 생성기(AuthCodeGenerator) 의존성

해당 빈을 필드로 주입받을지, 사용하는 메서드의 파라미터로 주입받을지 고민입니다..

AuthService 의 필드 의존

이 경우 테스트가 조금 불편합니다. 인증코드를 연속 2번 발급받으면 이전에 발급받은 인증번호는 사용하지 못하는 경우를 테스트 하기 어렵습니다.
현재는 이를 테스트하기 위해 AuthService.createAuthCode() 에서 발급한 authCode 를 반환하게 하여 테스트하고 있습니다. (테스트에서만 사용)
FixAuthCodeGenerator 를 주입하면 2번 연속 발급받은 인증 코드가 모두 똑같아서 테스트가 불가능합니다.

인증코드 생성하는 AuthService.createAuthCode() 메서드의 파라미터 의존

이 경우, controller 에서 AuthCodeGenerator 를 AuthService 에 주입해줘야 합니다.
- 이 방법을 선택하지 않은 이유는 인증 코드를 어떤 방식으로 생성하는지를 결정하는건 controller의 책임이 아닌 것 같아서 선택하지 않았습니다.

인증 코드 검증 시 동시성 이슈

현재 인증코드 검증 완료 시 AuthCode.isUsed 를 update 하는 형식으로 진행하고 있는데요. 여기서 동시성 이슈가 발생할 수 있을것같아요.
동일한 인증 코드로 검증 요청이 2번 연속으로 왔을 때 상황인데요.
1. 먼저 온 인증 요청이 isUsed 가 true 인지 확인 -> false 라 검증 통과, 아직 isUsed true 로 못바꿈
2. 나중에 온 인증 요청도 isUsed 가 true 인지 확인 -> false 라 검증 통과
그래서 비관적 락으로 잡으려고 하는데 어떻게 생각하시나여??

This2sho

고생하셨습니다 레오씨
처음 구현하는건데 잘하셨네유
확인하고 리뷰 남겼으니 봐주세여

This2sho · 2024-02-25T17:48:44Z

src/main/java/com/example/parking/auth/authcode/infrastructure/AuthCodeSender.java

+
+public interface AuthCodeSender {
+
+    public void send(String destination, String authCode);


Suggested change

public void send(String destination, String authCode);

void send(String destination, String authCode);

This2sho · 2024-02-25T17:49:01Z

src/main/java/com/example/parking/auth/authcode/infrastructure/MailAuthCodeSender.java

+    @Override
+    public void send(String destination, String authCode) {
+        SimpleMailMessage mailMessage = new SimpleMailMessage();
+


어디서 보냈는지는 설정안해도 되나여?

그러게요 .!!?

sender나 title 같은거는 안써줘도될까요 ?

받는입장에서 뭔가 당황스러울수도..?

This2sho · 2024-02-25T17:52:37Z

src/main/java/com/example/parking/auth/authcode/event/AuthCodeSendEvent.java

@@ -0,0 +1,20 @@
+package com.example.parking.auth.authcode.event;
+
+public class AuthCodeSendEvent {


record로 만들어도 될거 같아유

This2sho · 2024-02-25T18:01:52Z

src/main/java/com/example/parking/auth/authcode/AuthCode.java

+    @Enumerated(EnumType.STRING)
+    private AuthCodeCategory authCodeCategory;
+
+    private Boolean isUsed = Boolean.FALSE;


isUsed말고 세션처럼 만료시간을 두는건 어떻게 생각하시나요?
@Where(clause = "expired_at > CURRENT_TIMESTAMP") 이거 객체단에 붙여서 조회할 때도 유효한 AuthCode만 가져올 수 있을거 같은데

This2sho · 2024-02-25T18:09:28Z

src/main/java/com/example/parking/auth/authcode/AuthCode.java

+    @GeneratedValue(strategy = GenerationType.IDENTITY)
+    private Long id;
+
+    private String destination;


destination을 AuthCodePlatform 랑 합쳐서 입력받는 타입을 지켜줘도 좋을거 같은데 어떻게 생각하시나여?
(Email이면 Email 형식이 맞는지 검증도 하고)

This2sho · 2024-02-25T18:12:39Z

src/main/java/com/example/parking/auth/authcode/AuthCodeCategory.java

+                .orElseThrow(() -> new InValidAuthCodeException("존재하지 않는 인증 코드 발급 행위입니다."));
+    }
+
+    public String getCategoryName() {


This2sho · 2024-02-25T18:31:37Z

src/test/java/com/example/parking/auth/AuthServiceTest.java

+//        this.authCodeRepository = authCodeRepository;
+//        this.authService = new AuthService(memberSessionRepository, authCodeRepository, fixAuthCodeGenerator,
+//                applicationEventPublisher);
+//    }


안쓰는 코드인가유?

This2sho · 2024-02-25T18:40:07Z

src/test/java/com/example/parking/auth/AuthServiceTest.java

-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatNoException;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
-
 @Transactional
 @SpringBootTest
 class AuthServiceTest {


이거 테스트 깨지는데 혹시 확인좀 가능할까유?

This2sho · 2024-02-25T18:47:41Z

src/test/java/com/example/parking/auth/AuthServiceTest.java

-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatNoException;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
-
 @Transactional


@Transactional 어노테이션을 클래스단에 붙인 이유를 알 수 있을까유?

이렇게 클래스단에 붙여버리면 authService의 메소드들의 트랜잭션 범위를 테스트할 수 없을거같은데

This2sho · 2024-02-25T18:55:26Z

src/test/java/com/example/parking/auth/AuthServiceTest.java

+        String newAuthCode = authService.createAuthCode(
+                new AuthCodeRequest(authCodeDestination, authCodePlatform.getPlatform(),
+                        authCodeCategory.getCategoryName())
+        );


제가 생각했을 때 이 부분 테스트하려면 Mock 객체를 사용하거나
첫번째 호출할 때는 111... 두번째 호출할 때는 222.. 이런식으로 생성하는 AuthCodeGenerator를 만들어볼거 같아유

jundonghyuk

영호씨 고생하셨습니다 !

깔끔하네용

생각해보면 좋을 것들 커멘트 남겼습니다 ㅎ_ㅎ

jundonghyuk · 2024-02-26T06:50:55Z

src/main/java/com/example/parking/auth/AuthService.java

+    public void certificateAuthCode(AuthCodeCertificateRequest authCodeCertificateRequest) {
+        String destination = authCodeCertificateRequest.getDestination();
+        AuthCodePlatform authCodePlatform = AuthCodePlatform.find(authCodeCertificateRequest.getAuthCodePlatform());
+        AuthCodeCategory authCodeCategory = AuthCodeCategory.find(authCodeCertificateRequest.getAuthCodeCategory());
+
+        AuthCode authCode = authCodeRepository.findRecentlyAuthCodeBy(destination, authCodePlatform, authCodeCategory)
+                .orElseThrow(() -> new InValidAuthCodeException("해당 형식의 인증코드가 존재하지 않습니다."));
+        authCode.certificate(authCodeCertificateRequest.getAuthCode());
+    }


authCode의 만료기간은 없을까요? 5분 이살 지났으면 유효하지 않다던지 ?
정책적인 문제긴한데, 조금 필요해 보여서요..!

jundonghyuk · 2024-02-26T06:58:40Z

src/main/java/com/example/parking/auth/authcode/application/AuthCodeEventListener.java

+    private final AuthCodeSender authCodeSender;
+
+    @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
+    public void sendAuthCode(AuthCodeSendEvent authCodeSendEvent) {


이 이벤트 리스너를 잘 몰라서 여쭤봅니다 !
파라미터 타입은 보고 이 리스너가 작동될지 결정을 하는 것인가요 ?

jundonghyuk · 2024-02-26T07:06:04Z

src/main/java/com/example/parking/auth/authcode/application/AuthCodeEventListener.java

+    @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
+    public void sendAuthCode(AuthCodeSendEvent authCodeSendEvent) {
+        String destination = authCodeSendEvent.getDestination();
+        String authCode = authCodeSendEvent.getAuthCode();
+        authCodeSender.send(destination, authCode);


두 가지 얘기를 해보고 싶습니다 !
첫 번째는 특정 이벤트를 리슨하는 클래스를 만들어주셨는데요 !

지금은 메일전송 밖에 없지만, 이 이벤트에 반응하여 다양한 로직이 이뤄져야 한다고 쳤을 때, 이렇게 하나의 이벤트를 리슨하는 클래스를 만들어서 안에서 한 번에 실행이 되게끔 하는게 관리의 용이성 때문인가요 ?

저는 authCodeSender가 이벤트를 리슨한다고 생각했었는데, 궁금해서 여쭤봅니다 !

두 번째는 Zero-payload방식에 대해서 얘기해보고 싶습니다.
현재 이벤트안에 다양한 정보가 들어가 있는데요 !
저는 뭔가 이렇게 페이로드에 여러 정보를 넣어서 보내면 이벤트를 발행하는 쪽이 이벤트를 구독하는 애들이 뭐가 필요한지 간접적으로 알고있는거 아닌가? 라는 생각이 들었습니다.

만약 이 이벤트를 구독하여 다른 기능들을 실행해야하는 객체들이 많아지면, 각자 원하는 정보가 다를 수도 있을 것같은데요. 그 정보는 자기들이 알아서 찾도록 authCode의 pk값만 Payload에 담아주는 것은 어떤가요 ?

물론 현재 상황에서는 오버엔지니어링으로 보이긴합니다 !
그냥 이벤트안의 내용물에 뭐가 들어가야할지에 대해 토의해보고싶습니다 !

jundonghyuk · 2024-02-26T07:06:19Z

src/main/java/com/example/parking/auth/authcode/infrastructure/MailAuthCodeSender.java

+@Component
+public class MailAuthCodeSender implements AuthCodeSender {
+
+    private final JavaMailSender mailSender;


신기방기하네요

jundonghyuk · 2024-02-26T07:06:57Z

src/main/java/com/example/parking/auth/authcode/infrastructure/MailAuthCodeSender.java

+    @Override
+    public void send(String destination, String authCode) {
+        SimpleMailMessage mailMessage = new SimpleMailMessage();
+


그러게요 .!!?

sender나 title 같은거는 안써줘도될까요 ?

받는입장에서 뭔가 당황스러울수도..?

jundonghyuk · 2024-02-26T07:18:34Z

src/main/java/com/example/parking/util/authcode/NumberAuthCodeGenerator.java

+    @Override
+    public String generateAuthCode() {
+        StringBuilder stringBuilder = new StringBuilder();
+        for (int i = 0; i < MAX_LENGTH; i++) {
+            stringBuilder.append(random.nextInt(9));
+        }
+        return stringBuilder.toString();
+    }


6자리 번호면 10^6 만큼의 경우의 수가 있을 것 같은데요 !

brute force attack에 대해서도 어떻게 하면 좋을지 생각해보면 좋을 것 같아요 !

시도횟수를 둔다던지 ??

약간 카드 결제 시도횟수같은 느낌 ?

youngh0 · 2024-03-01T17:32:42Z

🤔세부 내용

인증코드 저장소를 mysql -> redis 로 변경했습니다.
- 테스트를 위해 testContainer 도입했습니다.
  - 컨터이너 생성을 한 번만 하기 위해 static 을 활용했고, 각 데이터베이스(MySQL, redis) 의 데이터를 지우는 코드도 @AfterEach 를 활용해서 작성했습니다.
인증코드 생성 시 동적 스케줄링을 통해 application.yml 에 설정한 시간 뒤에 삭제되도록 해서 인증 시간이 유효한 데이터만 관리되도록 하였습니다.
인증코드 플랫폼 (메일) 별 형식을 확인하는 validator 구현했습니다

🫵리뷰 중점사항

이벤트 리스너의 위치를 어디다 두는게 좋을까요...
인증코드, destination, platform 등등을 조합해 redis 에 저장되는 키를 생성하는 AuthCodeKeyConverter.convert 있습니다. 이걸 static 으로 두는게 좋을지, 빈 등록을 할지 고민됩니다.
- 해당 메서드는 인증 코드 생성, 인증 코드 삭제 동적 스케줄링 즉, AuthService, AuthCodeEventListener 에서 사용됩니다.
- 판단을 못하겠어서 일단 static 으로 했는데 의견 듣고 수정하겠습니다!
인증코드 검증 요청 시 유효한 요청이면 해당 레코드를 redis 에서 지우고 있습니다. 그래서 Delete 메서드를 사용하고 있는데 어떻게 생각하시나요?? http method 와 응답코드가 고민 되네요..

This2sho

고생하셨습니다. 영호씨

도커에서 계속 오류가나서 테스트는 못해봤고
코드보면서 생각들 커맨트 남겼습니다~

This2sho · 2024-03-10T07:59:02Z

src/test/java/com/example/parking/container/MySQLDataCleaner.java

+//        jdbcTemplate.execute("TRUNCATE TABLE parking");
+//        jdbcTemplate.execute("TRUNCATE TABLE member_session");
+//        jdbcTemplate.execute("TRUNCATE TABLE search_condition");
+//        jdbcTemplate.execute("TRUNCATE TABLE member");


주석 지우셔도 될거같아유

This2sho · 2024-03-10T10:09:51Z

src/main/java/com/example/parking/auth/AuthService.java

-        applicationEventPublisher.publishEvent(new AuthCodeSendEvent(destination, randomAuthCode));
+        String authCodeKey = AuthCodeKeyConverter.convert(randomAuthCode, destination, authCodePlatform.getPlatform(),
+                authCodeCategory.getCategoryName());
+        redisTemplate.opsForValue().set(authCodeKey, "true");


"true" 대신 authCode가 value에 들어가는게 맞는거 같아여
key는 destination([email protected]), categoryName(findPassword) 이것만 있어도 충분할거 같아유

그리고 저장할 때, 만료시간도 설정하면 될거같아유

This2sho · 2024-03-10T11:11:12Z

src/test/java/com/example/parking/container/ContainerTest.java

+                .withExposedPorts(6379);
+        REDIS_CONTAINER.start();
+        System.setProperty("spring.data.redis.host", REDIS_CONTAINER.getHost());
+        System.setProperty("spring.data.redis.port", String.valueOf(REDIS_CONTAINER.getRedisPort()));


21번 라인에 .withExposedPorts(6379); 랑 위에서 포트 지정해주는거랑 무슨 차이인가여?

This2sho · 2024-03-10T11:21:22Z

src/main/java/com/example/parking/auth/authcode/event/AuthCodeCreateEvent.java

+import lombok.Getter;
+
+@Getter
+public class AuthCodeCreateEvent {


이벤트 같은건 record로 해도 될거같은데여

This2sho · 2024-03-10T11:22:55Z

src/main/java/com/example/parking/auth/authcode/application/AuthCodeEventListener.java

+        String authCodeCategory = authCodeCreateEvent.getAuthCodeCategory();
+
+        String authCodeKey = AuthCodeKeyConverter.convert(authCode, destination, authCodePlatform, authCodeCategory);
+        taskScheduler.schedule(() -> redisTemplate.delete(authCodeKey), Instant.now().plusSeconds(authCodeExpired));


redis에 만료 시간두는게 낫지 않을까여?

This2sho · 2024-03-10T11:25:40Z

src/main/java/com/example/parking/auth/authcode/application/PlatformValidator.java

+
+public interface PlatformValidator {
+
+    boolean inInvalidForm(String destination);


Suggested change

boolean inInvalidForm(String destination);

boolean isInvalidForm(String destination);

오타인가여?

# Conflicts: # src/test/java/com/example/parking/auth/MemberSessionRepositoryTest.java

github-actions · 2024-03-12T18:09:20Z

Test Results

120 tests 120 ✅ 3s ⏱️
25 suites 0 💤
25 files 0 ❌

Results for commit bd7c135.

youngh0 added 24 commits February 21, 2024 13:52

feat: 인증 코드 엔티티 정의

383217b

feat: 인증 방식 enum 정의

347a84a

feat: 난수 인증 코드 생성기 정의

1a4ce08

feat: AuthCode 레포지토리 생성

e899788

feat: 인증코드 저장 로직 작성

50d46fa

refactor: 패키지 경로 수정

f1c927e

refactor: import 수정

70634e4

refactor: auth 패키지 내부 분리

0554e3a

feat: 메일 라이브러리 의존성 추가

e15e4d9

feat: 메일 전송 구현

c2d070c

feat: AuthCodeRequest dto 생성

ce28a2f

feat: 인증코드, 수신자 조합으로 조회하는 메서드 구현

9b642ac

refactor: 인터페이스 메서드 public 제거

005f643

feat: 인증코드 저장 로직 구현

686a33a

feat: 인증 코드 전송 이벤트 코드 구현

c85ed99

feat: 인증 코드 조회 메서드 수정

dffcd00

feat: 인증 코드 유효성 검증 로직 작성

efacd42

feat: 인증 코드 타입 enum 이름 변경

1b9f2b1

feat: 인증 코드가 발생한 행위 변수 추가

38a4133

feat: 인증 코드 검증 메서드 구현

0dec47f

feat: getter 정의

09bcb85

refacor: 변수명 수정

c6274db

test: 인증코드 검증 기능 테스트 작성

2d06a4a

feat: 인증코드 예외 작성

a5f932d

youngh0 added the 📝feature 기능 추가 label Feb 21, 2024

youngh0 requested review from This2sho and jundonghyuk February 21, 2024 09:35

youngh0 self-assigned this Feb 21, 2024

This2sho requested changes Feb 25, 2024

View reviewed changes

jundonghyuk requested changes Feb 26, 2024

View reviewed changes

youngh0 added 12 commits March 2, 2024 00:12

import 변경

9aa310c

feat: 인증코드 생성 이벤트 네이밍 수정

7ff7fbe

feat: 이벤트 페이로드 수정

007dc2c

feat: 인증 코드 생성 시 5분 뒤 삭제 스케줄링 구현

c5019c8

feat: 인증코드 생성 요청 응답 dto 생성

9dad1ae

refactor: 개행 추가

4f2fa22

feat: 인증코드 컨트롤러 구현

3d841b5

refactor: 사용하지 않는 클래스 삭제

acb987b

feat: redis 에 저장되는 키 형태 변환기 구현

0eaeefb

feat: 플랫폼 별 destination 형식 유효성 검사

64217a1

feat: super() 추가

c4a6783

feat: 메서드 이름 수정

c4ad856

youngh0 requested review from This2sho and jundonghyuk March 1, 2024 17:32

test: 테스트 변수 수정

0d0fba6

This2sho requested changes Mar 10, 2024

View reviewed changes

youngh0 added 11 commits March 12, 2024 22:13

refactor: 주석제거

da3fd60

feat: 인증 코드 저장 시 유효기간 설정

446940e

refactor: 메서드 네이밍 수정

eab7d17

feat: 테스트 yml 추가

4ad7a0c

feat: test bean 주입

eaa58f5

feat: ci 구문 변경

e2bbab5

feat: yml 추가

1f67dbe

test: yml 생성

30a646b

feat: ci 새로고침

eefe2ff

feat: ci 돌리기

0357388

Merge remote-tracking branch 'origin/main' into feat/30-send-auth-mail

bd7c135

# Conflicts: # src/test/java/com/example/parking/auth/MemberSessionRepositoryTest.java

youngh0 merged commit 2bebe2e into main Mar 12, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

인증 코드 메일 전송 및 검증 기능 구현 #32

인증 코드 메일 전송 및 검증 기능 구현 #32

youngh0 commented Feb 21, 2024

This2sho left a comment

This2sho Feb 25, 2024

This2sho Feb 25, 2024

jundonghyuk Feb 26, 2024

This2sho Feb 25, 2024

This2sho Feb 25, 2024

This2sho Feb 25, 2024

This2sho Feb 25, 2024

This2sho Feb 25, 2024

This2sho Feb 25, 2024

This2sho Feb 25, 2024

This2sho Feb 25, 2024

jundonghyuk left a comment

jundonghyuk Feb 26, 2024

jundonghyuk Feb 26, 2024

jundonghyuk Feb 26, 2024

jundonghyuk Feb 26, 2024

jundonghyuk Feb 26, 2024

jundonghyuk Feb 26, 2024

youngh0 commented Mar 1, 2024 •

edited

Loading

This2sho left a comment

This2sho Mar 10, 2024

This2sho Mar 10, 2024

This2sho Mar 10, 2024

This2sho Mar 10, 2024

This2sho Mar 10, 2024

This2sho Mar 10, 2024

github-actions bot commented Mar 12, 2024


		public interface AuthCodeSender {

		public void send(String destination, String authCode);

	public void send(String destination, String authCode);
	void send(String destination, String authCode);

		@@ -0,0 +1,20 @@
		package com.example.parking.auth.authcode.event;

		public class AuthCodeSendEvent {


		public interface PlatformValidator {

		boolean inInvalidForm(String destination);

	boolean inInvalidForm(String destination);
	boolean isInvalidForm(String destination);

인증 코드 메일 전송 및 검증 기능 구현 #32

인증 코드 메일 전송 및 검증 기능 구현 #32

Conversation

youngh0 commented Feb 21, 2024

👍관련 이슈

🤔세부 내용

🫵리뷰 중점사항

인증 코드 생성기(AuthCodeGenerator) 의존성

인증 코드 검증 시 동시성 이슈

This2sho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jundonghyuk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youngh0 commented Mar 1, 2024 • edited Loading

🤔세부 내용

🫵리뷰 중점사항

This2sho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 12, 2024

Test Results

youngh0 commented Mar 1, 2024 •

edited

Loading