-
Notifications
You must be signed in to change notification settings - Fork 0
로그인 테스트 완료 #1
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
base: master
Are you sure you want to change the base?
로그인 테스트 완료 #1
Conversation
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.
Pull request overview
This PR implements social login functionality for Google, Kakao, and Naver authentication providers. The implementation follows a clean architecture pattern with distinct layers (UI, Presentation, Domain, Data, Remote, Local) and includes comprehensive OAuth integration for all three providers.
Key Changes:
- Implemented social authentication flow for Google, Kakao, and Naver with complete OAuth integration
- Added login/logout UI with test screen and DataStore-based credential persistence
- Configured dependency injection modules across all architectural layers with proper scoping
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 24 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/main/java/project/side/ui/screen/LoginScreen.kt | Composable login screen with buttons for all three social providers |
| ui/src/main/java/project/side/ui/MainActivity.kt | Activity setup with login screen integration and use case injection |
| ui/src/main/res/values/themes.xml | App theme definition with splash screen configuration |
| ui/src/main/res/values/strings.xml | App name string resource |
| ui/src/main/res/drawable/ic_splash_empty.xml | Transparent splash screen icon |
| ui/src/main/AndroidManifest.xml | MainActivity declaration with launcher intent |
| ui/build.gradle.kts | Dependencies for Hilt navigation and splash screen |
| presentation/src/main/java/project/side/presentation/viewmodel/LoginViewModel.kt | ViewModel managing login/logout state and coordinating with use cases |
| domain/src/main/java/project/side/domain/usecase/auth/LoginUseCase.kt | Use case orchestrating login flow for different providers |
| domain/src/main/java/project/side/domain/usecase/auth/LogoutUseCase.kt | Use case orchestrating logout flow for different providers |
| domain/src/main/java/project/side/domain/repository/AuthRepository.kt | Repository interface defining authentication operations |
| domain/src/main/java/project/side/domain/model/SocialAuthType.kt | Enum defining supported social auth types |
| domain/src/main/java/project/side/domain/model/LoginState.kt | Sealed class representing login flow states |
| domain/src/main/java/project/side/domain/model/LogoutState.kt | Sealed class representing logout flow states |
| data/src/main/java/project/side/data/repository/AuthRepositoryImpl.kt | Repository implementation coordinating social and API authentication |
| data/src/main/java/project/side/data/datasource/AuthDataSource.kt | Interface for API-based authentication operations |
| data/src/main/java/project/side/data/datasource/AuthDataStoreSource.kt | Interface for local auth data persistence |
| data/src/main/java/project/side/data/datasource/SocialAuthDataSource.kt | Interface for social provider authentication |
| data/src/main/java/project/side/data/model/DataApiResult.kt | Sealed class for API result wrapping |
| data/src/main/java/project/side/data/model/LoginResult.kt | Data model for successful login response |
| data/src/main/java/project/side/data/model/SocialLoginResult.kt | Data model for social login response |
| remote/src/main/java/project/side/remote/login/GoogleAuth.kt | Google OAuth implementation using Credential Manager |
| remote/src/main/java/project/side/remote/login/KakaoAuth.kt | Kakao OAuth implementation with account and talk login |
| remote/src/main/java/project/side/remote/login/NaverAuth.kt | Naver OAuth implementation |
| remote/src/main/java/project/side/remote/datasource/SocialAuthDataSourceImpl.kt | Implementation coordinating all social auth providers |
| remote/src/main/java/project/side/remote/datasource/AuthDataSourceImpl.kt | Implementation handling API authentication calls |
| remote/src/main/java/project/side/remote/api/AuthService.kt | Retrofit service interface for auth endpoints |
| remote/src/main/java/project/side/remote/model/login/LoginResponse.kt | API request/response models for login |
| remote/src/main/java/project/side/remote/di/RemoteModule.kt | Dependency injection for remote data sources |
| remote/src/main/java/project/side/remote/di/SocialAuthModule.kt | Dependency injection for social auth with activity context |
| remote/build.gradle.kts | Social login SDK dependencies and Google client ID configuration |
| local/src/main/java/project/side/local/datasource/AuthDataStoreSourceImpl.kt | DataStore implementation for auth data persistence |
| local/src/main/java/project/side/local/di/LocalModule.kt | Dependency injection for DataStore |
| local/build.gradle.kts | DataStore dependencies |
| app/src/main/java/project/side/ikdaman/IkdamanApplication.kt | Kakao and Naver SDK initialization |
| app/src/main/java/project/side/ikdaman/di/ActivityModule.kt | Activity-scoped dependency injection for auth flow |
| app/src/main/AndroidManifest.xml | Kakao OAuth redirect handler and permissions |
| app/build.gradle.kts | BuildConfig fields for API keys and signing configuration |
| app/proguard-rules.pro | ProGuard rules for Kakao SDK |
| settings.gradle.kts | Kakao Maven repository |
| gradle/libs.versions.toml | Version catalog for social login and DataStore dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Button(onClick = { | ||
| viewModel?.naverLogin(loginUseCase!!) | ||
| }, modifier = Modifier.fillMaxWidth()) { | ||
| Text("Naver Login") |
Copilot
AI
Jan 3, 2026
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.
Force-unwrapping nullable parameters with the non-null assertion operator (!!) can cause crashes. If loginUseCase is null, this will throw a NullPointerException. Consider using safe calls or require checks at the function entry point instead.
| val NICKNAME_KEY = stringPreferencesKey("nickname") | ||
| } | ||
|
|
||
| val nickname: Flow<String?> = authDataStore.data.map { prefs -> |
Copilot
AI
Jan 3, 2026
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.
The nickname Flow property is defined but never exposed through the interface. If this is meant to be publicly accessible, it should be added to the AuthDataStoreSource interface. Otherwise, it should be private.
| val nickname: Flow<String?> = authDataStore.data.map { prefs -> | |
| private val nickname: Flow<String?> = authDataStore.data.map { prefs -> |
| override suspend fun googleLogout(): Boolean { | ||
| try { | ||
| GoogleAuth.logout(context) | ||
| return true | ||
| } catch (e: Exception) { | ||
| return false | ||
| } |
Copilot
AI
Jan 3, 2026
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.
Catching all exceptions with a generic Exception handler and silently returning false can hide real problems. Consider catching specific exception types or at least logging the error for debugging purposes.
| signingConfigs { | ||
| create("release") { | ||
| storeFile = project.rootProject.file("release.keystore") | ||
| storePassword = properties.getProperty("KEYSTORE_PASSWORD") | ||
| keyAlias = properties.getProperty("KEY_ALIAS") | ||
| keyPassword = properties.getProperty("KEY_PASSWORD") | ||
| } |
Copilot
AI
Jan 3, 2026
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.
The signingConfig block references release.keystore and loads credentials from key.properties. Ensure both release.keystore and key.properties are properly gitignored and not committed to version control to prevent exposing sensitive signing credentials.
| @Provides | ||
| @ActivityScoped | ||
| fun provideAuthRepository( | ||
| authDataSource: AuthDataSource, | ||
| socialAuthDataSource: SocialAuthDataSource, | ||
| authDataStoreSource: AuthDataStoreSource | ||
| ): AuthRepository = AuthRepositoryImpl(authDataSource, socialAuthDataSource, authDataStoreSource) |
Copilot
AI
Jan 3, 2026
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.
Using ActivityScoped for the AuthRepository is unusual since repositories typically should be Singleton scoped to maintain consistent state across the application. However, since SocialAuthDataSource requires an Activity context, this creates a dependency chain issue. Consider refactoring to inject the Activity context only where needed rather than scoping the entire repository to the activity lifecycle.
| val provider = socialResult.provider!! | ||
| val loginResult = authDataSource.login( | ||
| socialResult.socialAccessToken!!, | ||
| provider, | ||
| socialResult.providerId!! |
Copilot
AI
Jan 3, 2026
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.
Force-unwrapping nullable properties with the non-null assertion operator (!!) can cause crashes. If any of these properties (provider, socialAccessToken, providerId) are null when isSuccess is true, this will throw a NullPointerException. Consider using safe calls with early returns or explicit null checks with proper error handling.
| val provider = socialResult.provider!! | |
| val loginResult = authDataSource.login( | |
| socialResult.socialAccessToken!!, | |
| provider, | |
| socialResult.providerId!! | |
| val provider = socialResult.provider | |
| val accessToken = socialResult.socialAccessToken | |
| val providerId = socialResult.providerId | |
| if (provider == null || accessToken == null || providerId == null) { | |
| emit(LoginState.Error("로그인 정보가 올바르지 않습니다.")) | |
| return | |
| } | |
| val loginResult = authDataSource.login( | |
| accessToken, | |
| provider, | |
| providerId |
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.
이거 예외처리는 필요해보입니다!
| data.refreshToken, | ||
| data.nickname | ||
| ) | ||
| emit(LoginState.Success) |
Copilot
AI
Jan 3, 2026
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.
When login API call fails, no error state is emitted to the flow. This means the UI will remain in loading state indefinitely. Add an else block to emit LoginState.Error when loginResult is not Success.
| emit(LoginState.Success) | |
| emit(LoginState.Success) | |
| } else { | |
| emit(LoginState.Error("로그인 실패")) |
| val kakaoAppKey = properties.getProperty("KAKAO_APP_KEY") | ||
| buildConfigField("String", "KAKAO_APP_KEY", "\"$kakaoAppKey\"") | ||
| manifestPlaceholders["KAKAO_APP_KEY"] = kakaoAppKey | ||
|
|
||
| buildConfigField("String", "NAVER_CLIENT_ID", "\"${properties.getProperty("NAVER_CLIENT_ID")}\"") | ||
| buildConfigField("String", "NAVER_CLIENT_SECRET", "\"${properties.getProperty("NAVER_CLIENT_SECRET")}\"") |
Copilot
AI
Jan 3, 2026
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.
Sensitive configuration values (KAKAO_APP_KEY, NAVER_CLIENT_ID, NAVER_CLIENT_SECRET) are being hardcoded in BuildConfig. While they're loaded from key.properties file, ensure the key.properties file is properly gitignored and not committed to version control to prevent exposing these secrets.
| @Provides | ||
| @ActivityScoped | ||
| fun provideLoginUseCase(authRepository: AuthRepository) = LoginUseCase(authRepository) | ||
|
|
||
|
|
||
| @Provides | ||
| @ActivityScoped | ||
| fun provideLogoutUseCase(authRepository: AuthRepository) = LogoutUseCase(authRepository) |
Copilot
AI
Jan 3, 2026
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.
Using ActivityScoped for UseCases is unusual. UseCases are typically application-level dependencies and should be Singleton scoped, especially since the repository they depend on is already ActivityScoped. This creates an inconsistent dependency graph where each activity gets its own use case instances.
| DataApiResult.Error("서버 오류: ${response.code()}, ${response.message()}") | ||
| } | ||
| } catch (e: Exception) { | ||
| DataApiResult.Error("네트워크 오류: ${e.message}") | ||
| } | ||
| } | ||
|
|
||
| override suspend fun logout(): DataApiResult<Unit> { | ||
| return try { | ||
| val response = authService.logout() | ||
| if (response.isSuccessful) { | ||
| DataApiResult.Success(Unit) | ||
| } else DataApiResult.Error("서버 오류: ${response.code()}, ${response.message()}") | ||
| } catch (e: Exception) { | ||
| DataApiResult.Error("서버 오류: ${e.message}") |
Copilot
AI
Jan 3, 2026
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.
The error message uses a generic format that could be confusing to users. Consider providing more specific and actionable error messages, or distinguishing between different types of server errors (e.g., 401 Unauthorized vs 500 Internal Server Error).
| DataApiResult.Error("서버 오류: ${response.code()}, ${response.message()}") | |
| } | |
| } catch (e: Exception) { | |
| DataApiResult.Error("네트워크 오류: ${e.message}") | |
| } | |
| } | |
| override suspend fun logout(): DataApiResult<Unit> { | |
| return try { | |
| val response = authService.logout() | |
| if (response.isSuccessful) { | |
| DataApiResult.Success(Unit) | |
| } else DataApiResult.Error("서버 오류: ${response.code()}, ${response.message()}") | |
| } catch (e: Exception) { | |
| DataApiResult.Error("서버 오류: ${e.message}") | |
| DataApiResult.Error(mapServerError(response.code(), response.message())) | |
| } | |
| } catch (e: Exception) { | |
| DataApiResult.Error("네트워크 오류: ${e.message}") | |
| } | |
| } | |
| private fun mapServerError(code: Int, message: String?): String { | |
| return when (code) { | |
| 400 -> "잘못된 요청입니다 (HTTP $code)." | |
| 401 -> "인증이 만료되었거나 유효하지 않습니다. 다시 로그인해주세요 (HTTP $code)." | |
| 403 -> "접근 권한이 없습니다 (HTTP $code)." | |
| 404 -> "요청한 리소스를 찾을 수 없습니다 (HTTP $code)." | |
| in 500..599 -> "서버 내부 오류가 발생했습니다 (HTTP $code)." | |
| else -> "알 수 없는 서버 오류가 발생했습니다 (HTTP $code: ${message ?: "no message"})" | |
| } | |
| } | |
| override suspend fun logout(): DataApiResult<Unit> { | |
| return try { | |
| val response = authService.logout() | |
| if (response.isSuccessful) { | |
| DataApiResult.Success(Unit) | |
| } else DataApiResult.Error(mapServerError(response.code(), response.message())) | |
| } catch (e: Exception) { | |
| DataApiResult.Error("네트워크 오류: ${e.message}") |
yewon-yw
left a comment
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.
로그인 로그아웃 동작 전부 확인했습니다~🙇🏻♂️🙇🏻♂️
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.
똑같은거 domain 모듈에 dataResource 이름으로 만들어놨었는데 지금 보니까 도메인에 있을 필요가 없어보이네요! 저도 이거 쓸게요
| val provider = socialResult.provider!! | ||
| val loginResult = authDataSource.login( | ||
| socialResult.socialAccessToken!!, | ||
| provider, | ||
| socialResult.providerId!! |
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.
이거 예외처리는 필요해보입니다!
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.
근데 이런식으로 하면 state 클래스가 계속 만들어질 것 같은데 도메인에 만들어둔 dataResource를 쓰면 안되나요?
| sealed class UIState { | ||
| data object Init : UIState() | ||
| data object Loading : UIState() | ||
| data class Success(val message: String) : UIState() | ||
| data class Error(val message: String) : UIState() | ||
| } | ||
|
|
||
| enum class AuthType { | ||
| GOOGLE, NAVER, KAKAO; | ||
|
|
||
| fun toDomainAuthType(): SocialAuthType { | ||
| return when (this) { | ||
| GOOGLE -> SocialAuthType.GOOGLE | ||
| NAVER -> SocialAuthType.NAVER | ||
| KAKAO -> SocialAuthType.KAKAO | ||
| } | ||
| } | ||
| } |
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.
얘네 모델로 따로 빼면 좋을 것 같아요!
Google, Kakao, naver 로그인 되는 것 확인