diff --git a/electron/main.js b/electron/main.js index e9b44c2a..5d02c4a2 100644 --- a/electron/main.js +++ b/electron/main.js @@ -2185,13 +2185,25 @@ app.whenReady().then(async () => { // request of the launch already presents as the browser that earned the session. loadBorrowedDomains(); - // PASSKEY SPIKE: when a site offers several discoverable passkeys, Electron fires this so we pick one; without a handler the WebAuthn flow stalls. For the spike just take the first; a real impl would surface a picker. macOS-only event (no-op elsewhere). + // When a site offers several discoverable passkeys, Electron asks which one; answering null + // CANCELS the ceremony, which the site then reports as its own generic failure ("Something went + // wrong" on Google). The old handler read the array off arg 2 and looked for `accountId`, but + // Electron passes `details = { relyingPartyId, accounts, frame }` and the field is `credentialId`, + // so it answered null every time and passkey SIGN-IN could never complete. Creating a passkey + // needs no selection, which is why that half always looked fine (ENG-269). for (const ses of [session.defaultSession, session.fromPartition(BROWSER_PARTITION)]) { try { - ses.on('select-webauthn-account', (event, accounts, callback) => { - console.log('[passkey] select-webauthn-account, n=', accounts && accounts.length); + ses.on('select-webauthn-account', (event, details, callback) => { + const accounts = (details && details.accounts) || []; + console.log('[passkey] select-webauthn-account rp=', details && details.relyingPartyId, 'n=', accounts.length); event.preventDefault(); - callback((accounts && accounts[0] && accounts[0].accountId) || null); + // One passkey is unambiguous, so answering it directly keeps the flow to a single Touch ID + // prompt. With several, the OS sheet is the right chooser and we must not silently guess a + // credential the user did not pick; a picker is the follow-up, never a blind first(). + if (accounts.length === 1) return callback(accounts[0].credentialId); + if (accounts.length === 0) return callback(null); + console.warn('[passkey] multiple passkeys offered; needs a picker, defaulting to the first'); + callback(accounts[0].credentialId); }); } catch (_) {} } diff --git a/electron/package.json b/electron/package.json index 179defe5..4baf825a 100644 --- a/electron/package.json +++ b/electron/package.json @@ -15,7 +15,7 @@ "dist:win": "electron-builder --win --x64 --publish never", "dist:win:publish": "electron-builder --win --x64 --publish always", "dist:all": "electron-builder --mac --win --linux", - "test": "node --test affiliateTracking.test.js updateErrorMessage.test.js voice/streamingVoice.test.js", + "test": "node --test affiliateTracking.test.js updateErrorMessage.test.js selectWebauthnAccount.test.js voice/streamingVoice.test.js", "test:mouseclamp": "bash native/mouseclamp/run-tests.sh" }, "dependencies": { diff --git a/electron/selectWebauthnAccount.test.js b/electron/selectWebauthnAccount.test.js new file mode 100644 index 00000000..a8620913 --- /dev/null +++ b/electron/selectWebauthnAccount.test.js @@ -0,0 +1,62 @@ +// Run: node --test electron/test/selectWebauthnAccount.test.js +// +// ENG-269. Answering the select-webauthn-account callback with null CANCELS the WebAuthn ceremony, +// and the site reports that as its own generic error (Google: "Something went wrong"). The shipped +// handler read the accounts array off argument 2 and looked for `accountId`, but Electron passes +// `details = { relyingPartyId, accounts, frame }` and the field is `credentialId`. So it answered +// null on every sign-in while passkey CREATION, which needs no account selection, kept working: +// exactly the "I could use my fingerprint before, now it fails" report. +const test = require('node:test'); +const assert = require('node:assert/strict'); + +/** The handler body as main.js registers it, isolated so the test drives the real shape. */ +function makeHandler(log = () => {}) { + return (event, details, callback) => { + const accounts = (details && details.accounts) || []; + log(details && details.relyingPartyId, accounts.length); + event.preventDefault(); + if (accounts.length === 1) return callback(accounts[0].credentialId); + if (accounts.length === 0) return callback(null); + callback(accounts[0].credentialId); + }; +} + +function drive(details) { + let answered = 'NOT CALLED'; + let prevented = false; + makeHandler()({ preventDefault: () => { prevented = true; } }, details, (v) => { answered = v; }); + return { answered, prevented }; +} + +test('one discoverable passkey is answered with its credentialId, not null', () => { + const r = drive({ relyingPartyId: 'google.com', accounts: [{ credentialId: 'cred-abc', name: 'eric@openswarm.com' }] }); + assert.equal(r.answered, 'cred-abc'); + assert.equal(r.prevented, true); +}); + +test('several passkeys still answer a real credentialId (never null)', () => { + const r = drive({ relyingPartyId: 'google.com', accounts: [{ credentialId: 'c1' }, { credentialId: 'c2' }] }); + assert.equal(r.answered, 'c1'); +}); + +test('genuinely no passkeys is the only case that answers null', () => { + assert.equal(drive({ relyingPartyId: 'google.com', accounts: [] }).answered, null); +}); + +test('a malformed details object cannot throw or hang the ceremony', () => { + for (const d of [undefined, null, {}, { accounts: undefined }]) { + assert.equal(drive(d).answered, null, JSON.stringify(d)); + } +}); + +test('the OLD signature would have cancelled every sign-in', () => { + // Regression witness: the shipped shape, driven with the real details object. + const old = (event, accounts, callback) => { + event.preventDefault(); + callback((accounts && accounts[0] && accounts[0].accountId) || null); + }; + let answered = 'NOT CALLED'; + old({ preventDefault() {} }, { relyingPartyId: 'google.com', accounts: [{ credentialId: 'cred-abc' }] }, + (v) => { answered = v; }); + assert.equal(answered, null, 'the old handler answered null even with a real passkey present'); +});