Skip to content
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

[FEATURE] 이메일 인증 기능 구현 #37 #46

Merged
merged 5 commits into from
Oct 17, 2023
Merged

Conversation

wooni89
Copy link
Collaborator

@wooni89 wooni89 commented Oct 17, 2023

이메일 인증기능 구현 #37 완료

- jwt필터 코드수정 (React연동 회원가입 후 토큰발급문제 수정)
- jwt필터 코드 다시 수정 (React연동 회원가입 후 토큰발급문제 수정)햣
Comment on lines 63 to 71
public String generateVerificationLink(User user) {
String verificationToken = generateUniqueToken();

user.activeStatus();
userRepository.save(user);

return "http://localhost:8080/mail/verify?token=" + verificationToken;
}

Copy link
Member

Choose a reason for hiding this comment

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

여기서 localhost를 직접 지정해주게 될 경우에는 개발서버로 배포를 할 시 사용을 못하게 됩니다.
Dynamic하게 변경하기 위해 application-local, application-dev로 분할하여 주입받으시기 바랍니다.

또한 해당 클래스는 메일발송만을 위한 유틸리티 클래스 이기 떄문에 사용자 인증을 위한 유틸리티는 따로 분리가 필요해 보입니다.

request.getEmail(),
"이메일 인증 링크를 클릭하여 이메일 인증을 완료하세요",
"mail/verificationEmail",
Map.of("verificationLink", sendMailUtil.generateVerificationLink(user))
Copy link
Member

Choose a reason for hiding this comment

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

generateVerificationLink메소드의 경우 MailHelper클래스에서 private Method로 제작한 이후에 this.generateVerificationLink(user) 방식으로 사용하는게 좋을 것 같습니다.

sendMailUtil은 말그대로 Mail만을 위한 UtilClass이기떄문에 해당 클래스에서 추가하는건 부적적해보입니다.

Copy link
Member

@donsonioc2010 donsonioc2010 left a comment

Choose a reason for hiding this comment

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

OK ㅌㅌ

@donsonioc2010 donsonioc2010 merged commit 8a00779 into dev Oct 17, 2023
1 check passed
@donsonioc2010 donsonioc2010 deleted the feat/37-mail-verify branch October 17, 2023 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants