-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Fix passkey login error experience #62480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
fd3105e
183bb94
ae78ebb
6b5317b
dda7d9a
4542f08
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,4 +1,10 @@ | ||||||
async function fetchWithErrorHandling(url, options = {}) { | ||||||
const browserSupportsPasskeys = | ||||||
typeof navigator.credentials !== 'undefined' && | ||||||
typeof window.PublicKeyCredential !== 'undefined' && | ||||||
typeof window.PublicKeyCredential.parseCreationOptionsFromJSON === 'function' && | ||||||
typeof window.PublicKeyCredential.parseRequestOptionsFromJSON === 'function'; | ||||||
|
||||||
async function fetchWithErrorHandling(url, options = {}) { | ||||||
const response = await fetch(url, { | ||||||
credentials: 'include', | ||||||
...options | ||||||
|
@@ -45,7 +51,7 @@ customElements.define('passkey-submit', class extends HTMLElement { | |||||
this.internals.form.addEventListener('submit', (event) => { | ||||||
if (event.submitter?.name === '__passkeySubmit') { | ||||||
event.preventDefault(); | ||||||
this.obtainCredentialAndSubmit(); | ||||||
this.obtainAndSubmitCredential(); | ||||||
} | ||||||
}); | ||||||
|
||||||
|
@@ -56,39 +62,49 @@ customElements.define('passkey-submit', class extends HTMLElement { | |||||
this.abortController?.abort(); | ||||||
} | ||||||
|
||||||
async obtainCredentialAndSubmit(useConditionalMediation = false) { | ||||||
async obtainCredential(useConditionalMediation, signal) { | ||||||
if (!browserSupportsPasskeys) { | ||||||
throw new Error('Some passkey features are missing. Please update your browser.'); | ||||||
} | ||||||
|
||||||
if (this.attrs.operation === 'Create') { | ||||||
return await createCredential(signal); | ||||||
} else if (this.attrs.operation === 'Request') { | ||||||
const email = new FormData(this.internals.form).get(this.attrs.emailName); | ||||||
const mediation = useConditionalMediation ? 'conditional' : undefined; | ||||||
return await requestCredential(email, mediation, signal); | ||||||
} else { | ||||||
throw new Error(`Unknown passkey operation '${this.attrs.operation}'.`); | ||||||
} | ||||||
} | ||||||
|
||||||
async obtainAndSubmitCredential(useConditionalMediation = false) { | ||||||
this.abortController?.abort(); | ||||||
this.abortController = new AbortController(); | ||||||
const signal = this.abortController.signal; | ||||||
const formData = new FormData(); | ||||||
try { | ||||||
let credential; | ||||||
if (this.attrs.operation === 'Create') { | ||||||
credential = await createCredential(signal); | ||||||
} else if (this.attrs.operation === 'Request') { | ||||||
const email = new FormData(this.internals.form).get(this.attrs.emailName); | ||||||
const mediation = useConditionalMediation ? 'conditional' : undefined; | ||||||
credential = await requestCredential(email, mediation, signal); | ||||||
} else { | ||||||
throw new Error(`Unknown passkey operation '${operation}'.`); | ||||||
} | ||||||
const credential = await this.obtainCredential(useConditionalMediation, signal); | ||||||
const credentialJson = JSON.stringify(credential); | ||||||
formData.append(`${this.attrs.name}.CredentialJson`, credentialJson); | ||||||
} catch (error) { | ||||||
if (error.name === 'AbortError') { | ||||||
// Canceled by user action, do not submit the form | ||||||
console.error(error); | ||||||
if (useConditionalMediation || error.name === 'AbortError') { | ||||||
// We do not relay the error to the user if: | ||||||
// 1. We are attempting conditional mediation, meaning the user did not initiate the operation. | ||||||
// 2. The user explicitly canceled the operation. | ||||||
return; | ||||||
} | ||||||
formData.append(`${this.attrs.name}.Error`, error.message); | ||||||
console.error(error); | ||||||
const errorMessage = error.name === 'NotAllowedError' ? 'Unable to authenticate.' : error.message; | ||||||
|
const errorMessage = error.name === 'NotAllowedError' ? 'Unable to authenticate.' : error.message; | |
const errorMessage = error.name === 'NotAllowedError' ? 'Unable to authenticate with passkey. See developer console for more details.' : error.message; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, maybe we could improve the wording here. The thing about "Unable to authenticate with passkey" is that IMO it could be interpreted as if passkey was obtained, but we just couldn't authenticate with it (implying a possible error on the server). Whereas, what actually happened is that the authenticator couldn't authenticate the user, so no passkey was provided to the browser. Maybe something like "No passkey was provided by the authenticator" could work?
And regarding "See developer console for more details" - this is definitely helpful to the developer, but might it be unusual to display this to the user? The fact that this error shows up in a user-facing part of the UI makes me a little hesitant to put that there. I guess the developer could always remove that bit if they want.
Uh oh!
There was an error while loading. Please reload this page.