Skip to content
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.

Sign up for GitHub

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

[FC] Handle process kills after returning from browsers in AuthSessions #6853

Merged
merged 10 commits into from
Jun 12, 2023
Merged

[FC] Handle process kills after returning from browsers in AuthSessions #6853

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6df95df
[skip ci] Start PR
carlosmuvi-stripe Jun 9, 2023
e0985db
Persist web flow state and restore auth session id after process kills.
carlosmuvi-stripe Jun 9, 2023
e238a25
Removes mutex.
carlosmuvi-stripe Jun 10, 2023
ccfd628
Removes mutex.
carlosmuvi-stripe Jun 10, 2023
99fd905
Updates tests.
carlosmuvi-stripe Jun 10, 2023
43ff797
Do not clear shared preferences in playground's onDestroy.
carlosmuvi-stripe Jun 11, 2023
cce5162
Updates tests.
carlosmuvi-stripe Jun 11, 2023
c9b74e9
Uses active auth session as reference.
carlosmuvi-stripe Jun 12, 2023
3c87131
Merge remote-tracking branch 'origin/master' into carlosmuvi/BANKCON-…
carlosmuvi-stripe Jun 12, 2023
005f09f
Merge fixes.
carlosmuvi-stripe Jun 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions ...a/com/stripe/android/financialconnections/example/FinancialConnectionsLauncherActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,25 @@
package com.stripe.android.financialconnections.example

import android.app.Activity
import android.content.Context
import android.content.Intent
import android.os.Bundle
import android.view.View
import android.view.ViewGroup
import android.widget.TextView
import androidx.appcompat.app.AppCompatActivity
import androidx.core.content.edit
import androidx.recyclerview.widget.DividerItemDecoration
import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.RecyclerView
import com.stripe.android.financialconnections.example.databinding.ActivityFinancialconnectionsLauncherBinding

class FinancialConnectionsLauncherActivity : AppCompatActivity() {

private val connectionsDebugSharedPrefs by lazy {
getSharedPreferences("FINANCIAL_CONNECTIONS_DEBUG", Context.MODE_PRIVATE)
}

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)

Expand All @@ -38,6 +44,12 @@ class FinancialConnectionsLauncherActivity : AppCompatActivity() {
}
}

override fun onResume() {
super.onResume()
// prevent playground configuration from leaking to example apps.
connectionsDebugSharedPrefs.edit { clear() }
}
Comment on lines +47 to +51
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Small change on the Playground in the example app to make it work across process kills -> we override a deeplink based on a shared preference for local testing. We were clearing them on Activity's onDestroy to prevent it from leaking to the official examples just in case.

Process kills kill the activity triggering the preference clear. Moved the clear to the launcher activity instead.


private class ExamplesAdapter constructor(
private val activity: Activity
) : RecyclerView.Adapter<ExamplesAdapter.ExamplesViewHolder>() {
Expand Down
6 changes: 0 additions & 6 deletions ...com/stripe/android/financialconnections/example/FinancialConnectionsPlaygroundActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,6 @@ class FinancialConnectionsPlaygroundActivity : AppCompatActivity() {
}
}

override fun onDestroy() {
super.onDestroy()
// prevent playground configuration from leaking to example apps.
connectionsDebugSharedPrefs.edit { clear() }
}

@Composable
private fun FinancialConnectionsScreen() {
val state: FinancialConnectionsPlaygroundState by viewModel.state.collectAsState()
Expand Down
3 changes: 3 additions & 0 deletions ...ain/java/com/stripe/android/financialconnections/features/partnerauth/PartnerAuthState.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ import com.airbnb.mvrx.Async
import com.airbnb.mvrx.Fail
import com.airbnb.mvrx.Loading
import com.airbnb.mvrx.MavericksState
import com.airbnb.mvrx.PersistState
import com.airbnb.mvrx.Success
import com.airbnb.mvrx.Uninitialized
import com.stripe.android.financialconnections.model.DataAccessNotice
import com.stripe.android.financialconnections.model.FinancialConnectionsAuthorizationSession
import com.stripe.android.financialconnections.model.FinancialConnectionsInstitution

internal data class PartnerAuthState(
@PersistState
val firstInit: Boolean = true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of persisting firstInit, would it make sense to persist a nullable session ID?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I like that better actually! Even if we can't "fetch the auth session by id" I find it more readable as well - c9b74e9

val payload: Async<Payload> = Uninitialized,
val viewEffect: ViewEffect? = null,
val authenticationStatus: Async<String> = Uninitialized
Expand Down
59 changes: 42 additions & 17 deletions ...java/com/stripe/android/financialconnections/features/partnerauth/PartnerAuthViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
package com.stripe.android.financialconnections.features.partnerauth

import android.webkit.URLUtil
import com.airbnb.mvrx.Async
import com.airbnb.mvrx.Fail
import com.airbnb.mvrx.Loading
import com.airbnb.mvrx.MavericksViewModel
import com.airbnb.mvrx.MavericksViewModelFactory
import com.airbnb.mvrx.Success
import com.airbnb.mvrx.Uninitialized
import com.airbnb.mvrx.ViewModelContext
import com.stripe.android.core.Logger
Expand All @@ -24,7 +22,7 @@ import com.stripe.android.financialconnections.domain.GoNext
import com.stripe.android.financialconnections.domain.PollAuthorizationSessionOAuthResults
import com.stripe.android.financialconnections.domain.PostAuthSessionEvent
import com.stripe.android.financialconnections.domain.PostAuthorizationSession
import com.stripe.android.financialconnections.exception.WebAuthFlowCancelledException
import com.stripe.android.financialconnections.exception.WebAuthFlowFailedException
import com.stripe.android.financialconnections.features.partnerauth.PartnerAuthState.Payload
import com.stripe.android.financialconnections.features.partnerauth.PartnerAuthState.ViewEffect.OpenBottomSheet
import com.stripe.android.financialconnections.features.partnerauth.PartnerAuthState.ViewEffect.OpenPartnerAuth
Expand All @@ -33,12 +31,14 @@ import com.stripe.android.financialconnections.model.FinancialConnectionsSession
import com.stripe.android.financialconnections.model.FinancialConnectionsSessionManifest.Pane
import com.stripe.android.financialconnections.navigation.NavigationDirections
import com.stripe.android.financialconnections.navigation.NavigationManager
import com.stripe.android.financialconnections.presentation.WebAuthFlowState
import com.stripe.android.financialconnections.ui.FinancialConnectionsSheetNativeActivity
import com.stripe.android.financialconnections.utils.UriUtils
import kotlinx.coroutines.launch
import java.util.Date
import javax.inject.Inject
import javax.inject.Named

@Suppress("LongParameterList")
internal class PartnerAuthViewModel @Inject constructor(
private val completeAuthorizationSession: CompleteAuthorizationSession,
Expand All @@ -58,8 +58,34 @@ internal class PartnerAuthViewModel @Inject constructor(

init {
logErrors()
observePayload()
createAuthSession()
withState {
if (it.firstInit) {
setState { copy(firstInit = false) }
launchBrowserIfNonOauth()
createAuthSession()
} else {
restoreAuthSession()
}
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't want to create a new session if coming from a process kill. We'll restore the session from the manifest instead.


private fun restoreAuthSession() {
suspend {
// if coming from a process kill, there should be a session
// re-fetch the manifest and use its active auth session instead of creating a new one
val manifest: FinancialConnectionsSessionManifest = getManifest()
val authSession = manifest.activeAuthSession?.also {
logger.debug("Restoring auth session ${it.id}")
} ?: createAuthorizationSession(
institution = requireNotNull(manifest.activeInstitution),
allowManualEntry = manifest.allowManualEntry
)
Payload(
authSession = authSession,
institution = requireNotNull(manifest.activeInstitution),
isStripeDirect = manifest.isStripeDirect ?: false
)
}.execute { copy(payload = it) }
}

private fun createAuthSession() {
Expand All @@ -70,6 +96,7 @@ internal class PartnerAuthViewModel @Inject constructor(
institution = requireNotNull(manifest.activeInstitution),
allowManualEntry = manifest.allowManualEntry
)
logger.debug("Created auth session ${authSession.id}")
Payload(
authSession = authSession,
institution = requireNotNull(manifest.activeInstitution),
Expand All @@ -85,7 +112,7 @@ internal class PartnerAuthViewModel @Inject constructor(
}.execute { copy(payload = it) }
}

private fun observePayload() {
private fun launchBrowserIfNonOauth() {
onAsync(
asyncProp = PartnerAuthState::payload,
onSuccess = {
Expand Down Expand Up @@ -134,27 +161,25 @@ internal class PartnerAuthViewModel @Inject constructor(
}

fun onWebAuthFlowFinished(
webStatus: Async<String>
webStatus: WebAuthFlowState
) {
logger.debug("Web AuthFlow status received $webStatus")
viewModelScope.launch {
when (webStatus) {
is Uninitialized -> {}
is Loading -> setState { copy(authenticationStatus = Loading()) }
is Success -> completeAuthorizationSession()
is Fail -> {
when (val error = webStatus.error) {
is WebAuthFlowCancelledException -> onAuthCancelled()
else -> onAuthFailed(error)
}
}
WebAuthFlowState.Canceled -> onAuthCancelled()
is WebAuthFlowState.Failed -> onAuthFailed(webStatus.message, webStatus.reason)
WebAuthFlowState.InProgress -> setState { copy(authenticationStatus = Loading()) }
is WebAuthFlowState.Success -> completeAuthorizationSession()
WebAuthFlowState.Uninitialized -> {}
}
}
}

private suspend fun onAuthFailed(
error: Throwable
message: String,
reason: String?
) {
val error = WebAuthFlowFailedException(message, reason)
kotlin.runCatching {
logger.debug("Auth failed, cancelling AuthSession")
val authSession = getManifest().activeAuthSession
Expand Down
Loading