-
Notifications
You must be signed in to change notification settings - Fork 0
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
[YS-114] fix: OAuth 로직 버그 수정 및 Auth에 대한 리팩터링 적용 #22
Merged
Merged
Changes from 34 commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
6d5afb7
refact: move naver response dto to naver package
Ji-soo708 66773f2
feat: add NaverAuthGateway for DIP
Ji-soo708 5d1ee01
refact: rename OAuthService to AuthService
Ji-soo708 c8a1ecc
refact: add generate token's logic to AuthService
Ji-soo708 decb6db
refact: move naver response to naver package
Ji-soo708 4cd937d
refact: refactor naver login logic for clean architecture
Ji-soo708 599c515
refact: refactor AuthController/Service for clean architecture
Ji-soo708 48662c2
refact: add findByOauthEmailAndStatus repository's method to MemberGa…
Ji-soo708 0b1dcc2
refact: rename memberId to id in Member
Ji-soo708 d25de54
refact: change email's api response for unification
Ji-soo708 dd37031
refact: change not null to nullable in OauthLoginResponse
Ji-soo708 64d1d7a
feat: merge dev
Ji-soo708 2ad8edd
feat: add UseCase to UseCase class
Ji-soo708 f607eb1
refact: delete unused file
Ji-soo708 b5e663c
feat: add GoogleAuthGateway for clean-architecture
Ji-soo708 1249720
refact: move NaverAuthGateway to feign package
Ji-soo708 42579a9
test: add FetchGoogleUserInfoUseCase test
Ji-soo708 c404b40
refact: refactor FetchGoogleUserInfoUseCase logic
Ji-soo708 097aff5
refact: refactor signInWithGoogle Controller due to changed logic
Ji-soo708 01722ed
style: rename variables for clarity
Ji-soo708 63c5c48
refact: refactor FetchNaverUserInfoUseCase logic
Ji-soo708 8a27666
feat: add EnableConfigurationProperties for OAuth
Ji-soo708 a2aa734
fix: fix GoogleAuthFeignClient logic
Ji-soo708 c68f932
feat: add ContentLengthRequestInterceptor due to feign-client's error
Ji-soo708 db853a7
refact: refactor google auth and naver auth logic
Ji-soo708 0c8e2bc
refact: delete unused annotation
Ji-soo708 0b5ee78
refact: change return value
Ji-soo708 d7ad5b5
style: add blank
Ji-soo708 4b2a36e
refactor: move class file or code
Ji-soo708 2a1eea0
refactor: delete unused file
Ji-soo708 9740ee5
feat: add Service annotation
Ji-soo708 20e12ee
test: delete unused name field in response
Ji-soo708 3db0edb
refact: delete unused annotation
Ji-soo708 d162589
refact: delete unused import
Ji-soo708 1f7bf30
refact: move feign interceptor to feign package
Ji-soo708 0e2623e
refact: delete unused code
Ji-soo708 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
32 changes: 0 additions & 32 deletions
32
src/main/kotlin/com/dobby/backend/application/mapper/OauthUserMapper.kt
This file was deleted.
Oops, something went wrong.
75 changes: 75 additions & 0 deletions
75
src/main/kotlin/com/dobby/backend/application/service/AuthService.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
package com.dobby.backend.application.service | ||
|
||
import com.dobby.backend.application.usecase.* | ||
import org.springframework.stereotype.Service | ||
|
||
@Service | ||
class AuthService( | ||
private val fetchGoogleUserInfoUseCase: FetchGoogleUserInfoUseCase, | ||
private val fetchNaverUserInfoUseCase: FetchNaverUserInfoUseCase, | ||
private val generateTokenWithRefreshTokenUseCase: GenerateTokenWithRefreshTokenUseCase, | ||
private val generateTestTokenUseCase: GenerateTestTokenUseCase, | ||
) { | ||
fun getGoogleUserInfo(authorizationCode: String): FetchGoogleUserInfoUseCase.Output { | ||
val result = fetchGoogleUserInfoUseCase.execute( | ||
FetchGoogleUserInfoUseCase.Input( | ||
authorizationCode = authorizationCode | ||
) | ||
) | ||
return FetchGoogleUserInfoUseCase.Output( | ||
isRegistered = result.isRegistered, | ||
accessToken = result.accessToken, | ||
refreshToken = result.refreshToken, | ||
memberId = result.memberId, | ||
name = result.name, | ||
oauthEmail = result.oauthEmail, | ||
role = result.role, | ||
provider = result.provider, | ||
) | ||
} | ||
|
||
fun getNaverUserInfo(authorizationCode: String, state: String): FetchNaverUserInfoUseCase.Output { | ||
val result = fetchNaverUserInfoUseCase.execute( | ||
FetchNaverUserInfoUseCase.Input( | ||
authorizationCode = authorizationCode, | ||
state = state | ||
) | ||
) | ||
return FetchNaverUserInfoUseCase.Output( | ||
isRegistered = result.isRegistered, | ||
accessToken = result.accessToken, | ||
refreshToken = result.refreshToken, | ||
memberId = result.memberId, | ||
name = result.name, | ||
oauthEmail = result.oauthEmail, | ||
role = result.role, | ||
provider = result.provider, | ||
) | ||
} | ||
|
||
fun forceToken(memberId: Long): GenerateTestTokenUseCase.Output { | ||
val result = generateTestTokenUseCase.execute( | ||
GenerateTestTokenUseCase.Input( | ||
memberId = memberId | ||
) | ||
) | ||
return GenerateTestTokenUseCase.Output( | ||
accessToken = result.accessToken, | ||
refreshToken = result.refreshToken, | ||
member = result.member | ||
) | ||
} | ||
|
||
fun signInWithRefreshToken(refreshToken: String): GenerateTokenWithRefreshTokenUseCase.Output { | ||
val result = generateTokenWithRefreshTokenUseCase.execute( | ||
GenerateTokenWithRefreshTokenUseCase.Input( | ||
refreshToken = refreshToken, | ||
) | ||
) | ||
return GenerateTokenWithRefreshTokenUseCase.Output( | ||
accessToken = result.accessToken, | ||
refreshToken = result.refreshToken, | ||
member = result.member | ||
) | ||
} | ||
} |
22 changes: 0 additions & 22 deletions
22
src/main/kotlin/com/dobby/backend/application/service/OauthService.kt
This file was deleted.
Oops, something went wrong.
91 changes: 46 additions & 45 deletions
91
src/main/kotlin/com/dobby/backend/application/usecase/FetchGoogleUserInfoUseCase.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,64 +1,65 @@ | ||
package com.dobby.backend.application.usecase | ||
|
||
import com.dobby.backend.application.mapper.OauthUserMapper | ||
import com.dobby.backend.domain.exception.SignInMemberException | ||
import com.dobby.backend.infrastructure.config.properties.GoogleAuthProperties | ||
import com.dobby.backend.domain.gateway.MemberGateway | ||
import com.dobby.backend.domain.gateway.TokenGateway | ||
import com.dobby.backend.domain.gateway.feign.GoogleAuthGateway | ||
import com.dobby.backend.infrastructure.database.entity.enum.MemberStatus | ||
import com.dobby.backend.infrastructure.database.entity.enum.ProviderType | ||
import com.dobby.backend.infrastructure.database.repository.MemberRepository | ||
import com.dobby.backend.infrastructure.feign.google.GoogleAuthFeignClient | ||
import com.dobby.backend.infrastructure.feign.google.GoogleUserInfoFeginClient | ||
import com.dobby.backend.infrastructure.token.JwtTokenProvider | ||
import com.dobby.backend.presentation.api.dto.request.auth.google.GoogleTokenRequest | ||
import com.dobby.backend.presentation.api.dto.request.auth.google.GoogleOauthLoginRequest | ||
import com.dobby.backend.presentation.api.dto.response.auth.google.GoogleTokenResponse | ||
import com.dobby.backend.presentation.api.dto.response.auth.OauthLoginResponse | ||
import com.dobby.backend.util.AuthenticationUtils | ||
import com.dobby.backend.infrastructure.database.entity.enum.RoleType | ||
|
||
class FetchGoogleUserInfoUseCase( | ||
private val googleAuthFeignClient: GoogleAuthFeignClient, | ||
private val googleUserInfoFeginClient: GoogleUserInfoFeginClient, | ||
private val jwtTokenProvider: JwtTokenProvider, | ||
private val googleAuthProperties: GoogleAuthProperties, | ||
private val memberRepository: MemberRepository | ||
) : UseCase<GoogleOauthLoginRequest, OauthLoginResponse> { | ||
private val googleAuthGateway: GoogleAuthGateway, | ||
private val memberGateway: MemberGateway, | ||
private val jwtTokenGateway: TokenGateway | ||
) : UseCase<FetchGoogleUserInfoUseCase.Input, FetchGoogleUserInfoUseCase.Output> { | ||
|
||
override fun execute(input: GoogleOauthLoginRequest): OauthLoginResponse { | ||
try { | ||
val googleTokenRequest = GoogleTokenRequest( | ||
code = input.authorizationCode, | ||
clientId = googleAuthProperties.clientId, | ||
clientSecret = googleAuthProperties.clientSecret, | ||
redirectUri = googleAuthProperties.redirectUri | ||
) | ||
data class Input( | ||
val authorizationCode: String | ||
) | ||
|
||
val oauthRes = fetchAccessToken(googleTokenRequest) | ||
val oauthToken = oauthRes.accessToken | ||
data class Output( | ||
val isRegistered: Boolean, | ||
val accessToken: String?, | ||
val refreshToken: String?, | ||
val memberId: Long?, | ||
val name: String?, | ||
val oauthEmail: String, | ||
val role: RoleType?, | ||
val provider: ProviderType | ||
) | ||
|
||
val userInfo = googleUserInfoFeginClient.getUserInfo("Bearer $oauthToken") | ||
val email = userInfo.email | ||
val regMember = memberRepository.findByOauthEmailAndStatus(email, MemberStatus.ACTIVE) | ||
?: throw SignInMemberException() | ||
override fun execute(input: Input): Output { | ||
val oauthToken = googleAuthGateway.getAccessToken(input.authorizationCode).accessToken | ||
val userInfo = googleAuthGateway.getUserInfo(oauthToken) | ||
val email = userInfo.email | ||
val member = email.let { memberGateway.findByOauthEmailAndStatus(it, MemberStatus.ACTIVE) } | ||
|
||
val regMemberAuthentication = AuthenticationUtils.createAuthentication(regMember) | ||
val jwtAccessToken = jwtTokenProvider.generateAccessToken(regMemberAuthentication) | ||
val jwtRefreshToken = jwtTokenProvider.generateRefreshToken(regMemberAuthentication) | ||
return if (member != null) { | ||
val jwtAccessToken = jwtTokenGateway.generateAccessToken(member) | ||
val jwtRefreshToken = jwtTokenGateway.generateRefreshToken(member) | ||
|
||
return OauthUserMapper.toDto( | ||
Output( | ||
isRegistered = true, | ||
accessToken = jwtAccessToken, | ||
refreshToken = jwtRefreshToken, | ||
oauthEmail = regMember.oauthEmail, | ||
oauthName = regMember.name ?: throw SignInMemberException(), | ||
role = regMember.role ?: throw SignInMemberException(), | ||
memberId = member.id, | ||
name = member.name, | ||
oauthEmail = member.oauthEmail, | ||
role = member.role, | ||
provider = ProviderType.GOOGLE | ||
) | ||
} else { | ||
// 등록된 멤버가 없으면 isRegistered = false, memberId = null | ||
Output( | ||
isRegistered = false, | ||
accessToken = null, | ||
refreshToken = null, | ||
memberId = null, | ||
name = null, | ||
oauthEmail = email, | ||
role = null, | ||
provider = ProviderType.GOOGLE | ||
) | ||
} catch (e: SignInMemberException) { | ||
throw SignInMemberException() | ||
} | ||
} | ||
|
||
private fun fetchAccessToken(googleTokenRequest: GoogleTokenRequest): GoogleTokenResponse { | ||
return googleAuthFeignClient.getAccessToken(googleTokenRequest) | ||
} | ||
} |
94 changes: 48 additions & 46 deletions
94
src/main/kotlin/com/dobby/backend/application/usecase/FetchNaverUserInfoUseCase.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,65 +1,67 @@ | ||
package com.dobby.backend.application.usecase | ||
|
||
import com.dobby.backend.application.mapper.OauthUserMapper | ||
import com.dobby.backend.domain.exception.SignInMemberException | ||
import com.dobby.backend.infrastructure.config.properties.NaverAuthProperties | ||
import com.dobby.backend.domain.gateway.MemberGateway | ||
import com.dobby.backend.domain.gateway.feign.NaverAuthGateway | ||
import com.dobby.backend.domain.gateway.TokenGateway | ||
import com.dobby.backend.infrastructure.database.entity.enum.MemberStatus | ||
import com.dobby.backend.infrastructure.database.entity.enum.ProviderType | ||
import com.dobby.backend.infrastructure.database.repository.MemberRepository | ||
import com.dobby.backend.infrastructure.feign.naver.NaverAuthFeignClient | ||
import com.dobby.backend.infrastructure.feign.naver.NaverUserInfoFeignClient | ||
import com.dobby.backend.infrastructure.token.JwtTokenProvider | ||
import com.dobby.backend.presentation.api.dto.request.auth.NaverOauthLoginRequest | ||
import com.dobby.backend.presentation.api.dto.request.auth.NaverTokenRequest | ||
import com.dobby.backend.presentation.api.dto.response.auth.NaverTokenResponse | ||
import com.dobby.backend.presentation.api.dto.response.auth.OauthLoginResponse | ||
import com.dobby.backend.util.AuthenticationUtils | ||
import com.dobby.backend.infrastructure.database.entity.enum.RoleType | ||
|
||
class FetchNaverUserInfoUseCase( | ||
private val naverAuthFeignClient: NaverAuthFeignClient, | ||
private val naverUserInfoFeginClient: NaverUserInfoFeignClient, | ||
private val jwtTokenProvider: JwtTokenProvider, | ||
private val naverAuthProperties: NaverAuthProperties, | ||
private val memberRepository: MemberRepository | ||
) : UseCase<NaverOauthLoginRequest, OauthLoginResponse> { | ||
private val naverAuthGateway: NaverAuthGateway, | ||
private val memberGateway: MemberGateway, | ||
private val jwtTokenGateway: TokenGateway | ||
) : UseCase<FetchNaverUserInfoUseCase.Input, FetchNaverUserInfoUseCase.Output> { | ||
|
||
override fun execute(input: NaverOauthLoginRequest): OauthLoginResponse { | ||
try { | ||
val naverTokenRequest = NaverTokenRequest( | ||
grantType = "authorization_code", | ||
clientId = naverAuthProperties.clientId, | ||
clientSecret = naverAuthProperties.clientSecret, | ||
code = input.authorizationCode, | ||
state = input.state | ||
) | ||
data class Input( | ||
val authorizationCode: String, | ||
val state: String | ||
) | ||
|
||
val oauthRes = fetchAccessToken(naverTokenRequest) | ||
val oauthToken = oauthRes.accessToken | ||
// TODO: 테스트 후, oauthEmail not null 처리 | ||
data class Output( | ||
val isRegistered: Boolean, | ||
val accessToken: String?, | ||
val refreshToken: String?, | ||
val memberId: Long?, | ||
val name: String?, | ||
val oauthEmail: String?, | ||
val role: RoleType?, | ||
val provider: ProviderType | ||
) | ||
|
||
val userInfo = naverUserInfoFeginClient.getUserInfo("Bearer $oauthToken") | ||
val email = userInfo.email | ||
val regMember = memberRepository.findByOauthEmailAndStatus(email, MemberStatus.ACTIVE) | ||
?: throw SignInMemberException() | ||
override fun execute(input: Input): Output { | ||
val oauthToken = naverAuthGateway.getAccessToken(input.authorizationCode, input.state).accessToken | ||
val userInfo = oauthToken?.let { naverAuthGateway.getUserInfo(it) } | ||
val email = userInfo?.email | ||
val member = email?.let { memberGateway.findByOauthEmailAndStatus(it, MemberStatus.ACTIVE) } | ||
|
||
val regMemberAuthentication = AuthenticationUtils.createAuthentication(regMember) | ||
val jwtAccessToken = jwtTokenProvider.generateAccessToken(regMemberAuthentication) | ||
val jwtRefreshToken = jwtTokenProvider.generateRefreshToken(regMemberAuthentication) | ||
return if (member != null) { | ||
val jwtAccessToken = jwtTokenGateway.generateAccessToken(member) | ||
val jwtRefreshToken = jwtTokenGateway.generateRefreshToken(member) | ||
|
||
return OauthUserMapper.toDto( | ||
Output( | ||
isRegistered = true, | ||
accessToken = jwtAccessToken, | ||
refreshToken = jwtRefreshToken, | ||
oauthEmail = regMember.oauthEmail, | ||
oauthName = regMember.name ?: throw SignInMemberException(), | ||
role = regMember.role ?: throw SignInMemberException(), | ||
memberId = member.id, | ||
name = member.name, | ||
oauthEmail = member.oauthEmail, | ||
role = member.role, | ||
provider = ProviderType.NAVER | ||
) | ||
} else { | ||
// 등록된 멤버가 없으면 isRegistered = false, memberId = null | ||
Output( | ||
isRegistered = false, | ||
accessToken = null, | ||
refreshToken = null, | ||
memberId = null, | ||
name = null, | ||
oauthEmail = email, | ||
role = null, | ||
provider = ProviderType.NAVER | ||
) | ||
} catch (e: SignInMemberException) { | ||
throw SignInMemberException() | ||
} | ||
} | ||
|
||
private fun fetchAccessToken(naverTokenRequest: NaverTokenRequest): NaverTokenResponse { | ||
return naverAuthFeignClient.getAccessToken(naverTokenRequest) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
개인적인 의문인데요, 현재
FetchNaverUseInfoUseCase
에서 입력/반환 필드값이 많아짐에 따라서 해당 UseCase 안에 모든 Input, Output들을 데이터 클래스로 정의하면 UseCase 자체의 코드 길이가 너무 길어지고, 가독성이 떨어질 것 같다는 생각을 했어요.그리고, 코드의 재사용성을 위해서 회원가입 로직을 구현한 저 또한 실험자/피험자에 대한 회원가입 응답값을 MemberResponse로 통일하며 구현하였기 때문에, 현재 방법으로는 많은 코드들을 고쳐야 할 것 같아서요.
그렇지만 클린아키텍처 원칙 상 presentation 계층에 application 계층이 의존할 수는 없으니까, 현재의 dto 패키지 자체를 application 계층으로 옮기는 편이 어떨까요?
그렇다면 UseCase의 Input/Output을 application 내부에서 처리하기 때문에 클린 아키텍처 원칙에도 위배되지 않고, DIP 원칙을 지키면서 코드 수정을 최소화하는 방향으로 리팩토링할 수 있을 것 같아요.
현재 방법으로는 너무 큰 대공사(...)가 될 것 같다는 우려가 있습니다.
지수님 의견은 어떠신지 여쭙고 싶습니다 😊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 리팩터링하면서 정말 어려움을 느꼈고, 수정님의 의견에 공감합니다. 😊
다만, 클린 아키텍처에서 DTO를 어떻게 처리하는 게 가장 적합할지 고민한 끝에 제 결론을 공유드리면 다음과 같습니다.
1. DTO가 RequestBody/ResponseBody를 의미하는 경우
-> presentation 계층에 위치시키는 것이 적합합니다.
특히, UseCase는 외부 프레임워크나 라이브러리에 의존해서는 안 되기 때문에, 이 PR에서 제안된 방식(presentation 계층에 DTO 배치)이 클린 아키텍처 원칙에 부합합니다.
클린 아키텍처를 따르기로 한 이상, 이 원칙은 함부로 바꿀 수 없는 핵심 규칙이라고 생각합니다.
또한, 클린 아키텍처에서 가장 중요한 부분은 UseCase라고 생각합니다. 만약 UseCase가 외부 의존성을 포함하고 있다면, 이를 본 다른 사람은 이 구조를 클린 아키텍처라고 인정하지 않을 겁니다.
오히려 잘못 설계된 구조라고 생각할 가능성이 크겠죠. 🤔
2. application 계층에서 DTO를 정의해 사용하는 방법
이 방법은 단순히 presentation에 있는 dto 패키지를 옮기는 것이 아니라, application과 presentation 간의 연결을 위해 추가적인 변환 클래스를 생성하는 방법입니다. 이 방법을 따른다면 진짜 큰 대공사가 되겠네요.. 저희가 원하는 방향도 아니고, 현재 상황에서는 오히려 비효율적일 거라 생각합니다.
저도 이 방식으로 리팩터링하게 된다면 대공사가 될 점에 대해 공감합니다. 하지만 긍정적으로 생각하면 아직 로그인과 회원가입만 구현한 초기 상태에서 발견해서 다행이라 생각해요! 🍀
지금은 힘들 수는 있지만 직접 리팩터링하는 과정에서 아키텍처를 바라보는 관점이 넓어질 거라 생각해요. 수정님도 회원가입 부분 리팩터링하다가 너무 아니다 싶으면 저한테 도움 요청해도 되니까 부담을 안 가지셨으면 좋겠어요.