From afa6dff0b9c8142488b71e0c4abe738bdac0c16f Mon Sep 17 00:00:00 2001 From: Edgar Story Date: Sun, 10 Nov 2024 12:16:17 +0000 Subject: [PATCH 1/8] =?UTF-8?q?Support=20FIDO=20authentication=20with=20de?= =?UTF-8?q?vices=20that=20don=E2=80=99t=20have=20a=20PIN=20code=20set?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Xcodes/Backend/AppState.swift | 2 +- .../Frontend/SignIn/SignInSecurityKeyPinView.swift | 7 +++++++ Xcodes/Resources/Localizable.xcstrings | 13 +++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/Xcodes/Backend/AppState.swift b/Xcodes/Backend/AppState.swift index 475602fe..c2164355 100644 --- a/Xcodes/Backend/AppState.swift +++ b/Xcodes/Backend/AppState.swift @@ -357,7 +357,7 @@ class AppState: ObservableObject { var fido2: FIDO2? - func createAndSubmitSecurityKeyAssertationWithPinCode(_ pinCode: String, sessionData: AppleSessionData, authOptions: AuthOptionsResponse) { + func createAndSubmitSecurityKeyAssertationWithPinCode(_ pinCode: String?, sessionData: AppleSessionData, authOptions: AuthOptionsResponse) { self.presentedSheet = .securityKeyTouchToConfirm guard let fsaChallenge = authOptions.fsaChallenge else { diff --git a/Xcodes/Frontend/SignIn/SignInSecurityKeyPinView.swift b/Xcodes/Frontend/SignIn/SignInSecurityKeyPinView.swift index 32b6028c..d2b16465 100644 --- a/Xcodes/Frontend/SignIn/SignInSecurityKeyPinView.swift +++ b/Xcodes/Frontend/SignIn/SignInSecurityKeyPinView.swift @@ -32,6 +32,9 @@ struct SignInSecurityKeyPinView: View { Button("Cancel", action: { isPresented = false }) .keyboardShortcut(.cancelAction) Spacer() + + Button("PIN not set", action: submitWithoutPinCode) + ProgressButton(isInProgress: appState.isProcessingAuthRequest, action: submitPinCode) { Text("Continue") @@ -50,6 +53,10 @@ struct SignInSecurityKeyPinView: View { func submitPinCode() { appState.createAndSubmitSecurityKeyAssertationWithPinCode(pin, sessionData: sessionData, authOptions: authOptions) } + + func submitWithoutPinCode() { + appState.createAndSubmitSecurityKeyAssertationWithPinCode(nil, sessionData: sessionData, authOptions: authOptions) + } } #Preview { diff --git a/Xcodes/Resources/Localizable.xcstrings b/Xcodes/Resources/Localizable.xcstrings index 24fc217b..e80c58bd 100644 --- a/Xcodes/Resources/Localizable.xcstrings +++ b/Xcodes/Resources/Localizable.xcstrings @@ -237,6 +237,16 @@ } } }, + "%@ (%@)" : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "%1$@ (%2$@)" + } + } + } + }, "%@ %@ %@" : { "localizations" : { "ar" : { @@ -17907,6 +17917,9 @@ } } } + }, + "PIN not set" : { + }, "Platforms" : { "localizations" : { From 2dc1bcdcbb63321019af70d6b9fa71fb538dabbe Mon Sep 17 00:00:00 2001 From: Edgar Story Date: Sun, 10 Nov 2024 22:06:14 +0000 Subject: [PATCH 2/8] Update requirement for LibFido2Swift library to be at least 0.1.3 --- Xcodes.xcodeproj/project.pbxproj | 2 +- .../project.xcworkspace/xcshareddata/swiftpm/Package.resolved | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Xcodes.xcodeproj/project.pbxproj b/Xcodes.xcodeproj/project.pbxproj index 4b79c55c..4648868c 100644 --- a/Xcodes.xcodeproj/project.pbxproj +++ b/Xcodes.xcodeproj/project.pbxproj @@ -1493,7 +1493,7 @@ repositoryURL = "https://github.com/kinoroy/LibFido2Swift.git"; requirement = { kind = upToNextMinorVersion; - minimumVersion = 0.1.0; + minimumVersion = 0.1.3; }; }; CA9FF86B25951C6E00E47BAF /* XCRemoteSwiftPackageReference "data" */ = { diff --git a/Xcodes.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Xcodes.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index f90bb44b..9da9975c 100644 --- a/Xcodes.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Xcodes.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -78,8 +78,8 @@ "repositoryURL": "https://github.com/kinoroy/LibFido2Swift.git", "state": { "branch": null, - "revision": "b77e5c6451bea69d15615d6578936b11777d9a6c", - "version": "0.1.2" + "revision": "ba96f1390db486b667c22bed2fe95579e047af88", + "version": "0.1.3" } }, { From 3d9cf73fc1e8f25262c99c35537097d1a47171a9 Mon Sep 17 00:00:00 2001 From: Edgar Story Date: Tue, 12 Nov 2024 09:02:45 +0000 Subject: [PATCH 3/8] Require LibFido2Swift to be 0.1.4 at least - This has the new functionality for checking if a device is attached. --- Xcodes.xcodeproj/project.pbxproj | 2 +- .../project.xcworkspace/xcshareddata/swiftpm/Package.resolved | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Xcodes.xcodeproj/project.pbxproj b/Xcodes.xcodeproj/project.pbxproj index 4648868c..dc39b3af 100644 --- a/Xcodes.xcodeproj/project.pbxproj +++ b/Xcodes.xcodeproj/project.pbxproj @@ -1493,7 +1493,7 @@ repositoryURL = "https://github.com/kinoroy/LibFido2Swift.git"; requirement = { kind = upToNextMinorVersion; - minimumVersion = 0.1.3; + minimumVersion = 0.1.4; }; }; CA9FF86B25951C6E00E47BAF /* XCRemoteSwiftPackageReference "data" */ = { diff --git a/Xcodes.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Xcodes.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 9da9975c..84e33365 100644 --- a/Xcodes.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Xcodes.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -78,8 +78,8 @@ "repositoryURL": "https://github.com/kinoroy/LibFido2Swift.git", "state": { "branch": null, - "revision": "ba96f1390db486b667c22bed2fe95579e047af88", - "version": "0.1.3" + "revision": "94d496d6f850dcbb3e8c4a27cd7eeabfad9f14e3", + "version": "0.1.4" } }, { From 36424a78e0b020ca475214ff44f41f04bdd94ccc Mon Sep 17 00:00:00 2001 From: Edgar Story Date: Tue, 12 Nov 2024 09:08:26 +0000 Subject: [PATCH 4/8] Make fido2 property a lazy var MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - This object was being kept around after being created and as we need it in some other functions it made sense to make it lazy and keep it around that way. - Arguably the FIDO2 instance could be removed after each time it’s been used, but as the FIDO2 class doesn’t have any state stored in it, it seems benign keeping it about for now. --- Xcodes/Backend/AppState.swift | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Xcodes/Backend/AppState.swift b/Xcodes/Backend/AppState.swift index c2164355..1d663053 100644 --- a/Xcodes/Backend/AppState.swift +++ b/Xcodes/Backend/AppState.swift @@ -355,8 +355,8 @@ class AppState: ObservableObject { .store(in: &cancellables) } - var fido2: FIDO2? - + private lazy var fido2 = FIDO2() + func createAndSubmitSecurityKeyAssertationWithPinCode(_ pinCode: String?, sessionData: AppleSessionData, authOptions: AuthOptionsResponse) { self.presentedSheet = .securityKeyTouchToConfirm @@ -379,8 +379,6 @@ class AppState: ObservableObject { Task { do { - let fido2 = FIDO2() - self.fido2 = fido2 let response = try fido2.respondToChallenge(args: ChallengeArgs(rpId: rpId, validCredentials: validCreds, devPin: pinCode, challenge: challenge, origin: origin)) Task { @MainActor in @@ -413,7 +411,7 @@ class AppState: ObservableObject { } func cancelSecurityKeyAssertationRequest() { - self.fido2?.cancel() + self.fido2.cancel() } private func handleAuthenticationFlowCompletion(_ completion: Subscribers.Completion) { From cfef2879b51781914e36c949ff8899222e2a53bf Mon Sep 17 00:00:00 2001 From: Edgar Story Date: Tue, 12 Nov 2024 09:12:21 +0000 Subject: [PATCH 5/8] Add function to check if fido2 device needs a PIN --- Xcodes/Backend/AppState.swift | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Xcodes/Backend/AppState.swift b/Xcodes/Backend/AppState.swift index 1d663053..a99b6e72 100644 --- a/Xcodes/Backend/AppState.swift +++ b/Xcodes/Backend/AppState.swift @@ -409,6 +409,15 @@ class AppState: ObservableObject { } } } + + func fido2DeviceNeedsPin() -> Bool { + do { + return try fido2.deviceHasPin() + } catch { + authError = error + return true + } + } func cancelSecurityKeyAssertationRequest() { self.fido2.cancel() From a43bf63aab9e984320ac883747b94e39974bd1b4 Mon Sep 17 00:00:00 2001 From: Edgar Story Date: Tue, 12 Nov 2024 09:12:44 +0000 Subject: [PATCH 6/8] Add function to check if a FIDO2 device is even connected --- Xcodes/Backend/AppState.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Xcodes/Backend/AppState.swift b/Xcodes/Backend/AppState.swift index a99b6e72..6ea6bc5d 100644 --- a/Xcodes/Backend/AppState.swift +++ b/Xcodes/Backend/AppState.swift @@ -410,6 +410,10 @@ class AppState: ObservableObject { } } + func fido2DeviceIsPresent() -> Bool { + fido2.hasDeviceAttached() + } + func fido2DeviceNeedsPin() -> Bool { do { return try fido2.deviceHasPin() From cc0366057630d38f3da7596cc168b850d794d595 Mon Sep 17 00:00:00 2001 From: Edgar Story Date: Tue, 12 Nov 2024 09:14:51 +0000 Subject: [PATCH 7/8] Push the setting of `authError` to happen on MainActor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The Xcode “Run Time Issue” breakpoint was being hit whenever an error was being set, complaining about this being set outside of the main thread. --- Xcodes/Backend/AppState.swift | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Xcodes/Backend/AppState.swift b/Xcodes/Backend/AppState.swift index 6ea6bc5d..d855d242 100644 --- a/Xcodes/Backend/AppState.swift +++ b/Xcodes/Backend/AppState.swift @@ -405,7 +405,9 @@ class AppState: ObservableObject { // we don't have to show an error // because the sheet will already be dismissed } catch { - authError = error + Task { @MainActor in + authError = error + } } } } @@ -418,7 +420,10 @@ class AppState: ObservableObject { do { return try fido2.deviceHasPin() } catch { - authError = error + Task { @MainActor in + authError = error + } + return true } } From 259ad0789a61e5218cf8f8b8558697fa9fea23c8 Mon Sep 17 00:00:00 2001 From: Edgar Story Date: Tue, 12 Nov 2024 09:20:38 +0000 Subject: [PATCH 8/8] Improve the support for PIN-less FIDO2 devices - We now check if in the handling of two factor option, the option to be used is a SecurityKey. If so, check if a FIDO2 device is attached and if it needs a PIN. - When a PIN is not required, we can just move straight onto assertation, the code for which will present the touch key UI. - Otherwise we fallback to the original flow. --- Xcodes/Backend/AppState.swift | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/Xcodes/Backend/AppState.swift b/Xcodes/Backend/AppState.swift index d855d242..e3997a11 100644 --- a/Xcodes/Backend/AppState.swift +++ b/Xcodes/Backend/AppState.swift @@ -305,11 +305,17 @@ class AppState: ObservableObject { } func handleTwoFactorOption(_ option: TwoFactorOption, authOptions: AuthOptionsResponse, serviceKey: String, sessionID: String, scnt: String) { - self.presentedSheet = .twoFactor(.init( - option: option, - authOptions: authOptions, - sessionData: AppleSessionData(serviceKey: serviceKey, sessionID: sessionID, scnt: scnt) - )) + let sessionData = AppleSessionData(serviceKey: serviceKey, sessionID: sessionID, scnt: scnt) + + if option == .securityKey, fido2DeviceIsPresent() && !fido2DeviceNeedsPin() { + createAndSubmitSecurityKeyAssertationWithPinCode(nil, sessionData: sessionData, authOptions: authOptions) + } else { + self.presentedSheet = .twoFactor(.init( + option: option, + authOptions: authOptions, + sessionData: sessionData + )) + } } func requestSMS(to trustedPhoneNumber: AuthOptionsResponse.TrustedPhoneNumber, authOptions: AuthOptionsResponse, sessionData: AppleSessionData) {