[FIX] 로그인 시 카카오 앱으로 리다이렉트 안되는 현상 수정 #53
Conversation
- `LoginScreen`에서 카카오 SDK를 직접 호출하여 로그인하도록 로직을 구현했습니다. - 카카오톡 앱 설치 여부에 따라 카카오톡 또는 카카오 계정으로 로그인을 시도하며, 실패 시 대체 로그인을 수행합니다. - 로그인 성공 시 `ViewModel`의 `onSocialLoginSuccess`를, 실패 시 `onSocialLoginFailure`를 호출하도록 변경했습니다. - `AuthRepository`에서 소셜 로그인 처리를 담당하던 `SocialLoginProcessor`를 제거하고, 외부에서 전달받은 액세스 토큰으로 서버 로그인을 요청하는 단일 메서드로 통합했습니다.
- 소셜 로그인 처리를 위해 사용했던 Strategy 패턴 관련 코드들을 삭제했습니다. - `SocialLoginDataSource` 인터페이스, `KakaoDataSource` 구현체, `SocialLoginProcessor`, 그리고 관련 `DataSourceModule`이 삭제되었습니다.
- 사용자가 카카오톡 앱으로 로그인 시도 중 취소하는 경우를 포함하여, 로그인 실패 시 카카오 계정으로 재시도하던 로직을 제거했습니다.
- 카카오 로그인(`loginWithKakao`) 함수 실행 시, 새로운 로그인을 보장하기 위해 기존 로그인 정보를 먼저 로그아웃하는 `logoutKakao()` 호출을 추가했습니다.
- 기존에 소셜 로그인 API를 호출하고 성공 시 개인정보처리방침 동의 바텀시트를 표시하던 방식에서, 바텀시트를 먼저 표시하도록 순서를 변경했습니다. - 사용자가 개인정보처리방침에 동의했을 때(`onPrivacyConsentComplete` 호출 시) 소셜 로그인 API를 호출하도록 로직을 수정했습니다.
- 기존에 카카오 로그인 재시도 시 `logout()`을 호출하던 로직을 `unlink()` (연결 끊기)를 호출하도록 변경했습니다. - 이에 따라 함수 이름도 `logoutKakao()`에서 `unlinkKakao()`로 수정했습니다.
- `LoginScreen`에서 `ProviderType.ETC` 분기에 있던 `TODO()` 호출 코드를 제거했습니다.
Summary of ChangesHello @stopstone, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 카카오 로그인 프로세스를 재구성하여 사용자 경험과 아키텍처 분리를 개선합니다. 핵심 변경 사항은 카카오 SDK 통합을 데이터 계층에서 UI 계층(LoginScreen)으로 옮겨 앱 간 로그인 처리를 직접 제어하고 로그인 흐름을 더 잘 관리할 수 있도록 한 것입니다. ViewModel은 이제 성공적인 UI 수준 인증 후 액세스 토큰을 받아 비즈니스 로직에만 집중합니다. 또한, 로그인 흐름은 토큰의 조기 제출을 방지하고 취소를 보다 원활하게 처리하도록 개선되어, 더 부드럽고 안전한 사용자 여정을 보장합니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where the Kakao login was not redirecting to the Kakao app correctly. The changes include moving the Kakao login logic to the UI layer, improving the login flow, and adding a feature to switch to the Kakao app based on whether it's installed. The data layer's dependency on the Kakao SDK has been removed. I have identified some areas for improvement, particularly in error handling and code clarity.
| error != null -> { | ||
| continuation.resume("") | ||
| } |
There was a problem hiding this comment.
In loginWithKakaoTalk, if error != null, you're resuming with an empty string. It would be better to propagate the error to the onFailure callback to handle the error case appropriately. This ensures that failures are properly communicated and handled.
when {
error != null -> {
continuation.resumeWith(Result.failure(error))
}
token != null -> continuation.resume(token.accessToken)
else -> continuation.resume("")
}| override suspend fun performSocialLogin( | ||
| accessToken: String, | ||
| providerType: ProviderType, | ||
| ): Result<Unit> = |
There was a problem hiding this comment.
The performSocialLogin function now directly receives the access token. Consider adding error handling to this function to manage potential issues with the token itself, such as an invalid or expired token. This will make the function more robust.
| when (providerType) { | ||
| ProviderType.KAKAO -> { | ||
| scope.launch { | ||
| unlinkKakao() | ||
| performKakaoLogin( | ||
| context = context, | ||
| onSuccess = { accessToken -> | ||
| viewModel.onSocialLoginSuccess(accessToken, ProviderType.KAKAO) | ||
| }, | ||
| onFailure = { exception -> | ||
| viewModel.onSocialLoginFailure(exception) | ||
| }, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| ProviderType.ETC -> { } | ||
| } |
There was a problem hiding this comment.
The when statement only handles ProviderType.KAKAO. Add a default branch or handle other provider types explicitly to avoid unexpected behavior if more providers are added in the future. This will make the code more maintainable and prevent potential runtime errors.
when (providerType) {
ProviderType.KAKAO -> {
scope.launch {
unlinkKakao()
performKakaoLogin(
context = context,
onSuccess = { accessToken ->
viewModel.onSocialLoginSuccess(accessToken, ProviderType.KAKAO)
},
onFailure = { exception ->
viewModel.onSocialLoginFailure(exception)
},
)
}
}
ProviderType.ETC -> { }
else -> {
// TODO: Handle other provider types or provide a default action
}
}| try { | ||
| val accessToken = | ||
| if (UserApiClient.instance.isKakaoTalkLoginAvailable(context)) { | ||
| loginWithKakaoTalk(context) | ||
| } else { | ||
| loginWithKakaoAccount(context) | ||
| } | ||
|
|
||
| if (accessToken.isNotEmpty()) { | ||
| onSuccess(accessToken) | ||
| } else { | ||
| onFailure(Exception("사용자가 로그인을 취소했습니다")) | ||
| } | ||
| } catch (exception: Exception) { | ||
| onFailure(exception) | ||
| } |
There was a problem hiding this comment.
The performKakaoLogin function catches Exception but doesn't handle specific exceptions. Consider catching ClientError or KakaoSdkError to handle specific Kakao SDK errors and provide more informative error messages to the user. This will improve the user experience by providing more context about the error.
| if (token == null || providerType == null) { | ||
| _event.send(LoginEvent.ShowError(Exception("로그인 정보가 없습니다"))) | ||
| return@launch |
There was a problem hiding this comment.
In onPrivacyConsentComplete, if token == null || providerType == null, you're sending a generic error. Consider providing more context about why the login information is missing. This will aid in debugging and provide a better user experience.
| if (token == null || providerType == null) { | |
| _event.send(LoginEvent.ShowError(Exception("로그인 정보가 없습니다"))) | |
| return@launch | |
| if (token == null || providerType == null) { | |
| val errorMessage = "Token: ${token == null}, ProviderType: ${providerType == null}" | |
| _event.send(LoginEvent.ShowError(Exception("로그인 정보가 없습니다 - $errorMessage"))) | |
| return@launch | |
| } |
- 기존에 로그인 버튼 클릭 시 즉시 실행되던 `unlinkKakao()` 호출을, 실제 카카오 로그인을 수행하기 직전으로 이동시켰습니다.
- 사용자가 카카오 로그인을 취소했을 때, 빈 accessToken을 반환하는 대신 예외를 발생시키도록 로직을 수정했습니다. - 로그인 취소 시 반환할 예외 메시지를 `strings.xml` 리소스(`login_user_cancelled`)로 추가하고 이를 사용하도록 변경했습니다. - `loginWithKakaoTalk`과 `loginWithKakaoAccount`에서 `handleKakaoLoginResult`를 호출할 때, 로그인 취소 메시지를 전달하도록 수정했습니다.
- `AuthRepository`에서 소셜 로그인을 요청하기 전에 액세스 토큰이 비어있지 않고 최소 길이를 만족하는지 확인하는 `validateAccessToken` 로직을 추가했습니다.
- `ProviderType` enum에서 사용하지 않는 `ETC`를 제거했습니다. - `LoginScreen`에서 `ETC` 타입 관련 분기 처리 로직을 제거했습니다.
- 카카오 로그인 실패 시 발생하는 에러를 `AuthError`, `ApiError`, `ClientError`, `KakaoSdkError` 등 구체적인 케이스로 나누어 처리하도록 구현했습니다. - 각 에러 상황에 맞는 사용자 안내 메시지를 `strings.xml`에 추가하고, 로그인 실패 시 해당 메시지를 표시하도록 변경했습니다.
rhkrwngud445
left a comment
rhkrwngud445
left a comment
There was a problem hiding this comment.
큰 수정을 이렇게.. 감사합니다!👍
가독성을 개선해볼만한 코멘트로 조금 남겼습니다! 확인 부탁드려요!
고생하셨습니다!😀
| _loginState.value.copy( | ||
| socialLoginToken = accessToken, | ||
| providerType = providerType, | ||
| showPrivacyBottomSheet = true, |
There was a problem hiding this comment.
바텀시트를 보여준다는 것은 view의 의존성이 있어보입니다!
동의를 받아야한다 등 어떤 목적인지에 대한 네이밍으로 고민해보면 어떨까요?
| updateLoadingState(isLoading = false) | ||
| _loginState.value = | ||
| _loginState.value.copy( | ||
| showPrivacyBottomSheet = false, |
There was a problem hiding this comment.
여기도 위 코멘트와 같이 확인 부탁드립니다!
| private suspend fun performKakaoLogin( | ||
| context: Context, | ||
| onSuccess: (String) -> Unit, | ||
| onFailure: (Throwable) -> Unit, | ||
| ) { | ||
| unlinkKakao() | ||
| try { | ||
| val accessToken = | ||
| if (UserApiClient.instance.isKakaoTalkLoginAvailable(context)) { | ||
| loginWithKakaoTalk(context) | ||
| } else { | ||
| loginWithKakaoAccount(context) | ||
| } | ||
| onSuccess(accessToken) | ||
| } catch (error: AuthError) { | ||
| // OAuth 인증 과정 에러 | ||
| onFailure(Exception(context.getString(R.string.login_auth_error))) | ||
| } catch (error: ApiError) { | ||
| // API 호출 에러 | ||
| onFailure(Exception(context.getString(R.string.login_api_error))) | ||
| } catch (error: ClientError) { | ||
| // SDK 내부 에러 | ||
| onFailure(Exception(context.getString(R.string.login_client_error))) | ||
| } catch (error: KakaoSdkError) { | ||
| // 카카오 SDK 에러 | ||
| onFailure(Exception(context.getString(R.string.login_sdk_error))) | ||
| } catch (exception: Exception) { | ||
| onFailure(exception) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * 카카오 로그아웃 | ||
| */ | ||
| private suspend fun unlinkKakao(): Unit = | ||
| suspendCancellableCoroutine { continuation -> | ||
| UserApiClient.instance.unlink { error -> | ||
| continuation.resume(Unit) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * 카카오톡으로 로그인 | ||
| */ | ||
| private suspend fun loginWithKakaoTalk(context: Context): String = | ||
| suspendCancellableCoroutine { continuation -> | ||
| UserApiClient.instance.loginWithKakaoTalk(context) { token, error -> | ||
| handleKakaoLoginResult( | ||
| token, | ||
| error, | ||
| context.getString(R.string.login_user_cancelled), | ||
| continuation, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * 카카오 계정으로 로그인 | ||
| */ | ||
| private suspend fun loginWithKakaoAccount(context: Context): String = | ||
| suspendCancellableCoroutine { continuation -> | ||
| UserApiClient.instance.loginWithKakaoAccount(context) { token, error -> | ||
| handleKakaoLoginResult( | ||
| token, | ||
| error, | ||
| context.getString(R.string.login_user_cancelled), | ||
| continuation, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * 카카오 로그인 결과 처리 | ||
| */ | ||
| private fun handleKakaoLoginResult( | ||
| token: OAuthToken?, | ||
| error: Throwable?, | ||
| cancelledMessage: String, | ||
| continuation: CancellableContinuation<String>, | ||
| ) { | ||
| when { | ||
| error != null -> { | ||
| continuation.resumeWith(Result.failure(Exception(cancelledMessage))) | ||
| } | ||
|
|
||
| token != null -> continuation.resume(token.accessToken) | ||
| else -> | ||
| continuation.resumeWith( | ||
| Result.failure(Exception(cancelledMessage)), | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Screen이 로직을 가져오면서 무거워진 것 같습니다😅
이 로직들은 별도의 클래스로 분리하여 remember로 관리하는 것은 어떨까요?
There was a problem hiding this comment.
감사합니다!!
로그인 로직을 분리한다면.. 패키지 네이밍과 위치는 어떻게 하면 좋을까요?
현재 생각하는 것은 presentation/feature/login/auth/KakaoLogin~.kt 로 하면 좋을 것 같다고 생각합니다!
There was a problem hiding this comment.
저는 멀티모듈 기준 최상단 또는 core내부가 적절해 보이는데요!
현재는 해당 login 패키지내 구성하고 이슈나 todo로 리마인드 될 수 있게 하면 좋을 것 같습니다!
현재 말씀주신 방법으로 구현주시면 될 것 같아요!
There was a problem hiding this comment.
코멘트 감사합니다!
우선은 login 패키지에 두고 이후 core로 이동시키겠습니다!
core는 공통, 재사용 가능한 것들을 넣는다는 생각에 생각 못했는데 감사합니다!
- 각 소셜 로그인 구현을 통합적으로 관리하기 위해 `SocialLoginHandler`를 도입했습니다. - 다양한 소셜 로그인 방식의 공통 인터페이스로 `SocialLoginProvider`를 정의했습니다. - 향후 새로운 소셜 로그인(Apple, Google 등)을 쉽게 추가할 수 있는 확장 가능한 구조를 마련했습니다.
- 카카오 SDK를 사용하여 소셜 로그인을 처리하는 `KakaoLoginManager` 클래스를 추가했습니다. - 카카오톡 앱 설치 여부에 따라 카카오톡 또는 카카오 계정으로 로그인을 분기하여 처리합니다.
- `LoginScreen.kt`에 있던 카카오 로그인 관련 로직을 `SocialLoginHandler` 클래스로 분리했습니다. - `LoginRoute`에서 `SocialLoginHandler`를 생성하고 사용하여 로그인 로직을 호출하도록 수정했습니다.
|
@rhkrwngud445 리뷰 감사합니다 리뷰주신 것을 반영해봤습니다! 실제 카카오 로그인 관련은 KakaoLoginManager에 구현하였고, 모두 feature/login/auth에 위치하였습니다 :) |
|
잘 추상화된 것 같네요! 바로 머지 주셔도 될 것 같습니다👍 |
작업 내용
1. 카카오 로그인 UI 계층으로 이동
LoginScreen에서 직접 카카오 로그인 처리
ViewModel은 비즈니스 로직만 처리
불필요한 코드 제거
2. 로그인 플로우 개선
토큰 전송 시점 변경
취소 처리 개선
3. 카카오톡 앱 전환 기능
확인 방법
카카오톡 미설치 상태:
참고 사항
로그인 전에 한 번 unlink하고 있습니다!
관련 이슈