[FEAT] 유저 로그인, 회원 가입, 토큰 갱신, 자동 로그인, 로그아웃, Authorization 전역 헤더 구현#20
Conversation
|
Warning Rate limit exceeded@Sangyoon98 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 8 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
워크스루인증 시스템을 변경사항
시퀀스 다이어그램sequenceDiagram
participant User
participant LoginScreen
participant AuthViewModel
participant AuthRepository
participant TokenRefreshService
participant AuthPreferences
participant NetworkModule
User->>LoginScreen: 로그인 시도 (이메일, 비밀번호)
LoginScreen->>AuthRepository: signIn(email, password)
AuthRepository->>NetworkModule: API 호출 (login)
NetworkModule-->>AuthRepository: LoginResponseDto 수신
AuthRepository->>AuthPreferences: saveUser(user)
AuthPreferences-->>AuthRepository: 저장 완료
AuthRepository-->>LoginScreen: User 반환
LoginScreen->>AuthViewModel: updateLoginState()
AuthViewModel->>AuthViewModel: isLoggedIn = true
AuthViewModel-->>LoginScreen: 상태 업데이트
LoginScreen->>User: 홈 화면으로 네비게이션
rect rgb(200, 220, 255)
Note over TokenRefreshService,AuthPreferences: 자동 토큰 갱신 흐름 (401 응답 시)
NetworkModule->>TokenAuthenticator: 401 응답 감지
TokenAuthenticator->>TokenRefreshService: refreshToken()
TokenRefreshService->>AuthPreferences: 저장된 refreshToken 조회
TokenRefreshService->>NetworkModule: API 호출 (refresh)
NetworkModule-->>TokenRefreshService: 새로운 토큰 수신
TokenRefreshService->>AuthPreferences: 새로운 토큰 저장
TokenRefreshService-->>TokenAuthenticator: Result<User> 반환
TokenAuthenticator->>NetworkModule: 재시도 요청 (새 토큰 포함)
end
코드 리뷰 예상 소요 시간🎯 4 (복잡함) | ⏱️ ~60분 분석 근거:
관련 PR
제안된 라벨
제안된 리뷰어
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
33Auto-Bot
requested review from
CHOOSLA,
Lee-Jong-Jin,
taemin3,
vivivim and
yangjiseonn
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/sampoom/android/feature/user/domain/AuthValidator.kt (1)
7-18: 이메일 검증 정규식 패턴 개선 필요현재 정규식 패턴에 다음 문제가 있습니다:
- 도메인 부분이 소문자만 허용하여 유효한 이메일을 거부할 수 있음
\\.+패턴이 연속된 점을 허용하여user@domain..com같은 무효한 이메일을 허용- TLD 구조를 제대로 검증하지 않음
더 견고한 패턴으로 개선하거나 Android의
Patterns.EMAIL_ADDRESS를 사용하는 것을 권장합니다:- val emailPattern = "[a-zA-Z0-9._-]+@[a-z]+\\.+[a-z]+".toRegex() - if (!email.matches(emailPattern)) { + if (!android.util.Patterns.EMAIL_ADDRESS.matcher(email).matches()) { return ValidationResult.Error(R.string.validation_email_invalid) }
🧹 Nitpick comments (15)
app/src/main/java/com/sampoom/android/feature/user/domain/AuthValidator.kt (1)
21-43: 특수 문자 검증 로직 명확화 권장Line 36의 특수 문자 검증이
!it.isLetterOrDigit()를 사용하여 공백, 탭 등도 특수 문자로 인정합니다. 의도한 동작이라면 문제없지만, 일반적으로 특수 문자는!@#$%등의 기호만 포함하도록 명시하는 것이 보안상 더 명확합니다.명시적인 특수 문자 집합을 사용하는 것을 고려해보세요:
val hasLetter = password.any { it.isLetter() } val hasDigit = password.any { it.isDigit() } - val hasSpecial = password.any { !it.isLetterOrDigit() } + val hasSpecial = password.any { it in "!@#$%^&*()_+-=[]{}|;:',.<>?/" }app/src/main/java/com/sampoom/android/feature/user/ui/LoginViewModel.kt (1)
53-65: 로그인 이메일 형식 검증 추가 권장현재 로그인 화면에서는 이메일이 비어있지 않은지만 확인하고 형식 검증을 하지 않습니다. 회원가입과 달리
AuthValidator.validateEmail()을 사용하지 않아, 사용자가 잘못된 이메일 형식을 입력해도 서버 요청 후에야 오류를 알 수 있습니다.이메일 형식 검증을 추가하여 UX를 개선하는 것을 고려해보세요:
private fun validateEmail() { - val result = AuthValidator.validateNotEmpty(_uiState.value.email, emailLabel) + val result = AuthValidator.validateEmail(_uiState.value.email) _uiState.value = _uiState.value.copy( emailError = result.toErrorMessage() ) }app/src/main/java/com/sampoom/android/feature/user/ui/SignUpScreen.kt (1)
88-88: 주석 처리된 코드 제거 권장사용하지 않는 주석 처리된 코드는 제거하는 것이 코드베이스를 깔끔하게 유지하는 데 도움이 됩니다.
-// snackbarHost = { CommonSnackBarHost(snackBarHostState) }app/src/main/java/com/sampoom/android/feature/user/data/mapper/AuthMappers.kt (1)
6-6: named parameter 사용을 권장합니다.현재 positional parameter를 사용한 매핑은 User 데이터 클래스의 필드 순서가 변경될 경우 런타임 오류나 잘못된 매핑을 유발할 수 있습니다.
다음과 같이 named parameter를 사용하도록 리팩토링하세요:
-fun LoginResponseDto.toModel(): User = User(userId, userName, role, accessToken, refreshToken, expiresIn) +fun LoginResponseDto.toModel(): User = User( + userId = userId, + userName = userName, + role = role, + accessToken = accessToken, + refreshToken = refreshToken, + expiresIn = expiresIn +)app/src/main/java/com/sampoom/android/feature/user/domain/model/User.kt (1)
3-10: 보안 및 관심사 분리를 고려해보세요.현재 User 도메인 모델에 accessToken과 refreshToken이 포함되어 있습니다. 비즈니스 데이터(userId, userName, role)와 인증 토큰을 함께 보관하는 것은 다음과 같은 우려사항이 있습니다:
- 토큰은 민감한 데이터로 로깅이나 직렬화 시 주의가 필요합니다
- 도메인 모델이 인증 관련 책임까지 가지게 됩니다
인증 플로우에서만 사용되는 것이라면 현재 구조도 충분하지만, 향후 확장성을 고려하여 토큰을 별도 데이터 클래스로 분리하는 것을 검토해보세요.
app/src/main/java/com/sampoom/android/app/navigation/AppNavHost.kt (3)
84-86: popUpTo(0) 대신 명시적 대상 사용.popUpTo(0)은 구현 의존적이며 깨질 수 있습니다. 그래프 id/시작 목적지를 명시하세요.
- navController.navigate(ROUTE_LOGIN) { - popUpTo(0) { inclusive = true } - } + navController.navigate(ROUTE_LOGIN) { + popUpTo(navController.graph.startDestinationId) { inclusive = true } + launchSingleTop = true + }동일 패턴을 Dashboard 쪽 네비게이션에도 적용하세요.
수정 후 "로그아웃 > 백스택 완전 초기화" 동작을 실제 기기에서 한 번 확인 부탁드립니다.
Also applies to: 181-183
270-284: DashboardScreen: 네비 콜백 제거 및 padding 중복 정리.
- onClick 콜백으로 네비를 직접 트리거하는 대신 logoutEvent 한 곳에서만 처리하도록 제거하세요.
- Column 에 padding 을 주었는데 Text 에 동일 padding 을 중복 적용했습니다.
-private fun DashboardScreen( - paddingValues: PaddingValues, - onClick: () -> Unit -) { +private fun DashboardScreen( + paddingValues: PaddingValues +) { val authViewModel: AuthViewModel = hiltViewModel() - Column(Modifier.padding(paddingValues)) { - Text("대시보드 화면", modifier = Modifier.padding(paddingValues)) + Column(Modifier.padding(paddingValues)) { + Text("대시보드 화면") - Button(onClick = { - authViewModel.signOut() - onClick() - }) { + Button(onClick = { authViewModel.signOut() }) { Text("로그아웃") } } }
126-129: agencyId 하드코딩 제거 TODO 처리.현재 1로 고정되어 있습니다. 실제 사용자 정보에서 가져오도록 수정이 필요합니다. 런타임 혼선을 피하려면 주석 대신 NotImplementedError() 등으로 임시 가드하거나, 안전 기본값/게이트를 두세요.
원하시면 사용자 컨텍스트에서 agencyId 를 주입하는 패턴(예: Session/AccountManager) 제안드리겠습니다.
Also applies to: 200-202
app/src/main/java/com/sampoom/android/core/network/TokenRefreshService.kt (2)
20-27: Retrofit 인스턴스를 매번 생성하지 말고 DI로 싱글톤 주입.refreshToken 호출마다 Retrofit 을 만들면 불필요한 오버헤드와 설정 분산이 생깁니다. 인터셉터 없는 전용 OkHttp/Retrofit 을 DI 모듈에서 싱글톤으로 제공하고, 여기서는 AuthApi 만 주입받아 호출하세요. 또한 baseUrl 하드코딩 대신 BuildConfig 등 환경설정을 사용하세요.
예시(개념 스케치):
- NetworkModule 에
@Named("refresh") OkHttpClient(인터셉터 없음) +@Named("refresh") Retrofit+@Named("refresh") AuthApi제공- TokenRefreshService 생성자:
TokenRefreshService(authPreferences: AuthPreferences, @Named("refresh") authApi: AuthApi)- // 새로운 Retrofit 인스턴스 생성 (인터셉터 없이) - val retrofit = Retrofit.Builder() - .baseUrl("https://sampoom.store/api/") - .addConverterFactory(GsonConverterFactory.create()) - .build() - - val authApi = retrofit.create(AuthApi::class.java) + // DI로 주입받은 전용 AuthApi 사용 (인터셉터 없음, 싱글톤)
29-40: User 업데이트 범위와 에러 구분 개선 제안.
- 서버가 사용자 속성(권한/이름 등)을 함께 갱신할 수 있다면, refresh 응답 기반으로 필요한 필드도 업데이트하는게 안전합니다. 현재는 토큰/만료만 갱신.
- runCatching 으로 예외를 전부 Result 로 감싸는 대신, 토큰 없음/사용자 없음/네트워크 오류를 구분해 상위에서 UX 를 달리 처리할 수 있게 하면 좋습니다(예: sealed error).
app/src/main/java/com/sampoom/android/feature/user/domain/repository/AuthRepository.kt (1)
15-19: 토큰 갱신 책임 단일화.인터셉터에서 TokenRefreshService 를 직접 사용하고, 레포에도 refreshToken() 이 있습니다. 두 경로가 분기되면 중복/불일치가 생깁니다. 한 곳(AuthRepository or Service)으로 책임을 모으고, 다른 곳은 이를 위임만 하도록 정리하세요.
app/src/main/java/com/sampoom/android/feature/user/ui/AuthViewModel.kt (1)
32-36: signOut 실패 처리 및 이벤트 방출 안정화예외 미처리로 크래시/침묵 실패 위험이 있습니다. 이벤트는 tryEmit로 비차단 방출 권장.
- fun signOut() = viewModelScope.launch { - signOutUseCase() - _isLoggedIn.value = false - _logoutEvent.emit(Unit) - } + fun signOut() = viewModelScope.launch { + runCatching { signOutUseCase() } + .onFailure { /* TODO: 오류 상태 노출(스낵바/로깅) */ } + _isLoggedIn.value = false + _logoutEvent.tryEmit(Unit) + }app/src/main/java/com/sampoom/android/feature/user/data/repository/AuthRepositoryImpl.kt (2)
51-65: runBlocking 기반 동기 getter 사용 최소화 및 IO 디스패처로 오프로딩refreshToken 내 getRefreshToken/getStoredUser는 블로킹입니다. 우선 IO로 오프로딩하고, 추후 suspend getter로 전환 권장.
- val refreshToken = preferences.getRefreshToken() ?: throw Exception("No refresh token available") + val refreshToken = kotlinx.coroutines.withContext(kotlinx.coroutines.Dispatchers.IO) { + preferences.getRefreshToken() + } ?: throw Exception("No refresh token available") @@ - val existingUser = preferences.getStoredUser() ?: throw Exception("No user information available") + val existingUser = kotlinx.coroutines.withContext(kotlinx.coroutines.Dispatchers.IO) { + preferences.getStoredUser() + } ?: throw Exception("No user information available")추가로, 도메인 친화적 오류 타입(예: NoRefreshTokenError)로 감싸면 상위 처리 단순화됩니다.
71-71: 만료 고려 없는 isSignedIn 판단토큰 존재만 확인하면 만료 토큰으로도 로그인 상태로 인식될 수 있습니다. 만료 검사 포함 권장.
- override fun isSignedIn(): Boolean = preferences.hasToken() + override fun isSignedIn(): Boolean = + preferences.hasToken() && !preferences.isTokenExpired()app/src/main/java/com/sampoom/android/core/datastore/AuthPreferences.kt (1)
87-88: 만료 무시한 hasToken만료된 토큰이 있어도 true 반환합니다. 만료 검사를 포함하세요.
- fun hasToken(): Boolean = !getAccessToken().isNullOrEmpty() && !getRefreshToken().isNullOrEmpty() + fun hasToken(): Boolean = + !getAccessToken().isNullOrEmpty() && + !getRefreshToken().isNullOrEmpty() && + !isTokenExpired()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (43)
app/src/main/java/com/sampoom/android/app/navigation/AppNavHost.kt(6 hunks)app/src/main/java/com/sampoom/android/core/datastore/AuthPreferences.kt(1 hunks)app/src/main/java/com/sampoom/android/core/network/NetworkModule.kt(1 hunks)app/src/main/java/com/sampoom/android/core/network/TokenInterceptor.kt(1 hunks)app/src/main/java/com/sampoom/android/core/network/TokenRefreshInterceptor.kt(1 hunks)app/src/main/java/com/sampoom/android/core/network/TokenRefreshService.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/auth/data/local/preferences/AuthPreferences.kt(0 hunks)app/src/main/java/com/sampoom/android/feature/auth/data/mapper/AuthMappers.kt(0 hunks)app/src/main/java/com/sampoom/android/feature/auth/data/remote/api/AuthApi.kt(0 hunks)app/src/main/java/com/sampoom/android/feature/auth/data/remote/dto/SignUpRequestDto.kt(0 hunks)app/src/main/java/com/sampoom/android/feature/auth/data/repository/AuthRepositoryImpl.kt(0 hunks)app/src/main/java/com/sampoom/android/feature/auth/domain/model/User.kt(0 hunks)app/src/main/java/com/sampoom/android/feature/auth/domain/repository/AuthRepository.kt(0 hunks)app/src/main/java/com/sampoom/android/feature/user/data/mapper/AuthMappers.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/data/remote/api/AuthApi.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/LoginRequestDto.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/LoginResponseDto.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/RefreshRequestDto.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/RefreshResponseDto.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/SignUpRequestDto.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/SignUpResponseDto.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/UpdateRequestDto.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/UpdateResponseDto.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/VerifyRequestDto.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/VerifyResponseDto.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/data/repository/AuthRepositoryImpl.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/di/AuthModules.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/domain/AuthValidator.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/domain/model/User.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/domain/repository/AuthRepository.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/domain/usecase/CheckLoginStateUseCase.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/domain/usecase/LoginUseCase.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/domain/usecase/SignOutUseCase.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/domain/usecase/SignUpUseCase.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/ui/AuthViewModel.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/ui/LoginScreen.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/ui/LoginUiEvent.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/ui/LoginUiState.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/ui/LoginViewModel.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/ui/SignUpScreen.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/ui/SignUpUiEvent.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/ui/SignUpUiState.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/ui/SignUpViewModel.kt(2 hunks)
💤 Files with no reviewable changes (7)
- app/src/main/java/com/sampoom/android/feature/auth/domain/repository/AuthRepository.kt
- app/src/main/java/com/sampoom/android/feature/auth/domain/model/User.kt
- app/src/main/java/com/sampoom/android/feature/auth/data/local/preferences/AuthPreferences.kt
- app/src/main/java/com/sampoom/android/feature/auth/data/remote/dto/SignUpRequestDto.kt
- app/src/main/java/com/sampoom/android/feature/auth/data/repository/AuthRepositoryImpl.kt
- app/src/main/java/com/sampoom/android/feature/auth/data/mapper/AuthMappers.kt
- app/src/main/java/com/sampoom/android/feature/auth/data/remote/api/AuthApi.kt
🧰 Additional context used
🧬 Code graph analysis (2)
app/src/main/java/com/sampoom/android/feature/user/domain/repository/AuthRepository.kt (1)
app/src/main/java/com/sampoom/android/feature/auth/domain/repository/AuthRepository.kt (1)
signUp(5-18)
app/src/main/java/com/sampoom/android/feature/user/data/repository/AuthRepositoryImpl.kt (2)
app/src/main/java/com/sampoom/android/feature/user/domain/repository/AuthRepository.kt (1)
signIn(15-15)app/src/main/java/com/sampoom/android/feature/auth/data/repository/AuthRepositoryImpl.kt (1)
api(12-47)
🔇 Additional comments (31)
app/src/main/java/com/sampoom/android/feature/user/ui/SignUpUiState.kt (1)
1-1: 패키지 이동이 적절하게 수행되었습니다.인증 관련 기능을
feature.user패키지로 이동하는 것은 PR의 목적과 일치하며, 코드 구조 개선에 도움이 됩니다.app/src/main/java/com/sampoom/android/feature/user/ui/LoginUiState.kt (1)
1-1: 패키지 이동이 완벽하게 적용되었습니다.검증 결과:
- ✅
LoginUiState.kt가 새 패키지(com.sampoom.android.feature.user.ui)로 올바르게 이동됨- ✅ 구 패키지(
com.sampoom.android.feature.auth.ui) 참조 없음- ✅
LoginViewModel.kt가 같은 패키지에 위치하여 import 정상 처리됨- ✅ 모든 의존성이 올바르게 업데이트됨
리팩토링이 완료되었고 빌드 문제가 없습니다.
app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/VerifyRequestDto.kt (1)
3-6: LGTM!DTO 구조가 명확하고 올바르게 정의되어 있습니다.
app/src/main/java/com/sampoom/android/feature/user/domain/AuthValidator.kt (2)
1-1: LGTM!패키지 마이그레이션이 PR 목표와 일치합니다.
46-72: LGTM!비밀번호 확인 검증과 일반 필드 검증 로직이 올바르게 구현되어 있으며,
ValidationResultsealed class 구조가 깔끔합니다.app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/VerifyResponseDto.kt (1)
3-8: LGTM!응답 DTO가 명확하게 정의되어 있습니다.
app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/SignUpRequestDto.kt (1)
3-10: LGTM!회원가입에 필요한 모든 필드가 적절하게 정의되어 있습니다.
app/src/main/java/com/sampoom/android/feature/user/ui/LoginViewModel.kt (2)
1-1: LGTM!패키지 마이그레이션과 import 업데이트가 일관성 있게 적용되었습니다.
Also applies to: 7-9
75-92: LGTM!로그인 제출 로직이 올바르게 구현되어 있습니다. 코루틴 사용, 에러 핸들링, 상태 관리가 적절합니다.
app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/SignUpResponseDto.kt (1)
1-7: LGTM!패키지 마이그레이션이 적절하게 수행되었습니다.
app/src/main/java/com/sampoom/android/feature/user/ui/LoginUiEvent.kt (1)
1-7: LGTM!패키지 마이그레이션이 완료되었으며, 이벤트 구조가 적절합니다.
app/src/main/java/com/sampoom/android/feature/user/ui/SignUpScreen.kt (3)
1-1: LGTM!패키지 마이그레이션이 올바르게 적용되었습니다.
3-42: LGTM!사용하지 않는 import 제거가 적절하게 수행되었습니다.
44-210: LGTM!회원가입 화면 구현이 올바르게 되어 있으며, 폼 검증과 에러 처리가 적절합니다.
app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/RefreshResponseDto.kt (1)
3-7: LGTM! 토큰 갱신 DTO 구조가 적절합니다.토큰 갱신을 위한 DTO 구조가 명확하며,
expiresIn을Long타입으로 정의한 것이 타임스탬프/만료 시간 처리에 적합합니다.app/src/main/java/com/sampoom/android/feature/user/ui/SignUpUiEvent.kt (1)
1-1: 패키지 이동이 올바르게 적용되었습니다.
feature.auth.ui에서feature.user.ui로의 패키지 이동이 PR의 리팩토링 목표와 일치합니다.app/src/main/java/com/sampoom/android/feature/user/di/AuthModules.kt (1)
1-5: DI 모듈의 패키지 및 임포트 업데이트가 정확합니다.패키지 선언과 임포트 경로가 새로운
feature.user구조에 맞게 올바르게 업데이트되었습니다.app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/LoginRequestDto.kt (1)
1-1: 패키지 이동이 올바르게 적용되었습니다.DTO의 패키지가
feature.user구조로 적절하게 이동되었습니다.app/src/main/java/com/sampoom/android/feature/user/ui/LoginScreen.kt (1)
1-1: 패키지 이동 및 불필요한 임포트 제거가 잘 적용되었습니다.
feature.user.ui로의 패키지 이동과 함께 사용하지 않는 임포트를 제거하여 코드가 깔끔해졌습니다.app/src/main/java/com/sampoom/android/feature/user/domain/usecase/SignOutUseCase.kt (1)
6-10: LGTM! UseCase 구현이 표준 패턴을 잘 따르고 있습니다.로그아웃 로직을 캡슐화한 UseCase 구현이 명확하며, suspend 함수와 operator를 적절히 활용하고 있습니다.
app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/LoginResponseDto.kt (1)
9-9:expiresIn타입을Long으로 변경한 것이 적절합니다.
Int에서Long으로 변경하여 타임스탬프 값의 오버플로우를 방지하고,RefreshResponseDto와 일관성을 유지하고 있습니다. 만료 시간 처리에 더 안전한 선택입니다.app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/UpdateResponseDto.kt (1)
3-10: LGTM! 사용자 정보 업데이트 DTO가 명확하게 정의되었습니다.사용자 프로필 정보를 담는 DTO 구조가 깔끔하고 적절하게 정의되어 있습니다.
app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/RefreshRequestDto.kt (1)
3-5: LGTM!토큰 갱신 요청을 위한 간단하고 명확한 DTO 구조입니다.
app/src/main/java/com/sampoom/android/feature/user/domain/usecase/CheckLoginStateUseCase.kt (1)
6-10: LGTM!로그인 상태 확인을 위한 간결하고 적절한 use case 구현입니다.
app/src/main/java/com/sampoom/android/feature/user/data/remote/dto/UpdateRequestDto.kt (1)
3-8: LGTM!유저 정보 업데이트를 위한 명확한 DTO 구조입니다.
app/src/main/java/com/sampoom/android/feature/user/domain/usecase/LoginUseCase.kt (1)
1-4: LGTM!auth에서 user 패키지로의 마이그레이션이 올바르게 적용되었습니다.
app/src/main/java/com/sampoom/android/feature/user/domain/usecase/SignUpUseCase.kt (1)
10-24: LGTM!파라미터 순서 변경(email/password 우선)과 이름 변경(name → userName)이 일관되게 적용되었으며, named parameter를 사용하여 안전하게 repository를 호출하고 있습니다.
app/src/main/java/com/sampoom/android/feature/user/ui/SignUpViewModel.kt (1)
139-146: LGTM!SignUpUseCase의 변경된 파라미터 시그니처와 일치하도록 올바르게 업데이트되었습니다. Named parameter를 사용하여 안전하게 호출하고 있으며, userName = s.name 매핑도 적절합니다.
app/src/main/java/com/sampoom/android/feature/user/domain/repository/AuthRepository.kt (2)
19-19: isSignedIn 동기 여부 확인.동기 Boolean 반환은 DataStore 등 I/O 를 동반하면 ANR/블로킹 리스크가 있습니다. 메모리 캐시 기반이 아니라면 suspend 또는 Flow 로 노출하는 방식을 고려하세요.
6-13: 네임드 아규먼트 안전성 확인 완료: 추가 조치 불필요전체 코드베이스 검색 결과,
signUp()호출부는 모두 네임드 아규먼트를 사용하고 있습니다.
SignUpUseCase.kt(라인 17-23):repository.signUp(email = email, password = password, workspace = workspace, branch = branch, userName = userName, position = position)형태로 호출- 위치 기반 인자(positional arguments)를 사용하는 호출부 없음
Kotlin에서 네임드 아규먼트는 파라미터 순서와 무관하므로, 현재 상태에서는 파라미터 순서 변경이 버그를 야기하지 않습니다.
app/src/main/java/com/sampoom/android/feature/user/data/repository/AuthRepositoryImpl.kt (1)
36-44: 검증 결과: 토큰/만료 필드가 모두 포함되어 있으며 코드는 정상입니다검증을 통해 확인한 결과,
LoginResponseDto.toModel()매퍼가accessToken,refreshToken,expiresIn을 모두 올바르게User도메인 모델로 매핑하고 있습니다.saveUser(user)는 이 모든 필드에 접근하며, 데이터 흐름에 문제가 없습니다.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (11)
app/build.gradle.kts (1)
36-36: 버전 범프가 PR의 변경 범위와 정렬되는지 확인해주세요.현재
versionName을 "1.0"에서 "1.0.0"으로 변경했습니다. 이는 기술적으로 패치 버전 추가일 뿐 버전 번호 증가가 아닙니다. 이 PR에서 구현한 사용자 인증 기능들(로그인, 회원가입, 토큰 갱신, 자동 로그인, 로그아웃, Authorization 헤더)은 상당한 새로운 기능이므로, 의미 있는 버전(예: "1.1.0" 또는 더 높은 버전)으로 범프하는 것을 권장합니다.app/src/main/java/com/sampoom/android/feature/user/ui/AuthViewModel.kt (1)
37-47: 에러 처리 추가를 권장합니다.
signOut()과handleTokenExpired()함수에서 유즈케이스 호출 시 예외가 발생하면_isLoggedIn.value = false와_logoutEvent.emit()이 실행되지 않을 수 있습니다. 보안상 토큰 삭제 실패 시에도 사용자를 로그아웃 상태로 표시해야 합니다.다음과 같이 try-finally 블록으로 감싸는 것을 권장합니다:
fun signOut() = viewModelScope.launch { - signOutUseCase() - _isLoggedIn.value = false - _logoutEvent.emit(Unit) + try { + signOutUseCase() + } finally { + _isLoggedIn.value = false + _logoutEvent.emit(Unit) + } } fun handleTokenExpired() = viewModelScope.launch { - clearTokensUseCase() - _isLoggedIn.value = false - _logoutEvent.emit(Unit) + try { + clearTokensUseCase() + } finally { + _isLoggedIn.value = false + _logoutEvent.emit(Unit) + } }app/src/main/java/com/sampoom/android/app/navigation/AppNavHost.kt (1)
17-17: 사용하지 않는 import 제거.
LaunchedEffect가 import되었지만 파일 내에서 사용되지 않습니다.-import androidx.compose.runtime.LaunchedEffectapp/src/main/java/com/sampoom/android/core/network/TokenAuthenticator.kt (1)
17-22: 401 재시도 루프 방지 로직 보강 제안X-Retry-Count로 1회 제한은 좋습니다. 추가로 priorResponse 체인 기반 방어를 넣으면 프록시/리다이렉트 경유 시 루프 위험을 더 낮출 수 있습니다.
다음과 같이 보완을 제안합니다:
override fun authenticate(route: Route?, response: Response): Request? { + // 과도한 재도전 방지 (priorResponse 체인 검사) + fun responseCount(r: Response?): Int { + var count = 0 + var cur = r + while (cur?.priorResponse != null) { + count++ + cur = cur.priorResponse + if (count >= 1) break // 1회만 허용 + } + return count + } // 이미 재시도된 요청인지 확인 if (response.request.header("X-Retry-Count") != null) { return null // 재시도 제한 } + if (responseCount(response) >= 1) return nullapp/src/main/java/com/sampoom/android/core/network/NetworkModule.kt (2)
47-59: 인터셉터 순서 조정: 로깅을 마지막에 배치현재 로깅이 토큰 주입 이전에 실행되어 최종 요청/재시도 요청의 상태가 로그에 반영되지 않을 수 있습니다. 민감 헤더는 이미 비식별화하므로 로깅을 마지막으로 이동하는 편이 관측에 유리합니다.
- .addInterceptor( - HttpLoggingInterceptor().apply { - level = if (BuildConfig.DEBUG) - HttpLoggingInterceptor.Level.BODY - else - HttpLoggingInterceptor.Level.NONE - redactHeader("Authorization") // 토큰 비식별화 - redactHeader("Cookie") // 쿠키 비식별화 - } - ) - .addInterceptor(tokenInterceptor) // 토큰 자동 삽입 + .addInterceptor(tokenInterceptor) // 토큰 자동 삽입 + .addInterceptor( + HttpLoggingInterceptor().apply { + level = if (BuildConfig.DEBUG) + HttpLoggingInterceptor.Level.BODY + else + HttpLoggingInterceptor.Level.NONE + redactHeader("Authorization") + redactHeader("Cookie") + } + ) .authenticator(tokenAuthenticator) // 토큰 갱신 (Interceptor 대신)
69-69: 하드코딩 baseUrl 외부화 제안환경별 전환(dev/stage/prod)을 위해 BuildConfig 또는 DI 설정으로 baseUrl 외부화를 권장합니다.
app/src/main/java/com/sampoom/android/core/datastore/CryptoManager.kt (3)
44-50: Base64 플래그 NO_WRAP 사용 권장 (개행 방지/일관성)Preferences에 저장 시 DEFAULT는 개행을 삽입할 수 있습니다. NO_WRAP으로 저장/복호화 일관성을 유지하세요.
- return Base64.encodeToString(iv + encrypted, Base64.DEFAULT) + return Base64.encodeToString(iv + encrypted, Base64.NO_WRAP)추가로 decrypt에서도 동일 플래그로 맞춰주세요(아래 코멘트 참조).
52-61: 복호화 입력 검증 + Base64 플래그 정합성최소 길이 검증 없이 IV(12바이트)를 슬라이싱합니다. 잘못된 입력 시 예외가 발생합니다.
- val encrypted = Base64.decode(encryptedText, Base64.DEFAULT) + val encrypted = Base64.decode(encryptedText, Base64.NO_WRAP) + require(encrypted.size >= 12) { "Invalid ciphertext" } val iv = encrypted.sliceArray(0..11) val ciphertext = encrypted.sliceArray(12 until encrypted.size)
27-41: 키 파라미터 강화(옵션): 256비트, StrongBox 우선가능한 환경에서 256비트 키와 StrongBox 백엔드를 우선 사용하면 보안성이 향상됩니다.
val keyGenParameterSpec = KeyGenParameterSpec.Builder( keyAlias, KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT ) .setBlockModes(KeyProperties.BLOCK_MODE_GCM) .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE) .setUserAuthenticationRequired(false) .setRandomizedEncryptionRequired(true) + .setKeySize(256) .build() keyGenerator.init(keyGenParameterSpec) keyGenerator.generateKey()StrongBox 사용은 기기별 가용성 체크 후
.setIsStrongBoxBacked(true)적용을 고려하세요.app/src/main/java/com/sampoom/android/core/datastore/AuthPreferences.kt (2)
56-84: 복호화 실패 예외 삼킴 — 진단 로그 추가 권장(민감정보 비노출)예외를 모두 null로 삼키면 원인 분석이 어려워집니다. 토큰/사용자 값은 로그에 출력하지 말고 에러 타입만 기록하세요.
- } catch (e: Exception) { - return null - } + } catch (e: Exception) { + android.util.Log.w("AuthPreferences", "Failed to decode stored user: ${e::class.simpleName}") + return null + }
86-103: 토큰 복호화 실패 예외 삼킴 — 최소한의 워닝 로그 권장동일 사유로 간단한 경고 로그를 남겨 추적성을 확보하세요.
- return try { - cryptoManager.decrypt(encrypted) - } catch (e: Exception) { - null - } + return try { + cryptoManager.decrypt(encrypted) + } catch (e: Exception) { + android.util.Log.w("AuthPreferences", "Failed to decrypt access token: ${e::class.simpleName}") + null + }- return try { - cryptoManager.decrypt(encrypted) - } catch (e: Exception) { - null - } + return try { + cryptoManager.decrypt(encrypted) + } catch (e: Exception) { + android.util.Log.w("AuthPreferences", "Failed to decrypt refresh token: ${e::class.simpleName}") + null + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/build.gradle.kts(1 hunks)app/src/main/java/com/sampoom/android/app/navigation/AppNavHost.kt(6 hunks)app/src/main/java/com/sampoom/android/core/datastore/AuthPreferences.kt(1 hunks)app/src/main/java/com/sampoom/android/core/datastore/CryptoManager.kt(1 hunks)app/src/main/java/com/sampoom/android/core/network/NetworkModule.kt(1 hunks)app/src/main/java/com/sampoom/android/core/network/TokenAuthenticator.kt(1 hunks)app/src/main/java/com/sampoom/android/core/network/TokenInterceptor.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/data/remote/api/AuthApi.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/data/repository/AuthRepositoryImpl.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/domain/usecase/ClearTokensUseCase.kt(1 hunks)app/src/main/java/com/sampoom/android/feature/user/ui/AuthViewModel.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/java/com/sampoom/android/core/network/TokenInterceptor.kt
- app/src/main/java/com/sampoom/android/feature/user/data/repository/AuthRepositoryImpl.kt
- app/src/main/java/com/sampoom/android/feature/user/data/remote/api/AuthApi.kt
🧰 Additional context used
🪛 detekt (1.23.8)
app/src/main/java/com/sampoom/android/core/datastore/AuthPreferences.kt
[warning] 80-80: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 90-90: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 99-99: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (3)
app/src/main/java/com/sampoom/android/feature/user/domain/usecase/ClearTokensUseCase.kt (1)
1-10: LGTM!토큰 삭제 유즈케이스가 올바르게 구현되었습니다. 의존성 주입과 suspend 함수 사용이 적절하며, 클린 아키텍처 패턴을 잘 따르고 있습니다.
app/src/main/java/com/sampoom/android/feature/user/ui/AuthViewModel.kt (1)
18-31: LGTM!ViewModel 구조가 잘 설계되었으며, 이전 리뷰에서 지적된
MutableSharedFlow버퍼 문제가extraBufferCapacity = 1로 적절히 해결되었습니다. 상태 관리와 이벤트 처리가 올바르게 구현되었습니다.app/src/main/java/com/sampoom/android/core/network/TokenAuthenticator.kt (1)
23-27: 무한 루프 우려는 해결됨 - TokenRefreshService가 별도 Retrofit 인스턴스 사용검증 결과, TokenRefreshService가 이미 별도의 Retrofit 인스턴스를 생성하고 있어 원래 우려했던 무한 재귀 호출 문제는 발생하지 않습니다. 리프레시 API 요청이 TokenAuthenticator/TokenInterceptor를 거치지 않으므로 안전합니다.
다만 TokenAuthenticator와 TokenInterceptor 모두에서
runBlocking사용 시 OkHttp 스레드 풀의 데드락 위험이 존재합니다. 특히 TokenInterceptor의authPreferences.getAccessToken()호출은 I/O 작업입니다. 향후 코루틴 기반으로 리팩토링하거나, 스레드 풀 크기 설정을 검토하시기 바랍니다.
5 participants
📝 Summary
🙏 Question & PR point
📬 Reference
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항