New issue
Advanced search Search tips

Issue 847985 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Feature


Sign in to add a comment

Implement UX for WebAuthn

Project Member Reported by engedy@chromium.org, May 30 2018

Issue description

This is a tracking bug encompassing the work involved in implementing a tab-modal dialog that is shown while a WebAuthn request is active in the tab. The dialog guides the user through transport selection, and, if selected, Bluetooth pairing.
 

Comment 1 by engedy@chromium.org, May 30 2018

Blockedon: 847993
Blockedon: 849309
Blockedon: 849323
Labels: Hotlist-WebAuthN-UI
Labels: -Hotlist-WebAuthN-UI Hotlist-WebAuthnUI
Blockedon: 868212
Blockedon: 866601
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d1db8447f1c04a55214d2d8de0d8c13e8a264cf1

commit d1db8447f1c04a55214d2d8de0d8c13e8a264cf1
Author: Jun Choi <hongjunchoi@chromium.org>
Date: Tue Aug 14 11:41:20 2018

Add Cancel logic to WebAuthN UI

Add logic to cancel ongoing WebAuthN request when user clicks "cancel"
button from any UI modal or "back" button from the initial welcome
screen.

Bug:  847985 
TBR: jam@chromium.org
Change-Id: Ia9a014cf78583c2512ceaebf822758aa0dc65d99
Reviewed-on: https://chromium-review.googlesource.com/1173489
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582886}
[modify] https://crrev.com/d1db8447f1c04a55214d2d8de0d8c13e8a264cf1/chrome/browser/webauthn/authenticator_request_dialog_model.cc
[modify] https://crrev.com/d1db8447f1c04a55214d2d8de0d8c13e8a264cf1/chrome/browser/webauthn/authenticator_request_dialog_model.h
[modify] https://crrev.com/d1db8447f1c04a55214d2d8de0d8c13e8a264cf1/chrome/browser/webauthn/chrome_authenticator_request_delegate.cc
[modify] https://crrev.com/d1db8447f1c04a55214d2d8de0d8c13e8a264cf1/chrome/browser/webauthn/chrome_authenticator_request_delegate.h
[modify] https://crrev.com/d1db8447f1c04a55214d2d8de0d8c13e8a264cf1/content/browser/webauth/authenticator_impl.cc
[modify] https://crrev.com/d1db8447f1c04a55214d2d8de0d8c13e8a264cf1/content/browser/webauth/authenticator_impl.h
[modify] https://crrev.com/d1db8447f1c04a55214d2d8de0d8c13e8a264cf1/content/browser/webauth/authenticator_impl_unittest.cc
[modify] https://crrev.com/d1db8447f1c04a55214d2d8de0d8c13e8a264cf1/content/public/browser/authenticator_request_client_delegate.cc
[modify] https://crrev.com/d1db8447f1c04a55214d2d8de0d8c13e8a264cf1/content/public/browser/authenticator_request_client_delegate.h

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1beb4c29f9246b05632ed32a92dffa1087b9cafb

commit 1beb4c29f9246b05632ed32a92dffa1087b9cafb
Author: Jun Choi <hongjunchoi@chromium.org>
Date: Thu Aug 16 03:04:38 2018

Plumb transport protocol from //device/fido to UI.

For all successful WebAuthN API transactions, notify observers of
FidoRequestHandler, then AuthenticatorRequestClientDelegate of which
transport protocol was used to talk to the authenticator that was used
to service the request.

This will be saved as Chromium preference, and used to infer which
transport type to default to when user invokes WebAuthN API again.

Bug:  847985 
Change-Id: I9ecf27209ec959d740664e34fe088a2e4499e982
Reviewed-on: https://chromium-review.googlesource.com/1175500
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583517}
[modify] https://crrev.com/1beb4c29f9246b05632ed32a92dffa1087b9cafb/content/browser/webauth/authenticator_impl.cc
[modify] https://crrev.com/1beb4c29f9246b05632ed32a92dffa1087b9cafb/content/browser/webauth/authenticator_impl.h
[modify] https://crrev.com/1beb4c29f9246b05632ed32a92dffa1087b9cafb/device/fido/fido_request_handler.h
[modify] https://crrev.com/1beb4c29f9246b05632ed32a92dffa1087b9cafb/device/fido/fido_request_handler_unittest.cc
[modify] https://crrev.com/1beb4c29f9246b05632ed32a92dffa1087b9cafb/device/fido/get_assertion_handler_unittest.cc
[modify] https://crrev.com/1beb4c29f9246b05632ed32a92dffa1087b9cafb/device/fido/get_assertion_request_handler.h
[modify] https://crrev.com/1beb4c29f9246b05632ed32a92dffa1087b9cafb/device/fido/make_credential_handler_unittest.cc
[modify] https://crrev.com/1beb4c29f9246b05632ed32a92dffa1087b9cafb/device/fido/make_credential_request_handler.h
[modify] https://crrev.com/1beb4c29f9246b05632ed32a92dffa1087b9cafb/device/fido/test_callback_receiver.h

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d53a039c8d9d5b9c83715f51ddd4934be5874a3c

commit d53a039c8d9d5b9c83715f51ddd4934be5874a3c
Author: Jun Choi <hongjunchoi@chromium.org>
Date: Thu Aug 16 23:38:15 2018

Make WebAuthN UI appear after device/fido signals

Make WebAuthN UI modals appear after all fields of
TransportAvailabilityInfo have been set by FidoRequestHandlerBase. Also,
Add rp_id to TransportAvailabilityInfo struct as rp id is shown in the
UI dialogs.

Bug:  847985 
Change-Id: I0c5341dfd98a192553ebd868461df405a5c0a5b1
Reviewed-on: https://chromium-review.googlesource.com/1176701
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583884}
[modify] https://crrev.com/d53a039c8d9d5b9c83715f51ddd4934be5874a3c/chrome/browser/webauthn/chrome_authenticator_request_delegate.cc
[modify] https://crrev.com/d53a039c8d9d5b9c83715f51ddd4934be5874a3c/chrome/browser/webauthn/chrome_authenticator_request_delegate.h
[modify] https://crrev.com/d53a039c8d9d5b9c83715f51ddd4934be5874a3c/content/browser/webauth/authenticator_impl.cc
[modify] https://crrev.com/d53a039c8d9d5b9c83715f51ddd4934be5874a3c/content/browser/webauth/authenticator_impl_unittest.cc
[modify] https://crrev.com/d53a039c8d9d5b9c83715f51ddd4934be5874a3c/content/public/browser/authenticator_request_client_delegate.cc
[modify] https://crrev.com/d53a039c8d9d5b9c83715f51ddd4934be5874a3c/content/public/browser/authenticator_request_client_delegate.h
[modify] https://crrev.com/d53a039c8d9d5b9c83715f51ddd4934be5874a3c/device/fido/fido_request_handler_base.cc
[modify] https://crrev.com/d53a039c8d9d5b9c83715f51ddd4934be5874a3c/device/fido/fido_request_handler_base.h
[modify] https://crrev.com/d53a039c8d9d5b9c83715f51ddd4934be5874a3c/device/fido/get_assertion_request_handler.cc
[modify] https://crrev.com/d53a039c8d9d5b9c83715f51ddd4934be5874a3c/device/fido/make_credential_request_handler.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c3bc3de73b9c092e9082b42953d79a966ad6d831

commit c3bc3de73b9c092e9082b42953d79a966ad6d831
Author: Jun Choi <hongjunchoi@chromium.org>
Date: Fri Aug 17 00:11:15 2018

Add Phone as a security key sheet view

Add sheet view to show when prompting users to use their phone as a
security key to authenticate via the WebAuthN API.

Bug:  847985 
Change-Id: I0bb605bf470456f5f29111834f69a71abfbe0430
Reviewed-on: https://chromium-review.googlesource.com/1177445
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583892}
[modify] https://crrev.com/c3bc3de73b9c092e9082b42953d79a966ad6d831/chrome/app/generated_resources.grd
[modify] https://crrev.com/c3bc3de73b9c092e9082b42953d79a966ad6d831/chrome/browser/ui/views/webauthn/sheet_view_factory.cc
[modify] https://crrev.com/c3bc3de73b9c092e9082b42953d79a966ad6d831/chrome/browser/ui/webauthn/authenticator_dialog_browsertest.cc
[modify] https://crrev.com/c3bc3de73b9c092e9082b42953d79a966ad6d831/chrome/browser/ui/webauthn/sheet_models.cc
[modify] https://crrev.com/c3bc3de73b9c092e9082b42953d79a966ad6d831/chrome/browser/ui/webauthn/sheet_models.h
[modify] https://crrev.com/c3bc3de73b9c092e9082b42953d79a966ad6d831/chrome/browser/webauthn/authenticator_request_dialog_model.cc
[modify] https://crrev.com/c3bc3de73b9c092e9082b42953d79a966ad6d831/chrome/browser/webauthn/authenticator_request_dialog_model.h

Project Member

Comment 13 by bugdroid1@chromium.org, Aug 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9357336772bc1ecd817465a3bf0cd4458906f8d6

commit 9357336772bc1ecd817465a3bf0cd4458906f8d6
Author: Balazs Engedy <engedy@chromium.org>
Date: Fri Aug 17 13:17:17 2018

Implement WebAuthn transport auto selection.

This CL also delays creating the AuthenticatorRequestDialogModel and
showing the dialog until after ChromeAuthenticatorRequestDelegate's
OnTransportAvailabilityEnumerated is called, plus renames Step::kInitial
to ::kWelcome.

Bug:  847985 
Change-Id: I3b46781ccea0309e1efc7c3fec45cbe4c43e9017
Reviewed-on: https://chromium-review.googlesource.com/1175120
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Jun Choi <hongjunchoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584041}
[modify] https://crrev.com/9357336772bc1ecd817465a3bf0cd4458906f8d6/chrome/browser/ui/views/webauthn/authenticator_dialog_view_browsertest.cc
[modify] https://crrev.com/9357336772bc1ecd817465a3bf0cd4458906f8d6/chrome/browser/ui/views/webauthn/sheet_view_factory.cc
[modify] https://crrev.com/9357336772bc1ecd817465a3bf0cd4458906f8d6/chrome/browser/ui/webauthn/authenticator_dialog_browsertest.cc
[modify] https://crrev.com/9357336772bc1ecd817465a3bf0cd4458906f8d6/chrome/browser/ui/webauthn/sheet_models.cc
[modify] https://crrev.com/9357336772bc1ecd817465a3bf0cd4458906f8d6/chrome/browser/ui/webauthn/sheet_models.h
[modify] https://crrev.com/9357336772bc1ecd817465a3bf0cd4458906f8d6/chrome/browser/webauthn/authenticator_request_dialog_model.cc
[modify] https://crrev.com/9357336772bc1ecd817465a3bf0cd4458906f8d6/chrome/browser/webauthn/authenticator_request_dialog_model.h
[add] https://crrev.com/9357336772bc1ecd817465a3bf0cd4458906f8d6/chrome/browser/webauthn/authenticator_request_dialog_model_unittest.cc
[modify] https://crrev.com/9357336772bc1ecd817465a3bf0cd4458906f8d6/chrome/browser/webauthn/chrome_authenticator_request_delegate.cc
[modify] https://crrev.com/9357336772bc1ecd817465a3bf0cd4458906f8d6/chrome/test/BUILD.gn

Project Member

Comment 14 by bugdroid1@chromium.org, Aug 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/738fec5b4d07254f007b6b029c27c25a5eca8b71

commit 738fec5b4d07254f007b6b029c27c25a5eca8b71
Author: Jun Choi <hongjunchoi@chromium.org>
Date: Fri Aug 17 22:48:15 2018

Dispatch touchId authenticator request from UI

Once UI for showing users to authenticate using Touch Id authenticator
is rendered, dispatch request to device/fido layer.

Bug:  847985 
Change-Id: I9531d412e5c3f6626a6a80bb3b1b6128712c132e
Reviewed-on: https://chromium-review.googlesource.com/1177442
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/master@{#584226}
[modify] https://crrev.com/738fec5b4d07254f007b6b029c27c25a5eca8b71/chrome/browser/webauthn/authenticator_request_dialog_model.cc
[modify] https://crrev.com/738fec5b4d07254f007b6b029c27c25a5eca8b71/chrome/browser/webauthn/authenticator_request_dialog_model.h
[modify] https://crrev.com/738fec5b4d07254f007b6b029c27c25a5eca8b71/chrome/browser/webauthn/chrome_authenticator_request_delegate.cc
[modify] https://crrev.com/738fec5b4d07254f007b6b029c27c25a5eca8b71/chrome/browser/webauthn/chrome_authenticator_request_delegate.h

Project Member

Comment 15 by bugdroid1@chromium.org, Aug 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d04798eea201639e80900576a3d2cba99a1ea19c

commit d04798eea201639e80900576a3d2cba99a1ea19c
Author: Jun Choi <hongjunchoi@chromium.org>
Date: Sat Aug 18 04:49:54 2018

Notify embedder when Cable discovery fails

Currently, FidoBleDiscoveryBase returns when ble adapter is not present.
This is problematic as the embedder layer explicitly waits for
NotifyDiscoveryStarted() to be invoked prior to launching UI. Make sure
to call NotifyDiscoveryStarted(false) when Bluetooth adapter is not
present.

Bug:  847985 
Change-Id: I968e2ff5f1408b7a5a56eb94b25240b8705b1c68
Reviewed-on: https://chromium-review.googlesource.com/1176977
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584302}
[modify] https://crrev.com/d04798eea201639e80900576a3d2cba99a1ea19c/device/fido/ble/fido_ble_discovery_base.cc
[modify] https://crrev.com/d04798eea201639e80900576a3d2cba99a1ea19c/device/fido/ble/fido_ble_discovery_unittest.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Aug 20

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e122308b4849b6566a42698b5b33401e8802d0cc

commit e122308b4849b6566a42698b5b33401e8802d0cc
Author: Martin Kreichgauer <martinkr@google.com>
Date: Mon Aug 20 23:17:38 2018

fido: populate TransportAvailabilityInfo.has_recognized_mac_touch_id_credential

This introduces PlatformAuthenticatorInfo as a parameter object to
FidoRequestHandlerBase::SetPlatformAuthenticatorOrMarkUnavailable. The
struct holds the existing std::unique_ptr<FidoAuthenticator>, which is
the platform authenticator itself, and an additional bool indicating
whether the authenticator has a matching credential for
GetAssertionRequest. The bool is plumbed through to
TransportAvailabilityInfo for consideration by the UI.

Bug:  847985 
Change-Id: Ice46685eef31eb29cb8fdcb0d471e4b40bfb97b3
Reviewed-on: https://chromium-review.googlesource.com/1179088
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Jun Choi <hongjunchoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584580}
[modify] https://crrev.com/e122308b4849b6566a42698b5b33401e8802d0cc/content/browser/webauth/authenticator_impl.cc
[modify] https://crrev.com/e122308b4849b6566a42698b5b33401e8802d0cc/content/browser/webauth/authenticator_impl.h
[modify] https://crrev.com/e122308b4849b6566a42698b5b33401e8802d0cc/device/fido/fido_request_handler_base.cc
[modify] https://crrev.com/e122308b4849b6566a42698b5b33401e8802d0cc/device/fido/fido_request_handler_base.h
[modify] https://crrev.com/e122308b4849b6566a42698b5b33401e8802d0cc/device/fido/fido_request_handler_unittest.cc
[modify] https://crrev.com/e122308b4849b6566a42698b5b33401e8802d0cc/device/fido/get_assertion_handler_unittest.cc
[modify] https://crrev.com/e122308b4849b6566a42698b5b33401e8802d0cc/device/fido/mac/authenticator.h
[modify] https://crrev.com/e122308b4849b6566a42698b5b33401e8802d0cc/device/fido/mac/authenticator.mm
[modify] https://crrev.com/e122308b4849b6566a42698b5b33401e8802d0cc/device/fido/mac/get_assertion_operation.mm
[modify] https://crrev.com/e122308b4849b6566a42698b5b33401e8802d0cc/device/fido/mac/keychain.mm
[modify] https://crrev.com/e122308b4849b6566a42698b5b33401e8802d0cc/device/fido/make_credential_handler_unittest.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Aug 21

Project Member

Comment 18 by bugdroid1@chromium.org, Aug 22

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/61b63bd8bb1ef2ec4b71453dcb06b2878ef9025d

commit 61b63bd8bb1ef2ec4b71453dcb06b2878ef9025d
Author: Martin Kreichgauer <martinkr@google.com>
Date: Wed Aug 22 09:51:19 2018

fido: disable back button in Touch ID UI sheet

TouchIdAuthenticator currently fails a DCHECK when GetAssertion or
MakeCredential get invoked multiple times. Using the Back button (and
then proceeding to Touch ID again) can trigger a second call to
DispatchRequest and therefore the DCHECK fail.

Clicking the back button on the Touch ID sheet would not dismiss the
native Touch ID dialog, which would then hover over the welcome screen
sheet. So simply disabling the button seems like the right thing to do.

Bug:  678128 , 847985 
Change-Id: I90dbf5ab016a177811575a4db61ca12d15e43841
Reviewed-on: https://chromium-review.googlesource.com/1184236
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584983}
[modify] https://crrev.com/61b63bd8bb1ef2ec4b71453dcb06b2878ef9025d/chrome/browser/ui/webauthn/sheet_models.cc
[modify] https://crrev.com/61b63bd8bb1ef2ec4b71453dcb06b2878ef9025d/chrome/browser/ui/webauthn/sheet_models.h

Project Member

Comment 19 by bugdroid1@chromium.org, Aug 22

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/42e740f0a8c1ea3fd7f90435983dfe9d772775ab

commit 42e740f0a8c1ea3fd7f90435983dfe9d772775ab
Author: Jun Choi <hongjunchoi@chromium.org>
Date: Wed Aug 22 17:25:24 2018

Move call to GetInfo command after AuthenticatorAdded()

In order to prevent system UI dialogs for BLE/Touch ID from appearing
prior to WebAuthN UI, we hold off on dispatching request to
authenticators until user consents on using the specified transport
type. However, in current implementation, we send GetInfo command to
external BLE authenticators as soon as we discover the device. This
would trigger OS UI dialog on Mac for unpaired devices and potentially
cause BLE connection failures on other platforms. As so, move call to
GetInfo command after FidoRequestHandlerBase::AuthenticatorAdded().

Bug:  847985 
Change-Id: I37b7cd8118c3edb799fa29c36700febbb2b3d08e
Reviewed-on: https://chromium-review.googlesource.com/1182946
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585130}
[modify] https://crrev.com/42e740f0a8c1ea3fd7f90435983dfe9d772775ab/content/browser/webauth/authenticator_impl_unittest.cc
[modify] https://crrev.com/42e740f0a8c1ea3fd7f90435983dfe9d772775ab/device/fido/fake_fido_discovery_unittest.cc
[modify] https://crrev.com/42e740f0a8c1ea3fd7f90435983dfe9d772775ab/device/fido/fido_authenticator.h
[modify] https://crrev.com/42e740f0a8c1ea3fd7f90435983dfe9d772775ab/device/fido/fido_device.h
[modify] https://crrev.com/42e740f0a8c1ea3fd7f90435983dfe9d772775ab/device/fido/fido_device_authenticator.cc
[modify] https://crrev.com/42e740f0a8c1ea3fd7f90435983dfe9d772775ab/device/fido/fido_device_authenticator.h
[modify] https://crrev.com/42e740f0a8c1ea3fd7f90435983dfe9d772775ab/device/fido/fido_discovery.cc
[modify] https://crrev.com/42e740f0a8c1ea3fd7f90435983dfe9d772775ab/device/fido/fido_discovery_unittest.cc
[modify] https://crrev.com/42e740f0a8c1ea3fd7f90435983dfe9d772775ab/device/fido/fido_request_handler_base.cc
[modify] https://crrev.com/42e740f0a8c1ea3fd7f90435983dfe9d772775ab/device/fido/fido_request_handler_base.h
[modify] https://crrev.com/42e740f0a8c1ea3fd7f90435983dfe9d772775ab/device/fido/fido_request_handler_unittest.cc
[modify] https://crrev.com/42e740f0a8c1ea3fd7f90435983dfe9d772775ab/device/fido/get_assertion_handler_unittest.cc
[modify] https://crrev.com/42e740f0a8c1ea3fd7f90435983dfe9d772775ab/device/fido/mac/authenticator.h
[modify] https://crrev.com/42e740f0a8c1ea3fd7f90435983dfe9d772775ab/device/fido/mac/authenticator.mm
[modify] https://crrev.com/42e740f0a8c1ea3fd7f90435983dfe9d772775ab/device/fido/make_credential_handler_unittest.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Aug 22

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305

commit a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305
Author: Sky Malice <skym@chromium.org>
Date: Wed Aug 22 19:48:06 2018

Revert "Move call to GetInfo command after AuthenticatorAdded()"

This reverts commit 42e740f0a8c1ea3fd7f90435983dfe9d772775ab.

Reason for revert: linux-chromeos-rel is failing device_unittests,
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-chromeos-rel/12315, speculativly reverting this CL.

BUG:  876841 

Original change's description:
> Move call to GetInfo command after AuthenticatorAdded()
> 
> In order to prevent system UI dialogs for BLE/Touch ID from appearing
> prior to WebAuthN UI, we hold off on dispatching request to
> authenticators until user consents on using the specified transport
> type. However, in current implementation, we send GetInfo command to
> external BLE authenticators as soon as we discover the device. This
> would trigger OS UI dialog on Mac for unpaired devices and potentially
> cause BLE connection failures on other platforms. As so, move call to
> GetInfo command after FidoRequestHandlerBase::AuthenticatorAdded().
> 
> Bug:  847985 
> Change-Id: I37b7cd8118c3edb799fa29c36700febbb2b3d08e
> Reviewed-on: https://chromium-review.googlesource.com/1182946
> Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
> Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#585130}

TBR=martinkr@google.com,jdoerrie@chromium.org,hongjunchoi@chromium.org

Change-Id: I6ba8d08a85a3e9319257d11e0b2b5e5e01cfc7b3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  847985 
Reviewed-on: https://chromium-review.googlesource.com/1185683
Reviewed-by: Sky Malice <skym@chromium.org>
Commit-Queue: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585210}
[modify] https://crrev.com/a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305/content/browser/webauth/authenticator_impl_unittest.cc
[modify] https://crrev.com/a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305/device/fido/fake_fido_discovery_unittest.cc
[modify] https://crrev.com/a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305/device/fido/fido_authenticator.h
[modify] https://crrev.com/a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305/device/fido/fido_device.h
[modify] https://crrev.com/a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305/device/fido/fido_device_authenticator.cc
[modify] https://crrev.com/a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305/device/fido/fido_device_authenticator.h
[modify] https://crrev.com/a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305/device/fido/fido_discovery.cc
[modify] https://crrev.com/a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305/device/fido/fido_discovery_unittest.cc
[modify] https://crrev.com/a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305/device/fido/fido_request_handler_base.cc
[modify] https://crrev.com/a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305/device/fido/fido_request_handler_base.h
[modify] https://crrev.com/a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305/device/fido/fido_request_handler_unittest.cc
[modify] https://crrev.com/a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305/device/fido/get_assertion_handler_unittest.cc
[modify] https://crrev.com/a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305/device/fido/mac/authenticator.h
[modify] https://crrev.com/a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305/device/fido/mac/authenticator.mm
[modify] https://crrev.com/a02fc8b85fb0acc2856cf3ba332ca1c4dc6bc305/device/fido/make_credential_handler_unittest.cc

Blockedon: 876841
Project Member

Comment 22 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/86e9b65dda82e86862f1fbfd0878600da1ae4be4

commit 86e9b65dda82e86862f1fbfd0878600da1ae4be4
Author: Balazs Engedy <engedy@chromium.org>
Date: Thu Aug 23 09:33:05 2018

Add key not/already registered error screens for WebAuthn.

The screens are never shown for now outside of tests.

Also rename the `error_timeout` illustration to just `error`, as it is
now used for all kinds of errors, not just timeouts.

Bug:  847985 ,  849323 
Change-Id: Ieb73254c20948c5cdffb7513249cbae61980e1a2
TBR: dschuyler@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1185008
Reviewed-by: Jun Choi <hongjunchoi@chromium.org>
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585434}
[modify] https://crrev.com/86e9b65dda82e86862f1fbfd0878600da1ae4be4/chrome/browser/browser_resources.grd
[rename] https://crrev.com/86e9b65dda82e86862f1fbfd0878600da1ae4be4/chrome/browser/resources/webauthn/2x/error.png
[rename] https://crrev.com/86e9b65dda82e86862f1fbfd0878600da1ae4be4/chrome/browser/resources/webauthn/error.png
[modify] https://crrev.com/86e9b65dda82e86862f1fbfd0878600da1ae4be4/chrome/browser/ui/views/webauthn/sheet_view_factory.cc
[modify] https://crrev.com/86e9b65dda82e86862f1fbfd0878600da1ae4be4/chrome/browser/ui/webauthn/authenticator_dialog_browsertest.cc
[modify] https://crrev.com/86e9b65dda82e86862f1fbfd0878600da1ae4be4/chrome/browser/ui/webauthn/sheet_models.cc
[modify] https://crrev.com/86e9b65dda82e86862f1fbfd0878600da1ae4be4/chrome/browser/ui/webauthn/sheet_models.h
[modify] https://crrev.com/86e9b65dda82e86862f1fbfd0878600da1ae4be4/chrome/browser/webauthn/authenticator_request_dialog_model.h

Project Member

Comment 23 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e03ef80704ff372bde51966b9ea54074754c8e34

commit e03ef80704ff372bde51966b9ea54074754c8e34
Author: Martin Kreichgauer <martinkr@google.com>
Date: Thu Aug 23 09:40:05 2018

fido: add a delay for dispatching requests to Touch ID from the UI

Bug:  678128 , 847985 , 876806 
Change-Id: I27acd32a7796edff6aabce304b698365063a4f7e
Reviewed-on: https://chromium-review.googlesource.com/1185237
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585439}
[modify] https://crrev.com/e03ef80704ff372bde51966b9ea54074754c8e34/chrome/browser/webauthn/authenticator_request_dialog_model.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6065c1d634f79eb56146abec87767c9f65ff796e

commit 6065c1d634f79eb56146abec87767c9f65ff796e
Author: Jun Choi <hongjunchoi@chromium.org>
Date: Thu Aug 23 19:04:48 2018

Reland "Move call to GetInfo command after AuthenticatorAdded()"

This is a reland of 42e740f0a8c1ea3fd7f90435983dfe9d772775ab

Original change's description:
> Move call to GetInfo command after AuthenticatorAdded()
> 
> In order to prevent system UI dialogs for BLE/Touch ID from appearing
> prior to WebAuthN UI, we hold off on dispatching request to
> authenticators until user consents on using the specified transport
> type. However, in current implementation, we send GetInfo command to
> external BLE authenticators as soon as we discover the device. This
> would trigger OS UI dialog on Mac for unpaired devices and potentially
> cause BLE connection failures on other platforms. As so, move call to
> GetInfo command after FidoRequestHandlerBase::AuthenticatorAdded().
> 
> Bug:  847985 
> Change-Id: I37b7cd8118c3edb799fa29c36700febbb2b3d08e
> Reviewed-on: https://chromium-review.googlesource.com/1182946
> Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
> Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#585130}

Bug:  847985 
Change-Id: I457228768db9abc25a57f167928e55f342c1f15e
Reviewed-on: https://chromium-review.googlesource.com/1187082
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/master@{#585569}
[modify] https://crrev.com/6065c1d634f79eb56146abec87767c9f65ff796e/content/browser/webauth/authenticator_impl_unittest.cc
[modify] https://crrev.com/6065c1d634f79eb56146abec87767c9f65ff796e/device/fido/fake_fido_discovery_unittest.cc
[modify] https://crrev.com/6065c1d634f79eb56146abec87767c9f65ff796e/device/fido/fido_authenticator.h
[modify] https://crrev.com/6065c1d634f79eb56146abec87767c9f65ff796e/device/fido/fido_device.h
[modify] https://crrev.com/6065c1d634f79eb56146abec87767c9f65ff796e/device/fido/fido_device_authenticator.cc
[modify] https://crrev.com/6065c1d634f79eb56146abec87767c9f65ff796e/device/fido/fido_device_authenticator.h
[modify] https://crrev.com/6065c1d634f79eb56146abec87767c9f65ff796e/device/fido/fido_discovery.cc
[modify] https://crrev.com/6065c1d634f79eb56146abec87767c9f65ff796e/device/fido/fido_discovery_unittest.cc
[modify] https://crrev.com/6065c1d634f79eb56146abec87767c9f65ff796e/device/fido/fido_request_handler_base.cc
[modify] https://crrev.com/6065c1d634f79eb56146abec87767c9f65ff796e/device/fido/fido_request_handler_base.h
[modify] https://crrev.com/6065c1d634f79eb56146abec87767c9f65ff796e/device/fido/fido_request_handler_unittest.cc
[modify] https://crrev.com/6065c1d634f79eb56146abec87767c9f65ff796e/device/fido/get_assertion_handler_unittest.cc
[modify] https://crrev.com/6065c1d634f79eb56146abec87767c9f65ff796e/device/fido/mac/authenticator.h
[modify] https://crrev.com/6065c1d634f79eb56146abec87767c9f65ff796e/device/fido/mac/authenticator.mm
[modify] https://crrev.com/6065c1d634f79eb56146abec87767c9f65ff796e/device/fido/make_credential_handler_unittest.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Aug 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ff1335d9909996e3f7d9fdc269c475c4fc2c24ae

commit ff1335d9909996e3f7d9fdc269c475c4fc2c24ae
Author: Balazs Engedy <engedy@chromium.org>
Date: Fri Aug 24 15:39:05 2018

WebAuthn: Add pop-up menu for choosing other transports.

Bug:  847985 
Change-Id: I2ae8ec26a2c01e2ffd1b1527cad0387c1e90f389
Reviewed-on: https://chromium-review.googlesource.com/1187104
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Jun Choi <hongjunchoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585851}
[modify] https://crrev.com/ff1335d9909996e3f7d9fdc269c475c4fc2c24ae/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/ff1335d9909996e3f7d9fdc269c475c4fc2c24ae/chrome/browser/ui/views/webauthn/authenticator_dialog_view_browsertest.cc
[modify] https://crrev.com/ff1335d9909996e3f7d9fdc269c475c4fc2c24ae/chrome/browser/ui/views/webauthn/authenticator_request_dialog_view.cc
[modify] https://crrev.com/ff1335d9909996e3f7d9fdc269c475c4fc2c24ae/chrome/browser/ui/views/webauthn/authenticator_request_dialog_view.h
[modify] https://crrev.com/ff1335d9909996e3f7d9fdc269c475c4fc2c24ae/chrome/browser/ui/views/webauthn/transport_list_view.cc
[modify] https://crrev.com/ff1335d9909996e3f7d9fdc269c475c4fc2c24ae/chrome/browser/ui/webauthn/authenticator_dialog_browsertest.cc
[modify] https://crrev.com/ff1335d9909996e3f7d9fdc269c475c4fc2c24ae/chrome/browser/ui/webauthn/authenticator_request_sheet_model.h
[add] https://crrev.com/ff1335d9909996e3f7d9fdc269c475c4fc2c24ae/chrome/browser/ui/webauthn/other_transports_menu_model.cc
[add] https://crrev.com/ff1335d9909996e3f7d9fdc269c475c4fc2c24ae/chrome/browser/ui/webauthn/other_transports_menu_model.h
[modify] https://crrev.com/ff1335d9909996e3f7d9fdc269c475c4fc2c24ae/chrome/browser/ui/webauthn/sheet_models.cc
[modify] https://crrev.com/ff1335d9909996e3f7d9fdc269c475c4fc2c24ae/chrome/browser/ui/webauthn/sheet_models.h
[modify] https://crrev.com/ff1335d9909996e3f7d9fdc269c475c4fc2c24ae/chrome/browser/ui/webauthn/transport_utils.cc
[modify] https://crrev.com/ff1335d9909996e3f7d9fdc269c475c4fc2c24ae/chrome/browser/ui/webauthn/transport_utils.h
[modify] https://crrev.com/ff1335d9909996e3f7d9fdc269c475c4fc2c24ae/chrome/browser/webauthn/authenticator_request_dialog_model.cc
[modify] https://crrev.com/ff1335d9909996e3f7d9fdc269c475c4fc2c24ae/chrome/browser/webauthn/authenticator_request_dialog_model.h

Project Member

Comment 26 by bugdroid1@chromium.org, Aug 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/72a7f8def14404fb01121ef8dcba7abb1898927f

commit 72a7f8def14404fb01121ef8dcba7abb1898927f
Author: Balazs Engedy <engedy@chromium.org>
Date: Mon Aug 27 14:43:17 2018

Gate WebAuthn UX on disabled-by-default base::Feature flag.

Previously, it was gated on a command-line option which cannot be
controlled via field trials. Also, enable the feature on continous
build.

Bug:  847985 
Change-Id: I07ae0a7ef372b27309017da538f1dd4f85a9e3e3
Reviewed-on: https://chromium-review.googlesource.com/1187157
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586258}
[modify] https://crrev.com/72a7f8def14404fb01121ef8dcba7abb1898927f/chrome/browser/webauthn/chrome_authenticator_request_delegate.cc
[modify] https://crrev.com/72a7f8def14404fb01121ef8dcba7abb1898927f/chrome/common/chrome_features.cc
[modify] https://crrev.com/72a7f8def14404fb01121ef8dcba7abb1898927f/chrome/common/chrome_features.h
[modify] https://crrev.com/72a7f8def14404fb01121ef8dcba7abb1898927f/chrome/common/chrome_switches.cc
[modify] https://crrev.com/72a7f8def14404fb01121ef8dcba7abb1898927f/chrome/common/chrome_switches.h
[modify] https://crrev.com/72a7f8def14404fb01121ef8dcba7abb1898927f/testing/variations/fieldtrial_testing_config.json

Project Member

Comment 27 by bugdroid1@chromium.org, Aug 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e4d9494e9ba97d8162f766fe1b9d63d4cf44d16d

commit e4d9494e9ba97d8162f766fe1b9d63d4cf44d16d
Author: Martin Kreichgauer <martinkr@google.com>
Date: Mon Aug 27 20:03:53 2018

fido: increase the delay for triggering Touch ID from the UI

Bug:  847985 
Change-Id: I4c6c004db4f5180200c421b8bc7b6c530a7d653f
Reviewed-on: https://chromium-review.googlesource.com/1189105
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/master@{#586383}
[modify] https://crrev.com/e4d9494e9ba97d8162f766fe1b9d63d4cf44d16d/chrome/browser/webauthn/authenticator_request_dialog_model.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Aug 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/311a2e6f35ab9ba7d998d2aeb2afc7c20d806d37

commit 311a2e6f35ab9ba7d998d2aeb2afc7c20d806d37
Author: Martin Kreichgauer <martinkr@google.com>
Date: Wed Aug 29 16:09:12 2018

webauthn: add special error sheet for GetAssertion on internal transport

This adds a special error screen for GetAssertion requests that are
about to be dispatched to Touch ID but there is no available credential
in the keychain. The screen can be reached by selecting Touch ID from
the transport selection UI. If Touch ID is the only available transport,
the UI auto advances to the error screen.

This also fixes a bug where the UI auto advanced to the regular Touch ID
screen on GetAssertion even though no matching credential was present.

Bug:  678128 , 847985 
Change-Id: Ia4ce02a495821c7d0a3caa34c7a2e7eaeb1cbc42
Reviewed-on: https://chromium-review.googlesource.com/1194954
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587140}
[modify] https://crrev.com/311a2e6f35ab9ba7d998d2aeb2afc7c20d806d37/chrome/browser/ui/views/webauthn/sheet_view_factory.cc
[modify] https://crrev.com/311a2e6f35ab9ba7d998d2aeb2afc7c20d806d37/chrome/browser/ui/webauthn/sheet_models.cc
[modify] https://crrev.com/311a2e6f35ab9ba7d998d2aeb2afc7c20d806d37/chrome/browser/ui/webauthn/sheet_models.h
[modify] https://crrev.com/311a2e6f35ab9ba7d998d2aeb2afc7c20d806d37/chrome/browser/webauthn/authenticator_request_dialog_model.cc
[modify] https://crrev.com/311a2e6f35ab9ba7d998d2aeb2afc7c20d806d37/chrome/browser/webauthn/authenticator_request_dialog_model.h
[modify] https://crrev.com/311a2e6f35ab9ba7d998d2aeb2afc7c20d806d37/chrome/browser/webauthn/authenticator_request_dialog_model_unittest.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Aug 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1abf6643628c17a905420f9366adf04a84f3ebf6

commit 1abf6643628c17a905420f9366adf04a84f3ebf6
Author: Martin Kreichgauer <martinkr@google.com>
Date: Thu Aug 30 14:55:29 2018

webauthn: prevent UI from dispatching to the same authenticator multiple times

Using the transport switch drop-down menu, it is possible to cycle away
from and then back to the Touch ID sheet, which currently causes the
request to be dispatched onto the TouchIdAuthenticator each time the
sheet is shown. This sort of works in the production build (it restarts
the request, which will make the Touch ID dialog disappear and then
reappear). In debug, however, it crashes on a DCHECK.

We haven't defined whether a FidoAuthenticator should be able to handle
multiple invocations of MakeCredential/GetAssertion over the lifetime of
the instance. Hence, the UI should stop doing this.

Bug:  678128 , 847985 
Change-Id: Id008de98bc8aa6477e9863ca2e22cd136316b423
Reviewed-on: https://chromium-review.googlesource.com/1196427
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587575}
[modify] https://crrev.com/1abf6643628c17a905420f9366adf04a84f3ebf6/chrome/browser/webauthn/authenticator_request_dialog_model.cc
[modify] https://crrev.com/1abf6643628c17a905420f9366adf04a84f3ebf6/chrome/browser/webauthn/authenticator_request_dialog_model.h
[modify] https://crrev.com/1abf6643628c17a905420f9366adf04a84f3ebf6/chrome/browser/webauthn/authenticator_request_dialog_model_unittest.cc

Status: Fixed (was: Started)
The core UX launched in M70. Further changes to UX will be tracked in various other bugs, so closing this out.

Sign in to add a comment