Secure backup tweaks (#2277)

* Use a custom SDK build that might fix incomplete recovery state false positives. Enable chat backup by default

* Get rid of the chat backup feature flag

* Fix force unwrap warning

* Fix long line warning

* Prevent the recovery key confirmation banner popping up before the verification state is fetched. Stop showing secure backup user avatar and menu badges while the session is not verified.

* Switch back to the release version of the SDK

* Fix inconsistent session verification states, improve/simplify how to deal with it

* Fix unit tests

* Cleanup and hopefully simplify home screen banner presentations
This commit is contained in:
Stefan Ceriu
2023-12-21 17:08:29 +02:00
committed by GitHub
parent d148827eae
commit 0ae71f740d
18 changed files with 67 additions and 82 deletions
@@ -263,7 +263,7 @@
{ {
"identity" : "swiftui-introspect", "identity" : "swiftui-introspect",
"kind" : "remoteSourceControl", "kind" : "remoteSourceControl",
"location" : "https://github.com/siteline/SwiftUI-Introspect", "location" : "https://github.com/siteline/SwiftUI-Introspect.git",
"state" : { "state" : {
"revision" : "b94da693e57eaf79d16464b8b7c90d09cba4e290", "revision" : "b94da693e57eaf79d16464b8b7c90d09cba4e290",
"version" : "0.9.2" "version" : "0.9.2"
@@ -45,7 +45,6 @@ final class AppSettings {
case shouldCollapseRoomStateEvents case shouldCollapseRoomStateEvents
case userSuggestionsEnabled case userSuggestionsEnabled
case swiftUITimelineEnabled case swiftUITimelineEnabled
case chatBackupEnabled
} }
private static var suiteName: String = InfoPlistReader.main.appGroupIdentifier private static var suiteName: String = InfoPlistReader.main.appGroupIdentifier
@@ -144,7 +143,13 @@ final class AppSettings {
/// Any pre-defined static client registrations for OIDC issuers. /// Any pre-defined static client registrations for OIDC issuers.
let oidcStaticRegistrations: [URL: String] = ["https://id.thirdroom.io/realms/thirdroom": "elementx"] let oidcStaticRegistrations: [URL: String] = ["https://id.thirdroom.io/realms/thirdroom": "elementx"]
/// The redirect URL used for OIDC. /// The redirect URL used for OIDC.
let oidcRedirectURL = URL(string: "\(InfoPlistReader.main.appScheme):/callback")! let oidcRedirectURL = {
guard let url = URL(string: "\(InfoPlistReader.main.appScheme):/callback") else {
fatalError("Invalid OIDC redirect URL")
}
return url
}()
/// The date that the call to `/login` completed successfully. This is used to put /// The date that the call to `/login` completed successfully. This is used to put
/// a hard wall on the history of encrypted messages until we have key backup. /// a hard wall on the history of encrypted messages until we have key backup.
@@ -261,9 +266,6 @@ final class AppSettings {
@UserPreference(key: UserDefaultsKeys.swiftUITimelineEnabled, defaultValue: false, storageType: .volatile) @UserPreference(key: UserDefaultsKeys.swiftUITimelineEnabled, defaultValue: false, storageType: .volatile)
var swiftUITimelineEnabled var swiftUITimelineEnabled
@UserPreference(key: UserDefaultsKeys.chatBackupEnabled, defaultValue: false, storageType: .userDefaults(store))
var chatBackupEnabled
#endif #endif
@@ -383,7 +383,7 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
return return
} }
guard isLastSession, appSettings.chatBackupEnabled else { guard isLastSession else {
ServiceLocator.shared.userIndicatorController.alertInfo = .init(id: .init(), ServiceLocator.shared.userIndicatorController.alertInfo = .init(id: .init(),
title: L10n.screenSignoutConfirmationDialogTitle, title: L10n.screenSignoutConfirmationDialogTitle,
message: L10n.screenSignoutConfirmationDialogContent, message: L10n.screenSignoutConfirmationDialogContent,
@@ -72,10 +72,29 @@ struct HomeScreenViewState: BindableState {
let userID: String let userID: String
var userDisplayName: String? var userDisplayName: String?
var userAvatarURL: URL? var userAvatarURL: URL?
var needsSessionVerification = false
var isSessionVerified: Bool?
var hasSessionVerificationBannerBeenDismissed = false
var showSessionVerificationBanner: Bool {
guard let isSessionVerified else {
return false
}
return !isSessionVerified && !hasSessionVerificationBannerBeenDismissed
}
var requiresSecureBackupSetup = false
var needsRecoveryKeyConfirmation = false var needsRecoveryKeyConfirmation = false
var showUserMenuBadge = false var hasRecoveryKeyConfirmationBannerBeenDismissed = false
var showSettingsMenuOptionBadge = false var showRecoveryKeyConfirmationBanner: Bool {
guard let isSessionVerified else {
return false
}
return isSessionVerified && needsRecoveryKeyConfirmation && !hasRecoveryKeyConfirmationBannerBeenDismissed
}
var rooms: [HomeScreenRoom] = [] var rooms: [HomeScreenRoom] = []
var roomListMode: HomeScreenRoomListMode = .skeletons var roomListMode: HomeScreenRoomListMode = .skeletons
@@ -31,7 +31,6 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
private let visibleItemRangePublisher = CurrentValueSubject<(range: Range<Int>, isScrolling: Bool), Never>((0..<0, false)) private let visibleItemRangePublisher = CurrentValueSubject<(range: Range<Int>, isScrolling: Bool), Never>((0..<0, false))
private var actionsSubject: PassthroughSubject<HomeScreenViewModelAction, Never> = .init() private var actionsSubject: PassthroughSubject<HomeScreenViewModelAction, Never> = .init()
var actions: AnyPublisher<HomeScreenViewModelAction, Never> { var actions: AnyPublisher<HomeScreenViewModelAction, Never> {
actionsSubject.eraseToAnyPublisher() actionsSubject.eraseToAnyPublisher()
} }
@@ -50,20 +49,6 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
super.init(initialViewState: .init(userID: userSession.userID), super.init(initialViewState: .init(userID: userSession.userID),
imageProvider: userSession.mediaProvider) imageProvider: userSession.mediaProvider)
userSession.callbacks
.receive(on: DispatchQueue.main)
.sink { [weak self] callback in
switch callback {
case .sessionVerificationNeeded:
self?.state.needsSessionVerification = true
case .didVerifySession:
self?.state.needsSessionVerification = false
default:
break
}
}
.store(in: &cancellables)
userSession.clientProxy.userAvatarURL userSession.clientProxy.userAvatarURL
.receive(on: DispatchQueue.main) .receive(on: DispatchQueue.main)
.weakAssign(to: \.state.userAvatarURL, on: self) .weakAssign(to: \.state.userAvatarURL, on: self)
@@ -74,15 +59,18 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
.weakAssign(to: \.state.userDisplayName, on: self) .weakAssign(to: \.state.userDisplayName, on: self)
.store(in: &cancellables) .store(in: &cancellables)
userSession.sessionVerificationState
.receive(on: DispatchQueue.main)
.weakAssign(to: \.state.isSessionVerified, on: self)
.store(in: &cancellables)
userSession.clientProxy.secureBackupController.recoveryKeyState userSession.clientProxy.secureBackupController.recoveryKeyState
.receive(on: DispatchQueue.main) .receive(on: DispatchQueue.main)
.sink { [weak self] recoveryKeyState in .sink { [weak self] recoveryKeyState in
guard let self, appSettings.chatBackupEnabled else { return } guard let self else { return }
let requiresSecureBackupSetup = recoveryKeyState == .disabled || recoveryKeyState == .incomplete let requiresSecureBackupSetup = recoveryKeyState == .disabled || recoveryKeyState == .incomplete
state.requiresSecureBackupSetup = requiresSecureBackupSetup
state.showUserMenuBadge = requiresSecureBackupSetup
state.showSettingsMenuOptionBadge = requiresSecureBackupSetup
state.needsRecoveryKeyConfirmation = recoveryKeyState == .incomplete state.needsRecoveryKeyConfirmation = recoveryKeyState == .incomplete
} }
@@ -138,9 +126,9 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
case .confirmRecoveryKey: case .confirmRecoveryKey:
actionsSubject.send(.presentSecureBackupSettings) actionsSubject.send(.presentSecureBackupSettings)
case .skipSessionVerification: case .skipSessionVerification:
state.needsSessionVerification = false state.hasSessionVerificationBannerBeenDismissed = true
case .skipRecoveryKeyConfirmation: case .skipRecoveryKeyConfirmation:
state.needsRecoveryKeyConfirmation = false state.hasRecoveryKeyConfirmationBannerBeenDismissed = true
case .updateVisibleItemRange(let range, let isScrolling): case .updateVisibleItemRange(let range, let isScrolling):
visibleItemRangePublisher.send((range, isScrolling)) visibleItemRangePublisher.send((range, isScrolling))
case .startChat: case .startChat:
@@ -191,9 +191,9 @@ struct HomeScreen: View {
@ViewBuilder @ViewBuilder
/// The session verification banner and invites button if either are needed. /// The session verification banner and invites button if either are needed.
private var topSection: some View { private var topSection: some View {
if context.viewState.needsSessionVerification { if context.viewState.showSessionVerificationBanner {
HomeScreenSessionVerificationBanner(context: context) HomeScreenSessionVerificationBanner(context: context)
} else if context.viewState.needsRecoveryKeyConfirmation { } else if context.viewState.showRecoveryKeyConfirmationBanner {
HomeScreenRecoveryKeyConfirmationBanner(context: context) HomeScreenRecoveryKeyConfirmationBanner(context: context)
} }
@@ -29,7 +29,7 @@ struct HomeScreenUserMenuButton: View {
Label { Label {
Text(L10n.commonSettings) Text(L10n.commonSettings)
} icon: { } icon: {
if context.viewState.showSettingsMenuOptionBadge { if context.viewState.requiresSecureBackupSetup, context.viewState.isSessionVerified == true {
CompoundIcon(asset: Asset.Images.settingsIconWithBadge) CompoundIcon(asset: Asset.Images.settingsIconWithBadge)
} else { } else {
CompoundIcon(\.settings) CompoundIcon(\.settings)
@@ -50,7 +50,7 @@ struct HomeScreenUserMenuButton: View {
avatarSize: .user(on: .home), avatarSize: .user(on: .home),
imageProvider: context.imageProvider) imageProvider: context.imageProvider)
.accessibilityIdentifier(A11yIdentifiers.homeScreen.userAvatar) .accessibilityIdentifier(A11yIdentifiers.homeScreen.userAvatar)
.overlayBadge(10, isBadged: context.viewState.showUserMenuBadge) .overlayBadge(10, isBadged: context.viewState.requiresSecureBackupSetup && context.viewState.isSessionVerified == true)
.compositingGroup() .compositingGroup()
} }
.accessibilityLabel(L10n.a11yUserMenu) .accessibilityLabel(L10n.a11yUserMenu)
@@ -49,7 +49,6 @@ protocol DeveloperOptionsProtocol: AnyObject {
var shouldCollapseRoomStateEvents: Bool { get set } var shouldCollapseRoomStateEvents: Bool { get set }
var userSuggestionsEnabled: Bool { get set } var userSuggestionsEnabled: Bool { get set }
var swiftUITimelineEnabled: Bool { get set } var swiftUITimelineEnabled: Bool { get set }
var chatBackupEnabled: Bool { get set }
var elementCallBaseURL: URL { get set } var elementCallBaseURL: URL { get set }
var elementCallUseEncryption: Bool { get set } var elementCallUseEncryption: Bool { get set }
@@ -32,13 +32,6 @@ struct DeveloperOptionsScreen: View {
} }
} }
Section("Security") {
Toggle(isOn: $context.chatBackupEnabled) {
Text("Chat backup")
Text("Requires app reboot")
}
}
Section("Timeline") { Section("Timeline") {
Toggle(isOn: $context.shouldCollapseRoomStateEvents) { Toggle(isOn: $context.shouldCollapseRoomStateEvents) {
Text("Collapse room state events") Text("Collapse room state events")
@@ -42,7 +42,6 @@ struct SettingsScreenViewState: BindableState {
var userAvatarURL: URL? var userAvatarURL: URL?
var userDisplayName: String? var userDisplayName: String?
var isSessionVerified: Bool? var isSessionVerified: Bool?
var chatBackupEnabled = false
var showSecureBackupBadge = false var showSecureBackupBadge = false
var showDeveloperOptions: Bool var showDeveloperOptions: Bool
@@ -44,8 +44,9 @@ class SettingsScreenViewModel: SettingsScreenViewModelType, SettingsScreenViewMo
.weakAssign(to: \.state.userDisplayName, on: self) .weakAssign(to: \.state.userDisplayName, on: self)
.store(in: &cancellables) .store(in: &cancellables)
appSettings.$chatBackupEnabled userSession.sessionVerificationState
.weakAssign(to: \.state.chatBackupEnabled, on: self) .receive(on: DispatchQueue.main)
.weakAssign(to: \.state.isSessionVerified, on: self)
.store(in: &cancellables) .store(in: &cancellables)
userSession.clientProxy.secureBackupController.recoveryKeyState userSession.clientProxy.secureBackupController.recoveryKeyState
@@ -53,33 +54,14 @@ class SettingsScreenViewModel: SettingsScreenViewModelType, SettingsScreenViewMo
.sink { [weak self] state in .sink { [weak self] state in
guard let self else { return } guard let self else { return }
self.state.showSecureBackupBadge = (state == .incomplete || state == .disabled) && appSettings.chatBackupEnabled self.state.showSecureBackupBadge = (state == .incomplete || state == .disabled)
} }
.store(in: &cancellables) .store(in: &cancellables)
Task { Task {
await userSession.clientProxy.loadUserAvatarURL() await userSession.clientProxy.loadUserAvatarURL()
await userSession.clientProxy.loadUserDisplayName() await userSession.clientProxy.loadUserDisplayName()
if let sessionVerificationController = userSession.sessionVerificationController,
case let .success(isVerified) = await sessionVerificationController.isVerified() {
state.isSessionVerified = isVerified
}
} }
userSession.callbacks
.receive(on: DispatchQueue.main)
.sink { [weak self] callback in
switch callback {
case .sessionVerificationNeeded:
self?.state.isSessionVerified = false
case .didVerifySession:
self?.state.isSessionVerified = true
default:
break
}
}
.store(in: &cancellables)
} }
override func process(viewAction: SettingsScreenViewAction) { override func process(viewAction: SettingsScreenViewAction) {
@@ -88,7 +88,7 @@ struct SettingsScreen: View {
ListRow(label: .default(title: L10n.actionCompleteVerification, ListRow(label: .default(title: L10n.actionCompleteVerification,
icon: \.checkCircle), icon: \.checkCircle),
kind: .button { context.send(viewAction: .sessionVerification) }) kind: .button { context.send(viewAction: .sessionVerification) })
} else if context.viewState.chatBackupEnabled { } else {
ListRow(label: .default(title: L10n.commonChatBackup, ListRow(label: .default(title: L10n.commonChatBackup,
icon: Image(asset: Asset.Images.secureBackupIcon)), icon: Image(asset: Asset.Images.secureBackupIcon)),
details: context.viewState.showSecureBackupBadge ? .icon(secureBackupBadge) : nil, details: context.viewState.showSecureBackupBadge ? .icon(secureBackupBadge) : nil,
@@ -67,7 +67,8 @@ class PostHogAnalyticsClient: AnalyticsClientProtocol {
} }
// Merge the updated user properties with the existing ones // Merge the updated user properties with the existing ones
self.pendingUserProperties = AnalyticsEvent.UserProperties(allChatsActiveFilter: userProperties.allChatsActiveFilter ?? pendingUserProperties.allChatsActiveFilter, ftueUseCaseSelection: userProperties.ftueUseCaseSelection ?? pendingUserProperties.ftueUseCaseSelection, self.pendingUserProperties = AnalyticsEvent.UserProperties(allChatsActiveFilter: userProperties.allChatsActiveFilter ?? pendingUserProperties.allChatsActiveFilter,
ftueUseCaseSelection: userProperties.ftueUseCaseSelection ?? pendingUserProperties.ftueUseCaseSelection,
numFavouriteRooms: userProperties.numFavouriteRooms ?? pendingUserProperties.numFavouriteRooms, numFavouriteRooms: userProperties.numFavouriteRooms ?? pendingUserProperties.numFavouriteRooms,
numSpaces: userProperties.numSpaces ?? pendingUserProperties.numSpaces) numSpaces: userProperties.numSpaces ?? pendingUserProperties.numSpaces)
} }
@@ -25,4 +25,5 @@ struct MockUserSession: UserSessionProtocol {
let clientProxy: ClientProxyProtocol let clientProxy: ClientProxyProtocol
let mediaProvider: MediaProviderProtocol let mediaProvider: MediaProviderProtocol
let voiceMessageMediaManager: VoiceMessageMediaManagerProtocol let voiceMessageMediaManager: VoiceMessageMediaManagerProtocol
var sessionVerificationState: CurrentValuePublisher<Bool?, Never> = .init(.init(true))
} }
@@ -29,9 +29,16 @@ class UserSession: UserSessionProtocol {
let clientProxy: ClientProxyProtocol let clientProxy: ClientProxyProtocol
let mediaProvider: MediaProviderProtocol let mediaProvider: MediaProviderProtocol
let voiceMessageMediaManager: VoiceMessageMediaManagerProtocol let voiceMessageMediaManager: VoiceMessageMediaManagerProtocol
let callbacks = PassthroughSubject<UserSessionCallback, Never>() let callbacks = PassthroughSubject<UserSessionCallback, Never>()
private(set) var sessionVerificationController: SessionVerificationControllerProxyProtocol? private(set) var sessionVerificationController: SessionVerificationControllerProxyProtocol?
private var sessionVerificationStateSubject: CurrentValueSubject<Bool?, Never> = .init(nil)
var sessionVerificationState: CurrentValuePublisher<Bool?, Never> {
sessionVerificationStateSubject.asCurrentValuePublisher()
}
init(clientProxy: ClientProxyProtocol, mediaProvider: MediaProviderProtocol, voiceMessageMediaManager: VoiceMessageMediaManagerProtocol) { init(clientProxy: ClientProxyProtocol, mediaProvider: MediaProviderProtocol, voiceMessageMediaManager: VoiceMessageMediaManagerProtocol) {
self.clientProxy = clientProxy self.clientProxy = clientProxy
self.mediaProvider = mediaProvider self.mediaProvider = mediaProvider
@@ -64,16 +71,14 @@ class UserSession: UserSessionProtocol {
tearDownSessionVerificationControllerWatchdog() tearDownSessionVerificationControllerWatchdog()
if !isVerified {
callbacks.send(.sessionVerificationNeeded)
}
self.sessionVerificationController = sessionVerificationController self.sessionVerificationController = sessionVerificationController
sessionVerificationStateSubject.send(isVerified)
sessionVerificationController.callbacks.sink { [weak self] callback in sessionVerificationController.callbacks.sink { [weak self] callback in
switch callback { switch callback {
case .finished: case .finished:
self?.callbacks.send(.didVerifySession) self?.sessionVerificationStateSubject.send(true)
default: default:
break break
} }
@@ -18,8 +18,6 @@ import Combine
import Foundation import Foundation
enum UserSessionCallback { enum UserSessionCallback {
case sessionVerificationNeeded
case didVerifySession
case didReceiveAuthError(isSoftLogout: Bool) case didReceiveAuthError(isSoftLogout: Bool)
} }
@@ -32,6 +30,7 @@ protocol UserSessionProtocol {
var mediaProvider: MediaProviderProtocol { get } var mediaProvider: MediaProviderProtocol { get }
var voiceMessageMediaManager: VoiceMessageMediaManagerProtocol { get } var voiceMessageMediaManager: VoiceMessageMediaManagerProtocol { get }
var sessionVerificationState: CurrentValuePublisher<Bool?, Never> { get }
var sessionVerificationController: SessionVerificationControllerProxyProtocol? { get } var sessionVerificationController: SessionVerificationControllerProxyProtocol? { get }
var callbacks: PassthroughSubject<UserSessionCallback, Never> { get } var callbacks: PassthroughSubject<UserSessionCallback, Never> { get }
@@ -32,12 +32,9 @@ final class UserSessionTests: XCTestCase {
func test_whenUserSessionReceivesSyncUpdateAndSessionControllerRetrievedAndSessionNotVerified_sessionVerificationNeededEventReceived() throws { func test_whenUserSessionReceivesSyncUpdateAndSessionControllerRetrievedAndSessionNotVerified_sessionVerificationNeededEventReceived() throws {
let expectation = expectation(description: "SessionVerificationNeeded expectation") let expectation = expectation(description: "SessionVerificationNeeded expectation")
userSession.callbacks.sink { callback in userSession.sessionVerificationState.sink { isVerified in
switch callback { if let isVerified, isVerified == false {
case .sessionVerificationNeeded:
expectation.fulfill() expectation.fulfill()
default:
break
} }
} }
.store(in: &cancellables) .store(in: &cancellables)
@@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1 version https://git-lfs.github.com/spec/v1
oid sha256:2b391abfdd2172848ae1a2510d50ea597e337425793560d01959b57429972ba2 oid sha256:ceef950f0e74453a572d90b85cea1a3ba2a702595358efb36d76285d51acee15
size 106343 size 106872