diff --git a/changelog.d/8353.bugfix b/changelog.d/8353.bugfix new file mode 100644 index 0000000000..f92b1adad2 --- /dev/null +++ b/changelog.d/8353.bugfix @@ -0,0 +1 @@ +Fix | Got asked twice about verification #8353 (and other verification banners problems) diff --git a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/Device.kt b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/Device.kt index 7f2b1232fe..4cb329175b 100644 --- a/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/Device.kt +++ b/matrix-sdk-android/src/rustCrypto/java/org/matrix/android/sdk/internal/crypto/Device.kt @@ -187,8 +187,7 @@ internal class Device @AssistedInject constructor( locallyVerified = innerDevice.locallyTrusted ), isBlocked = innerDevice.isBlocked, - // TODO - firstTimeSeenLocalTs = null + firstTimeSeenLocalTs = innerDevice.firstTimeSeenTs.toLong() ) } } diff --git a/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationBottomSheet.kt b/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationBottomSheet.kt index 5005ccd12b..e63c149a26 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationBottomSheet.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/verification/self/SelfVerificationBottomSheet.kt @@ -156,7 +156,7 @@ class SelfVerificationBottomSheet : VectorBaseBottomSheetDialogFragment { - dismiss() + dismiss() } is VerificationBottomSheetViewEvents.ConfirmCancel -> { // TODO? applies to self? @@ -229,6 +229,9 @@ class SelfVerificationBottomSheet : VectorBaseBottomSheetDialogFragment state.unknownSessions.invoke()?.let { unknownDevices -> + val uid = PopupAlertManager.REVIEW_LOGIN_UID if (unknownDevices.firstOrNull()?.currentSessionTrust == true) { - val uid = PopupAlertManager.REVIEW_LOGIN_UID alertManager.cancelAlert(uid) val olderUnverified = unknownDevices.filter { !it.isNew } val newest = unknownDevices.firstOrNull { it.isNew }?.deviceInfo @@ -172,6 +174,9 @@ class NewHomeDetailFragment : // In this case we prompt to go to settings to review logins promptToReviewChanges(uid, state, olderUnverified.map { it.deviceInfo }) } + } else { + // cancel as there are not anymore untrusted devices + alertManager.cancelAlert(uid) } } } @@ -278,7 +283,14 @@ class NewHomeDetailFragment : uid = uid, title = getString(R.string.review_unverified_sessions_title), description = getString(R.string.review_unverified_sessions_description), - iconId = R.drawable.ic_shield_warning + iconId = R.drawable.ic_shield_warning, + shouldBeDisplayedIn = { activity -> + // do not show when there is an ongoing verification flow + if (activity is VectorBaseActivity<*>) { + activity.supportFragmentManager.findFragmentByTag(SelfVerificationBottomSheet.TAG) == null && + activity !is QrCodeScannerActivity + } else true + } ).apply { viewBinder = VerificationVectorAlert.ViewBinder(user, avatarRenderer) colorInt = colorProvider.getColorFromAttribute(R.attr.colorPrimary) @@ -351,9 +363,9 @@ class NewHomeDetailFragment : }) } - /* ========================================================================================== - * KeysBackupBanner Listener - * ========================================================================================== */ +/* ========================================================================================== + * KeysBackupBanner Listener + * ========================================================================================== */ override fun onCloseClicked() { serverBackupStatusViewModel.handle(ServerBackupStatusAction.OnBannerClosed) diff --git a/vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt b/vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt index 6ba5976eb8..7a28c98a7d 100644 --- a/vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/UnknownDeviceDetectorSharedViewModel.kt @@ -39,7 +39,6 @@ import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.sample import kotlinx.coroutines.launch -import org.matrix.android.sdk.api.extensions.orFalse import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.session.crypto.model.DeviceInfo @@ -116,8 +115,14 @@ class UnknownDeviceDetectorSharedViewModel @AssistedInject constructor( session.sessionParams.deviceId != it.deviceId } .filter { info -> - // filter out verified sessions or those which do not support encryption (i.e. without crypto info) - cryptoList.firstOrNull { info.deviceId == it.deviceId }?.isVerified?.not().orFalse() + val matchingDeviceWithKeys = cryptoList.firstOrNull { it.deviceId == info.deviceId } + if (matchingDeviceWithKeys == null) { + // filter out verified sessions or those which do not support encryption (i.e. without crypto info) + false + } else { + // Only report unverified + !matchingDeviceWithKeys.isVerified + } } // filter out ignored devices .filter { shouldShowUnverifiedSessionsAlertUseCase.execute(it.deviceId) } @@ -136,7 +141,7 @@ class UnknownDeviceDetectorSharedViewModel @AssistedInject constructor( } .distinctUntilChanged() .execute { async -> - Timber.v("## Detector trigger passed distinct") + Timber.v("## Detector trigger passed distinct ${async.invoke()}") copy( myMatrixItem = session.getUserOrDefault(session.myUserId).toMatrixItem(), unknownSessions = async diff --git a/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt b/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt index b38805f05a..3496ced21c 100644 --- a/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt +++ b/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt @@ -242,8 +242,8 @@ class DefaultNavigator @Inject constructor( } if (context is AppCompatActivity) { - SelfVerificationBottomSheet.forTransaction(tx.transactionId) - .show(context.supportFragmentManager, "VERIF") + SelfVerificationBottomSheet.forTransaction(tx.transactionId) + .show(context.supportFragmentManager, "VERIF") } } } @@ -258,7 +258,7 @@ class DefaultNavigator @Inject constructor( ) if (context is AppCompatActivity) { context.supportFragmentManager.commitTransaction(allowStateLoss = true) { - add(SelfVerificationBottomSheet.forTransaction(request.transactionId), "VERIF") + add(SelfVerificationBottomSheet.forTransaction(request.transactionId), SelfVerificationBottomSheet.TAG) } } } @@ -266,25 +266,10 @@ class DefaultNavigator @Inject constructor( override fun requestSelfSessionVerification(context: Context) { coroutineScope.launch { - // TODO - // val session = sessionHolder.getSafeActiveSession() ?: return@launch -// val otherSessions = session.cryptoService() -// .getCryptoDeviceInfoList(session.myUserId) -// .filter { it.deviceId != session.sessionParams.deviceId } -// .map { it.deviceId } if (context is AppCompatActivity) { context.supportFragmentManager.commitTransaction(allowStateLoss = true) { - add(SelfVerificationBottomSheet.verifyOwnUntrustedDevice(), "VERIF") + add(SelfVerificationBottomSheet.verifyOwnUntrustedDevice(), SelfVerificationBottomSheet.TAG) } -// if (otherSessions.isNotEmpty()) { -// val pr = session.cryptoService().verificationService().requestSelfKeyVerification( -// supportedVerificationMethodsProvider.provide()) -// VerificationBottomSheet.forSelfVerification(session, pr.transactionId) -// .show(context.supportFragmentManager, VerificationBottomSheet.WAITING_SELF_VERIF_TAG) -// } else { -// VerificationBottomSheet.forSelfVerification(session) -// .show(context.supportFragmentManager, VerificationBottomSheet.WAITING_SELF_VERIF_TAG) -// } } } } @@ -293,7 +278,7 @@ class DefaultNavigator @Inject constructor( // val session = sessionHolder.getSafeActiveSession() ?: return coroutineScope.launch(Dispatchers.Main) { SelfVerificationBottomSheet.forTransaction(transactionId) - .show(fragmentActivity.supportFragmentManager, "SELF_VERIF_TAG") + .show(fragmentActivity.supportFragmentManager, SelfVerificationBottomSheet.TAG) } } diff --git a/vector/src/main/java/im/vector/app/features/popup/PopupAlertManager.kt b/vector/src/main/java/im/vector/app/features/popup/PopupAlertManager.kt index 02275c933e..c59b2c7216 100644 --- a/vector/src/main/java/im/vector/app/features/popup/PopupAlertManager.kt +++ b/vector/src/main/java/im/vector/app/features/popup/PopupAlertManager.kt @@ -164,7 +164,7 @@ class PopupAlertManager @Inject constructor( next = alertQueue.maxByOrNull { it.priority } // If next alert with highest priority is higher than the current one, we should display it // and add the current one to queue again. - if (next != null && next.priority > currentAlerter?.priority ?: Int.MIN_VALUE) { + if (next != null && next.priority > (currentAlerter?.priority ?: Int.MIN_VALUE)) { alertQueue.remove(next) currentAlerter?.also { alertQueue.add(0, it)