[YS-114] fix: OAuth 로직 버그 수정 및 Auth에 대한 리팩터링 적용 #22

Ji-soo708 · 2025-01-08T05:45:08Z

💡 작업 내용

AuthController 및 AuthService에 DIP 적용하여 리팩터링
GoogleOAuth 버그 해결
Naver/GoogleOAuth 로직 리팩토링

✅ 셀프 체크리스트

PR 제목을 형식에 맞게 작성했나요?
브랜치 전략에 맞는 브랜치에 PR을 올리고 있나요?
테스트는 잘 통과했나요?
빌드에 성공했나요?
본인을 assign 해주세요.
해당 PR에 맞는 label을 붙여주세요.

🙋🏻‍ 확인해주세요

@chock-cho 님과 먼저 논의가 필요한 부분이 있어 hold합니다

🔗 Jira 티켓

https://yappsocks.atlassian.net/browse/YS-114

…teway

Ji-soo708

클린 아키텍처에 위반된 Auth 로직을 수정하려 했는데 어쩌다 심각한 버그🐛를 발견한 거 같아서(?) 꽤 PR이 커졌네요... 수정님과 OAuth 관련해서 논의해야 해서 이 PR은 논의가 끝날 때까지 홀드하겠습니다.

해당 PR에서 크게 바뀐 부분은 다음과 같습니다
구글 OAuth의 경우, OAuth 로직 관련한 이슈때문에 수정하지 않았습니다.

AuthController/AuthService: 클린아키텍처에 위배된 부분을 수정
NaverOAuthSignIn: 전반적인 로직 수정 (코멘트 참고)
Auth 관련 UseCase 안에서 Repository를 바로 불러오는 부분을 Gateway를 통해 불러오도록 수정

Ji-soo708 · 2025-01-08T05:51:38Z

src/main/kotlin/com/dobby/backend/application/usecase/FetchNaverUserInfoUseCase.kt

+            } else {
+                // 등록된 멤버가 없으면 isRegistered = false, memberId = null
+                Output(
+                    isRegistered = false,
+                    accessToken = "",
+                    refreshToken = "",
+                    memberId = null,
+                    oauthEmail = email ?: "",
+                    oauthName = "",
+                    role = null,
+                    provider = ProviderType.NAVER
+                )
+            }
+        } catch (e: Exception) {
            throw SignInMemberException()


기존 NaverOAuth 로직을 DIP 원칙을 준수하도록 리팩터링한 뿐만 아니라 전체적으로 수정했습니다.

제가 이해한 흐름이 맞다면, 기존 로직에서는 그라밋에 회원가입되어 있지 않은 회원이 OAuth 로그인을 수행하면 isRegistered=false가 포함된 응답이 반환되는 것이 아니라 SignInMemberException 예외가 반환됩니다. 저번에 논의했듯이 isRegistered로 회원가입 여부를 나타내 프론트에서 회원가입 API 호출 여부를 결정하기로 했어서 이를 따르려면 해당 PR에서의 로직이 적합하다고 생각합니다!

수정된 NaverOAuthSignin 코드에서 이러한 로직이 정확히 반영되었는지 확인 부탁드립니다! 그리고 혹시, 수정님이 기존에 작성하셨던 GoogleOAuth가 이런 흐름으로 구현하신 게 맞다면 알려주세요~ (추가로 nullable이 많은 이유는 추후 네이버 OAuth를 테스트해야하기 때문입니다)

accessToken = "", refreshToken = "",

최근 커밋에서 가입하지 않은 회원일 때, 빈 문자열이 아니라 null을 반환하도록 수정했습니다.

헉 지수님께서 말씀하신 부분이 정확해요 😭

isRegistered 필드가 반영된 응답값 통일하는 과정에서 처리 관련 Bug가 발생한 것 같네요!
저는 GoogleOAuth였을 때 바로 SigninException 오류가 뜨고, 해당 오류가 반환되면 프론트에서 /signup으로 리다이렉팅해야 한다고 생각했었던 것 같아요.
근데 저번 회의로 응답값을 통일하기로 했으니까, 관련 로직들을 리팩토링 작업에서 전부 수정해야 할 것 같아요.

Ji-soo708 · 2025-01-08T05:52:21Z

src/main/kotlin/com/dobby/backend/domain/gateway/NaverAuthGateway.kt

+interface NaverAuthGateway {
+    fun getAccessToken(authorizationCode: String, state: String): String?
+    fun getUserInfo(accessToken: String): NaverInfoResponse?


이번에 리팩터링하면서 깨달았는데 FeignClient도 외부 라이브러리니까 Gateway를 선언해서 리팩터링을 해야할 거 같습니다!

네 그렇겠네요!

Ji-soo708 · 2025-01-08T05:53:08Z

src/test/kotlin/com/dobby/backend/application/usecase/FetchNaverUserInfoTest.kt

+        // 테스트 2: 등록된 멤버가 없는 경우
+        every { memberGateway.findByOauthEmailAndStatus("[email protected]", MemberStatus.ACTIVE) } returns mockEmptyMember
+
+        `when`("등록되지 않은 유저가 있는 경우") {
+            val result: FetchNaverUserInfoUseCase.Output = fetchNaverUserInfoUseCase.execute(input)
+
+            then("isRegistered는 false, memberId는 null이어야 한다") {
+                result.isRegistered shouldBe false
+                result.memberId shouldBe null
+                result.oauthEmail shouldBe "[email protected]"


제가 처음에 의도했던 인증 로직을 테스트코드로 작성했습니다!

chock-cho · 2025-01-08T16:33:32Z

src/main/kotlin/com/dobby/backend/application/usecase/FetchNaverUserInfoUseCase.kt

개인적인 의문인데요, 현재 FetchNaverUseInfoUseCase 에서 입력/반환 필드값이 많아짐에 따라서 해당 UseCase 안에 모든 Input, Output들을 데이터 클래스로 정의하면 UseCase 자체의 코드 길이가 너무 길어지고, 가독성이 떨어질 것 같다는 생각을 했어요.

그리고, 코드의 재사용성을 위해서 회원가입 로직을 구현한 저 또한 실험자/피험자에 대한 회원가입 응답값을 MemberResponse로 통일하며 구현하였기 때문에, 현재 방법으로는 많은 코드들을 고쳐야 할 것 같아서요.

그렇지만 클린아키텍처 원칙 상 presentation 계층에 application 계층이 의존할 수는 없으니까, 현재의 dto 패키지 자체를 application 계층으로 옮기는 편이 어떨까요?
그렇다면 UseCase의 Input/Output을 application 내부에서 처리하기 때문에 클린 아키텍처 원칙에도 위배되지 않고, DIP 원칙을 지키면서 코드 수정을 최소화하는 방향으로 리팩토링할 수 있을 것 같아요.

현재 방법으로는 너무 큰 대공사(...)가 될 것 같다는 우려가 있습니다.
지수님 의견은 어떠신지 여쭙고 싶습니다 😊

저도 리팩터링하면서 정말 어려움을 느꼈고, 수정님의 의견에 공감합니다. 😊
다만, 클린 아키텍처에서 DTO를 어떻게 처리하는 게 가장 적합할지 고민한 끝에 제 결론을 공유드리면 다음과 같습니다.

1. DTO가 RequestBody/ResponseBody를 의미하는 경우
-> presentation 계층에 위치시키는 것이 적합합니다.
특히, UseCase는 외부 프레임워크나 라이브러리에 의존해서는 안 되기 때문에, 이 PR에서 제안된 방식(presentation 계층에 DTO 배치)이 클린 아키텍처 원칙에 부합합니다.
클린 아키텍처를 따르기로 한 이상, 이 원칙은 함부로 바꿀 수 없는 핵심 규칙이라고 생각합니다.

또한, 클린 아키텍처에서 가장 중요한 부분은 UseCase라고 생각합니다. 만약 UseCase가 외부 의존성을 포함하고 있다면, 이를 본 다른 사람은 이 구조를 클린 아키텍처라고 인정하지 않을 겁니다.
오히려 잘못 설계된 구조라고 생각할 가능성이 크겠죠. 🤔

2. application 계층에서 DTO를 정의해 사용하는 방법
이 방법은 단순히 presentation에 있는 dto 패키지를 옮기는 것이 아니라, application과 presentation 간의 연결을 위해 추가적인 변환 클래스를 생성하는 방법입니다. 이 방법을 따른다면 진짜 큰 대공사가 되겠네요.. 저희가 원하는 방향도 아니고, 현재 상황에서는 오히려 비효율적일 거라 생각합니다.

저도 이 방식으로 리팩터링하게 된다면 대공사가 될 점에 대해 공감합니다. 하지만 긍정적으로 생각하면 아직 로그인과 회원가입만 구현한 초기 상태에서 발견해서 다행이라 생각해요! 🍀
지금은 힘들 수는 있지만 직접 리팩터링하는 과정에서 아키텍처를 바라보는 관점이 넓어질 거라 생각해요. 수정님도 회원가입 부분 리팩터링하다가 너무 아니다 싶으면 저한테 도움 요청해도 되니까 부담을 안 가지셨으면 좋겠어요.

Ji-soo708

리팩터링 PR이었는데 어쩌다보니 버그를 해결하는 PR이 되어 굉장히 뚱뚱해졌네요... 😂
제가 고친 주요 부분은 따로 코멘트로 남겼습니다! GoogleOAuth쪽 로직만 챙겨보시면 될 거 같아요.

로컬에서 회원이 아닌 경우/회원인 경우 두 가지에 대해 테스트했을 때, 잘 동작합니다!

가입되지 않은 경우

가입된 경우

Ji-soo708 · 2025-01-09T11:28:27Z

src/main/kotlin/com/dobby/backend/infrastructure/ContentLengthRequestInterceptor.kt

FeignClient에서 Content-Length 헤더가 기본으로 설정되지 않아서 @Post 요청 시 411 에러가 발생하는 문제를 겪었습니다.

해결 방법은 두 가지가 있습니다
1. 직접 Content-Length를 0으로 설정하거나 길이를 지정
2. FeignClient의 httpclient나 okhttp를 추가하여 사용

두 번째 방법이 더 깔끔하지만, 저희 프로젝트에서는 적용이 안되는 이슈가 있어 FeignClient의 인터셉터를 통해 RequestTemplate을 수정하여 body와 Content-Length 헤더를 직접 설정하는 방식으로 문제를 해결했습니다.

저도 지금까지 직접 OAuth 구현할 때는 FeignClient를 많이 사용했는데 이런 문제는 처음 마주치네요... 정확히 어떨 때 이런 문제가 생기고 안생기고는 더 찾아봐야 할 거 같습니다. 😂

Ji-soo708 · 2025-01-09T11:29:17Z

src/main/kotlin/com/dobby/backend/infrastructure/config/properties/GoogleAuthProperties.kt

@@ -4,8 +4,9 @@ import org.springframework.boot.context.properties.ConfigurationProperties
 import org.springframework.stereotype.Component

 @Component
-@ConfigurationProperties(prefix = "spring.security.oauth.client.registration.google")
+@ConfigurationProperties(prefix = "spring.security.oauth2.client.registration.google")


여기서 환경변수 위치 경로가 잘못 설정되어 있어 환경변수를 아예 못불러오고 있었습니다... 😂

저도 개인적으로 원인 찾다가 이 부분을 찾아내서,, 환경 변수 로드 문제를 해결한 기억이 나네요! 💦
Typo가 있었던 것을 계속 놓치고 있었던 것 같아요.

Ji-soo708 · 2025-01-09T11:31:13Z

src/main/kotlin/com/dobby/backend/infrastructure/feign/google/GoogleAuthFeignClient.kt

+    @PostMapping
+    fun getAccessToken(
+        @RequestParam(name = "code") code: String,
+        @RequestParam(name = "client_id") clientId: String,
+        @RequestParam(name = "client_secret") clientSecret: String,
+        @RequestParam(name = "redirect_uri") redirectUri: String,
+        @RequestParam(name = "grant_type") grantType: String = "authorization_code",


이 부분은 트러블슈팅하면서 API 명세서와 비교해가며 문제를 찾고자 바로 RequestParam으로 적용했는데, 직관적이라서 이렇게 두어도 괜찮을 것 같아서 유지했습니다.

기존의 @RequestBody가 아무래도 통일성이 있어 사례가 많더라도 그렇게 구현했는데, 외부 API는 역시 여러 사례들을 보아가면서 구현해야 빠르게 트러블 슈팅할 수 있는 것 같아요 🤔 감사합니다!

Ji-soo708 · 2025-01-09T11:33:19Z

...ain/kotlin/com/dobby/backend/presentation/api/dto/response/auth/google/GoogleInfoResponse.kt

-
-    @JsonProperty("name")
-    val name : String


현재 로직 상에서 oauth 계정의 name을 가져오는 부분이 없어 항상 null만 들어가 제거했습니다. 저희 서비스에서는 어차피 email만 필요해서 상관없을 거 같습니다!

Ji-soo708 · 2025-01-09T11:44:19Z

src/main/kotlin/com/dobby/backend/infrastructure/feign/google/GoogleUserInfoFeginClient.kt

 interface GoogleUserInfoFeginClient {
-    @GetMapping("/oauth2/v3/userinfo?access_token={OAUTH_TOKEN}")
-    fun getUserInfo(@PathVariable("OAUTH_TOKEN")token: String): GoogleInfoResponse
+    @GetMapping("/oauth2/v3/userinfo")
+    fun getUserInfo(
+        @RequestHeader("Authorization") token: String
+    ): GoogleInfoResponse


PathVariable이 아닌 헤더로 넘겨줘야 동작합니다. 이 부분에 대한 공식문서를 찾고 싶었는데 도저히 나오지가 않네요...

여러 예제들을 참고하며 /oauth2/v3/userinfo 엔드포인트에 대한 처리를 RequestHeader 방식으로 시도했더니, 문제가 해결되었습니다.

오 헤더에서 주입되지 않은 문제였군요! 원래 FeignClient 전에 WebClient로 OAuth를 구현했었을 때는 RequestHeader을 썼었는데, 제가 참고했었던 FeignClient 관련 레퍼런스에서는 PathVariable로 넘겨줘서 그렇게 구현했었던 기억이 납니다! 💦

관련해서는 그 원인이 뭔지 좀 더 생각해보아야겠네요... 😊
감사합니다!

sonarqubecloud · 2025-01-09T12:07:47Z

Quality Gate failed

Failed conditions
71.9% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

chock-cho

리팩터링 작업도 만만치 않으셨을 텐데,
GoogleOAuth 에서 발생하는 Bug 트러블슈팅까지 정말 수고 많으셨습니다! 😊

Ji-soo708 added 10 commits January 8, 2025 13:17

refact: move naver response dto to naver package

6d5afb7

feat: add NaverAuthGateway for DIP

66773f2

refact: rename OAuthService to AuthService

5d1ee01

refact: add generate token's logic to AuthService

c8a1ecc

refact: move naver response to naver package

decb6db

refact: refactor naver login logic for clean architecture

4cd937d

refact: refactor AuthController/Service for clean architecture

599c515

refact: add findByOauthEmailAndStatus repository's method to MemberGa…

48662c2

…teway

refact: rename memberId to id in Member

0b1dcc2

refact: change email's api response for unification

d25de54

Ji-soo708 added ♻️ REFACTORING 리팩토링 🔒 HOLD 홀드 labels Jan 8, 2025

github-actions bot changed the title ~~refact: AuthController 및 AuthService에 DIP 적용 및 NaverOAuth 로직 리팩토링~~ [YS-114] refact: AuthController 및 AuthService에 DIP 적용 및 NaverOAuth 로직 리팩토링 Jan 8, 2025

Ji-soo708 commented Jan 8, 2025

View reviewed changes

Ji-soo708 requested a review from chock-cho January 8, 2025 05:58

Ji-soo708 self-assigned this Jan 8, 2025

refact: change not null to nullable in OauthLoginResponse

dd37031

chock-cho reviewed Jan 9, 2025

View reviewed changes

Ji-soo708 added 12 commits January 9, 2025 10:11

feat: merge dev

64d1d7a

feat: add UseCase to UseCase class

2ad8edd

refact: delete unused file

f607eb1

feat: add GoogleAuthGateway for clean-architecture

b5e663c

refact: move NaverAuthGateway to feign package

1249720

test: add FetchGoogleUserInfoUseCase test

42579a9

refact: refactor FetchGoogleUserInfoUseCase logic

c404b40

refact: refactor signInWithGoogle Controller due to changed logic

097aff5

style: rename variables for clarity

01722ed

refact: refactor FetchNaverUserInfoUseCase logic

63c5c48

feat: add EnableConfigurationProperties for OAuth

8a27666

fix: fix GoogleAuthFeignClient logic

a2aa734

Ji-soo708 added 12 commits January 9, 2025 18:33

feat: add ContentLengthRequestInterceptor due to feign-client's error

c68f932

refact: refactor google auth and naver auth logic

db853a7

refact: delete unused annotation

0c8e2bc

refact: change return value

0b5ee78

style: add blank

d7ad5b5

refactor: move class file or code

4b2a36e

refactor: delete unused file

2a1eea0

feat: add Service annotation

9740ee5

test: delete unused name field in response

20e12ee

refact: delete unused annotation

3db0edb

refact: delete unused import

d162589

refact: move feign interceptor to feign package

1f7bf30

Ji-soo708 commented Jan 9, 2025

View reviewed changes

Ji-soo708 requested a review from chock-cho January 9, 2025 11:54

Ji-soo708 added 🐛 BUG 버그 and removed 🔒 HOLD 홀드 labels Jan 9, 2025

Ji-soo708 changed the title ~~[YS-114] refact: AuthController 및 AuthService에 DIP 적용 및 NaverOAuth 로직 리팩토링~~ [YS-114] fix: OAuth 로직 버그 수정 및 Auth에 대한 리팩터링 적용 Jan 9, 2025

refact: delete unused code

0e2623e

chock-cho approved these changes Jan 9, 2025

View reviewed changes

chock-cho merged commit acf71b2 into dev Jan 9, 2025
2 of 3 checks passed

chock-cho deleted the feat/YS-114 branch January 9, 2025 12:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[YS-114] fix: OAuth 로직 버그 수정 및 Auth에 대한 리팩터링 적용 #22

[YS-114] fix: OAuth 로직 버그 수정 및 Auth에 대한 리팩터링 적용 #22

Ji-soo708 commented Jan 8, 2025 •

edited

Loading

Ji-soo708 left a comment

Ji-soo708 Jan 8, 2025

Ji-soo708 Jan 8, 2025

chock-cho Jan 8, 2025

Ji-soo708 Jan 8, 2025

chock-cho Jan 8, 2025

Ji-soo708 Jan 8, 2025 •

edited

Loading

chock-cho Jan 8, 2025

Ji-soo708 Jan 9, 2025

Ji-soo708 left a comment •

edited

Loading

Ji-soo708 Jan 9, 2025 •

edited

Loading

Ji-soo708 Jan 9, 2025 •

edited

Loading

chock-cho Jan 9, 2025 •

edited

Loading

Ji-soo708 Jan 9, 2025

chock-cho Jan 9, 2025

Ji-soo708 Jan 9, 2025

Ji-soo708 Jan 9, 2025

chock-cho Jan 9, 2025

sonarqubecloud bot commented Jan 9, 2025

chock-cho left a comment

[YS-114] fix: OAuth 로직 버그 수정 및 Auth에 대한 리팩터링 적용 #22

[YS-114] fix: OAuth 로직 버그 수정 및 Auth에 대한 리팩터링 적용 #22

Conversation

Ji-soo708 commented Jan 8, 2025 • edited Loading

💡 작업 내용

✅ 셀프 체크리스트

🙋🏻‍ 확인해주세요

🔗 Jira 티켓

Ji-soo708 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

Ji-soo708 Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ji-soo708 left a comment • edited Loading

Choose a reason for hiding this comment

Ji-soo708 Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

Ji-soo708 Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

chock-cho Jan 9, 2025 • 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

sonarqubecloud bot commented Jan 9, 2025

Quality Gate failed

chock-cho left a comment

Choose a reason for hiding this comment

Ji-soo708 commented Jan 8, 2025 •

edited

Loading

Ji-soo708 Jan 8, 2025 •

edited

Loading

Ji-soo708 left a comment •

edited

Loading

Ji-soo708 Jan 9, 2025 •

edited

Loading

Ji-soo708 Jan 9, 2025 •

edited

Loading

chock-cho Jan 9, 2025 •

edited

Loading