Closed Bug 1330138 Opened 3 years ago Closed 3 years ago

Divide U2F and WebAuthn APIs into their own directories

Categories

(Core :: DOM: Device Interfaces, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: qdot, Assigned: qdot)

References

Details

Attachments

(3 files)

While they achieve the same goal, the U2F and WebAuthn APIs don't really share much code at the moment. We should have them in different directories so that it is obvious they are separate APIs.
Comment on attachment 8825970 [details]
Bug 1330138 - Divide U2F and WebAuthn into separate directories;

https://reviewboard.mozilla.org/r/104018/#review104760

A couple of nits, but I confirm this rename is good and didn't touch anything substantive.

::: dom/webauthn/WebAuthentication.cpp:22
(Diff revision 1)
> +#define PREF_U2F_USBTOKEN_ENABLED  "security.webauth.u2f_enable_usbtoken"
> +
> +namespace mozilla {
> +namespace dom {
> +
> +static mozilla::LazyLogModule gWebauthLog("awebauthn");

Nit: Out of curiosity, why rename the log module to `awebauthn` from `webauth_u2f`? (It's the `a` prefix that gets me)

::: dom/webauthn/tests/mochitest.ini:2
(Diff revision 1)
> +[DEFAULT]
> +# Feature does not function on e10s (Disabled in Bug 1297552)

nit: technically untrue in this file. but we can remove this incorrect comment later.
Attachment #8825970 - Flags: review?(jjones) → review+
Comment on attachment 8825972 [details]
Bug 1330138 - Remove unused USBToken class from U2F API;

https://reviewboard.mozilla.org/r/104022/#review104762
Attachment #8825972 - Flags: review?(jjones) → review+
Comment on attachment 8825971 [details]
Bug 1330138 - Differentiate prefs for u2f and webauthn apis

https://reviewboard.mozilla.org/r/104020/#review104764

Seems good to me.
Attachment #8825971 - Flags: review?(jjones) → review+
(In reply to J.C. Jones [:jcj] from comment #4)
> ::: dom/webauthn/WebAuthentication.cpp:22
> (Diff revision 1)
> > +#define PREF_U2F_USBTOKEN_ENABLED  "security.webauth.u2f_enable_usbtoken"
> > +
> > +namespace mozilla {
> > +namespace dom {
> > +
> > +static mozilla::LazyLogModule gWebauthLog("awebauthn");
> 
> Nit: Out of curiosity, why rename the log module to `awebauthn` from
> `webauth_u2f`? (It's the `a` prefix that gets me)

That was a typo. Thanks for catching that. :)

> ::: dom/webauthn/tests/mochitest.ini:2
> (Diff revision 1)
> > +[DEFAULT]
> > +# Feature does not function on e10s (Disabled in Bug 1297552)
> 
> nit: technically untrue in this file. but we can remove this incorrect
> comment later.

I took it out now. Also, looking at the U2F stuff, it'll probably be able to use the same IPC protocol as WebAuthn with minimal changes, so it may start working in non-e10s also.
Comment on attachment 8825971 [details]
Bug 1330138 - Differentiate prefs for u2f and webauthn apis

https://reviewboard.mozilla.org/r/104020/#review104900
Attachment #8825971 - Flags: review?(amarchesini) → review+
Pushed by kmachulis@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f3cdeb82990
Divide U2F and WebAuthn into separate directories; r=jcj
https://hg.mozilla.org/integration/mozilla-inbound/rev/c582598b2065
Differentiate prefs for u2f and webauthn apis; r=jcj r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8d2fc52fdee
Remove unused USBToken class from U2F API; r=jcj
You need to log in before you can comment on or make changes to this bug.