Skip to content

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

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.

Sign up for GitHub

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

Merged
mdiarra3 merged 14 commits into from
Nov 30, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
6bd5a80
ft unlock support fix
mdiarra3 Nov 16, 2023
3ad7bfa
ensure its valid
mdiarra3 Nov 16, 2023
b33381e
changelog: User-Facing Improvements, Authentication, Only show F/t Un…
mdiarra3 Nov 16, 2023
e7c1a75
fix spec
mdiarra3 Nov 16, 2023
fe8ba4a
refactor to make method async
mdiarra3 Nov 17, 2023
afb3809
Merge remote-tracking branch 'origin/main' into LG-11430-hide-ft-unlu…
mdiarra3 Nov 20, 2023
a37cd4d
fix javascript test
mdiarra3 Nov 20, 2023
77b2bf7
fix schema
mdiarra3 Nov 20, 2023
b1011d8
add public key credential supported in separate file
mdiarra3 Nov 20, 2023
8ae821a
update platform auth available spec
mdiarra3 Nov 20, 2023
8322a9e
Fix webauthn setup linting
mdiarra3 Nov 21, 2023
4b8147d
make sure to wait for async to pass before confirming test
mdiarra3 Nov 21, 2023
f076ca8
add hidden spec fix
mdiarra3 Nov 21, 2023
d66a337
fix capitalization
mdiarra3 Nov 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions app/javascript/packages/webauthn/is-webauthn-platform-authenticator-available.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { useDefineProperty } from '@18f/identity-test-helpers';
import isWebauthnPlatformAuthenticatorAvailable from './is-webauthn-platform-authenticator-available';

describe('isWebauthnPlatformAuthenticatorAvailable', () => {
const defineProperty = useDefineProperty();

context('browser does not support webauthn', () => {
beforeEach(() => {
defineProperty(window, 'PublicKeyCredential', {
configurable: true,
value: undefined,
});
});

it('resolves to false', async () => {
await expect(isWebauthnPlatformAuthenticatorAvailable()).to.eventually.equal(false);
});
});

context('browser supports webauthn', () => {
context('device does not have platform authenticator available', () => {
beforeEach(() => {
defineProperty(window, 'PublicKeyCredential', {
configurable: true,
value: { isUserVerifyingPlatformAuthenticatorAvailable: () => Promise.resolve(false) },
});
});

it('resolves to false', async () => {
await expect(isWebauthnPlatformAuthenticatorAvailable()).to.eventually.equal(false);
});
});

context('device has platform authenticator available', () => {
beforeEach(() => {
defineProperty(window, 'PublicKeyCredential', {
configurable: true,
value: { isUserVerifyingPlatformAuthenticatorAvailable: () => Promise.resolve(true) },
});
});

it('resolves to false', async () => {
await expect(isWebauthnPlatformAuthenticatorAvailable()).to.eventually.equal(true);
});
});
});
});
6 changes: 6 additions & 0 deletions app/javascript/packages/webauthn/is-webauthn-platform-authenticator-available.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export type IsWebauthnPlatformAvailable = () => Promise<boolean>;

const isWebauthnPlatformAuthenticatorAvailable: IsWebauthnPlatformAvailable = async () =>
!!(await window.PublicKeyCredential?.isUserVerifyingPlatformAuthenticatorAvailable());

export default isWebauthnPlatformAuthenticatorAvailable;
39 changes: 33 additions & 6 deletions app/javascript/packages/webauthn/webauthn-input-element.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,23 @@
import sinon from 'sinon';
import quibble from 'quibble';
import { waitFor } from '@testing-library/dom';
import type { IsWebauthnPasskeySupported } from './is-webauthn-passkey-supported';
import type { IsWebauthnPlatformAvailable } from './is-webauthn-platform-authenticator-available';

describe('WebauthnInputElement', () => {
const isWebauthnPasskeySupported = sinon.stub<
Parameters<IsWebauthnPasskeySupported>,
ReturnType<IsWebauthnPasskeySupported>
>();

const isWebauthnPlatformAvailable = sinon.stub<
Parameters<IsWebauthnPlatformAvailable>,
ReturnType<IsWebauthnPlatformAvailable>
>();

before(async () => {
quibble('./is-webauthn-passkey-supported', isWebauthnPasskeySupported);
quibble('./is-webauthn-platform-authenticator-available', isWebauthnPlatformAvailable);
await import('./webauthn-input-element');
});

Expand All @@ -21,6 +29,7 @@ describe('WebauthnInputElement', () => {
context('unsupported passkey not shown', () => {
beforeEach(() => {
isWebauthnPasskeySupported.returns(false);
isWebauthnPlatformAvailable.resolves(false);
document.body.innerHTML = `<lg-webauthn-input hidden></lg-webauthn-input>`;
});

Expand All @@ -34,6 +43,7 @@ describe('WebauthnInputElement', () => {
context('unsupported passkey shown', () => {
beforeEach(() => {
isWebauthnPasskeySupported.returns(false);
isWebauthnPlatformAvailable.resolves(false);
document.body.innerHTML = `<lg-webauthn-input show-unsupported-passkey hidden></lg-webauthn-input>`;
});

Expand All @@ -47,15 +57,32 @@ describe('WebauthnInputElement', () => {
});

context('device supports passkey', () => {
beforeEach(() => {
isWebauthnPasskeySupported.returns(true);
document.body.innerHTML = `<lg-webauthn-input hidden></lg-webauthn-input>`;
context('unsupported publickeycredential not shown', () => {
beforeEach(() => {
isWebauthnPlatformAvailable.resolves(false);
isWebauthnPasskeySupported.returns(true);
document.body.innerHTML = `<lg-webauthn-input hidden></lg-webauthn-input>`;
});

it('stays hidden', () => {
const element = document.querySelector('lg-webauthn-input')!;

expect(element.hidden).to.be.true();
});
});

it('becomes visible', () => {
const element = document.querySelector('lg-webauthn-input')!;
context('publickeycredential input is shown', () => {
beforeEach(() => {
isWebauthnPasskeySupported.returns(true);
isWebauthnPlatformAvailable.resolves(true);
document.body.innerHTML = `<lg-webauthn-input hidden></lg-webauthn-input>`;
});

it('becomes visible', async () => {
const element = document.querySelector('lg-webauthn-input')!;

expect(element.hidden).to.be.false();
await waitFor(() => expect(element.hidden).to.be.false());
});
});
});
});
5 changes: 3 additions & 2 deletions app/javascript/packages/webauthn/webauthn-input-element.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import isWebauthnPasskeySupported from './is-webauthn-passkey-supported';
import isWebauthnPlatformAuthenticatorAvailable from './is-webauthn-platform-authenticator-available';

export class WebauthnInputElement extends HTMLElement {
connectedCallback() {
Expand All @@ -13,12 +14,12 @@ export class WebauthnInputElement extends HTMLElement {
return this.hasAttribute('show-unsupported-passkey');
}

toggleVisibleIfPasskeySupported() {
async toggleVisibleIfPasskeySupported() {
if (!this.hasAttribute('hidden')) {
return;
}

if (isWebauthnPasskeySupported()) {
if (isWebauthnPasskeySupported() && (await isWebauthnPlatformAuthenticatorAvailable())) {
this.hidden = false;
} else if (this.showUnsupportedPasskey) {
this.hidden = false;
Expand Down
5 changes: 4 additions & 1 deletion spec/features/webauthn/hidden_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

RSpec.describe 'webauthn hide' do
include JavascriptDriverHelper
include WebAuthnHelper

describe 'security key' do
let(:option_id) { 'two_factor_options_form_selection_webauthn' }
Expand Down Expand Up @@ -58,9 +59,11 @@
expect(webauthn_option_hidden?).to eq(true)
end

context 'with supported browser', driver: :headless_chrome_mobile do
context 'with supported browser and platform authenticator available',
driver: :headless_chrome_mobile do
it 'displays the authenticator option' do
sign_up_and_set_password
simulate_platform_authenticator_available

expect(webauthn_option_hidden?).to eq(false)
end
Expand Down
9 changes: 9 additions & 0 deletions spec/support/features/webauthn_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,15 @@ def set_hidden_field(id, value)
end
end

def simulate_platform_authenticator_available
page.evaluate_script(<<~JS)
window.PublicKeyCredential.isUserVerifyingPlatformAuthenticatorAvailable = () => Promise.resolve(true);
JS
page.evaluate_script(<<~JS)
document.querySelectorAll('lg-webauthn-input').forEach((input) => input.connectedCallback());
JS
Comment on lines +134 to +136
Copy link
Copy Markdown
Contributor

@aduth aduth Nov 21, 2023

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.

end

def protocol
'http://'
end
Expand Down