-
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: 학생 회원가입 인증 로직 변경 #669
Changes from all commits
ae2abe8
22d37ce
f98be84
efd1ca1
41ad2eb
a0e77f9
fbe86fa
ad97670
929c46a
586247d
39fa2ec
7d2000e
0d2a3bc
778db01
35c891d
d6c4d00
b52dee9
3ee57a9
d621dff
8bdb6ad
928c76c
cbffb70
a419bb8
43500d7
ff8591f
585f0ef
8ccf80f
cff3b07
a5d8bf6
a08bca1
ade14bb
b20c003
7886762
cad7093
3cfb91a
8738553
58ee074
ff6f641
80b791b
c8af4cf
f5abd9b
3c77829
f209a67
8b21338
c4788b8
d436385
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
package in.koreatech.koin.domain.user.model.redis; | ||
|
||
import in.koreatech.koin.domain.user.dto.StudentRegisterRequest; | ||
import in.koreatech.koin.domain.user.model.*; | ||
import lombok.Getter; | ||
import org.springframework.data.annotation.Id; | ||
import org.springframework.data.redis.core.RedisHash; | ||
import org.springframework.data.redis.core.TimeToLive; | ||
import org.springframework.data.redis.core.index.Indexed; | ||
import org.springframework.security.crypto.password.PasswordEncoder; | ||
|
||
@Getter | ||
@RedisHash(value = "StudentTemporaryStatus") | ||
public class StudentTemporaryStatus { | ||
|
||
private static final long CACHE_EXPIRE_SECOND = 60 * 60 * 10L; | ||
|
||
@Id | ||
@Indexed | ||
private String email; | ||
|
||
@Indexed | ||
private String authToken; | ||
|
||
@Indexed | ||
private String nickname; | ||
Comment on lines
+18
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
private String name; | ||
|
||
private String password; | ||
|
||
private UserGender gender; | ||
|
||
private boolean isGraduated; | ||
|
||
private String department; | ||
|
||
private String studentNumber; | ||
|
||
private String phoneNumber; | ||
|
||
@TimeToLive | ||
private Long expiration; | ||
|
||
public StudentTemporaryStatus(String email, String authToken, String nickname, String name, String password, | ||
UserGender gender, boolean isGraduated, String department, String studentNumber, String phoneNumber) { | ||
this.email = email; | ||
this.authToken = authToken; | ||
this.nickname = nickname; | ||
this.name = name; | ||
this.password = password; | ||
this.gender = gender; | ||
this.isGraduated = isGraduated; | ||
this.department = department; | ||
this.studentNumber = studentNumber; | ||
this.phoneNumber = phoneNumber; | ||
this.expiration = CACHE_EXPIRE_SECOND; | ||
} | ||
|
||
public static StudentTemporaryStatus of(StudentRegisterRequest request, String authToken) { | ||
return new StudentTemporaryStatus(request.email(), authToken, request.nickname(), request.name(), request.password(), request.gender(), | ||
request.isGraduated(), request.department(), request.studentNumber(), request.phoneNumber()); | ||
} | ||
|
||
public Student toStudent(PasswordEncoder passwordEncoder) { | ||
User user = User.builder() | ||
.password(passwordEncoder.encode(password)) | ||
.email(email) | ||
.name(name) | ||
.nickname(nickname) | ||
.gender(gender) | ||
.phoneNumber(phoneNumber) | ||
.isAuthed(true) | ||
.isDeleted(false) | ||
.userType(UserType.STUDENT) | ||
.authToken(authToken) | ||
.build(); | ||
|
||
return Student.builder() | ||
.user(user) | ||
.anonymousNickname("익명_" + (System.currentTimeMillis())) | ||
.isGraduated(isGraduated) | ||
.userIdentity(UserIdentity.UNDERGRADUATE) | ||
.department(department) | ||
.studentNumber(studentNumber) | ||
.build(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,19 @@ | ||||||
package in.koreatech.koin.domain.user.repository; | ||||||
|
||||||
import in.koreatech.koin.domain.user.model.redis.StudentTemporaryStatus; | ||||||
import org.springframework.data.repository.Repository; | ||||||
|
||||||
import java.util.Optional; | ||||||
|
||||||
public interface StudentRedisRepository extends Repository<StudentTemporaryStatus, String> { | ||||||
|
||||||
StudentTemporaryStatus save(StudentTemporaryStatus studentTemporaryStatus); | ||||||
|
||||||
Optional<StudentTemporaryStatus> findById(String email); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CfindByEmail이 낫지 않을까요?
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저도 처음에 findByEmail로 했는데, 현재 key값인 email은 Id로 해야만 데이터를 찾을 수 있더라고요 ㅋㅋ |
||||||
|
||||||
Optional<StudentTemporaryStatus> findByNickname(String nickname); | ||||||
|
||||||
Optional<StudentTemporaryStatus> findByAuthToken(String authToken); | ||||||
|
||||||
void deleteById(String email); | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,9 +4,11 @@ | |
import java.util.Optional; | ||
import java.util.UUID; | ||
|
||
import in.koreatech.koin.domain.user.model.*; | ||
import in.koreatech.koin.domain.user.model.redis.StudentTemporaryStatus; | ||
import in.koreatech.koin.domain.user.repository.StudentRedisRepository; | ||
import org.joda.time.LocalDateTime; | ||
import org.springframework.context.ApplicationEventPublisher; | ||
import org.springframework.dao.DataIntegrityViolationException; | ||
import org.springframework.security.crypto.password.PasswordEncoder; | ||
import org.springframework.stereotype.Service; | ||
import org.springframework.transaction.annotation.Transactional; | ||
|
@@ -24,13 +26,6 @@ | |
import in.koreatech.koin.domain.user.exception.DuplicationNicknameException; | ||
import in.koreatech.koin.domain.user.exception.StudentDepartmentNotValidException; | ||
import in.koreatech.koin.domain.user.exception.StudentNumberNotValidException; | ||
import in.koreatech.koin.domain.user.model.AuthResult; | ||
import in.koreatech.koin.domain.user.model.Student; | ||
import in.koreatech.koin.domain.user.model.StudentDepartment; | ||
import in.koreatech.koin.domain.user.model.StudentEmailRequestEvent; | ||
import in.koreatech.koin.domain.user.model.User; | ||
import in.koreatech.koin.domain.user.model.UserGender; | ||
import in.koreatech.koin.domain.user.model.UserToken; | ||
import in.koreatech.koin.domain.user.repository.StudentRepository; | ||
import in.koreatech.koin.domain.user.repository.UserRepository; | ||
import in.koreatech.koin.domain.user.repository.UserTokenRepository; | ||
|
@@ -50,6 +45,7 @@ | |
public class StudentService { | ||
|
||
private final StudentRepository studentRepository; | ||
private final StudentRedisRepository studentRedisRepository; | ||
private final UserRepository userRepository; | ||
private final PasswordEncoder passwordEncoder; | ||
private final MailService mailService; | ||
|
@@ -66,12 +62,13 @@ public StudentResponse getStudent(Integer userId) { | |
@Transactional | ||
public StudentLoginResponse studentLogin(StudentLoginRequest request) { | ||
User user = userRepository.getByEmail(request.email()); | ||
Optional<StudentTemporaryStatus> studentTemporaryStatus = studentRedisRepository.findById(request.email()); | ||
|
||
if (!user.isSamePassword(passwordEncoder, request.password())) { | ||
throw new KoinIllegalArgumentException("비밀번호가 틀렸습니다."); | ||
} | ||
|
||
if (!user.isAuthed()) { | ||
if (studentTemporaryStatus.isPresent()) { | ||
throw new AuthorizationException("미인증 상태입니다. 아우누리에서 인증메일을 확인해주세요"); | ||
} | ||
|
||
|
@@ -114,45 +111,66 @@ public void checkDepartmentValid(String department) { | |
|
||
@Transactional | ||
public ModelAndView authenticate(AuthTokenRequest request) { | ||
Optional<User> user = userRepository.findByAuthToken(request.authToken()); | ||
return new AuthResult(user, eventPublisher, clock).toModelAndViewForStudent(); | ||
Optional<StudentTemporaryStatus> studentTemporaryStatus = studentRedisRepository.findByAuthToken(request.authToken()); | ||
|
||
if (studentTemporaryStatus.isEmpty()) { | ||
ModelAndView modelAndView = new ModelAndView("error_config"); | ||
modelAndView.addObject("errorMessage", "토큰이 유효하지 않습니다."); | ||
return modelAndView; | ||
} | ||
|
||
Student student = studentTemporaryStatus.get().toStudent(passwordEncoder); | ||
|
||
studentRepository.save(student); | ||
userRepository.save(student.getUser()); | ||
|
||
studentRedisRepository.deleteById(student.getUser().getEmail()); | ||
eventPublisher.publishEvent(new StudentRegisterEvent(student.getUser().getEmail())); | ||
|
||
return new ModelAndView("success_register_config"); | ||
} | ||
|
||
@Transactional | ||
public void studentRegister(StudentRegisterRequest request, String serverURL) { | ||
Student student = request.toStudent(passwordEncoder, clock); | ||
try { | ||
validateStudentRegister(student); | ||
studentRepository.save(student); | ||
userRepository.save(student.getUser()); | ||
mailService.sendMail(request.email(), new StudentRegistrationData(serverURL, student.getUser().getAuthToken())); | ||
eventPublisher.publishEvent(new StudentEmailRequestEvent(request.email())); | ||
} catch (DataIntegrityViolationException e) { | ||
// 동시성 문제를 처리하기 위한 코드 | ||
throw KoinIllegalArgumentException.withDetail("요청이 너무 빠릅니다."); | ||
} | ||
|
||
validateStudentRegister(request); | ||
String authToken = UUID.randomUUID().toString(); | ||
|
||
StudentTemporaryStatus studentTemporaryStatus = StudentTemporaryStatus.of(request, authToken); | ||
studentRedisRepository.save(studentTemporaryStatus); | ||
|
||
mailService.sendMail(request.email(), new StudentRegistrationData(serverURL, authToken)); | ||
eventPublisher.publishEvent(new StudentEmailRequestEvent(request.email())); | ||
} | ||
|
||
private void validateStudentRegister(Student student) { | ||
EmailAddress emailAddress = EmailAddress.from(student.getUser().getEmail()); | ||
private void validateStudentRegister(StudentRegisterRequest request) { | ||
EmailAddress emailAddress = EmailAddress.from(request.email()); | ||
emailAddress.validateKoreatechEmail(); | ||
|
||
validateDataExist(student); | ||
validateStudentNumber(student.getStudentNumber()); | ||
checkDepartmentValid(student.getDepartment()); | ||
validateDataExist(request); | ||
validateStudentNumber(request.studentNumber()); | ||
checkDepartmentValid(request.department()); | ||
} | ||
|
||
private void validateDataExist(Student student) { | ||
userRepository.findByEmail(student.getUser().getEmail()) | ||
.ifPresent(user -> { | ||
throw DuplicationEmailException.withDetail("email: " + student.getUser().getEmail()); | ||
}); | ||
|
||
if (student.getUser().getNickname() != null) { | ||
userRepository.findByNickname(student.getUser().getNickname()) | ||
private void validateDataExist(StudentRegisterRequest request) { | ||
userRepository.findByEmail(request.email()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AfindBy를 사용한 이유가 있었군요👀 |
||
.ifPresent(user -> { | ||
throw DuplicationNicknameException.withDetail("nickname: " + student.getUser().getNickname()); | ||
throw DuplicationEmailException.withDetail("email: " + request.email()); | ||
}); | ||
studentRedisRepository.findById(request.email()) | ||
.ifPresent(studentTemporaryStatus -> { | ||
throw DuplicationEmailException.withDetail("email: " + request.email()); | ||
}); | ||
|
||
if (request.nickname() != null) { | ||
userRepository.findByNickname(request.nickname()) | ||
.ifPresent(user -> { | ||
throw DuplicationNicknameException.withDetail("nickname: " + request.nickname()); | ||
}); | ||
studentRedisRepository.findByNickname(request.nickname()) | ||
.ifPresent(studentTemporaryStatus -> { | ||
throw DuplicationNicknameException.withDetail("nickname: " + request.nickname()); | ||
}); | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,11 @@ | |
|
||
import java.time.LocalDateTime; | ||
import java.util.Objects; | ||
import java.util.Optional; | ||
import java.util.UUID; | ||
|
||
import in.koreatech.koin.domain.user.model.redis.StudentTemporaryStatus; | ||
import in.koreatech.koin.domain.user.repository.StudentRedisRepository; | ||
import org.springframework.context.ApplicationEventPublisher; | ||
import org.springframework.security.crypto.password.PasswordEncoder; | ||
import org.springframework.stereotype.Service; | ||
|
@@ -42,6 +45,7 @@ public class UserService { | |
private final JwtProvider jwtProvider; | ||
private final UserRepository userRepository; | ||
private final StudentRepository studentRepository; | ||
private final StudentRedisRepository studentRedisRepository; | ||
private final OwnerRepository ownerRepository; | ||
private final PasswordEncoder passwordEncoder; | ||
private final UserTokenRepository userTokenRepository; | ||
|
@@ -51,12 +55,13 @@ public class UserService { | |
@Transactional | ||
public UserLoginResponse login(UserLoginRequest request) { | ||
User user = userRepository.getByEmail(request.email()); | ||
Optional<StudentTemporaryStatus> studentTemporaryStatus = studentRedisRepository.findById(request.email()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. C이 로직의 경우는 getByEmail을 사용해서 예외처리 해주는 건 어떨까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 일반적으로 Repository에서 getBy를 통해 예외처리를 하는 상황은 "데이터를 찾을 수 없습니다."라는 일반적인 예외를 발생시킬때 사용하는것같습니다. 하지만 현재 "미인증 상태입니다. 아우누리에서 인증메일을 확인해주세요"라는 예외처리를 내야 하는 상황인데 이걸 Repository에 구현하는건 너무 사소한 부분까지 Repository계층에서 책임을 맡게되는것같아 계층의 경계가 모호해지는것같아서 서비스계층에서 예외처리 했습니다. 이 부분에 대해 혹시 어덯게 생각하시나요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Repository에서 이를 처리한다면, 혹시나 나중에 findById를 쓰게 되는 메소드가 생긴다면 문제가 생길 수도 있겠네요. |
||
|
||
if (!user.isSamePassword(passwordEncoder, request.password())) { | ||
throw new KoinIllegalArgumentException("비밀번호가 틀렸습니다."); | ||
} | ||
|
||
if (!user.isAuthed()) { | ||
if (studentTemporaryStatus.isPresent()) { | ||
throw new AuthorizationException("미인증 상태입니다. 아우누리에서 인증메일을 확인해주세요"); | ||
} | ||
|
||
|
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.
A
저번 레거시부터 느꼈지만, 굳이 회원가입 인증 토큰을 10시간 보관하는게 괜찮은가 싶네요
10시간동안 안 받을 정도면..🤔
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.
그러게요... 근데 수정하게 된다면 팀 전체 논의사항으로 올려야 할것같네요..
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.
지금 인증번호를 받는 형식이 아니라 저는 괜찮을 수도 있을 것 같아요
일반적으로 인증번호는 시간을 짧게 하고 저렇게 메일로 보내서 접속하면 인증되게 하는 방식은 엄청 길게 주더라구요
(얼마전에 microsoft 관련해서도 까먹고 있다가 6개월 만에 가서 받았는데 됐음)