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

[fix] #295 - HttpOnly 쿠키에서 자동으로 refreshToken을 추출해 처리하도록 수정 완료 #296

Merged
merged 7 commits into from
Dec 28, 2024

Conversation

hoonyworld
Copy link
Member

@hoonyworld hoonyworld commented Dec 28, 2024

Related issue 🛠

Work Description ✏️

  • HttpOnly 쿠키에서 자동으로 refreshToken을 추출해 처리하도록 수정을 하였습니다.
  • 변경 이유는 295번 이슈 참고해주시면 감사하겠습니다!!

Trouble Shooting ⚽️

스웨거에서 쿠키 테스트 불가능한 문제

image
  • 스웨거에서는 리프레쉬 토큰을 쿠키에 넣는 기능을 제공해주지 않는 것 같습니다.
  • 따라서 클라이언트 측에 해당 API는 Postman을 통해 테스트해야 작동을 한다는 것을 알리려고 합니다.

Related ScreenShot 📷

image
  • 변경된 로직으로 리프레쉬 토큰을 통해 액세스 토큰 재발급 API가 성공한 모습입니다.
  • 포스트맨에서는 로그인 시 리프레쉬 토큰을 쿠키에 넣어주고, 리프레쉬 토큰을 통한 액세스 토큰 재발급 API 시도 시 자동으로 쿠키에 값을 넣어줍니다.

Uncompleted Tasks 😅

To Reviewers 📢

  • 머지 후 dev 서버의 nginx reverse proxy 컨테이너에서 클라이언트 요청에 포함된 쿠키를 그대로 스프링 컨테이너로 전달하는 지 확인해봐야 할 것 같습니다!

) {
AccessTokenGetSuccess accessTokenGetSuccess = authenticationService.generateAccessTokenFromRefreshToken(refreshToken);
AccessTokenGenerateResponse response = authenticationService.generateAccessTokenFromRefreshToken(refreshToken);
Copy link
Collaborator

Choose a reason for hiding this comment

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

사소한 부분이긴 한데 상단에 response이름을 살렸던 것처럼
response 대신 accessTokenGenerateResponse 로 적으면 더 통일성 있을 것 같습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

이 부분은 저희가 Controller에서 API 작업 시 응닶 DTO return 시 response 로 네이밍을 써왔어서 통일성있게 response로 적어보았습니다!!

(BookingController 참고하시면 좋을 것 같아요!)

Copy link
Collaborator

Choose a reason for hiding this comment

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

아하 MemberApi에는
HttpServletResponse httpServletResponse
이런식으로 네이밍을 적으셔서 controller에서도 통일하는 게 좋지 않을까 생각했는데 api와 controller도 네이밍 통일하면 좋을 것 같습니다!
저는 개인적으로는 코드 길이가 조금 길어지더라도 이해도를 위해 dto 네이밍을 그대로 사용하는 방식을 더 선호하는데 어떻게 생각하시나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 그대로 사용하는 방식이 좋을 것 같습니다

이 부분 나중에 멀티모듈 도입 시, 한번에 바꾸는거 어떠신가요? (바꿔야 할 네이밍들이 너무 많아서요...)

Copy link
Collaborator

Choose a reason for hiding this comment

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

좋습니다~~

Copy link
Collaborator

@hyerinhwang-sailin hyerinhwang-sailin left a comment

Choose a reason for hiding this comment

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

LGTM~

@hoonyworld hoonyworld merged commit 55f4bd6 into develop Dec 28, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug] HttpOnly 쿠키로 인한 Refresh Token 접근 불가 문제 및 API 요청 방식 개선
2 participants