Divide U2F and WebAuthn APIs into their own directories

RESOLVED FIXED in Firefox 53

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: qdot, Assigned: qdot)

Tracking

Trunk
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(3 attachments)

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 hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 4

2 years ago
mozreview-review
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 5

2 years ago
mozreview-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 6

2 years ago
mozreview-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+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(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 11

2 years ago
mozreview-review
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+

Comment 12

2 years ago
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

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9f3cdeb82990
https://hg.mozilla.org/mozilla-central/rev/c582598b2065
https://hg.mozilla.org/mozilla-central/rev/a8d2fc52fdee
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.