Merge pull request #2874 from element-hq/feature/fga/fix_2692

Fix modal contents overlapping screen lock pin #2692
This commit is contained in:
ganfra 2024-05-21 15:53:51 +02:00 committed by GitHub
commit ac123bdcd8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
24 changed files with 364 additions and 99 deletions

View file

@ -0,0 +1,25 @@
<!--
~ 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
~
~ http://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.
-->
<manifest xmlns:android="http://schemas.android.com/apk/res/android">
<application>
<activity
android:name="io.element.android.features.lockscreen.impl.unlock.activity.PinUnlockActivity"
android:configChanges="screenSize|screenLayout|orientation|keyboardHidden|keyboard|navigation|uiMode"/>
</application>
</manifest>

View file

@ -16,18 +16,20 @@
package io.element.android.features.lockscreen.impl
import android.content.Context
import android.content.Intent
import com.bumble.appyx.core.modality.BuildContext
import com.bumble.appyx.core.node.Node
import com.squareup.anvil.annotations.ContributesBinding
import io.element.android.features.lockscreen.api.LockScreenEntryPoint
import io.element.android.features.lockscreen.impl.unlock.activity.PinUnlockActivity
import io.element.android.libraries.architecture.createNode
import io.element.android.libraries.di.AppScope
import javax.inject.Inject
@ContributesBinding(AppScope::class)
class DefaultLockScreenEntryPoint @Inject constructor() : LockScreenEntryPoint {
override fun nodeBuilder(parentNode: Node, buildContext: BuildContext): LockScreenEntryPoint.NodeBuilder {
var innerTarget: LockScreenEntryPoint.Target = LockScreenEntryPoint.Target.Unlock
override fun nodeBuilder(parentNode: Node, buildContext: BuildContext, navTarget: LockScreenEntryPoint.Target): LockScreenEntryPoint.NodeBuilder {
val callbacks = mutableListOf<LockScreenEntryPoint.Callback>()
return object : LockScreenEntryPoint.NodeBuilder {
@ -36,15 +38,9 @@ class DefaultLockScreenEntryPoint @Inject constructor() : LockScreenEntryPoint {
return this
}
override fun target(target: LockScreenEntryPoint.Target): LockScreenEntryPoint.NodeBuilder {
innerTarget = target
return this
}
override fun build(): Node {
val inputs = LockScreenFlowNode.Inputs(
when (innerTarget) {
LockScreenEntryPoint.Target.Unlock -> LockScreenFlowNode.NavTarget.Unlock
when (navTarget) {
LockScreenEntryPoint.Target.Setup -> LockScreenFlowNode.NavTarget.Setup
LockScreenEntryPoint.Target.Settings -> LockScreenFlowNode.NavTarget.Settings
}
@ -54,4 +50,8 @@ class DefaultLockScreenEntryPoint @Inject constructor() : LockScreenEntryPoint {
}
}
}
override fun pinUnlockIntent(context: Context): Intent {
return PinUnlockActivity.newIntent(context)
}
}

View file

@ -30,7 +30,6 @@ import io.element.android.anvilannotations.ContributesNode
import io.element.android.features.lockscreen.api.LockScreenEntryPoint
import io.element.android.features.lockscreen.impl.settings.LockScreenSettingsFlowNode
import io.element.android.features.lockscreen.impl.setup.LockScreenSetupFlowNode
import io.element.android.features.lockscreen.impl.unlock.PinUnlockNode
import io.element.android.libraries.architecture.BackstackView
import io.element.android.libraries.architecture.BaseFlowNode
import io.element.android.libraries.architecture.NodeInputs
@ -44,20 +43,17 @@ class LockScreenFlowNode @AssistedInject constructor(
@Assisted plugins: List<Plugin>,
) : BaseFlowNode<LockScreenFlowNode.NavTarget>(
backstack = BackStack(
initialElement = plugins.filterIsInstance(Inputs::class.java).first().initialNavTarget,
initialElement = plugins.filterIsInstance<Inputs>().first().initialNavTarget,
savedStateMap = buildContext.savedStateMap,
),
buildContext = buildContext,
plugins = plugins,
) {
data class Inputs(
val initialNavTarget: NavTarget = NavTarget.Unlock,
val initialNavTarget: NavTarget,
) : NodeInputs
sealed interface NavTarget : Parcelable {
@Parcelize
data object Unlock : NavTarget
@Parcelize
data object Setup : NavTarget
@ -75,10 +71,6 @@ class LockScreenFlowNode @AssistedInject constructor(
override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node {
return when (navTarget) {
NavTarget.Unlock -> {
val inputs = PinUnlockNode.Inputs(isInAppUnlock = false)
createNode<PinUnlockNode>(buildContext, plugins = listOf(inputs))
}
NavTarget.Setup -> {
val callback = OnSetupDoneCallback(plugins())
createNode<LockScreenSetupFlowNode>(buildContext, plugins = listOf(callback))

View file

@ -103,13 +103,12 @@ class LockScreenSettingsFlowNode @AssistedInject constructor(
override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node {
return when (navTarget) {
NavTarget.Unlock -> {
val inputs = PinUnlockNode.Inputs(isInAppUnlock = true)
val callback = object : PinUnlockNode.Callback {
override fun onUnlock() {
backstack.newRoot(NavTarget.Settings)
}
}
createNode<PinUnlockNode>(buildContext, plugins = listOf(inputs, callback))
createNode<PinUnlockNode>(buildContext, plugins = listOf(callback))
}
NavTarget.SetupPin -> {
createNode<SetupPinNode>(buildContext)

View file

@ -26,8 +26,6 @@ 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.libraries.architecture.NodeInputs
import io.element.android.libraries.architecture.inputs
import io.element.android.libraries.di.SessionScope
@ContributesNode(SessionScope::class)
@ -40,12 +38,6 @@ class PinUnlockNode @AssistedInject constructor(
fun onUnlock()
}
data class Inputs(
val isInAppUnlock: Boolean
) : NodeInputs
private val inputs: Inputs = inputs()
private fun onUnlock() {
plugins<Callback>().forEach {
it.onUnlock()
@ -62,7 +54,9 @@ class PinUnlockNode @AssistedInject constructor(
}
PinUnlockView(
state = state,
isInAppUnlock = inputs.isInAppUnlock,
// 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,
modifier = modifier
)
}

View file

@ -29,11 +29,11 @@ import io.element.android.features.lockscreen.impl.biometric.BiometricUnlockMana
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.lockscreen.impl.unlock.signout.SignOut
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.architecture.runCatchingUpdatingState
import io.element.android.libraries.core.bool.orFalse
import io.element.android.libraries.matrix.api.MatrixClient
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import javax.inject.Inject
@ -41,7 +41,7 @@ import javax.inject.Inject
class PinUnlockPresenter @Inject constructor(
private val pinCodeManager: PinCodeManager,
private val biometricUnlockManager: BiometricUnlockManager,
private val matrixClient: MatrixClient,
private val signOut: SignOut,
private val coroutineScope: CoroutineScope,
private val pinUnlockHelper: PinUnlockHelper,
) : Presenter<PinUnlockState> {
@ -179,7 +179,7 @@ class PinUnlockPresenter @Inject constructor(
private fun CoroutineScope.signOut(signOutAction: MutableState<AsyncData<String?>>) = launch {
suspend {
matrixClient.logout(ignoreSdkError = true)
signOut()
}.runCatchingUpdatingState(signOutAction)
}
}

View file

@ -0,0 +1,71 @@
/*
* 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
*
* http://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.lockscreen.impl.unlock.activity
import android.content.Context
import android.content.Intent
import android.os.Bundle
import androidx.activity.OnBackPressedCallback
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.libraries.architecture.bindings
import kotlinx.coroutines.launch
import javax.inject.Inject
class PinUnlockActivity : AppCompatActivity() {
internal companion object {
fun newIntent(context: Context): Intent {
return Intent(context, PinUnlockActivity::class.java)
}
}
@Inject lateinit var presenter: PinUnlockPresenter
@Inject lateinit var lockScreenService: LockScreenService
override fun onCreate(savedInstanceState: Bundle?) {
enableEdgeToEdge()
super.onCreate(savedInstanceState)
bindings<PinUnlockBindings>().inject(this)
setContent {
ElementTheme {
val state = presenter.present()
PinUnlockView(state = state, isInAppUnlock = false)
}
}
lifecycleScope.launch {
lockScreenService.lockState.collect { state ->
if (state == LockScreenLockState.Unlocked) {
finish()
}
}
}
val onBackPressedCallback = object : OnBackPressedCallback(true) {
override fun handleOnBackPressed() {
moveTaskToBack(true)
}
}
onBackPressedDispatcher.addCallback(this, onBackPressedCallback)
}
}

View file

@ -0,0 +1,26 @@
/*
* 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
*
* http://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.lockscreen.impl.unlock.di
import com.squareup.anvil.annotations.ContributesTo
import io.element.android.features.lockscreen.impl.unlock.activity.PinUnlockActivity
import io.element.android.libraries.di.AppScope
@ContributesTo(AppScope::class)
interface PinUnlockBindings {
fun inject(activity: PinUnlockActivity)
}

View file

@ -0,0 +1,40 @@
/*
* 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
*
* http://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.lockscreen.impl.unlock.signout
import com.squareup.anvil.annotations.ContributesBinding
import io.element.android.libraries.di.AppScope
import io.element.android.libraries.matrix.api.MatrixClientProvider
import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService
import javax.inject.Inject
@ContributesBinding(AppScope::class)
class DefaultSignOut @Inject constructor(
private val authenticationService: MatrixAuthenticationService,
private val matrixClientProvider: MatrixClientProvider,
) : SignOut {
override suspend fun invoke(): String? {
val currentSession = authenticationService.getLatestSessionId()
return if (currentSession != null) {
matrixClientProvider.getOrRestore(currentSession)
.getOrThrow()
.logout(ignoreSdkError = true)
} else {
error("No session to sign out")
}
}
}

View file

@ -0,0 +1,21 @@
/*
* 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
*
* http://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.lockscreen.impl.unlock.signout
interface SignOut {
suspend operator fun invoke(): String?
}

View file

@ -0,0 +1,61 @@
/*
* 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
*
* http://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.lockscreen.impl.unlock
import com.google.common.truth.Truth.assertThat
import io.element.android.features.lockscreen.impl.unlock.signout.DefaultSignOut
import io.element.android.libraries.matrix.test.FakeMatrixClient
import io.element.android.libraries.matrix.test.FakeMatrixClientProvider
import io.element.android.libraries.matrix.test.auth.FakeAuthenticationService
import io.element.android.tests.testutils.lambda.assert
import io.element.android.tests.testutils.lambda.lambdaRecorder
import kotlinx.coroutines.test.runTest
import org.junit.Test
class DefaultSignOutTest {
private val matrixClient = FakeMatrixClient()
private val authenticationService = FakeAuthenticationService()
private val matrixClientProvider = FakeMatrixClientProvider(getClient = { Result.success(matrixClient) })
private val sut = DefaultSignOut(authenticationService, matrixClientProvider)
@Test
fun `when no active session then it throws`() = runTest {
authenticationService.getLatestSessionIdLambda = { null }
val result = runCatching { sut.invoke() }
assertThat(result.isFailure).isTrue()
}
@Test
fun `with one active session and successful logout on client`() = runTest {
val logoutLambda = lambdaRecorder<Boolean, String?> { _: Boolean -> null }
authenticationService.getLatestSessionIdLambda = { matrixClient.sessionId }
matrixClient.logoutLambda = logoutLambda
val result = runCatching { sut.invoke() }
assertThat(result.isSuccess).isTrue()
assert(logoutLambda).isCalledOnce()
}
@Test
fun `with one active session and and failed logout on client`() = runTest {
val logoutLambda = lambdaRecorder<Boolean, String?> { _: Boolean -> error("Failed to logout") }
authenticationService.getLatestSessionIdLambda = { matrixClient.sessionId }
matrixClient.logoutLambda = logoutLambda
val result = runCatching { sut.invoke() }
assertThat(result.isFailure).isTrue()
assert(logoutLambda).isCalledOnce()
}
}

View file

@ -0,0 +1,28 @@
/*
* 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
*
* http://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.lockscreen.impl.unlock
import io.element.android.features.lockscreen.impl.unlock.signout.SignOut
import io.element.android.tests.testutils.simulateLongTask
class FakeSignOut(
var lambda: () -> String? = { null }
) : SignOut {
override suspend fun invoke(): String? = simulateLongTask {
lambda()
}
}

View file

@ -28,8 +28,10 @@ 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.pin.model.assertText
import io.element.android.features.lockscreen.impl.unlock.keypad.PinKeypadModel
import io.element.android.features.lockscreen.impl.unlock.signout.SignOut
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.matrix.test.FakeMatrixClient
import io.element.android.tests.testutils.lambda.assert
import io.element.android.tests.testutils.lambda.lambdaRecorder
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.test.runTest
import org.junit.Test
@ -104,7 +106,9 @@ class PinUnlockPresenterTest {
@Test
fun `present - forgot pin flow`() = runTest {
val presenter = createPinUnlockPresenter(this)
val signOutLambda = lambdaRecorder<String?> { null }
val signOut = FakeSignOut(signOutLambda)
val presenter = createPinUnlockPresenter(this, signOut = signOut)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
@ -131,6 +135,7 @@ class PinUnlockPresenterTest {
awaitItem().also { state ->
assertThat(state.signOutAction).isInstanceOf(AsyncData.Success::class.java)
}
assert(signOutLambda).isCalledOnce().withNoParameter()
}
}
@ -142,6 +147,7 @@ class PinUnlockPresenterTest {
scope: CoroutineScope,
biometricUnlockManager: BiometricUnlockManager = FakeBiometricUnlockManager(),
callback: PinCodeManager.Callback = DefaultPinCodeManagerCallback(),
signOut: SignOut = FakeSignOut(),
): PinUnlockPresenter {
val pinCodeManager = aPinCodeManager().apply {
addCallback(callback)
@ -150,7 +156,7 @@ class PinUnlockPresenterTest {
return PinUnlockPresenter(
pinCodeManager = pinCodeManager,
biometricUnlockManager = biometricUnlockManager,
matrixClient = FakeMatrixClient(),
signOut = signOut,
coroutineScope = scope,
pinUnlockHelper = PinUnlockHelper(biometricUnlockManager, pinCodeManager),
)