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] oauth2 로그인 기능 추가 (구글,네이버,카카오) #37

Merged
merged 14 commits into from
Jan 8, 2024

Conversation

liyusang1
Copy link
Contributor

@liyusang1 liyusang1 commented Jan 7, 2024

🎯 목적

  • 새 기능 (New Feature)

🛠 작성/변경 사항

  • oauth2 로그인 기능(구글,네이버,카카오)가 추가 되었습니다.

🔗 관련 이슈


image
image
image

PrincipalOauth2UserService 클래스 추가
OAuth2LoginSuccessHandler 추가
SecurityConfig에 Oauth2 설정값 추가
OAuth2UserInfo 추가
GoogleUserInfo 추가
NaverUserInfo 추가
KaKaoUserInfo 추가
yml파일들에 oauth2 설정 추가
findByMemberBaseEmailAndProvider 추가
MemberEntity 수정
updateProfileImage 추가
Member Builder에서 provider 설정 가능
get OAuth2/info api 추가
oauth2 로그인 http 파일 추가
@liyusang1 liyusang1 added the feat 기능을 추가합니다. label Jan 7, 2024
@liyusang1 liyusang1 self-assigned this Jan 7, 2024
Copy link

github-actions bot commented Jan 7, 2024

Test Results

1 tests  ±0   1 ✅ ±0   0s ⏱️ ±0s
1 suites ±0   0 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit d5ef2ac. ± Comparison against base commit 2f08a20.

♻️ This comment has been updated with latest results.

test application.yml에 설정값 추가
Copy link
Member

@meena2003 meena2003 left a comment

Choose a reason for hiding this comment

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

어려운 코드였을텐데 코드를 잘 짜신것 같습니다.
고생하셨습니다 (따봉)


@GetMapping("/oauth2/info")
public ResponseEntity<ResponseDTO<LoginResponseDto>> oauth2Test(
@RequestParam String token, @RequestParam String email, @RequestParam String name) {
Copy link
Member

Choose a reason for hiding this comment

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

가독성을 위해 한 칸 내린 매개변수들은 한 번의 탭을 추가하는 건 어떨까요? 바로 밑 코드들과 같은 라인이라 구분이 되면 좋을듯 합니다.
예를 들어,

@GetMapping("/test/jwt")
public ResponseEntity<ResponseDTO<TestUserResponseDto>> test(
           @AuthenticationPrincipal PrincipalDetails principalDetails) {
      Member member = principalDetails.getMember();
      ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ctrl alt l로 google code java convetion에 맞춰 적용할 경우 자동으로 다시 수정되는 부분입니다.
google code java convetion을 사용하고 있기 때문에 이대로 하는게 맞는거같습니다.

Copy link
Member

Choose a reason for hiding this comment

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

예전 커밋이지만 지금 발견해서 말씀드립니다.
32번째 줄을 보면 메소드명은 signup인데, Dto명에선 signUp으로 되어 있어요.
일반적으로 사용하는 signUp으로 통일하면 어떨까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵 이부분 반영하겠습니다 실수한거같네요

Copy link
Member

Choose a reason for hiding this comment

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

memberBase 필드를 protected로 한 이유가 있나요? private으로 해도 되지 않나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

private 보다 더 넓은 범위에서 쓰기 위해 protected가 맞는거 같습니다.

@@ -7,5 +7,6 @@
public interface MemberRepository extends JpaRepository<Member, Long> {

Optional<Member> findByMemberBaseEmail(String email);
Optional<Member> findByMemberBaseEmailAndProvider(String email,String provider);
Copy link
Member

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.

google code java convetion을 적용하지 않았네요 반영해서 수정했습니다

code convention 적용하지 않은 부분 적용
@liyusang1 liyusang1 merged commit 24d108e into develop Jan 8, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat 기능을 추가합니다.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants