From a9a1b3ad3d992617d40dd57ef41fd44949f95fbe Mon Sep 17 00:00:00 2001 From: Jens Utbult Date: Mon, 25 Nov 2024 18:04:44 +0100 Subject: [PATCH 1/3] Continuation was sometimes called twice. OATH error handlig improved. --- Authenticator/Model/MainViewModel.swift | 4 +-- Authenticator/Model/OATHSession.swift | 35 ++++++++++++++++++------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/Authenticator/Model/MainViewModel.swift b/Authenticator/Model/MainViewModel.swift index 2b30a6c1..d1159b78 100644 --- a/Authenticator/Model/MainViewModel.swift +++ b/Authenticator/Model/MainViewModel.swift @@ -106,15 +106,13 @@ class MainViewModel: ObservableObject { } } } catch { - self?.sessionTask?.cancel() // Only handle .otpEnabledError by presenting the disable OTP modal if let sessionError = error as? OATHSessionError, sessionError == .otpEnabledError { - self?.sessionTask?.cancel() self?.presentDisableOTP = true } else { - guard let oathSessionError = error as? OATHSessionError, oathSessionError != .connectionCancelled else { return } self?.connectionError = error } + self?.sessionTask?.cancel() } } } diff --git a/Authenticator/Model/OATHSession.swift b/Authenticator/Model/OATHSession.swift index 8508d0d5..6481de32 100644 --- a/Authenticator/Model/OATHSession.swift +++ b/Authenticator/Model/OATHSession.swift @@ -146,12 +146,27 @@ class OATHSessionHandler: NSObject, YKFManagerDelegate { self.wiredConnectionCallback = { connection in if connection.isKind(of: YKFSmartCardConnection.self) && deviceType == .phone { connection.managementSession { session, error in - guard let session else { continuation.resume(throwing: error!); return } + guard let session else { + self.wiredContinuation?.resume(throwing: error!) + self.wiredContinuation = nil + self.wiredConnectionCallback = nil + return + } session.getDeviceInfo { deviceInfo, error in - guard let deviceInfo else { continuation.resume(throwing: error!); return } - guard let configuration = deviceInfo.configuration else { continuation.resume(throwing: "Error!!!"); return } + guard let deviceInfo else { + self.wiredContinuation?.resume(throwing: error!) + self.wiredContinuation = nil + self.wiredConnectionCallback = nil + return + } + guard let configuration = deviceInfo.configuration else { + self.wiredContinuation?.resume(throwing: "Error!!!") + self.wiredContinuation = nil + self.wiredConnectionCallback = nil + return + } guard !configuration.isEnabled(.OTP, overTransport: .USB) || SettingsConfig.isOTPOverUSBIgnored(deviceId: deviceInfo.serialNumber) else { - continuation.resume(throwing: OATHSessionError.otpEnabledError) + self.wiredContinuation?.resume(throwing: OATHSessionError.otpEnabledError) self.wiredContinuation = nil self.wiredConnectionCallback = nil return @@ -159,9 +174,9 @@ class OATHSessionHandler: NSObject, YKFManagerDelegate { connection.oathSession { session, error in if let session { self.currentSession = session - continuation.resume(returning: OATHSession(session: session, type: .wired)) + self.wiredContinuation?.resume(returning: OATHSession(session: session, type: .wired)) } else { - continuation.resume(throwing: error!) + self.wiredContinuation?.resume(throwing: error!) } self.wiredContinuation = nil self.wiredConnectionCallback = nil @@ -173,9 +188,9 @@ class OATHSessionHandler: NSObject, YKFManagerDelegate { connection.oathSession { session, error in if let session { self.currentSession = session - continuation.resume(returning: OATHSession(session: session, type: .wired)) + self.wiredContinuation?.resume(returning: OATHSession(session: session, type: .wired)) } else { - continuation.resume(throwing: error!) + self.wiredContinuation?.resume(throwing: error!) } self.wiredContinuation = nil self.wiredConnectionCallback = nil @@ -202,9 +217,9 @@ class OATHSessionHandler: NSObject, YKFManagerDelegate { connection.oathSession { session, error in if let session { self.currentSession = session - continuation.resume(returning: OATHSession(session: session, type: .nfc)) + self.nfcContinuation?.resume(returning: OATHSession(session: session, type: .nfc)) } else { - continuation.resume(throwing: error!) + self.nfcContinuation?.resume(throwing: error!) } self.nfcContinuation = nil } From fbcd9d67eae182bd3f232822efcbcd44e989419f Mon Sep 17 00:00:00 2001 From: Jens Utbult Date: Tue, 26 Nov 2024 09:43:42 +0100 Subject: [PATCH 2/3] Fix bug where a cancellation error would be passed on to a alert when it should be ignored. --- Authenticator/Model/MainViewModel.swift | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Authenticator/Model/MainViewModel.swift b/Authenticator/Model/MainViewModel.swift index d1159b78..6e64ad00 100644 --- a/Authenticator/Model/MainViewModel.swift +++ b/Authenticator/Model/MainViewModel.swift @@ -106,9 +106,12 @@ class MainViewModel: ObservableObject { } } } catch { - // Only handle .otpEnabledError by presenting the disable OTP modal - if let sessionError = error as? OATHSessionError, sessionError == .otpEnabledError { - self?.presentDisableOTP = true + if let sessionError = error as? OATHSessionError { + if sessionError == .otpEnabledError { + self?.presentDisableOTP = true + } else if sessionError != .connectionCancelled { + self?.connectionError = error + } } else { self?.connectionError = error } From ee6039d7ee5ca0fbd862eeaafe8c5312a29d1bba Mon Sep 17 00:00:00 2001 From: Jens Utbult Date: Tue, 26 Nov 2024 09:55:24 +0100 Subject: [PATCH 3/3] Better error type thrown when device info is invalid. --- Authenticator/Model/OATHSession.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Authenticator/Model/OATHSession.swift b/Authenticator/Model/OATHSession.swift index 6481de32..e68b5718 100644 --- a/Authenticator/Model/OATHSession.swift +++ b/Authenticator/Model/OATHSession.swift @@ -21,6 +21,7 @@ enum OATHSessionError: Error, LocalizedError, Equatable { case credentialAlreadyPresent(YKFOATHCredentialTemplate) case otpEnabledError case connectionCancelled + case invalidDeviceInfo public var errorDescription: String? { switch self { @@ -30,6 +31,8 @@ enum OATHSessionError: Error, LocalizedError, Equatable { return String(localized: "Yubico OTP enabled", comment: "OATH otp enabled error message") case .connectionCancelled: return String(localized: "Connection cancelled by user", comment: "Internal error message not to be displayed to the user.") + case .invalidDeviceInfo: + return String(localized: "Invalid device info received from YubiKey", comment: "Internal error message not to be displayed to the user.") } } } @@ -160,7 +163,7 @@ class OATHSessionHandler: NSObject, YKFManagerDelegate { return } guard let configuration = deviceInfo.configuration else { - self.wiredContinuation?.resume(throwing: "Error!!!") + self.wiredContinuation?.resume(throwing: OATHSessionError.invalidDeviceInfo) self.wiredContinuation = nil self.wiredConnectionCallback = nil return