New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 866601 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 847985



Sign in to add a comment

Make device/fido observable to UI components

Project Member Reported by hongjunchoi@chromium.org, Jul 23

Issue description

Make events triggered/found in device/fido module be observable from chrome/browser/webauth UI components. 

 
Cc: martinkr@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 27

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

commit 7d7139aca3288d519b3c81fe69e0ffe268fc3570
Author: Jun Choi <hongjunchoi@chromium.org>
Date: Fri Jul 27 21:02:04 2018

Make device/fido observable from UI components

Create observer interface so that WebAuthN UI components (i.e.
ChromeAuthenticaterRequestDelegate and AuthenticatorRequestDialog) can
get notified of device discovery.

Bug:  866601 
Change-Id: I0af544f4c3754bd0b76b6a354833ed12858a0967
Reviewed-on: https://chromium-review.googlesource.com/1147307
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578782}
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/chrome/browser/webauthn/authenticator_request_dialog_model.cc
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/chrome/browser/webauthn/authenticator_request_dialog_model.h
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/chrome/browser/webauthn/chrome_authenticator_request_delegate.cc
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/chrome/browser/webauthn/chrome_authenticator_request_delegate.h
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/content/browser/webauth/authenticator_impl.cc
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/content/browser/webauth/authenticator_impl.h
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/content/browser/webauth/authenticator_impl_unittest.cc
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/content/public/browser/BUILD.gn
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/content/public/browser/DEPS
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/content/public/browser/authenticator_request_client_delegate.cc
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/content/public/browser/authenticator_request_client_delegate.h
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/content/test/BUILD.gn
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/ble/fido_ble_device.cc
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/ble/fido_ble_device.h
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/ble/fido_ble_discovery_base.cc
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/cable/fido_cable_device.cc
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/cable/fido_cable_device.h
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/cable/fido_cable_discovery_unittest.cc
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/fido_authenticator.h
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/fido_device.h
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/fido_device_authenticator.cc
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/fido_device_authenticator.h
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/fido_request_handler_base.cc
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/fido_request_handler_base.h
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/hid/fido_hid_device.cc
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/hid/fido_hid_device.h
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/mac/authenticator.h
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/mac/authenticator.mm
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/mock_fido_device.cc
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/mock_fido_device.h
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/virtual_fido_device.cc
[modify] https://crrev.com/7d7139aca3288d519b3c81fe69e0ffe268fc3570/device/fido/virtual_fido_device.h

Blocking: 847985
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 13

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

commit e4aee4ff3c5981448f8b9cbc9bc948f005d86392
Author: Jun Choi <hongjunchoi@chromium.org>
Date: Mon Aug 13 19:35:07 2018

Add transport type to CredentialDescriptor

Relying parties can enforce restriction on which transport type should
be used to call GetAssertion API using
PublicKeyCredentialDescriptor.transports[1]. As so, add transport field
to PublicKeyCredentialDescriptor object.

Furthermore, fix the blink layer type conversion for
PublicKeyCredentialDescriptor. AuthenticatorTransport field is optional.
When AuthenticatorTransport field is not set, we should allow all
transport types. Currently, transport list in credential descriptor is
set to empty array. Change this so that transport list is set to array
of all transport types instead.

[1]: https://w3c.github.io/webauthn/#enumdef-authenticatortransport

Bug:  866601 
Change-Id: I9cf2a17977a1078db772908be23745f06a680af8
Reviewed-on: https://chromium-review.googlesource.com/1166469
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582666}
[modify] https://crrev.com/e4aee4ff3c5981448f8b9cbc9bc948f005d86392/content/browser/webauth/authenticator_impl_unittest.cc
[modify] https://crrev.com/e4aee4ff3c5981448f8b9cbc9bc948f005d86392/content/browser/webauth/authenticator_type_converters.cc
[modify] https://crrev.com/e4aee4ff3c5981448f8b9cbc9bc948f005d86392/device/fido/get_assertion_handler_unittest.cc
[modify] https://crrev.com/e4aee4ff3c5981448f8b9cbc9bc948f005d86392/device/fido/get_assertion_request_handler.cc
[modify] https://crrev.com/e4aee4ff3c5981448f8b9cbc9bc948f005d86392/device/fido/public_key_credential_descriptor.cc
[modify] https://crrev.com/e4aee4ff3c5981448f8b9cbc9bc948f005d86392/device/fido/public_key_credential_descriptor.h
[modify] https://crrev.com/e4aee4ff3c5981448f8b9cbc9bc948f005d86392/third_party/blink/public/platform/modules/webauthn/authenticator.mojom
[modify] https://crrev.com/e4aee4ff3c5981448f8b9cbc9bc948f005d86392/third_party/blink/renderer/modules/credentialmanager/credential_manager_type_converters.cc
[modify] https://crrev.com/e4aee4ff3c5981448f8b9cbc9bc948f005d86392/third_party/blink/renderer/modules/credentialmanager/public_key_credential_descriptor.idl

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 14

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

commit 84abf8e6330b94853e5c3aa70ba235dafffb45ec
Author: Jun Choi <hongjunchoi@chromium.org>
Date: Tue Aug 14 00:03:00 2018

Implement struct based embedder/device interface

Currently, ChromeAuthenticatorRequestDelegate implements
FidoRequestHandlerBase::AuthenticatorMap interface, and each events in
transport layer trigger observer function in the embedder.

In order to make embedder interface simpler, aggregate all transport
layer events that is required prior to initiating WebAuthN UI dialog
in FidoRequestHandlerBase. Once all the information is gathered, combine
all retrieved data in FidoUiAprioriData struct and invoke single
OnTransportLayerInfoReceived() observer function in the embedder layer.

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

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 14

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 16

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

commit 159452d91b7e1af55e442397f6a8ce1e85416307
Author: Balazs Engedy <engedy@chromium.org>
Date: Thu Aug 16 15:18:18 2018

Only create FidoDiscoveries for transports that are supported and allowed.

Ensure that FidoRequestHandlerBase only instantiates FidoDiscoveries for
transports that are in the intersection of those supported by the client,
and those allowed by the relying party; where the allowed transports are:

 (1) For MakeCredential calls:
   -- {BLE, caBLE, USB, NFC} if `authenticatorAtttachment` == `cross-platform`,
   -- {Internal} if `authenticatorAtttachment` == `platform`,
   -- (all) if `authenticatorAtttachment` == `any`.

 (2) For GetAssertion calls:
   -- (all) if `allowedCredentials` is undefined or empty, or if it contains
      an entry with `transports` undefined or empty,
   -- the union of `transports` lists in all elements of `allowedCredentials`.

FidoRequestHandlerBase::Observer::OnTransportAvailabilityEnumerated is also
changed to be consistent with this, i.e., the |available_transports| list in
TransportAvailabilityInfo will also be this intersection set.

Finally, the CL ensure that this method is always called on the observer,
even if enumerating transports is finished before the observer is set.

Bug:  866601 
Change-Id: I00fbd6684fd52851149bdb0dd39800aafda54630
Reviewed-on: https://chromium-review.googlesource.com/1175118
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583654}
[modify] https://crrev.com/159452d91b7e1af55e442397f6a8ce1e85416307/content/browser/webauth/authenticator_impl.cc
[modify] https://crrev.com/159452d91b7e1af55e442397f6a8ce1e85416307/content/browser/webauth/authenticator_impl_unittest.cc
[modify] https://crrev.com/159452d91b7e1af55e442397f6a8ce1e85416307/device/fido/fido_request_handler.h
[modify] https://crrev.com/159452d91b7e1af55e442397f6a8ce1e85416307/device/fido/fido_request_handler_base.cc
[modify] https://crrev.com/159452d91b7e1af55e442397f6a8ce1e85416307/device/fido/fido_request_handler_base.h
[modify] https://crrev.com/159452d91b7e1af55e442397f6a8ce1e85416307/device/fido/fido_request_handler_unittest.cc
[modify] https://crrev.com/159452d91b7e1af55e442397f6a8ce1e85416307/device/fido/fido_transport_protocol.cc
[modify] https://crrev.com/159452d91b7e1af55e442397f6a8ce1e85416307/device/fido/fido_transport_protocol.h
[modify] https://crrev.com/159452d91b7e1af55e442397f6a8ce1e85416307/device/fido/get_assertion_handler_unittest.cc
[modify] https://crrev.com/159452d91b7e1af55e442397f6a8ce1e85416307/device/fido/get_assertion_request_handler.cc
[modify] https://crrev.com/159452d91b7e1af55e442397f6a8ce1e85416307/device/fido/get_assertion_request_handler.h
[modify] https://crrev.com/159452d91b7e1af55e442397f6a8ce1e85416307/device/fido/make_credential_handler_unittest.cc
[modify] https://crrev.com/159452d91b7e1af55e442397f6a8ce1e85416307/device/fido/make_credential_request_handler.cc
[modify] https://crrev.com/159452d91b7e1af55e442397f6a8ce1e85416307/device/fido/make_credential_request_handler.h
[modify] https://crrev.com/159452d91b7e1af55e442397f6a8ce1e85416307/device/fido/mock_fido_device.h

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 16

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

commit 4af417495467f055d66a42661fdcdb1cbf8610b0
Author: Jun Choi <hongjunchoi@chromium.org>
Date: Thu Aug 16 20:41:04 2018

Add request dispatching logic to AuthenticatorDelegate

In order to prevent MacTouchId and BLE system UI dialog from appearing
prior to user explictly selecting which transport type to use,
ChromeAuthenticatorRequestDelegate should be able to "lazily" dispatch
WebAuthN request to FidoAuthenticators. Add logic trigger request from
embedder layer.

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

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 20

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

commit 0c4fdc59cf081abceceedd696f7fb679d1c2e44e
Author: Balazs Engedy <engedy@chromium.org>
Date: Mon Aug 20 19:15:28 2018

Display relying party ID on WebAuthn UI.

Also, fix the plumbing of |rp_id| out of FidoRequestHandlers into
TransportAvailabilityInfo, and add unittests to verify correctness.

Bug:  866601 ,  849323 
Change-Id: I0e8c1250208c52306501fab7b9c55a2b1a34bbd1
Reviewed-on: https://chromium-review.googlesource.com/1181265
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Jun Choi <hongjunchoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584526}
[modify] https://crrev.com/0c4fdc59cf081abceceedd696f7fb679d1c2e44e/chrome/browser/ui/views/webauthn/authenticator_dialog_view_browsertest.cc
[modify] https://crrev.com/0c4fdc59cf081abceceedd696f7fb679d1c2e44e/chrome/browser/ui/webauthn/authenticator_dialog_browsertest.cc
[modify] https://crrev.com/0c4fdc59cf081abceceedd696f7fb679d1c2e44e/chrome/browser/ui/webauthn/sheet_models.cc
[modify] https://crrev.com/0c4fdc59cf081abceceedd696f7fb679d1c2e44e/chrome/browser/ui/webauthn/sheet_models.h
[modify] https://crrev.com/0c4fdc59cf081abceceedd696f7fb679d1c2e44e/chrome/browser/webauthn/authenticator_request_dialog_model.h
[modify] https://crrev.com/0c4fdc59cf081abceceedd696f7fb679d1c2e44e/device/fido/get_assertion_handler_unittest.cc
[modify] https://crrev.com/0c4fdc59cf081abceceedd696f7fb679d1c2e44e/device/fido/get_assertion_request_handler.cc
[modify] https://crrev.com/0c4fdc59cf081abceceedd696f7fb679d1c2e44e/device/fido/make_credential_handler_unittest.cc
[modify] https://crrev.com/0c4fdc59cf081abceceedd696f7fb679d1c2e44e/device/fido/make_credential_request_handler.cc

Status: Fixed (was: Started)
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 29

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

commit ed4966f7cecb25e6cfe11eef69484f6eaf2cd87f
Author: Balazs Engedy <engedy@chromium.org>
Date: Wed Aug 29 22:07:45 2018

Plumb `interesting` failure reasons to embedder.

Add AuthenticatorRequestClientDelegate::DidFailWithInterestingReason
that informs the embedder when the request being serviced by
AuthenticatorImpl fails because of either:
 -- timeout,
 -- the key already being registered for MakeCredential,
 -- the key not being registered for GetAssertion.

Bug:  866601 
Change-Id: Id018033c0b306a14ebf33d307ab8c69f7d8ad96d
Reviewed-on: https://chromium-review.googlesource.com/1195490
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587306}
[modify] https://crrev.com/ed4966f7cecb25e6cfe11eef69484f6eaf2cd87f/content/browser/webauth/authenticator_impl.cc
[modify] https://crrev.com/ed4966f7cecb25e6cfe11eef69484f6eaf2cd87f/content/browser/webauth/authenticator_impl.h
[modify] https://crrev.com/ed4966f7cecb25e6cfe11eef69484f6eaf2cd87f/content/browser/webauth/authenticator_impl_unittest.cc
[modify] https://crrev.com/ed4966f7cecb25e6cfe11eef69484f6eaf2cd87f/content/public/browser/authenticator_request_client_delegate.cc
[modify] https://crrev.com/ed4966f7cecb25e6cfe11eef69484f6eaf2cd87f/content/public/browser/authenticator_request_client_delegate.h

Sign in to add a comment