Skip to content

Feature/issue 161#163

Open
potential1205 wants to merge 3 commits intomainfrom
feature/issue-161
Open

Feature/issue 161#163
potential1205 wants to merge 3 commits intomainfrom
feature/issue-161

Conversation

@potential1205
Copy link
Contributor

@potential1205 potential1205 commented Nov 30, 2024

연관 이슈

키워드와 이슈번호를 작성해주세요

리뷰 요구사항

리뷰어가 중점적으로 봐야할 부분을 작성해주세요

  • redirectUri 및 clientId 등 환경 변수 처리와 요청 흐름 검증
  • checkKakaoOauthUser에서 회원 유무를 정확히 조회하고 예외 상황이 올바르게 처리되었는지 확인
  • 회원가입 로직(joinKakaoOauthUser)에서 입력 데이터가 정확히 저장되는지 확인
  • 외부 API 호출 실패(getToken, getUserInfo) 및 서버 내부 예외 처리(IOException 등)가 충분히 고려되었는지 확인
  • 각 예외가 적절한 HTTP 상태 코드와 메시지를 반환하는지 확인
  • KakaoOauthClient에 작성된 logout, reissue는 당장 사용하지 않지만 앞으로 사용할 예정임

public void retrieveKakaoOauthAuthorization(HttpServletResponse httpServletResponse) {
String requestUrl = String.format(
"%s?response_type=%s&client_id=%s&redirect_uri=%s",
kakaoOauthUrl, "code", clientId, redirectUri
Copy link
Contributor

Choose a reason for hiding this comment

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

"code" 는 string 안에 넣어도 되지 않을까요? 굳이 변수로 빼낸 이유가 있나요?

@Override
public void checkKakaoOauthUser(String accessToken) {
try {
KakaoOauthUserInfoResp kakaoOauthUserInfoResp = kakaoOauthClient.getUserInfo(accessToken);
Copy link
Contributor

Choose a reason for hiding this comment

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

getUserInfo() 는 직접 생성된 client의 메서드인데, 왜 try catch를 여기서 하는거죠?
getUserInfo() 내부에서 이루어져야될 것 같습니다. 만약 필요하다면

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

카카오 OAuth 기능 개발

2 participants