Skip to content

Remind unverified sessions with a banner once a week (PSG-892) #7694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/7694.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remind unverified sessions with a banner once a week
8 changes: 6 additions & 2 deletions library/ui-strings/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2647,8 +2647,12 @@
<string name="unencrypted">Unencrypted</string>
<string name="encrypted_unverified">Encrypted by an unverified device</string>
<string name="key_authenticity_not_guaranteed">The authenticity of this encrypted message can\'t be guaranteed on this device.</string>
<string name="review_logins">Review where you’re logged in</string>
<string name="verify_other_sessions">Verify all your sessions to ensure your account &amp; messages are safe</string>
<!-- TODO TO BE REMOVED -->
<string name="review_logins" tools:ignore="UnusedResources">Review where you’re logged in</string>
<!-- TODO TO BE REMOVED -->
<string name="verify_other_sessions" tools:ignore="UnusedResources">Verify all your sessions to ensure your account &amp; messages are safe</string>
<string name="review_unverified_sessions_title">You have unverified sessions</string>
<string name="review_unverified_sessions_description">Review to ensure your account is safe</string>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 🚫 The resource R.string.verify_other_sessions appears to be unused

<!-- Argument will be replaced by the other session name (e.g, Desktop, mobile) -->
<string name="verify_this_session">Verify the new login accessing your account: %1$s</string>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ class DebugVectorFeatures(
override fun isVoiceBroadcastEnabled(): Boolean = read(DebugFeatureKeys.voiceBroadcastEnabled)
?: vectorFeatures.isVoiceBroadcastEnabled()

override fun isUnverifiedSessionsAlertEnabled(): Boolean = read(DebugFeatureKeys.unverifiedSessionsAlertEnabled)
?: vectorFeatures.isUnverifiedSessionsAlertEnabled()

fun <T> override(value: T?, key: Preferences.Key<T>) = updatePreferences {
if (value == null) {
it.remove(key)
Expand Down Expand Up @@ -151,4 +154,5 @@ object DebugFeatureKeys {
val qrCodeLoginForAllServers = booleanPreferencesKey("qr-code-login-for-all-servers")
val reciprocateQrCodeLogin = booleanPreferencesKey("reciprocate-qr-code-login")
val voiceBroadcastEnabled = booleanPreferencesKey("voice-broadcast-enabled")
val unverifiedSessionsAlertEnabled = booleanPreferencesKey("unverified-sessions-alert-enabled")
}
4 changes: 4 additions & 0 deletions vector-config/src/main/java/im/vector/app/config/Config.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package im.vector.app.config

import kotlin.time.Duration.Companion.days

/**
* Set of flags to configure the application.
*/
Expand Down Expand Up @@ -93,4 +95,6 @@ object Config {
* Can be disabled by providing Analytics.Disabled
*/
val NIGHTLY_ANALYTICS_CONFIG = RELEASE_ANALYTICS_CONFIG.copy(sentryEnvironment = "NIGHTLY")

val SHOW_UNVERIFIED_SESSIONS_ALERT_AFTER_MILLIS = 7.days.inWholeMilliseconds // 1 Week
}
2 changes: 2 additions & 0 deletions vector/src/main/java/im/vector/app/features/VectorFeatures.kt
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ interface VectorFeatures {
fun isQrCodeLoginForAllServers(): Boolean
fun isReciprocateQrCodeLogin(): Boolean
fun isVoiceBroadcastEnabled(): Boolean
fun isUnverifiedSessionsAlertEnabled(): Boolean
}

class DefaultVectorFeatures : VectorFeatures {
Expand All @@ -63,4 +64,5 @@ class DefaultVectorFeatures : VectorFeatures {
override fun isQrCodeLoginForAllServers(): Boolean = false
override fun isReciprocateQrCodeLogin(): Boolean = false
override fun isVoiceBroadcastEnabled(): Boolean = true
override fun isUnverifiedSessionsAlertEnabled(): Boolean = true
}
Original file line number Diff line number Diff line change
Expand Up @@ -239,12 +239,12 @@ class HomeDetailFragment :
.requestSessionVerification(vectorBaseActivity, newest.deviceId ?: "")
}
unknownDeviceDetectorSharedViewModel.handle(
UnknownDeviceDetectorSharedViewModel.Action.IgnoreDevice(newest.deviceId?.let { listOf(it) }.orEmpty())
UnknownDeviceDetectorSharedViewModel.Action.IgnoreNewLogin(newest.deviceId?.let { listOf(it) }.orEmpty())
)
}
dismissedAction = Runnable {
unknownDeviceDetectorSharedViewModel.handle(
UnknownDeviceDetectorSharedViewModel.Action.IgnoreDevice(newest.deviceId?.let { listOf(it) }.orEmpty())
UnknownDeviceDetectorSharedViewModel.Action.IgnoreNewLogin(newest.deviceId?.let { listOf(it) }.orEmpty())
)
}
}
Expand All @@ -256,8 +256,8 @@ class HomeDetailFragment :
alertManager.postVectorAlert(
VerificationVectorAlert(
uid = uid,
title = getString(R.string.review_logins),
description = getString(R.string.verify_other_sessions),
title = getString(R.string.review_unverified_sessions_title),
description = getString(R.string.review_unverified_sessions_description),
iconId = R.drawable.ic_shield_warning
).apply {
viewBinder = VerificationVectorAlert.ViewBinder(user, avatarRenderer)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* 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 im.vector.app.features.home

import im.vector.app.features.settings.VectorPreferences
import javax.inject.Inject

class IsNewLoginAlertShownUseCase @Inject constructor(
private val vectorPreferences: VectorPreferences,
) {

fun execute(deviceId: String?): Boolean {
deviceId ?: return false

return vectorPreferences.isNewLoginAlertShownForDevice(deviceId)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -253,12 +253,12 @@ class NewHomeDetailFragment :
.requestSessionVerification(vectorBaseActivity, newest.deviceId ?: "")
}
unknownDeviceDetectorSharedViewModel.handle(
UnknownDeviceDetectorSharedViewModel.Action.IgnoreDevice(newest.deviceId?.let { listOf(it) }.orEmpty())
UnknownDeviceDetectorSharedViewModel.Action.IgnoreNewLogin(newest.deviceId?.let { listOf(it) }.orEmpty())
)
}
dismissedAction = Runnable {
unknownDeviceDetectorSharedViewModel.handle(
UnknownDeviceDetectorSharedViewModel.Action.IgnoreDevice(newest.deviceId?.let { listOf(it) }.orEmpty())
UnknownDeviceDetectorSharedViewModel.Action.IgnoreNewLogin(newest.deviceId?.let { listOf(it) }.orEmpty())
)
}
}
Expand All @@ -270,8 +270,8 @@ class NewHomeDetailFragment :
alertManager.postVectorAlert(
VerificationVectorAlert(
uid = uid,
title = getString(R.string.review_logins),
description = getString(R.string.verify_other_sessions),
title = getString(R.string.review_unverified_sessions_title),
description = getString(R.string.review_unverified_sessions_description),
iconId = R.drawable.ic_shield_warning
).apply {
viewBinder = VerificationVectorAlert.ViewBinder(user, avatarRenderer)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* 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 im.vector.app.features.home

import im.vector.app.features.settings.VectorPreferences
import javax.inject.Inject

class SetNewLoginAlertShownUseCase @Inject constructor(
private val vectorPreferences: VectorPreferences,
) {

fun execute(deviceIds: List<String>) {
deviceIds.forEach {
vectorPreferences.setNewLoginAlertShownForDevice(it)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* 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 im.vector.app.features.home

import im.vector.app.core.time.Clock
import im.vector.app.features.settings.VectorPreferences
import javax.inject.Inject

class SetUnverifiedSessionsAlertShownUseCase @Inject constructor(
private val vectorPreferences: VectorPreferences,
private val clock: Clock,
) {

fun execute(deviceIds: List<String>) {
val epochMillis = clock.epochMillis()
deviceIds.forEach {
vectorPreferences.setUnverifiedSessionsAlertLastShownMillis(it, epochMillis)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* 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 im.vector.app.features.home

import im.vector.app.config.Config
import im.vector.app.core.time.Clock
import im.vector.app.features.VectorFeatures
import im.vector.app.features.settings.VectorPreferences
import javax.inject.Inject

class ShouldShowUnverifiedSessionsAlertUseCase @Inject constructor(
private val vectorFeatures: VectorFeatures,
private val vectorPreferences: VectorPreferences,
private val clock: Clock,
) {

fun execute(deviceId: String?): Boolean {
deviceId ?: return false

val isUnverifiedSessionsAlertEnabled = vectorFeatures.isUnverifiedSessionsAlertEnabled()
val unverifiedSessionsAlertLastShownMillis = vectorPreferences.getUnverifiedSessionsAlertLastShownMillis(deviceId)
return isUnverifiedSessionsAlertEnabled &&
clock.epochMillis() - unverifiedSessionsAlertLastShownMillis >= Config.SHOW_UNVERIFIED_SESSIONS_ALERT_AFTER_MILLIS
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package im.vector.app.features.home
import com.airbnb.mvrx.Async
import com.airbnb.mvrx.MavericksState
import com.airbnb.mvrx.MavericksViewModelFactory
import com.airbnb.mvrx.Success
import com.airbnb.mvrx.Uninitialized
import com.airbnb.mvrx.ViewModelContext
import dagger.assisted.Assisted
Expand All @@ -33,7 +32,6 @@ import im.vector.app.core.platform.EmptyViewEvents
import im.vector.app.core.platform.VectorViewModel
import im.vector.app.core.platform.VectorViewModelAction
import im.vector.app.core.time.Clock
import im.vector.app.features.settings.VectorPreferences
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.launchIn
Expand Down Expand Up @@ -63,12 +61,16 @@ data class DeviceDetectionInfo(
class UnknownDeviceDetectorSharedViewModel @AssistedInject constructor(
@Assisted initialState: UnknownDevicesState,
session: Session,
private val vectorPreferences: VectorPreferences,
clock: Clock,
private val shouldShowUnverifiedSessionsAlertUseCase: ShouldShowUnverifiedSessionsAlertUseCase,
private val setUnverifiedSessionsAlertShownUseCase: SetUnverifiedSessionsAlertShownUseCase,
private val isNewLoginAlertShownUseCase: IsNewLoginAlertShownUseCase,
private val setNewLoginAlertShownUseCase: SetNewLoginAlertShownUseCase,
) : VectorViewModel<UnknownDevicesState, UnknownDeviceDetectorSharedViewModel.Action, EmptyViewEvents>(initialState) {

sealed class Action : VectorViewModelAction {
data class IgnoreDevice(val deviceIds: List<String>) : Action()
data class IgnoreNewLogin(val deviceIds: List<String>) : Action()
}

@AssistedFactory
Expand All @@ -86,21 +88,13 @@ class UnknownDeviceDetectorSharedViewModel @AssistedInject constructor(
}
}

private val ignoredDeviceList = ArrayList<String>()

init {
val currentSessionTs = session.cryptoService().getCryptoDeviceInfo(session.myUserId)
.firstOrNull { it.deviceId == session.sessionParams.deviceId }
?.firstTimeSeenLocalTs
?: clock.epochMillis()
Timber.v("## Detector - Current Session first time seen $currentSessionTs")

ignoredDeviceList.addAll(
vectorPreferences.getUnknownDeviceDismissedList().also {
Timber.v("## Detector - Remembered ignored list $it")
}
)

combine(
session.flow().liveUserCryptoDevices(session.myUserId),
session.flow().liveMyDevicesInfo(),
Expand All @@ -114,13 +108,15 @@ class UnknownDeviceDetectorSharedViewModel @AssistedInject constructor(
cryptoList.firstOrNull { info.deviceId == it.deviceId }?.isVerified?.not().orFalse()
}
// filter out ignored devices
.filter { !ignoredDeviceList.contains(it.deviceId) }
.filter { shouldShowUnverifiedSessionsAlertUseCase.execute(it.deviceId) }
.sortedByDescending { it.lastSeenTs }
.map { deviceInfo ->
val deviceKnownSince = cryptoList.firstOrNull { it.deviceId == deviceInfo.deviceId }?.firstTimeSeenLocalTs ?: 0
val isNew = isNewLoginAlertShownUseCase.execute(deviceInfo.deviceId).not() && deviceKnownSince > currentSessionTs

DeviceDetectionInfo(
deviceInfo,
deviceKnownSince > currentSessionTs + 60_000, // short window to avoid false positive,
isNew,
pInfo.getOrNull()?.selfSigned != null // adding this to pass distinct when cross sign change
)
}
Expand Down Expand Up @@ -150,22 +146,11 @@ class UnknownDeviceDetectorSharedViewModel @AssistedInject constructor(
override fun handle(action: Action) {
when (action) {
is Action.IgnoreDevice -> {
ignoredDeviceList.addAll(action.deviceIds)
// local echo
withState { state ->
state.unknownSessions.invoke()?.let { detectedSessions ->
val updated = detectedSessions.filter { !action.deviceIds.contains(it.deviceInfo.deviceId) }
setState {
copy(unknownSessions = Success(updated))
}
}
}
setUnverifiedSessionsAlertShownUseCase.execute(action.deviceIds)
}
is Action.IgnoreNewLogin -> {
setNewLoginAlertShownUseCase.execute(action.deviceIds)
}
}
}

override fun onCleared() {
vectorPreferences.storeUnknownDeviceDismissedList(ignoredDeviceList)
super.onCleared()
}
}
Loading