Add WebAuthn bindings and client interface.

Add WebAuthn bindings and client interface.

This CL adds the WebAuthn bindings and interface to support
WebAuthentication.makeCredential and WebAuthentication.getAssertion.

The draft spec is here: https://w3c.github.io/webauthn/

The interface is likely to change as the spec matures. We're tracking the
comments and suggestions from this review via issues opened with the working
group:
https://github.com/w3c/webauthn/issues/310
https://github.com/w3c/webauthn/issues/311
https://github.com/w3c/webauthn/issues/312
https://github.com/w3c/webauthn/issues/313

This patch also proactively implements the change to WebAuthnAttestation
suggested here: https://github.com/w3c/webauthn/issues/244

BUG=664630
Review-Url: https://codereview.chromium.org/2533863002
Cr-Commit-Position: refs/heads/master@{#445239}
Committed: https://chromium.googlesource.com/chromium/src/+/8d43296f5c9c5c7f5f362ea599881fbd28df4898

[kpaulhamus, 2016-11-28 18:28:54]

kpaulhamus@chromium.org changed reviewers:
+ esprehn@chromium.org, mkwst@chromium.org

[haraken, 2016-11-29 02:07:10]

Can we add tests?

Also it would be nicer if you could split the giant CL into smaller pieces.

[blink-reviews, 2016-11-29 02:25:01]

This is part 1 of many CLs, and I held off on tests to keep it from getting
larger than it had to be. Unfortunately, I don't think I can break this up
and still be able to compile, as the interfaces are all cross-referencing.
(There's no real functionality right now; it's just the barebones framework
for the interfaces defined in the spec.)

On Mon, Nov 28, 2016 at 6:07 PM <haraken@chromium.org> wrote:

> Can we add tests?
>
> Also it would be nicer if you could split the giant CL into smaller pieces.
>
>
> https://codereview.chromium.org/2533863002/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-11-29 02:25:01]

This is part 1 of many CLs, and I held off on tests to keep it from getting
larger than it had to be. Unfortunately, I don't think I can break this up
and still be able to compile, as the interfaces are all cross-referencing.
(There's no real functionality right now; it's just the barebones framework
for the interfaces defined in the spec.)

On Mon, Nov 28, 2016 at 6:07 PM <haraken@chromium.org> wrote:

> Can we add tests?
>
> Also it would be nicer if you could split the giant CL into smaller pieces.
>
>
> https://codereview.chromium.org/2533863002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[esprehn, 2016-11-29 06:00:49]

This api needs a review. Prefixing an api with "Web" is very unusual, I think
WebSockets might be the only instance of that? Web Bluetooth just uses Bluetooth
for example.

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/Account.idl (right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/Account.idl:6: 
Ditto, the spec needs to use scoped names.

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/ClientData.idl (right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/ClientData.idl:7: dictionary
ClientData {
This dictionary is too generically named. The spec needs to use more specific
names.

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/NavigatorAuth.idl
(right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/NavigatorAuth.idl:8:
[RuntimeEnabled=WebAuthentication] readonly attribute WebAuthentication
authentication;
I don't think we've ever shipped an api with a Web prefix before. This probably
needs tag review.

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/webauthentication/ScopedCredentialDescriptor.idl
(right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/ScopedCredentialDescriptor.idl:19:
};
No space

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/WebAuthnAssertion.idl
(right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/WebAuthnAssertion.idl:10: ]
interface WebAuthnAssertion {
Why not just WebAuth or Auth? The n in the middle doesn't mean anything and is
going to be a wart for developers to remember for the next 30 years.

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/WebAuthnAttestation.idl
(right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/WebAuthnAttestation.idl:9:
SecureContext
These should all probably have constructors.

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/WebAuthnExtensions.idl
(right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/WebAuthnExtensions.idl:8: };
Why add an empty dictionary?

[Mike West, 2016-11-29 06:30:16]

I agree with Elliot's suggestions. The naming here is weird, `WebAuthn` in
particular. We don't need to adopt the group's name in the interfaces we expose
to the web.

On 2016/11/29 at 02:07:10, haraken wrote:
> Can we add tests?

You can add tests pretty easily with idlharness.js. Look at things like
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/http/test...
for example (and
https://github.com/w3c/testharness.js/blob/master/docs/idlharness.md for docs).

[kpaulhamus, 2016-11-29 22:30:45]

On 2016/11/29 06:00:49, esprehn wrote:
> This api needs a review. Prefixing an api with "Web" is very unusual, I think
> WebSockets might be the only instance of that? Web Bluetooth just uses
Bluetooth
> for example.
> 

Also WebUSB, right? And in the same way WebUSBs APIs are obtained via
navigator.usb, we're accessing these via navigator.authentication.

But I'm going ahead and passing along the questions about naming to the working
group to see what they say - particularly with regards to WebAuthn.

>
https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
> File third_party/WebKit/Source/modules/webauthentication/Account.idl (right):
> 
>
https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
> third_party/WebKit/Source/modules/webauthentication/Account.idl:6: 
> Ditto, the spec needs to use scoped names.
> 
>
https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
> File third_party/WebKit/Source/modules/webauthentication/ClientData.idl
(right):
> 
>
https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
> third_party/WebKit/Source/modules/webauthentication/ClientData.idl:7:
dictionary
> ClientData {
> This dictionary is too generically named. The spec needs to use more specific
> names.
> 
>
https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
> File third_party/WebKit/Source/modules/webauthentication/NavigatorAuth.idl
> (right):
> 
>
https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
> third_party/WebKit/Source/modules/webauthentication/NavigatorAuth.idl:8:
> [RuntimeEnabled=WebAuthentication] readonly attribute WebAuthentication
> authentication;
> I don't think we've ever shipped an api with a Web prefix before. This
probably
> needs tag review.
> 
>
https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
> File
>
third_party/WebKit/Source/modules/webauthentication/ScopedCredentialDescriptor.idl
> (right):
> 
>
https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
>
third_party/WebKit/Source/modules/webauthentication/ScopedCredentialDescriptor.idl:19:
> };
> No space
> 
>
https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
> File third_party/WebKit/Source/modules/webauthentication/WebAuthnAssertion.idl
> (right):
> 
>
https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
> third_party/WebKit/Source/modules/webauthentication/WebAuthnAssertion.idl:10:
]
> interface WebAuthnAssertion {
> Why not just WebAuth or Auth? The n in the middle doesn't mean anything and is
> going to be a wart for developers to remember for the next 30 years.
> 
>
https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
> File
third_party/WebKit/Source/modules/webauthentication/WebAuthnAttestation.idl
> (right):
> 
>
https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
> third_party/WebKit/Source/modules/webauthentication/WebAuthnAttestation.idl:9:
> SecureContext
> These should all probably have constructors.
> 
>
https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
> File
third_party/WebKit/Source/modules/webauthentication/WebAuthnExtensions.idl
> (right):
> 
>
https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
> third_party/WebKit/Source/modules/webauthentication/WebAuthnExtensions.idl:8:
};
> Why add an empty dictionary?

This dictionary can contain zero or more extensions; later I expect to add one,
but for now it's here because it's referenced by some of the other interfaces.

[kpaulhamus, 2016-12-01 21:44:52]

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/NavigatorAuth.idl
(right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/NavigatorAuth.idl:8:
[RuntimeEnabled=WebAuthentication] readonly attribute WebAuthentication
authentication;
On 2016/11/29 06:00:49, esprehn wrote:
> I don't think we've ever shipped an api with a Web prefix before. This
probably
> needs tag review.

Just noting we talked about this and decided the 'Web' can stay for now.

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/webauthentication/ScopedCredentialDescriptor.idl
(right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/ScopedCredentialDescriptor.idl:19:
};
On 2016/11/29 06:00:49, esprehn wrote:
> No space

Done.

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/WebAuthnExtensions.idl
(right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/WebAuthnExtensions.idl:8: };
On 2016/11/29 06:00:49, esprehn wrote:
> Why add an empty dictionary?

This dictionary can contain zero or more extensions; later I expect to add one,
but for now it's here because it's referenced by some of the other interfaces.

[esprehn, 2016-12-01 21:55:36]

I meant keep the Web prefix on the directories. We don't prefix the interfaces
we ship for USB.

[kpaulhamus, 2016-12-17 00:00:24]

Description was changed from

==========
Add WebAuthn bindings and client interface.

This CL adds the WebAuthn bindings and interface to support
WebAuthentication.makeCredential and WebAuthentication.getAssertion.

The draft spec is here: https://w3c.github.io/webauthn/

BUG=664630
==========

to

==========
Add WebAuthn bindings and client interface.

This CL adds the WebAuthn bindings and interface to support
WebAuthentication.makeCredential and WebAuthentication.getAssertion.

The draft spec is here: https://w3c.github.io/webauthn/

The interface is likely to change as the spec matures. We're tracking the
comments and suggestions from this review via issues opened with the working
group:
https://github.com/w3c/webauthn/issues/310
https://github.com/w3c/webauthn/issues/311
https://github.com/w3c/webauthn/issues/312

BUG=664630
==========

[kpaulhamus, 2016-12-17 00:01:40]

Description was changed from

==========
Add WebAuthn bindings and client interface.

This CL adds the WebAuthn bindings and interface to support
WebAuthentication.makeCredential and WebAuthentication.getAssertion.

The draft spec is here: https://w3c.github.io/webauthn/

The interface is likely to change as the spec matures. We're tracking the
comments and suggestions from this review via issues opened with the working
group:
https://github.com/w3c/webauthn/issues/310
https://github.com/w3c/webauthn/issues/311
https://github.com/w3c/webauthn/issues/312

BUG=664630
==========

to

==========
Add WebAuthn bindings and client interface.

This CL adds the WebAuthn bindings and interface to support
WebAuthentication.makeCredential and WebAuthentication.getAssertion.

The draft spec is here: https://w3c.github.io/webauthn/

The interface is likely to change as the spec matures. We're tracking the
comments and suggestions from this review via issues opened with the working
group:
https://github.com/w3c/webauthn/issues/310
https://github.com/w3c/webauthn/issues/311
https://github.com/w3c/webauthn/issues/312

This patch also proactively implements the change to WebAuthnAttestation
suggested here: https://github.com/w3c/webauthn/issues/244

BUG=664630
==========

[kpaulhamus, 2016-12-17 00:12:39]

All right, I've raised the comments about naming and scoping to the working
group. I've created issues with the working group to track them and linked them
in the CL description. 

Also, as this is my first patch, could one of you run try jobs for me?

[esprehn, 2016-12-17 00:13:15]

The CQ bit was checked by esprehn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-17 00:14:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-17 02:08:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-17 02:08:52]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[kpaulhamus, 2016-12-20 00:01:12]

The CQ bit was checked by kpaulhamus@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-20 00:01:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-20 02:27:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-20 02:27:17]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[kpaulhamus, 2016-12-20 19:14:49]

Description was changed from

==========
Add WebAuthn bindings and client interface.

This CL adds the WebAuthn bindings and interface to support
WebAuthentication.makeCredential and WebAuthentication.getAssertion.

The draft spec is here: https://w3c.github.io/webauthn/

The interface is likely to change as the spec matures. We're tracking the
comments and suggestions from this review via issues opened with the working
group:
https://github.com/w3c/webauthn/issues/310
https://github.com/w3c/webauthn/issues/311
https://github.com/w3c/webauthn/issues/312

This patch also proactively implements the change to WebAuthnAttestation
suggested here: https://github.com/w3c/webauthn/issues/244

BUG=664630
==========

to

==========
Add WebAuthn bindings and client interface.

This CL adds the WebAuthn bindings and interface to support
WebAuthentication.makeCredential and WebAuthentication.getAssertion.

The draft spec is here: https://w3c.github.io/webauthn/

The interface is likely to change as the spec matures. We're tracking the
comments and suggestions from this review via issues opened with the working
group:
https://github.com/w3c/webauthn/issues/310
https://github.com/w3c/webauthn/issues/311
https://github.com/w3c/webauthn/issues/312
https://github.com/w3c/webauthn/issues/313

This patch also proactively implements the change to WebAuthnAttestation
suggested here: https://github.com/w3c/webauthn/issues/244

BUG=664630
==========

[kpaulhamus, 2016-12-20 19:17:00]

Turns out I do have try jobs access. I'm getting failures, although I'm really
not sure how to interpret them or go about addressing them.
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/Account.idl (right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/Account.idl:6: 
On 2016/11/29 06:00:49, esprehn wrote:
> Ditto, the spec needs to use scoped names.
Tracking this with
https://github.com/w3c/webauthn/issues/312

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/ClientData.idl (right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/ClientData.idl:7: dictionary
ClientData {
On 2016/11/29 06:00:49, esprehn wrote:
> This dictionary is too generically named. The spec needs to use more specific
> names.

Tracking with https://github.com/w3c/webauthn/issues/312

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/NavigatorAuth.idl
(right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/NavigatorAuth.idl:8:
[RuntimeEnabled=WebAuthentication] readonly attribute WebAuthentication
authentication;
On 2016/12/01 21:44:52, kpaulhamus wrote:
> On 2016/11/29 06:00:49, esprehn wrote:
> > I don't think we've ever shipped an api with a Web prefix before. This
> probably
> > needs tag review.
> 
> Just noting we talked about this and decided the 'Web' can stay for now.

Tracking with https://github.com/w3c/webauthn/issues/310

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/WebAuthnAssertion.idl
(right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/WebAuthnAssertion.idl:10: ]
interface WebAuthnAssertion {
On 2016/11/29 06:00:49, esprehn wrote:
> Why not just WebAuth or Auth? The n in the middle doesn't mean anything and is
> going to be a wart for developers to remember for the next 30 years.

Tracking with https://github.com/w3c/webauthn/issues/311

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/WebAuthnAttestation.idl
(right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/WebAuthnAttestation.idl:9:
SecureContext
On 2016/11/29 06:00:49, esprehn wrote:
> These should all probably have constructors.

Tracking with https://github.com/w3c/webauthn/issues/313

[kpaulhamus, 2017-01-09 21:47:40]

The CQ bit was checked by kpaulhamus@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-09 21:48:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-09 21:54:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-09 21:54:02]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[kpaulhamus, 2017-01-09 21:56:37]

The CQ bit was checked by kpaulhamus@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-09 21:57:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kpaulhamus, 2017-01-09 23:25:02]

Hi Elliott, Mike,
I made changes based on the latest activity in the working group that address
some of your earlier comments. Can you review? Thanks!

[commit-bot: I haz the power, 2017-01-10 00:37:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-10 00:37:25]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[kpaulhamus, 2017-01-11 21:38:54]

The CQ bit was checked by kpaulhamus@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 21:39:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-11 21:43:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-11 21:43:52]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[kpaulhamus, 2017-01-11 22:29:12]

The CQ bit was checked by kpaulhamus@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 22:29:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kpaulhamus, 2017-01-11 22:40:56]

The CQ bit was checked by kpaulhamus@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 22:42:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-12 01:05:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-12 01:05:48]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[esprehn, 2017-01-19 01:52:06]

Description was changed from

==========
Add WebAuthn bindings and client interface.

This CL adds the WebAuthn bindings and interface to support
WebAuthentication.makeCredential and WebAuthentication.getAssertion.

The draft spec is here: https://w3c.github.io/webauthn/

The interface is likely to change as the spec matures. We're tracking the
comments and suggestions from this review via issues opened with the working
group:
https://github.com/w3c/webauthn/issues/310
https://github.com/w3c/webauthn/issues/311
https://github.com/w3c/webauthn/issues/312
https://github.com/w3c/webauthn/issues/313

This patch also proactively implements the change to WebAuthnAttestation
suggested here: https://github.com/w3c/webauthn/issues/244

BUG=664630
==========

to

==========
Add WebAuthn bindings and client interface.

This CL adds the WebAuthn bindings and interface to support
WebAuthentication.makeCredential and WebAuthentication.getAssertion.

The draft spec is here: https://w3c.github.io/webauthn/

The interface is likely to change as the spec matures. We're tracking the
comments and suggestions from this review via issues opened with the working
group:
https://github.com/w3c/webauthn/issues/310
https://github.com/w3c/webauthn/issues/311
https://github.com/w3c/webauthn/issues/312
https://github.com/w3c/webauthn/issues/313

This patch also proactively implements the change to WebAuthnAttestation
suggested here: https://github.com/w3c/webauthn/issues/244

BUG=664630
==========

[esprehn, 2017-01-19 01:58:58]

esprehn@chromium.org changed reviewers:
+ ikilpatrick@chromium.org

[esprehn, 2017-01-19 01:59:00]

lgtm w/ some nits. For future API review you should get ikilpatrick@ who's the
lead for API ergonomics.

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/OWNERS (right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/OWNERS:1:
kpaulhamus@chromium.org
Lets not add an OWNERS until you're comfortable in the codebase.

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/ScopedCredential.h
(right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/ScopedCredential.h:34:
switch (m_type) {
DCHECK_EQ(m_type, ScopedCredentialType::SCOPEDCRED);
return "ScopedCred";

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webauth/AssertionOptions.idl (right):

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webauth/AssertionOptions.idl:7: dictionary
AssertionOptions {
This is still too generically named btw.

"Assertion" could mean anything in any spec.

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webauth/ScopedCredential.h (right):

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webauth/ScopedCredential.h:33: String type()
const {
If you make this return a StringView you can avoid the allocation inside blink
and the bindings will go directly to v8::String

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webauth/ScopedCredential.h:46: if (type ==
"ScopedCred")
This is wasted work with the return below. Just add a comment?

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webauth/ScopedCredentialDescriptor.idl
(right):

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webauth/ScopedCredentialDescriptor.idl:18:
sequence <Transport> transports;
No space before <

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webauth/ScopedCredentialOptions.idl
(right):

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webauth/ScopedCredentialOptions.idl:10:
sequence <ScopedCredentialDescriptor> excludeList;
ditto

[kpaulhamus, 2017-01-19 23:25:35]

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/OWNERS (right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/OWNERS:1:
kpaulhamus@chromium.org
On 2017/01/19 01:58:59, esprehn wrote:
> Lets not add an OWNERS until you're comfortable in the codebase.

Yeah, I had removed it in a later patch set

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/webauthentication/ScopedCredential.h
(right):

https://codereview.chromium.org/2533863002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/webauthentication/ScopedCredential.h:34:
switch (m_type) {
On 2017/01/19 01:58:59, esprehn wrote:
> DCHECK_EQ(m_type, ScopedCredentialType::SCOPEDCRED);
> return "ScopedCred";

Done.

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webauth/AssertionOptions.idl (right):

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webauth/AssertionOptions.idl:7: dictionary
AssertionOptions {
On 2017/01/19 01:58:59, esprehn wrote:
> This is still too generically named btw.
> 
> "Assertion" could mean anything in any spec.

K, I'll call it AuthenticationAssertionOptions for now and talk with the working
group.

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webauth/ScopedCredential.h (right):

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webauth/ScopedCredential.h:33: String type()
const {
On 2017/01/19 01:58:59, esprehn wrote:
> If you make this return a StringView you can avoid the allocation inside blink
> and the bindings will go directly to v8::String

Leaving as String, since setReturnValueFromString doesn't support this yet.

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webauth/ScopedCredential.h:46: if (type ==
"ScopedCred")
On 2017/01/19 01:58:59, esprehn wrote:
> This is wasted work with the return below. Just add a comment?

Done.

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webauth/ScopedCredentialDescriptor.idl
(right):

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webauth/ScopedCredentialDescriptor.idl:18:
sequence <Transport> transports;
On 2017/01/19 01:59:00, esprehn wrote:
> No space before <

Done.

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webauth/ScopedCredentialOptions.idl
(right):

https://codereview.chromium.org/2533863002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webauth/ScopedCredentialOptions.idl:10:
sequence <ScopedCredentialDescriptor> excludeList;
On 2017/01/19 01:59:00, esprehn wrote:
> ditto

Done.

[kpaulhamus, 2017-01-19 23:27:43]

The CQ bit was checked by kpaulhamus@chromium.org

[kpaulhamus, 2017-01-19 23:27:44]

The patchset sent to the CQ was uploaded after l-g-t-m from esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/2533863002/#ps160001
(title: "Fixing nits and renamed AssertionOptions")

[kpaulhamus, 2017-01-19 23:28:22]

The CQ bit was unchecked by kpaulhamus@chromium.org

[commit-bot: I haz the power, 2017-01-19 23:28:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kpaulhamus, 2017-01-19 23:28:36]

The CQ bit was checked by kpaulhamus@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 23:31:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 23:31:53]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[kpaulhamus, 2017-01-20 00:32:31]

The CQ bit was checked by kpaulhamus@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-20 00:34:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-20 00:37:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-20 00:37:55]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[kpaulhamus, 2017-01-20 00:59:21]

The CQ bit was checked by kpaulhamus@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-20 01:00:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-20 01:04:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-20 01:04:13]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[kpaulhamus, 2017-01-20 01:04:40]

The CQ bit was unchecked by kpaulhamus@chromium.org

[kpaulhamus, 2017-01-20 01:22:10]

The CQ bit was checked by kpaulhamus@chromium.org to run a CQ dry run

[kpaulhamus, 2017-01-20 01:22:44]

The CQ bit was unchecked by kpaulhamus@chromium.org

[commit-bot: I haz the power, 2017-01-20 01:22:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kpaulhamus, 2017-01-20 21:33:20]

The CQ bit was checked by kpaulhamus@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-20 21:33:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-20 23:34:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-20 23:34:44]

Dry run: This issue passed the CQ dry run.

[kpaulhamus, 2017-01-21 01:14:56]

The CQ bit was checked by kpaulhamus@chromium.org

[kpaulhamus, 2017-01-21 01:14:57]

The patchset sent to the CQ was uploaded after l-g-t-m from esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/2533863002/#ps180001
(title: "Layout test expected file tweaks")

[commit-bot: I haz the power, 2017-01-21 01:15:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-21 01:24:28]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1484961296661360,
"parent_rev": "170fe4f55dd622189fdb5ade13301e996149e076", "commit_rev":
"8d43296f5c9c5c7f5f362ea599881fbd28df4898"}

[commit-bot: I haz the power, 2017-01-21 01:25:22]

Description was changed from

==========
Add WebAuthn bindings and client interface.

This CL adds the WebAuthn bindings and interface to support
WebAuthentication.makeCredential and WebAuthentication.getAssertion.

The draft spec is here: https://w3c.github.io/webauthn/

The interface is likely to change as the spec matures. We're tracking the
comments and suggestions from this review via issues opened with the working
group:
https://github.com/w3c/webauthn/issues/310
https://github.com/w3c/webauthn/issues/311
https://github.com/w3c/webauthn/issues/312
https://github.com/w3c/webauthn/issues/313

This patch also proactively implements the change to WebAuthnAttestation
suggested here: https://github.com/w3c/webauthn/issues/244

BUG=664630
==========

to

==========
Add WebAuthn bindings and client interface.

This CL adds the WebAuthn bindings and interface to support
WebAuthentication.makeCredential and WebAuthentication.getAssertion.

The draft spec is here: https://w3c.github.io/webauthn/

The interface is likely to change as the spec matures. We're tracking the
comments and suggestions from this review via issues opened with the working
group:
https://github.com/w3c/webauthn/issues/310
https://github.com/w3c/webauthn/issues/311
https://github.com/w3c/webauthn/issues/312
https://github.com/w3c/webauthn/issues/313

This patch also proactively implements the change to WebAuthnAttestation
suggested here: https://github.com/w3c/webauthn/issues/244

BUG=664630

Review-Url: https://codereview.chromium.org/2533863002
Cr-Commit-Position: refs/heads/master@{#445239}
Committed:
https://chromium.googlesource.com/chromium/src/+/8d43296f5c9c5c7f5f362ea59988...
==========

[commit-bot: I haz the power, 2017-01-21 01:25:24]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/8d43296f5c9c5c7f5f362ea59988...