Bug 1773760 - part 2: make AttestationConveyancePreference a DOMString. r=keeler,webidl,smaug

Depends on D167746

Differential Revision: https://phabricator.services.mozilla.com/D167747

dom/webauthn/AndroidWebAuthnTokenManager.cpp

@@ -143,20 +143,17 @@ RefPtr<U2FRegisterPromise> AndroidWebAuthnTokenManager::Register(
                           java::sdk::Integer::ValueOf(1));
 
           // Get the attestation preference and override if the user asked
-          AttestationConveyancePreference attestation =
-              extra.attestationConveyancePreference();
-
           if (aForceNoneAttestation) {
             // Add UI support to trigger this, bug 1550164
-            attestation = AttestationConveyancePreference::None;
+            GECKOBUNDLE_PUT(authSelBundle, "attestationPreference",
+                            jni::StringParam(u"none"_ns));
+          } else {
+            const nsString& attestation =
+                extra.attestationConveyancePreference();
+            GECKOBUNDLE_PUT(authSelBundle, "attestationPreference",
+                            jni::StringParam(attestation));
           }
 
-          nsString attestPref;
-          attestPref.AssignASCII(
-              AttestationConveyancePreferenceValues::GetString(attestation));
-          GECKOBUNDLE_PUT(authSelBundle, "attestationPreference",
-                          jni::StringParam(attestPref));
-
           const WebAuthnAuthenticatorSelection& sel =
               extra.AuthenticatorSelection();
           if (sel.requireResidentKey()) {

dom/webauthn/PWebAuthnTransaction.ipdl

@@ -16,7 +16,6 @@
 
 include protocol PBackground;
 
-using mozilla::dom::AttestationConveyancePreference from "mozilla/dom/WebAuthnUtil.h";
 using mozilla::dom::MaybeDiscardedBrowsingContext from "mozilla/dom/BrowsingContext.h";
 using mozilla::dom::UserVerificationRequirement from "mozilla/dom/WebAuthnUtil.h";
 
@@ -83,7 +82,7 @@ struct WebAuthnMakeCredentialExtraInfo {
   CoseAlg[] coseAlgs;
   WebAuthnExtension[] Extensions;
   WebAuthnAuthenticatorSelection AuthenticatorSelection;
-  AttestationConveyancePreference attestationConveyancePreference;
+  nsString attestationConveyancePreference;
 };
 
 struct WebAuthnMakeCredentialInfo {

dom/webauthn/U2FTokenManager.cpp

@@ -467,11 +467,15 @@ void U2FTokenManager::Register(
   if (aTransactionInfo.Extra().isSome()) {
     const auto& extra = aTransactionInfo.Extra().ref();
 
-    AttestationConveyancePreference attestation =
-        extra.attestationConveyancePreference();
-
-    noneAttestationRequested =
-        attestation == AttestationConveyancePreference::None;
+    // The default attestation type is "none", so set
+    // noneAttestationRequested=false only if the RP's preference matches one of
+    // the other known types.
+    const nsString& attestation = extra.attestationConveyancePreference();
+    if (attestation.EqualsLiteral("direct") ||
+        attestation.EqualsLiteral("indirect") ||
+        attestation.EqualsLiteral("enterprise")) {
+      noneAttestationRequested = false;
+    }
   }
 #endif  // not MOZ_WIDGET_ANDROID
 

dom/webauthn/WebAuthnManager.cpp

@@ -385,7 +385,7 @@ already_AddRefed<Promise> WebAuthnManager::MakeCredential(
 
   const auto& selection = aOptions.mAuthenticatorSelection;
   const auto& attachment = selection.mAuthenticatorAttachment;
-  const AttestationConveyancePreference& attestation = aOptions.mAttestation;
+  const nsString& attestation = aOptions.mAttestation;
 
   // Attachment
   Maybe<nsString> authenticatorAttachment;

dom/webauthn/WebAuthnUtil.h

@@ -71,13 +71,6 @@ struct ParamTraits<mozilla::dom::UserVerificationRequirement>
           mozilla::dom::UserVerificationRequirement::Required,
           mozilla::dom::UserVerificationRequirement::EndGuard_> {};
 
-template <>
-struct ParamTraits<mozilla::dom::AttestationConveyancePreference>
-    : public ContiguousEnumSerializer<
-          mozilla::dom::AttestationConveyancePreference,
-          mozilla::dom::AttestationConveyancePreference::None,
-          mozilla::dom::AttestationConveyancePreference::EndGuard_> {};
-
 }  // namespace IPC
 
 #endif  // mozilla_dom_WebAuthnUtil_h

dom/webauthn/WinWebAuthnManager.cpp

@@ -266,21 +266,15 @@ void WinWebAuthnManager::Register(
     winRequireResidentKey = sel.requireResidentKey();
 
     // AttestationConveyance
-    AttestationConveyancePreference attestation =
-        extra.attestationConveyancePreference();
-    switch (attestation) {
-      case AttestationConveyancePreference::Direct:
-        winAttestation = WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_DIRECT;
-        break;
-      case AttestationConveyancePreference::Indirect:
-        winAttestation = WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_INDIRECT;
-        break;
-      case AttestationConveyancePreference::None:
-        winAttestation = WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_NONE;
-        break;
-      default:
-        winAttestation = WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_ANY;
-        break;
+    const nsString& attestation = extra.attestationConveyancePreference();
+    if (attestation.EqualsLiteral("none")) {
+      winAttestation = WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_NONE;
+    } else if (attestation.EqualsLiteral("indirect")) {
+      winAttestation = WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_INDIRECT;
+    } else if (attestation.EqualsLiteral("direct")) {
+      winAttestation = WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_DIRECT;
+    } else {
+      winAttestation = WEBAUTHN_ATTESTATION_CONVEYANCE_PREFERENCE_ANY;
     }
 
     if (extra.Extensions().Length() >

dom/webauthn/tests/test_webauthn_attestation_conveyance.html

@@ -83,6 +83,11 @@
         .then(verifyAnonymizedCertificate)
         .catch(arrivingHereIsBad);
 
+      // Request an unknown attestation type. This should be treated as "none".
+      await requestMakeCredential("unknown")
+        .then(verifyAnonymizedCertificate)
+        .catch(arrivingHereIsBad);
+
       // Request no attestation.
       await requestMakeCredential("none")
         .then(verifyAnonymizedCertificate)
@@ -112,14 +117,6 @@
         })
         .catch(arrivingHereIsBad);
     });
-
-    // Test failure cases for make credential.
-    add_task(async function test_make_credential_failures() {
-      // Request a platform authenticator.
-      await requestMakeCredential("unknown")
-        .then(arrivingHereIsBad)
-        .catch(expectTypeError);
-    });
   </script>
 
 </body>

dom/webauthn/tests/test_webauthn_make_credential.html

@@ -69,6 +69,7 @@
     add_task(test_complete_account);
     add_task(test_too_large_user_id);
     add_task(test_excluding_unknown_transports);
+    add_task(test_unknown_attestation_type);
     add_task(test_unknown_authenticator_attachment);
 
     function arrivingHereIsGood(aResult) {
@@ -383,6 +384,16 @@
                   .catch(arrivingHereIsBad);
     }
 
+    async function test_unknown_attestation_type() {
+      let makeCredentialOptions = {
+        rp, user, challenge: gCredentialChallenge, pubKeyCredParams: [param],
+        attestation: "unknown"
+      };
+      return credm.create({publicKey: makeCredentialOptions })
+                  .then(arrivingHereIsGood)
+                  .catch(arrivingHereIsBad);
+    }
+
     async function test_unknown_authenticator_attachment() {
       let makeCredentialOptions = {
         rp, user, challenge: gCredentialChallenge, pubKeyCredParams: [param],

dom/webidl/WebAuthentication.webidl

@@ -60,7 +60,7 @@ dictionary PublicKeyCredentialCreationOptions {
     sequence<PublicKeyCredentialDescriptor>      excludeCredentials = [];
     // FIXME: bug 1493860: should this "= {}" be here?
     AuthenticatorSelectionCriteria               authenticatorSelection = {};
-    AttestationConveyancePreference              attestation = "none";
+    DOMString                                    attestation = "none";
     // FIXME: bug 1493860: should this "= {}" be here?
     AuthenticationExtensionsClientInputs         extensions = {};
 };
@@ -85,12 +85,6 @@ dictionary AuthenticatorSelectionCriteria {
     UserVerificationRequirement  userVerification = "preferred";
 };
 
-enum AttestationConveyancePreference {
-    "none",
-    "indirect",
-    "direct"
-};
-
 enum UserVerificationRequirement {
     "required",
     "preferred",