Skip to content

Commit bd59c5c

Browse files
committed
fix(desktop): autofill identifier-first sign-ins
1 parent 9b22924 commit bd59c5c

3 files changed

Lines changed: 103 additions & 10 deletions

File tree

apps/desktop/src/main/browser-credentials/fill.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,50 @@ describe('performing a fill', () => {
195195
})
196196
})
197197

198+
it('fills the email step of a two-step sign-in without sending the password', async () => {
199+
const context = setup()
200+
context.coordinator.noteFormState(context.contents as unknown as WebContents, {
201+
origin: ORIGIN,
202+
hasLoginForm: true,
203+
hasPasswordField: false,
204+
})
205+
await context.coordinator.showChooser(WINDOW, { x: 10, y: 20 })
206+
const template = vi.mocked(Menu.buildFromTemplate).mock.calls.at(-1)?.[0] as Array<{
207+
click: () => void
208+
}>
209+
210+
template[0].click()
211+
await settle()
212+
213+
// The page has nowhere to put a password, so it does not get one.
214+
expect(context.contents.send).toHaveBeenCalledWith('browser-credentials:fill', {
215+
origin: ORIGIN,
216+
username: 'ada',
217+
password: undefined,
218+
})
219+
})
220+
221+
it('sends the password to a shell that never reported whether a field exists', async () => {
222+
const context = setup()
223+
// Older preloads omit the flag; assuming a password field keeps them working.
224+
context.coordinator.noteFormState(context.contents as unknown as WebContents, {
225+
origin: ORIGIN,
226+
hasLoginForm: true,
227+
})
228+
await context.coordinator.showChooser(WINDOW, { x: 10, y: 20 })
229+
const template = vi.mocked(Menu.buildFromTemplate).mock.calls.at(-1)?.[0] as Array<{
230+
click: () => void
231+
}>
232+
233+
template[0].click()
234+
await settle()
235+
236+
expect(context.contents.send).toHaveBeenCalledWith(
237+
'browser-credentials:fill',
238+
expect.objectContaining({ password: 'hunter2' })
239+
)
240+
})
241+
198242
it('refuses after the page navigated between choosing and clicking', async () => {
199243
const context = setup()
200244
const template = await openChooser(context)

apps/desktop/src/main/browser-credentials/fill.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,15 @@ const logger = createLogger('BrowserCredentialFill')
2626
interface FormState {
2727
origin: string
2828
hasLoginForm: boolean
29+
/**
30+
* Whether the page currently has somewhere to put a password.
31+
*
32+
* False on the first step of an identifier-first sign-in, which asks for an
33+
* email and only reveals the password field after it is submitted. Those
34+
* steps are still worth filling — the username is what they want — so the
35+
* password simply is not sent to a page that has nowhere to put it.
36+
*/
37+
hasPasswordField: boolean
2938
/** Bumped on every navigation, so a stale authorization cannot be replayed. */
3039
generation: number
3140
}
@@ -41,6 +50,8 @@ export interface FillCoordinatorDeps {
4150
export interface FormStateReport {
4251
origin: string
4352
hasLoginForm: boolean
53+
/** Absent from shells that predate identifier-first support; assumed true. */
54+
hasPasswordField?: boolean
4455
}
4556

4657
export class FillCoordinator {
@@ -67,6 +78,7 @@ export class FillCoordinator {
6778
this.states.set(contents, {
6879
origin,
6980
hasLoginForm: report.hasLoginForm,
81+
hasPasswordField: report.hasPasswordField ?? true,
7082
generation: this.generationFor(contents),
7183
})
7284
}
@@ -166,7 +178,9 @@ export class FillCoordinator {
166178
contents.send('browser-credentials:fill', {
167179
origin: state.origin,
168180
username: credential.username,
169-
password: credential.password,
181+
// Withheld on an identifier-first step: the page has no password field,
182+
// so sending it would put plaintext in a document that cannot use it.
183+
password: state.hasPasswordField ? credential.password : undefined,
170184
})
171185
// Counts and outcomes only — never the origin, username, or password.
172186
logger.info('Filled a saved credential at the user\u2019s request')

apps/desktop/src/preload/browser/index.ts

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ const RESCAN_DEBOUNCE_MS = 250
2323

2424
interface DetectedForm {
2525
username: HTMLInputElement | null
26-
password: HTMLInputElement
26+
/** Absent on an identifier-first step, which asks for the email alone. */
27+
password: HTMLInputElement | null
2728
}
2829

2930
/** Held only in this isolated world; never serialized to main or the page. */
@@ -74,15 +75,46 @@ function findUsernameField(password: HTMLInputElement): HTMLInputElement | null
7475
return preceding.at(-1) ?? candidates[0] ?? null
7576
}
7677

77-
function reportFormState(): void {
78+
/**
79+
* The identifier field of a sign-in step that has no password field yet.
80+
*
81+
* Two-step sign-in — email, Continue, then the password on the next screen —
82+
* is now the norm at Google, Okta, and most workplace tools. Requiring a
83+
* password field would mean the key icon never appears on the step where the
84+
* user actually needs it. Only the page's own declaration counts here: an
85+
* `autocomplete` token naming a username or email, or an email input. That is
86+
* narrow enough to leave newsletter boxes and search fields alone.
87+
*/
88+
function findIdentifierField(): HTMLInputElement | null {
89+
for (const field of document.querySelectorAll('input')) {
90+
if (!isFillable(field)) continue
91+
const hint = String(field.getAttribute('autocomplete') || '').toLowerCase()
92+
if (hint === 'username' || hint === 'email') return field
93+
if (String(field.type || '').toLowerCase() === 'email') return field
94+
}
95+
return null
96+
}
97+
98+
function detectForm(): DetectedForm | null {
7899
const password = findPasswordField()
79-
detected = password ? { password, username: findUsernameField(password) } : null
100+
if (password) return { password, username: findUsernameField(password) }
101+
const username = findIdentifierField()
102+
return username ? { password: null, username } : null
103+
}
104+
105+
function reportFormState(): void {
106+
detected = detectForm()
80107

81108
const origin = window.location.origin
82-
const fingerprint = `${origin}|${detected !== null}`
109+
const hasPasswordField = detected?.password != null
110+
const fingerprint = `${origin}|${detected !== null}|${hasPasswordField}`
83111
if (fingerprint === lastReported) return
84112
lastReported = fingerprint
85-
ipcRenderer.send(FORM_STATE_CHANNEL, { origin, hasLoginForm: detected !== null })
113+
ipcRenderer.send(FORM_STATE_CHANNEL, {
114+
origin,
115+
hasLoginForm: detected !== null,
116+
hasPasswordField,
117+
})
86118
}
87119

88120
function scheduleRescan(): void {
@@ -111,18 +143,21 @@ function setFieldValue(field: HTMLInputElement, value: string): void {
111143

112144
ipcRenderer.on(
113145
FILL_CHANNEL,
114-
(_event, payload: { origin: string; username: string; password: string }) => {
146+
(_event, payload: { origin: string; username: string; password?: string }) => {
115147
// Last line of defence against a navigation between the user's choice and
116148
// this message arriving: the main process binds the fill to an origin, and
117149
// the live document has to still agree.
118150
if (!payload || payload.origin !== window.location.origin || !detected) return
119151
if (detected.username && payload.username) {
120152
setFieldValue(detected.username, payload.username)
121153
}
122-
setFieldValue(detected.password, payload.password)
154+
if (detected.password && payload.password) {
155+
setFieldValue(detected.password, payload.password)
156+
}
123157
// Never submitted. Autofill and submission stay separate so the user can
124-
// confirm the site and the account before anything is sent.
125-
detected.password.focus()
158+
// confirm the site and the account before anything is sent. Focus lands on
159+
// whichever field the user still has to deal with.
160+
;(detected.password ?? detected.username)?.focus()
126161
}
127162
)
128163

0 commit comments

Comments
 (0)