From 2e08c07dad693c1ff78c4ab1da46e1ed9cf45bb8 Mon Sep 17 00:00:00 2001 From: ariskotsomitopoulos Date: Mon, 16 May 2022 13:05:38 +0300 Subject: [PATCH 01/44] Enhance decryption to prevent DUPLICATED_MESSAGE_INDEX when decrypting the same eventId Improve code format --- .../crypto/replay_attack/ReplayAttackTest.kt | 160 ++++++++++++++++++ .../sdk/internal/crypto/MXOlmDevice.kt | 100 ++++++----- .../algorithms/megolm/MXMegolmDecryption.kt | 1 + 3 files changed, 218 insertions(+), 43 deletions(-) create mode 100644 matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replay_attack/ReplayAttackTest.kt diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replay_attack/ReplayAttackTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replay_attack/ReplayAttackTest.kt new file mode 100644 index 0000000000..5c9892e264 --- /dev/null +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replay_attack/ReplayAttackTest.kt @@ -0,0 +1,160 @@ +/* + * Copyright (c) 2022 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 org.matrix.android.sdk.internal.crypto.replay_attack + +import android.util.Log +import androidx.test.filters.LargeTest +import org.junit.Assert +import org.junit.Assert.assertEquals +import org.junit.FixMethodOrder +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import org.junit.runners.MethodSorters +import org.matrix.android.sdk.InstrumentedTest +import org.matrix.android.sdk.api.session.events.model.EventType +import org.matrix.android.sdk.api.session.events.model.toModel +import org.matrix.android.sdk.api.session.room.Room +import org.matrix.android.sdk.api.session.room.model.message.MessageContent +import org.matrix.android.sdk.api.session.room.send.SendState +import org.matrix.android.sdk.api.session.room.timeline.TimelineSettings +import org.matrix.android.sdk.common.CommonTestHelper +import org.matrix.android.sdk.common.CryptoTestHelper +import org.matrix.android.sdk.common.TestConstants + +@RunWith(JUnit4::class) +@FixMethodOrder(MethodSorters.JVM) +@LargeTest +class ReplayAttackTest : InstrumentedTest { + + @Test + fun replayAttackTest() { + val testHelper = CommonTestHelper(context()) + val cryptoTestHelper = CryptoTestHelper(testHelper) + val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoom(true) + + val e2eRoomID = cryptoTestData.roomId + + // Alice + val aliceSession = cryptoTestData.firstSession + val aliceRoomPOV = aliceSession.roomService().getRoom(e2eRoomID)!! + + // Bob + val bobSession = cryptoTestData.secondSession + val bobRoomPOV = bobSession!!.roomService().getRoom(e2eRoomID)!! + + assertEquals(bobRoomPOV.roomSummary()?.joinedMembersCount, 2) + Log.v("##REPLAY ATTACK", "Alice and Bob are in roomId: $e2eRoomID") + + + val sentEvents = testHelper.sendTextMessage(aliceRoomPOV, "Hello", 20) + +// val aliceMessageId: String? = sendMessageInRoom(aliceRoomPOV, "Hello Bob, I am Alice!", testHelper) + Assert.assertTrue("Message should be sent", sentEvents.size == 20) + Log.v("##REPLAY ATTACK", "Alice sent message to roomId: $e2eRoomID") + + // Bob should be able to decrypt the message +// testHelper.waitWithLatch { latch -> +// testHelper.retryPeriodicallyWithLatch(latch) { +// val timelineEvent = bobSession.roomService().getRoom(e2eRoomID)?.timelineService()?.getTimelineEvent(aliceMessageId!!) +// (timelineEvent != null && +// timelineEvent.isEncrypted() && +// timelineEvent.root.getClearType() == EventType.MESSAGE).also { +// if (it) { +// Log.v("#E2E TEST", "Bob can decrypt the message: ${timelineEvent?.root?.getDecryptedTextSummary()}") +// } +// } +// } +// } +// +// // Create a new user +// val arisSession = testHelper.createAccount("aris", SessionTestParams(true)) +// Log.v("#E2E TEST", "Aris user created") +// +// // Alice invites new user to the room +// testHelper.runBlockingTest { +// Log.v("#E2E TEST", "Alice invites ${arisSession.myUserId}") +// aliceRoomPOV.membershipService().invite(arisSession.myUserId) +// } +// +// waitForAndAcceptInviteInRoom(arisSession, e2eRoomID, testHelper) +// +// ensureMembersHaveJoined(aliceSession, arrayListOf(arisSession), e2eRoomID, testHelper) +// Log.v("#E2E TEST", "Aris has joined roomId: $e2eRoomID") +// +// when (roomHistoryVisibility) { +// RoomHistoryVisibility.WORLD_READABLE, +// RoomHistoryVisibility.SHARED, +// null +// -> { +// // Aris should be able to decrypt the message +// testHelper.waitWithLatch { latch -> +// testHelper.retryPeriodicallyWithLatch(latch) { +// val timelineEvent = arisSession.roomService().getRoom(e2eRoomID)?.timelineService()?.getTimelineEvent(aliceMessageId!!) +// (timelineEvent != null && +// timelineEvent.isEncrypted() && +// timelineEvent.root.getClearType() == EventType.MESSAGE +// ).also { +// if (it) { +// Log.v("#E2E TEST", "Aris can decrypt the message: ${timelineEvent?.root?.getDecryptedTextSummary()}") +// } +// } +// } +// } +// } +// RoomHistoryVisibility.INVITED, +// RoomHistoryVisibility.JOINED -> { +// // Aris should not even be able to get the message +// testHelper.waitWithLatch { latch -> +// testHelper.retryPeriodicallyWithLatch(latch) { +// val timelineEvent = arisSession.roomService().getRoom(e2eRoomID) +// ?.timelineService() +// ?.getTimelineEvent(aliceMessageId!!) +// timelineEvent == null +// } +// } +// } +// } + +// testHelper.signOutAndClose(arisSession) + cryptoTestData.cleanUp(testHelper) + } + + private fun sendMessageInRoom(aliceRoomPOV: Room, text: String, testHelper: CommonTestHelper): String? { + aliceRoomPOV.sendService().sendTextMessage(text) + var sentEventId: String? = null + testHelper.waitWithLatch(4 * TestConstants.timeOutMillis) { latch -> + val timeline = aliceRoomPOV.timelineService().createTimeline(null, TimelineSettings(60)) + timeline.start() + testHelper.retryPeriodicallyWithLatch(latch) { + val decryptedMsg = timeline.getSnapshot() + .filter { it.root.getClearType() == EventType.MESSAGE } + .also { list -> + val message = list.joinToString(",", "[", "]") { "${it.root.type}|${it.root.sendState}" } + Log.v("#E2E TEST", "Timeline snapshot is $message") + } + .filter { it.root.sendState == SendState.SYNCED } + .firstOrNull { it.root.getClearContent().toModel()?.body?.startsWith(text) == true } + sentEventId = decryptedMsg?.eventId + decryptedMsg != null + } + + timeline.dispose() + } + return sentEventId + } +} diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt index 68a1519670..16e6bb173d 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt @@ -99,6 +99,8 @@ internal class MXOlmDevice @Inject constructor( // The second level keys are strings of form "||" private val inboundGroupSessionMessageIndexes: MutableMap> = HashMap() + private val replayAttackMap: MutableMap = HashMap() + init { // Retrieve the account from the store try { @@ -763,59 +765,71 @@ internal class MXOlmDevice @Inject constructor( suspend fun decryptGroupMessage(body: String, roomId: String, timeline: String?, + eventId: String, sessionId: String, senderKey: String): OlmDecryptionResult { val sessionHolder = getInboundGroupSession(sessionId, senderKey, roomId) val wrapper = sessionHolder.wrapper val inboundGroupSession = wrapper.olmInboundGroupSession ?: throw MXCryptoError.Base(MXCryptoError.ErrorType.UNABLE_TO_DECRYPT, "Session is null") - // Check that the room id matches the original one for the session. This stops - // the HS pretending a message was targeting a different room. - if (roomId == wrapper.roomId) { - val decryptResult = try { - sessionHolder.mutex.withLock { - inboundGroupSession.decryptMessage(body) - } - } catch (e: OlmException) { - Timber.tag(loggerTag.value).e(e, "## decryptGroupMessage () : decryptMessage failed") - throw MXCryptoError.OlmError(e) - } - - if (timeline?.isNotBlank() == true) { - val timelineSet = inboundGroupSessionMessageIndexes.getOrPut(timeline) { mutableSetOf() } - - val messageIndexKey = senderKey + "|" + sessionId + "|" + decryptResult.mIndex - - if (timelineSet.contains(messageIndexKey)) { - val reason = String.format(MXCryptoError.DUPLICATE_MESSAGE_INDEX_REASON, decryptResult.mIndex) - Timber.tag(loggerTag.value).e("## decryptGroupMessage() timelineId=$timeline: $reason") - throw MXCryptoError.Base(MXCryptoError.ErrorType.DUPLICATED_MESSAGE_INDEX, reason) - } - - timelineSet.add(messageIndexKey) - } - - inboundGroupSessionStore.storeInBoundGroupSession(sessionHolder, sessionId, senderKey) - val payload = try { - val adapter = MoshiProvider.providesMoshi().adapter(JSON_DICT_PARAMETERIZED_TYPE) - val payloadString = convertFromUTF8(decryptResult.mDecryptedMessage) - adapter.fromJson(payloadString) - } catch (e: Exception) { - Timber.tag(loggerTag.value).e("## decryptGroupMessage() : fails to parse the payload") - throw MXCryptoError.Base(MXCryptoError.ErrorType.BAD_DECRYPTED_FORMAT, MXCryptoError.BAD_DECRYPTED_FORMAT_TEXT_REASON) - } - - return OlmDecryptionResult( - payload, - wrapper.keysClaimed, - senderKey, - wrapper.forwardingCurve25519KeyChain - ) - } else { + if (roomId != wrapper.roomId) { + // Check that the room id matches the original one for the session. This stops + // the HS pretending a message was targeting a different room. val reason = String.format(MXCryptoError.INBOUND_SESSION_MISMATCH_ROOM_ID_REASON, roomId, wrapper.roomId) Timber.tag(loggerTag.value).e("## decryptGroupMessage() : $reason") throw MXCryptoError.Base(MXCryptoError.ErrorType.INBOUND_SESSION_MISMATCH_ROOM_ID, reason) } + val decryptResult = try { + sessionHolder.mutex.withLock { + inboundGroupSession.decryptMessage(body) + } + } catch (e: OlmException) { + Timber.tag(loggerTag.value).e(e, "## decryptGroupMessage () : decryptMessage failed") + throw MXCryptoError.OlmError(e) + } + + val messageIndexKey = senderKey + "|" + sessionId + "|" + roomId + "|" + decryptResult.mIndex + Timber.tag(loggerTag.value).d("##########################################################") + Timber.tag(loggerTag.value).d("## decryptGroupMessage() timeline: $timeline") + Timber.tag(loggerTag.value).d("## decryptGroupMessage() senderKey: $senderKey") + Timber.tag(loggerTag.value).d("## decryptGroupMessage() sessionId: $sessionId") + Timber.tag(loggerTag.value).d("## decryptGroupMessage() roomId: $roomId") + Timber.tag(loggerTag.value).d("## decryptGroupMessage() eventId: $eventId") + Timber.tag(loggerTag.value).d("## decryptGroupMessage() mIndex: ${decryptResult.mIndex}") + + if (timeline?.isNotBlank() == true) { + val timelineSet = inboundGroupSessionMessageIndexes.getOrPut(timeline) { mutableSetOf() } + if (timelineSet.contains(messageIndexKey) && messageIndexKey.alreadyUsed(eventId)) { + val reason = String.format(MXCryptoError.DUPLICATE_MESSAGE_INDEX_REASON, decryptResult.mIndex) + Timber.tag(loggerTag.value).e("## decryptGroupMessage() timelineId=$timeline: $reason") + throw MXCryptoError.Base(MXCryptoError.ErrorType.DUPLICATED_MESSAGE_INDEX, reason) + } + timelineSet.add(messageIndexKey) + } + replayAttackMap[messageIndexKey] = eventId + inboundGroupSessionStore.storeInBoundGroupSession(sessionHolder, sessionId, senderKey) + val payload = try { + val adapter = MoshiProvider.providesMoshi().adapter(JSON_DICT_PARAMETERIZED_TYPE) + val payloadString = convertFromUTF8(decryptResult.mDecryptedMessage) + adapter.fromJson(payloadString) + } catch (e: Exception) { + Timber.tag(loggerTag.value).e("## decryptGroupMessage() : fails to parse the payload") + throw MXCryptoError.Base(MXCryptoError.ErrorType.BAD_DECRYPTED_FORMAT, MXCryptoError.BAD_DECRYPTED_FORMAT_TEXT_REASON) + } + + return OlmDecryptionResult( + payload, + wrapper.keysClaimed, + senderKey, + wrapper.forwardingCurve25519KeyChain + ) + } + + /** + * Determines whether or not the messageKey has already been used to decrypt another eventId + */ + private fun String.alreadyUsed(eventId: String): Boolean { + return replayAttackMap[this] != null && replayAttackMap[this] != eventId } /** diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmDecryption.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmDecryption.kt index d65b05f655..aff8013784 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmDecryption.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmDecryption.kt @@ -78,6 +78,7 @@ internal class MXMegolmDecryption( encryptedEventContent.ciphertext, event.roomId, timeline, + eventId = event.eventId.orEmpty(), encryptedEventContent.sessionId, encryptedEventContent.senderKey ) From e706c5a3c8c264feb3d0b19e5efe3dbd56d9dd35 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Tue, 17 May 2022 11:09:14 +0200 Subject: [PATCH 02/44] Adds up navigation from spaces --- .../vector/app/features/home/HomeActivity.kt | 75 ++++++++++--------- .../features/home/HomeActivitySharedAction.kt | 3 +- .../app/features/home/HomeDetailFragment.kt | 70 ++++++++--------- .../src/main/res/drawable/ic_arrow_back.xml | 5 ++ .../main/res/layout/fragment_home_detail.xml | 17 ++++- vector/src/main/res/values/strings.xml | 1 + 6 files changed, 98 insertions(+), 73 deletions(-) create mode 100644 vector/src/main/res/drawable/ic_arrow_back.xml diff --git a/vector/src/main/java/im/vector/app/features/home/HomeActivity.kt b/vector/src/main/java/im/vector/app/features/home/HomeActivity.kt index cc202868cc..b4943ce1b4 100644 --- a/vector/src/main/java/im/vector/app/features/home/HomeActivity.kt +++ b/vector/src/main/java/im/vector/app/features/home/HomeActivity.kt @@ -199,43 +199,13 @@ class HomeActivity : when (sharedAction) { is HomeActivitySharedAction.OpenDrawer -> views.drawerLayout.openDrawer(GravityCompat.START) is HomeActivitySharedAction.CloseDrawer -> views.drawerLayout.closeDrawer(GravityCompat.START) - is HomeActivitySharedAction.OpenGroup -> { - views.drawerLayout.closeDrawer(GravityCompat.START) - - // Temporary - // When switching from space to group or group to space, we need to reload the fragment - // To be removed when dropping legacy groups - if (sharedAction.clearFragment) { - replaceFragment(views.homeDetailFragmentContainer, HomeDetailFragment::class.java, allowStateLoss = true) - } else { - // nop - } - // we might want to delay that to avoid having the drawer animation lagging - // would be probably better to let the drawer do that? in the on closed callback? - } - is HomeActivitySharedAction.OpenSpacePreview -> { - startActivity(SpacePreviewActivity.newIntent(this, sharedAction.spaceId)) - } - is HomeActivitySharedAction.AddSpace -> { - createSpaceResultLauncher.launch(SpaceCreationActivity.newIntent(this)) - } - is HomeActivitySharedAction.ShowSpaceSettings -> { - // open bottom sheet - SpaceSettingsMenuBottomSheet - .newInstance(sharedAction.spaceId, object : SpaceSettingsMenuBottomSheet.InteractionListener { - override fun onShareSpaceSelected(spaceId: String) { - ShareSpaceBottomSheet.show(supportFragmentManager, spaceId) - } - }) - .show(supportFragmentManager, "SPACE_SETTINGS") - } - is HomeActivitySharedAction.OpenSpaceInvite -> { - SpaceInviteBottomSheet.newInstance(sharedAction.spaceId) - .show(supportFragmentManager, "SPACE_INVITE") - } - HomeActivitySharedAction.SendSpaceFeedBack -> { - bugReporter.openBugReportScreen(this, ReportType.SPACE_BETA_FEEDBACK) - } + is HomeActivitySharedAction.OpenGroup -> openGroup(sharedAction.shouldClearFragment) + is HomeActivitySharedAction.OpenSpacePreview -> startActivity(SpacePreviewActivity.newIntent(this, sharedAction.spaceId)) + is HomeActivitySharedAction.AddSpace -> createSpaceResultLauncher.launch(SpaceCreationActivity.newIntent(this)) + is HomeActivitySharedAction.ShowSpaceSettings -> showSpaceSettings(sharedAction.spaceId) + is HomeActivitySharedAction.OpenSpaceInvite -> openSpaceInvite(sharedAction.spaceId) + HomeActivitySharedAction.SendSpaceFeedBack -> bugReporter.openBugReportScreen(this, ReportType.SPACE_BETA_FEEDBACK) + HomeActivitySharedAction.CloseGroup -> closeGroup() } } .launchIn(lifecycleScope) @@ -272,6 +242,37 @@ class HomeActivity : homeActivityViewModel.handle(HomeActivityViewActions.ViewStarted) } + private fun openGroup(shouldClearFragment: Boolean) { + views.drawerLayout.closeDrawer(GravityCompat.START) + + // When switching from space to group or group to space, we need to reload the fragment + if (shouldClearFragment) { + replaceFragment(views.homeDetailFragmentContainer, HomeDetailFragment::class.java, allowStateLoss = true) + } else { + // do nothing + } + } + + private fun showSpaceSettings(spaceId: String) { + // open bottom sheet + SpaceSettingsMenuBottomSheet + .newInstance(spaceId, object : SpaceSettingsMenuBottomSheet.InteractionListener { + override fun onShareSpaceSelected(spaceId: String) { + ShareSpaceBottomSheet.show(supportFragmentManager, spaceId) + } + }) + .show(supportFragmentManager, "SPACE_SETTINGS") + } + + private fun openSpaceInvite(spaceId: String) { + SpaceInviteBottomSheet.newInstance(spaceId) + .show(supportFragmentManager, "SPACE_INVITE") + } + + private fun closeGroup() { + views.drawerLayout.openDrawer(GravityCompat.START) + } + private fun handleShowAnalyticsOptIn() { navigator.openAnalyticsOptIn(this) } diff --git a/vector/src/main/java/im/vector/app/features/home/HomeActivitySharedAction.kt b/vector/src/main/java/im/vector/app/features/home/HomeActivitySharedAction.kt index 6047a1e55e..cebde7fe0a 100644 --- a/vector/src/main/java/im/vector/app/features/home/HomeActivitySharedAction.kt +++ b/vector/src/main/java/im/vector/app/features/home/HomeActivitySharedAction.kt @@ -24,7 +24,8 @@ import im.vector.app.core.platform.VectorSharedAction sealed class HomeActivitySharedAction : VectorSharedAction { object OpenDrawer : HomeActivitySharedAction() object CloseDrawer : HomeActivitySharedAction() - data class OpenGroup(val clearFragment: Boolean) : HomeActivitySharedAction() + data class OpenGroup(val shouldClearFragment: Boolean) : HomeActivitySharedAction() + object CloseGroup : HomeActivitySharedAction() object AddSpace : HomeActivitySharedAction() data class OpenSpacePreview(val spaceId: String) : HomeActivitySharedAction() data class OpenSpaceInvite(val spaceId: String) : HomeActivitySharedAction() diff --git a/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt b/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt index 2753ba817d..9ab1ad60e7 100644 --- a/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt @@ -60,6 +60,11 @@ import org.matrix.android.sdk.api.session.group.model.GroupSummary import org.matrix.android.sdk.api.session.room.model.RoomSummary import javax.inject.Inject +/* + * TODO: + * 1) Change the hamburger menu to a back button when in a space + * 2) Make the back navigation button follow the same behaviour + */ class HomeDetailFragment @Inject constructor( private val avatarRenderer: AvatarRenderer, private val colorProvider: ColorProvider, @@ -130,12 +135,8 @@ class HomeDetailFragment @Inject constructor( viewModel.onEach(HomeDetailViewState::roomGroupingMethod) { roomGroupingMethod -> when (roomGroupingMethod) { - is RoomGroupingMethod.ByLegacyGroup -> { - onGroupChange(roomGroupingMethod.groupSummary) - } - is RoomGroupingMethod.BySpace -> { - onSpaceChange(roomGroupingMethod.spaceSummary) - } + is RoomGroupingMethod.ByLegacyGroup -> onGroupChange(roomGroupingMethod.groupSummary) + is RoomGroupingMethod.BySpace -> onSpaceChange(roomGroupingMethod.spaceSummary) } } @@ -147,6 +148,10 @@ class HomeDetailFragment @Inject constructor( updateTabVisibilitySafely(R.id.bottom_action_dial_pad, showDialPadTab) } + views.groupToolbarNavigateUp.setOnClickListener { + navigateUpOneParentSpace() + } + viewModel.observeViewEvents { viewEvent -> when (viewEvent) { HomeDetailViewEvents.CallStarted -> handleCallStarted() @@ -157,7 +162,6 @@ class HomeDetailFragment @Inject constructor( unknownDeviceDetectorSharedViewModel.onEach { state -> state.unknownSessions.invoke()?.let { unknownDevices -> -// Timber.v("## Detector Triggerred in fragment - ${unknownDevices.firstOrNull()}") if (unknownDevices.firstOrNull()?.currentSessionTrust == true) { val uid = "review_login" alertManager.cancelAlert(uid) @@ -190,6 +194,15 @@ class HomeDetailFragment @Inject constructor( } } + private fun navigateUpOneParentSpace() = with(appStateHandler) { + val parentId = when (val roomGroupingMethod = getCurrentRoomGroupingMethod()) { + is RoomGroupingMethod.BySpace -> roomGroupingMethod.spaceSummary?.flattenParentIds?.firstOrNull { it.isNotBlank() } + else -> null + } + setCurrentSpace(parentId) + sharedActionViewModel.post(HomeActivitySharedAction.CloseGroup) + } + private fun handleCallStarted() { dismissLoadingDialog() val fragmentTag = HomeTab.DialPad.toFragmentTag() @@ -203,20 +216,16 @@ class HomeDetailFragment @Inject constructor( override fun onResume() { super.onResume() - // update notification tab if needed updateTabVisibilitySafely(R.id.bottom_action_notification, vectorPreferences.labAddNotificationTab()) callManager.checkForProtocolsSupportIfNeeded() + refreshSpaceState() + } - // Current space/group is not live so at least refresh toolbar on resume - appStateHandler.getCurrentRoomGroupingMethod()?.let { roomGroupingMethod -> - when (roomGroupingMethod) { - is RoomGroupingMethod.ByLegacyGroup -> { - onGroupChange(roomGroupingMethod.groupSummary) - } - is RoomGroupingMethod.BySpace -> { - onSpaceChange(roomGroupingMethod.spaceSummary) - } - } + private fun refreshSpaceState() { + when (val roomGroupingMethod = appStateHandler.getCurrentRoomGroupingMethod()) { + is RoomGroupingMethod.ByLegacyGroup -> onGroupChange(roomGroupingMethod.groupSummary) + is RoomGroupingMethod.BySpace -> onSpaceChange(roomGroupingMethod.spaceSummary) + else -> Unit } } @@ -260,12 +269,12 @@ class HomeDetailFragment @Inject constructor( viewBinder = VerificationVectorAlert.ViewBinder(user, avatarRenderer) colorInt = colorProvider.getColorFromAttribute(R.attr.colorPrimary) contentAction = Runnable { - (weakCurrentActivity?.get() as? VectorBaseActivity<*>)?.let { + (weakCurrentActivity?.get() as? VectorBaseActivity<*>)?.let { activity -> // mark as ignored to avoid showing it again unknownDeviceDetectorSharedViewModel.handle( UnknownDeviceDetectorSharedViewModel.Action.IgnoreDevice(oldUnverified.mapNotNull { it.deviceId }) ) - it.navigator.openSettings(it, EXTRA_DIRECT_ACCESS_SECURITY_PRIVACY_MANAGE_SESSIONS) + activity.navigator.openSettings(activity, EXTRA_DIRECT_ACCESS_SECURITY_PRIVACY_MANAGE_SESSIONS) } } dismissedAction = Runnable { @@ -289,9 +298,13 @@ class HomeDetailFragment @Inject constructor( private fun onSpaceChange(spaceSummary: RoomSummary?) { if (spaceSummary == null) { views.groupToolbarSpaceTitleView.isVisible = false + views.groupToolbarAvatarImageView.isVisible = true + views.groupToolbarNavigateUp.isVisible = false } else { views.groupToolbarSpaceTitleView.isVisible = true views.groupToolbarSpaceTitleView.text = spaceSummary.displayName + views.groupToolbarAvatarImageView.isVisible = false + views.groupToolbarNavigateUp.isVisible = true } } @@ -324,11 +337,11 @@ class HomeDetailFragment @Inject constructor( withState(viewModel) { when (it.roomGroupingMethod) { is RoomGroupingMethod.ByLegacyGroup -> { - // nothing do far + // do nothing } is RoomGroupingMethod.BySpace -> { - it.roomGroupingMethod.spaceSummary?.let { - sharedActionViewModel.post(HomeActivitySharedAction.ShowSpaceSettings(it.roomId)) + it.roomGroupingMethod.spaceSummary?.let { spaceSummary -> + sharedActionViewModel.post(HomeActivitySharedAction.ShowSpaceSettings(spaceSummary.roomId)) } } } @@ -348,17 +361,6 @@ class HomeDetailFragment @Inject constructor( viewModel.handle(HomeDetailAction.SwitchTab(tab)) true } - -// val menuView = bottomNavigationView.getChildAt(0) as BottomNavigationMenuView - -// bottomNavigationView.getOrCreateBadge() -// menuView.forEachIndexed { index, view -> -// val itemView = view as BottomNavigationItemView -// val badgeLayout = LayoutInflater.from(requireContext()).inflate(R.layout.vector_home_badge_unread_layout, menuView, false) -// val unreadCounterBadgeView: UnreadCounterBadgeView = badgeLayout.findViewById(R.id.actionUnreadCounterBadgeView) -// itemView.addView(badgeLayout) -// unreadCounterBadgeViews.add(index, unreadCounterBadgeView) -// } } private fun updateUIForTab(tab: HomeTab) { diff --git a/vector/src/main/res/drawable/ic_arrow_back.xml b/vector/src/main/res/drawable/ic_arrow_back.xml new file mode 100644 index 0000000000..8452791cfd --- /dev/null +++ b/vector/src/main/res/drawable/ic_arrow_back.xml @@ -0,0 +1,5 @@ + + + diff --git a/vector/src/main/res/layout/fragment_home_detail.xml b/vector/src/main/res/layout/fragment_home_detail.xml index a53f001125..c734f55a4e 100644 --- a/vector/src/main/res/layout/fragment_home_detail.xml +++ b/vector/src/main/res/layout/fragment_home_detail.xml @@ -38,6 +38,7 @@ android:layout_height="40dp" android:layout_marginTop="8dp" android:layout_marginBottom="8dp" + android:visibility="gone" android:contentDescription="@string/a11y_open_drawer" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintStart_toStartOf="parent" @@ -74,6 +75,20 @@ + + - \ No newline at end of file + diff --git a/vector/src/main/res/values/strings.xml b/vector/src/main/res/values/strings.xml index 3e6bdad70b..c0421c7d6c 100644 --- a/vector/src/main/res/values/strings.xml +++ b/vector/src/main/res/values/strings.xml @@ -1777,6 +1777,7 @@ Send attachment Open the navigation drawer + Navigate up one space Open the create room menu Close the create room menu… Create a new direct conversation From bc500a567a96a1fe6611b18c29c344ec52f03a03 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Tue, 17 May 2022 11:28:37 +0200 Subject: [PATCH 03/44] Adds back button implementation of navigating up spaces --- .../app/features/home/HomeDetailFragment.kt | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt b/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt index 9ab1ad60e7..f7266db214 100644 --- a/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt @@ -33,6 +33,7 @@ import im.vector.app.R import im.vector.app.RoomGroupingMethod import im.vector.app.core.extensions.commitTransaction import im.vector.app.core.extensions.toMvRxBundle +import im.vector.app.core.platform.OnBackPressed import im.vector.app.core.platform.VectorBaseActivity import im.vector.app.core.platform.VectorBaseFragment import im.vector.app.core.resources.ColorProvider @@ -60,11 +61,6 @@ import org.matrix.android.sdk.api.session.group.model.GroupSummary import org.matrix.android.sdk.api.session.room.model.RoomSummary import javax.inject.Inject -/* - * TODO: - * 1) Change the hamburger menu to a back button when in a space - * 2) Make the back navigation button follow the same behaviour - */ class HomeDetailFragment @Inject constructor( private val avatarRenderer: AvatarRenderer, private val colorProvider: ColorProvider, @@ -74,7 +70,8 @@ class HomeDetailFragment @Inject constructor( private val appStateHandler: AppStateHandler ) : VectorBaseFragment(), KeysBackupBanner.Delegate, - CurrentCallsView.Callback { + CurrentCallsView.Callback, + OnBackPressed { private val viewModel: HomeDetailViewModel by fragmentViewModel() private val unknownDeviceDetectorSharedViewModel: UnknownDeviceDetectorSharedViewModel by activityViewModel() @@ -149,7 +146,7 @@ class HomeDetailFragment @Inject constructor( } views.groupToolbarNavigateUp.setOnClickListener { - navigateUpOneParentSpace() + navigateUpOneSpace() } viewModel.observeViewEvents { viewEvent -> @@ -194,15 +191,14 @@ class HomeDetailFragment @Inject constructor( } } - private fun navigateUpOneParentSpace() = with(appStateHandler) { - val parentId = when (val roomGroupingMethod = getCurrentRoomGroupingMethod()) { - is RoomGroupingMethod.BySpace -> roomGroupingMethod.spaceSummary?.flattenParentIds?.firstOrNull { it.isNotBlank() } - else -> null - } - setCurrentSpace(parentId) + private fun navigateUpOneSpace() { + val parentId = getCurrentSpace()?.flattenParentIds?.lastOrNull() + appStateHandler.setCurrentSpace(parentId) sharedActionViewModel.post(HomeActivitySharedAction.CloseGroup) } + private fun getCurrentSpace() = (appStateHandler.getCurrentRoomGroupingMethod() as? RoomGroupingMethod.BySpace)?.spaceSummary + private fun handleCallStarted() { dismissLoadingDialog() val fragmentTag = HomeTab.DialPad.toFragmentTag() @@ -438,7 +434,6 @@ class HomeDetailFragment @Inject constructor( } override fun invalidate() = withState(viewModel) { -// Timber.v(it.toString()) views.bottomNavigationView.getOrCreateBadge(R.id.bottom_action_people).render(it.notificationCountPeople, it.notificationHighlightPeople) views.bottomNavigationView.getOrCreateBadge(R.id.bottom_action_rooms).render(it.notificationCountRooms, it.notificationHighlightRooms) views.bottomNavigationView.getOrCreateBadge(R.id.bottom_action_notification).render(it.notificationCountCatchup, it.notificationHighlightCatchup) @@ -498,4 +493,11 @@ class HomeDetailFragment @Inject constructor( } return this } + + override fun onBackPressed(toolbarButton: Boolean) = if (getCurrentSpace() != null) { + navigateUpOneSpace() + true + } else { + false + } } From 520c3e474cf1a39dfb4cad36edfd87620bae6f47 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Tue, 17 May 2022 12:02:29 +0200 Subject: [PATCH 04/44] Adds changelog file --- changelog.d/6073.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6073.feature diff --git a/changelog.d/6073.feature b/changelog.d/6073.feature new file mode 100644 index 0000000000..295f45f3ce --- /dev/null +++ b/changelog.d/6073.feature @@ -0,0 +1 @@ +Adds up navigation in spaces From 4adaa20f728d72bb0847c9107d44c7f09458060d Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Tue, 17 May 2022 12:16:45 +0200 Subject: [PATCH 05/44] Fixes default visibilities in fragment_home_detail --- vector/src/main/res/layout/fragment_home_detail.xml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vector/src/main/res/layout/fragment_home_detail.xml b/vector/src/main/res/layout/fragment_home_detail.xml index c734f55a4e..301abef4b8 100644 --- a/vector/src/main/res/layout/fragment_home_detail.xml +++ b/vector/src/main/res/layout/fragment_home_detail.xml @@ -38,7 +38,6 @@ android:layout_height="40dp" android:layout_marginTop="8dp" android:layout_marginBottom="8dp" - android:visibility="gone" android:contentDescription="@string/a11y_open_drawer" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintStart_toStartOf="parent" @@ -82,7 +81,7 @@ android:src="@drawable/ic_arrow_back" android:layout_marginEnd="8dp" android:contentDescription="@string/a11y_navigate_up_space" - android:visibility="visible" + android:visibility="gone" app:tint="?vctr_content_secondary" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintStart_toStartOf="parent" From a0a7d3e7f6963aa27ab6a91c580ce580b0721226 Mon Sep 17 00:00:00 2001 From: ariskotsomitopoulos Date: Tue, 17 May 2022 16:28:30 +0300 Subject: [PATCH 06/44] Enhance reply attack to prevent DUPLICATED_MESSAGE_INDEX while decrypting the same event --- .../crypto/replay_attack/ReplayAttackTest.kt | 160 ------------------ .../crypto/replayattack/ReplayAttackTest.kt | 109 ++++++++++++ .../sync/handler/room/RoomSyncHandler.kt | 9 +- 3 files changed, 117 insertions(+), 161 deletions(-) delete mode 100644 matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replay_attack/ReplayAttackTest.kt create mode 100644 matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replayattack/ReplayAttackTest.kt diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replay_attack/ReplayAttackTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replay_attack/ReplayAttackTest.kt deleted file mode 100644 index 5c9892e264..0000000000 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replay_attack/ReplayAttackTest.kt +++ /dev/null @@ -1,160 +0,0 @@ -/* - * Copyright (c) 2022 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 org.matrix.android.sdk.internal.crypto.replay_attack - -import android.util.Log -import androidx.test.filters.LargeTest -import org.junit.Assert -import org.junit.Assert.assertEquals -import org.junit.FixMethodOrder -import org.junit.Test -import org.junit.runner.RunWith -import org.junit.runners.JUnit4 -import org.junit.runners.MethodSorters -import org.matrix.android.sdk.InstrumentedTest -import org.matrix.android.sdk.api.session.events.model.EventType -import org.matrix.android.sdk.api.session.events.model.toModel -import org.matrix.android.sdk.api.session.room.Room -import org.matrix.android.sdk.api.session.room.model.message.MessageContent -import org.matrix.android.sdk.api.session.room.send.SendState -import org.matrix.android.sdk.api.session.room.timeline.TimelineSettings -import org.matrix.android.sdk.common.CommonTestHelper -import org.matrix.android.sdk.common.CryptoTestHelper -import org.matrix.android.sdk.common.TestConstants - -@RunWith(JUnit4::class) -@FixMethodOrder(MethodSorters.JVM) -@LargeTest -class ReplayAttackTest : InstrumentedTest { - - @Test - fun replayAttackTest() { - val testHelper = CommonTestHelper(context()) - val cryptoTestHelper = CryptoTestHelper(testHelper) - val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoom(true) - - val e2eRoomID = cryptoTestData.roomId - - // Alice - val aliceSession = cryptoTestData.firstSession - val aliceRoomPOV = aliceSession.roomService().getRoom(e2eRoomID)!! - - // Bob - val bobSession = cryptoTestData.secondSession - val bobRoomPOV = bobSession!!.roomService().getRoom(e2eRoomID)!! - - assertEquals(bobRoomPOV.roomSummary()?.joinedMembersCount, 2) - Log.v("##REPLAY ATTACK", "Alice and Bob are in roomId: $e2eRoomID") - - - val sentEvents = testHelper.sendTextMessage(aliceRoomPOV, "Hello", 20) - -// val aliceMessageId: String? = sendMessageInRoom(aliceRoomPOV, "Hello Bob, I am Alice!", testHelper) - Assert.assertTrue("Message should be sent", sentEvents.size == 20) - Log.v("##REPLAY ATTACK", "Alice sent message to roomId: $e2eRoomID") - - // Bob should be able to decrypt the message -// testHelper.waitWithLatch { latch -> -// testHelper.retryPeriodicallyWithLatch(latch) { -// val timelineEvent = bobSession.roomService().getRoom(e2eRoomID)?.timelineService()?.getTimelineEvent(aliceMessageId!!) -// (timelineEvent != null && -// timelineEvent.isEncrypted() && -// timelineEvent.root.getClearType() == EventType.MESSAGE).also { -// if (it) { -// Log.v("#E2E TEST", "Bob can decrypt the message: ${timelineEvent?.root?.getDecryptedTextSummary()}") -// } -// } -// } -// } -// -// // Create a new user -// val arisSession = testHelper.createAccount("aris", SessionTestParams(true)) -// Log.v("#E2E TEST", "Aris user created") -// -// // Alice invites new user to the room -// testHelper.runBlockingTest { -// Log.v("#E2E TEST", "Alice invites ${arisSession.myUserId}") -// aliceRoomPOV.membershipService().invite(arisSession.myUserId) -// } -// -// waitForAndAcceptInviteInRoom(arisSession, e2eRoomID, testHelper) -// -// ensureMembersHaveJoined(aliceSession, arrayListOf(arisSession), e2eRoomID, testHelper) -// Log.v("#E2E TEST", "Aris has joined roomId: $e2eRoomID") -// -// when (roomHistoryVisibility) { -// RoomHistoryVisibility.WORLD_READABLE, -// RoomHistoryVisibility.SHARED, -// null -// -> { -// // Aris should be able to decrypt the message -// testHelper.waitWithLatch { latch -> -// testHelper.retryPeriodicallyWithLatch(latch) { -// val timelineEvent = arisSession.roomService().getRoom(e2eRoomID)?.timelineService()?.getTimelineEvent(aliceMessageId!!) -// (timelineEvent != null && -// timelineEvent.isEncrypted() && -// timelineEvent.root.getClearType() == EventType.MESSAGE -// ).also { -// if (it) { -// Log.v("#E2E TEST", "Aris can decrypt the message: ${timelineEvent?.root?.getDecryptedTextSummary()}") -// } -// } -// } -// } -// } -// RoomHistoryVisibility.INVITED, -// RoomHistoryVisibility.JOINED -> { -// // Aris should not even be able to get the message -// testHelper.waitWithLatch { latch -> -// testHelper.retryPeriodicallyWithLatch(latch) { -// val timelineEvent = arisSession.roomService().getRoom(e2eRoomID) -// ?.timelineService() -// ?.getTimelineEvent(aliceMessageId!!) -// timelineEvent == null -// } -// } -// } -// } - -// testHelper.signOutAndClose(arisSession) - cryptoTestData.cleanUp(testHelper) - } - - private fun sendMessageInRoom(aliceRoomPOV: Room, text: String, testHelper: CommonTestHelper): String? { - aliceRoomPOV.sendService().sendTextMessage(text) - var sentEventId: String? = null - testHelper.waitWithLatch(4 * TestConstants.timeOutMillis) { latch -> - val timeline = aliceRoomPOV.timelineService().createTimeline(null, TimelineSettings(60)) - timeline.start() - testHelper.retryPeriodicallyWithLatch(latch) { - val decryptedMsg = timeline.getSnapshot() - .filter { it.root.getClearType() == EventType.MESSAGE } - .also { list -> - val message = list.joinToString(",", "[", "]") { "${it.root.type}|${it.root.sendState}" } - Log.v("#E2E TEST", "Timeline snapshot is $message") - } - .filter { it.root.sendState == SendState.SYNCED } - .firstOrNull { it.root.getClearContent().toModel()?.body?.startsWith(text) == true } - sentEventId = decryptedMsg?.eventId - decryptedMsg != null - } - - timeline.dispose() - } - return sentEventId - } -} diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replayattack/ReplayAttackTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replayattack/ReplayAttackTest.kt new file mode 100644 index 0000000000..cb672f5e8d --- /dev/null +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replayattack/ReplayAttackTest.kt @@ -0,0 +1,109 @@ +/* + * Copyright (c) 2022 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 org.matrix.android.sdk.internal.crypto.replayattack + +import androidx.test.filters.LargeTest +import org.amshove.kluent.internal.assertFailsWith +import org.junit.Assert +import org.junit.Assert.assertEquals +import org.junit.FixMethodOrder +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import org.junit.runners.MethodSorters +import org.matrix.android.sdk.InstrumentedTest +import org.matrix.android.sdk.api.session.crypto.MXCryptoError +import org.matrix.android.sdk.common.CommonTestHelper +import org.matrix.android.sdk.common.CryptoTestHelper + +@RunWith(JUnit4::class) +@FixMethodOrder(MethodSorters.JVM) +@LargeTest +class ReplayAttackTest : InstrumentedTest { + + @Test + fun replayAttackAlreadyDecryptedEventTest() { + val testHelper = CommonTestHelper(context()) + val cryptoTestHelper = CryptoTestHelper(testHelper) + val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoom(true) + + val e2eRoomID = cryptoTestData.roomId + + // Alice + val aliceSession = cryptoTestData.firstSession + val aliceRoomPOV = aliceSession.roomService().getRoom(e2eRoomID)!! + + // Bob + val bobSession = cryptoTestData.secondSession + val bobRoomPOV = bobSession!!.roomService().getRoom(e2eRoomID)!! + assertEquals(bobRoomPOV.roomSummary()?.joinedMembersCount, 2) + + // Alice will send a message + val sentEvents = testHelper.sendTextMessage(aliceRoomPOV, "Hello I will be decrypted twice", 1) + Assert.assertTrue("Message should be sent", sentEvents.size == 1) + + val fakeEventId = sentEvents[0].eventId + "_fake" + val fakeEventWithTheSameIndex = + sentEvents[0].copy(eventId = fakeEventId, root = sentEvents[0].root.copy(eventId = fakeEventId)) + + testHelper.runBlockingTest { + // Lets assume we are from the main timelineId + val timelineId = "timelineId" + // Lets decrypt the original event + aliceSession.cryptoService().decryptEvent(sentEvents[0].root, timelineId) + // Lets decrypt the fake event that will have the same message index + val exception = assertFailsWith { + // An exception should be thrown while the same index would have been used for the previous decryption + aliceSession.cryptoService().decryptEvent(fakeEventWithTheSameIndex.root, timelineId) + } + assertEquals(MXCryptoError.ErrorType.DUPLICATED_MESSAGE_INDEX, exception.errorType) + } + cryptoTestData.cleanUp(testHelper) + } + + @Test + fun replayAttackSameEventTest() { + val testHelper = CommonTestHelper(context()) + val cryptoTestHelper = CryptoTestHelper(testHelper) + val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoom(true) + + val e2eRoomID = cryptoTestData.roomId + + // Alice + val aliceSession = cryptoTestData.firstSession + val aliceRoomPOV = aliceSession.roomService().getRoom(e2eRoomID)!! + + // Bob + val bobSession = cryptoTestData.secondSession + val bobRoomPOV = bobSession!!.roomService().getRoom(e2eRoomID)!! + assertEquals(bobRoomPOV.roomSummary()?.joinedMembersCount, 2) + + // Alice will send a message + val sentEvents = testHelper.sendTextMessage(aliceRoomPOV, "Hello I will be decrypted twice", 1) + Assert.assertTrue("Message should be sent", sentEvents.size == 1) + + testHelper.runBlockingTest { + // Lets assume we are from the main timelineId + val timelineId = "timelineId" + // Lets decrypt the original event + aliceSession.cryptoService().decryptEvent(sentEvents[0].root, timelineId) + // Lets try to decrypt the same event + aliceSession.cryptoService().decryptEvent(sentEvents[0].root, timelineId) + } + cryptoTestData.cleanUp(testHelper) + } +} diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt index a3be8b56a1..879bde1862 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt @@ -25,6 +25,7 @@ import org.matrix.android.sdk.api.session.crypto.MXCryptoError import org.matrix.android.sdk.api.session.crypto.model.OlmDecryptionResult import org.matrix.android.sdk.api.session.events.model.Event import org.matrix.android.sdk.api.session.events.model.EventType +import org.matrix.android.sdk.api.session.events.model.isThread import org.matrix.android.sdk.api.session.events.model.toModel import org.matrix.android.sdk.api.session.homeserver.HomeServerCapabilitiesService import org.matrix.android.sdk.api.session.initsync.InitSyncStep @@ -520,9 +521,10 @@ internal class RoomSyncHandler @Inject constructor( private fun decryptIfNeeded(event: Event, roomId: String) { try { + val timelineId = generateTimelineId(roomId, event) // Event from sync does not have roomId, so add it to the event first // note: runBlocking should be used here while we are in realm single thread executor, to avoid thread switching - val result = runBlocking { cryptoService.decryptEvent(event.copy(roomId = roomId), "") } + val result = runBlocking { cryptoService.decryptEvent(event.copy(roomId = roomId), timelineId) } event.mxDecryptionResult = OlmDecryptionResult( payload = result.clearEvent, senderKey = result.senderCurve25519Key, @@ -537,6 +539,11 @@ internal class RoomSyncHandler @Inject constructor( } } + private fun generateTimelineId(roomId: String, event: Event): String { + val threadIndicator = if (event.isThread()) "_thread_" else "_" + return "${RoomSyncHandler::class.java.simpleName}$threadIndicator$roomId" + } + data class EphemeralResult( val typingUserIds: List = emptyList() ) From abbc57429a7a38d9ff5e3e091ddbe93f39e03196 Mon Sep 17 00:00:00 2001 From: ariskotsomitopoulos Date: Tue, 17 May 2022 17:16:27 +0300 Subject: [PATCH 07/44] Add changelog --- changelog.d/6077.sdk | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6077.sdk diff --git a/changelog.d/6077.sdk b/changelog.d/6077.sdk new file mode 100644 index 0000000000..80310a28f5 --- /dev/null +++ b/changelog.d/6077.sdk @@ -0,0 +1 @@ +Improve replay attacks and reduce duplicate message index errors From b9adbb7d60bee6a09163c24678610206edc85f48 Mon Sep 17 00:00:00 2001 From: ariskotsomitopoulos Date: Wed, 18 May 2022 14:05:58 +0300 Subject: [PATCH 08/44] PR remarks --- .../crypto/replayattack/ReplayAttackTest.kt | 9 ++++++-- .../sdk/internal/crypto/MXOlmDevice.kt | 21 ++++++------------- .../sync/handler/room/RoomSyncHandler.kt | 8 +++---- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replayattack/ReplayAttackTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replayattack/ReplayAttackTest.kt index cb672f5e8d..69be4a3678 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replayattack/ReplayAttackTest.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replayattack/ReplayAttackTest.kt @@ -20,6 +20,7 @@ import androidx.test.filters.LargeTest import org.amshove.kluent.internal.assertFailsWith import org.junit.Assert import org.junit.Assert.assertEquals +import org.junit.Assert.fail import org.junit.FixMethodOrder import org.junit.Test import org.junit.runner.RunWith @@ -101,8 +102,12 @@ class ReplayAttackTest : InstrumentedTest { val timelineId = "timelineId" // Lets decrypt the original event aliceSession.cryptoService().decryptEvent(sentEvents[0].root, timelineId) - // Lets try to decrypt the same event - aliceSession.cryptoService().decryptEvent(sentEvents[0].root, timelineId) + try { + // Lets try to decrypt the same event + aliceSession.cryptoService().decryptEvent(sentEvents[0].root, timelineId) + } catch (ex: Throwable) { + fail("Shouldn't throw a decryption error for same event") + } } cryptoTestData.cleanUp(testHelper) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt index 16e6bb173d..87384b3fe2 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt @@ -96,10 +96,9 @@ internal class MXOlmDevice @Inject constructor( // So, store these message indexes per timeline id. // // The first level keys are timeline ids. - // The second level keys are strings of form "||" - private val inboundGroupSessionMessageIndexes: MutableMap> = HashMap() - - private val replayAttackMap: MutableMap = HashMap() + // The second level values is a Map that represents: + // "|||" --> eventId + private val inboundGroupSessionMessageIndexes: MutableMap> = HashMap() init { // Retrieve the account from the store @@ -798,15 +797,14 @@ internal class MXOlmDevice @Inject constructor( Timber.tag(loggerTag.value).d("## decryptGroupMessage() mIndex: ${decryptResult.mIndex}") if (timeline?.isNotBlank() == true) { - val timelineSet = inboundGroupSessionMessageIndexes.getOrPut(timeline) { mutableSetOf() } - if (timelineSet.contains(messageIndexKey) && messageIndexKey.alreadyUsed(eventId)) { + val replayAttackMap = inboundGroupSessionMessageIndexes.getOrPut(timeline) { mutableMapOf() } + if (replayAttackMap.contains(messageIndexKey) && replayAttackMap[messageIndexKey] != eventId) { val reason = String.format(MXCryptoError.DUPLICATE_MESSAGE_INDEX_REASON, decryptResult.mIndex) Timber.tag(loggerTag.value).e("## decryptGroupMessage() timelineId=$timeline: $reason") throw MXCryptoError.Base(MXCryptoError.ErrorType.DUPLICATED_MESSAGE_INDEX, reason) } - timelineSet.add(messageIndexKey) + replayAttackMap[messageIndexKey] = eventId } - replayAttackMap[messageIndexKey] = eventId inboundGroupSessionStore.storeInBoundGroupSession(sessionHolder, sessionId, senderKey) val payload = try { val adapter = MoshiProvider.providesMoshi().adapter(JSON_DICT_PARAMETERIZED_TYPE) @@ -825,13 +823,6 @@ internal class MXOlmDevice @Inject constructor( ) } - /** - * Determines whether or not the messageKey has already been used to decrypt another eventId - */ - private fun String.alreadyUsed(eventId: String): Boolean { - return replayAttackMap[this] != null && replayAttackMap[this] != eventId - } - /** * Reset replay attack data for the given timeline. * diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt index 879bde1862..cf916dc907 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt @@ -25,7 +25,6 @@ import org.matrix.android.sdk.api.session.crypto.MXCryptoError import org.matrix.android.sdk.api.session.crypto.model.OlmDecryptionResult import org.matrix.android.sdk.api.session.events.model.Event import org.matrix.android.sdk.api.session.events.model.EventType -import org.matrix.android.sdk.api.session.events.model.isThread import org.matrix.android.sdk.api.session.events.model.toModel import org.matrix.android.sdk.api.session.homeserver.HomeServerCapabilitiesService import org.matrix.android.sdk.api.session.initsync.InitSyncStep @@ -521,7 +520,7 @@ internal class RoomSyncHandler @Inject constructor( private fun decryptIfNeeded(event: Event, roomId: String) { try { - val timelineId = generateTimelineId(roomId, event) + val timelineId = generateTimelineId(roomId) // Event from sync does not have roomId, so add it to the event first // note: runBlocking should be used here while we are in realm single thread executor, to avoid thread switching val result = runBlocking { cryptoService.decryptEvent(event.copy(roomId = roomId), timelineId) } @@ -539,9 +538,8 @@ internal class RoomSyncHandler @Inject constructor( } } - private fun generateTimelineId(roomId: String, event: Event): String { - val threadIndicator = if (event.isThread()) "_thread_" else "_" - return "${RoomSyncHandler::class.java.simpleName}$threadIndicator$roomId" + private fun generateTimelineId(roomId: String): String { + return "${RoomSyncHandler::class.java.simpleName}$roomId" } data class EphemeralResult( From 5532e7dfa1c69969394cb7aa9f333b063c17218b Mon Sep 17 00:00:00 2001 From: ariskotsomitopoulos Date: Wed, 18 May 2022 14:57:50 +0300 Subject: [PATCH 09/44] Fix copyright --- .../sdk/internal/crypto/replayattack/ReplayAttackTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replayattack/ReplayAttackTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replayattack/ReplayAttackTest.kt index 69be4a3678..9932375ed2 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replayattack/ReplayAttackTest.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replayattack/ReplayAttackTest.kt @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 New Vector Ltd + * Copyright 2022 The Matrix.org Foundation C.I.C. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From 93264812c0cd07d93d26e1149adde918cba3a2a6 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 19 May 2022 10:20:46 +0200 Subject: [PATCH 10/44] Adds back navigation to spaces --- .../java/im/vector/app/AppStateHandler.kt | 14 ++++++++++--- .../app/features/home/HomeDetailFragment.kt | 21 +++++++++++++++---- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/vector/src/main/java/im/vector/app/AppStateHandler.kt b/vector/src/main/java/im/vector/app/AppStateHandler.kt index 1608d561bc..d44af53a55 100644 --- a/vector/src/main/java/im/vector/app/AppStateHandler.kt +++ b/vector/src/main/java/im/vector/app/AppStateHandler.kt @@ -72,6 +72,8 @@ class AppStateHandler @Inject constructor( val selectedRoomGroupingFlow = selectedSpaceDataSource.stream() + private val spaceBackstack = ArrayDeque() + fun getCurrentRoomGroupingMethod(): RoomGroupingMethod? { // XXX we should somehow make it live :/ just a work around // For example just after creating a space and switching to it the @@ -87,12 +89,16 @@ class AppStateHandler @Inject constructor( } } - fun setCurrentSpace(spaceId: String?, session: Session? = null, persistNow: Boolean = false) { + fun setCurrentSpace(spaceId: String?, session: Session? = null, persistNow: Boolean = false, isForwardNavigation: Boolean = true) { + val currentSpace = (selectedSpaceDataSource.currentValue?.orNull() as? RoomGroupingMethod.BySpace)?.space() val uSession = session ?: activeSessionHolder.getSafeActiveSession() ?: return - if (selectedSpaceDataSource.currentValue?.orNull() is RoomGroupingMethod.BySpace && - spaceId == selectedSpaceDataSource.currentValue?.orNull()?.space()?.roomId) return + if (currentSpace != null && spaceId == currentSpace.roomId) return val spaceSum = spaceId?.let { uSession.getRoomSummary(spaceId) } + if (isForwardNavigation) { + spaceBackstack.addLast(currentSpace?.roomId) + } + if (persistNow) { uiStateRepository.storeGroupingMethod(true, uSession.sessionId) uiStateRepository.storeSelectedSpace(spaceSum?.roomId, uSession.sessionId) @@ -151,6 +157,8 @@ class AppStateHandler @Inject constructor( }.launchIn(session.coroutineScope) } + fun getSpaceBackstack() = spaceBackstack + fun safeActiveSpaceId(): String? { return (selectedSpaceDataSource.currentValue?.orNull() as? RoomGroupingMethod.BySpace)?.spaceSummary?.roomId } diff --git a/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt b/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt index f7266db214..213c9fe193 100644 --- a/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt @@ -146,7 +146,7 @@ class HomeDetailFragment @Inject constructor( } views.groupToolbarNavigateUp.setOnClickListener { - navigateUpOneSpace() + navigateBack() } viewModel.observeViewEvents { viewEvent -> @@ -191,10 +191,23 @@ class HomeDetailFragment @Inject constructor( } } + private fun navigateBack() { + try { + val lastSpace = appStateHandler.getSpaceBackstack().removeLast() + setCurrentSpace(lastSpace) + } catch (e: NoSuchElementException) { + navigateUpOneSpace() + } + } + + private fun setCurrentSpace(spaceId: String?) { + appStateHandler.setCurrentSpace(spaceId, isForwardNavigation = false) + sharedActionViewModel.post(HomeActivitySharedAction.CloseGroup) + } + private fun navigateUpOneSpace() { val parentId = getCurrentSpace()?.flattenParentIds?.lastOrNull() - appStateHandler.setCurrentSpace(parentId) - sharedActionViewModel.post(HomeActivitySharedAction.CloseGroup) + setCurrentSpace(parentId) } private fun getCurrentSpace() = (appStateHandler.getCurrentRoomGroupingMethod() as? RoomGroupingMethod.BySpace)?.spaceSummary @@ -495,7 +508,7 @@ class HomeDetailFragment @Inject constructor( } override fun onBackPressed(toolbarButton: Boolean) = if (getCurrentSpace() != null) { - navigateUpOneSpace() + navigateBack() true } else { false From 2081e2c45b859e2e0fc3f66838d4f8ab154a3439 Mon Sep 17 00:00:00 2001 From: ariskotsomitopoulos Date: Fri, 20 May 2022 13:32:55 +0300 Subject: [PATCH 11/44] Reduce log visibility to verbose --- .../android/sdk/internal/crypto/MXOlmDevice.kt | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt index 87384b3fe2..a657d757fb 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt @@ -788,13 +788,13 @@ internal class MXOlmDevice @Inject constructor( } val messageIndexKey = senderKey + "|" + sessionId + "|" + roomId + "|" + decryptResult.mIndex - Timber.tag(loggerTag.value).d("##########################################################") - Timber.tag(loggerTag.value).d("## decryptGroupMessage() timeline: $timeline") - Timber.tag(loggerTag.value).d("## decryptGroupMessage() senderKey: $senderKey") - Timber.tag(loggerTag.value).d("## decryptGroupMessage() sessionId: $sessionId") - Timber.tag(loggerTag.value).d("## decryptGroupMessage() roomId: $roomId") - Timber.tag(loggerTag.value).d("## decryptGroupMessage() eventId: $eventId") - Timber.tag(loggerTag.value).d("## decryptGroupMessage() mIndex: ${decryptResult.mIndex}") + Timber.tag(loggerTag.value).v("##########################################################") + Timber.tag(loggerTag.value).v("## decryptGroupMessage() timeline: $timeline") + Timber.tag(loggerTag.value).v("## decryptGroupMessage() senderKey: $senderKey") + Timber.tag(loggerTag.value).v("## decryptGroupMessage() sessionId: $sessionId") + Timber.tag(loggerTag.value).v("## decryptGroupMessage() roomId: $roomId") + Timber.tag(loggerTag.value).v("## decryptGroupMessage() eventId: $eventId") + Timber.tag(loggerTag.value).v("## decryptGroupMessage() mIndex: ${decryptResult.mIndex}") if (timeline?.isNotBlank() == true) { val replayAttackMap = inboundGroupSessionMessageIndexes.getOrPut(timeline) { mutableMapOf() } From 7bc880e6bf88abfcd38f51d8d02e4ad92f488fa0 Mon Sep 17 00:00:00 2001 From: ariskotsomitopoulos Date: Fri, 20 May 2022 15:33:29 +0300 Subject: [PATCH 12/44] Improve documentation --- .../java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt index a657d757fb..841e41da73 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt @@ -756,6 +756,7 @@ internal class MXOlmDevice @Inject constructor( * @param body the base64-encoded body of the encrypted message. * @param roomId the room in which the message was received. * @param timeline the id of the timeline where the event is decrypted. It is used to prevent replay attack. + * @param eventId the eventId of the message that will be decrypted * @param sessionId the session identifier. * @param senderKey the base64-encoded curve25519 key of the sender. * @return the decrypting result. Nil if the sessionId is unknown. From 1bea28e27a99ea9cb7abff9496370e14f02d4ece Mon Sep 17 00:00:00 2001 From: ariskotsomitopoulos Date: Fri, 20 May 2022 16:03:39 +0300 Subject: [PATCH 13/44] fix documentation format --- .../matrix/android/sdk/internal/crypto/MXOlmDevice.kt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt index 37d9a9f567..1d25d82549 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt @@ -753,13 +753,13 @@ internal class MXOlmDevice @Inject constructor( /** * Decrypt a received message with an inbound group session. * - * @param body the base64-encoded body of the encrypted message. - * @param roomId the room in which the message was received. - * @param timeline the id of the timeline where the event is decrypted. It is used to prevent replay attack. - * @param eventId the eventId of the message that will be decrypted + * @param body the base64-encoded body of the encrypted message. + * @param roomId the room in which the message was received. + * @param timeline the id of the timeline where the event is decrypted. It is used to prevent replay attack. + * @param eventId the eventId of the message that will be decrypted * @param sessionId the session identifier. * @param senderKey the base64-encoded curve25519 key of the sender. - * @return the decrypting result. Nil if the sessionId is unknown. + * @return the decrypting result. Null if the sessionId is unknown. */ @Throws(MXCryptoError::class) suspend fun decryptGroupMessage(body: String, From 85f35929382ed9f29f709ca9ffe5b3510c4bb877 Mon Sep 17 00:00:00 2001 From: ariskotsomitopoulos Date: Fri, 20 May 2022 17:03:15 +0300 Subject: [PATCH 14/44] PR remarks --- .../sdk/internal/crypto/replayattack/ReplayAttackTest.kt | 3 ++- .../sdk/internal/session/sync/handler/room/RoomSyncHandler.kt | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replayattack/ReplayAttackTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replayattack/ReplayAttackTest.kt index 9932375ed2..09c340a14f 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replayattack/ReplayAttackTest.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/replayattack/ReplayAttackTest.kt @@ -55,7 +55,7 @@ class ReplayAttackTest : InstrumentedTest { // Alice will send a message val sentEvents = testHelper.sendTextMessage(aliceRoomPOV, "Hello I will be decrypted twice", 1) - Assert.assertTrue("Message should be sent", sentEvents.size == 1) + assertEquals(1, sentEvents.size) val fakeEventId = sentEvents[0].eventId + "_fake" val fakeEventWithTheSameIndex = @@ -96,6 +96,7 @@ class ReplayAttackTest : InstrumentedTest { // Alice will send a message val sentEvents = testHelper.sendTextMessage(aliceRoomPOV, "Hello I will be decrypted twice", 1) Assert.assertTrue("Message should be sent", sentEvents.size == 1) + assertEquals(sentEvents.size, 1) testHelper.runBlockingTest { // Lets assume we are from the main timelineId diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt index df03b24a89..53fc9dc6b9 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt @@ -539,7 +539,7 @@ internal class RoomSyncHandler @Inject constructor( } private fun generateTimelineId(roomId: String): String { - return "${RoomSyncHandler::class.java.simpleName}$roomId" + return "RoomSyncHandler$roomId" } data class EphemeralResult( From a07c1b8afa9f54629b7267ab7dc3f0e05256a665 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Mon, 23 May 2022 13:46:53 +0100 Subject: [PATCH 15/44] Removes top bar back arrow --- .../vector/app/features/home/HomeDetailFragment.kt | 8 -------- .../src/main/res/layout/fragment_home_detail.xml | 14 -------------- 2 files changed, 22 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt b/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt index 213c9fe193..4eedb528d1 100644 --- a/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt @@ -145,10 +145,6 @@ class HomeDetailFragment @Inject constructor( updateTabVisibilitySafely(R.id.bottom_action_dial_pad, showDialPadTab) } - views.groupToolbarNavigateUp.setOnClickListener { - navigateBack() - } - viewModel.observeViewEvents { viewEvent -> when (viewEvent) { HomeDetailViewEvents.CallStarted -> handleCallStarted() @@ -307,13 +303,9 @@ class HomeDetailFragment @Inject constructor( private fun onSpaceChange(spaceSummary: RoomSummary?) { if (spaceSummary == null) { views.groupToolbarSpaceTitleView.isVisible = false - views.groupToolbarAvatarImageView.isVisible = true - views.groupToolbarNavigateUp.isVisible = false } else { views.groupToolbarSpaceTitleView.isVisible = true views.groupToolbarSpaceTitleView.text = spaceSummary.displayName - views.groupToolbarAvatarImageView.isVisible = false - views.groupToolbarNavigateUp.isVisible = true } } diff --git a/vector/src/main/res/layout/fragment_home_detail.xml b/vector/src/main/res/layout/fragment_home_detail.xml index 301abef4b8..e3db68434e 100644 --- a/vector/src/main/res/layout/fragment_home_detail.xml +++ b/vector/src/main/res/layout/fragment_home_detail.xml @@ -74,20 +74,6 @@ - - Date: Fri, 29 Apr 2022 11:55:43 +0100 Subject: [PATCH 16/44] adding feature flag and redirecting the sign in flow - temporarily to the combined register fragment --- .../features/debug/features/DebugFeaturesStateFactory.kt | 5 +++++ .../app/features/debug/features/DebugVectorFeatures.kt | 4 ++++ .../src/main/java/im/vector/app/features/VectorFeatures.kt | 2 ++ .../vector/app/features/onboarding/OnboardingViewEvents.kt | 1 + .../vector/app/features/onboarding/OnboardingViewModel.kt | 4 +++- .../app/features/onboarding/ftueauth/FtueAuthVariant.kt | 7 ++++++- 6 files changed, 21 insertions(+), 2 deletions(-) diff --git a/vector/src/debug/java/im/vector/app/features/debug/features/DebugFeaturesStateFactory.kt b/vector/src/debug/java/im/vector/app/features/debug/features/DebugFeaturesStateFactory.kt index 00a073f832..aa4df5e308 100644 --- a/vector/src/debug/java/im/vector/app/features/debug/features/DebugFeaturesStateFactory.kt +++ b/vector/src/debug/java/im/vector/app/features/debug/features/DebugFeaturesStateFactory.kt @@ -60,6 +60,11 @@ class DebugFeaturesStateFactory @Inject constructor( key = DebugFeatureKeys.onboardingCombinedRegister, factory = VectorFeatures::isOnboardingCombinedRegisterEnabled ), + createBooleanFeature( + label = "FTUE Combined login", + key = DebugFeatureKeys.onboardingCombinedLogin, + factory = VectorFeatures::isOnboardingCombinedLoginEnabled + ), ) ) } diff --git a/vector/src/debug/java/im/vector/app/features/debug/features/DebugVectorFeatures.kt b/vector/src/debug/java/im/vector/app/features/debug/features/DebugVectorFeatures.kt index 1bc37ff97e..f36b1a804a 100644 --- a/vector/src/debug/java/im/vector/app/features/debug/features/DebugVectorFeatures.kt +++ b/vector/src/debug/java/im/vector/app/features/debug/features/DebugVectorFeatures.kt @@ -57,6 +57,9 @@ class DebugVectorFeatures( override fun isOnboardingCombinedRegisterEnabled(): Boolean = read(DebugFeatureKeys.onboardingCombinedRegister) ?: vectorFeatures.isOnboardingCombinedRegisterEnabled() + override fun isOnboardingCombinedLoginEnabled(): Boolean = read(DebugFeatureKeys.onboardingCombinedLogin) + ?: vectorFeatures.isOnboardingCombinedLoginEnabled() + override fun isScreenSharingEnabled(): Boolean = read(DebugFeatureKeys.screenSharing) ?: vectorFeatures.isScreenSharingEnabled() @@ -113,6 +116,7 @@ object DebugFeatureKeys { val onboardingUseCase = booleanPreferencesKey("onboarding-splash-carousel") val onboardingPersonalize = booleanPreferencesKey("onboarding-personalize") val onboardingCombinedRegister = booleanPreferencesKey("onboarding-combined-register") + val onboardingCombinedLogin = booleanPreferencesKey("onboarding-combined-login") val liveLocationSharing = booleanPreferencesKey("live-location-sharing") val screenSharing = booleanPreferencesKey("screen-sharing") } diff --git a/vector/src/main/java/im/vector/app/features/VectorFeatures.kt b/vector/src/main/java/im/vector/app/features/VectorFeatures.kt index e3fded2824..6a7a0865de 100644 --- a/vector/src/main/java/im/vector/app/features/VectorFeatures.kt +++ b/vector/src/main/java/im/vector/app/features/VectorFeatures.kt @@ -26,6 +26,7 @@ interface VectorFeatures { fun isOnboardingUseCaseEnabled(): Boolean fun isOnboardingPersonalizeEnabled(): Boolean fun isOnboardingCombinedRegisterEnabled(): Boolean + fun isOnboardingCombinedLoginEnabled(): Boolean fun isScreenSharingEnabled(): Boolean enum class OnboardingVariant { @@ -42,5 +43,6 @@ class DefaultVectorFeatures : VectorFeatures { override fun isOnboardingUseCaseEnabled() = true override fun isOnboardingPersonalizeEnabled() = false override fun isOnboardingCombinedRegisterEnabled() = false + override fun isOnboardingCombinedLoginEnabled() = false override fun isScreenSharingEnabled(): Boolean = true } diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewEvents.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewEvents.kt index 6ffece4ab6..5dbcd162f3 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewEvents.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewEvents.kt @@ -37,6 +37,7 @@ sealed class OnboardingViewEvents : VectorViewEvents { object OpenUseCaseSelection : OnboardingViewEvents() object OpenServerSelection : OnboardingViewEvents() object OpenCombinedRegister : OnboardingViewEvents() + object OpenCombinedLogin : OnboardingViewEvents() object EditServerSelection : OnboardingViewEvents() data class OnServerSelectionDone(val serverType: ServerType) : OnboardingViewEvents() object OnLoginFlowRetrieved : OnboardingViewEvents() diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt index cf730a0266..4ee26c2976 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt @@ -194,7 +194,9 @@ class OnboardingViewModel @AssistedInject constructor( } else { OnboardingViewEvents.OpenServerSelection } - OnboardingFlow.SignIn, + OnboardingFlow.SignIn -> if (vectorFeatures.isOnboardingCombinedRegisterEnabled()) { + OnboardingViewEvents.OpenCombinedLogin + } else OnboardingViewEvents.OpenServerSelection OnboardingFlow.SignInSignUp -> OnboardingViewEvents.OpenServerSelection } _viewEvents.post(nextOnboardingStep) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt index 8430b483d2..d7b8171225 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt @@ -227,10 +227,15 @@ class FtueAuthVariant( option = commonOption ) } - OnboardingViewEvents.OnHomeserverEdited -> activity.popBackstack() + OnboardingViewEvents.OnHomeserverEdited -> activity.popBackstack() + OnboardingViewEvents.OpenCombinedLogin -> onStartCombinedLogin() } } + private fun onStartCombinedLogin() { + addRegistrationStageFragmentToBackstack(FtueAuthCombinedRegisterFragment::class.java) + } + private fun onRegistrationFlow(viewEvents: OnboardingViewEvents.RegistrationFlowResult) { when { registrationShouldFallback(viewEvents) -> displayFallbackWebDialog() From 4b6f74364d68e09265eb1ecb4d6665ba6d75a230 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 5 May 2022 14:01:18 +0100 Subject: [PATCH 17/44] adding dedicated login action --- .../features/onboarding/OnboardingAction.kt | 1 + .../onboarding/OnboardingViewModel.kt | 28 +- .../ftueauth/FtueAuthCombinedLoginFragment.kt | 198 +++++++++++++ .../onboarding/ftueauth/FtueAuthVariant.kt | 2 +- .../layout/fragment_ftue_combined_login.xml | 275 ++++++++++++++++++ vector/src/main/res/values/donottranslate.xml | 2 + 6 files changed, 493 insertions(+), 13 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedLoginFragment.kt create mode 100644 vector/src/main/res/layout/fragment_ftue_combined_login.xml diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingAction.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingAction.kt index 9f7dce56ea..d5b12a8071 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingAction.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingAction.kt @@ -49,6 +49,7 @@ sealed interface OnboardingAction : VectorViewModelAction { // Login or Register, depending on the signMode data class LoginOrRegister(val username: String, val password: String, val initialDeviceName: String) : OnboardingAction data class Register(val username: String, val password: String, val initialDeviceName: String) : OnboardingAction + data class Login(val username: String, val password: String, val initialDeviceName: String) : OnboardingAction object StopEmailValidationCheck : OnboardingAction data class PostRegisterAction(val registerAction: RegisterAction) : OnboardingAction diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt index 4ee26c2976..1ca2556c71 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt @@ -141,6 +141,7 @@ class OnboardingViewModel @AssistedInject constructor( is OnboardingAction.HomeServerChange -> withAction(action) { handleHomeserverChange(action) } is OnboardingAction.LoginOrRegister -> handleLoginOrRegister(action).also { lastAction = action } is OnboardingAction.Register -> handleRegisterWith(action).also { lastAction = action } + is OnboardingAction.Login -> handleLogin(action).also { lastAction = action } is OnboardingAction.LoginWithToken -> handleLoginWithToken(action) is OnboardingAction.WebLoginSuccess -> handleWebLoginSuccess(action) is OnboardingAction.ResetPassword -> handleResetPassword(action) @@ -188,18 +189,21 @@ class OnboardingViewModel @AssistedInject constructor( } private fun continueToPageAfterSplash(onboardingFlow: OnboardingFlow) { - val nextOnboardingStep = when (onboardingFlow) { - OnboardingFlow.SignUp -> if (vectorFeatures.isOnboardingUseCaseEnabled()) { - OnboardingViewEvents.OpenUseCaseSelection - } else { - OnboardingViewEvents.OpenServerSelection + when (onboardingFlow) { + OnboardingFlow.SignUp -> { + _viewEvents.post( + if (vectorFeatures.isOnboardingUseCaseEnabled()) { + OnboardingViewEvents.OpenUseCaseSelection + } else { + OnboardingViewEvents.OpenServerSelection + } + ) } - OnboardingFlow.SignIn -> if (vectorFeatures.isOnboardingCombinedRegisterEnabled()) { - OnboardingViewEvents.OpenCombinedLogin - } else OnboardingViewEvents.OpenServerSelection - OnboardingFlow.SignInSignUp -> OnboardingViewEvents.OpenServerSelection + OnboardingFlow.SignIn -> if (vectorFeatures.isOnboardingCombinedLoginEnabled()) { + handle(OnboardingAction.HomeServerChange.SelectHomeServer(defaultHomeserverUrl)) + } else _viewEvents.post(OnboardingViewEvents.OpenServerSelection) + OnboardingFlow.SignInSignUp -> _viewEvents.post(OnboardingViewEvents.OpenServerSelection) } - _viewEvents.post(nextOnboardingStep) } private fun handleUserAcceptCertificate(action: OnboardingAction.UserAcceptCertificate) { @@ -487,7 +491,7 @@ class OnboardingViewModel @AssistedInject constructor( private fun handleLoginOrRegister(action: OnboardingAction.LoginOrRegister) = withState { state -> when (state.signMode) { SignMode.Unknown -> error("Developer error, invalid sign mode") - SignMode.SignIn -> handleLogin(action) + SignMode.SignIn -> handleLogin(OnboardingAction.Login(action.username, action.password, action.initialDeviceName)) SignMode.SignUp -> handleRegisterWith(OnboardingAction.Register(action.username, action.password, action.initialDeviceName)) SignMode.SignInWithMatrixId -> handleDirectLogin(action, null) } @@ -506,7 +510,7 @@ class OnboardingViewModel @AssistedInject constructor( } } - private fun handleLogin(action: OnboardingAction.LoginOrRegister) { + private fun handleLogin(action: OnboardingAction.Login) { val safeLoginWizard = loginWizard if (safeLoginWizard == null) { diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedLoginFragment.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedLoginFragment.kt new file mode 100644 index 0000000000..66c7199acb --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthCombinedLoginFragment.kt @@ -0,0 +1,198 @@ +/* + * Copyright 2019 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 im.vector.app.features.onboarding.ftueauth + +import android.os.Build +import android.os.Bundle +import android.view.LayoutInflater +import android.view.View +import android.view.ViewGroup +import android.view.inputmethod.EditorInfo +import androidx.autofill.HintConstants +import androidx.core.text.isDigitsOnly +import androidx.core.view.isVisible +import androidx.lifecycle.lifecycleScope +import com.airbnb.mvrx.withState +import im.vector.app.R +import im.vector.app.core.extensions.content +import im.vector.app.core.extensions.editText +import im.vector.app.core.extensions.hasContentFlow +import im.vector.app.core.extensions.hasSurroundingSpaces +import im.vector.app.core.extensions.hideKeyboard +import im.vector.app.core.extensions.hidePassword +import im.vector.app.core.extensions.realignPercentagesToParent +import im.vector.app.core.extensions.toReducedUrl +import im.vector.app.databinding.FragmentFtueCombinedLoginBinding +import im.vector.app.features.login.LoginMode +import im.vector.app.features.login.SSORedirectRouterActivity +import im.vector.app.features.login.SocialLoginButtonsView +import im.vector.app.features.onboarding.OnboardingAction +import im.vector.app.features.onboarding.OnboardingViewEvents +import im.vector.app.features.onboarding.OnboardingViewState +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach +import org.matrix.android.sdk.api.auth.data.SsoIdentityProvider +import org.matrix.android.sdk.api.failure.isInvalidPassword +import org.matrix.android.sdk.api.failure.isInvalidUsername +import org.matrix.android.sdk.api.failure.isLoginEmailUnknown +import javax.inject.Inject + +class FtueAuthCombinedLoginFragment @Inject constructor() : AbstractSSOFtueAuthFragment() { + + override fun getBinding(inflater: LayoutInflater, container: ViewGroup?): FragmentFtueCombinedLoginBinding { + return FragmentFtueCombinedLoginBinding.inflate(inflater, container, false) + } + + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + super.onViewCreated(view, savedInstanceState) + setupSubmitButton() + views.createAccountRoot.realignPercentagesToParent() + views.editServerButton.debouncedClicks { + viewModel.handle(OnboardingAction.PostViewEvent(OnboardingViewEvents.EditServerSelection)) + } + + views.createAccountPasswordInput.editText().setOnEditorActionListener { _, actionId, _ -> + if (actionId == EditorInfo.IME_ACTION_DONE) { + submit() + return@setOnEditorActionListener true + } + return@setOnEditorActionListener false + } + } + + private fun setupSubmitButton() { + views.createAccountSubmit.setOnClickListener { submit() } + observeInputFields() + .onEach { + views.createAccountPasswordInput.error = null + views.createAccountInput.error = null + views.createAccountSubmit.isEnabled = it + } + .launchIn(viewLifecycleOwner.lifecycleScope) + } + + private fun observeInputFields() = combine( + views.createAccountInput.hasContentFlow { it.trim() }, + views.createAccountPasswordInput.hasContentFlow(), + transform = { isLoginNotEmpty, isPasswordNotEmpty -> isLoginNotEmpty && isPasswordNotEmpty } + ) + + private fun submit() { + withState(viewModel) { state -> + cleanupUi() + + val login = views.createAccountInput.content() + val password = views.createAccountPasswordInput.content() + + // This can be called by the IME action, so deal with empty cases + var error = 0 + if (login.isEmpty()) { + views.createAccountInput.error = getString(R.string.error_empty_field_choose_user_name) + error++ + } + if (state.isNumericOnlyUserIdForbidden() && login.isDigitsOnly()) { + views.createAccountInput.error = getString(R.string.error_forbidden_digits_only_username) + error++ + } + if (password.isEmpty()) { + views.createAccountPasswordInput.error = getString(R.string.error_empty_field_choose_password) + error++ + } + + if (error == 0) { + viewModel.handle(OnboardingAction.Login(login, password, getString(R.string.login_default_session_public_name))) + } + } + } + + private fun cleanupUi() { + views.createAccountSubmit.hideKeyboard() + views.createAccountInput.error = null + views.createAccountPasswordInput.error = null + } + + override fun resetViewModel() { + viewModel.handle(OnboardingAction.ResetAuthenticationAttempt) + } + + override fun onError(throwable: Throwable) { + // Trick to display the error without text. + views.createAccountInput.error = " " + when { + throwable.isInvalidUsername() -> { + views.createAccountInput.error = errorFormatter.toHumanReadable(throwable) + } + throwable.isLoginEmailUnknown() -> { + views.createAccountInput.error = getString(R.string.login_login_with_email_error) + } + throwable.isInvalidPassword() && views.createAccountPasswordInput.hasSurroundingSpaces() -> { + views.createAccountPasswordInput.error = getString(R.string.auth_invalid_login_param_space_in_password) + } + else -> { + super.onError(throwable) + } + } + } + + override fun updateWithState(state: OnboardingViewState) { + setupUi(state) + setupAutoFill() + + views.selectedServerName.text = state.selectedHomeserver.userFacingUrl.toReducedUrl() + views.selectedServerDescription.text = state.selectedHomeserver.description + + if (state.isLoading) { + // Ensure password is hidden + views.createAccountPasswordInput.editText().hidePassword() + } + } + + private fun setupUi(state: OnboardingViewState) { + when (state.selectedHomeserver.preferredLoginMode) { + is LoginMode.SsoAndPassword -> renderSsoProviders(state.deviceId, state.selectedHomeserver.preferredLoginMode.ssoIdentityProviders) + else -> hideSsoProviders() + } + } + + private fun renderSsoProviders(deviceId: String?, ssoProviders: List?) { + views.ssoGroup.isVisible = ssoProviders?.isNotEmpty() == true + views.ssoButtons.mode = SocialLoginButtonsView.Mode.MODE_CONTINUE + views.ssoButtons.ssoIdentityProviders = ssoProviders?.sorted() + views.ssoButtons.listener = SocialLoginButtonsView.InteractionListener { id -> + viewModel.getSsoUrl( + redirectUrl = SSORedirectRouterActivity.VECTOR_REDIRECT_URL, + deviceId = deviceId, + providerId = id + )?.let { openInCustomTab(it) } + } + } + + private fun hideSsoProviders() { + views.ssoGroup.isVisible = false + views.ssoButtons.ssoIdentityProviders = null + } + + private fun setupAutoFill() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + views.createAccountInput.setAutofillHints(HintConstants.AUTOFILL_HINT_NEW_USERNAME) + views.createAccountPasswordInput.setAutofillHints(HintConstants.AUTOFILL_HINT_NEW_PASSWORD) + } + } + + private fun OnboardingViewState.isNumericOnlyUserIdForbidden() = selectedHomeserver.userFacingUrl == getString(R.string.matrix_org_server_url) +} diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt index d7b8171225..5ad6b7e78d 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt @@ -233,7 +233,7 @@ class FtueAuthVariant( } private fun onStartCombinedLogin() { - addRegistrationStageFragmentToBackstack(FtueAuthCombinedRegisterFragment::class.java) + addRegistrationStageFragmentToBackstack(FtueAuthCombinedLoginFragment::class.java) } private fun onRegistrationFlow(viewEvents: OnboardingViewEvents.RegistrationFlowResult) { diff --git a/vector/src/main/res/layout/fragment_ftue_combined_login.xml b/vector/src/main/res/layout/fragment_ftue_combined_login.xml new file mode 100644 index 0000000000..684d6cf671 --- /dev/null +++ b/vector/src/main/res/layout/fragment_ftue_combined_login.xml @@ -0,0 +1,275 @@ + + + + + + + + + + + + + + + + + + + + + + + +