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/LS-6: 카카오, 구글 OAuth2 #16

Merged
merged 17 commits into from
Jul 10, 2024
Merged

feat/LS-6: 카카오, 구글 OAuth2 #16

merged 17 commits into from
Jul 10, 2024

Conversation

clean2001
Copy link
Collaborator

@clean2001 clean2001 commented Jul 7, 2024

📝 PR 타입

  • 기능 추가
  • 기능 수정
  • 기능 삭제
  • 리팩토링
  • docs 작업, swagger 작업
  • 의존성, 환경 변수, 빌드 관련 코드 업데이트

📢 변경 사항

  • 로그인 시 카카오 소셜 로그인 기능을 추가했습니다.
  • 로그인 시 구글 소셜 로그인 기능을 추가했습니다.

구현 내용 노션에 간단하게 문서화 하여 내일(7/8) 중으로 공유해드리겠습니다!

❗️To Reviewer

  • 회원이 아닌 유저가 로그인하는 상황에 대해
    회원이 아닌 유저가 로그인하면, 이름을 입력받아서 회원 가입을 할 수 있도록 하는 상황을 가정하고 구현했습니다.
    따라서 소셜로그인 한 유저가 DB에 없다면 Exception을 발생시키는 상황입니다.
    이런 방식이 괜찮은지 의견 여쭙고 싶습니다!

  • 회원 탈퇴의 방식
    현재 "회원 탈퇴" api는 구현하지 않은 상황입니다.
    탈퇴 기능을 구현할 때, Member 엔티티의 삭제 여부 필드(예를 들면 delYn)를 true로 바꿈으로써 구현할 생각이었는데, Member 엔티티에 삭제 여부 필드가 없어서 아직은 구현하지 않았습니다.
    @mikekks 혹시 BaseEntity를 안만드신 이유가 있으신 건지 궁금합니다.
    또한 탈퇴를 어떻게 처리하면 좋을지 의견 여쭙고 싶습니다!

⚙️ 테스트 결과

  • 회원가입이 되지 않은 채로 소셜 로그인 시도
image
  • 회원 가입 (유저에게서 이름을 입력받음)
    image

  • 회원 가입이 된 유저가 소셜로그인

image

👉 반영 브랜치

@clean2001 clean2001 added the 🚀 feature 새로운 기능 개발 label Jul 7, 2024
@clean2001 clean2001 self-assigned this Jul 7, 2024
Copy link
Collaborator

@mikekks mikekks left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 궁금한 부분은 질문 남겼습니다!!

추가로 PR에 대한 제 생각 공유해드리겠습니다!

  1. "회원이 아닌 유저가 로그인하는 상황에 대해"
  • 이 부분은 클라와 합의가 된 방식이라면 좋은 것 같습니다!!
  1. "회원 탈퇴의 방식"
  • 회원탈퇴는 soft delete 방식 생각하신게 맞을까요?? 좋은 것 같습니다! 거기까진 고려가 안돼서 컬럼 추가가 필요할 것 같아요! 그렇다면 나중에 batch 작업하는 것도 추가해야 겠네요!
  1. BaseEntity
  • 이 부분은 제가 놓친 것 같습니다..! 추가하는게 좋을 것 같은데 모든 엔티티에 추가하는게 좋을까요?!

Comment on lines +1 to +3
dependencies {
implementation project(path: ':layer-domain')
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기 build.gradle은 일부러 안지우신건가요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

앗 저거는 external에서 domain 의존성이 필요한 경우가 있었는데, option + enter 쳤더니 인텔리제이가 알아서 만들어 준것 같아요
다시 작업 할 때 파일 삭제하고 전체 build.gradle에 추가하겠습니다

Comment on lines +47 to +49
} catch(Exception e) {
throw new BaseCustomException(INVALID_TOKEN);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

엇 여기 BaseCustomException 은 TokenException extends BaseCustomException 식으로 상속해서 사용하는 것을 의도했었는데요! 혹시 어떻게 생각하시나요?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아 그게 더 좋을 것 같습니다!
수정하겠습니다

return switch (socialType) {
case KAKAO -> kakaoService.getMemberInfo(socialAccessToken);
case GOOGLE -> googleService.getMemberInfo(socialAccessToken);
default -> throw new BaseCustomException(INVALID_SOCIAL_TYPE);
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기도 AuthException extends BaseCustomExcepiton 으로 의도했는데 어떻게 생각하시나요?!

Comment on lines 35 to 37
if(memberOpt.isPresent()) {
throw new BaseCustomException(NOT_A_NEW_MEMBER);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

요기도 같은 맥락입니다!

Comment on lines -42 to 45
throw new TokenException();
throw new BaseCustomException(INVALID_REFRESH_TOKEN);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

요기도 같은 맥락입니다!

Comment on lines 109 to 111
if(!currentMember.getId().equals(memberId)) {
throw new BaseCustomException(FORBIDDEN);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 Member 도메인 내 함수로 하는 건 어떠신가요? 그렇게 하면 장점으로 설계가 더 도메인의 역할이 중요해지고, 테스트 코드 작성도 용이해지기에 말씀드려봤습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

그게 더 좋을 것 같습니다
그렇게 수정하겠습니다!

@raymondanythings raymondanythings merged commit 332a261 into develop Jul 10, 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