Skip to content

LG-11430: hide ft unlock unless has public credential support#9609

Merged
mdiarra3 merged 14 commits intomainfrom
LG-11430-hide-ft-unluck-passkey-support
Nov 30, 2023
Merged

LG-11430: hide ft unlock unless has public credential support#9609
mdiarra3 merged 14 commits intomainfrom
LG-11430-hide-ft-unluck-passkey-support

Conversation

@mdiarra3
Copy link
Contributor

@mdiarra3 mdiarra3 commented Nov 16, 2023

🎫 Ticket

LG-11430: hide ft unlock if not available

🛠 Summary of changes

This puts back the check for ensuring that platform authenticator is available for the device not just that the browser supports it.

📜 Testing Plan

  • Users with properly set up devices are still able to view F/T Unlock at account creation
  • users without properly set up devices will not be able to view f/t unlock at account creation

@mdiarra3 mdiarra3 marked this pull request as ready for review November 20, 2023 14:39
@aduth
Copy link
Contributor

aduth commented Nov 21, 2023

Can you add the pull request template to the original comment?

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a little tricky to test, but I was able to mostly fake it and verify the input is hidden. LGTM! 👍

@aduth
Copy link
Contributor

aduth commented Nov 21, 2023

For the RSpec tests, we might need to find a way to fake support in JavaScript-enabled contexts, similar to what we're doing here:

page.evaluate_script(<<~JS)
base64ToArrayBuffer = (base64) => Uint8Array.from(atob(base64), (c) => c.charCodeAt(0)).buffer;
JS
page.evaluate_script(<<~JS)
navigator.credentials.get = () => Promise.resolve({
rawId: base64ToArrayBuffer(#{credential_id.to_json}),
response: {
authenticatorData: base64ToArrayBuffer(#{authenticator_data.to_json}),
clientDataJSON: base64ToArrayBuffer(#{verification_client_data_json.to_json}),
signature: base64ToArrayBuffer(#{signature.to_json}),
},
});
JS

e.g. Maybe something like?

page.evaluate_script(<<~JS)
  window.PublicKeyCredential.isUserVerifyingPlatformAuthenticatorAvailable = () => Promise.resolve(true);
JS

Comment on lines +134 to +136
page.evaluate_script(<<~JS)
document.querySelectorAll('lg-webauthn-input').forEach((input) => input.connectedCallback());
JS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof, I'd forgot about needing this. I think it makes sense, though I'd be a little concerned about future maintainability if the connected callback has some side effects that might be doubled-up by calling it explicitly like this.

In the past I'd wondered if we could use the Chrome DevTools protocol support for WebAuthn to add a "virtual" authenticator, but it was experimental and I had a hard time getting it working. Might be something to reconsider in the future.

@mdiarra3 mdiarra3 requested a review from a team November 21, 2023 15:43
@mdiarra3 mdiarra3 merged commit 5faec5b into main Nov 30, 2023
@mdiarra3 mdiarra3 deleted the LG-11430-hide-ft-unluck-passkey-support branch November 30, 2023 17:55
@solipet solipet mentioned this pull request Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants