-
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
feat: 소셜로그인 및 토큰 관리 로직 구현 #421
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.
삼성티엠
val isLogin: LiveData<Boolean> get() = _isLogin | ||
|
||
fun setIsLogin(isLogin: Boolean) { | ||
this._isLogin.value = isLogin |
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.
c: 이 this 없어도 되는거 아닌가요?
@@ -0,0 +1,36 @@ | |||
package com.teamwss.websoso.ui.splash | |||
|
|||
import android.util.Log |
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.
c: 이거 쓰고계신가용?
viewModelScope.launch { | ||
runCatching { | ||
val withdrawReason = when (withdrawReason.value) { | ||
ETC_INPUT_REASON -> withdrawEtcReason.value | ||
else -> withdrawReason.value | ||
} | ||
// TODO: API 개발 완료 시 reason을 사용하여 회원 탈퇴 진행 | ||
}?:"" |
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.
r: 포맷팅
@Provides | ||
@Singleton | ||
@Secured | ||
fun provideSecuredOkHttpClient( | ||
@Log logInterceptor: Interceptor, | ||
@Auth authInterceptor: Interceptor, | ||
websosoAuthenticator: WebsosoAuthenticator | ||
): OkHttpClient = OkHttpClient.Builder() | ||
.addInterceptor(logInterceptor) | ||
.addInterceptor(authInterceptor) | ||
.authenticator(websosoAuthenticator) | ||
.connectTimeout(60, TimeUnit.SECONDS) | ||
.readTimeout(30, TimeUnit.SECONDS) | ||
.writeTimeout(15, TimeUnit.SECONDS) | ||
.build() | ||
|
||
@Provides | ||
@Singleton | ||
@Unsecured | ||
fun provideUnsecuredOkHttpClient( | ||
@Log logInterceptor: Interceptor | ||
): OkHttpClient = OkHttpClient.Builder() | ||
.addInterceptor(logInterceptor) | ||
.connectTimeout(60, TimeUnit.SECONDS) | ||
.readTimeout(30, TimeUnit.SECONDS) | ||
.writeTimeout(15, TimeUnit.SECONDS) | ||
.build() |
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.
@Provides | |
@Singleton | |
@Secured | |
fun provideSecuredOkHttpClient( | |
@Log logInterceptor: Interceptor, | |
@Auth authInterceptor: Interceptor, | |
websosoAuthenticator: WebsosoAuthenticator | |
): OkHttpClient = OkHttpClient.Builder() | |
.addInterceptor(logInterceptor) | |
.addInterceptor(authInterceptor) | |
.authenticator(websosoAuthenticator) | |
.connectTimeout(60, TimeUnit.SECONDS) | |
.readTimeout(30, TimeUnit.SECONDS) | |
.writeTimeout(15, TimeUnit.SECONDS) | |
.build() | |
@Provides | |
@Singleton | |
@Unsecured | |
fun provideUnsecuredOkHttpClient( | |
@Log logInterceptor: Interceptor | |
): OkHttpClient = OkHttpClient.Builder() | |
.addInterceptor(logInterceptor) | |
.connectTimeout(60, TimeUnit.SECONDS) | |
.readTimeout(30, TimeUnit.SECONDS) | |
.writeTimeout(15, TimeUnit.SECONDS) | |
.build() | |
@Provides | |
@Singleton | |
@Secured | |
fun provideSecuredOkHttpClient( | |
@Log logInterceptor: Interceptor, | |
@Auth authInterceptor: Interceptor, | |
websosoAuthenticator: WebsosoAuthenticator, | |
): OkHttpClient = createOkHttpClientBuilder(logInterceptor) | |
.addInterceptor(authInterceptor) | |
.authenticator(websosoAuthenticator) | |
.build() | |
@Provides | |
@Singleton | |
@Unsecured | |
fun provideUnsecuredOkHttpClient( | |
@Log logInterceptor: Interceptor, | |
): OkHttpClient = createOkHttpClientBuilder(logInterceptor).build() | |
private fun createOkHttpClientBuilder(logInterceptor: Interceptor): OkHttpClient.Builder { | |
return OkHttpClient.Builder() | |
.addInterceptor(logInterceptor) | |
.connectTimeout(60, TimeUnit.SECONDS) | |
.readTimeout(30, TimeUnit.SECONDS) | |
.writeTimeout(15, TimeUnit.SECONDS) | |
} |
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.
개인적으로 데이터스토어는 UDF를 구축하기에 아주 좋은 저장소입니다. 그래서 suspend 함수들만 지원한다고 생각해유
suspend함수는 리턴값으로 이용하기 적절치 않으니, 구독-발행 패턴이 좀 더 적절해보입니다.
단순히 키값을 사용하는 용도로는 쉐어드프리퍼런스도 나쁘지 않다고 생각함다.
뷰에서 로직은 최대한 덜어내고 카카오sdk 사용은 뷰모델에서 콜백으로 소통해도 좋을 것 같아요
|
||
val newAccessToken = runCatching { | ||
runBlocking { | ||
authRepository.reissueToken() |
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.
r: api통신이 5초가 걸린다면 .. ?
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.
이거 knownIssue로 남겨두겠습니다. #425
annotation class Secured | ||
|
||
@Qualifier | ||
@Retention(AnnotationRetention.BINARY) |
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.
a: 왜 바이너리로 설정했나요
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.
알아보니 BINARY로 설정하면 컴파일된 바이트코드에 어노테이션 정보가 남지만 런타임에는 사용할 수 없다고합니다.
런타임에 어노테이션 정보를 필요로 하지 않을 때 사용합니다
} | ||
|
||
suspend fun logout() { | ||
runCatching { |
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.
c: 런캐칭도 한곳에서 하면 좋을듯!
startActivity(OnboardingActivity.getIntent(this)) | ||
lifecycleScope.launch { | ||
runCatching { | ||
kakaoAuthService.login() |
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.
c: 뷰모델의 콜백을 로그인에게 주는것은 어떠한지요?
그럼 콜백을 통해 뷰모델 프로퍼티를 업데이트하고, 그것을 관찰하는 옵저버가 아래 로직을 실행하는 ,,
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.
아 일단은 현상을 유지하고 더 좋은 로직이 생각나면 변경하겠습니다!
@@ -1,21 +1,57 @@ | |||
package com.teamwss.websoso.ui.login | |||
|
|||
import android.util.Log |
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.
r: 로그 컷
fun setIsLogin(isLogin: Boolean) { | ||
this._isLogin.value = isLogin | ||
} | ||
|
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.
r: 아하 여기 있었군요. 파라미터로 넘겨주지말고 인텐트자체를 savedStateHandler로 받아보시죵
val autoLoginJob = launch { splashViewModel.autoLogin() } | ||
val delayJob = launch { delay(1000L) } | ||
|
||
joinAll(autoLoginJob, delayJob) |
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.
r: 뷰모델 함수가 어차피 새로운 코루틴을 만들고 있어서 여기서 굳이 코루틴을 만들 필요가 없을 것 같아용
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.
카카오 로그인 구현을 처음봐서 질문이 많네요 😅
플로우로 미리 보여주셔서 이해가 쉬웠어요 고생하셨습니다 👍
@Singleton | ||
class WebsosoAuthenticator @Inject constructor( | ||
private val authRepository: AuthRepository, | ||
@ApplicationContext private val context: Context, |
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.
c: 이 Authenticator에서 context를 사용하지 않고 구현할 수 있는 다른 방법은 없나요 ? android 의존성을 데이터가 가지고 있으니 어색한 것 같아요
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.
401이 발생 -> 토큰 재발급 -> 재발급 실패시 로그인으로 돌아가는 기능 때문에 필요한건데 콜백을 만들어서 대응하려고 시도해봤는데 어렵더라구요 어떤 방법이 있을까요? 포포리 같은 경우도 그냥 의존성을 받고 있긴한데
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.
어렵네요 ... ^..^ 그냥 현행유지 하시죠
.header("Content-Type", CONTENT_TYPE) | ||
val request = requestBuilder.build() | ||
chain.proceed(request) | ||
@Log |
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.
a: 요 Log 어노테이션은 어떤 역할인지 궁금해요!
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.
logging이란 이름도 괜찮을것같아요
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.
Log.d
랑 겹쳐서 logging으로 수정하는 것도 좋아보이네요
fun provideSecuredOkHttpClient( | ||
@Log logInterceptor: Interceptor, | ||
@Auth authInterceptor: Interceptor, | ||
websosoAuthenticator: WebsosoAuthenticator |
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.
r: 트레일링 콤마
@Singleton | ||
@Unsecured | ||
fun provideUnsecuredOkHttpClient( | ||
@Log logInterceptor: Interceptor |
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.
r: 여기두 트콤
@Module | ||
@InstallIn(ActivityComponent::class) | ||
interface Binder { | ||
@Binds | ||
fun bindKakaoAuthService(service: KakaoAuthService): OAuthService | ||
} |
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.
a: 바인드 메서드를 인스턴스 클래스로 묶으신 이유가 무엇인지 궁금합니다
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.
OAuthService의 구현체인 KakaoAuthService 바인드하기 위해서는 저렇게 작성하는 것말고 다르게 작성하는 방법이 있나요?
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.
진짜 몰라서 여쭤봤습니다 확인이요
private var accessToken: String = "" | ||
private var refreshToken: String = "" | ||
|
||
fun setTokens(accessToken: String, refreshToken: String) { |
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.
c: 우리 뷰모델에서 사용하는 건 updateXXX
로 쓰기로 했어욥
splashViewModel.autoLogin() | ||
} | ||
|
||
private fun setObserver() { |
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.
r:
private fun setObserver() { | |
private fun setupObserver() { |
) : ViewModel() { | ||
|
||
private var _isAutoLoginSuccess = MutableLiveData(false) | ||
val isAutoLoginSuccess: LiveData<Boolean> = _isAutoLoginSuccess |
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.
c:
val isAutoLoginSuccess: LiveData<Boolean> = _isAutoLoginSuccess | |
val isAutoLoginSuccess: LiveData<Boolean> get() = _isAutoLoginSuccess |
private var _isAutoLoginSuccess = MutableLiveData(false) | ||
val isAutoLoginSuccess: LiveData<Boolean> = _isAutoLoginSuccess | ||
|
||
fun autoLogin() { |
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.
c: 얘도 자동로그인 확인해서 값을 업데이트 하는 거니까 updateXXX로 사용하면 좋을 것 같아요
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.
자동로그인을 실행하는 함수라고 생각하여 지금 네이밍을 지었고
updateXXX를 사용하려면 updateIsAutoLoginSuccess
라는 이름을 가질 수 있겠는데 호감이 가는 이름은 아닌 것 같군요
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.
a: 굳이 Success인것까지 밝힐 필요는 없어요! 타입이 Boolean하니까요
@@ -48,5 +48,16 @@ | |||
app:layout_constraintBottom_toBottomOf="parent" | |||
app:menu="@menu/menu_main_bnv" | |||
tools:menu="@menu/menu_main_bnv" /> | |||
|
|||
<View | |||
android:id="@+id/v_main_guest" |
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.
r: 뷰는 view_xxx_
가 컨벤션!
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.
고생하셨씀다~!
@@ -60,7 +60,7 @@ object SecureStorageModule { | |||
fileName, | |||
masterKey, | |||
EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV, | |||
EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM | |||
EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM, |
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.
a: EncryptedSharedPreferences는 안드로이드에서 deprecated된것으로 알고 있어요. 잔버그가 많은친구라.
관련해서 직접 암호화모듈을 만드는것도 좋아보여요 :) 노운이슈로 남겨두시고 담당자는 추후에 논의해보죠
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.
#427 deprecated된건 모르던 사실이군요 이슈를 파서 담당자가 대응할 수 있도록 조치하겠습니다.
data object Loading : LoginUiState() | ||
data class Success( | ||
val isRegistered: Boolean, | ||
val accessToken: String, | ||
val refreshToken: String, | ||
) : LoginUiState() | ||
|
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.
r: 토큰이 UiState에 담길일이 있을까요? UI상태값은 아닌것 같아요
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.
인정합니다!
} | ||
var refreshToken: String | ||
get() = savedStateHandle[REFRESH_TOKEN_KEY] ?: "" | ||
set(value) { savedStateHandle[REFRESH_TOKEN_KEY] = value } |
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.
c: 커스텀 세터앞에도 가시성제어자가 붙나요
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.
객체만이 본인의 상태를 업데이트 할 수 있도록 수정하였습니다.
private var _isAutoLoginSuccess = MutableLiveData(false) | ||
val isAutoLoginSuccess: LiveData<Boolean> = _isAutoLoginSuccess | ||
|
||
fun autoLogin() { |
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.
a: 굳이 Success인것까지 밝힐 필요는 없어요! 타입이 Boolean하니까요
📌𝘐𝘴𝘴𝘶𝘦𝘴
📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯
먼저 PR단위가 커지게 된 것에 대해 사과의 말씀 드립니다 (솔직히 소셜로그인은 인정아닌가?)
본격적인 코드리뷰 이전 2가지 권장 사항
비회원 유저 로직 구현
MainActivity
로Intent
를 통해서isLogin
을false
으로 넘겨준다 (디폴트는 true)MainActivity
전체를 덮고 있는v_main_guest
뷰를 Visible 시켜 어떤 화면을 터치해도 로그인 유도 모달이 나오도록 처리회원 로직 구현
카카오 sdk를 통해
OAuth AccessToken
을 받는다. 서버 로그인/회원가입 api에OAuth AccessToken
을 주어JWT AccessToken과 refreshToken
을 받는다.카카오 로그인시 가입된 유저라면? ->
JWT AccessToken과 refreshToken
를 로컬에 저장하고 자동로그인 변수 또한 true로 변경해준다.카카오 로그인시 가입되지 않은 유저라면 ->
JWT AccessToken과 refreshToken
를OnboardingActivity
를Intent
로 넘겨 최종 회원가입에 성공한다면 로컬에 토큰을 저장한다.Autenticator
response.code == 401
를 감지한다AccessToken
이 만료되었거나 혹은 유효하지 않은 토큰이라는 뜻이기 때문에refreshToken
을 통해 재발급을 시도한다.LoginActivity
로 쫒아내고 토큰을 모두 날려버린다.@secured 와 @unsecured의 차이
AuthApi
에는 헤더Authorization
가 없이 작동하는 api들이 있기 때문에@Unsecured
입니다. (당연히 처음 카카오톡 소셜로그인으로 가입하면 헤더 토큰이 없겠죠?)AuthApi
에 들어있지만 헤더 토큰이 필요한 친구들은@Header
를 통해서 헤더 토큰을 넣어주고 있습니다.📷𝘚𝘤𝘳𝘦𝘦𝘯𝘴𝘩𝘰𝘵
게스트 로그인
Screen_recording_20241113_020434.mp4
유저 로그인
Screen_recording_20241113_032315.mp4
유저 로그아웃
Screen_recording_20241113_032340.mp4
💬𝘛𝘰 𝘙𝘦𝘷𝘪𝘦𝘸𝘦𝘳𝘴