Closed Bug 1573245 Opened 4 months ago Closed 3 months ago

Web Authentication - Change AuthenticatorTransport to be other than an `enum`

Categories

(Core :: DOM: Web Authentication, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Future
Webcompat Priority ?
Tracking Status
firefox-esr60 --- wontfix
firefox-esr68 70+ fixed
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- fixed
firefox71 --- fixed

People

(Reporter: jcj, Assigned: jcj)

References

()

Details

Attachments

(1 file, 1 obsolete file)

It was a mistake in the WebIDL of WebAuthn to define the PublicKeyCredentialDescriptor as taking a AuthenticatorTransport directly, as that breaks extensibility.

https://searchfox.org/mozilla-central/source/dom/webidl/WebAuthentication.webidl#152

Upstream we're deciding how to fix the spec, but this will need to move to a USVString and the enumeration-checks be done inside WebAuthn's code, rather than having WebIDL abort.

This will be a webcompat issue with Chrome, since they're proceeding with this, even potentially ahead of the spec fix.

Type: defect → enhancement
Webcompat Priority: --- → ?

[Tracking Requested - why for this release]:

For broad compatibility, we'll want this in ESR. The fix is small and easily tested.

Pushed by jjones@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cfdf258c11ae
Change AuthenticatorTransport to be string, not `enum` r=bzbarsky,keeler
Attachment #9093200 - Attachment description: Bug 1573245 - Include BindingUtils for WebAuthnManager on a CLOSED TREE → Bug 1573245 - Include BindingUtils.h for WebAuthnManager on a CLOSED TREE
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0bf953d07a4f
Include BindingUtils.h for WebAuthnManager on a CLOSED TREE

Backed out 2 changesets (bug 1573245) for Windows build bustages at WebAuthnManager.cpp on a CLOSED TREE.

Backout link: https://hg.mozilla.org/integration/autoland/rev/04da00a61c6df437250e3af0d20f8ec72bbe9f48

**Push with failures:**https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&revision=cfdf258c11ae6136fdaa1e0786dba757fb284c48&selectedJob=266977836

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=266977836&repo=autoland&lineNumber=34256

Log snippet:

[task 2019-09-17T05:21:15.319Z] 05:21:15 INFO - mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/dom/reporting'
[task 2019-09-17T05:21:15.319Z] 05:21:15 INFO - mkdir -p '.deps/'
[task 2019-09-17T05:21:15.319Z] 05:21:15 INFO - mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/dom/reporting'
[task 2019-09-17T05:21:15.349Z] 05:21:15 INFO - mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/dom/reporting'
[task 2019-09-17T05:21:15.349Z] 05:21:15 INFO - dom/reporting/Unified_cpp_dom_reporting0.obj
[task 2019-09-17T05:21:15.349Z] 05:21:15 INFO - mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/dom/reporting'
[task 2019-09-17T05:21:15.369Z] 05:21:15 INFO - mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/dom/webauthn'
[task 2019-09-17T05:21:15.369Z] 05:21:15 INFO - z:/build/fetches/sccache/sccache.exe z:/build/fetches/clang/bin/clang.exe --driver-mode=cl -m32 -FoUnified_cpp_dom_webauthn0.obj -c -Iz:/build/build/src/obj-firefox/dist/stl_wrappers -DNDEBUG=1 -DTRIMMED=1 -DWIN32_LEAN_AND_MEAN -D_WIN32 -DWIN32 -D_CRT_RAND_S -DCERT_CHAIN_PARA_HAS_EXTRA_FIELDS -DOS_WIN=1 -D_UNICODE -DCHROMIUM_BUILD -DU_STATIC_IMPLEMENTATION -DUNICODE -D_WINDOWS -D_SECURE_ATL -DCOMPILER_MSVC -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -Iz:/build/build/src/dom/webauthn -Iz:/build/build/src/obj-firefox/dom/webauthn -Iz:/build/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -Iz:/build/build/src/ipc/chromium/src -Iz:/build/build/src/ipc/glue -Iz:/build/build/src/dom/base -Iz:/build/build/src/dom/crypto -Iz:/build/build/src/security/manager/ssl -Iz:/build/build/src/third_party/rust -Iz:/build/build/src/obj-firefox/dist/include -Iz:/build/build/src/obj-firefox/dist/include/nspr -Iz:/build/build/src/obj-firefox/dist/include/nss -MD -FI z:/build/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -Qunused-arguments -guard:cf -Qunused-arguments -fcrash-diagnostics-dir=z:/build/public/build -TP -nologo -Zc:sizedDealloc- -D_HAS_EXCEPTIONS=0 -guard:cf -W3 -Gy -Zc:inline -arch:SSE2 -Gw -Wno-inline-new-delete -Wno-invalid-offsetof -Wno-microsoft-enum-value -Wno-microsoft-include -Wno-unknown-pragmas -Wno-ignored-pragmas -Wno-deprecated-declarations -Wno-invalid-noreturn -Wno-inconsistent-missing-override -Wno-implicit-exception-spec-mismatch -Wno-unused-local-typedef -Wno-ignored-attributes -Wno-used-but-marked-unused -D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING -GR- -Z7 -Xclang -load -Xclang z:/build/build/src/obj-firefox/build/clang-plugin/clang-plugin.dll -Xclang -add-plugin -Xclang moz-check -O2 -Oy- -WX -Xclang -MP -Xclang -dependency-file -Xclang .deps/Unified_cpp_dom_webauthn0.obj.pp -Xclang -MT -Xclang Unified_cpp_dom_webauthn0.obj Unified_cpp_dom_webauthn0.cpp
[task 2019-09-17T05:21:15.369Z] 05:21:15 INFO - In file included from Unified_cpp_dom_webauthn0.cpp:74:
[task 2019-09-17T05:21:15.370Z] 05:21:15 INFO - z:/build/build/src/dom/webauthn/WebAuthnManager.cpp(539,19): error: no matching function for call to 'FindEnumStringIndexImpl'
[task 2019-09-17T05:21:15.370Z] 05:21:15 INFO - int i = FindEnumStringIndexImpl(
[task 2019-09-17T05:21:15.370Z] 05:21:15 INFO - ^~~~~~~~~~~~~~~~~~~~~~~
[task 2019-09-17T05:21:15.370Z] 05:21:15 INFO - z:/build/build/src/obj-firefox/dist/include\mozilla/dom/BindingUtils.h(1285,12): note: candidate template ignored: could not match 'const CharT ' against 'typename raw_type<char16_t, int>::type' (aka 'char16ptr_t')
[task 2019-09-17T05:21:15.370Z] 05:21:15 INFO - inline int FindEnumStringIndexImpl(const CharT
chars, size_t length,
[task 2019-09-17T05:21:15.370Z] 05:21:15 INFO - ^
[task 2019-09-17T05:21:15.370Z] 05:21:15 INFO - 1 error generated.
[task 2019-09-17T05:21:15.370Z] 05:21:15 INFO - z:/build/build/src/config/rules.mk:785: recipe for target 'Unified_cpp_dom_webauthn0.obj' failed
[task 2019-09-17T05:21:15.371Z] 05:21:15 INFO - mozmake.EXE[4]: *** [Unified_cpp_dom_webauthn0.obj] Error 1
[task 2019-09-17T05:21:15.371Z] 05:21:15 INFO - mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/dom/webauthn'
[task 2019-09-17T05:21:15.371Z] 05:21:15 INFO - z:/build/build/src/config/recurse.mk:74: recipe for target 'dom/webauthn/target-objects' failed
[task 2019-09-17T05:21:15.371Z] 05:21:15 INFO - mozmake.EXE[3]: *** [dom/webauthn/target-objects] Error 2
[task 2019-09-17T05:21:15.371Z] 05:21:15 INFO - mozmake.EXE[3]: *** Waiting for unfinished jobs....
[task 2019-09-17T05:21:16.587Z] 05:21:16 INFO - mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/dom/xul'
[task 2019-09-17T05:21:16.587Z] 05:21:16 INFO - z:/build/fetches/sccache/sccache.exe z:/build/fetches/clang/bin/clang.exe --driver-mode=cl -m32 -FoUnified_cpp_dom_xul0.obj -c -Iz:/build/build/src/obj-firefox/dist/stl_wrappers -DNDEBUG=1 -DTRIMMED=1 -DMOZ_BREAK_XUL_OVERLAYS -DWIN32_LEAN_AND_MEAN -D_WIN32 -DWIN32 -D_CRT_RAND_S -DCERT_CHAIN_PARA_HAS_EXTRA_FIELDS -DOS_WIN=1 -D_UNICODE -DCHROMIUM_BUILD -DU_STATIC_IMPLEMENTATION -DUNICODE -D_WINDOWS -D_SECURE_ATL -DCOMPILER_MSVC -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -Iz:/build/build/src/dom/xul -Iz:/build/build/src/obj-firefox/dom/xul -Iz:/build/build/src/docshell/base -Iz:/build/build/src/dom/base -Iz:/build/build/src/dom/html -Iz:/build/build/src/dom/xbl -Iz:/build/build/src/dom/xml -Iz:/build/build/src/layout/base -Iz:/build/build/src/layout/generic -Iz:/build/build/src/layout/style -Iz:/build/build/src/layout/xul -Iz:/build/build/src/layout/xul/tree -Iz:/build/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -Iz:/build/build/src/ipc/chromium/src -Iz:/build/build/src/ipc/glue -Iz:/build/build/src/obj-firefox/dist/include -Iz:/build/build/src/obj-firefox/dist/include/nspr -Iz:/build/build/src/obj-firefox/dist/include/nss -MD -FI z:/build/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -Qunused-arguments -guard:cf -Qunused-arguments -fcrash-diagnostics-dir=z:/build/public/build -TP -nologo -Zc:sizedDealloc- -D_HAS_EXCEPTIONS=0 -guard:cf -W3 -Gy -Zc:inline -arch:SSE2 -Gw -Wno-inline-new-delete -Wno-invalid-offsetof -Wno-microsoft-enum-value -Wno-microsoft-include -Wno-unknown-pragmas -Wno-ignored-pragmas -Wno-deprecated-declarations -Wno-invalid-noreturn -Wno-inconsistent-missing-override -Wno-implicit-exception-spec-mismatch -Wno-unused-local-typedef -Wno-ignored-attributes -Wno-used-but-marked-unused -D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING -GR- -Z7 -Xclang -load -Xclang z:/build/build/src/obj-firefox/build/clang-plugin/clang-plugin.dll -Xclang -add-plugin -Xclang moz-check -O2 -Oy- -WX -Xclang -MP -Xclang -dependency-file -Xclang .deps/Unified_cpp_dom_xul0.obj.pp -Xclang -MT -Xclang Unified_cpp_dom_xul0.obj Unified_cpp_dom_xul0.cpp
[task 2019-09-17T05:21:16.587Z] 05:21:16 INFO - mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/dom/xul'

Flags: needinfo?(jjones)
Attachment #9093200 - Attachment is obsolete: true
Pushed by jjones@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/069df0a1d93c
Change AuthenticatorTransport to be string, not `enum` r=bzbarsky,keeler
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Flags: needinfo?(jjones)

Please nominate this for Beta and ESR68 approval when you get a chance.

Flags: needinfo?(jjones)
Flags: in-testsuite+

Comment on attachment 9084823 [details]
Bug 1573245 - Change AuthenticatorTransport to be string, not enum

Beta/Release Uplift Approval Request

  • User impact if declined: Decreased webcompat.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This moves the enumeration decoding out of WebIDL and into application code so that an error isn't fatal. The comparison loop is standard, and it's covered by automated tests.
  • String changes made/needed: None

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Google Chrome made a breaking change for this a year in advance of the spec movement, and released a Google Accounts feature in July that makes use of that. As-is, Firefox has to be special-cased to not run into this TypeError. It'd be good to avoid the compliance issue if it happens elsewhere for the lifetime of ESR68.
  • User impact if declined: Potentially more webcompat problems with WebAuthn
  • Fix Landed on Version: 71
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This moves the enumeration decoding out of WebIDL and into application code so that an error isn't fatal. The comparison loop is standard, and it's covered by automated tests.
  • String or UUID changes made by this patch: None
Flags: needinfo?(jjones)
Attachment #9084823 - Flags: approval-mozilla-esr68?
Attachment #9084823 - Flags: approval-mozilla-beta?

Comment on attachment 9084823 [details]
Bug 1573245 - Change AuthenticatorTransport to be string, not enum

Compat fix needed for Web Authentication. Approved for 70.0b8 and 68.2esr.

Attachment #9084823 - Flags: approval-mozilla-esr68?
Attachment #9084823 - Flags: approval-mozilla-esr68+
Attachment #9084823 - Flags: approval-mozilla-beta?
Attachment #9084823 - Flags: approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.