Add timeouts after verifying the device owner when linking a new device. (#5970)

* Support for waitingForAuth continuation sender during Link new device * Stop automatically regenerating new QR codes when they expire. This reverts part of commit e235e3ba81. * Add a timeout after the Face/TouchID verification when linking a desktop computer. --------- Co-authored-by: Hugh Nimmo-Smith <hughns@element.io>
2026-11-02 21:33:26 +00:00 · 2026-08-06 14:56:34 +01:00
parent a0521d1fdf
commit a82726a991
11 changed files with 125 additions and 112 deletions
@@ -57,7 +57,7 @@ class LinkNewDeviceFlowCoordinator: FlowCoordinatorProtocol {
                
                switch action {
                case .linkMobileDevice(let progressPublisher):
-                    presentQRCodeScreen(mode: .linkMobile(progressPublisher, flowParameters.userSession.clientProxy))
+                    presentQRCodeScreen(mode: .linkMobile(progressPublisher))
                case .linkDesktopComputer:
                    presentQRCodeScreen(mode: .linkDesktop(flowParameters.userSession.clientProxy.linkNewDeviceService()))
                case .verifyWithAppLockPIN(let continuation):
@@ -39,7 +39,7 @@ enum QRCodeLoginScreenMode {
    /// Configures the screen to link another device by scanning a QR code.
    case linkDesktop(LinkNewDeviceServiceProtocol)
    /// Configures the screen to link another device by showing it a QR code.
-    case linkMobile(LinkNewDeviceService.LinkMobileProgressPublisher, ClientProxyProtocol)
+    case linkMobile(LinkNewDeviceService.LinkMobileProgressPublisher)
 }

 struct QRCodeLoginScreenViewState: BindableState {
@@ -107,7 +107,7 @@ nonisolated enum QRCodeLoginState: Equatable {
    case displayCode(DisplayCodeState)
    
    /// Initial state where the user can link another device using the shown QR code.
-    case displayQR(DisplayQRState)
+    case displayQR(UIImage)
    /// The user needs to enter the two digit code to confirm the channel is secure
    case confirmCode(CheckCodeState)
    
@@ -171,13 +171,6 @@ nonisolated enum QRCodeLoginState: Equatable {
        }
    }
    
-    nonisolated enum DisplayQRState: Equatable {
-        /// The QR code is available and is being shown.
-        case active(UIImage)
-        /// The QR code being shown has expired. We will need to generate a new one.
-        case expired
-    }
-    
    nonisolated enum DisplayCodeState: Equatable {
        case deviceCode(String)
        case verificationCode(String)
@@ -13,6 +13,11 @@ typealias QRCodeLoginScreenViewModelType = StateStoreViewModel<QRCodeLoginScreen

 class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScreenViewModelProtocol {
    private let appMediator: AppMediatorProtocol
+    /// The time allowed between verifying the device's owner (on the previous screen), to having
+    /// scanned a QR code and established a secure channel for the `linkDesktopComputer` flow.
+    ///
+    /// The generated QR code handles this automatically when linking a mobile device and QR login doesn't need it.
+    private let linkDesktopTimeout: Duration
    
    private let actionsSubject: PassthroughSubject<QRCodeLoginScreenViewModelAction, Never> = .init()
    var actionsPublisher: AnyPublisher<QRCodeLoginScreenViewModelAction, Never> {
@@ -21,18 +26,21 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr
    
    private var currentTask: AnyCancellable?
    private var oAuthResultTask: AnyCancellable?
+    private var expirationTask: AnyCancellable?
    
    init(mode: QRCodeLoginScreenMode,
         canSignInManually: Bool,
-         appMediator: AppMediatorProtocol) {
+         appMediator: AppMediatorProtocol,
+         linkDesktopTimeout: Duration = .seconds(120)) {
        self.appMediator = appMediator
+        self.linkDesktopTimeout = linkDesktopTimeout
        
        let initialViewState: QRCodeLoginState = switch mode {
        case .login: .loginInstructions
        case .linkDesktop: .linkDesktopInstructions
-        case .linkMobile(let progressPublisher, _):
+        case .linkMobile(let progressPublisher):
            switch progressPublisher.value {
-            case .qrReady(let image): .displayQR(.active(image))
+            case .qrReady(let image): .displayQR(image)
            default: .error(.unknown)
            }
        }
@@ -40,8 +48,13 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr
        super.init(initialViewState: .init(state: initialViewState, mode: mode, canSignInManually: canSignInManually))
        setupSubscriptions()
        
-        if case .linkMobile(let progressPublisher, _) = mode {
+        switch mode {
+        case .linkDesktop:
+            scheduleLinkDesktopExpiration()
+        case .linkMobile(let progressPublisher):
            listenToDisplayQRProgress(progressPublisher: progressPublisher)
+        case .login:
+            break
        }
    }
    
@@ -180,9 +193,10 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr
                case .starting:
                    break // Nothing to do, the state was set above.
                case .establishingSecureChannel(let checkCodeString):
+                    expirationTask?.cancel()
                    state.state = .displayCode(.deviceCode(checkCodeString))
-                case .waitingForAuthorisation(let url):
-                    requestOAuthAuthorization(url: url)
+                case .waitingForAuthorisation(let verificationURL, let continuationSender):
+                    requestOAuthAuthorization(url: verificationURL, continuationSender: continuationSender)
                case .syncingSecrets:
                    break // Nothing to do.
                case .done:
@@ -221,8 +235,8 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr
                    break // Nothing to do, we are already showing the code by the time this method is called.
                case .qrScanned(let checkCodeSender):
                    state.state = .confirmCode(.inputCode(checkCodeSender))
-                case .waitingForAuthorisation(let url):
-                    requestOAuthAuthorization(url: url)
+                case .waitingForAuthorisation(let url, let continuationSender):
+                    requestOAuthAuthorization(url: url, continuationSender: continuationSender)
                case .syncingSecrets:
                    break // Nothing to do.
                case .done:
@@ -232,37 +246,29 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr
            }
    }
    
-    private func regenerateQRCode() async {
-        guard case .linkMobile(_, let clientProxy) = state.mode else {
-            fatalError("Cannot display a QR code in this mode.")
+    /// Starts the timeout for scanning a desktop's QR code. The flow will expire (cancelling the scan)
+    /// if the secure channel isn't established within the timeout period.
+    private func scheduleLinkDesktopExpiration() {
+        guard case .linkDesktop = state.mode else { fatalError("Expiration requested on an unexpected flow") }
+        
+        expirationTask = Task { [weak self, linkDesktopTimeout] in
+            try? await Task.sleep(for: linkDesktopTimeout, tolerance: .zero)
+            guard !Task.isCancelled else { return }
+            self?.handleExpiration()
+        }
+        .asCancellable()
+    }
+    
+    private func handleExpiration() {
+        // Don't override an error that's already being shown.
+        if case .error = state.state {
+            currentTask = nil // Cancel the scan (just in case).
+            return
        }
        
-        let linkNewDeviceService = clientProxy.linkNewDeviceService()
-        let progressPublisher = linkNewDeviceService.linkMobileDevice()
-        
-        do {
-            async let minimumDelay = Task.sleep(for: .seconds(1)) // To show the spinner for long enough so there isn't a flicker.
-            async let qrReadyProgress = progressPublisher.values
-                .first { progress in
-                    switch progress {
-                    case .qrReady: true
-                    default: false
-                    }
-                }
-            
-            guard case .qrReady(let image) = try await (qrReadyProgress, minimumDelay).0 else {
-                state.state = .error(.unknown)
-                return
-            }
-            
-            state.state = .displayQR(.active(image))
-        } catch let error as QRCodeLoginError {
-            handleError(error)
-        } catch {
-            handleError(.unknown)
-        }
-        
-        listenToDisplayQRProgress(progressPublisher: progressPublisher)
+        MXLog.info("Timed out establishing a secure channel, expiring.")
+        currentTask = nil // Cancel the scan.
+        handleError(.expired)
    }
    
    private func sendCheckCode() async {
@@ -284,7 +290,11 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr
        }
    }
    
-    private func requestOAuthAuthorization(url: URL) {
+    private func requestOAuthAuthorization(url: URL, continuationSender: ContinuationMessageSenderProxy) {
+        // There's no OAuth redirect, so the WAS always returns a cancellation failure (both for a declined system prompt and
+        // when closing after successful authorisation). We can't use the continuation as intended, so confirm it and continue.
+        Task { await continuationSender.confirm() }
+        
        let (stream, continuation) = AsyncStream<Result<Void, OAuthError>>.makeStream()
        actionsSubject.send(.requestOAuthAuthorisation(url, continuation))
        
@@ -324,9 +334,6 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr
            state.state = .error(.declined)
        case .linkingNotSupported:
            state.state = .error(.linkingNotSupported)
-        case .expired where state.state.isDisplayQR:
-            state.state = .displayQR(.expired)
-            Task { await regenerateQRCode() }
        case .expired:
            state.state = .error(.expired)
        case .slidingSyncNotAvailable:
@@ -341,6 +348,7 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr
    /// Only for mocking initial states
    fileprivate init(state: QRCodeLoginState, mode: QRCodeLoginScreenMode, canSignInManually: Bool, checkCodeInput: String) {
        appMediator = AppMediatorMock(.init())
+        linkDesktopTimeout = .seconds(3600)
        super.init(initialViewState: .init(state: state,
                                           mode: mode,
                                           canSignInManually: canSignInManually,
@@ -214,26 +214,19 @@ struct QRCodeLoginScreen: View {
    
    @ViewBuilder
    private var displayQRContent: some View {
-        if case let .displayQR(qrState) = context.viewState.state {
+        if case let .displayQR(image) = context.viewState.state {
            FullscreenDialog(topPadding: 24, horizontalPadding: 24) {
                VStack(spacing: 32) {
                    TitleAndIcon(title: L10n.screenLinkNewDeviceMobileTitle(InfoPlistReader.main.productionAppName),
                                 icon: \.takePhotoSolid,
                                 iconStyle: .default)
                    
-                    switch qrState {
-                    case .active(let image):
-                        Image(uiImage: image)
-                            .interpolation(.none) // to stop it getting blurred
-                            .resizable()
-                            .scaledToFit()
-                            .frame(width: 200, height: 200)
-                            .accessibilityLabel(L10n.a11yQrCode)
-                    case .expired:
-                        ProgressView()
-                            .controlSize(.large)
-                            .frame(width: 200, height: 200)
-                    }
+                    Image(uiImage: image)
+                        .interpolation(.none) // to stop it getting blurred
+                        .resizable()
+                        .scaledToFit()
+                        .frame(width: 200, height: 200)
+                        .accessibilityLabel(L10n.a11yQrCode)
                    
                    SFNumberedListView(items: context.viewState.instructions.linkMobileItems)
                }
@@ -351,8 +344,7 @@ struct QRCodeLoginScreen_Previews: PreviewProvider, TestablePreview {
    static let deviceNotSignedInStateViewModel = QRCodeLoginScreenViewModel.mock(state: .scan(.scanFailed(.deviceNotSignedIn)))
    
    /// Showing
-    static let showingStateViewModel = QRCodeLoginScreenViewModel.mock(state: .displayQR(.active(LinkNewDeviceServiceMock.mockQRCodeImage)))
-    static let showingExpiredStateViewModel = QRCodeLoginScreenViewModel.mock(state: .displayQR(.expired))
+    static let showingStateViewModel = QRCodeLoginScreenViewModel.mock(state: .displayQR(LinkNewDeviceServiceMock.mockQRCodeImage))
    
    // Displaying codes
    static let deviceCodeStateViewModel = QRCodeLoginScreenViewModel.mock(state: .displayCode(.deviceCode("12")))
@@ -384,8 +376,6 @@ struct QRCodeLoginScreen_Previews: PreviewProvider, TestablePreview {
        
        ElementNavigationStack { QRCodeLoginScreen(context: showingStateViewModel.context) }
            .previewDisplayName("Showing")
-        ElementNavigationStack { QRCodeLoginScreen(context: showingExpiredStateViewModel.context) }
-            .previewDisplayName("Showing expired")
        
        ElementNavigationStack { QRCodeLoginScreen(context: deviceCodeStateViewModel.context) }
            .previewDisplayName("Device code")
@@ -377,7 +377,7 @@ private extension HumanQrLoginError {
            .qrCodeError(.deviceNotSignedIn)
        case .UnsupportedQrCodeType:
            .qrCodeError(.invalidQRCode)
-        case .Unknown, .OAuthMetadataInvalid, .CheckCodeAlreadySent, .CheckCodeCannotBeSent, .ContinuationCannotBeSent, .ContinuationAlreadySent:
+        case .Unknown, .OAuthMetadataInvalid, .CheckCodeAlreadySent, .CheckCodeCannotBeSent, .ContinuationAlreadySent, .ContinuationCannotBeSent:
            .qrCodeError(.unknown)
        }
    }
@@ -29,7 +29,7 @@ class LinkNewDeviceService: LinkNewDeviceServiceProtocol {
        case starting
        case qrReady(UIImage)
        case qrScanned(CheckCodeSenderProxy)
-        case waitingForAuthorisation(verificationURL: URL)
+        case waitingForAuthorisation(verificationURL: URL, continuationSender: ContinuationMessageSenderProxy)
        case syncingSecrets
        case done
    }
@@ -38,7 +38,7 @@ class LinkNewDeviceService: LinkNewDeviceServiceProtocol {
    enum LinkDesktopProgress: Equatable {
        case starting
        case establishingSecureChannel(checkCodeString: String)
-        case waitingForAuthorisation(verificationURL: URL)
+        case waitingForAuthorisation(verificationURL: URL, continuationSender: ContinuationMessageSenderProxy)
        case syncingSecrets
        case done
    }
@@ -163,10 +163,10 @@ extension LinkNewDeviceService.LinkMobileProgress: CustomStringConvertible {
                throw Error.invalidQRCodeData
            }
        case .qrScanned(let checkCodeSender): .qrScanned(.init(underlyingSender: checkCodeSender))
-        case .waitingForAuth(let verificationURI, _):
+        case .waitingForAuth(let verificationURI, let continuationSender):
            // verificationURI is a String; ASWebAuthenticationSession requires a URL.
            if let url = URL(string: verificationURI) {
-                .waitingForAuthorisation(verificationURL: url)
+                .waitingForAuthorisation(verificationURL: url, continuationSender: .init(underlyingSender: continuationSender))
            } else {
                throw Error.invalidVerificationURI(verificationURI)
            }
@@ -194,10 +194,10 @@ extension LinkNewDeviceService.LinkDesktopProgress: CustomStringConvertible {
        self = switch rustProgress {
        case .starting: .starting
        case .establishingSecureChannel(_, let checkCodeString): .establishingSecureChannel(checkCodeString: checkCodeString)
-        case .waitingForAuth(let verificationURI, _):
+        case .waitingForAuth(let verificationURI, let continuationSender):
            // verificationURI is a String; ASWebAuthenticationSession requires a URL.
            if let url = URL(string: verificationURI) {
-                .waitingForAuthorisation(verificationURL: url)
+                .waitingForAuthorisation(verificationURL: url, continuationSender: .init(underlyingSender: continuationSender))
            } else {
                throw Error.invalidVerificationURI(verificationURI)
            }
@@ -240,8 +240,9 @@ private extension QRCodeLoginError {
    }
 }

-nonisolated class CheckCodeSenderProxy: Equatable {
+final nonisolated class CheckCodeSenderProxy: Equatable, Sendable {
    static func == (lhs: CheckCodeSenderProxy, rhs: CheckCodeSenderProxy) -> Bool {
+        // Not ideal but good enough as the equality check is purely for removing duplicates from the progress stream.
        lhs.underlyingSender === rhs.underlyingSender
    }
    
@@ -256,6 +257,35 @@ nonisolated class CheckCodeSenderProxy: Equatable {
    }
 }

+final nonisolated class ContinuationMessageSenderProxy: Equatable, Sendable {
+    static func == (lhs: ContinuationMessageSenderProxy, rhs: ContinuationMessageSenderProxy) -> Bool {
+        // Not ideal but good enough as the equality check is purely for removing duplicates from the progress stream.
+        lhs.underlyingSender === rhs.underlyingSender
+    }
+    
+    let underlyingSender: ContinuationMessageSenderProtocol
+    
+    init(underlyingSender: ContinuationMessageSenderProtocol) {
+        self.underlyingSender = underlyingSender
+    }
+    
+    func confirm() async {
+        do {
+            try await underlyingSender.confirm()
+        } catch {
+            MXLog.error("Failed to confirm the continuation of the login grant: \(error)")
+        }
+    }
+    
+    func cancel() async {
+        do {
+            try await underlyingSender.cancel()
+        } catch {
+            MXLog.error("Failed to cancel the login grant: \(error)")
+        }
+    }
+}
+
 extension UIImage {
    convenience init?(qrCodeData: Data) {
        let qrContext = CIContext()
@@ -1,3 +0,0 @@
-version https://git-lfs.github.com/spec/v1
-oid sha256:35b2338f432dd2d65581f94680382a33af87881bfd36e2a426a96f879ed1d93f
-size 119474
@@ -1,3 +0,0 @@
-version https://git-lfs.github.com/spec/v1
-oid sha256:3f34b4d5fb95d223b8b1cf850fe74fc75d031da0a7f0a359b5a4c3e29fa83669
-size 142744
@@ -1,3 +0,0 @@
-version https://git-lfs.github.com/spec/v1
-oid sha256:c86cc8592649e4d3e65a6632fd02756455ffd48bbdc35b12d7db5a8ba147c988
-size 70650
@@ -1,3 +0,0 @@
-version https://git-lfs.github.com/spec/v1
-oid sha256:2bbc55c8ae626a1e652864927dfa2eab2860c85dca9372cca73e0af982d7ca41
-size 96693
@@ -9,7 +9,6 @@
 import Combine
@testable import ElementX
 import MatrixRustSDKMocks
-import SwiftUI
 import Testing

@MainActor
@@ -19,7 +18,6 @@ struct QRCodeLoginScreenViewModelTests {
    var qrLoginProgressSubject: CurrentValueSubject<QRLoginProgress, AuthenticationServiceError>!
    var qrCodeLoginService: QRCodeLoginServiceMock!
    
-    var regeneratedQRCodeImage: UIImage!
    var linkMobileProgressSubject: CurrentValueSubject<LinkNewDeviceService.LinkMobileProgress, QRCodeLoginError>!
    var linkDesktopProgressSubject: CurrentValueSubject<LinkNewDeviceService.LinkDesktopProgress, QRCodeLoginError>!
    var linkNewDeviceService: LinkNewDeviceServiceMock!
@@ -152,7 +150,8 @@ struct QRCodeLoginScreenViewModelTests {
            guard case .requestOAuthAuthorisation = action else { return false }
            return true
        }
-        linkDesktopProgressSubject.send(.waitingForAuthorisation(verificationURL: .homeDirectory))
+        linkDesktopProgressSubject.send(.waitingForAuthorisation(verificationURL: .homeDirectory,
+                                                                 continuationSender: .init(underlyingSender: ContinuationMessageSenderSDKMock())))
        try await deferredAction.fulfill()
        
        let currentState = context.viewState.state
@@ -168,6 +167,25 @@ struct QRCodeLoginScreenViewModelTests {
        try await deferredAction.fulfill()
    }
    
+    @Test
+    mutating func linkDesktopExpiresWithoutSecureChannel() async throws {
+        setup(mode: .linkDesktop, linkDesktopTimeout: .zero)
+        #expect(context.viewState.state == .linkDesktopInstructions)
+        
+        // The timeout begins as soon as the screen appears, so the flow expires before a channel is established.
+        let deferred = deferFulfillment(context.$viewState) { $0.state == .error(.expired) }
+        try await deferred.fulfill()
+    }
+    
+    @Test
+    mutating func loginDoesNotExpire() async throws {
+        setup(mode: .login, linkDesktopTimeout: .zero)
+        
+        // The timeout only applies when linking a desktop, so signing in must never expire.
+        let deferredFailure = deferFailure(context.$viewState, timeout: .seconds(1)) { $0.state == .error(.expired) }
+        try await deferredFailure.fulfill()
+    }
+    
    @Test
    mutating func linkMobileDevice() async throws {
        setup(mode: .linkMobile)
@@ -188,7 +206,8 @@ struct QRCodeLoginScreenViewModelTests {
            guard case .requestOAuthAuthorisation = action else { return false }
            return true
        }
-        linkMobileProgressSubject.send(.waitingForAuthorisation(verificationURL: .homeDirectory))
+        linkMobileProgressSubject.send(.waitingForAuthorisation(verificationURL: .homeDirectory,
+                                                                continuationSender: .init(underlyingSender: ContinuationMessageSenderSDKMock())))
        try await deferredAction.fulfill()
        
        let currentState = context.viewState.state
@@ -204,20 +223,9 @@ struct QRCodeLoginScreenViewModelTests {
        try await deferredAction.fulfill()
    }
    
-    @Test
-    mutating func linkMobileDeviceQRCodeExpiry() async throws {
-        setup(mode: .linkMobile)
-        #expect(context.viewState.state.isDisplayQR)
-        
-        let deferred = deferFulfillment(context.$viewState, keyPath: \.state, transitionValues: [.displayQR(.expired),
-                                                                                                 .displayQR(.active(regeneratedQRCodeImage))])
-        linkMobileProgressSubject.send(completion: .failure(.expired))
-        try await deferred.fulfill()
-    }
-    
    // MARK: - Helpers
    
-    private mutating func setup(mode: Mode) {
+    private mutating func setup(mode: Mode, linkDesktopTimeout: Duration = .seconds(120)) {
        qrLoginProgressSubject = .init(.starting)
        qrCodeLoginService = QRCodeLoginServiceMock()
        qrCodeLoginService.loginWithQRCodeDataReturnValue = qrLoginProgressSubject.asCurrentValuePublisher()
@@ -227,11 +235,6 @@ struct QRCodeLoginScreenViewModelTests {
        linkNewDeviceService = LinkNewDeviceServiceMock(.init(linkMobileProgressPublisher: linkMobileProgressSubject.asCurrentValuePublisher(),
                                                              linkDesktopProgressPublisher: linkDesktopProgressSubject.asCurrentValuePublisher()))
        
-        regeneratedQRCodeImage = LinkNewDeviceServiceMock.mockQRCodeImage
-        let clientProxy = ClientProxyMock(.init())
-        clientProxy.linkNewDeviceServiceReturnValue = LinkNewDeviceServiceMock(.init(linkMobileProgressPublisher: .init(.qrReady(regeneratedQRCodeImage)),
-                                                                                     linkDesktopProgressPublisher: .init(.starting)))
-        
        let screenMode: QRCodeLoginScreenMode
        switch mode {
        case .login:
@@ -239,12 +242,13 @@ struct QRCodeLoginScreenViewModelTests {
        case .linkDesktop:
            screenMode = .linkDesktop(linkNewDeviceService)
        case .linkMobile:
-            screenMode = .linkMobile(linkNewDeviceService.linkMobileDevice(), clientProxy)
+            screenMode = .linkMobile(linkNewDeviceService.linkMobileDevice())
        }
        
        appMediator = AppMediatorMock(.init())
        viewModel = QRCodeLoginScreenViewModel(mode: screenMode,
                                               canSignInManually: true,
-                                               appMediator: appMediator)
+                                               appMediator: appMediator,
+                                               linkDesktopTimeout: linkDesktopTimeout)
    }
 }