From 7278fae142d93facacc033f95b6ad1a38e48804f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 23 Aug 2023 17:23:12 +0200 Subject: [PATCH 1/2] Handle remarks from PR #1127 --- .../ConfirmAccountProviderPresenter.kt | 9 ++++----- .../features/logout/api/LogoutPreferenceScreen.kt | 7 +++++-- .../preferences/impl/root/PreferencesRootView.kt | 2 +- .../element/android/libraries/matrix/api/MatrixClient.kt | 3 ++- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenter.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenter.kt index 61ef4d724b..d5412cb139 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenter.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenter.kt @@ -68,9 +68,9 @@ class ConfirmAccountProviderPresenter @AssistedInject constructor( } LaunchedEffect(Unit) { - launch { - defaultOidcActionFlow.collect { - onOidcAction(it, loginFlowAction) + defaultOidcActionFlow.collect { oidcAction -> + if (oidcAction != null) { + onOidcAction(oidcAction, loginFlowAction) } } } @@ -113,10 +113,9 @@ class ConfirmAccountProviderPresenter @AssistedInject constructor( } private suspend fun onOidcAction( - oidcAction: OidcAction?, + oidcAction: OidcAction, loginFlowAction: MutableState>, ) { - oidcAction ?: return loginFlowAction.value = Async.Loading() when (oidcAction) { OidcAction.GoBack -> { diff --git a/features/logout/api/src/main/kotlin/io/element/android/features/logout/api/LogoutPreferenceScreen.kt b/features/logout/api/src/main/kotlin/io/element/android/features/logout/api/LogoutPreferenceScreen.kt index f9f23ac9c4..7be1c977f2 100644 --- a/features/logout/api/src/main/kotlin/io/element/android/features/logout/api/LogoutPreferenceScreen.kt +++ b/features/logout/api/src/main/kotlin/io/element/android/features/logout/api/LogoutPreferenceScreen.kt @@ -34,7 +34,7 @@ import io.element.android.libraries.designsystem.preview.ElementPreviewLight @Composable fun LogoutPreferenceView( state: LogoutPreferenceState, - onSuccessLogout: (String?) -> Unit = {} + onSuccessLogout: (logoutUrlResult: String?) -> Unit ) { val eventSink = state.eventSink if (state.logoutAction is Async.Success) { @@ -96,5 +96,8 @@ internal fun LogoutPreferenceViewDarkPreview() = ElementPreviewDark { ContentToP @Composable private fun ContentToPreview() { - LogoutPreferenceView(aLogoutPreferenceState()) + LogoutPreferenceView( + aLogoutPreferenceState(), + onSuccessLogout = {} + ) } diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootView.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootView.kt index c24a2ec875..df108c03a3 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootView.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootView.kt @@ -57,7 +57,7 @@ fun PreferencesRootView( onOpenRageShake: () -> Unit, onOpenAbout: () -> Unit, onOpenDeveloperSettings: () -> Unit, - onSuccessLogout: (String?) -> Unit, + onSuccessLogout: (logoutUrlResult: String?) -> Unit, modifier: Modifier = Modifier, ) { val snackbarHostState = rememberSnackbarHostState(snackbarMessage = state.snackbarMessage) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/MatrixClient.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/MatrixClient.kt index be64a3371f..2ca4420145 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/MatrixClient.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/MatrixClient.kt @@ -58,7 +58,8 @@ interface MatrixClient : Closeable { /** * Logout the user. - * Returns an optional URL. When the URL is there, it should be presented to the user after logout for RP initiated logout on their account page. + * Returns an optional URL. When the URL is there, it should be presented to the user after logout for + * Relying Party (RP) initiated logout on their account page. */ suspend fun logout(): String? suspend fun loadUserDisplayName(): Result From fefc8fa3da2bfb06763033515a8e14cfc5813f8a Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 23 Aug 2023 18:11:08 +0200 Subject: [PATCH 2/2] Add a workaround to detect the Chrome Custom Tab closing (when there is no redirection). --- .../features/login/impl/LoginFlowNode.kt | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/LoginFlowNode.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/LoginFlowNode.kt index fe47fb1b67..e545bf7b86 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/LoginFlowNode.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/LoginFlowNode.kt @@ -22,7 +22,9 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext +import androidx.lifecycle.lifecycleScope import com.bumble.appyx.core.composable.Children +import com.bumble.appyx.core.lifecycle.subscribe import com.bumble.appyx.core.modality.BuildContext import com.bumble.appyx.core.node.Node import com.bumble.appyx.core.plugin.Plugin @@ -33,6 +35,8 @@ import com.bumble.appyx.navmodel.backstack.operation.singleTop import dagger.assisted.Assisted import dagger.assisted.AssistedInject import io.element.android.anvilannotations.ContributesNode +import io.element.android.features.login.api.oidc.OidcAction +import io.element.android.features.login.api.oidc.OidcActionFlow import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource import io.element.android.features.login.impl.oidc.CustomTabAvailabilityChecker import io.element.android.features.login.impl.oidc.customtab.CustomTabHandler @@ -51,6 +55,8 @@ import io.element.android.libraries.architecture.inputs import io.element.android.libraries.di.AppScope import io.element.android.libraries.matrix.api.auth.OidcDetails import io.element.android.libraries.theme.ElementTheme +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch import kotlinx.parcelize.Parcelize @ContributesNode(AppScope::class) @@ -61,6 +67,7 @@ class LoginFlowNode @AssistedInject constructor( private val customTabHandler: CustomTabHandler, private val accountProviderDataSource: AccountProviderDataSource, private val defaultLoginUserStory: DefaultLoginUserStory, + private val oidcActionFlow: OidcActionFlow, ) : BackstackNode( backstack = BackStack( initialElement = NavTarget.ConfirmAccountProvider, @@ -78,9 +85,26 @@ class LoginFlowNode @AssistedInject constructor( private val inputs: Inputs = inputs() + private var customChromeTabStarted = false + override fun onBuilt() { super.onBuilt() defaultLoginUserStory.setLoginFlowIsDone(false) + lifecycle.subscribe( + onResume = { + if (customChromeTabStarted) { + customChromeTabStarted = false + // Workaround to detect that the Custom Chrome Tab has been closed + // If there is no coming OidcAction (that would end this Node), + // consider that the user has cancelled the login + // by pressing back or by closing the Custom Chrome Tab. + lifecycleScope.launch { + delay(5000) + oidcActionFlow.post(OidcAction.GoBack) + } + } + } + ) } sealed interface NavTarget : Parcelable { @@ -113,7 +137,10 @@ class LoginFlowNode @AssistedInject constructor( override fun onOidcDetails(oidcDetails: OidcDetails) { if (customTabAvailabilityChecker.supportCustomTab()) { // In this case open a Chrome Custom tab - activity?.let { customTabHandler.open(it, darkTheme, oidcDetails.url) } + activity?.let { + customChromeTabStarted = true + customTabHandler.open(it, darkTheme, oidcDetails.url) + } } else { // Fallback to WebView mode backstack.push(NavTarget.OidcView(oidcDetails))