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

[Autofill Auth Refactor] Wrapping all base::Value with base::Optional

This will allow to replace ".is_dict()" or ".is_none()" checks to ".has_value()"
checks.

Bug: 949269
Change-Id: I5cfd37515cdbed4f765b80bfc213877a2458d0db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1972948
Commit-Queue: Jared Saul <jsaul@google.com>
Reviewed-by: default avatarJared Saul <jsaul@google.com>
Reviewed-by: default avatarSiyu An <siyua@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726989}
parent d8f26b3e
...@@ -322,7 +322,7 @@ void CreditCardFIDOAuthenticator::OptChange( ...@@ -322,7 +322,7 @@ void CreditCardFIDOAuthenticator::OptChange(
request_details.fido_authenticator_response = request_details.fido_authenticator_response =
std::move(authenticator_response); std::move(authenticator_response);
opt_change_metric = opt_change_metric =
request_details.fido_authenticator_response.FindKey( request_details.fido_authenticator_response->FindKey(
"fido_assertion_info") "fido_assertion_info")
? AutofillMetrics::WebauthnOptInParameters::kWithRequestChallenge ? AutofillMetrics::WebauthnOptInParameters::kWithRequestChallenge
: AutofillMetrics::WebauthnOptInParameters::kWithCreationChallenge; : AutofillMetrics::WebauthnOptInParameters::kWithCreationChallenge;
......
...@@ -50,7 +50,8 @@ void FullCardRequest::GetFullCard(const CreditCard& card, ...@@ -50,7 +50,8 @@ void FullCardRequest::GetFullCard(const CreditCard& card,
base::WeakPtr<ResultDelegate> result_delegate, base::WeakPtr<ResultDelegate> result_delegate,
base::WeakPtr<UIDelegate> ui_delegate) { base::WeakPtr<UIDelegate> ui_delegate) {
DCHECK(ui_delegate); DCHECK(ui_delegate);
GetFullCard(card, reason, result_delegate, ui_delegate, base::Value()); GetFullCard(card, reason, result_delegate, ui_delegate,
/*fido_assertion_info=*/base::nullopt);
} }
void FullCardRequest::GetFullCardViaFIDO( void FullCardRequest::GetFullCardViaFIDO(
...@@ -63,15 +64,16 @@ void FullCardRequest::GetFullCardViaFIDO( ...@@ -63,15 +64,16 @@ void FullCardRequest::GetFullCardViaFIDO(
std::move(fido_assertion_info)); std::move(fido_assertion_info));
} }
void FullCardRequest::GetFullCard(const CreditCard& card, void FullCardRequest::GetFullCard(
AutofillClient::UnmaskCardReason reason, const CreditCard& card,
base::WeakPtr<ResultDelegate> result_delegate, AutofillClient::UnmaskCardReason reason,
base::WeakPtr<UIDelegate> ui_delegate, base::WeakPtr<ResultDelegate> result_delegate,
base::Value fido_assertion_info) { base::WeakPtr<UIDelegate> ui_delegate,
base::Optional<base::Value> fido_assertion_info) {
// Retrieval of card information should happen via CVC auth or FIDO, but not // Retrieval of card information should happen via CVC auth or FIDO, but not
// both. Use |ui_delegate|'s existence as evidence of doing CVC auth and // both. Use |ui_delegate|'s existence as evidence of doing CVC auth and
// |fido_assertion_info| as evidence of doing FIDO auth. // |fido_assertion_info| as evidence of doing FIDO auth.
DCHECK_NE(fido_assertion_info.is_dict(), !!ui_delegate); DCHECK_NE(fido_assertion_info.has_value(), !!ui_delegate);
DCHECK(result_delegate); DCHECK(result_delegate);
// Only one request can be active at a time. If the member variable // Only one request can be active at a time. If the member variable
...@@ -153,7 +155,7 @@ void FullCardRequest::OnUnmaskPromptClosed() { ...@@ -153,7 +155,7 @@ void FullCardRequest::OnUnmaskPromptClosed() {
void FullCardRequest::OnDidGetUnmaskRiskData(const std::string& risk_data) { void FullCardRequest::OnDidGetUnmaskRiskData(const std::string& risk_data) {
request_->risk_data = risk_data; request_->risk_data = risk_data;
if (!request_->user_response.cvc.empty() || if (!request_->user_response.cvc.empty() ||
!request_->fido_assertion_info.is_none()) { request_->fido_assertion_info.has_value()) {
SendUnmaskCardRequest(); SendUnmaskCardRequest();
} }
} }
...@@ -171,12 +173,12 @@ void FullCardRequest::OnDidGetRealPan( ...@@ -171,12 +173,12 @@ void FullCardRequest::OnDidGetRealPan(
// If the CVC field is populated, that means the user performed a CVC check. // If the CVC field is populated, that means the user performed a CVC check.
// If FIDO AssertionInfo is populated, then the user must have performed FIDO // If FIDO AssertionInfo is populated, then the user must have performed FIDO
// authentication. Exactly one of these fields must be populated. // authentication. Exactly one of these fields must be populated.
DCHECK_NE(request_->user_response.cvc.empty(), DCHECK_NE(!request_->user_response.cvc.empty(),
request_->fido_assertion_info.is_none()); request_->fido_assertion_info.has_value());
if (!request_->user_response.cvc.empty()) { if (!request_->user_response.cvc.empty()) {
AutofillMetrics::LogRealPanDuration( AutofillMetrics::LogRealPanDuration(
AutofillTickClock::NowTicks() - real_pan_request_timestamp_, result); AutofillTickClock::NowTicks() - real_pan_request_timestamp_, result);
} else if (!request_->fido_assertion_info.is_none()) { } else if (request_->fido_assertion_info.has_value()) {
AutofillMetrics::LogCardUnmaskDurationAfterWebauthn( AutofillMetrics::LogCardUnmaskDurationAfterWebauthn(
AutofillTickClock::NowTicks() - real_pan_request_timestamp_, result); AutofillTickClock::NowTicks() - real_pan_request_timestamp_, result);
} }
......
...@@ -128,7 +128,7 @@ class FullCardRequest final : public CardUnmaskDelegate { ...@@ -128,7 +128,7 @@ class FullCardRequest final : public CardUnmaskDelegate {
AutofillClient::UnmaskCardReason reason, AutofillClient::UnmaskCardReason reason,
base::WeakPtr<ResultDelegate> result_delegate, base::WeakPtr<ResultDelegate> result_delegate,
base::WeakPtr<UIDelegate> ui_delegate, base::WeakPtr<UIDelegate> ui_delegate,
base::Value fido_assertion_info); base::Optional<base::Value> fido_assertion_info);
// CardUnmaskDelegate: // CardUnmaskDelegate:
void OnUnmaskPromptAccepted( void OnUnmaskPromptAccepted(
......
...@@ -387,14 +387,15 @@ class UnmaskCardRequest : public PaymentsRequest { ...@@ -387,14 +387,15 @@ class UnmaskCardRequest : public PaymentsRequest {
// Either FIDO assertion info is set or CVC is set, never both. // Either FIDO assertion info is set or CVC is set, never both.
bool is_cvc_auth = !request_details_.user_response.cvc.empty(); bool is_cvc_auth = !request_details_.user_response.cvc.empty();
bool is_fido_auth = request_details_.fido_assertion_info.is_dict(); bool is_fido_auth = request_details_.fido_assertion_info.has_value();
DCHECK_NE(is_cvc_auth, is_fido_auth); DCHECK_NE(is_cvc_auth, is_fido_auth);
if (is_cvc_auth) { if (is_cvc_auth) {
request_dict.SetKey("encrypted_cvc", base::Value("__param:s7e_13_cvc")); request_dict.SetKey("encrypted_cvc", base::Value("__param:s7e_13_cvc"));
} else { } else {
request_dict.SetKey("fido_assertion_info", request_dict.SetKey(
std::move(request_details_.fido_assertion_info)); "fido_assertion_info",
std::move(request_details_.fido_assertion_info.value()));
} }
std::string json_request; std::string json_request;
...@@ -516,12 +517,12 @@ class OptChangeRequest : public PaymentsRequest { ...@@ -516,12 +517,12 @@ class OptChangeRequest : public PaymentsRequest {
} }
request_dict.SetKey("reason", base::Value(reason)); request_dict.SetKey("reason", base::Value(reason));
if (request_details_.fido_authenticator_response.is_dict()) { if (request_details_.fido_authenticator_response.has_value()) {
base::Value fido_authentication_info(base::Value::Type::DICTIONARY); base::Value fido_authentication_info(base::Value::Type::DICTIONARY);
fido_authentication_info.SetKey( fido_authentication_info.SetKey(
"fido_authenticator_response", "fido_authenticator_response",
std::move(request_details_.fido_authenticator_response)); std::move(request_details_.fido_authenticator_response.value()));
if (!request_details_.card_authorization_token.empty()) { if (!request_details_.card_authorization_token.empty()) {
fido_authentication_info.SetKey( fido_authentication_info.SetKey(
...@@ -1021,7 +1022,11 @@ PaymentsClient::UnmaskRequestDetails::UnmaskRequestDetails( ...@@ -1021,7 +1022,11 @@ PaymentsClient::UnmaskRequestDetails::UnmaskRequestDetails(
card = other.card; card = other.card;
risk_data = other.risk_data; risk_data = other.risk_data;
user_response = other.user_response; user_response = other.user_response;
fido_assertion_info = other.fido_assertion_info.Clone(); if (other.fido_assertion_info.has_value()) {
fido_assertion_info = other.fido_assertion_info->Clone();
} else {
fido_assertion_info.reset();
}
} }
PaymentsClient::UnmaskRequestDetails::~UnmaskRequestDetails() = default; PaymentsClient::UnmaskRequestDetails::~UnmaskRequestDetails() = default;
...@@ -1053,7 +1058,11 @@ PaymentsClient::OptChangeRequestDetails::OptChangeRequestDetails( ...@@ -1053,7 +1058,11 @@ PaymentsClient::OptChangeRequestDetails::OptChangeRequestDetails(
const OptChangeRequestDetails& other) { const OptChangeRequestDetails& other) {
app_locale = other.app_locale; app_locale = other.app_locale;
reason = other.reason; reason = other.reason;
fido_authenticator_response = other.fido_authenticator_response.Clone(); if (other.fido_authenticator_response.has_value()) {
fido_authenticator_response = other.fido_authenticator_response->Clone();
} else {
fido_authenticator_response.reset();
}
card_authorization_token = other.card_authorization_token; card_authorization_token = other.card_authorization_token;
} }
PaymentsClient::OptChangeRequestDetails::~OptChangeRequestDetails() = default; PaymentsClient::OptChangeRequestDetails::~OptChangeRequestDetails() = default;
......
...@@ -83,7 +83,7 @@ class PaymentsClient { ...@@ -83,7 +83,7 @@ class PaymentsClient {
bool offer_fido_opt_in = false; bool offer_fido_opt_in = false;
// Public Key Credential Request Options required for authentication. // Public Key Credential Request Options required for authentication.
// https://www.w3.org/TR/webauthn/#dictdef-publickeycredentialrequestoptions // https://www.w3.org/TR/webauthn/#dictdef-publickeycredentialrequestoptions
base::Optional<base::Value> fido_request_options; base::Optional<base::Value> fido_request_options = base::nullopt;
// Set of credit cards ids that are eligible for FIDO Authentication. // Set of credit cards ids that are eligible for FIDO Authentication.
std::set<std::string> fido_eligible_card_ids; std::set<std::string> fido_eligible_card_ids;
}; };
...@@ -100,7 +100,7 @@ class PaymentsClient { ...@@ -100,7 +100,7 @@ class PaymentsClient {
CreditCard card; CreditCard card;
std::string risk_data; std::string risk_data;
CardUnmaskDelegate::UserProvidedUnmaskDetails user_response; CardUnmaskDelegate::UserProvidedUnmaskDetails user_response;
base::Value fido_assertion_info; base::Optional<base::Value> fido_assertion_info = base::nullopt;
}; };
// Information retrieved from an UnmaskRequest. // Information retrieved from an UnmaskRequest.
...@@ -159,7 +159,7 @@ class PaymentsClient { ...@@ -159,7 +159,7 @@ class PaymentsClient {
Reason reason; Reason reason;
// Signature required for enrolling user into FIDO authentication for future // Signature required for enrolling user into FIDO authentication for future
// card unmasking. // card unmasking.
base::Value fido_authenticator_response; base::Optional<base::Value> fido_authenticator_response = base::nullopt;
// An opaque token used to logically chain consecutive UnmaskCard and // An opaque token used to logically chain consecutive UnmaskCard and
// OptChange calls together. // OptChange calls together.
std::string card_authorization_token = std::string(); std::string card_authorization_token = std::string();
...@@ -176,10 +176,10 @@ class PaymentsClient { ...@@ -176,10 +176,10 @@ class PaymentsClient {
base::Optional<bool> user_is_opted_in; base::Optional<bool> user_is_opted_in;
// Challenge required for enrolling user into FIDO authentication for future // Challenge required for enrolling user into FIDO authentication for future
// card unmasking. // card unmasking.
base::Optional<base::Value> fido_creation_options; base::Optional<base::Value> fido_creation_options = base::nullopt;
// Challenge required for authorizing user for FIDO authentication for // Challenge required for authorizing user for FIDO authentication for
// future card unmasking. // future card unmasking.
base::Optional<base::Value> fido_request_options; base::Optional<base::Value> fido_request_options = base::nullopt;
}; };
// A collection of the information required to make a credit card upload // A collection of the information required to make a credit card upload
......
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