[fido] Make PublicKeyCredentialDescriptor::credential_type() type-safe

This change makes PublicKeyCredentialDescriptor credential_type to be of
enum type CredentialType instead of std::string, increasing type-safety.

Bug: 780078
Change-Id: I570e698ecb9b13d7cc988eb52293a65f466c8115
Reviewed-on: https://chromium-review.googlesource.com/1041951
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558274}

content/browser/webauth/authenticator_type_converters.cc

@@ -77,9 +77,7 @@ TypeConverter<std::vector<::device::PublicKeyCredentialDescriptor>,
 
   for (const auto& credential : input) {
     credential_descriptors.emplace_back(::device::PublicKeyCredentialDescriptor(
-        device::to_string(
-            ConvertTo<::device::CredentialType>(credential->type)),
-        credential->id));
+        ConvertTo<::device::CredentialType>(credential->type), credential->id));
   }
   return credential_descriptors;
 }

device/fido/authenticator_get_assertion_response.cc

@@ -50,8 +50,8 @@ AuthenticatorGetAssertionResponse::CreateFromU2fSignResponse(
 
   AuthenticatorGetAssertionResponse response(std::move(authenticator_data),
                                              std::move(signature));
-  response.SetCredential(PublicKeyCredentialDescriptor(
-      to_string(CredentialType::kPublicKey), key_handle));
+  response.SetCredential(
+      PublicKeyCredentialDescriptor(CredentialType::kPublicKey, key_handle));
 
   return std::move(response);
 }

device/fido/ctap_request_unittest.cc

@@ -187,7 +187,7 @@ TEST(CTAPRequestTest, TestConstructGetAssertionRequest) {
 
   std::vector<PublicKeyCredentialDescriptor> allowed_list;
   allowed_list.push_back(PublicKeyCredentialDescriptor(
-      "public-key",
+      CredentialType::kPublicKey,
       {0xf2, 0x20, 0x06, 0xde, 0x4f, 0x90, 0x5a, 0xf6, 0x8a, 0x43, 0x94,
        0x2f, 0x02, 0x4f, 0x2a, 0x5e, 0xce, 0x60, 0x3d, 0x9c, 0x6d, 0x4b,
        0x3d, 0xf8, 0xbe, 0x08, 0xed, 0x01, 0xfc, 0x44, 0x26, 0x46, 0xd0,
@@ -195,7 +195,7 @@ TEST(CTAPRequestTest, TestConstructGetAssertionRequest) {
        0x08, 0xd9, 0x4f, 0xcb, 0xee, 0x82, 0xb9, 0xb2, 0xef, 0x66, 0x77,
        0xaf, 0x0a, 0xdc, 0xc3, 0x58, 0x52, 0xea, 0x6b, 0x9e}));
   allowed_list.push_back(PublicKeyCredentialDescriptor(
-      "public-key",
+      CredentialType::kPublicKey,
       {0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03,
        0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03,
        0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03,

device/fido/get_assertion_handler_unittest.cc

@@ -44,7 +44,7 @@ class FidoGetAssertionHandlerTest : public ::testing::Test {
     CtapGetAssertionRequest request_param(
         kRpId, fido_parsing_utils::Materialize(kClientDataHash));
     request_param.SetAllowList(
-        {{to_string(CredentialType::kPublicKey),
+        {{CredentialType::kPublicKey,
           fido_parsing_utils::Materialize(
               test_data::kTestGetAssertionCredentialId)}});
 

device/fido/get_assertion_task_unittest.cc

@@ -59,7 +59,7 @@ TEST_F(FidoGetAssertionTaskTest, TestGetAssertionSuccess) {
 
   CtapGetAssertionRequest request_param(
       kRpId, fido_parsing_utils::Materialize(kClientDataHash));
-  request_param.SetAllowList({{to_string(CredentialType::kPublicKey),
+  request_param.SetAllowList({{CredentialType::kPublicKey,
                                fido_parsing_utils::Materialize(
                                    test_data::kTestGetAssertionCredentialId)}});
 

device/fido/mac/get_assertion_operation.mm

@@ -92,7 +92,7 @@ void GetAssertionOperation::PromptTouchIdDone(bool success, NSError* err) {
   std::set<std::vector<uint8_t>> allowed_credential_ids;
   if (request_.allow_list()) {
     for (const PublicKeyCredentialDescriptor& desc : *request_.allow_list()) {
-      if (desc.credential_type() != "public-key") {
+      if (desc.credential_type() != CredentialType::kPublicKey) {
         continue;
       }
       allowed_credential_ids.insert(desc.id());

device/fido/public_key_credential_descriptor.cc

@@ -26,21 +26,22 @@ PublicKeyCredentialDescriptor::CreateFromCBORValue(
 
   const cbor::CBORValue::MapValue& map = cbor.GetMap();
   auto type = map.find(cbor::CBORValue(kCredentialTypeKey));
-  if (type == map.end() || !type->second.is_string())
+  if (type == map.end() || !type->second.is_string() ||
+      type->second.GetString() != kPublicKey)
     return base::nullopt;
 
   auto id = map.find(cbor::CBORValue(kCredentialIdKey));
   if (id == map.end() || !id->second.is_bytestring())
     return base::nullopt;
 
-  return PublicKeyCredentialDescriptor(type->second.GetString(),
+  return PublicKeyCredentialDescriptor(CredentialType::kPublicKey,
                                        id->second.GetBytestring());
 }
 
 PublicKeyCredentialDescriptor::PublicKeyCredentialDescriptor(
-    std::string credential_type,
+    CredentialType credential_type,
     std::vector<uint8_t> id)
-    : credential_type_(std::move(credential_type)), id_(std::move(id)) {}
+    : credential_type_(credential_type), id_(std::move(id)) {}
 
 PublicKeyCredentialDescriptor::PublicKeyCredentialDescriptor(
     const PublicKeyCredentialDescriptor& other) = default;
@@ -60,7 +61,7 @@ cbor::CBORValue PublicKeyCredentialDescriptor::ConvertToCBOR() const {
   cbor::CBORValue::MapValue cbor_descriptor_map;
   cbor_descriptor_map[cbor::CBORValue(kCredentialIdKey)] = cbor::CBORValue(id_);
   cbor_descriptor_map[cbor::CBORValue(kCredentialTypeKey)] =
-      cbor::CBORValue(credential_type_);
+      cbor::CBORValue(to_string(credential_type_));
   return cbor::CBORValue(std::move(cbor_descriptor_map));
 }
 

device/fido/public_key_credential_descriptor.h

@@ -12,6 +12,7 @@
 #include "base/component_export.h"
 #include "base/optional.h"
 #include "components/cbor/cbor_values.h"
+#include "device/fido/fido_constants.h"
 
 namespace device {
 
@@ -24,7 +25,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) PublicKeyCredentialDescriptor {
   static base::Optional<PublicKeyCredentialDescriptor> CreateFromCBORValue(
       const cbor::CBORValue& cbor);
 
-  PublicKeyCredentialDescriptor(std::string credential_type,
+  PublicKeyCredentialDescriptor(CredentialType credential_type,
                                 std::vector<uint8_t> id);
   PublicKeyCredentialDescriptor(const PublicKeyCredentialDescriptor& other);
   PublicKeyCredentialDescriptor(PublicKeyCredentialDescriptor&& other);
@@ -36,11 +37,11 @@ class COMPONENT_EXPORT(DEVICE_FIDO) PublicKeyCredentialDescriptor {
 
   cbor::CBORValue ConvertToCBOR() const;
 
-  const std::string& credential_type() const { return credential_type_; }
+  CredentialType credential_type() const { return credential_type_; }
   const std::vector<uint8_t>& id() const { return id_; }
 
  private:
-  std::string credential_type_;
+  CredentialType credential_type_;
   std::vector<uint8_t> id_;
 };
 

device/fido/u2f_command_constructor.cc

@@ -47,7 +47,7 @@ base::Optional<std::vector<uint8_t>> ConvertToU2fRegisterCommand(
 base::Optional<std::vector<uint8_t>> ConvertToU2fCheckOnlySignCommand(
     const CtapMakeCredentialRequest& request,
     const PublicKeyCredentialDescriptor& key_handle) {
-  if (key_handle.credential_type() != kPublicKey)
+  if (key_handle.credential_type() != CredentialType::kPublicKey)
     return base::nullopt;
 
   return ConstructU2fSignCommand(

device/fido/u2f_command_constructor_unittest.cc

@@ -119,7 +119,7 @@ TEST(U2fCommandConstructorTest,
      TestConvertCtapMakeCredentialToU2fCheckOnlySign) {
   auto make_credential_param = ConstructMakeCredentialRequest();
   PublicKeyCredentialDescriptor credential_descriptor(
-      kPublicKey,
+      CredentialType::kPublicKey,
       fido_parsing_utils::Materialize(test_data::kU2fSignKeyHandle));
   std::vector<PublicKeyCredentialDescriptor> exclude_list;
   exclude_list.push_back(credential_descriptor);
@@ -139,7 +139,8 @@ TEST(U2fCommandConstructorTest,
      TestConvertCtapMakeCredentialToU2fCheckOnlySignWithInvalidCredentialType) {
   auto make_credential_param = ConstructMakeCredentialRequest();
   PublicKeyCredentialDescriptor credential_descriptor(
-      "UnknownCredentialType",
+      // Purposefully construct an invalid CredentialType.
+      static_cast<CredentialType>(-1),
       fido_parsing_utils::Materialize(test_data::kU2fSignKeyHandle));
   std::vector<PublicKeyCredentialDescriptor> exclude_list;
   exclude_list.push_back(credential_descriptor);
@@ -205,7 +206,7 @@ TEST(U2fCommandConstructorTest, TestConvertCtapGetAssertionToU2fSignRequest) {
   auto get_assertion_req = ConstructGetAssertionRequest();
   std::vector<PublicKeyCredentialDescriptor> allowed_list;
   allowed_list.push_back(PublicKeyCredentialDescriptor(
-      kPublicKey,
+      CredentialType::kPublicKey,
       fido_parsing_utils::Materialize(test_data::kU2fSignKeyHandle)));
   get_assertion_req.SetAllowList(std::move(allowed_list));
 
@@ -228,7 +229,7 @@ TEST(U2fCommandConstructorTest, TestU2fSignUserVerificationRequirement) {
   auto get_assertion_req = ConstructGetAssertionRequest();
   std::vector<PublicKeyCredentialDescriptor> allowed_list;
   allowed_list.push_back(PublicKeyCredentialDescriptor(
-      kPublicKey,
+      CredentialType::kPublicKey,
       fido_parsing_utils::Materialize(test_data::kU2fSignKeyHandle)));
   get_assertion_req.SetAllowList(std::move(allowed_list));
   get_assertion_req.SetUserVerification(UserVerificationRequirement::kRequired);