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

[Feature] Spring Security를 이용한 OAuth 2.0(KAKAO, NAVER) 로그인 구현 #30

Merged
merged 27 commits into from
Nov 6, 2023

Conversation

hojeong2747
Copy link
Member

💡 연관된 이슈

close #29

📝 작업 내용

  • OAuth2 Client 의존성 추가
  • ClientConfig/OAuthApiClient 인터페이스 추가
  • Kakao/NaverApiClient 컴포넌트 클래스 추가
  • OAuthInfoResponse 인터페이스 추가
  • Kakao/NaverInfoResponse 클래스 추가
  • RequestOAuthInfoService 컴포넌트 클래스 추가
  • OAuthController/Service/Dto 추가
  • 이동봉사자 소셜 로그인 API 구현
  • Jwt 토큰 재발행 API 구현
  • 이동봉사자 소셜 로그인 추가 회원가입 API 구현

💬 리뷰 요구 사항

  • JwtService 인증 허가 메소드 소셜 로그인 분기 처리를 했는데, username으로 socialId 값을 넣어주는 게 문제 없을지 궁금!
  • role을 되게 야무지게 사용하고 있는데 (= 여러 군데에서 다양하게) 샥 검토 해주시면 감사하겠습니당 (로컬 테스트는 완료!)

@hojeong2747 hojeong2747 added ✨ Feature 기능 개발 Priority: Medium 우선순위 중간 🐰 Hojeong 담당자 labels Nov 5, 2023
Copy link
Member

@kyeong-hyeok kyeong-hyeok left a comment

Choose a reason for hiding this comment

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

코드 하나하나 생각하고 짠 게 보여요! 👍 수고하셨습니다~~
리뷰 한 번 확인해 주세요!

private void reIssueRefreshAndAccessToken(Long id, String roleName) {
public LoginResponse reIssueToken(String refreshToken) {

// AccessToken 으로 이동봉사자 찾기
Copy link
Member

Choose a reason for hiding this comment

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

정말 정말 정말 사소한 주석..ㅎㅎ

security = { @SecurityRequirement(name = "bearer-key") },
responses = {@ApiResponse(responseCode = "204", description = "이동봉사자 소셜 로그인 추가 회원가입 성공")
, @ApiResponse(responseCode = "400"
, description = "V1, 닉네임은 한글, 숫자만 사용 가능합니다. \t\n V1, 닉네임은 필수 입력 값입니다. \t\n V1, 2~10자의 닉네임이어야 합니다 \t\n A2, 이미 사용 중인 닉네임입니다. \t\n " +
Copy link
Member

Choose a reason for hiding this comment

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

닉네임은 한글, 숫자만 사용 가능합니다.에 대한 Validation 처리가 SocialSignUpRequest에서 처리가 안 되고 있습니다!
그리고 위에 내 거 복사한 거 같은데 닉네임이어야 합니다 맨 끝에 점(.) .. ㅎㅎ 위에도 추가해 주세요

Copy link
Member Author

Choose a reason for hiding this comment

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

마침표까지 찾는 폼 .. 대박

record를 처음 쓰기도 하고 커스텀 V1 예외와 swagger 명세에 예외 처리를 다 적는 게 처음이라 아직 활용이 익숙치 않은 느낌이었는데요, 덕분에 정리가 된 느낌입니다! 꼼꼼하고 친절한 개발자 👍

@PatchMapping("/volunteers/sign-up/social")
public ResponseEntity<LoginResponse> volunteerSocialSignUp(@RequestBody SocialSignUpRequest socialSignUpRequest, HttpServletRequest request) {
LoginResponse response = authService.volunteerSocialSignUp(socialSignUpRequest, request);
return ResponseEntity.ok(response);
Copy link
Member

Choose a reason for hiding this comment

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

추가 회원가입 시에 LoginResponse가 필요할까요?! 로그인 시에 accessToken과 refreshToken을 생성해 반환하지만, 추가 회원가입 시에는 토큰을 생성해서 반환하지 않아도 될 것 같다고 생각합니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

논의 완료~! 추가 회원가입 시 토큰 재발행은 해주지 않기!

, content = @Content(schema = @Schema(implementation = ErrorResponse.class)))
})
@PatchMapping("/volunteers/sign-up/social")
public ResponseEntity<LoginResponse> volunteerSocialSignUp(@RequestBody SocialSignUpRequest socialSignUpRequest, HttpServletRequest request) {
Copy link
Member

Choose a reason for hiding this comment

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

HttpServletRequest 대신 @AuthenticationPrincipal로 로그인한 사용자의 정보를 가져오는 방식으로 진행해도 좋을 것 같습니다! 그러면 Service의 로직들이 조금 더 간편해질 것 같아요! 밑에서 조금 더 자세히 설명하겠습니다.


// 토큰 재발행
String refreshToken = redisUtil.get(roleName, id);
return jwtService.reIssueToken(refreshToken);
Copy link
Member

Choose a reason for hiding this comment

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

아까 앞에서 말한 것을 설명해 본다면 다음과 같습니다!
Controller에서 @AuthenticationPrincipal UserDetails loginUser로 받아왔다면, VolunteerLoginService에서 구현한 UserDetails이 반환되는데 loginUser에 email과 role의 값이 있어 바로 가져올 수 있고, 해당 email로 회원을 찾는다면 좀 더 간편해질 것 같다고 생각돼요!

하지만 문제가 있습니다. 저희가 지금 소셜 로그인의 경우 이메일을 null값으로 두었는데 @AuthenticationPrincipal을 사용한다면 이메일을 랜덤으로 생성해 저장해야 할 것 같습니다! 그렇게 한다면 로그인한 사용자의 정보를 조금 더 쉽게 가져올 수 있을 것 같아요!
@AuthenticationPrincipal은 UserDetailsService에서 Return한 객체를 파라미터로 직접 받아 사용하는데 저희가 UserDetailsService를 구현한 클래스는 IntermediaryLoginService, VolunteerLoginService 두 가지로 모두 username을 이메일로 사용하고 있습니다!
(만약 socialId도 username으로 사용하게 된다면 이동봉사자 회원의 경우 해당 어노테이션을 사용했을 때 username이 socialId인지 email인지 구분이 필요해 로직이 추가될 것 같습니다.)

이걸 바꾸게 된다면 수정해야 할 부분

  1. JwtService의 saveVolunteerAuthentication 로직
  • username을 socialId 혹은 email로 설정하는데 이렇게 했을 때 호정님이 구현한 VolunteerLoginService에서 username을 email로 설정한 것에 차이가 있습니다! 인증 정보를 저장할 때와, 로그인한 사용자 정보를 불러올 때의 username의 값에 차이가 존재하는데, 프로젝트 내에서 일관되게 사용하는 게 좋다고 생각됩니다.
    (서로 다른 username을 사용했을 때 정확히 어떤 문제가 일어날지는 모르지만! username을 통일한다면 유지보수, 데이터 통합 측면으로 이점이 있다고 생각합니다.)

추가
전에 말했던 것처럼 토큰 재발행 부분은 추가 회원가입 시 빠져도 좋을 것 같다고 생각됩니다!
회원가입 시에는 토큰 발급을 해주지 않아도 될 것 같습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

구현되어 있는 로직 사용만 해봤지, 직접 구현하는 건 처음이라 안 그래도 HttpServletRequest,@AuthenticationPrincipal 사용 차이가 궁금해서 메모장에 적어놨는데! 어떻게 알고 이렇게 꼼꼼하게 알려주네용 ㅎ.ㅎ VolunteerLoginService를 생각하지 못했는데 통일된 로직으로 변경해야 할 것 같습니다.

반영하여 변경된 사항은 다음과 같습니다!

  • saveVolunteerAuthentication 에서 동일하게 username에 email 저장
  • 소셜 로그인 시 소셜 로그인 유저도 loginUser 사용 위한 email 랜덤 저장
  • 토큰 재발행 부분은 추가 회원가입 시 X

, content = @Content(schema = @Schema(implementation = ErrorResponse.class)))
})
@PostMapping("/reissue-token")
public ResponseEntity<LoginResponse> reIssueToken(@Valid @RequestBody RefreshTokenRequest request) {
Copy link
Member

Choose a reason for hiding this comment

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

정말 사소한 건데 제약 조건이 없다면 @Valid는 빼도 좋을 것 같습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

디테일 최고다! 필수 전송 값으로 validation 추가 했으니 한 번 봐주세요~! 유지보수에서는 Dto에서 어떤 값이 필수인지 넣어주는 게 좋았어서 넣었습니다!

@Operation(summary = "이동봉사자 소셜 로그인", description = "이동봉사자 소셜 로그인을 합니다.",
responses = {@ApiResponse(responseCode = "204", description = "이동봉사자 소셜 로그인 성공")
, @ApiResponse(responseCode = "400"
, description = ""
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
Member Author

Choose a reason for hiding this comment

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

ㅎㅎ 비워두고 까먹었네요. 진짜 잘 찾아준다 .. 👍

}

private Long saveVolunteer(SocialType socialType, String socialId) {
Volunteer createdVolunteer = new Volunteer(VolunteerRole.GUEST, socialType, socialId);
Copy link
Member

Choose a reason for hiding this comment

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

생성자를 이용할 수도 있고 Builder 패턴을 이용할 수도 있을 것 같아요 참고만!
빌더 패턴은 파라미터가 많아질 경우의 가독성을 높여줍니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

Lombok은 기본적으로 한 클래스에 하나의 빌더만을 지원한다고 합니다! 둘 이상의 생성자에 빌더를 사용하려면, 각 빌더에 builderMethodName 속성으로 빌더 메소드를 추가해서 사용할 수 있다고 하는데 -> Lombok의 기본 빌더 패턴과는 다소 다른 방법이라고 나오긴 하네요.

저도 빌더 쓰고 싶은데 뭐가 좋은 방법일까요~? 우선 많은 필드가 쓰이는 생성자 하나를 빌더로 쓰는 거로 하고 좀 더 알아보고 반영해 보면 좋을 것 같아요 !! :)

private Long saveVolunteer(SocialType socialType, String socialId) {
Volunteer createdVolunteer = new Volunteer(VolunteerRole.GUEST, socialType, socialId);

return volunteerRepository.save(createdVolunteer).getId();
Copy link
Member

Choose a reason for hiding this comment

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

헉 이렇게도 반환할 수 있구나 배워갑니다!

.password(password)
.roles(volunteer.getRole().name())
.build();
}
Copy link
Member

Choose a reason for hiding this comment

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

위에서 username 설정과 관련해서 말한 부분입니다! username을 통일하는 것이 좋을 것 같다고 생각하는데 혹시 호정님의 생각은 어떠신가요?

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
Member

@kyeong-hyeok kyeong-hyeok left a comment

Choose a reason for hiding this comment

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

좋아요! 빌더 패턴은 조금 더 알아보고 적용하면 좋을 것 같습니다! 👍


// 추가 정보 업데이트
String nickname = socialSignUpRequest.nickname();
volunteer.updateSocialVolunteer(nickname, VolunteerRole.VOLUNTEER); // GUEST -> VOLUNTEER
Copy link
Member

Choose a reason for hiding this comment

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

로직이 훨씬 더 간편해졌네요!! 토큰 검증 후 해당 Service 로직에 접근한 것이라, 토큰 관련 Exception이 없어지도록 코드를 작성한 것 좋습니다!

public record SocialLoginRequest(
@NotBlank(message = "AccessToken은 필수 전송 값입니다.")
String accessToken,
@NotNull(message = "provider는 필수 전송 값입니다.")
Copy link
Member

Choose a reason for hiding this comment

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

provider에는 NotBlank가 아닌 NotNull을 둔 이유가 따로 있을까요?!

Copy link
Member Author

Choose a reason for hiding this comment

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

네!! NotBlank로 하니 SocialType을 검증하기 위한 검증기를 찾을 수 없다는 오류가 나더라구요,
NotBlank 어노테이션은 문자열이 비어 있지 않아야 함을 나타내기 위한 건데, provider 타입이 SocialType이라 NotBlank를 직접 사용할 수 없다고 해요. enum이라 NotNull로 설정했습니다!

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
Member

@kyeong-hyeok kyeong-hyeok left a comment

Choose a reason for hiding this comment

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

소셜 추가 회원가입 부분만 수정하면 될 것 같습니다!

public ResponseEntity<Void> volunteerSocialSignUp(@AuthenticationPrincipal UserDetails loginUser, @RequestBody SocialSignUpRequest socialSignUpRequest) {
authService.volunteerSocialSignUp(loginUser.getUsername(), socialSignUpRequest);
return ResponseEntity.noContent().build();
}
Copy link
Member

Choose a reason for hiding this comment

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

소셜 추가 회원가입의 확정 와이어 프레임을 확인해 봤을 때 이용약관, 닉네임, 프로필 이미지가 있어서 추가해줘야 할 것 같습니다! 프로필 이미지는 번호로 받아서 SocialSignUpRequest의 세 가지 필드를 넣으면 될 것 같아요~

Copy link
Member Author

Choose a reason for hiding this comment

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

구현 방식도 설계해 주다니!! 감사합니다 (꾸벅)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feature 기능 개발 🐰 Hojeong 담당자 Priority: Medium 우선순위 중간
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Spring Security를 이용한 OAuth 2.0(KAKAO, NAVER) 로그인 구현
2 participants