Allow for absence of CVC when sending PaymentsClient::UploadCardRequests

Bug: 759910
Change-Id: I6ca3a6a168d0abc12d53af350f49144bb990412f
Reviewed-on: https://chromium-review.googlesource.com/742356Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Commit-Queue: Jared Saul <jsaul@google.com>
Cr-Commit-Position: refs/heads/master@{#512965}

components/autofill/core/browser/credit_card_save_manager.cc

@@ -428,7 +428,8 @@ void CreditCardSaveManager::SendUploadCardRequest() {
   upload_request_.app_locale = app_locale_;
   // If the upload request does not have card CVC, populate it with the value
   // provided by the user:
-  if (upload_request_.cvc.empty()) {
+  if (upload_request_.cvc.empty() &&
+      IsAutofillUpstreamRequestCvcIfMissingExperimentEnabled()) {
     DCHECK(client_->GetSaveCardBubbleController());
     upload_request_.cvc =
         client_->GetSaveCardBubbleController()->GetCvcEnteredByUser();

components/autofill/core/browser/payments/payments_client.cc

@@ -53,6 +53,9 @@ const char kUploadCardRequestPath[] =
 const char kUploadCardRequestFormat[] =
     "requestContentType=application/json; charset=utf-8&request=%s"
     "&s7e_1_pan=%s&s7e_13_cvc=%s";
+const char kUploadCardRequestFormatWithoutCvc[] =
+    "requestContentType=application/json; charset=utf-8&request=%s"
+    "&s7e_1_pan=%s";
 
 const char kTokenServiceConsumerId[] = "wallet_client";
 const char kPaymentsOAuth2Scope[] =
@@ -341,7 +344,8 @@ class UploadCardRequest : public PaymentsRequest {
   std::string GetRequestContent() override {
     base::DictionaryValue request_dict;
     request_dict.SetString("encrypted_pan", "__param:s7e_1_pan");
-    request_dict.SetString("encrypted_cvc", "__param:s7e_13_cvc");
+    if (!request_details_.cvc.empty())
+      request_dict.SetString("encrypted_cvc", "__param:s7e_13_cvc");
     request_dict.SetKey("risk_data_encoded",
                         BuildRiskDictionary(request_details_.risk_data));
 
@@ -384,13 +388,21 @@ class UploadCardRequest : public PaymentsRequest {
         AutofillType(CREDIT_CARD_NUMBER), app_locale);
     std::string json_request;
     base::JSONWriter::Write(request_dict, &json_request);
-    std::string request_content = base::StringPrintf(
-        kUploadCardRequestFormat,
-        net::EscapeUrlEncodedData(json_request, true).c_str(),
-        net::EscapeUrlEncodedData(base::UTF16ToASCII(pan), true).c_str(),
-        net::EscapeUrlEncodedData(base::UTF16ToASCII(request_details_.cvc),
-                                  true)
-            .c_str());
+    std::string request_content;
+    if (request_details_.cvc.empty()) {
+      request_content = base::StringPrintf(
+          kUploadCardRequestFormatWithoutCvc,
+          net::EscapeUrlEncodedData(json_request, true).c_str(),
+          net::EscapeUrlEncodedData(base::UTF16ToASCII(pan), true).c_str());
+    } else {
+      request_content = base::StringPrintf(
+          kUploadCardRequestFormat,
+          net::EscapeUrlEncodedData(json_request, true).c_str(),
+          net::EscapeUrlEncodedData(base::UTF16ToASCII(pan), true).c_str(),
+          net::EscapeUrlEncodedData(base::UTF16ToASCII(request_details_.cvc),
+                                    true)
+              .c_str());
+    }
     VLOG(3) << "savecard request body: " << request_content;
     return request_content;
   }

components/autofill/core/browser/payments/payments_client_unittest.cc

@@ -103,13 +103,14 @@ class PaymentsClientTest : public testing::Test,
                               "language-LOCALE");
   }
 
-  void StartUploading() {
+  void StartUploading(bool include_cvc) {
     token_service_->AddAccount("example@gmail.com");
     identity_provider_->LogIn("example@gmail.com");
     PaymentsClient::UploadRequestDetails request_details;
     request_details.billing_customer_number = 111222333444;
     request_details.card = test::GetCreditCard();
-    request_details.cvc = base::ASCIIToUTF16("123");
+    if (include_cvc)
+      request_details.cvc = base::ASCIIToUTF16("123");
     request_details.context_token = base::ASCIIToUTF16("context token");
     request_details.risk_data = "some risk data";
     request_details.app_locale = "language-LOCALE";
@@ -264,7 +265,7 @@ TEST_F(PaymentsClientTest, GetDetailsRemovesNonLocationData) {
 }
 
 TEST_F(PaymentsClientTest, UploadSuccessWithoutServerId) {
-  StartUploading();
+  StartUploading(/*include_cvc=*/true);
   IssueOAuthToken();
   ReturnResponse(net::HTTP_OK, "{}");
   EXPECT_EQ(AutofillClient::SUCCESS, result_);
@@ -272,7 +273,7 @@ TEST_F(PaymentsClientTest, UploadSuccessWithoutServerId) {
 }
 
 TEST_F(PaymentsClientTest, UploadSuccessWithServerId) {
-  StartUploading();
+  StartUploading(/*include_cvc=*/true);
   IssueOAuthToken();
   ReturnResponse(net::HTTP_OK, "{ \"credit_card_id\": \"InstrumentData:1\" }");
   EXPECT_EQ(AutofillClient::SUCCESS, result_);
@@ -280,7 +281,7 @@ TEST_F(PaymentsClientTest, UploadSuccessWithServerId) {
 }
 
 TEST_F(PaymentsClientTest, UploadIncludesNonLocationData) {
-  StartUploading();
+  StartUploading(/*include_cvc=*/true);
 
   // Verify that the recipient name field and test names do appear in the upload
   // data.
@@ -304,7 +305,7 @@ TEST_F(PaymentsClientTest, UploadIncludesNonLocationData) {
 
 TEST_F(PaymentsClientTest,
        UploadRequestIncludesBillingCustomerNumberInRequestIfExperimentOn) {
-  StartUploading();
+  StartUploading(/*include_cvc=*/true);
 
   // Verify that the billing customer number is included in the request.
   EXPECT_TRUE(
@@ -317,13 +318,33 @@ TEST_F(
     UploadRequestDoesNotIncludeBillingCustomerNumberInRequestIfExperimentOff) {
   DisableAutofillSendBillingCustomerNumberExperiment();
 
-  StartUploading();
+  StartUploading(/*include_cvc=*/true);
 
   // Verify that the billing customer number is not included in the request.
   EXPECT_TRUE(GetUploadData().find("external_customer_id") ==
               std::string::npos);
 }
 
+TEST_F(PaymentsClientTest, UploadIncludesCvcInRequestIfProvided) {
+  StartUploading(/*include_cvc=*/true);
+
+  // Verify that the encrypted_cvc and s7e_13_cvc parameters were included in
+  // the request.
+  EXPECT_TRUE(GetUploadData().find("encrypted_cvc") != std::string::npos);
+  EXPECT_TRUE(GetUploadData().find("__param:s7e_13_cvc") != std::string::npos);
+  EXPECT_TRUE(GetUploadData().find("&s7e_13_cvc=") != std::string::npos);
+}
+
+TEST_F(PaymentsClientTest, UploadDoesNotIncludeCvcInRequestIfNotProvided) {
+  StartUploading(/*include_cvc=*/false);
+
+  // Verify that the encrypted_cvc and s7e_13_cvc parameters were not included
+  // in the request.
+  EXPECT_TRUE(GetUploadData().find("encrypted_cvc") == std::string::npos);
+  EXPECT_TRUE(GetUploadData().find("__param:s7e_13_cvc") == std::string::npos);
+  EXPECT_TRUE(GetUploadData().find("&s7e_13_cvc=") == std::string::npos);
+}
+
 TEST_F(PaymentsClientTest, GetDetailsFollowedByUploadSuccess) {
   StartGettingUploadDetails();
   ReturnResponse(
@@ -333,7 +354,7 @@ TEST_F(PaymentsClientTest, GetDetailsFollowedByUploadSuccess) {
 
   result_ = AutofillClient::NONE;
 
-  StartUploading();
+  StartUploading(/*include_cvc=*/true);
   IssueOAuthToken();
   ReturnResponse(net::HTTP_OK, "{}");
   EXPECT_EQ(AutofillClient::SUCCESS, result_);