-
Notifications
You must be signed in to change notification settings - Fork 0
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
JWT refresh 로직 추가 #4
Conversation
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.
리뷰 남깁니다! 화이팅입니다 ㅎㅎ
userInfo.put("name", user.getName()); | ||
userInfo.put("loginId", user.getLoginId()); | ||
UserInfo userInfo = new UserInfo(); | ||
userInfo.setName(user.getName()); |
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.
constructor를 만드는게 아니라 set으로 한 이유가 있을까요?
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.
setter 사용은 지양하는 편이 좋습니다!
|
||
@PostMapping | ||
public ResponseEntity<JWToken> renweAccessToken(@RequestBody JWToken token){ | ||
return new ResponseEntity<JWToken>(jwtService.renewAccessToken(token.getRefreshToken()), HttpStatus.OK); |
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.
return을 꼭 ResponseEntity로 감쌀 필요가 있을까요?
userInfo.setName(user.getName()); | ||
userInfo.setLoginId(user.getLoginId()); | ||
|
||
if(jwtClaims.getExpiration().before(new Date())){ // refreshToken이 만료되지 않은 경우 accessToken만 생성 |
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.
if 안에 if가 있는 로직은 지양하는 것이 좋습니다. 어떻게 바꾸는게 좋을까요?
if(jwtClaims.getExpiration().before(new Date())){ // refreshToken이 만료되지 않은 경우 accessToken만 생성 | ||
token.setAccessToken(createAccessToken(userInfo)); | ||
return token; | ||
}else{ // refreshToken이 만료된 경우 access,refreshToken 둘 다 생성 |
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.
사실은 else도 되도록 안쓰는게 좋습니다!
@@ -13,5 +12,6 @@ public interface UserRepository extends JpaRepository<User, Long> { | |||
public List<User> findAll(); | |||
public Optional<User> findByLoginId(String loginId); | |||
public Optional<User> findByLoginIdAndPassword(String loginId, String password); | |||
public Optional<User> findByRefreshToken(String refreshToken); |
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.
User 객체를 보니 Refreshtoken이나 LoginId 등 unique 가 안되어 있는 것 같은데, 만약 같은 Refreshtoken값을 가진 유저가 생기면 어떻게 될까요?
JWT refresh 로직 추가