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

[Feat] #44 TokenRefresh하기~ #48

Merged
merged 7 commits into from
Oct 11, 2024
Merged

Conversation

suhyeon7497
Copy link
Contributor

✨ 작업 내용

  • /token-refresh로 get요청을 보내면 Authorization Cookie를 검증하고 refresh합니다.

✨ 참고 사항

  • refreshToken은 redis에 저장됩니다.

⏰ 현재 버그


✏ Git Close

redis에 refreshToken이 있으면 재발급합니다.
refreshToken이 없을 경우, 잘못된 토큰 에러메세지를 반환합니다.

관련 이슈: #44
@suhyeon7497 suhyeon7497 added the ✨ Feature 기능 개발 label Oct 9, 2024
@suhyeon7497 suhyeon7497 self-assigned this Oct 9, 2024
@BaeJunH0 BaeJunH0 self-requested a review October 10, 2024 10:06
cors setting

관련이슈 : #44
Copy link
Contributor

@BaeJunH0 BaeJunH0 left a comment

Choose a reason for hiding this comment

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

큰 문제가 있는 건 아니라서 Approve 다시 합니다요
한 번씩 읽어는 주세용

@AllArgsConstructor
@Getter
public enum UserErroCode implements ErrorCode {
NOT_FOUND(HttpStatus.NOT_FOUND, "U001", "User is not found");
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment on lines 20 to 22
return !refreshTokenRepository.findById(username).orElseThrow(() ->
InplaceException.of(AuthorizationErrorCode.INVALID_TOKEN))
.getRefreshToken().equals(refreshToken);
Copy link
Contributor

Choose a reason for hiding this comment

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

살짝 읽기 불편한데 orElseThrow 앞에서 줄 바꾸면 안될까요? 아니면 람다식 부분에서
() -> "..." 부분을 한 줄에 하면 좋을 것 같습니다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

인정합니다.


@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@AllArgsConstructor
Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분 필요한가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

필요합니다......

@Repository
public interface RefreshTokenRepository extends CrudRepository<RefreshToken, String> {

Optional<RefreshToken> findById(String id);
Copy link
Contributor

Choose a reason for hiding this comment

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

기본 형식 메서드인데 재정의 해둬야 하나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

인정합니다!


@RestController
@RequiredArgsConstructor
public class RefreshTokenController {
Copy link
Contributor

Choose a reason for hiding this comment

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

나중에 명세 작성해주세용

Copy link
Contributor Author

Choose a reason for hiding this comment

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

했습니다!

jwtUtil.getUsername(refreshToken), refreshToken);
addTokenToCookie(response, reIssuedToken);

return ResponseEntity.ok().build();
Copy link
Contributor

Choose a reason for hiding this comment

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

저희 new ResponseEntity() 형식으로 하기로 한 거 아니었나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오케이입니다.

Comment on lines 39 to 56
private void addTokenToCookie(HttpServletResponse response, ReIssued reIssuedToken) {
Cookie accessTokenCookie = createCookie("access_token", reIssuedToken.accessToken());
Cookie refreshTokenCookie = createCookie("refresh_token", reIssuedToken.refreshToken());
response.addCookie(accessTokenCookie);
response.addCookie(refreshTokenCookie);
}

private boolean cannotRefreshToken(Cookie cookie) {
return AuthorizationUtil.getUserId() == null || !jwtUtil.isRefreshToken(cookie.getValue());
}

private Cookie createCookie(String key, String value) {
Cookie cookie = new Cookie(key, value);
cookie.setMaxAge(60 * 60);
cookie.setPath("/");
cookie.setHttpOnly(true);
return cookie;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Util 클래스를 만들어야 할까요... Controller에서 처리할 일은 아닌가 싶기도 하구요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

util만들었습니다!

Copy link
Contributor

@sanghee0820 sanghee0820 left a comment

Choose a reason for hiding this comment

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

수정 부탁드립니다.

Comment on lines 16 to 28
@Override
public HttpStatus httpStatus() {
return null;
}

@Override
public String code() {
return "";
}

@Override
public String message() {
return "";
Copy link
Contributor

Choose a reason for hiding this comment

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

@Getter 로 사용하지 말고 인터페이스에 있는 값 사용 부탁드립니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵!

getmethod 빼먹은 부분 을 추가 하였습니다.
관련 이슈 : #44
@sanghee0820 sanghee0820 merged commit add8b10 into weekly/6 Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feature 기능 개발
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants