Commit db4d6dca authored by Manas Verma's avatar Manas Verma Committed by Commit Bot

[Autofill Auth] Autofill FIDO Auth should retrieve dcvv for cloud tokens.

Bug: 949269
Change-Id: I914b20bc5532a4df59120f6954235b1a5daa9013
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1977185Reviewed-by: default avatarJared Saul <jsaul@google.com>
Commit-Queue: Manas Verma <manasverma@google.com>
Cr-Commit-Position: refs/heads/master@{#726601}
parent 23970ca4
...@@ -497,7 +497,8 @@ void CreditCardAccessManager::OnCVCAuthenticationComplete( ...@@ -497,7 +497,8 @@ void CreditCardAccessManager::OnCVCAuthenticationComplete(
#if !defined(OS_IOS) #if !defined(OS_IOS)
void CreditCardAccessManager::OnFIDOAuthenticationComplete( void CreditCardAccessManager::OnFIDOAuthenticationComplete(
bool did_succeed, bool did_succeed,
const CreditCard* card) { const CreditCard* card,
const base::string16& cvc) {
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
// Close the Webauthn verify pending dialog. If FIDO authentication succeeded, // Close the Webauthn verify pending dialog. If FIDO authentication succeeded,
// card is filled to the form, otherwise fall back to CVC authentication which // card is filled to the form, otherwise fall back to CVC authentication which
...@@ -507,7 +508,7 @@ void CreditCardAccessManager::OnFIDOAuthenticationComplete( ...@@ -507,7 +508,7 @@ void CreditCardAccessManager::OnFIDOAuthenticationComplete(
if (did_succeed) { if (did_succeed) {
is_authentication_in_progress_ = false; is_authentication_in_progress_ = false;
accessor_->OnCreditCardFetched(did_succeed, card); accessor_->OnCreditCardFetched(did_succeed, card, cvc);
can_fetch_unmask_details_.Signal(); can_fetch_unmask_details_.Signal();
} else { } else {
// Fall back to CVC if WebAuthn failed. // Fall back to CVC if WebAuthn failed.
......
...@@ -146,8 +146,10 @@ class CreditCardAccessManager : public CreditCardCVCAuthenticator::Requester, ...@@ -146,8 +146,10 @@ class CreditCardAccessManager : public CreditCardCVCAuthenticator::Requester,
#if !defined(OS_IOS) #if !defined(OS_IOS)
// CreditCardFIDOAuthenticator::Requester: // CreditCardFIDOAuthenticator::Requester:
void OnFIDOAuthenticationComplete(bool did_succeed, void OnFIDOAuthenticationComplete(
const CreditCard* card = nullptr) override; bool did_succeed,
const CreditCard* card = nullptr,
const base::string16& cvc = base::string16()) override;
void OnFidoAuthorizationComplete(bool did_succeed) override; void OnFidoAuthorizationComplete(bool did_succeed) override;
#endif #endif
......
...@@ -80,6 +80,7 @@ namespace { ...@@ -80,6 +80,7 @@ namespace {
const char kTestGUID[] = "00000000-0000-0000-0000-000000000001"; const char kTestGUID[] = "00000000-0000-0000-0000-000000000001";
const char kTestNumber[] = "4234567890123456"; // Visa const char kTestNumber[] = "4234567890123456"; // Visa
const char kTestCvc[] = "123";
#if !defined(OS_IOS) #if !defined(OS_IOS)
// Base64 encoding of "This is a test challenge". // Base64 encoding of "This is a test challenge".
...@@ -110,10 +111,12 @@ class TestAccessor : public CreditCardAccessManager::Accessor { ...@@ -110,10 +111,12 @@ class TestAccessor : public CreditCardAccessManager::Accessor {
if (did_succeed_) { if (did_succeed_) {
DCHECK(card); DCHECK(card);
number_ = card->number(); number_ = card->number();
cvc_ = cvc;
} }
} }
base::string16 number() { return number_; } base::string16 number() { return number_; }
base::string16 cvc() { return cvc_; }
bool did_succeed() { return did_succeed_; } bool did_succeed() { return did_succeed_; }
...@@ -122,6 +125,8 @@ class TestAccessor : public CreditCardAccessManager::Accessor { ...@@ -122,6 +125,8 @@ class TestAccessor : public CreditCardAccessManager::Accessor {
bool did_succeed_ = false; bool did_succeed_ = false;
// The card number returned from OnCreditCardFetched(). // The card number returned from OnCreditCardFetched().
base::string16 number_; base::string16 number_;
// The returned CVC, if any.
base::string16 cvc_;
base::WeakPtrFactory<TestAccessor> weak_ptr_factory_{this}; base::WeakPtrFactory<TestAccessor> weak_ptr_factory_{this};
}; };
...@@ -238,7 +243,7 @@ class CreditCardAccessManagerTest : public testing::Test { ...@@ -238,7 +243,7 @@ class CreditCardAccessManagerTest : public testing::Test {
// Mock user response. // Mock user response.
payments::FullCardRequest::UserProvidedUnmaskDetails details; payments::FullCardRequest::UserProvidedUnmaskDetails details;
details.cvc = base::ASCIIToUTF16("123"); details.cvc = base::ASCIIToUTF16(kTestCvc);
full_card_request->OnUnmaskPromptAccepted(details); full_card_request->OnUnmaskPromptAccepted(details);
payments::PaymentsClient::UnmaskResponseDetails response; payments::PaymentsClient::UnmaskResponseDetails response;
...@@ -301,7 +306,8 @@ class CreditCardAccessManagerTest : public testing::Test { ...@@ -301,7 +306,8 @@ class CreditCardAccessManagerTest : public testing::Test {
// Returns true if full card request was sent from FIDO auth. // Returns true if full card request was sent from FIDO auth.
bool GetRealPanForFIDOAuth(AutofillClient::PaymentsRpcResult result, bool GetRealPanForFIDOAuth(AutofillClient::PaymentsRpcResult result,
const std::string& real_pan) { const std::string& real_pan,
const std::string& dcvv = std::string()) {
payments::FullCardRequest* full_card_request = payments::FullCardRequest* full_card_request =
GetFIDOAuthenticator()->full_card_request_.get(); GetFIDOAuthenticator()->full_card_request_.get();
...@@ -309,8 +315,8 @@ class CreditCardAccessManagerTest : public testing::Test { ...@@ -309,8 +315,8 @@ class CreditCardAccessManagerTest : public testing::Test {
return false; return false;
payments::PaymentsClient::UnmaskResponseDetails response; payments::PaymentsClient::UnmaskResponseDetails response;
full_card_request->OnDidGetRealPan(result, full_card_request->OnDidGetRealPan(
response.with_real_pan(real_pan)); result, response.with_real_pan(real_pan).with_dcvv(dcvv));
return true; return true;
} }
...@@ -471,6 +477,7 @@ TEST_F(CreditCardAccessManagerTest, FetchServerCardCVCSuccess) { ...@@ -471,6 +477,7 @@ TEST_F(CreditCardAccessManagerTest, FetchServerCardCVCSuccess) {
EXPECT_TRUE(GetRealPanForCVCAuth(AutofillClient::SUCCESS, kTestNumber)); EXPECT_TRUE(GetRealPanForCVCAuth(AutofillClient::SUCCESS, kTestNumber));
EXPECT_TRUE(accessor_->did_succeed()); EXPECT_TRUE(accessor_->did_succeed());
EXPECT_EQ(ASCIIToUTF16(kTestNumber), accessor_->number()); EXPECT_EQ(ASCIIToUTF16(kTestNumber), accessor_->number());
EXPECT_EQ(ASCIIToUTF16(kTestCvc), accessor_->cvc());
} }
// Ensures that FetchCreditCard() returns a failure upon a negative response // Ensures that FetchCreditCard() returns a failure upon a negative response
...@@ -519,6 +526,7 @@ TEST_F(CreditCardAccessManagerTest, FetchServerCardCVCTryAgainFailure) { ...@@ -519,6 +526,7 @@ TEST_F(CreditCardAccessManagerTest, FetchServerCardCVCTryAgainFailure) {
EXPECT_TRUE(GetRealPanForCVCAuth(AutofillClient::SUCCESS, kTestNumber)); EXPECT_TRUE(GetRealPanForCVCAuth(AutofillClient::SUCCESS, kTestNumber));
EXPECT_TRUE(accessor_->did_succeed()); EXPECT_TRUE(accessor_->did_succeed());
EXPECT_EQ(ASCIIToUTF16(kTestNumber), accessor_->number()); EXPECT_EQ(ASCIIToUTF16(kTestNumber), accessor_->number());
EXPECT_EQ(ASCIIToUTF16(kTestCvc), accessor_->cvc());
} }
// Ensures that CardUnmaskPreflightCalled metrics are logged correctly. // Ensures that CardUnmaskPreflightCalled metrics are logged correctly.
...@@ -639,6 +647,45 @@ TEST_F(CreditCardAccessManagerTest, FetchServerCardFIDOSuccess) { ...@@ -639,6 +647,45 @@ TEST_F(CreditCardAccessManagerTest, FetchServerCardFIDOSuccess) {
"Autofill.BetterAuth.CardUnmaskDuration.Fido.Success", 1); "Autofill.BetterAuth.CardUnmaskDuration.Fido.Success", 1);
} }
// Ensures that FetchCreditCard() returns the full PAN upon a successful
// WebAuthn verification and response from payments.
TEST_F(CreditCardAccessManagerTest, FetchServerCardFIDOSuccessWithDcvv) {
// Enable both features and opt user in for FIDO auth.
scoped_feature_list_.Reset();
scoped_feature_list_.InitWithFeatures(
{features::kAutofillCreditCardAuthentication,
features::kAutofillAlwaysReturnCloudTokenizedCard},
{});
::autofill::prefs::SetCreditCardFIDOAuthEnabled(autofill_client_.GetPrefs(),
true);
// General setup.
CreateServerCard(kTestGUID, kTestNumber);
CreditCard* card = credit_card_access_manager_->GetCreditCard(kTestGUID);
GetFIDOAuthenticator()->SetUserVerifiable(true);
payments_client_->AddFidoEligibleCard(card->server_id(), kCredentialId,
kGooglePaymentsRpid);
credit_card_access_manager_->PrepareToFetchCreditCard();
WaitForCallbacks();
credit_card_access_manager_->FetchCreditCard(card, accessor_->GetWeakPtr());
WaitForCallbacks();
// FIDO Success.
TestCreditCardFIDOAuthenticator::GetAssertion(GetFIDOAuthenticator(),
/*did_succeed=*/true);
// Mock Payments response that includes DCVV along with Full PAN.
EXPECT_TRUE(
GetRealPanForFIDOAuth(AutofillClient::SUCCESS, kTestNumber, kTestCvc));
// Expect accessor to successfully retrieve the DCVV.
EXPECT_TRUE(accessor_->did_succeed());
EXPECT_EQ(ASCIIToUTF16(kTestNumber), accessor_->number());
EXPECT_EQ(ASCIIToUTF16(kTestCvc), accessor_->cvc());
}
// Ensures that CVC prompt is invoked after WebAuthn fails. // Ensures that CVC prompt is invoked after WebAuthn fails.
TEST_F(CreditCardAccessManagerTest, TEST_F(CreditCardAccessManagerTest,
FetchServerCardFIDOVerificationFailureCVCFallback) { FetchServerCardFIDOVerificationFailureCVCFallback) {
...@@ -673,6 +720,7 @@ TEST_F(CreditCardAccessManagerTest, ...@@ -673,6 +720,7 @@ TEST_F(CreditCardAccessManagerTest,
EXPECT_TRUE(GetRealPanForCVCAuth(AutofillClient::SUCCESS, kTestNumber)); EXPECT_TRUE(GetRealPanForCVCAuth(AutofillClient::SUCCESS, kTestNumber));
EXPECT_TRUE(accessor_->did_succeed()); EXPECT_TRUE(accessor_->did_succeed());
EXPECT_EQ(ASCIIToUTF16(kTestNumber), accessor_->number()); EXPECT_EQ(ASCIIToUTF16(kTestNumber), accessor_->number());
EXPECT_EQ(ASCIIToUTF16(kTestCvc), accessor_->cvc());
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
histogram_name, AutofillMetrics::WebauthnResultMetric::kNotAllowedError, histogram_name, AutofillMetrics::WebauthnResultMetric::kNotAllowedError,
...@@ -715,6 +763,7 @@ TEST_F(CreditCardAccessManagerTest, ...@@ -715,6 +763,7 @@ TEST_F(CreditCardAccessManagerTest,
EXPECT_TRUE(GetRealPanForCVCAuth(AutofillClient::SUCCESS, kTestNumber)); EXPECT_TRUE(GetRealPanForCVCAuth(AutofillClient::SUCCESS, kTestNumber));
EXPECT_TRUE(accessor_->did_succeed()); EXPECT_TRUE(accessor_->did_succeed());
EXPECT_EQ(ASCIIToUTF16(kTestNumber), accessor_->number()); EXPECT_EQ(ASCIIToUTF16(kTestNumber), accessor_->number());
EXPECT_EQ(ASCIIToUTF16(kTestCvc), accessor_->cvc());
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
histogram_name, AutofillMetrics::WebauthnResultMetric::kSuccess, 1); histogram_name, AutofillMetrics::WebauthnResultMetric::kSuccess, 1);
...@@ -750,6 +799,7 @@ TEST_F(CreditCardAccessManagerTest, ...@@ -750,6 +799,7 @@ TEST_F(CreditCardAccessManagerTest,
EXPECT_TRUE(GetRealPanForCVCAuth(AutofillClient::SUCCESS, kTestNumber)); EXPECT_TRUE(GetRealPanForCVCAuth(AutofillClient::SUCCESS, kTestNumber));
EXPECT_TRUE(accessor_->did_succeed()); EXPECT_TRUE(accessor_->did_succeed());
EXPECT_EQ(ASCIIToUTF16(kTestNumber), accessor_->number()); EXPECT_EQ(ASCIIToUTF16(kTestNumber), accessor_->number());
EXPECT_EQ(ASCIIToUTF16(kTestCvc), accessor_->cvc());
} }
// Ensures that CVC prompt is invoked when the pre-flight call to Google // Ensures that CVC prompt is invoked when the pre-flight call to Google
...@@ -767,6 +817,7 @@ TEST_F(CreditCardAccessManagerTest, FetchServerCardFIDOTimeoutCVCFallback) { ...@@ -767,6 +817,7 @@ TEST_F(CreditCardAccessManagerTest, FetchServerCardFIDOTimeoutCVCFallback) {
EXPECT_TRUE(GetRealPanForCVCAuth(AutofillClient::SUCCESS, kTestNumber)); EXPECT_TRUE(GetRealPanForCVCAuth(AutofillClient::SUCCESS, kTestNumber));
EXPECT_TRUE(accessor_->did_succeed()); EXPECT_TRUE(accessor_->did_succeed());
EXPECT_EQ(ASCIIToUTF16(kTestNumber), accessor_->number()); EXPECT_EQ(ASCIIToUTF16(kTestNumber), accessor_->number());
EXPECT_EQ(ASCIIToUTF16(kTestCvc), accessor_->cvc());
} }
// Ensures that FetchCreditCard() returns the full PAN upon a successful // Ensures that FetchCreditCard() returns the full PAN upon a successful
...@@ -914,6 +965,7 @@ TEST_F(CreditCardAccessManagerTest, FetchExpiredServerCardInvokesCvcPrompt) { ...@@ -914,6 +965,7 @@ TEST_F(CreditCardAccessManagerTest, FetchExpiredServerCardInvokesCvcPrompt) {
// Expect CVC prompt to be invoked. // Expect CVC prompt to be invoked.
EXPECT_TRUE(GetRealPanForCVCAuth(AutofillClient::SUCCESS, kTestNumber)); EXPECT_TRUE(GetRealPanForCVCAuth(AutofillClient::SUCCESS, kTestNumber));
EXPECT_EQ(ASCIIToUTF16(kTestNumber), accessor_->number()); EXPECT_EQ(ASCIIToUTF16(kTestNumber), accessor_->number());
EXPECT_EQ(ASCIIToUTF16(kTestCvc), accessor_->cvc());
} }
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
......
...@@ -473,7 +473,7 @@ void CreditCardFIDOAuthenticator::OnFullCardRequestSucceeded( ...@@ -473,7 +473,7 @@ void CreditCardFIDOAuthenticator::OnFullCardRequestSucceeded(
const base::string16& cvc) { const base::string16& cvc) {
DCHECK_EQ(AUTHENTICATION_FLOW, current_flow_); DCHECK_EQ(AUTHENTICATION_FLOW, current_flow_);
current_flow_ = NONE_FLOW; current_flow_ = NONE_FLOW;
requester_->OnFIDOAuthenticationComplete(/*did_succeed=*/true, &card); requester_->OnFIDOAuthenticationComplete(/*did_succeed=*/true, &card, cvc);
} }
void CreditCardFIDOAuthenticator::OnFullCardRequestFailed() { void CreditCardFIDOAuthenticator::OnFullCardRequestFailed() {
......
...@@ -69,7 +69,8 @@ class CreditCardFIDOAuthenticator ...@@ -69,7 +69,8 @@ class CreditCardFIDOAuthenticator
virtual ~Requester() {} virtual ~Requester() {}
virtual void OnFIDOAuthenticationComplete( virtual void OnFIDOAuthenticationComplete(
bool did_succeed, bool did_succeed,
const CreditCard* card = nullptr) = 0; const CreditCard* card = nullptr,
const base::string16& cvc = base::string16()) = 0;
virtual void OnFidoAuthorizationComplete(bool did_succeed) = 0; virtual void OnFidoAuthorizationComplete(bool did_succeed) = 0;
}; };
CreditCardFIDOAuthenticator(AutofillDriver* driver, AutofillClient* client); CreditCardFIDOAuthenticator(AutofillDriver* driver, AutofillClient* client);
......
...@@ -30,7 +30,8 @@ void TestAuthenticationRequester::OnCVCAuthenticationComplete( ...@@ -30,7 +30,8 @@ void TestAuthenticationRequester::OnCVCAuthenticationComplete(
#if !defined(OS_IOS) #if !defined(OS_IOS)
void TestAuthenticationRequester::OnFIDOAuthenticationComplete( void TestAuthenticationRequester::OnFIDOAuthenticationComplete(
bool did_succeed, bool did_succeed,
const CreditCard* card) { const CreditCard* card,
const base::string16& cvc) {
did_succeed_ = did_succeed; did_succeed_ = did_succeed;
if (did_succeed_) { if (did_succeed_) {
DCHECK(card); DCHECK(card);
......
...@@ -42,8 +42,10 @@ class TestAuthenticationRequester ...@@ -42,8 +42,10 @@ class TestAuthenticationRequester
#if !defined(OS_IOS) #if !defined(OS_IOS)
// CreditCardFIDOAuthenticator::Requester: // CreditCardFIDOAuthenticator::Requester:
void OnFIDOAuthenticationComplete(bool did_succeed, void OnFIDOAuthenticationComplete(
const CreditCard* card = nullptr) override; bool did_succeed,
const CreditCard* card = nullptr,
const base::string16& cvc = base::string16()) override;
void OnFidoAuthorizationComplete(bool did_succeed) override; void OnFidoAuthorizationComplete(bool did_succeed) override;
void IsUserVerifiableCallback(bool is_user_verifiable); void IsUserVerifiableCallback(bool is_user_verifiable);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment