-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: 기존 학교 이메일 인증 API 기능(동작 방식) 재구성 #381
Conversation
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인했습니다~
src/main/java/com/gdschongik/gdsc/domain/member/domain/Member.java
Outdated
Show resolved
Hide resolved
@@ -43,6 +43,7 @@ public enum ErrorCode { | |||
UNIV_NOT_VERIFIED(HttpStatus.CONFLICT, "재학생 인증이 완료되지 않았습니다."), | |||
DISCORD_NOT_VERIFIED(HttpStatus.CONFLICT, "디스코드 인증이 완료되지 않았습니다."), | |||
BEVY_NOT_VERIFIED(HttpStatus.CONFLICT, "GDSC Bevy 가입이 완료되지 않았습니다."), | |||
EMAIL_ALREADY_VERIFIED(HttpStatus.CONFLICT, "이미 이메일 인증된 회원입니다."), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line 50과 중복되는 것 같습니다~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 이건 line50과 좀 다른게 line50의 경우엔
이메일 중복검증 이고 이건 인증을 이미 한 회원이 재인증한경우 에러뱉도록 하는거에요!
src/main/java/com/gdschongik/gdsc/global/exception/ErrorCode.java
Outdated
Show resolved
Hide resolved
src/main/java/com/gdschongik/gdsc/global/util/email/EmailVerificationTokenUtil.java
Outdated
Show resolved
Hide resolved
.../java/com/gdschongik/gdsc/domain/email/application/UnivEmailVerificationLinkSendService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/gdschongik/gdsc/domain/email/application/UnivEmailVerificationService.java
Outdated
Show resolved
Hide resolved
...ain/java/com/gdschongik/gdsc/domain/email/dto/request/UnivEmailTokenVerificationRequest.java
Outdated
Show resolved
Hide resolved
public ResponseEntity<Void> sendUnivEmailVerificationLink( | ||
@RequestParam(VERIFY_EMAIL_REQUEST_PARAMETER_KEY) String verificationCode) { | ||
univEmailVerificationService.verifyMemberUnivEmail(verificationCode); | ||
@RequestBody @Valid UnivEmailTokenVerificationRequest verificationToken) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dto는 request라는 이름으로 받고 있습니다~
@RequestBody @Valid UnivEmailTokenVerificationRequest verificationToken) { | |
@RequestBody @Valid UnivEmailTokenVerificationRequest request) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메일로 보내는 링크는 /onboarding/verify-email?%token={token value}
의 형태던데,
그걸 클릭하면 인증되는 방식 아닌가요?
만약 맞다면 파라미터가 아닌 body로 토큰을 받는 이유가 궁금합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메일로 보내는 링크는
/onboarding/verify-email?%token={token value}
의 형태던데, 그걸 클릭하면 인증되는 방식 아닌가요? 만약 맞다면 파라미터가 아닌 body로 토큰을 받는 이유가 궁금합니다!
이거 하나만 설명 부탁드립니다!
src/main/java/com/gdschongik/gdsc/domain/email/application/UnivEmailVerificationService.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Cho Sangwook <[email protected]>
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
단순한 궁금증인데 이건 테스트코드로 못짜나용?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
백엔드 팀 온보딩 가이드
를 보시면 커밋 메시지 컨벤션이 정리되어있습니다.
대체로 잘 지켜주셨는데, 콜론 앞에는 공백이 없어야 합니다!
해당 내용 한번 더 확인해보시고 pr 제목만 컨벤션에 맞게 수정 부탁드립니다!
외부 의존성 경계를 기준으로 모킹해서 테스트할 수 있어요 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
…into feature/376-univ-email-verify
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
슬기화이팅 |
🌱 관련 이슈
🤔 작업 배경 및 사유
기존 학교 이메일로 인증번호를 전송하고 그 값을 redis에 저장 후 ttl전에 인증요청을 할 시 인증되도록 구현되어 있습니다.
이 방식을 사용 할 시 아래와 같은 문제점이 나오게 되었습니다.
따라서 해당 문제점을 해결하기 위해 인증 링크를 통해 인증번호 입력 페이지 없이 인증가능하도록 기능 변경을 하게되었습니다.
📌 작업 내용 및 특이사항
-이메일 인증하기 api GET -> PATCH로 수정
마지막으로, 조금 우려되는 사항이 해당방식으로 했을 시, jwt로만 검증하다보니 여러 인증메일을 요청한다면, 그중 어떤 링크를 선택하든 인증이 된다는 점이 우려됩니다. 사실 이건 어떤 방식이든 제가 재인증 검증을 넣어둬서 상관없지만 기획이 아래처럼 어떠냐에 따라 변경해야 할 수도 있을것 같습니다.
이렇게되므로 기획적으로 한번 논의해봐야할것 같습니다.
☑️ToDo
📝 참고사항
📚 기타
코드는 기존 작업자 코드 스타일대로 최대한 짯는데 여러분들 입맛에 맞을지는 모르겠네요 ㅠ