-
Notifications
You must be signed in to change notification settings - Fork 49
Webauthn autocomplete extra fixes #577
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
base: develop
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| --- | ||
| '@forgerock/javascript-sdk': patch | ||
| --- | ||
|
|
||
| WebAuthn improvements | ||
|
|
||
| - Fix parsing of WebAuthn scripts when `asScript` is true | ||
| - Improve handling when conditional mediation is not supported | ||
| - Enable re-invocation of WebAuthn requests | ||
| - Enable modification of options passed to navigator.credentials.get() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,6 +48,12 @@ type WebAuthnMetadata = WebAuthnAuthenticationMetadata | WebAuthnRegistrationMet | |
| type WebAuthnTextOutput = WebAuthnTextOutputRegistration; | ||
| const TWO_SECOND = 2000; | ||
|
|
||
| declare global { | ||
| interface Window { | ||
| PingWebAuthnAbortController: AbortController; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Utility for integrating a web browser's WebAuthn API. | ||
| * | ||
|
|
@@ -65,19 +71,19 @@ const TWO_SECOND = 2000; | |
| * } | ||
| * ``` | ||
| * | ||
| * Conditional UI (Autofill) Support: | ||
| * Conditional mediation (Autofill) Support: | ||
| * | ||
| * ```js | ||
| * // Check if browser supports conditional UI | ||
| * // Check if browser supports conditional mediation | ||
| * const supportsConditionalUI = await FRWebAuthn.isConditionalUISupported(); | ||
| * | ||
| * if (supportsConditionalUI) { | ||
| * // The authenticate() method automatically handles conditional UI | ||
| * // The authenticate() method automatically handles conditional mediation | ||
| * // when the server indicates support via conditionalWebAuthn: true | ||
| * // in the metadata. No additional code changes needed. | ||
| * await FRWebAuthn.authenticate(step); | ||
| * | ||
| * // For conditional UI to work in the browser, add autocomplete="webauthn" | ||
| * // For conditional mediation to work in the browser, add autocomplete="webauthn" | ||
| * // to your username input field: | ||
| * // <input type="text" name="username" autocomplete="webauthn" /> | ||
| * } | ||
|
|
@@ -117,12 +123,21 @@ abstract class FRWebAuthn { | |
| } | ||
|
|
||
| /** | ||
| * Checks if the browser supports conditional UI (autofill) for WebAuthn. | ||
| * Checks if the browser supports WebAuthn. | ||
| * | ||
| * @return boolean indicating if WebAuthn is available | ||
| */ | ||
| public static isWebAuthnSupported(): boolean { | ||
| return !!window.PublicKeyCredential; | ||
| } | ||
|
|
||
| /** | ||
| * Checks if the browser supports conditional mediation (autofill) for WebAuthn. | ||
| * | ||
| * @return Promise<boolean> indicating if conditional mediation is available | ||
| */ | ||
| public static async isConditionalUISupported(): Promise<boolean> { | ||
| if (!window.PublicKeyCredential) { | ||
| public static async isConditionalMediationSupported(): Promise<boolean> { | ||
| if (!this.isWebAuthnSupported()) { | ||
| return false; | ||
| } | ||
|
|
||
|
|
@@ -138,41 +153,49 @@ abstract class FRWebAuthn { | |
|
|
||
| /** | ||
| * Populates the step with the necessary authentication outcome. | ||
| * Automatically handles conditional UI if indicated by the server metadata. | ||
| * Automatically handles conditional mediation if indicated by the server metadata. | ||
| * | ||
| * @param step The step that contains WebAuthn authentication data | ||
| * @param optionsTransformer Augments the derived options with custom behaviour | ||
| * @return The populated step | ||
| */ | ||
| public static async authenticate(step: FRStep): Promise<FRStep> { | ||
| public static async authenticate( | ||
| step: FRStep, | ||
| optionsTransformer: (options: CredentialRequestOptions) => CredentialRequestOptions = ( | ||
| options, | ||
| ) => options, | ||
| ): Promise<FRStep> { | ||
| const { hiddenCallback, metadataCallback, textOutputCallback } = this.getCallbacks(step); | ||
| if (hiddenCallback && (metadataCallback || textOutputCallback)) { | ||
| let outcome: ReturnType<typeof this.getAuthenticationOutcome>; | ||
| let credential: PublicKeyCredential | null = null; | ||
| const options: CredentialRequestOptions = {}; | ||
|
|
||
| try { | ||
| let publicKey: PublicKeyCredentialRequestOptions; | ||
| let useConditionalUI = false; | ||
|
|
||
| if (metadataCallback) { | ||
| const meta = metadataCallback.getOutputValue('data') as WebAuthnAuthenticationMetadata; | ||
|
|
||
| // Check if server indicates conditional UI should be used | ||
| useConditionalUI = meta.conditional === 'true'; | ||
| publicKey = this.createAuthenticationPublicKey(meta); | ||
|
|
||
| credential = await this.getAuthenticationCredential( | ||
| publicKey as PublicKeyCredentialRequestOptions, | ||
| useConditionalUI, | ||
| ); | ||
| outcome = this.getAuthenticationOutcome(credential); | ||
| const mediation = meta.mediation as CredentialMediationRequirement; | ||
|
|
||
| if (mediation === 'conditional') { | ||
| const isConditionalMediationSupported = await this.isConditionalMediationSupported(); | ||
| if (!isConditionalMediationSupported) { | ||
| const e = new Error( | ||
| 'Conditional mediation was requested, but is not supported by this browser.', | ||
| ); | ||
| e.name = WebAuthnOutcomeType.NotSupportedError; | ||
| throw e; | ||
| } | ||
| } | ||
|
|
||
| options.publicKey = this.createAuthenticationPublicKey(meta); | ||
| options.mediation = mediation; | ||
| } else if (textOutputCallback) { | ||
| publicKey = parseWebAuthnAuthenticateText(textOutputCallback.getMessage()); | ||
|
|
||
| credential = await this.getAuthenticationCredential( | ||
| publicKey as PublicKeyCredentialRequestOptions, | ||
| false, // Script-based callbacks don't support conditional UI | ||
| ); | ||
| outcome = this.getAuthenticationOutcome(credential); | ||
| const metadata = this.extractMetadata(textOutputCallback.getMessage()); | ||
| if (metadata) { | ||
| options.publicKey = this.createAuthenticationPublicKey( | ||
| metadata as WebAuthnAuthenticationMetadata, | ||
| ); | ||
| } else { | ||
| options.publicKey = parseWebAuthnAuthenticateText(textOutputCallback.getMessage()); | ||
| } | ||
| } else { | ||
| throw new Error('No Credential found from Public Key'); | ||
| } | ||
|
|
@@ -187,6 +210,12 @@ abstract class FRWebAuthn { | |
| throw error; | ||
| } | ||
|
|
||
| const credential: PublicKeyCredential | null = await this.getAuthenticationCredential( | ||
| optionsTransformer(options), | ||
| ); | ||
| const outcome: ReturnType<typeof this.getAuthenticationOutcome> = | ||
| this.getAuthenticationOutcome(credential); | ||
|
|
||
| if (metadataCallback) { | ||
| const meta = metadataCallback.getOutputValue('data') as WebAuthnAuthenticationMetadata; | ||
| if (meta?.supportsJsonResponse && credential && 'authenticatorAttachment' in credential) { | ||
|
|
@@ -236,7 +265,13 @@ abstract class FRWebAuthn { | |
| ); | ||
| outcome = this.getRegistrationOutcome(credential); | ||
| } else if (textOutputCallback) { | ||
| publicKey = parseWebAuthnRegisterText(textOutputCallback.getMessage()); | ||
| const metadata = this.extractMetadata(textOutputCallback.getMessage()); | ||
|
|
||
| if (metadata) { | ||
| publicKey = this.createRegistrationPublicKey(metadata as WebAuthnRegistrationMetadata); | ||
| } else { | ||
| publicKey = parseWebAuthnRegisterText(textOutputCallback.getMessage()); | ||
| } | ||
| credential = await this.getRegistrationCredential( | ||
| publicKey as PublicKeyCredentialCreationOptions, | ||
| ); | ||
|
|
@@ -349,37 +384,23 @@ abstract class FRWebAuthn { | |
| /** | ||
| * Retrieves the credential from the browser Web Authentication API. | ||
| * | ||
| * @param options The public key options associated with the request | ||
| * @param useConditionalUI Whether to use conditional UI (autofill) | ||
| * @param options The options associated with the request | ||
| * @return The credential | ||
| */ | ||
| public static async getAuthenticationCredential( | ||
| options: PublicKeyCredentialRequestOptions, | ||
| useConditionalUI = false, | ||
| options: CredentialRequestOptions, | ||
| ): Promise<PublicKeyCredential | null> { | ||
| // Feature check before we attempt authenticating | ||
| if (!window.PublicKeyCredential) { | ||
| if (!this.isWebAuthnSupported()) { | ||
| const e = new Error('PublicKeyCredential not supported by this browser'); | ||
| e.name = WebAuthnOutcomeType.NotSupportedError; | ||
| throw e; | ||
| } | ||
| // Build the credential request options | ||
| const credentialRequestOptions: CredentialRequestOptions = { | ||
| publicKey: options, | ||
| }; | ||
|
|
||
| // Add conditional mediation if requested and supported | ||
| if (useConditionalUI) { | ||
| const isConditionalSupported = await this.isConditionalUISupported(); | ||
| if (isConditionalSupported) { | ||
| credentialRequestOptions.mediation = 'conditional' as CredentialMediationRequirement; | ||
| } else { | ||
| // eslint-disable-next-line no-console | ||
| FRLogger.warn('Conditional UI was requested, but is not supported by this browser.'); | ||
| } | ||
| } | ||
|
|
||
| const credential = await navigator.credentials.get(credentialRequestOptions); | ||
| const credential = await navigator.credentials.get({ | ||
| ...options, | ||
| signal: this.createAbortController().signal, | ||
| }); | ||
| return credential as PublicKeyCredential; | ||
| } | ||
|
|
||
|
|
@@ -448,7 +469,7 @@ abstract class FRWebAuthn { | |
| options: PublicKeyCredentialCreationOptions, | ||
| ): Promise<PublicKeyCredential | null> { | ||
| // Feature check before we attempt registering a device | ||
| if (!window.PublicKeyCredential) { | ||
| if (this.isWebAuthnSupported()) { | ||
| const e = new Error('PublicKeyCredential not supported by this browser'); | ||
| e.name = WebAuthnOutcomeType.NotSupportedError; | ||
| throw e; | ||
|
|
@@ -525,7 +546,7 @@ abstract class FRWebAuthn { | |
| challenge: Uint8Array.from(atob(challenge), (c) => c.charCodeAt(0)).buffer, | ||
| timeout, | ||
| }; | ||
| // For conditional UI, allowCredentials can be omitted. | ||
| // For conditional mediation, allowCredentials can be omitted. | ||
| // For standard WebAuthn, it may or may not be present. | ||
| // Only add the property if the array is not empty. | ||
| if (allowCredentialsValue && allowCredentialsValue.length > 0) { | ||
|
|
@@ -599,6 +620,25 @@ abstract class FRWebAuthn { | |
| }, | ||
| }; | ||
| } | ||
|
|
||
| private static createAbortController() { | ||
| window.PingWebAuthnAbortController?.abort(); | ||
|
|
||
| const abortController = new AbortController(); | ||
| window.PingWebAuthnAbortController = abortController; | ||
| return abortController; | ||
| } | ||
|
Comment on lines
+625
to
+630
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need the abort controller as part of the window? I don't see us using it from the Window, but maybe it's needed elsewhere? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The webauthn processing persists across page loads, meaning that we need to store it somewhere that can survive that. This method will abort the previous controller, cancelling previous flows, before creating a new one, meaning that it doesn't need to be referenced outside of this method.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it, okay. hmm we typically don't want to modify the window but I see where your coming from. Let me think on this. |
||
|
|
||
| private static extractMetadata(message: string): object | null { | ||
| const contextMatch = message.match(/^var scriptContext = (.*);*$/m); | ||
| const jsonString = contextMatch?.[1]; | ||
|
|
||
| if (jsonString) { | ||
| return JSON.parse(jsonString); | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
| } | ||
|
|
||
| export default FRWebAuthn; | ||
|
|
@@ -608,4 +648,4 @@ export type { | |
| WebAuthnCallbacks, | ||
| WebAuthnRegistrationMetadata, | ||
| }; | ||
| export { WebAuthnOutcome, WebAuthnStepType }; | ||
| export { WebAuthnOutcome, WebAuthnOutcomeType, WebAuthnStepType }; | ||
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.
Is this the only possible value for
mediation?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.
Currently with the backend the only values this could be "conditional" or not present. There are other valid values however, as per https://developer.mozilla.org/en-US/docs/Web/API/CredentialsContainer/get#mediation. Currently AM does not send any of these other values.