Merge pull request #3356 from element-hq/feature/bma/signOutFixes

Small fixes around logging out.
element-hq · Aug 29, 2024 · 1009fb3 · 1009fb3
2 parents 004679c + aae4487
commit 1009fb3
Show file tree

Hide file tree

Showing 15 changed files with 95 additions and 38 deletions.
diff --git a/.../impl/src/main/kotlin/io/element/android/features/lockscreen/impl/unlock/PinUnlockNode.kt b/.../impl/src/main/kotlin/io/element/android/features/lockscreen/impl/unlock/PinUnlockNode.kt
@@ -16,16 +16,20 @@
 
 package io.element.android.features.lockscreen.impl.unlock
 
+import android.app.Activity
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.platform.LocalContext
 import com.bumble.appyx.core.modality.BuildContext
 import com.bumble.appyx.core.node.Node
 import com.bumble.appyx.core.plugin.Plugin
 import com.bumble.appyx.core.plugin.plugins
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedInject
 import io.element.android.anvilannotations.ContributesNode
+import io.element.android.compound.theme.ElementTheme
+import io.element.android.features.logout.api.util.onSuccessLogout
 import io.element.android.libraries.di.SessionScope
 
 @ContributesNode(SessionScope::class)
@@ -47,6 +51,8 @@ class PinUnlockNode @AssistedInject constructor(
     @Composable
     override fun View(modifier: Modifier) {
         val state = presenter.present()
+        val activity = LocalContext.current as Activity
+        val isDark = ElementTheme.isLightTheme.not()
         LaunchedEffect(state.isUnlocked) {
             if (state.isUnlocked) {
                 onUnlock()
@@ -57,6 +63,7 @@ class PinUnlockNode @AssistedInject constructor(
             // UnlockNode is only used for in-app unlock, so we can safely set isInAppUnlock to true.
             // It's set to false in PinUnlockActivity.
             isInAppUnlock = true,
+            onSuccessLogout = { onSuccessLogout(activity, isDark, it) },
             modifier = modifier
         )
     }

diff --git a/.../src/main/kotlin/io/element/android/features/lockscreen/impl/unlock/PinUnlockPresenter.kt b/.../src/main/kotlin/io/element/android/features/lockscreen/impl/unlock/PinUnlockPresenter.kt
@@ -30,6 +30,7 @@ import io.element.android.features.lockscreen.impl.pin.PinCodeManager
 import io.element.android.features.lockscreen.impl.pin.model.PinEntry
 import io.element.android.features.lockscreen.impl.unlock.keypad.PinKeypadModel
 import io.element.android.features.logout.api.LogoutUseCase
+import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.AsyncData
 import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.architecture.runCatchingUpdatingState
@@ -61,7 +62,7 @@ class PinUnlockPresenter @Inject constructor(
             mutableStateOf(false)
         }
         val signOutAction = remember {
-            mutableStateOf<AsyncData<String?>>(AsyncData.Uninitialized)
+            mutableStateOf<AsyncAction<String?>>(AsyncAction.Uninitialized)
         }
         var biometricUnlockResult by remember {
             mutableStateOf<BiometricUnlock.AuthenticationResult?>(null)
@@ -177,7 +178,7 @@ class PinUnlockPresenter @Inject constructor(
         }
     }
 
-    private fun CoroutineScope.signOut(signOutAction: MutableState<AsyncData<String?>>) = launch {
+    private fun CoroutineScope.signOut(signOutAction: MutableState<AsyncAction<String?>>) = launch {
         suspend {
             logoutUseCase.logout(ignoreSdkError = true)
         }.runCatchingUpdatingState(signOutAction)

diff --git a/...impl/src/main/kotlin/io/element/android/features/lockscreen/impl/unlock/PinUnlockState.kt b/...impl/src/main/kotlin/io/element/android/features/lockscreen/impl/unlock/PinUnlockState.kt
@@ -19,14 +19,15 @@ package io.element.android.features.lockscreen.impl.unlock
 import io.element.android.features.lockscreen.impl.biometric.BiometricUnlock
 import io.element.android.features.lockscreen.impl.biometric.BiometricUnlockError
 import io.element.android.features.lockscreen.impl.pin.model.PinEntry
+import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.AsyncData
 
 data class PinUnlockState(
     val pinEntry: AsyncData<PinEntry>,
     val showWrongPinTitle: Boolean,
     val remainingAttempts: AsyncData<Int>,
     val showSignOutPrompt: Boolean,
-    val signOutAction: AsyncData<String?>,
+    val signOutAction: AsyncAction<String?>,
     val showBiometricUnlock: Boolean,
     val isUnlocked: Boolean,
     val biometricUnlockResult: BiometricUnlock.AuthenticationResult?,

diff --git a/.../main/kotlin/io/element/android/features/lockscreen/impl/unlock/PinUnlockStateProvider.kt b/.../main/kotlin/io/element/android/features/lockscreen/impl/unlock/PinUnlockStateProvider.kt
@@ -19,6 +19,7 @@ package io.element.android.features.lockscreen.impl.unlock
 import androidx.compose.ui.tooling.preview.PreviewParameterProvider
 import io.element.android.features.lockscreen.impl.biometric.BiometricUnlock
 import io.element.android.features.lockscreen.impl.pin.model.PinEntry
+import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.AsyncData
 
 open class PinUnlockStateProvider : PreviewParameterProvider<PinUnlockState> {
@@ -30,7 +31,7 @@ open class PinUnlockStateProvider : PreviewParameterProvider<PinUnlockState> {
             aPinUnlockState(showSignOutPrompt = true),
             aPinUnlockState(showBiometricUnlock = false),
             aPinUnlockState(showSignOutPrompt = true, remainingAttempts = 0),
-            aPinUnlockState(signOutAction = AsyncData.Loading()),
+            aPinUnlockState(signOutAction = AsyncAction.Loading),
         )
 }
 
@@ -42,7 +43,7 @@ fun aPinUnlockState(
     showBiometricUnlock: Boolean = true,
     biometricUnlockResult: BiometricUnlock.AuthenticationResult? = null,
     isUnlocked: Boolean = false,
-    signOutAction: AsyncData<String?> = AsyncData.Uninitialized,
+    signOutAction: AsyncAction<String?> = AsyncAction.Uninitialized,
 ) = PinUnlockState(
     pinEntry = AsyncData.Success(pinEntry),
     showWrongPinTitle = showWrongPinTitle,

diff --git a/.../impl/src/main/kotlin/io/element/android/features/lockscreen/impl/unlock/PinUnlockView.kt b/.../impl/src/main/kotlin/io/element/android/features/lockscreen/impl/unlock/PinUnlockView.kt
@@ -39,7 +39,9 @@ import androidx.compose.material.icons.filled.Lock
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.LaunchedEffect
+import androidx.compose.runtime.getValue
 import androidx.compose.runtime.remember
+import androidx.compose.runtime.rememberUpdatedState
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.focus.FocusRequester
@@ -56,6 +58,7 @@ import io.element.android.features.lockscreen.impl.components.PinEntryTextField
 import io.element.android.features.lockscreen.impl.pin.model.PinDigit
 import io.element.android.features.lockscreen.impl.pin.model.PinEntry
 import io.element.android.features.lockscreen.impl.unlock.keypad.PinKeypad
+import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.AsyncData
 import io.element.android.libraries.designsystem.atomic.atoms.RoundedIconAtom
 import io.element.android.libraries.designsystem.components.ProgressDialog
@@ -74,6 +77,7 @@ import io.element.android.libraries.ui.strings.CommonStrings
 fun PinUnlockView(
     state: PinUnlockState,
     isInAppUnlock: Boolean,
+    onSuccessLogout: (logoutUrlResult: String?) -> Unit,
     modifier: Modifier = Modifier,
 ) {
     OnLifecycleEvent { _, event ->
@@ -91,9 +95,21 @@ fun PinUnlockView(
                 onDismiss = { state.eventSink(PinUnlockEvents.ClearSignOutPrompt) },
             )
         }
-        if (state.signOutAction is AsyncData.Loading) {
-            ProgressDialog(text = stringResource(id = R.string.screen_signout_in_progress_dialog_content))
+        when (state.signOutAction) {
+            AsyncAction.Loading -> {
+                ProgressDialog(text = stringResource(id = R.string.screen_signout_in_progress_dialog_content))
+            }
+            is AsyncAction.Success -> {
+                val latestOnSuccessLogout by rememberUpdatedState(onSuccessLogout)
+                LaunchedEffect(state) {
+                    latestOnSuccessLogout(state.signOutAction.data)
+                }
+            }
+            AsyncAction.Confirming,
+            is AsyncAction.Failure,
+            AsyncAction.Uninitialized -> Unit
         }
+
         if (state.showBiometricUnlockError) {
             ErrorDialog(
                 content = state.biometricUnlockErrorMessage ?: "",
@@ -363,6 +379,7 @@ internal fun PinUnlockViewInAppPreview(@PreviewParameter(PinUnlockStateProvider:
         PinUnlockView(
             state = state,
             isInAppUnlock = true,
+            onSuccessLogout = {},
         )
     }
 }
@@ -374,6 +391,7 @@ internal fun PinUnlockViewPreview(@PreviewParameter(PinUnlockStateProvider::clas
         PinUnlockView(
             state = state,
             isInAppUnlock = false,
+            onSuccessLogout = {},
         )
     }
 }
diff --git a/...n/kotlin/io/element/android/features/lockscreen/impl/unlock/activity/PinUnlockActivity.kt b/...n/kotlin/io/element/android/features/lockscreen/impl/unlock/activity/PinUnlockActivity.kt
@@ -24,11 +24,13 @@ import androidx.activity.compose.setContent
 import androidx.activity.enableEdgeToEdge
 import androidx.appcompat.app.AppCompatActivity
 import androidx.lifecycle.lifecycleScope
+import io.element.android.compound.theme.ElementTheme
 import io.element.android.features.lockscreen.api.LockScreenLockState
 import io.element.android.features.lockscreen.api.LockScreenService
 import io.element.android.features.lockscreen.impl.unlock.PinUnlockPresenter
 import io.element.android.features.lockscreen.impl.unlock.PinUnlockView
 import io.element.android.features.lockscreen.impl.unlock.di.PinUnlockBindings
+import io.element.android.features.logout.api.util.onSuccessLogout
 import io.element.android.libraries.architecture.bindings
 import io.element.android.libraries.designsystem.theme.ElementThemeApp
 import io.element.android.libraries.preferences.api.store.AppPreferencesStore
@@ -53,7 +55,12 @@ class PinUnlockActivity : AppCompatActivity() {
         setContent {
             ElementThemeApp(appPreferencesStore) {
                 val state = presenter.present()
-                PinUnlockView(state = state, isInAppUnlock = false)
+                val isDark = ElementTheme.isLightTheme.not()
+                PinUnlockView(
+                    state = state,
+                    isInAppUnlock = false,
+                    onSuccessLogout = { onSuccessLogout(this, isDark, it) },
+                )
             }
         }
         lifecycleScope.launch {

diff --git a/.../test/kotlin/io/element/android/features/lockscreen/impl/unlock/PinUnlockPresenterTest.kt b/.../test/kotlin/io/element/android/features/lockscreen/impl/unlock/PinUnlockPresenterTest.kt
@@ -29,6 +29,7 @@ import io.element.android.features.lockscreen.impl.pin.model.PinEntry
 import io.element.android.features.lockscreen.impl.pin.model.assertText
 import io.element.android.features.lockscreen.impl.unlock.keypad.PinKeypadModel
 import io.element.android.features.logout.test.FakeLogoutUseCase
+import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.AsyncData
 import io.element.android.tests.testutils.lambda.assert
 import io.element.android.tests.testutils.lambda.lambdaRecorder
@@ -51,7 +52,7 @@ class PinUnlockPresenterTest {
                 assertThat(state.showWrongPinTitle).isFalse()
                 assertThat(state.showSignOutPrompt).isFalse()
                 assertThat(state.isUnlocked).isFalse()
-                assertThat(state.signOutAction).isInstanceOf(AsyncData.Uninitialized::class.java)
+                assertThat(state.signOutAction).isInstanceOf(AsyncAction.Uninitialized::class.java)
                 assertThat(state.remainingAttempts).isInstanceOf(AsyncData.Uninitialized::class.java)
             }
             awaitItem().also { state ->
@@ -106,7 +107,7 @@ class PinUnlockPresenterTest {
 
     @Test
     fun `present - forgot pin flow`() = runTest {
-        val signOutLambda = lambdaRecorder<Boolean, String> { "" }
+        val signOutLambda = lambdaRecorder<Boolean, String?> { "" }
         val signOut = FakeLogoutUseCase(signOutLambda)
         val presenter = createPinUnlockPresenter(this, logoutUseCase = signOut)
         moleculeFlow(RecompositionMode.Immediate) {
@@ -133,7 +134,7 @@ class PinUnlockPresenterTest {
             }
             skipItems(2)
             awaitItem().also { state ->
-                assertThat(state.signOutAction).isInstanceOf(AsyncData.Success::class.java)
+                assertThat(state.signOutAction).isInstanceOf(AsyncAction.Success::class.java)
             }
             assert(signOutLambda).isCalledOnce()
         }

diff --git a/features/logout/api/build.gradle.kts b/features/logout/api/build.gradle.kts
@@ -22,6 +22,7 @@ android {
 }
 
 dependencies {
+    implementation(projects.libraries.androidutils)
     implementation(projects.libraries.architecture)
     implementation(projects.libraries.designsystem)
     implementation(projects.libraries.uiStrings)

diff --git a/features/logout/api/src/main/kotlin/io/element/android/features/logout/api/LogoutUseCase.kt b/features/logout/api/src/main/kotlin/io/element/android/features/logout/api/LogoutUseCase.kt
@@ -23,9 +23,10 @@ interface LogoutUseCase {
     /**
      * Log out the current user and then perform any needed cleanup tasks.
      * @param ignoreSdkError if true, the SDK error will be ignored and the user will be logged out anyway.
-     * @return the session id of the logged out user.
+     * @return 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(ignoreSdkError: Boolean): String
+    suspend fun logout(ignoreSdkError: Boolean): String?
 
     interface Factory {
         fun create(sessionId: String): LogoutUseCase

diff --git a/features/logout/api/src/main/kotlin/io/element/android/features/logout/api/util/Util.kt b/features/logout/api/src/main/kotlin/io/element/android/features/logout/api/util/Util.kt
@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) 2024 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.features.logout.api.util
+
+import android.app.Activity
+import io.element.android.libraries.androidutils.browser.openUrlInChromeCustomTab
+import timber.log.Timber
+
+fun onSuccessLogout(
+    activity: Activity,
+    darkTheme: Boolean,
+    url: String?,
+) {
+    Timber.d("Success logout with result url: $url")
+    url?.let {
+        activity.openUrlInChromeCustomTab(null, darkTheme, it)
+    }
+}
diff --git a/...gout/impl/src/main/kotlin/io/element/android/features/logout/impl/DefaultLogoutUseCase.kt b/...gout/impl/src/main/kotlin/io/element/android/features/logout/impl/DefaultLogoutUseCase.kt
@@ -35,9 +35,9 @@ class DefaultLogoutUseCase @AssistedInject constructor(
         override fun create(sessionId: String): DefaultLogoutUseCase
     }
 
-    override suspend fun logout(ignoreSdkError: Boolean): String {
+    override suspend fun logout(ignoreSdkError: Boolean): String? {
         val matrixClient = matrixClientProvider.getOrRestore(SessionId(sessionId)).getOrThrow()
-        matrixClient.logout(ignoreSdkError = ignoreSdkError)
-        return sessionId
+        val result = matrixClient.logout(ignoreSdkError = ignoreSdkError)
+        return result
     }
 }
diff --git a/features/logout/impl/src/main/kotlin/io/element/android/features/logout/impl/LogoutNode.kt b/features/logout/impl/src/main/kotlin/io/element/android/features/logout/impl/LogoutNode.kt
@@ -27,10 +27,10 @@ import com.bumble.appyx.core.plugin.plugins
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedInject
 import io.element.android.anvilannotations.ContributesNode
+import io.element.android.compound.theme.ElementTheme
 import io.element.android.features.logout.api.LogoutEntryPoint
-import io.element.android.libraries.androidutils.browser.openUrlInChromeCustomTab
+import io.element.android.features.logout.api.util.onSuccessLogout
 import io.element.android.libraries.di.SessionScope
-import timber.log.Timber
 
 @ContributesNode(SessionScope::class)
 class LogoutNode @AssistedInject constructor(
@@ -42,21 +42,15 @@ class LogoutNode @AssistedInject constructor(
         plugins<LogoutEntryPoint.Callback>().forEach { it.onChangeRecoveryKeyClick() }
     }
 
-    private fun onSuccessLogout(activity: Activity, url: String?) {
-        Timber.d("Success logout with result url: $url")
-        url?.let {
-            activity.openUrlInChromeCustomTab(null, false, it)
-        }
-    }
-
     @Composable
     override fun View(modifier: Modifier) {
         val state = presenter.present()
         val activity = LocalContext.current as Activity
+        val isDark = ElementTheme.isLightTheme.not()
         LogoutView(
             state = state,
             onChangeRecoveryKeyClick = ::onChangeRecoveryKeyClick,
-            onSuccessLogout = { onSuccessLogout(activity, it) },
+            onSuccessLogout = { onSuccessLogout(activity, isDark, it) },
             onBackClick = ::navigateUp,
             modifier = modifier,
         )

diff --git a/.../logout/test/src/main/kotlin/io/element/android/features/logout/test/FakeLogoutUseCase.kt b/.../logout/test/src/main/kotlin/io/element/android/features/logout/test/FakeLogoutUseCase.kt
@@ -20,9 +20,9 @@ import io.element.android.features.logout.api.LogoutUseCase
 import io.element.android.tests.testutils.lambda.lambdaError
 
 class FakeLogoutUseCase(
-    var logoutLambda: (Boolean) -> String = lambdaError()
+    var logoutLambda: (Boolean) -> String? = { lambdaError() }
 ) : LogoutUseCase {
-    override suspend fun logout(ignoreSdkError: Boolean): String {
+    override suspend fun logout(ignoreSdkError: Boolean): String? {
         return logoutLambda(ignoreSdkError)
     }
 }
diff --git a/.../src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootNode.kt b/.../src/main/kotlin/io/element/android/features/preferences/impl/root/PreferencesRootNode.kt
@@ -30,10 +30,10 @@ import io.element.android.anvilannotations.ContributesNode
 import io.element.android.compound.theme.ElementTheme
 import io.element.android.features.logout.api.direct.DirectLogoutEvents
 import io.element.android.features.logout.api.direct.DirectLogoutView
+import io.element.android.features.logout.api.util.onSuccessLogout
 import io.element.android.libraries.androidutils.browser.openUrlInChromeCustomTab
 import io.element.android.libraries.di.SessionScope
 import io.element.android.libraries.matrix.api.user.MatrixUser
-import timber.log.Timber
 
 @ContributesNode(SessionScope::class)
 class PreferencesRootNode @AssistedInject constructor(
@@ -94,13 +94,6 @@ class PreferencesRootNode @AssistedInject constructor(
         }
     }
 
-    private fun onSuccessLogout(activity: Activity, url: String?) {
-        Timber.d("Success (direct) logout with result url: $url")
-        url?.let {
-            activity.openUrlInChromeCustomTab(null, false, it)
-        }
-    }
-
     private fun onOpenNotificationSettings() {
         plugins<Callback>().forEach { it.onOpenNotificationSettings() }
     }
@@ -153,7 +146,7 @@ class PreferencesRootNode @AssistedInject constructor(
         directLogoutView.Render(
             state = state.directLogoutState,
             onSuccessLogout = {
-                onSuccessLogout(activity, it)
+                onSuccessLogout(activity, isDark, it)
             }
         )
     }

diff --git a/.../io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenterTest.kt b/.../io/element/android/features/preferences/impl/developer/DeveloperSettingsPresenterTest.kt
@@ -166,7 +166,7 @@ class DeveloperSettingsPresenterTest {
 
     @Test
     fun `present - toggling simplified sliding sync changes the preferences and logs out the user`() = runTest {
-        val logoutCallRecorder = lambdaRecorder<Boolean, String> { "" }
+        val logoutCallRecorder = lambdaRecorder<Boolean, String?> { "" }
         val logoutUseCase = FakeLogoutUseCase(logoutLambda = logoutCallRecorder)
         val preferences = InMemoryAppPreferencesStore()
         val presenter = createDeveloperSettingsPresenter(preferencesStore = preferences, logoutUseCase = logoutUseCase)