Feat/#425 카카오 소셜 로그인을 구현한다. #434

seokhwan-an · 2023-09-20T04:20:34Z

📝작업 내용

이번에 카카오 소셜 로그인을 구현하고 기존의 구글 소셜 로그인과 추상화하는 작업을 진행했습니다. 발급된 토큰을 저장하는 기능은 #424 가 merge되면 추가하겠습니다!. 그리고 카카오 소셜 로그인과 구글 소셜 로그인이 공존하면서 발생했던 문제점이 하나 있었습니다. 이 부분에 대해서 스플릿과 이야기를 나누었는데 베로와 바론의 의견도 듣고 싶어서 상황설명을 하겠습니다.

현재 구글 소셜 로그인을 통해 email을 받아왔고 이를 통해 사용자를 식별하는 것으로 이용했습니다.
카카오 소셜 로그인은 email이 사용자 동의에 의해서만 email을 받아올 수 있습니다.
- email을 가져오는 것을 필수로 하려면 검수를 받아야합니다.
그래서 카카오는 플랫폼 id를 통해서 사용자를 식별하는 것으로 했습니다.
- 카카오 닉네임으로 사용자를 식별할까 했는데 닉네임은 다른 사용자와 같을 수 있는 경우도 있고 사용자가 카카오 닉네임을 변경할 수 있는 어려움이 있었습니다.
그래서 현재 테이블의 member의 email 열을 구글 소셜 로그인은 email을 카카오 소셜 로그인은 플랫폼 id를 저장하고 회원가입시 사용자를 식별하고 이용하는 방식으로 하고자 했습니다. 혹시 email 열의 이름을 무엇으로 바꾸면 좋은지와 혹시 이 방법 외에도 더 좋은 방안이 있다면 이야기 해주세요!

💬리뷰 참고사항

소셜 로그인 추상화를 해보았는데 그 부분을 집중적으로 확인 부탁드립니다!

#️⃣연관된 이슈

close [FEAT] 카카오 소셜 로그인을 구현한다. #425

github-actions · 2023-09-20T04:22:43Z

Unit Test Results

  82 files   82 suites 12s ⏱️
313 tests 313 ✔️ 0 💤 0 ❌
316 runs 316 ✔️ 0 💤 0 ❌

Results for commit e45050a.

♻️ This comment has been updated with latest results.

somsom13

고생하셨습니다 아코쨩!^^ 소셜로그인의 달인이 되셨군요.
몇 가지 질문과 피드백 남겼습니다!

그리고 OAuthProvider 추상화는 아코가 고민하시는 걸 옆에서 봤고, 지금 제 지식으로는 더 좋은 방법이 떠오르지 않아서 피드백 남기지 않았습니다. (✿◠‿◠)

email과 카카오 고유 ID의 경우에는 UserIdentifier 같은 이름이면 좋을 것 같다는 생각이 들어요.
기존의 PK와는 혼동되지 않아야 할 것 같긴 하지만요 ^__^

somsom13 · 2023-09-20T06:27:25Z

backend/src/main/java/shook/shook/auth/application/AuthService.java

@@ -16,19 +14,17 @@
 public class AuthService {

    private final MemberService memberService;
-    private final GoogleInfoProvider googleInfoProvider;
+    private final OAuthExecutionFinder oauthExecutionFinder;


이름이 ExecutionFinder 로 결정된 이유가 궁금해용 ㅎㅎ
Provider를 가져와주는 객체인 것 같아서 OAuthProviderFinder 로 지었을 수도 있었을 것 같아서요!

바론이 말해준 네이밍이 더 와닿는 것 같습니다. 저는 실행기를 찾는다에 집중해서 Execution이라는 네이밍을 이용했었습니다. 네이밍은 항상 어렵네요..😥

somsom13 · 2023-09-20T06:43:58Z

backend/src/main/java/shook/shook/auth/application/AuthService.java

+        final String accessTokenResponse = oAuthInfoProvider.getAccessToken(authorizationCode);
+        final String memberInfo = oAuthInfoProvider.getMemberInfo(accessTokenResponse);


getAccessToken 은 oauth로부터 받아온 accessToken을 반환하는 메서드고, getMemberInfo 는 해당 accessToken 으로부터 memberInfo 를 파싱해오는 것 같아요.

getAccessToken 메서드가 이 곳에서만 사용되고, accessToken을 oauth로부터 받는 것 ~ memberInfo를 만드는 것 까지 모두 oauthInfoProvider에서 이루어지는 것 같은데 메서드를 분리하신 이유가 있을까요?

일단 이 accessToken을 가져오는 것과 memberInfo를 가져오는 것을 분리한 이유는 일단 두 기능 모두 Oauth에서 핵심이라고 생각을 해서 분리를 했습니다. 그리고 accessToken을 가져오는 것에 대한 메소드를 public으로 둔 이유는 AuthService에서 이 흐름을 명시해주어서 oauth의 흐름이 더 쉽게 파악할 수 있을 것 같다는 생각을 했었습니다. 또한 각각 기능 모두 핵심이다보니 테스트를 진행하기 위해서 Public으로 두고 테스트를 진행했던 이유도 있습니다.(물론 현재는 RestTemplate을 mock으로 두고 예외테스트만 진행하고 있습니다.)

somsom13 · 2023-09-20T06:45:45Z

backend/src/main/java/shook/shook/auth/application/GoogleInfoProvider.java

+            return Objects.requireNonNull(response.getBody()).getEmail();
        } catch (HttpClientErrorException e) {
            throw new OAuthException.InvalidAccessTokenException();
        } catch (HttpServerErrorException e) {
            throw new OAuthException.GoogleServerException();
        }


requireNonNull 를 사용하면 검증하려는 파라미터가 null이면 NullPointerException이 발생한다 하는데, 이에 대한 예외 처리는 없어도 괜찮은가요??

그 부분까지 고려하지 못했었네요. 일단 여기는 외부 api를 이용해서 요청을 보내고 응답을 받는 과정이기 때문에 null이 넘어오는 상황이 없다고 생각을 했습니다. 즉, null이 넘어온다는 것은 이미 서버 error가 발생한다라고 생각을 했고 이는 이미 try catch를 통해서 처리를 하고 있어서 이 부분을 고려하지 않았었네요 null requireNonNull을 이용한 만큼 이 부분도 예외처리를 추가하겠습니다!

somsom13 · 2023-09-20T06:46:54Z

backend/src/main/java/shook/shook/auth/application/GoogleInfoProvider.java

@@ -76,7 +78,7 @@ public GoogleAccessTokenResponse getAccessToken(final String authorizationCode)
                params,
                GoogleAccessTokenResponse.class);

-            return Objects.requireNonNull(response.getBody());
+            return Objects.requireNonNull(response.getBody()).getAccessToken();


위 커멘트와 동일합니다! 😄

somsom13 · 2023-09-20T06:51:12Z

backend/src/main/java/shook/shook/auth/application/KakaoInfoProvider.java

+    private static final String TOKEN_PREFIX = "Bearer ";
+    private static final String GRANT_TYPE = "authorization_code";
+    private static final String AUTHORIZATION_HEADER = "Authorization";


GoogleInfoProvider가 가지는 상수와 똑같은 것 같아요! 둘을 통일하는 것은 어떨까요?

oauth에 따라 요구하는 grant type, token prefix 등이 달라질 수 있어서 분리해두신건가요??

현재 이 부분은 googlelnfoProvider와 kakaoInfoProvider 에서 모두 공통으로 이용되고 있는데 이 둘을 추상화한 OAuthInfoProvider에서 이용할 수 있게 하려면 public으로 두어야 하는 상황인데 이 부분에 대해서는 어떻게 처리하면 좋을까요?

somsom13 · 2023-09-20T06:52:26Z

backend/src/main/java/shook/shook/auth/application/OAuthType.java

+
+public enum OAuthType {
+
+    GOOGLE, KAKAO;


somsom13 · 2023-09-20T06:54:31Z

backend/src/main/java/shook/shook/auth/ui/AuthController.java

+//    @GetMapping("/login/kakao")
+//    public ResponseEntity<LoginResponse> kakaoLogin(
+//        @RequestParam("code") final String authorizationCode,
+//        final HttpServletResponse response
+//    ) {
+//        System.out.println(authorizationCode);
+//        final TokenPair tokenPair = authService.kakaoLogin(authorizationCode);
+//        final Cookie cookie = cookieProvider.createRefreshTokenCookie(tokenPair.getRefreshToken());
+//        response.addCookie(cookie);
+//        final LoginResponse loginResponse = new LoginResponse(tokenPair.getAccessToken());
+//        return ResponseEntity.ok(loginResponse);
+//    }


w(ﾟДﾟ)w ┌( ´_ゝ` )┐

이 부분 지웠어야 했는데 실수를 범했네요.. 몹시 부끄럽네요😥

somsom13 · 2023-09-20T07:00:40Z

backend/src/main/java/shook/shook/member/application/MemberService.java

+        final Member newMember = new Member(email, BASIC_NICKNAME);
+        final Member savedMember = memberRepository.save(newMember);
+        savedMember.updateNickname(savedMember.getNickname() + savedMember.getId());
+        return savedMember;


여기서 updateNickname을 기본 닉네임 + 고유 id 로 연결된 문자열 (ex. shook1203) 으로 구성되는 것이 맞나용?? 고유 id를 붙이는 이유가 사용자들의 닉네임이 중복되지 않게 하기 위한 것인가요?.?

중복 닉네임을 막기 위해서라면 UUID 를 사용하면 되지 않을까요?!

현재는 db의 id로 하는 것이 중복되는 가능성이 없어서 했는데 더 고민해볼께요

somsom13 · 2023-09-20T07:02:33Z

backend/src/test/java/shook/shook/auth/application/OAuthTypeTest.java

+        assertThat(result).isEqualTo(expect);
+    }
+
+    private static Stream<Arguments> requestOauthTypeAndResult() {


Cyma-s

크게 변경점은 없어서 어프루브했습니다!
email 관련된 부분은 providerId 같은 컬럼으로 두고, 플랫폼을 저장하는 provider 열을 추가하는 건 어떨까요?

Cyma-s · 2023-09-20T14:04:09Z

backend/src/main/java/shook/shook/auth/application/KakaoInfoProvider.java

+
+    private static final String TOKEN_PREFIX = "Bearer ";
+    private static final String GRANT_TYPE = "authorization_code";
+    private static final String AUTHORIZATION_HEADER = "Authorization";


해당 상수는 HttpHeaders 에 정의되어 있는 상수인데, 재정의하지 않고 해당 상수를 쓰는 것에 대해서는 어떻게 생각하시나요??

AUTHORIZATION 헤더는 HttpHeaders에 정의된 상수를 이용했습니다 혹시 Bearer과 authorization_code 도 정의 되어 있는 것인지 궁금합니다. 저는 찾아도 안나타나네요.

Cyma-s · 2023-09-20T14:07:41Z

backend/src/main/java/shook/shook/auth/application/KakaoInfoProvider.java

+            params.add("code", authorizationCode);
+
+            HttpHeaders headers = new HttpHeaders();
+            headers.set(HttpHeaders.CONTENT_TYPE, "application/x-www-form-urlencoded;charset=utf-8");


이 부분도 상수를 사용해서 변경해볼 수 있을 것 같은데 어떤가요??

이 부분도 상수로 빼겠습니다!

Cyma-s · 2023-09-20T14:13:03Z

backend/src/main/java/shook/shook/auth/application/dto/KakaoMemberInfoResponse.java

+    @JsonProperty("id")
+    private Long id;


필요한 필드가 스네이크 케이스로 되어 있지 않더라도 JsonProperty 를 붙여주어야 하나요?

불필요한 JsonProperty 제거하겠습니다!

Cyma-s · 2023-09-20T14:19:57Z

backend/src/main/java/shook/shook/member/application/MemberService.java

+        final Member newMember = new Member(email, BASIC_NICKNAME);
+        final Member savedMember = memberRepository.save(newMember);
+        savedMember.updateNickname(savedMember.getNickname() + savedMember.getId());
+        return savedMember;


중복 닉네임을 막기 위해서라면 UUID 를 사용하면 되지 않을까요?!

세부사항 - 소셜 로그인 과정 중에 accessToken을 가져오는 것과 사용자 정보를 조회하는 부분을 추상화 - authService는 각 infoProvider를 의존하는것이 아닌 OAuthInfoProvider를 의존하도록 수정

세부사항 - 예외 핸들링 원복 - 불필요한 로그 제거

세부사항 - 명확한 클래스 명칭으로 수정 - requireNonNull을 이용할 시 nullPointException 처리 - 상수 분리

seokhwan-an added [ 🌙 BE ] 백엔드 크루들의 멋진 개발 이야기 하나둘셋 야! ✨ Feat 꼼꼼한 기능 구현 중요하죠 labels Sep 20, 2023

seokhwan-an added this to the [5차] 성능 개선 및 사용자 피드백 반영된 기능 구현 milestone Sep 20, 2023

seokhwan-an self-assigned this Sep 20, 2023

seokhwan-an requested review from Cyma-s, splitCoding and somsom13 as code owners September 20, 2023 04:20

somsom13 requested changes Sep 20, 2023

View reviewed changes

Cyma-s approved these changes Sep 20, 2023

View reviewed changes

seokhwan-an added 12 commits September 21, 2023 18:17

refactor: email 형식 제약조건 변경하기

84a6fbe

refactor: member의 nickname을 부여하는 방식 변경

1cb05b4

feat: 카카오 로그인 기능 추가

7b5bf3c

refactor: OAuth 추상화하기

7975053

세부사항 - 소셜 로그인 과정 중에 accessToken을 가져오는 것과 사용자 정보를 조회하는 부분을 추상화 - authService는 각 infoProvider를 의존하는것이 아닌 OAuthInfoProvider를 의존하도록 수정

refactor: QA를 위한 수정

c010608

refactor: CI 실패 수정

20fcb39

refactor: 에러 상세화를 위한 수정

5162de7

refactor: 카카오 로그인 확인을 위한 로그 추가

7a426ec

refactor: 서버 엑세스 코드 확인 위한 로그 추가

4a0cda6

refactor: 서버 엑세스 코드 확인 위한 로그 추가

0edb8bc

refactor: QA를 종료후 기존의 코드 원복

5c8740d

세부사항 - 예외 핸들링 원복 - 불필요한 로그 제거

refactor : 코드리뷰 반영

e74e5cc

세부사항 - 명확한 클래스 명칭으로 수정 - requireNonNull을 이용할 시 nullPointException 처리 - 상수 분리

seokhwan-an force-pushed the feat/#425 branch from 6a131b1 to e74e5cc Compare September 21, 2023 09:23

seokhwan-an and others added 3 commits September 21, 2023 18:28

refactor: CI 실패하는 오류 수정

b7c5028

refactor: 서브모듈 최신화

24edbf7

Merge branch 'main' into feat/#425

e45050a

seokhwan-an merged commit d78c279 into main Sep 21, 2023

seokhwan-an deleted the feat/#425 branch September 21, 2023 09:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/#425 카카오 소셜 로그인을 구현한다. #434

Feat/#425 카카오 소셜 로그인을 구현한다. #434

seokhwan-an commented Sep 20, 2023 •

edited

Loading

github-actions bot commented Sep 20, 2023 •

edited

Loading

somsom13 left a comment •

edited

Loading

somsom13 Sep 20, 2023

seokhwan-an Sep 20, 2023

somsom13 Sep 20, 2023

seokhwan-an Sep 20, 2023

somsom13 Sep 20, 2023 •

edited

Loading

seokhwan-an Sep 20, 2023

somsom13 Sep 20, 2023

somsom13 Sep 20, 2023

seokhwan-an Sep 20, 2023

somsom13 Sep 20, 2023

somsom13 Sep 20, 2023

seokhwan-an Sep 20, 2023

somsom13 Sep 20, 2023

Cyma-s Sep 20, 2023

seokhwan-an Sep 21, 2023 •

edited

Loading

somsom13 Sep 20, 2023

Cyma-s left a comment

Cyma-s Sep 20, 2023

seokhwan-an Sep 21, 2023

Cyma-s Sep 20, 2023

seokhwan-an Sep 21, 2023

Cyma-s Sep 20, 2023

seokhwan-an Sep 21, 2023

Cyma-s Sep 20, 2023

		final String accessTokenResponse = oAuthInfoProvider.getAccessToken(authorizationCode);
		final String memberInfo = oAuthInfoProvider.getMemberInfo(accessTokenResponse);

Feat/#425 카카오 소셜 로그인을 구현한다. #434

Feat/#425 카카오 소셜 로그인을 구현한다. #434

Conversation

seokhwan-an commented Sep 20, 2023 • edited Loading

📝작업 내용

💬리뷰 참고사항

#️⃣연관된 이슈

github-actions bot commented Sep 20, 2023 • edited Loading

Unit Test Results

somsom13 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

somsom13 Sep 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seokhwan-an Sep 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cyma-s left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seokhwan-an commented Sep 20, 2023 •

edited

Loading

github-actions bot commented Sep 20, 2023 •

edited

Loading

somsom13 left a comment •

edited

Loading

somsom13 Sep 20, 2023 •

edited

Loading

seokhwan-an Sep 21, 2023 •

edited

Loading