Link new device with QR code tweaks. (#5563)

* Hook up Rust's InvalidCheckCode failure to the right state when linking a new device.

* Automatically regenerate a new QR code instead of ending the flow when it expires.
This commit is contained in:
Doug
2026-05-11 14:22:34 +01:00
committed by GitHub
parent ded86dd66e
commit e235e3ba81
11 changed files with 106 additions and 31 deletions
@@ -51,7 +51,7 @@ class LinkNewDeviceFlowCoordinator: FlowCoordinatorProtocol {
switch action { switch action {
case .linkMobileDevice(let progressPublisher): case .linkMobileDevice(let progressPublisher):
presentQRCodeScreen(mode: .linkMobile(progressPublisher)) presentQRCodeScreen(mode: .linkMobile(progressPublisher, flowParameters.userSession.clientProxy))
case .linkDesktopComputer: case .linkDesktopComputer:
presentQRCodeScreen(mode: .linkDesktop(flowParameters.userSession.clientProxy.linkNewDeviceService())) presentQRCodeScreen(mode: .linkDesktop(flowParameters.userSession.clientProxy.linkNewDeviceService()))
case .dismiss: case .dismiss:
@@ -39,7 +39,7 @@ enum QRCodeLoginScreenMode {
/// Configures the screen to link another device by scanning a QR code. /// Configures the screen to link another device by scanning a QR code.
case linkDesktop(LinkNewDeviceServiceProtocol) case linkDesktop(LinkNewDeviceServiceProtocol)
/// Configures the screen to link another device by showing it a QR code. /// 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 { struct QRCodeLoginScreenViewState: BindableState {
@@ -107,7 +107,7 @@ enum QRCodeLoginState: Equatable {
case displayCode(DisplayCodeState) case displayCode(DisplayCodeState)
/// Initial state where the user can link another device using the shown QR code. /// 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 /// The user needs to enter the two digit code to confirm the channel is secure
case confirmCode(CheckCodeState) 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 { enum DisplayCodeState: Equatable {
case deviceCode(String) case deviceCode(String)
case verificationCode(String) case verificationCode(String)
@@ -30,9 +30,9 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr
let initialViewState: QRCodeLoginState = switch mode { let initialViewState: QRCodeLoginState = switch mode {
case .login: .loginInstructions case .login: .loginInstructions
case .linkDesktop: .linkDesktopInstructions case .linkDesktop: .linkDesktopInstructions
case .linkMobile(let progressPublisher): case .linkMobile(let progressPublisher, _):
switch progressPublisher.value { switch progressPublisher.value {
case .qrReady(let image): .displayQR(image) case .qrReady(let image): .displayQR(.active(image))
default: .error(.unknown) default: .error(.unknown)
} }
} }
@@ -40,7 +40,7 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr
super.init(initialViewState: .init(state: initialViewState, mode: mode, canSignInManually: canSignInManually)) super.init(initialViewState: .init(state: initialViewState, mode: mode, canSignInManually: canSignInManually))
setupSubscriptions() setupSubscriptions()
if case .linkMobile(let progressPublisher) = mode { if case .linkMobile(let progressPublisher, _) = mode {
listenToDisplayQRProgress(progressPublisher: progressPublisher) 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 { private func sendCheckCode() async {
guard case let .confirmCode(.inputCode(checkCodeSender)) = state.state else { guard case let .confirmCode(.inputCode(checkCodeSender)) = state.state else {
fatalError("Attempting to check code from the wrong state.") fatalError("Attempting to check code from the wrong state.")
} }
state.state = .confirmCode(.sendingCode)
let stringValue = state.bindings.checkCodeInput let stringValue = state.bindings.checkCodeInput
let code = UInt8(stringValue) ?? 0 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 { do {
MXLog.info("Valid code entered, sending.") MXLog.info("Valid code entered, sending.")
try await checkCodeSender.send(code: code) try await checkCodeSender.send(code: code)
@@ -291,10 +318,15 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr
state.state = .error(.cancelled) state.state = .error(.cancelled)
case .connectionInsecure: case .connectionInsecure:
state.state = .error(.connectionNotSecure) state.state = .error(.connectionNotSecure)
case .invalidCheckCode:
state.state = .confirmCode(.invalidCode)
case .declined: case .declined:
state.state = .error(.declined) state.state = .error(.declined)
case .linkingNotSupported: case .linkingNotSupported:
state.state = .error(.linkingNotSupported) state.state = .error(.linkingNotSupported)
case .expired where state.state.isDisplayQR:
state.state = .displayQR(.expired)
Task { await regenerateQRCode() }
case .expired: case .expired:
state.state = .error(.expired) state.state = .error(.expired)
case .slidingSyncNotAvailable: case .slidingSyncNotAvailable:
@@ -207,19 +207,26 @@ struct QRCodeLoginScreen: View {
@ViewBuilder @ViewBuilder
private var displayQRContent: some View { 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) { FullscreenDialog(topPadding: 24, horizontalPadding: 24) {
VStack(spacing: 32) { VStack(spacing: 32) {
TitleAndIcon(title: L10n.screenLinkNewDeviceMobileTitle(InfoPlistReader.main.productionAppName), TitleAndIcon(title: L10n.screenLinkNewDeviceMobileTitle(InfoPlistReader.main.productionAppName),
icon: \.takePhotoSolid, icon: \.takePhotoSolid,
iconStyle: .default) iconStyle: .default)
Image(uiImage: image) switch qrState {
.interpolation(.none) // to stop it getting blurred case .active(let image):
.resizable() Image(uiImage: image)
.scaledToFit() .interpolation(.none) // to stop it getting blurred
.frame(width: 200, height: 200) .resizable()
.accessibilityLabel(L10n.a11yQrCode) .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) SFNumberedListView(items: context.viewState.instructions.linkMobileItems)
} }
@@ -337,7 +344,8 @@ struct QRCodeLoginScreen_Previews: PreviewProvider, TestablePreview {
static let deviceNotSignedInStateViewModel = QRCodeLoginScreenViewModel.mock(state: .scan(.scanFailed(.deviceNotSignedIn))) static let deviceNotSignedInStateViewModel = QRCodeLoginScreenViewModel.mock(state: .scan(.scanFailed(.deviceNotSignedIn)))
/// Showing /// 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 // Displaying codes
static let deviceCodeStateViewModel = QRCodeLoginScreenViewModel.mock(state: .displayCode(.deviceCode("12"))) static let deviceCodeStateViewModel = QRCodeLoginScreenViewModel.mock(state: .displayCode(.deviceCode("12")))
@@ -369,6 +377,8 @@ struct QRCodeLoginScreen_Previews: PreviewProvider, TestablePreview {
ElementNavigationStack { QRCodeLoginScreen(context: showingStateViewModel.context) } ElementNavigationStack { QRCodeLoginScreen(context: showingStateViewModel.context) }
.previewDisplayName("Showing") .previewDisplayName("Showing")
ElementNavigationStack { QRCodeLoginScreen(context: showingExpiredStateViewModel.context) }
.previewDisplayName("Showing expired")
ElementNavigationStack { QRCodeLoginScreen(context: deviceCodeStateViewModel.context) } ElementNavigationStack { QRCodeLoginScreen(context: deviceCodeStateViewModel.context) }
.previewDisplayName("Device code") .previewDisplayName("Device code")
@@ -111,6 +111,7 @@ enum QRCodeLoginError: Error, Equatable {
case providerNotAllowed(scannedProvider: String, allowedProviders: [String]) case providerNotAllowed(scannedProvider: String, allowedProviders: [String])
case cancelled case cancelled
case connectionInsecure case connectionInsecure
case invalidCheckCode
case declined case declined
case linkingNotSupported case linkingNotSupported
case expired case expired
@@ -195,8 +195,10 @@ extension LinkNewDeviceService.LinkDesktopProgress: CustomStringConvertible {
private extension QRCodeLoginError { private extension QRCodeLoginError {
init(rustError: HumanQrGrantLoginError) { init(rustError: HumanQrGrantLoginError) {
self = switch rustError { self = switch rustError {
case .InvalidCheckCode, .ConnectionInsecure: case .ConnectionInsecure:
.connectionInsecure .connectionInsecure
case .InvalidCheckCode:
.invalidCheckCode
case .UnsupportedProtocol: case .UnsupportedProtocol:
.linkingNotSupported .linkingNotSupported
case .Expired, .NotFound, .DeviceNotFound: case .Expired, .NotFound, .DeviceNotFound:
@@ -227,13 +229,6 @@ class CheckCodeSenderProxy: Equatable {
func send(code: UInt8) async throws { func send(code: UInt8) async throws {
try await underlyingSender.send(code: code) 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 { extension UIImage {
@@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:35b2338f432dd2d65581f94680382a33af87881bfd36e2a426a96f879ed1d93f
size 119474
@@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:3f34b4d5fb95d223b8b1cf850fe74fc75d031da0a7f0a359b5a4c3e29fa83669
size 142744
@@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:c86cc8592649e4d3e65a6632fd02756455ffd48bbdc35b12d7db5a8ba147c988
size 70650
@@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:2bbc55c8ae626a1e652864927dfa2eab2860c85dca9372cca73e0af982d7ca41
size 96693
@@ -9,6 +9,7 @@
import Combine import Combine
@testable import ElementX @testable import ElementX
import MatrixRustSDKMocks import MatrixRustSDKMocks
import SwiftUI
import Testing import Testing
@MainActor @MainActor
@@ -18,6 +19,7 @@ struct QRCodeLoginScreenViewModelTests {
var qrLoginProgressSubject: CurrentValueSubject<QRLoginProgress, AuthenticationServiceError>! var qrLoginProgressSubject: CurrentValueSubject<QRLoginProgress, AuthenticationServiceError>!
var qrCodeLoginService: QRCodeLoginServiceMock! var qrCodeLoginService: QRCodeLoginServiceMock!
var regeneratedQRCodeImage: UIImage!
var linkMobileProgressSubject: CurrentValueSubject<LinkNewDeviceService.LinkMobileProgress, QRCodeLoginError>! var linkMobileProgressSubject: CurrentValueSubject<LinkNewDeviceService.LinkMobileProgress, QRCodeLoginError>!
var linkDesktopProgressSubject: CurrentValueSubject<LinkNewDeviceService.LinkDesktopProgress, QRCodeLoginError>! var linkDesktopProgressSubject: CurrentValueSubject<LinkNewDeviceService.LinkDesktopProgress, QRCodeLoginError>!
var linkNewDeviceService: LinkNewDeviceServiceMock! var linkNewDeviceService: LinkNewDeviceServiceMock!
@@ -202,6 +204,17 @@ struct QRCodeLoginScreenViewModelTests {
try await deferredAction.fulfill() 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 // MARK: - Helpers
private mutating func setup(mode: Mode) { private mutating func setup(mode: Mode) {
@@ -214,6 +227,11 @@ struct QRCodeLoginScreenViewModelTests {
linkNewDeviceService = LinkNewDeviceServiceMock(.init(linkMobileProgressPublisher: linkMobileProgressSubject.asCurrentValuePublisher(), linkNewDeviceService = LinkNewDeviceServiceMock(.init(linkMobileProgressPublisher: linkMobileProgressSubject.asCurrentValuePublisher(),
linkDesktopProgressPublisher: linkDesktopProgressSubject.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 let screenMode: QRCodeLoginScreenMode
switch mode { switch mode {
case .login: case .login:
@@ -221,7 +239,7 @@ struct QRCodeLoginScreenViewModelTests {
case .linkDesktop: case .linkDesktop:
screenMode = .linkDesktop(linkNewDeviceService) screenMode = .linkDesktop(linkNewDeviceService)
case .linkMobile: case .linkMobile:
screenMode = .linkMobile(linkNewDeviceService.linkMobileDevice()) screenMode = .linkMobile(linkNewDeviceService.linkMobileDevice(), clientProxy)
} }
appMediator = AppMediatorMock.default appMediator = AppMediatorMock.default