refactor: 응답에서 refresh token은 제외하여 전달하도록#646
refactor: 응답에서 refresh token은 제외하여 전달하도록#646whqtker wants to merge 4 commits intosolid-connection:developfrom
Conversation
whqtker
requested review from
Gyuhyeok99,
Hexeong,
JAEHEE25,
lsy1307,
sukangpunch and
wibaek
as code owners
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
sukangpunch
left a comment
sukangpunch
left a comment
There was a problem hiding this comment.
고생하셨습니다!! 궁금한 점 리뷰 달았습니다.
| boolean isRegistered, | ||
| String accessToken, | ||
| String refreshToken) implements OAuthResponse { | ||
| @JsonIgnore String refreshToken) implements OAuthResponse { |
There was a problem hiding this comment.
제 생각에 Response 객체는 http 응답 본문을 나타내는 객체라고 생각하여 명확하게 엑세스 토큰과 isRegisterd 필드만 담은 response dto를 따로 두어 컨트롤러에서 매핑하여 리턴하는 방식은 어떠신가요?
There was a problem hiding this comment.
저도 동의합니다~ isRegisterd이건 왜 필요한건가요??
전 내부에서 쿠키에 세팅하는 객체를 예를들어
public record SignInResult(
AccessToken accessToken,
RefreshToken refreshToken
) { }
이런식으로 쓰고
controller에서 응답용으론
public record SignInResponse(
String accessToken
) { }
이렇게 하면 되는 거 아닌가 했는데 놓친 히스토리가 있나 싶어서요!
There was a problem hiding this comment.
그러네요 코드에서는 isRegistered 필드를 참조하는 로직이 없네요 이 부분도 말씀하신 내용 반영하면서 수정하겠습니다 !
| boolean isRegistered, | ||
| String accessToken, | ||
| String refreshToken) implements OAuthResponse { | ||
| @JsonIgnore String refreshToken) implements OAuthResponse { |
There was a problem hiding this comment.
저도 동의합니다~ isRegisterd이건 왜 필요한건가요??
전 내부에서 쿠키에 세팅하는 객체를 예를들어
public record SignInResult(
AccessToken accessToken,
RefreshToken refreshToken
) { }
이런식으로 쓰고
controller에서 응답용으론
public record SignInResponse(
String accessToken
) { }
이렇게 하면 되는 거 아닌가 했는데 놓친 히스토리가 있나 싶어서요!
3 participants
관련 이슈
작업 내용
로그인 응답에서 refresh token은 제외하도록@JsonIgnore어노테이션을 붙였습니다.아예 해당 필드는 제거할 수는 없는게, 응답 DTO에서 refresh token 필드를 통해 쿠키를 설정하고 있습니다. 따라서 프론트에게 전달만 되지 않도록 설정했습니다.
리뷰 반영하여 별도 DTO를 생성했습니다.
더 좋은 방법이 있다면 말씀해주세요 ~
추가로
sign-up엔드포인트에는 쿠키로 리프레시 토큰을 전달하고 있지 않아서(응답으로 전달하고 있었음), 쿠키로 전달하도록 추가 반영했습니다 !특이 사항
리뷰 요구사항 (선택)