From aeb3b9f94b95faf04b7d61d1c2da4c61f383f618 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=85berg?= Date: Mon, 31 Mar 2025 21:53:09 +0200 Subject: [PATCH 1/5] PM-19138: Add try-catch to desktop-autofill (#13964) --- .../services/desktop-autofill.service.ts | 132 +++++++++--------- 1 file changed, 66 insertions(+), 66 deletions(-) diff --git a/apps/desktop/src/autofill/services/desktop-autofill.service.ts b/apps/desktop/src/autofill/services/desktop-autofill.service.ts index 70a9280d8ef..9912e9459a1 100644 --- a/apps/desktop/src/autofill/services/desktop-autofill.service.ts +++ b/apps/desktop/src/autofill/services/desktop-autofill.service.ts @@ -147,7 +147,7 @@ export class DesktopAutofillService implements OnDestroy { } listenIpc() { - ipc.autofill.listenPasskeyRegistration((clientId, sequenceNumber, request, callback) => { + ipc.autofill.listenPasskeyRegistration(async (clientId, sequenceNumber, request, callback) => { this.logService.warning("listenPasskeyRegistration", clientId, sequenceNumber, request); this.logService.warning( "listenPasskeyRegistration2", @@ -155,19 +155,19 @@ export class DesktopAutofillService implements OnDestroy { ); const controller = new AbortController(); - void this.fido2AuthenticatorService - .makeCredential( + + try { + const response = await this.fido2AuthenticatorService.makeCredential( this.convertRegistrationRequest(request), { windowXy: request.windowXy }, controller, - ) - .then((response) => { - callback(null, this.convertRegistrationResponse(request, response)); - }) - .catch((error) => { - this.logService.error("listenPasskeyRegistration error", error); - callback(error, null); - }); + ); + + callback(null, this.convertRegistrationResponse(request, response)); + } catch (error) { + this.logService.error("listenPasskeyRegistration error", error); + callback(error, null); + } }); ipc.autofill.listenPasskeyAssertionWithoutUserInterface( @@ -179,55 +179,56 @@ export class DesktopAutofillService implements OnDestroy { request, ); - // For some reason the credentialId is passed as an empty array in the request, so we need to - // get it from the cipher. For that we use the recordIdentifier, which is the cipherId. - if (request.recordIdentifier && request.credentialId.length === 0) { - const activeUserId = await firstValueFrom( - this.accountService.activeAccount$.pipe(getOptionalUserId), - ); - if (!activeUserId) { - this.logService.error("listenPasskeyAssertion error", "Active user not found"); - callback(new Error("Active user not found"), null); - return; - } - - const cipher = await this.cipherService.get(request.recordIdentifier, activeUserId); - if (!cipher) { - this.logService.error("listenPasskeyAssertion error", "Cipher not found"); - callback(new Error("Cipher not found"), null); - return; - } - - const decrypted = await cipher.decrypt( - await this.cipherService.getKeyForCipherKeyDecryption(cipher, activeUserId), - ); - - const fido2Credential = decrypted.login.fido2Credentials?.[0]; - if (!fido2Credential) { - this.logService.error("listenPasskeyAssertion error", "Fido2Credential not found"); - callback(new Error("Fido2Credential not found"), null); - return; - } - - request.credentialId = Array.from( - parseCredentialId(decrypted.login.fido2Credentials?.[0].credentialId), - ); - } - const controller = new AbortController(); - void this.fido2AuthenticatorService - .getAssertion( + + try { + // For some reason the credentialId is passed as an empty array in the request, so we need to + // get it from the cipher. For that we use the recordIdentifier, which is the cipherId. + if (request.recordIdentifier && request.credentialId.length === 0) { + const activeUserId = await firstValueFrom( + this.accountService.activeAccount$.pipe(getOptionalUserId), + ); + if (!activeUserId) { + this.logService.error("listenPasskeyAssertion error", "Active user not found"); + callback(new Error("Active user not found"), null); + return; + } + + const cipher = await this.cipherService.get(request.recordIdentifier, activeUserId); + if (!cipher) { + this.logService.error("listenPasskeyAssertion error", "Cipher not found"); + callback(new Error("Cipher not found"), null); + return; + } + + const decrypted = await cipher.decrypt( + await this.cipherService.getKeyForCipherKeyDecryption(cipher, activeUserId), + ); + + const fido2Credential = decrypted.login.fido2Credentials?.[0]; + if (!fido2Credential) { + this.logService.error("listenPasskeyAssertion error", "Fido2Credential not found"); + callback(new Error("Fido2Credential not found"), null); + return; + } + + request.credentialId = Array.from( + parseCredentialId(decrypted.login.fido2Credentials?.[0].credentialId), + ); + } + + const response = await this.fido2AuthenticatorService.getAssertion( this.convertAssertionRequest(request), { windowXy: request.windowXy }, controller, - ) - .then((response) => { - callback(null, this.convertAssertionResponse(request, response)); - }) - .catch((error) => { - this.logService.error("listenPasskeyAssertion error", error); - callback(error, null); - }); + ); + + callback(null, this.convertAssertionResponse(request, response)); + } catch (error) { + this.logService.error("listenPasskeyAssertion error", error); + callback(error, null); + return; + } }, ); @@ -235,19 +236,18 @@ export class DesktopAutofillService implements OnDestroy { this.logService.warning("listenPasskeyAssertion", clientId, sequenceNumber, request); const controller = new AbortController(); - void this.fido2AuthenticatorService - .getAssertion( + try { + const response = await this.fido2AuthenticatorService.getAssertion( this.convertAssertionRequest(request), { windowXy: request.windowXy }, controller, - ) - .then((response) => { - callback(null, this.convertAssertionResponse(request, response)); - }) - .catch((error) => { - this.logService.error("listenPasskeyAssertion error", error); - callback(error, null); - }); + ); + + callback(null, this.convertAssertionResponse(request, response)); + } catch (error) { + this.logService.error("listenPasskeyAssertion error", error); + callback(error, null); + } }); } From bffce2272ded561e916ceb9de546ebaaa52f1d88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=85berg?= Date: Fri, 4 Apr 2025 16:50:02 +0200 Subject: [PATCH 2/5] works --- .../macos_provider/src/registration.rs | 1 + apps/desktop/desktop_native/napi/index.d.ts | 1 + apps/desktop/desktop_native/napi/src/lib.rs | 1 + .../CredentialProviderViewController.swift | 13 ++++++++++++- apps/desktop/package.json | 1 + 5 files changed, 16 insertions(+), 1 deletion(-) diff --git a/apps/desktop/desktop_native/macos_provider/src/registration.rs b/apps/desktop/desktop_native/macos_provider/src/registration.rs index 9e697b75c16..c961566a86c 100644 --- a/apps/desktop/desktop_native/macos_provider/src/registration.rs +++ b/apps/desktop/desktop_native/macos_provider/src/registration.rs @@ -14,6 +14,7 @@ pub struct PasskeyRegistrationRequest { user_verification: UserVerification, supported_algorithms: Vec, window_xy: Position, + excluded_credentials: Vec>, } #[derive(uniffi::Record, Serialize, Deserialize)] diff --git a/apps/desktop/desktop_native/napi/index.d.ts b/apps/desktop/desktop_native/napi/index.d.ts index ca1fe29e254..aad229d9b53 100644 --- a/apps/desktop/desktop_native/napi/index.d.ts +++ b/apps/desktop/desktop_native/napi/index.d.ts @@ -130,6 +130,7 @@ export declare namespace autofill { userVerification: UserVerification supportedAlgorithms: Array windowXy: Position + excludedCredentials: Array> } export interface PasskeyRegistrationResponse { rpId: string diff --git a/apps/desktop/desktop_native/napi/src/lib.rs b/apps/desktop/desktop_native/napi/src/lib.rs index f02be2b27b6..7ec714e626c 100644 --- a/apps/desktop/desktop_native/napi/src/lib.rs +++ b/apps/desktop/desktop_native/napi/src/lib.rs @@ -534,6 +534,7 @@ pub mod autofill { pub user_verification: UserVerification, pub supported_algorithms: Vec, pub window_xy: Position, + pub excluded_credentials: Vec>, } #[napi(object)] diff --git a/apps/desktop/macos/autofill-extension/CredentialProviderViewController.swift b/apps/desktop/macos/autofill-extension/CredentialProviderViewController.swift index 5568b2e75db..1b29c5c4684 100644 --- a/apps/desktop/macos/autofill-extension/CredentialProviderViewController.swift +++ b/apps/desktop/macos/autofill-extension/CredentialProviderViewController.swift @@ -246,6 +246,16 @@ class CredentialProviderViewController: ASCredentialProviderViewController { UserVerification.discouraged } + // Convert excluded credentials to an array of credential IDs + var excludedCredentialIds: [Data] = [] + if #available(macOSApplicationExtension 15.0, *) { + if let excludedCreds = request.excludedCredentials { + excludedCredentialIds = excludedCreds.map { $0.credentialID } + } + } else { + // Fallback on earlier versions + } + let req = PasskeyRegistrationRequest( rpId: passkeyIdentity.relyingPartyIdentifier, userName: passkeyIdentity.userName, @@ -253,7 +263,8 @@ class CredentialProviderViewController: ASCredentialProviderViewController { clientDataHash: request.clientDataHash, userVerification: userVerification, supportedAlgorithms: request.supportedAlgorithms.map{ Int32($0.rawValue) }, - windowXy: self.getWindowPosition() + windowXy: self.getWindowPosition(), + excludedCredentials: excludedCredentialIds ) logger.log("[autofill-extension] prepareInterface(passkey) calling preparePasskeyRegistration") diff --git a/apps/desktop/package.json b/apps/desktop/package.json index d4fe93d05b9..cff595009af 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -19,6 +19,7 @@ "postinstall": "electron-rebuild", "start": "cross-env ELECTRON_IS_DEV=0 ELECTRON_NO_UPDATER=1 electron ./build", "build-native": "cd desktop_native && node build.js", + "build-native-macos": "cd desktop_native && ./macos_provider/build.sh && node build.js cross-platform", "build": "cross-env NODE_OPTIONS=\"--max-old-space-size=8192\" concurrently -n Main,Rend,Prel -c yellow,cyan \"npm run build:main\" \"npm run build:renderer\" \"npm run build:preload\"", "build:dev": "concurrently -n Main,Rend -c yellow,cyan \"npm run build:main:dev\" \"npm run build:renderer:dev\"", "build:preload": "cross-env NODE_ENV=production webpack --config webpack.preload.js", From d56eaf48a9de7c1f76f8a234c2117b965bf5a574 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=85berg?= Date: Fri, 4 Apr 2025 17:08:09 +0200 Subject: [PATCH 3/5] Add mapping --- .../src/autofill/services/desktop-autofill.service.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/autofill/services/desktop-autofill.service.ts b/apps/desktop/src/autofill/services/desktop-autofill.service.ts index 9912e9459a1..d26b721c4c1 100644 --- a/apps/desktop/src/autofill/services/desktop-autofill.service.ts +++ b/apps/desktop/src/autofill/services/desktop-autofill.service.ts @@ -270,7 +270,10 @@ export class DesktopAutofillService implements OnDestroy { alg, type: "public-key", })), - excludeCredentialDescriptorList: [], + excludeCredentialDescriptorList: request.excludedCredentials.map((credentialId) => ({ + id: new Uint8Array(credentialId), + type: "public-key" as const, + })), requireResidentKey: true, requireUserVerification: request.userVerification === "required" || request.userVerification === "preferred", From bf22d9341108ee139bb1576bae8a1429a0ffc338 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=85berg?= Date: Fri, 4 Apr 2025 17:13:53 +0200 Subject: [PATCH 4/5] remove the build script --- apps/desktop/package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/desktop/package.json b/apps/desktop/package.json index cff595009af..d4fe93d05b9 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -19,7 +19,6 @@ "postinstall": "electron-rebuild", "start": "cross-env ELECTRON_IS_DEV=0 ELECTRON_NO_UPDATER=1 electron ./build", "build-native": "cd desktop_native && node build.js", - "build-native-macos": "cd desktop_native && ./macos_provider/build.sh && node build.js cross-platform", "build": "cross-env NODE_OPTIONS=\"--max-old-space-size=8192\" concurrently -n Main,Rend,Prel -c yellow,cyan \"npm run build:main\" \"npm run build:renderer\" \"npm run build:preload\"", "build:dev": "concurrently -n Main,Rend -c yellow,cyan \"npm run build:main:dev\" \"npm run build:renderer:dev\"", "build:preload": "cross-env NODE_ENV=production webpack --config webpack.preload.js", From 0c836e10c8225e703a63fd89febfb14de4853fdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=85berg?= Date: Fri, 4 Apr 2025 17:14:37 +0200 Subject: [PATCH 5/5] cleanup --- .../autofill-extension/CredentialProviderViewController.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/apps/desktop/macos/autofill-extension/CredentialProviderViewController.swift b/apps/desktop/macos/autofill-extension/CredentialProviderViewController.swift index 1b29c5c4684..9c5b404f464 100644 --- a/apps/desktop/macos/autofill-extension/CredentialProviderViewController.swift +++ b/apps/desktop/macos/autofill-extension/CredentialProviderViewController.swift @@ -252,8 +252,6 @@ class CredentialProviderViewController: ASCredentialProviderViewController { if let excludedCreds = request.excludedCredentials { excludedCredentialIds = excludedCreds.map { $0.credentialID } } - } else { - // Fallback on earlier versions } let req = PasskeyRegistrationRequest(