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 3e3d2d6cfb
commit edc478c115
18 changed files with 67 additions and 82 deletions

View File

@@ -263,7 +263,7 @@
{
"identity" : "swiftui-introspect",
"kind" : "remoteSourceControl",
"location" : "https://github.com/siteline/SwiftUI-Introspect",
"location" : "https://github.com/siteline/SwiftUI-Introspect.git",
"state" : {
"revision" : "b94da693e57eaf79d16464b8b7c90d09cba4e290",
"version" : "0.9.2"

View File

@@ -45,7 +45,6 @@ final class AppSettings {
case shouldCollapseRoomStateEvents
case userSuggestionsEnabled
case swiftUITimelineEnabled
case chatBackupEnabled
}
private static var suiteName: String = InfoPlistReader.main.appGroupIdentifier
@@ -144,7 +143,13 @@ final class AppSettings {
/// Any pre-defined static client registrations for OIDC issuers.
let oidcStaticRegistrations: [URL: String] = ["https://id.thirdroom.io/realms/thirdroom": "elementx"]
/// 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
/// 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)
var swiftUITimelineEnabled
@UserPreference(key: UserDefaultsKeys.chatBackupEnabled, defaultValue: false, storageType: .userDefaults(store))
var chatBackupEnabled
#endif

View File

@@ -383,7 +383,7 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
return
}
guard isLastSession, appSettings.chatBackupEnabled else {
guard isLastSession else {
ServiceLocator.shared.userIndicatorController.alertInfo = .init(id: .init(),
title: L10n.screenSignoutConfirmationDialogTitle,
message: L10n.screenSignoutConfirmationDialogContent,

View File

@@ -72,10 +72,29 @@ struct HomeScreenViewState: BindableState {
let userID: String
var userDisplayName: String?
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 showUserMenuBadge = false
var showSettingsMenuOptionBadge = false
var hasRecoveryKeyConfirmationBannerBeenDismissed = false
var showRecoveryKeyConfirmationBanner: Bool {
guard let isSessionVerified else {
return false
}
return isSessionVerified && needsRecoveryKeyConfirmation && !hasRecoveryKeyConfirmationBannerBeenDismissed
}
var rooms: [HomeScreenRoom] = []
var roomListMode: HomeScreenRoomListMode = .skeletons

View File

@@ -31,7 +31,6 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
private let visibleItemRangePublisher = CurrentValueSubject<(range: Range<Int>, isScrolling: Bool), Never>((0..<0, false))
private var actionsSubject: PassthroughSubject<HomeScreenViewModelAction, Never> = .init()
var actions: AnyPublisher<HomeScreenViewModelAction, Never> {
actionsSubject.eraseToAnyPublisher()
}
@@ -50,20 +49,6 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
super.init(initialViewState: .init(userID: userSession.userID),
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
.receive(on: DispatchQueue.main)
.weakAssign(to: \.state.userAvatarURL, on: self)
@@ -74,15 +59,18 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
.weakAssign(to: \.state.userDisplayName, on: self)
.store(in: &cancellables)
userSession.sessionVerificationState
.receive(on: DispatchQueue.main)
.weakAssign(to: \.state.isSessionVerified, on: self)
.store(in: &cancellables)
userSession.clientProxy.secureBackupController.recoveryKeyState
.receive(on: DispatchQueue.main)
.sink { [weak self] recoveryKeyState in
guard let self, appSettings.chatBackupEnabled else { return }
guard let self else { return }
let requiresSecureBackupSetup = recoveryKeyState == .disabled || recoveryKeyState == .incomplete
state.showUserMenuBadge = requiresSecureBackupSetup
state.showSettingsMenuOptionBadge = requiresSecureBackupSetup
state.requiresSecureBackupSetup = requiresSecureBackupSetup
state.needsRecoveryKeyConfirmation = recoveryKeyState == .incomplete
}
@@ -138,9 +126,9 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
case .confirmRecoveryKey:
actionsSubject.send(.presentSecureBackupSettings)
case .skipSessionVerification:
state.needsSessionVerification = false
state.hasSessionVerificationBannerBeenDismissed = true
case .skipRecoveryKeyConfirmation:
state.needsRecoveryKeyConfirmation = false
state.hasRecoveryKeyConfirmationBannerBeenDismissed = true
case .updateVisibleItemRange(let range, let isScrolling):
visibleItemRangePublisher.send((range, isScrolling))
case .startChat:

View File

@@ -191,9 +191,9 @@ struct HomeScreen: View {
@ViewBuilder
/// The session verification banner and invites button if either are needed.
private var topSection: some View {
if context.viewState.needsSessionVerification {
if context.viewState.showSessionVerificationBanner {
HomeScreenSessionVerificationBanner(context: context)
} else if context.viewState.needsRecoveryKeyConfirmation {
} else if context.viewState.showRecoveryKeyConfirmationBanner {
HomeScreenRecoveryKeyConfirmationBanner(context: context)
}

View File

@@ -29,7 +29,7 @@ struct HomeScreenUserMenuButton: View {
Label {
Text(L10n.commonSettings)
} icon: {
if context.viewState.showSettingsMenuOptionBadge {
if context.viewState.requiresSecureBackupSetup, context.viewState.isSessionVerified == true {
CompoundIcon(asset: Asset.Images.settingsIconWithBadge)
} else {
CompoundIcon(\.settings)
@@ -50,7 +50,7 @@ struct HomeScreenUserMenuButton: View {
avatarSize: .user(on: .home),
imageProvider: context.imageProvider)
.accessibilityIdentifier(A11yIdentifiers.homeScreen.userAvatar)
.overlayBadge(10, isBadged: context.viewState.showUserMenuBadge)
.overlayBadge(10, isBadged: context.viewState.requiresSecureBackupSetup && context.viewState.isSessionVerified == true)
.compositingGroup()
}
.accessibilityLabel(L10n.a11yUserMenu)

View File

@@ -49,7 +49,6 @@ protocol DeveloperOptionsProtocol: AnyObject {
var shouldCollapseRoomStateEvents: Bool { get set }
var userSuggestionsEnabled: Bool { get set }
var swiftUITimelineEnabled: Bool { get set }
var chatBackupEnabled: Bool { get set }
var elementCallBaseURL: URL { get set }
var elementCallUseEncryption: Bool { get set }

View File

@@ -32,13 +32,6 @@ struct DeveloperOptionsScreen: View {
}
}
Section("Security") {
Toggle(isOn: $context.chatBackupEnabled) {
Text("Chat backup")
Text("Requires app reboot")
}
}
Section("Timeline") {
Toggle(isOn: $context.shouldCollapseRoomStateEvents) {
Text("Collapse room state events")

View File

@@ -42,7 +42,6 @@ struct SettingsScreenViewState: BindableState {
var userAvatarURL: URL?
var userDisplayName: String?
var isSessionVerified: Bool?
var chatBackupEnabled = false
var showSecureBackupBadge = false
var showDeveloperOptions: Bool

View File

@@ -44,8 +44,9 @@ class SettingsScreenViewModel: SettingsScreenViewModelType, SettingsScreenViewMo
.weakAssign(to: \.state.userDisplayName, on: self)
.store(in: &cancellables)
appSettings.$chatBackupEnabled
.weakAssign(to: \.state.chatBackupEnabled, on: self)
userSession.sessionVerificationState
.receive(on: DispatchQueue.main)
.weakAssign(to: \.state.isSessionVerified, on: self)
.store(in: &cancellables)
userSession.clientProxy.secureBackupController.recoveryKeyState
@@ -53,33 +54,14 @@ class SettingsScreenViewModel: SettingsScreenViewModelType, SettingsScreenViewMo
.sink { [weak self] state in
guard let self else { return }
self.state.showSecureBackupBadge = (state == .incomplete || state == .disabled) && appSettings.chatBackupEnabled
self.state.showSecureBackupBadge = (state == .incomplete || state == .disabled)
}
.store(in: &cancellables)
Task {
await userSession.clientProxy.loadUserAvatarURL()
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) {

View File

@@ -88,7 +88,7 @@ struct SettingsScreen: View {
ListRow(label: .default(title: L10n.actionCompleteVerification,
icon: \.checkCircle),
kind: .button { context.send(viewAction: .sessionVerification) })
} else if context.viewState.chatBackupEnabled {
} else {
ListRow(label: .default(title: L10n.commonChatBackup,
icon: Image(asset: Asset.Images.secureBackupIcon)),
details: context.viewState.showSecureBackupBadge ? .icon(secureBackupBadge) : nil,

View File

@@ -67,7 +67,8 @@ class PostHogAnalyticsClient: AnalyticsClientProtocol {
}
// 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,
numSpaces: userProperties.numSpaces ?? pendingUserProperties.numSpaces)
}

View File

@@ -25,4 +25,5 @@ struct MockUserSession: UserSessionProtocol {
let clientProxy: ClientProxyProtocol
let mediaProvider: MediaProviderProtocol
let voiceMessageMediaManager: VoiceMessageMediaManagerProtocol
var sessionVerificationState: CurrentValuePublisher<Bool?, Never> = .init(.init(true))
}

View File

@@ -29,9 +29,16 @@ class UserSession: UserSessionProtocol {
let clientProxy: ClientProxyProtocol
let mediaProvider: MediaProviderProtocol
let voiceMessageMediaManager: VoiceMessageMediaManagerProtocol
let callbacks = PassthroughSubject<UserSessionCallback, Never>()
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) {
self.clientProxy = clientProxy
self.mediaProvider = mediaProvider
@@ -64,16 +71,14 @@ class UserSession: UserSessionProtocol {
tearDownSessionVerificationControllerWatchdog()
if !isVerified {
callbacks.send(.sessionVerificationNeeded)
}
self.sessionVerificationController = sessionVerificationController
sessionVerificationStateSubject.send(isVerified)
sessionVerificationController.callbacks.sink { [weak self] callback in
switch callback {
case .finished:
self?.callbacks.send(.didVerifySession)
self?.sessionVerificationStateSubject.send(true)
default:
break
}

View File

@@ -18,8 +18,6 @@ import Combine
import Foundation
enum UserSessionCallback {
case sessionVerificationNeeded
case didVerifySession
case didReceiveAuthError(isSoftLogout: Bool)
}
@@ -32,6 +30,7 @@ protocol UserSessionProtocol {
var mediaProvider: MediaProviderProtocol { get }
var voiceMessageMediaManager: VoiceMessageMediaManagerProtocol { get }
var sessionVerificationState: CurrentValuePublisher<Bool?, Never> { get }
var sessionVerificationController: SessionVerificationControllerProxyProtocol? { get }
var callbacks: PassthroughSubject<UserSessionCallback, Never> { get }

View File

@@ -32,12 +32,9 @@ final class UserSessionTests: XCTestCase {
func test_whenUserSessionReceivesSyncUpdateAndSessionControllerRetrievedAndSessionNotVerified_sessionVerificationNeededEventReceived() throws {
let expectation = expectation(description: "SessionVerificationNeeded expectation")
userSession.callbacks.sink { callback in
switch callback {
case .sessionVerificationNeeded:
userSession.sessionVerificationState.sink { isVerified in
if let isVerified, isVerified == false {
expectation.fulfill()
default:
break
}
}
.store(in: &cancellables)

View File

@@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:2b391abfdd2172848ae1a2510d50ea597e337425793560d01959b57429972ba2
size 106343
oid sha256:ceef950f0e74453a572d90b85cea1a3ba2a702595358efb36d76285d51acee15
size 106872