Skip to content

Conversation

@UiHyeon-Kim
Copy link
Member

@UiHyeon-Kim UiHyeon-Kim commented Jan 19, 2026

♟️ Issue

✨ 주요 변경 사항

  • 구글 및 카카오 로그인 연결
  • 카카오 디벨로퍼스에 앱 등록 후 앱 네이티브 키 사용 설정
  • 실 서버 구축 시 테스트 필요
  • 그 외 TODO로 할 것 작성

✅ 체크리스트

  • 🌱 merge 브랜치 확인
  • 🛠️ 빌드 성공
  • 💬 관련 이슈 연결 ([Closes | Fixes | Resolves] #123)

🔍 중점 리뷰 사항

📸 스크린샷

e.g. <img width="45%" src="링크" />

- Google, Kakao 로그인 라이브러리 추가
- `google-services` Gradle 플러그인 적용
- Access/Refresh 토큰 저장, 조회, 삭제 기능 추가
- `AuthToken` 데이터 클래스 추가 (access/refresh token)
- `User` 데이터 클래스 추가 (id, email, nickname, profileImgUrl)
- Hilt를 사용한 네트워크 관련 의존성 주입 모듈 구현
- 로그인 요청을 제외 모든 API 요청에 대해 Authorization 헤더에 AccessToken을 추가하는 AuthInterceptor를 구현
- `socialLogin` 함수의 임시 구현 추가
- `.gitkeep` 파일 삭제
- AuthRepositoryImpl을 AuthRepository 인터페이스에 바인딩
- SocialLoginUseCase: 소셜 로그인 기능
- SaveTokenUseCase: 인증 토큰 저장 기능
- GetTokenUseCase: 저장된 인증 토큰 조회 기능
- LogoutUseCase: 로그아웃 (토큰 삭제) 기능
- 소셜 로그인, 로그아웃, 토큰 확인 기능을 포함하는 `LoginViewModel` 추가
- 로그인 상태(Idle, Loading, Success, Error)를 나타내는 `LoginUiState` 정의
- `build.gradle.kts`에 `buildConfig` 플래그를 활성화하여 빌드 구성에 접근할 수 있도록 변경
- Kakao SDK 사용을 위해 `settings.gradle.kts`에 maven 리포지토리 추가
- `libs.versions.toml`에 Google Sign-In 관련 라이브러리 (credentials, googleid) 추가 및 버전 관리
- `presentation` 모듈의 `build.gradle.kts`에 Google Sign-In 라이브러리 의존성 추가
- Application 클래스에서 Kakao SDK 초기화 코드 추가
- 디버그 빌드에서 Timber 로깅 라이브러리 활성화
- presentation 모듈에 `social` 번들 등록
- `local.properties`에서 `GOOGLE_WEB_CLIENT_ID`를 읽어 BuildConfig에 추가
- 잘못된 자격 증명 유형 및 알 수 없는 오류에 대한 문자열 리소스를 추가
- 한국어와 기본(영어) 문자열을 모두 포함
- Credential Manager를 사용하여 Google 로그인 로직 구현
- 카카오 SDK를 사용하여 Kakao 로그인 로직 구현
- 각 소셜 로그인 결과를 처리하는 Manager 클래스 추가
- 구글, 카카오 로그인 버튼에 사용될 아이콘 리소스 추가
- Kakao Developers에서 권장하는 Kakao SDK, OkHttp, Retrofit2에 대한 ProGuard 규칙을 추가
- 코드 난독화 및 최적화 관련 설정 포함
- 추후 배포시 활성화 권장
- domain에서 data 레이어의 내용을 알 필요 없도록 프로바이더 구현
- 토큰 프로바이더 바인딩 등록
- `GOOGLE_WEB_CLIENT_ID`를 presentation에서 data 모듈로 이동
- data 모듈에 소셜 SDK 및 Timber 의존성 추가
- presentation 모듈에서 불필요해진 소셜 로그인 관련 코드 및 의존성 제거
- 실행을 위해 app 모듈 google service 플러그인 삭제
- `presentation` 레이어에 있던 GoogleLoginManager를 삭제하고, `data` 레이어에 GoogleAuthDataSource를 새로 생성하여 로그인 책임을 이동
- 기존 `GetGoogleIdOption`에서 `GetSignInWithGoogleOption`으로 API를 변경
- GoogleAuthDataSource를 이용한 구글 ID 토큰 요청 로직 추가
- `AuthRepository`에 `signInWithGoogle` 메서드 정의 및 구현
- 기존 `TokenDataStore` 관련 로직 제거
- RepositoryModule에 Hilt의 SingletonComponent를 사용하도록 설정
- NetworkModule에 임시 API BASE_URL 추가
- provideAuthInterceptor이 TokenProvider를 받도록 변경
- 구글 로그인을 위한 `GoogleLoginUseCase` 추가
- 토큰 관리를 `AuthRepository`에서 `TokenProvider`로 분리
- `GetTokenUseCase`, `SaveTokenUseCase`, `LogoutUseCase`가 `TokenProvider`를 사용하도록 수정
- `LoginUiState` 의 Success를 data class 로 변경하고, 신규 유저인지 판단하는 값 추가
- `LoginViewModel` 필요한 값만 매개변수로 받고 구글 로그인과 카카오 로그인 코드 작성
- 추후 컬러 모양 등 변경 필요
- `KakaoLoginManager`가 ActivityScoped를 가지도록 변경
- context를 생성자에서 받아 불변이고 로그인, 로그아웃 함수에서 내부 상태 변경이 없어 @stable 붙임
- `LoginScreen` Composable 함수 추가
- Google 및 Kakao 로그인 버튼 UI 및 로직 연동
- 로그인 성공 시 `onLoginSuccess` 콜백 호출
- 로그인 상태 관리를 위한 `LoginUiState` 및 `LoginViewModel` 연동
- Preview 구현
- 스플래시 화면 UI 및 로직 추가
- 앱 상태 확인 후 로그인 또는 홈 화면으로 분기 처리
- `GetTokenUseCase`를 사용하여 토큰 유무에 따라 화면 이동 결정
- .gitkeep 파일 삭제
- AuthService 인터페이스 및 관련 데이터 모델 추가
- `socialLogin`, `signInWithGoogle`, `refreshToken` API 함수 정의
- `LoginRequest`, `LoginResponse`, `RefreshRequest` 데이터 클래스 추가
- Mapper 작성
- isLoggedIn의 명확한 Flow 반환을 나타내기 위해 isLoggedInFlow로 메서드명 수정
- OkHttp Authenticator를 사용하여 토큰 재발급 기능 추가
- 토큰 갱신 API 호출 시에는 Auth Interceptor를 건너뛰도록 수정
- 재발급 로직 처리를 위해 `AuthService` 의존성 주입
순환 호출 문제로 런타임 에러 발생을 수정

- 토큰 재발급 로직이 Auth Interceptor를 타지 않도록 별도의 Retrofit 인스턴스를 사용하도록 수정
- 재발급 전용 `RefreshTokenApi` 인터페이스를 추가하고, `AuthService`에서 관련 로직을 제거
- `TokenAuthenticator`는 `RefreshTokenApi`를 사용하여 토큰 재발급을 수행하도록 변경
- 로그인 버튼 너비를 화면 전체로 확장하고 모서리를 둥글게 처리
- 화면 전체에 수평 패딩(16dp) 추가
- `KakaoLoginManager` 인스턴스를 Composable 내에서 생성하도록 변경
- 로그인 에러 발생 시 에러 상태를 초기화하는 로직 추가
 into feature/auth

# Conflicts:
#	presentation/build.gradle.kts
#	presentation/src/main/java/com/smtm/pickle/presentation/login/LoginScreen.kt
#	presentation/src/main/java/com/smtm/pickle/presentation/login/LoginViewModel.kt
#	presentation/src/main/java/com/smtm/pickle/presentation/splash/SplashScreen.kt
#	presentation/src/main/java/com/smtm/pickle/presentation/splash/SplashViewModel.kt
#	presentation/src/main/res/values/strings.xml
- 기본 `strings.xml` 파일의 bottom navigation 메뉴명을 영문으로 변경
- 한국어 메뉴명은 `values-ko/strings.xml` 파일로 분리
- `invaild_credential_type`의 오타를 `invalid_credential_type`으로 수정
- `ButtonSection`의 카카오 및 구글 로그인 버튼 디자인을 피그마 시안에 맞게 수정
- `LoginScreen`의 컴포넌트 간 간격 조정을 위해 `Spacer`의 `weight` 값을 변경
- 로그인 버튼 구현 후 삭제될 임시 로그인 버튼에 주석 추가
- 기존의 `SocialLoginUseCase`를 `KakaoLoginUseCase`로 변경하고, `GoogleLoginUseCase`를 분리하여 각 소셜 로그인 로직을 명확하게 구분했습니다.
- 소셜 로그인 API 연동을 통해 실제 서버와 통신하도록 구현하고, 기존의 목 데이터를 제거했습니다.
- `loginWithGoogle` 함수가 내부적으로 `socialLogin`을 호출하도록 로직을 통합했습니다.
- `LoginRequest` 데이터 클래스의 필드에 대한 주석을 추가하여 각 필드의 용도를 명확히 했습니다.
- `LoginScreen`에서 로그인 성공 후 신규 및 기존 유저를 분기 처리하기 위한 TODO 주석을 추가했습니다.
- `LoginUiState.Success` 상태일 때, 신규 사용자와 기존 사용자를 구분하여 화면을 이동시키는 로직을 구현
- 임시 로그인 버튼 관련 `TODO` 주석의 내용을 '로그인 구현 후'에서 '로그인 서버 연결 후'로 명확하게 수정
@UiHyeon-Kim UiHyeon-Kim self-assigned this Jan 19, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 19, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link

Summary of Changes

Hello @UiHyeon-Kim, 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!

이 Pull Request는 앱에 소셜 로그인 기능을 통합하여 사용자 인증 경험을 개선하는 데 중점을 둡니다. 구글 및 카카오 로그인을 지원하기 위한 포괄적인 인증 시스템 아키텍처를 구축했으며, 이는 토큰 관리, API 통신, 그리고 UI 통합을 포함합니다. 이를 통해 앱의 확장성과 보안성을 확보하고, 향후 사용자 관리 기능 개발의 기반을 마련했습니다.

Highlights

  • 소셜 로그인 기능 구현: 구글 및 카카오 소셜 로그인 기능을 구현하여 사용자가 다양한 방법으로 앱에 접근할 수 있도록 했습니다. 이는 인증 토큰 관리, API 연동, UI 컴포넌트 추가 등을 포함합니다.
  • 인증 시스템 아키텍처 구축: Hilt를 사용한 의존성 주입, DataStore를 활용한 로컬 토큰 저장, OkHttp Interceptor 및 Authenticator를 통한 토큰 갱신 로직 등 견고한 인증 시스템 아키텍처를 설계하고 구현했습니다.
  • 모듈 간 역할 분리 및 도메인 계층 정의: 데이터, 도메인, 프리젠테이션 모듈 간의 명확한 역할 분리를 통해 코드의 응집도를 높이고 유지보수성을 향상시켰습니다. 특히 도메인 계층에 인증 관련 모델(AuthToken, SocialLoginType, User)과 유스케이스를 정의했습니다.
  • 빌드 및 의존성 관리 업데이트: Gradle 빌드 스크립트를 업데이트하여 BuildConfig 필드를 추가하고, Kakao SDK, Google Identity SDK 등 소셜 로그인 관련 라이브러리 의존성을 libs.versions.toml을 통해 관리하도록 설정했습니다.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

이 PR은 구글 및 카카오 소셜 로그인 기능을 구현하고 관련 데이터, 도메인, 프레젠테이션 레이어를 구성합니다. 전반적으로 클린 아키텍처를 잘 따르고 있으며, Hilt를 사용한 의존성 주입과 StateFlow를 이용한 UI 상태 관리 등 좋은 패턴을 사용하고 있습니다. 다만, 몇 가지 치명적인 버그와 개선이 필요한 부분들이 있습니다. 카카오 SDK 초기화 시 API 키가 문자열로 잘못 전달되어 기능이 동작하지 않을 수 있으며, Result API의 잘못된 사용으로 앱이 비정상 종료될 수 있는 문제가 있습니다. 또한, runBlocking의 사용, Hilt 의존성 주입 문제, 코드 중복 등 개선이 필요한 부분에 대한 피드백을 남겼습니다. 이 부분들을 수정하면 더 안정적이고 유지보수하기 좋은 코드가 될 것입니다.

return@Interceptor chain.proceed(originalRequest)
}

val accessToken = runBlocking { tokenProvider.getAccessToken() }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

runBlocking을 사용하여 인터셉터 내에서 동기적으로 토큰을 가져오고 있습니다. OkHttp 인터셉터는 백그라운드 스레드에서 실행되므로 당장 문제가 발생하지 않을 수 있지만, runBlocking은 현재 스레드를 차단하므로 네트워킹 성능에 영향을 줄 수 있습니다. 이 패턴은 코루틴과 동기적인 OkHttp API를 함께 사용할 때 흔히 사용되지만, 잠재적인 성능 문제를 인지하고 있어야 합니다.

viewModel: LoginViewModel = hiltViewModel()
) {
val context = LocalContext.current
val kakaoLoginManager = remember(context) { KakaoLoginManager(context) }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

KakaoLoginManager@ActivityScoped로 Hilt 의존성으로 관리되도록 설계되었지만, remember { KakaoLoginManager(context) }를 통해 수동으로 인스턴스화하고 있습니다. 이는 Hilt의 의존성 주입 및 스코프 관리 기능을 우회하는 것으로, 예기치 않은 동작을 유발할 수 있습니다. KakaoLoginManagerActivityContext를 필요로 하므로, Activity 레벨에서 Hilt를 통해 주입받은 후 Composable 함수로 전달하는 방식을 고려해 보세요.

@HiltViewModel
class SplashViewModel @Inject constructor(

private val getTokenUseCase: GetTokenUseCase,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

GetTokenUseCaseSplashViewModel에 주입되었지만, 실제로는 사용되지 않고 있습니다. 스플래시 화면에서 기존 로그인 세션을 확인하는 로직을 추가할 계획이 아니라면, 불필요한 의존성은 제거하는 것이 좋습니다.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다른 브랜치 파서 화면 작성 후 사용 예정

@UiHyeon-Kim
Copy link
Member Author

@coderabbitai .coderabbit.yaml 파일이 잘 적용 됐는지 리뷰 부탁해도 될까?

@coderabbitai
Copy link

coderabbitai bot commented Jan 20, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@UiHyeon-Kim 네, .coderabbit.yaml 파일을 확인해드리겠습니다!


🏁 Script executed:

#!/bin/bash
# Read the .coderabbit.yaml file to check its contents
cat .coderabbit.yaml

Length of output: 48


🛠 Debug Information Gathered


@UiHyeon-Kim

리포지토리에 .coderabbit.yaml 파일이 존재하지 않습니다. 현재 CodeRabbit은 기본 설정으로 동작하고 있습니다.

현재 적용된 주요 설정

수집된 설정 정보를 확인한 결과, 다음과 같이 기본 설정이 적용되어 있습니다:

  • 리뷰 언어: en-US (영어)
  • 리뷰 프로필: chill (표준)
  • 자동 리뷰: 활성화
  • 증분 리뷰: 활성화
  • 시퀀스 다이어그램: 활성화
  • 코드 리뷰 노력도 예측: 활성화
  • 각종 린터 도구들: 모두 활성화 (ESLint, Biome, detekt 등)

.coderabbit.yaml 파일 생성 방법

만약 커스텀 설정을 원하시면, 리포지토리 루트에 .coderabbit.yaml 파일을 생성하시면 됩니다. 예를 들어, 한국어로 리뷰를 받고 싶으시다면:

language: ko-KR
reviews:
  profile: chill  # 또는 assertive (더 상세한 피드백)
  auto_review:
    enabled: true
    drafts: false  # 드래프트 PR 리뷰 안함

더 많은 설정 옵션은 CodeRabbit 문서를 참고하시거나, 특정 설정이 필요하시면 말씀해주세요!

@UiHyeon-Kim
Copy link
Member Author

@coderabbitai 다시 확인해줄래?

@coderabbitai
Copy link

coderabbitai bot commented Jan 20, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@UiHyeon-Kim 네, 다시 확인해드리겠습니다!


🏁 Script executed:

#!/bin/bash
# Check if .coderabbit.yaml exists and read its contents
if [ -f .coderabbit.yaml ]; then
    echo "=== .coderabbit.yaml file found ==="
    cat .coderabbit.yaml
elif [ -f .coderabbit.yml ]; then
    echo "=== .coderabbit.yml file found ==="
    cat .coderabbit.yml
else
    echo "=== No .coderabbit.yaml or .coderabbit.yml file found ==="
fi

# Also check if there's a file in .github directory
if [ -f .github/coderabbit.yaml ]; then
    echo "=== .github/coderabbit.yaml file found ==="
    cat .github/coderabbit.yaml
elif [ -f .github/coderabbit.yml ]; then
    echo "=== .github/coderabbit.yml file found ==="
    cat .github/coderabbit.yml
fi

Length of output: 57


@UiHyeon-Kim

확인 결과, 여전히 .coderabbit.yaml 파일이 리포지토리에 없습니다.

확인 사항

다음 위치들을 모두 확인했지만 파일이 발견되지 않았습니다:

  • .coderabbit.yaml
  • .coderabbit.yml
  • .github/coderabbit.yaml
  • .github/coderabbit.yml

가능한 원인

  1. 파일을 로컬에만 생성하고 아직 푸시하지 않으신 경우

    • 파일을 생성하셨다면 git add .coderabbit.yamlgit push로 푸시해주세요
  2. 파일을 아직 생성하지 않으신 경우

    • 한국어 리뷰를 원하시면 아래 설정으로 파일을 생성하시면 됩니다:
language: ko-KR
reviews:
  profile: chill
  auto_review:
    enabled: true
    drafts: false
  path_filters:
    - "!**/build/**"
    - "!**/generated/**"

파일 생성 후 커밋하여 푸시하시면, 다음 리뷰부터 설정이 적용됩니다!

혹시 특정 설정을 원하시거나 도움이 필요하시면 말씀해주세요 😊

- `build.gradle.kts`에서 `local.properties`의 카카오 네이티브 앱 키를 읽어와 `manifestPlaceholders`와 `buildConfigField`에 설정
- `AndroidManifest.xml`에 인터넷 권한을 추가하고, 카카오 로그인을 위한 `AuthCodeHandlerActivity`와 리다이렉트 URI 스킴(scheme)을 설정
- `PickleApplication.kt`에서 카카오 SDK 초기화 시 `BuildConfig`로부터 네이티브 앱 키를 동적으로 가져오도록 수정
Copy link
Collaborator

@JuhyeokLee97 JuhyeokLee97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

충분히 사고하고 개발한 게 느껴지는 PR 이었습니다! 👍
수고하셨습니다!
Gemini가 남겨준 코멘트들도 확인해보시면 좋을 것 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

한국어랑 영어를 구분한 이유가 있으실까요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

한국어와 영어를 구분했다기보다 둘 다 고려했다고 보는 게 맞을 것 같습니다!

values 와 values-ko 를 분리하면 기기를 한국어로 쓰는 사용자에겐 지정한 텍스트가 한글로 보이게 되고, 기기를 영어로 쓰는 사용자에겐 영어로 보이게 됩니다!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

영어와 한국어를 구분하는 것은 저희 방향성과 다른 것 같습니다.
이후에 디자인 작업이 들어갈 때마다 문자열 리소스에 대한 영어 정의가 되어야 해서 업무 범위가 커지므로 현재에는 영어 지원을 고려하지 않아도 될 것 같습니다.

Comment on lines +123 to +125
# Firebase
google-gms-google-services = { id = "com.google.gms.google-services", version.ref = "googleGmsPlugin" }

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 플러그인은 사용하는 곳이 없는데 추가된 이유가 있을까요??

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

초기에 추가했던 것으로 기억하는데, 다른 플러그인이랑 헷갈려서 넣었던 것으로 기억합니다.
추후 FCM 알림을 쓸 때 필요할 것 같아 놔뒀는데 지금은 지워두는 게 나을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 작업과는 관련이 없기 때문에 히스토리 파악을 위해서 제거하는 것이 좋을 것 같습니다.

Comment on lines +13 to +19
val localProperties = Properties().apply {
val localPropertiesFile = rootProject.file("local.properties")
if (localPropertiesFile.exists()) {
load(localPropertiesFile.inputStream())
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

local.properties 에 추가된 내용 슬랙으로 공유 부탁드립니다.

Comment on lines +2 to +3
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="com.smtm.pickle">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

패키지를 Manifest에 명시하신 이유가 있을까요?

구글 Manifest 공식 문서에서는 AGP 7.3 이상 부터는 Manifest에 명시하지 않고 build.gradle 명시하라고 권장하고 있습니다.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

카카오 디벨로퍼스를 참고해서 작성했는데, 해당 내용은 방금 알게 됐네요! 지워두겠습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

변경해주시면 작업된 커밋 링크 주시면 감사하겠습니다!

Comment on lines +75 to +76
// Social SDK
implementation(libs.bundles.social)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앱 모듈에서 social bundle에 있는 것들을 모두 사용할 예정인가요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

구글 로그인을 data로 빼면서 이 부분 수정을 못했네요 수정하겠습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

token 정보를 메모리에 올려서 사용하면 외부에서 token 정보를 확인할 때 runBlocking을 사용하지 않을 수 있을 것 같습니다.
네트워크 통신 시, 백그라운드에서 동작하지만 runBlocking 사용을 지양하는 것이 좋을 것 같습니다.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

캐시 적용 완료했습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TokenManager 보다 TokenProviderImpl에서 캐싱된 토큰을 관리하는 것이 좋을 것 같습니다.
캐싱된 토큰을 읽고 쓰는 책임은 TokenProvider가 갖는 것이 바람직한 것 같아요.

그리고 TokenManager라는 이름은 책임이 모호해서 TokenRefresher로 변경해서 책임을 명확하게 하는 것이 좋을 것 같습니다.

또한 TokenProvider에서 읽고 쓰는 작업이 있으니 TokenRepository로 네이밍을 변경해도 괜찮을 것 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동시성 문제와 코드 가독성을 고려해서 코드 제안드립니다.
동시성 문제는 API가 2개 이상 호출된 경우 비동기적으로 Refresh가 여러번 발생하여 갱신 API 또한 2번 이상 호출될 수 있습니다.
그래서 갱신 API는 1회만 호출하도록 하면 효율적일 것 같습니다.

코드 가독성은 매직 넘버를 상수화하고 갱신하는 로직을 별도 함수로 빼봤습니다.

import com.smtm.pickle.data.source.remote.api.RefreshTokenApi
import com.smtm.pickle.data.source.remote.model.LoginResponse
import com.smtm.pickle.domain.model.auth.AuthToken
import com.smtm.pickle.domain.provider.TokenProvider
import kotlinx.coroutines.runBlocking
import okhttp3.Authenticator
import okhttp3.Request
import okhttp3.Response
import okhttp3.Route
import javax.inject.Inject
import javax.inject.Singleton

@Singleton
class TokenAuthenticator @Inject constructor(
    private val tokenProvider: TokenProvider,
    private val refreshApi: RefreshTokenApi,
) : Authenticator {

    private val lock = Any()

    override fun authenticate(route: Route?, response: Response): Request? {
        if (responseCount(response) >= MAX_RETRY_COUNT) return null

        synchronized(lock) {
            val currentToken = runBlocking { tokenProvider.getAccessToken() }
            val requestToken = response.request.header("Authorization")?.removePrefix("Bearer ")

            if (currentToken != null && currentToken != requestToken) {
                return response.request.newBuilder()
                    .header("Authorization", "Bearer $currentToken")
                    .build()
            }

            val newToken = runBlocking { refreshToken() } ?: return null

            return response.request.newBuilder()
                .header("Authorization", "Bearer ${newToken.accessToken}")
                .build()
        }
    }

    private suspend fun refreshToken(): LoginResponse? {
        val refreshToken = tokenProvider.getRefreshToken() ?: return null

        val newToken = runCatching {
            refreshApi.refreshToken("Bearer $refreshToken")
        }.getOrNull()

        if (newToken == null) {
            tokenProvider.clearToken()
            return null
        }

        tokenProvider.saveToken(
            AuthToken(
                access = newToken.accessToken,
                refresh = newToken.refreshToken
            )
        )

        return newToken
    }

    // 순환 참조 방지
    private fun responseCount(response: Response): Int {
        var count = 0
        var prior = response.priorResponse

        while (prior != null) {
            count++
            prior = prior.priorResponse
        }

        return count
    }

    companion object {
        private const val MAX_RETRY_COUNT = 1
    }
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 학습하면서 개선중입니다!
변경 후 한 번 더 코드리뷰 부탁드리겠습니다 ㅎㅎ

Comment on lines 17 to 18
private val socialLoginUseCase: KakaoLoginUseCase,
private val googleLoginUseCase: GoogleLoginUseCase,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

socialLoginUseCase 변수명도 googleLoginUseCase처럼 kakaoLoginUseCase로 변경하면 좋을 것 같습니다.
UseCase를 사용하면 좋은 점 중 하나는 파라미터만 확인하더라도 대략적으로 클래스의 동작을 알 수 있는 것인데, 소셜 로그인과 구글 로그인 유즈케이스가 있어서 혼란을 줄 수 있을 것 같습니다.

- `socialLoginUseCase`를 `kakaoLoginUseCase`로 명칭을 변경하여 역할을 명확히 함.
- `loginWithGoogle` 함수에서 `map` 대신 `mapCatching`을 사용하여 예외 처리 개선.
- `TokenProvider`에서 `getRefreshToken`이 `DataStore`의 `getRefreshToken`을 직접 호출하도록 수정
- 불필요해진 `RefreshRequest.kt` 데이터 클래스 삭제.
- `TokenDataStore`의 토큰 조회 로직을 `getAuthToken` 확장 함수로 추출하여 중복을 제거
- `TokenManager`를 도입하여 토큰 갱신 로직을 중앙에서 관리하도록 변경했습니다. 이는 여러 API 요청이 동시에 토큰 갱신을 시도할 때 발생할 수 있는 동시성 문제를 `Mutex`를 사용하여 해결합니다.
- `TokenAuthenticator`는 `TokenManager`를 통해 토큰을 갱신하도록 로직을 단순화하고, 기존의 자체 토큰 갱신 및 순환 참조 방지 코드를 제거했습니다.
- Access Token의 만료 여부를 클라이언트에서 직접 확인할 수 있도록 JWT payload를 파싱하는 `isExpired()` 확장 함수를 추가했습니다.
- `TokenManager`는 갱신 요청이 진행 중일 때 다른 요청들이 대기하도록 하여, 불필요한 중복 갱신 호출을 방지합니다.
- `TokenManager`를 도입하여 토큰 상태를 메모리에 캐싱하고 관리하는 로직 구현
- 앱 시작 시 `initTokenUseCase`를 통해 `TokenManager`가 토큰을 미리 로드하도록 `SplashViewModel`의 로직 수정
- `TokenManager` 내부의 토큰 갱신 로직을 캐시된 토큰을 우선적으로 사용하도록 변경하고, 토큰 갱신 실패 또는 만료 시 캐시를 비우도록 수정
- 기존의 `GetTokenUseCase`를 `GetAccessTokenUseCase`와 `InitTokenUseCase`로 분리하여 역할을 명확히 함
- `TokenProvider`, `TokenDataStore`에서 더 이상 사용되지 않는 `getAccessToken()` 함수를 제거
- `IsLoggedInUseCase`으로 스플래시 로그인 여부 파악하도록 구현
Comment on lines +80 to +86
try {
job.await()
} finally {
refreshLock.withLock {
if (refreshJob == job) refreshJob = null
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

finally에서 withLock을 사용하고 있는데 상위 스코프에서 동일한 Mutex의 withLock을 사용하고 있어서 데드락이 발생할 것 같습니다.
확인 부탁드립니다.

Comment on lines +51 to +53
// 이미 refresh 중이면 대기
refreshJob?.let { return it.await() }

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refreshJob을 만든 이유가 무엇일까요?
비동기적으로 처리하기 위함일까요?

private val tokenProvider: TokenProvider,
private val refreshApi: RefreshTokenApi,
) {
private var cachedToken: AuthToken? = null
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

모든 스레드에서 가시성을 보장하기 위해서 @Volatile 어노테이션을 쓰면 좋을 것 같습니다.

Comment on lines +46 to +49
// Access 코드를 이미 획득했는지 판단
cachedToken
?.takeIf { it.access.isNotBlank() && !it.access.isExpired() }
?.let { return it }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

access.isExpired()로 클라이언트에서도 토큰 만료를 확인하고 있는데요.
refreshTokenIfNeeded()가 호출되는 경우는 서버에서 401 응답을 내려줄 때인데 클라이언트에서도 추가로 확인하는 이유가 무엇일까요?
또한 서버 시간과 클라이언트 시스템 시간의 차이로 발생하는 문제는 없을까요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants