diff --git a/ElementX/Sources/FlowCoordinators/LinkNewDeviceFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/LinkNewDeviceFlowCoordinator.swift index 1e1504a66..00229f52d 100644 --- a/ElementX/Sources/FlowCoordinators/LinkNewDeviceFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/LinkNewDeviceFlowCoordinator.swift @@ -51,7 +51,7 @@ class LinkNewDeviceFlowCoordinator: FlowCoordinatorProtocol { switch action { case .linkMobileDevice(let progressPublisher): - presentQRCodeScreen(mode: .linkMobile(progressPublisher)) + presentQRCodeScreen(mode: .linkMobile(progressPublisher, flowParameters.userSession.clientProxy)) case .linkDesktopComputer: presentQRCodeScreen(mode: .linkDesktop(flowParameters.userSession.clientProxy.linkNewDeviceService())) case .dismiss: diff --git a/ElementX/Sources/Screens/QRCodeLoginScreen/QRCodeLoginScreenModels.swift b/ElementX/Sources/Screens/QRCodeLoginScreen/QRCodeLoginScreenModels.swift index 27b97be24..a2f1d7164 100644 --- a/ElementX/Sources/Screens/QRCodeLoginScreen/QRCodeLoginScreenModels.swift +++ b/ElementX/Sources/Screens/QRCodeLoginScreen/QRCodeLoginScreenModels.swift @@ -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) + case linkMobile(LinkNewDeviceService.LinkMobileProgressPublisher, ClientProxyProtocol) } struct QRCodeLoginScreenViewState: BindableState { @@ -107,7 +107,7 @@ enum QRCodeLoginState: Equatable { case displayCode(DisplayCodeState) /// Initial state where the user can link another device using the shown QR code. - case displayQR(UIImage) + case displayQR(DisplayQRState) /// The user needs to enter the two digit code to confirm the channel is secure case confirmCode(CheckCodeState) @@ -171,6 +171,13 @@ enum QRCodeLoginState: Equatable { } } + 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 + } + enum DisplayCodeState: Equatable { case deviceCode(String) case verificationCode(String) diff --git a/ElementX/Sources/Screens/QRCodeLoginScreen/QRCodeLoginScreenViewModel.swift b/ElementX/Sources/Screens/QRCodeLoginScreen/QRCodeLoginScreenViewModel.swift index cb7fac582..15a3a7250 100644 --- a/ElementX/Sources/Screens/QRCodeLoginScreen/QRCodeLoginScreenViewModel.swift +++ b/ElementX/Sources/Screens/QRCodeLoginScreen/QRCodeLoginScreenViewModel.swift @@ -30,9 +30,9 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr 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(image) + case .qrReady(let image): .displayQR(.active(image)) default: .error(.unknown) } } @@ -40,7 +40,7 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr super.init(initialViewState: .init(state: initialViewState, mode: mode, canSignInManually: canSignInManually)) setupSubscriptions() - if case .linkMobile(let progressPublisher) = mode { + if case .linkMobile(let progressPublisher, _) = mode { listenToDisplayQRProgress(progressPublisher: progressPublisher) } } @@ -232,22 +232,49 @@ 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.") + } + + 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) + } + private func sendCheckCode() async { guard case let .confirmCode(.inputCode(checkCodeSender)) = state.state else { fatalError("Attempting to check code from the wrong state.") } + state.state = .confirmCode(.sendingCode) + let stringValue = state.bindings.checkCodeInput let code = UInt8(stringValue) ?? 0 - if !checkCodeSender.validate(checkCode: code) { - MXLog.error("Invalid code entered.") - state.state = .confirmCode(.invalidCode) - return - } - - state.state = .confirmCode(.sendingCode) - do { MXLog.info("Valid code entered, sending.") try await checkCodeSender.send(code: code) @@ -291,10 +318,15 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr state.state = .error(.cancelled) case .connectionInsecure: state.state = .error(.connectionNotSecure) + case .invalidCheckCode: + state.state = .confirmCode(.invalidCode) case .declined: 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: diff --git a/ElementX/Sources/Screens/QRCodeLoginScreen/View/QRCodeLoginScreen.swift b/ElementX/Sources/Screens/QRCodeLoginScreen/View/QRCodeLoginScreen.swift index 045740b7c..368587991 100644 --- a/ElementX/Sources/Screens/QRCodeLoginScreen/View/QRCodeLoginScreen.swift +++ b/ElementX/Sources/Screens/QRCodeLoginScreen/View/QRCodeLoginScreen.swift @@ -207,19 +207,26 @@ struct QRCodeLoginScreen: View { @ViewBuilder private var displayQRContent: some View { - if case let .displayQR(image) = context.viewState.state { + if case let .displayQR(qrState) = context.viewState.state { FullscreenDialog(topPadding: 24, horizontalPadding: 24) { VStack(spacing: 32) { TitleAndIcon(title: L10n.screenLinkNewDeviceMobileTitle(InfoPlistReader.main.productionAppName), icon: \.takePhotoSolid, iconStyle: .default) - Image(uiImage: image) - .interpolation(.none) // to stop it getting blurred - .resizable() - .scaledToFit() - .frame(width: 200, height: 200) - .accessibilityLabel(L10n.a11yQrCode) + 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) + } SFNumberedListView(items: context.viewState.instructions.linkMobileItems) } @@ -337,7 +344,8 @@ struct QRCodeLoginScreen_Previews: PreviewProvider, TestablePreview { static let deviceNotSignedInStateViewModel = QRCodeLoginScreenViewModel.mock(state: .scan(.scanFailed(.deviceNotSignedIn))) /// Showing - static let showingStateViewModel = QRCodeLoginScreenViewModel.mock(state: .displayQR(LinkNewDeviceServiceMock.mockQRCodeImage)) + static let showingStateViewModel = QRCodeLoginScreenViewModel.mock(state: .displayQR(.active(LinkNewDeviceServiceMock.mockQRCodeImage))) + static let showingExpiredStateViewModel = QRCodeLoginScreenViewModel.mock(state: .displayQR(.expired)) // Displaying codes static let deviceCodeStateViewModel = QRCodeLoginScreenViewModel.mock(state: .displayCode(.deviceCode("12"))) @@ -369,6 +377,8 @@ 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") diff --git a/ElementX/Sources/Services/Authentication/AuthenticationServiceProtocol.swift b/ElementX/Sources/Services/Authentication/AuthenticationServiceProtocol.swift index bb3bde9aa..ed4cb8dab 100644 --- a/ElementX/Sources/Services/Authentication/AuthenticationServiceProtocol.swift +++ b/ElementX/Sources/Services/Authentication/AuthenticationServiceProtocol.swift @@ -111,6 +111,7 @@ enum QRCodeLoginError: Error, Equatable { case providerNotAllowed(scannedProvider: String, allowedProviders: [String]) case cancelled case connectionInsecure + case invalidCheckCode case declined case linkingNotSupported case expired diff --git a/ElementX/Sources/Services/Authentication/LinkNewDeviceService.swift b/ElementX/Sources/Services/Authentication/LinkNewDeviceService.swift index c3aba5255..cf4e95661 100644 --- a/ElementX/Sources/Services/Authentication/LinkNewDeviceService.swift +++ b/ElementX/Sources/Services/Authentication/LinkNewDeviceService.swift @@ -195,8 +195,10 @@ extension LinkNewDeviceService.LinkDesktopProgress: CustomStringConvertible { private extension QRCodeLoginError { init(rustError: HumanQrGrantLoginError) { self = switch rustError { - case .InvalidCheckCode, .ConnectionInsecure: + case .ConnectionInsecure: .connectionInsecure + case .InvalidCheckCode: + .invalidCheckCode case .UnsupportedProtocol: .linkingNotSupported case .Expired, .NotFound, .DeviceNotFound: @@ -227,13 +229,6 @@ class CheckCodeSenderProxy: Equatable { func send(code: UInt8) async throws { try await underlyingSender.send(code: code) } - - /// Bypassed for now whilst we wait for an SDK update (however its worth noting that - /// things should still fail if the wrong code is provided, just not necessarily with - /// the right error being shown). https://github.com/matrix-org/matrix-rust-sdk/pull/5957 - func validate(checkCode: UInt8) -> Bool { - true - } } extension UIImage { diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/qRCodeLoginScreen.Showing-expired-iPad-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/qRCodeLoginScreen.Showing-expired-iPad-en-GB.png new file mode 100644 index 000000000..5804925f2 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/qRCodeLoginScreen.Showing-expired-iPad-en-GB.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:35b2338f432dd2d65581f94680382a33af87881bfd36e2a426a96f879ed1d93f +size 119474 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/qRCodeLoginScreen.Showing-expired-iPad-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/qRCodeLoginScreen.Showing-expired-iPad-pseudo.png new file mode 100644 index 000000000..502a70fb9 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/qRCodeLoginScreen.Showing-expired-iPad-pseudo.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:3f34b4d5fb95d223b8b1cf850fe74fc75d031da0a7f0a359b5a4c3e29fa83669 +size 142744 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/qRCodeLoginScreen.Showing-expired-iPhone-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/qRCodeLoginScreen.Showing-expired-iPhone-en-GB.png new file mode 100644 index 000000000..0e2b389d6 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/qRCodeLoginScreen.Showing-expired-iPhone-en-GB.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:c86cc8592649e4d3e65a6632fd02756455ffd48bbdc35b12d7db5a8ba147c988 +size 70650 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/qRCodeLoginScreen.Showing-expired-iPhone-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/qRCodeLoginScreen.Showing-expired-iPhone-pseudo.png new file mode 100644 index 000000000..b4cd8dde3 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/qRCodeLoginScreen.Showing-expired-iPhone-pseudo.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:2bbc55c8ae626a1e652864927dfa2eab2860c85dca9372cca73e0af982d7ca41 +size 96693 diff --git a/UnitTests/Sources/QRCodeLoginScreenViewModelTests.swift b/UnitTests/Sources/QRCodeLoginScreenViewModelTests.swift index 1db37eb07..e4e43ef3a 100644 --- a/UnitTests/Sources/QRCodeLoginScreenViewModelTests.swift +++ b/UnitTests/Sources/QRCodeLoginScreenViewModelTests.swift @@ -9,6 +9,7 @@ import Combine @testable import ElementX import MatrixRustSDKMocks +import SwiftUI import Testing @MainActor @@ -18,6 +19,7 @@ struct QRCodeLoginScreenViewModelTests { var qrLoginProgressSubject: CurrentValueSubject! var qrCodeLoginService: QRCodeLoginServiceMock! + var regeneratedQRCodeImage: UIImage! var linkMobileProgressSubject: CurrentValueSubject! var linkDesktopProgressSubject: CurrentValueSubject! var linkNewDeviceService: LinkNewDeviceServiceMock! @@ -202,6 +204,17 @@ 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) { @@ -214,6 +227,11 @@ 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: @@ -221,7 +239,7 @@ struct QRCodeLoginScreenViewModelTests { case .linkDesktop: screenMode = .linkDesktop(linkNewDeviceService) case .linkMobile: - screenMode = .linkMobile(linkNewDeviceService.linkMobileDevice()) + screenMode = .linkMobile(linkNewDeviceService.linkMobileDevice(), clientProxy) } appMediator = AppMediatorMock.default