From 8880eeda407aac4c587552d61e9318bccce6e959 Mon Sep 17 00:00:00 2001 From: ganfra Date: Wed, 29 Jan 2025 14:16:57 +0100 Subject: [PATCH] feat(security&privacy) : improve and document code after PR review. --- .../SecurityAndPrivacyPresenter.kt | 26 +++++++++++-------- .../SecurityAndPrivacyState.kt | 2 +- .../SecurityAndPrivacyStateProvider.kt | 2 +- .../SecurityAndPrivacyView.kt | 6 +++-- .../EditRoomAddressPresenter.kt | 10 +++---- .../SecurityAndPrivacyPresenterTest.kt | 12 ++++----- .../libraries/matrix/api/room/MatrixRoom.kt | 4 +-- 7 files changed, 32 insertions(+), 30 deletions(-) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyPresenter.kt index 199cd607f2..769ff87bae 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyPresenter.kt @@ -96,7 +96,7 @@ class SecurityAndPrivacyPresenter @AssistedInject constructor( address = savedSettings.address, ) - var showEncryptionConfirmation by remember(savedSettings.isEncrypted) { mutableStateOf(false) } + var showEnableEncryptionConfirmation by remember(savedSettings.isEncrypted) { mutableStateOf(false) } val permissions by room.securityAndPrivacyPermissionsAsState(syncUpdateFlow.value) fun handleEvents(event: SecurityAndPrivacyEvents) { @@ -116,7 +116,7 @@ class SecurityAndPrivacyPresenter @AssistedInject constructor( if (editedIsEncrypted) { editedIsEncrypted = false } else { - showEncryptionConfirmation = true + showEnableEncryptionConfirmation = true } } is SecurityAndPrivacyEvents.ChangeHistoryVisibility -> { @@ -130,10 +130,10 @@ class SecurityAndPrivacyPresenter @AssistedInject constructor( } SecurityAndPrivacyEvents.EditRoomAddress -> navigator.openEditRoomAddress() SecurityAndPrivacyEvents.CancelEnableEncryption -> { - showEncryptionConfirmation = false + showEnableEncryptionConfirmation = false } SecurityAndPrivacyEvents.ConfirmEnableEncryption -> { - showEncryptionConfirmation = false + showEnableEncryptionConfirmation = false editedIsEncrypted = true } SecurityAndPrivacyEvents.DismissSaveError -> { @@ -146,7 +146,7 @@ class SecurityAndPrivacyPresenter @AssistedInject constructor( savedSettings = savedSettings, editedSettings = editedSettings, homeserverName = homeserverName, - showEncryptionConfirmation = showEncryptionConfirmation, + showEnableEncryptionConfirmation = showEnableEncryptionConfirmation, saveAction = saveAction.value, permissions = permissions, eventSink = ::handleEvents @@ -189,8 +189,9 @@ class SecurityAndPrivacyPresenter @AssistedInject constructor( } } val updateJoinRule = async { - if (editedSettings.roomAccess != savedSettings.roomAccess) { - room.updateJoinRule(editedSettings.roomAccess.map()) + val joinRule = editedSettings.roomAccess.map() + if (editedSettings.roomAccess != savedSettings.roomAccess && joinRule != null) { + room.updateJoinRule(joinRule) } else { Result.success(Unit) } @@ -239,19 +240,21 @@ private fun JoinRule?.map(): SecurityAndPrivacyRoomAccess { JoinRule.Public -> SecurityAndPrivacyRoomAccess.Anyone JoinRule.Knock, is JoinRule.KnockRestricted -> SecurityAndPrivacyRoomAccess.AskToJoin is JoinRule.Restricted -> SecurityAndPrivacyRoomAccess.SpaceMember + JoinRule.Invite -> SecurityAndPrivacyRoomAccess.InviteOnly + // All other cases are not supported so we default to InviteOnly is JoinRule.Custom, - JoinRule.Invite, JoinRule.Private, null -> SecurityAndPrivacyRoomAccess.InviteOnly } } -private fun SecurityAndPrivacyRoomAccess.map(): JoinRule { +private fun SecurityAndPrivacyRoomAccess.map(): JoinRule? { return when (this) { SecurityAndPrivacyRoomAccess.Anyone -> JoinRule.Public SecurityAndPrivacyRoomAccess.AskToJoin -> JoinRule.Knock SecurityAndPrivacyRoomAccess.InviteOnly -> JoinRule.Private - SecurityAndPrivacyRoomAccess.SpaceMember -> error("Unsupported") + // SpaceMember can't be selected in the ui + SecurityAndPrivacyRoomAccess.SpaceMember -> null } } @@ -260,7 +263,8 @@ private fun RoomHistoryVisibility?.map(): SecurityAndPrivacyHistoryVisibility { RoomHistoryVisibility.WorldReadable -> SecurityAndPrivacyHistoryVisibility.Anyone RoomHistoryVisibility.Joined, RoomHistoryVisibility.Invited -> SecurityAndPrivacyHistoryVisibility.SinceInvite - RoomHistoryVisibility.Shared, + RoomHistoryVisibility.Shared -> SecurityAndPrivacyHistoryVisibility.SinceSelection + // All other cases are not supported so we default to SinceSelection is RoomHistoryVisibility.Custom, null -> SecurityAndPrivacyHistoryVisibility.SinceSelection } diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyState.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyState.kt index 75b53bbd58..eb22ec2597 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyState.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyState.kt @@ -18,7 +18,7 @@ data class SecurityAndPrivacyState( // the settings the user wants to apply. val editedSettings: SecurityAndPrivacySettings, val homeserverName: String, - val showEncryptionConfirmation: Boolean, + val showEnableEncryptionConfirmation: Boolean, val saveAction: AsyncAction, private val permissions: SecurityAndPrivacyPermissions, val eventSink: (SecurityAndPrivacyEvents) -> Unit diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyStateProvider.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyStateProvider.kt index 7fe1fea52f..907424108f 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyStateProvider.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyStateProvider.kt @@ -79,7 +79,7 @@ fun aSecurityAndPrivacyState( editedSettings = editedSettings, savedSettings = savedSettings, homeserverName = homeserverName, - showEncryptionConfirmation = showEncryptionConfirmation, + showEnableEncryptionConfirmation = showEncryptionConfirmation, saveAction = saveAction, permissions = permissions, eventSink = eventSink diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyView.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyView.kt index c58687fde4..59dfeeaca0 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyView.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/SecurityAndPrivacyView.kt @@ -100,9 +100,10 @@ fun SecurityAndPrivacyView( if (state.showEncryptionSection) { EncryptionSection( isRoomEncrypted = state.editedSettings.isEncrypted, + // encryption can't be disabled once enabled canToggleEncryption = !state.savedSettings.isEncrypted, onToggleEncryption = { state.eventSink(SecurityAndPrivacyEvents.ToggleEncryptionState) }, - showConfirmation = state.showEncryptionConfirmation, + showConfirmation = state.showEnableEncryptionConfirmation, onDismissConfirmation = { state.eventSink(SecurityAndPrivacyEvents.CancelEnableEncryption) }, onConfirmEncryption = { state.eventSink(SecurityAndPrivacyEvents.ConfirmEnableEncryption) }, ) @@ -206,6 +207,7 @@ private fun RoomAccessSection( trailingContent = ListItemContent.RadioButton(selected = edited == SecurityAndPrivacyRoomAccess.Anyone), onClick = { onSelectOption(SecurityAndPrivacyRoomAccess.Anyone) }, ) + // Show space member option, but disabled as we don't support this option for now. if (saved == SecurityAndPrivacyRoomAccess.SpaceMember) { ListItem( headlineContent = { Text(text = stringResource(R.string.screen_security_and_privacy_room_access_space_members_option_title)) }, @@ -342,7 +344,6 @@ private fun HistoryVisibilitySection( title = stringResource(R.string.screen_security_and_privacy_room_history_section_header), modifier = modifier, ) { - Spacer(Modifier.height(16.dp)) for (availableOption in availableOptions) { val isSelected = availableOption == editedOption HistoryVisibilityItem( @@ -351,6 +352,7 @@ private fun HistoryVisibilitySection( onSelectOption = onSelectOption, ) } + // Also show the saved option if it's not in the available options, but disabled if (savedOptions != null && !availableOptions.contains(savedOptions)) { HistoryVisibilityItem( option = savedOptions, diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/editroomaddress/EditRoomAddressPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/editroomaddress/EditRoomAddressPresenter.kt index 884034ca1e..153b751d08 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/editroomaddress/EditRoomAddressPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/securityandprivacy/editroomaddress/EditRoomAddressPresenter.kt @@ -118,12 +118,10 @@ class EditRoomAddressPresenter @AssistedInject constructor( // Otherwise, only update the alternative aliases and keep the current canonical alias else -> { val newAlternativeAliases = buildList { + // New alias is added first, so we make sure we pick it first add(newRoomAlias) - for (alias in room.alternativeAliases) { - if (alias != savedAliasFromHomeserver) { - add(alias) - } - } + // Add all other aliases, except the one we just removed from the room directory + addAll(room.alternativeAliases.filter { it != savedAliasFromHomeserver }) } room.updateCanonicalAlias(savedCanonicalAlias, newAlternativeAliases).getOrThrow() } @@ -141,5 +139,5 @@ private fun MatrixRoom.firstAliasMatching(serverName: String): RoomAlias? { if (canonicalAlias?.matchesServer(serverName) == true) { return canonicalAlias } - return alternativeAliases.firstOrNull { it.value.contains(serverName) } + return alternativeAliases.firstOrNull { it.matchesServer(serverName) } } diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/securityandprivacy/SecurityAndPrivacyPresenterTest.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/securityandprivacy/SecurityAndPrivacyPresenterTest.kt index 6d1afe7582..1618e15b88 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/securityandprivacy/SecurityAndPrivacyPresenterTest.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/securityandprivacy/SecurityAndPrivacyPresenterTest.kt @@ -37,7 +37,7 @@ class SecurityAndPrivacyPresenterTest { with(awaitItem()) { assertThat(editedSettings).isEqualTo(savedSettings) assertThat(canBeSaved).isFalse() - assertThat(showEncryptionConfirmation).isFalse() + assertThat(showEnableEncryptionConfirmation).isFalse() assertThat(saveAction).isEqualTo(AsyncAction.Uninitialized) assertThat(showRoomAccessSection).isFalse() assertThat(showRoomVisibilitySections).isFalse() @@ -47,7 +47,7 @@ class SecurityAndPrivacyPresenterTest { with(awaitItem()) { assertThat(editedSettings).isEqualTo(savedSettings) assertThat(canBeSaved).isFalse() - assertThat(showEncryptionConfirmation).isFalse() + assertThat(showEnableEncryptionConfirmation).isFalse() assertThat(saveAction).isEqualTo(AsyncAction.Uninitialized) assertThat(showRoomAccessSection).isTrue() assertThat(showRoomVisibilitySections).isFalse() @@ -138,21 +138,21 @@ class SecurityAndPrivacyPresenterTest { eventSink(SecurityAndPrivacyEvents.ToggleEncryptionState) } with(awaitItem()) { - assertThat(showEncryptionConfirmation).isTrue() + assertThat(showEnableEncryptionConfirmation).isTrue() eventSink(SecurityAndPrivacyEvents.CancelEnableEncryption) } with(awaitItem()) { - assertThat(showEncryptionConfirmation).isFalse() + assertThat(showEnableEncryptionConfirmation).isFalse() eventSink(SecurityAndPrivacyEvents.ToggleEncryptionState) } with(awaitItem()) { - assertThat(showEncryptionConfirmation).isTrue() + assertThat(showEnableEncryptionConfirmation).isTrue() eventSink(SecurityAndPrivacyEvents.ConfirmEnableEncryption) } skipItems(1) with(awaitItem()) { assertThat(editedSettings.isEncrypted).isTrue() - assertThat(showEncryptionConfirmation).isFalse() + assertThat(showEnableEncryptionConfirmation).isFalse() assertThat(canBeSaved).isTrue() eventSink(SecurityAndPrivacyEvents.ToggleEncryptionState) } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt index 6dd27defee..a5640ff1d6 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt @@ -428,9 +428,7 @@ interface MatrixRoom : Closeable { /** * Returns the visibility for this room in the room directory. - * - * [Public](`RoomVisibility::Public`) rooms are listed in the room - * directory and can be found using it. + * If the room is not published, the result will be [RoomVisibility.Private]. */ suspend fun getRoomVisibility(): Result