Commit 9c76fd45 authored by Martin Kreichgauer's avatar Martin Kreichgauer Committed by Commit Bot

fido: don't show a retry error on the first UV attempt

Make AuthTokenRequester not invoke its Delegate's
PromptInternalUVRetry() method the first time it calls GetUvToken() and
awaits a user interaction.

Fixed: 1142969
Change-Id: I38ce9ebd35a968c7346ce0cecd6a3109a0b6fb34
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2504598
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: default avatarNina Satragno <nsatragno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821883}
parent 650740ea
...@@ -138,7 +138,9 @@ void AuthTokenRequester::OnGetUVRetries( ...@@ -138,7 +138,9 @@ void AuthTokenRequester::OnGetUVRetries(
return; return;
} }
delegate_->PromptForInternalUVRetry(response->retries); if (is_internal_uv_retry_) {
delegate_->PromptForInternalUVRetry(response->retries);
}
authenticator_->GetUvToken({std::begin(options_.token_permissions), authenticator_->GetUvToken({std::begin(options_.token_permissions),
std::end(options_.token_permissions)}, std::end(options_.token_permissions)},
options_.rp_id, options_.rp_id,
...@@ -178,6 +180,7 @@ void AuthTokenRequester::OnGetUVToken( ...@@ -178,6 +180,7 @@ void AuthTokenRequester::OnGetUVToken(
if (status == CtapDeviceResponseCode::kCtap2ErrUvInvalid) { if (status == CtapDeviceResponseCode::kCtap2ErrUvInvalid) {
// The attempt failed, but a retry is possible. // The attempt failed, but a retry is possible.
is_internal_uv_retry_ = true;
ObtainTokenFromInternalUV(); ObtainTokenFromInternalUV();
return; return;
} }
......
...@@ -164,6 +164,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) AuthTokenRequester { ...@@ -164,6 +164,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) AuthTokenRequester {
Options options_; Options options_;
bool authenticator_was_selected_ = false; bool authenticator_was_selected_ = false;
bool is_internal_uv_retry_ = false;
base::WeakPtrFactory<AuthTokenRequester> weak_factory_{this}; base::WeakPtrFactory<AuthTokenRequester> weak_factory_{this};
}; };
......
...@@ -42,7 +42,8 @@ class TestAuthTokenRequesterDelegate : public AuthTokenRequester::Delegate { ...@@ -42,7 +42,8 @@ class TestAuthTokenRequesterDelegate : public AuthTokenRequester::Delegate {
base::Optional<pin::TokenResponse>& response() { return response_; } base::Optional<pin::TokenResponse>& response() { return response_; }
bool pin_was_set() { return pin_was_set_; } bool pin_was_set() { return pin_was_set_; }
bool pin_was_collected() { return pin_was_collected_; } bool pin_was_collected() { return pin_was_collected_; }
bool internal_uv_was_prompted() { return internal_uv_was_prompted_; } bool internal_uv_was_retried() { return internal_uv_num_retries_ > 0u; }
size_t internal_uv_num_retries() { return internal_uv_num_retries_; }
bool internal_uv_was_locked() { return internal_uv_was_locked_; } bool internal_uv_was_locked() { return internal_uv_was_locked_; }
private: private:
...@@ -61,7 +62,7 @@ class TestAuthTokenRequesterDelegate : public AuthTokenRequester::Delegate { ...@@ -61,7 +62,7 @@ class TestAuthTokenRequesterDelegate : public AuthTokenRequester::Delegate {
std::move(provide_pin_cb).Run(pin_); std::move(provide_pin_cb).Run(pin_);
} }
void PromptForInternalUVRetry(int attempts) override { void PromptForInternalUVRetry(int attempts) override {
internal_uv_was_prompted_ = true; internal_uv_num_retries_++;
} }
void InternalUVLockedForAuthToken() override { void InternalUVLockedForAuthToken() override {
internal_uv_was_locked_ = true; internal_uv_was_locked_ = true;
...@@ -83,7 +84,7 @@ class TestAuthTokenRequesterDelegate : public AuthTokenRequester::Delegate { ...@@ -83,7 +84,7 @@ class TestAuthTokenRequesterDelegate : public AuthTokenRequester::Delegate {
bool pin_was_collected_ = false; bool pin_was_collected_ = false;
bool pin_was_set_ = false; bool pin_was_set_ = false;
bool internal_uv_was_prompted_ = false; size_t internal_uv_num_retries_ = 0u;
bool internal_uv_was_locked_ = false; bool internal_uv_was_locked_ = false;
base::RunLoop wait_for_result_loop_; base::RunLoop wait_for_result_loop_;
...@@ -225,7 +226,7 @@ TEST_F(AuthTokenRequesterTest, AuthenticatorWithoutUVTokenSupport) { ...@@ -225,7 +226,7 @@ TEST_F(AuthTokenRequesterTest, AuthenticatorWithoutUVTokenSupport) {
EXPECT_FALSE(delegate_->pin_was_set()); EXPECT_FALSE(delegate_->pin_was_set());
EXPECT_FALSE(delegate_->pin_was_collected()); EXPECT_FALSE(delegate_->pin_was_collected());
} }
EXPECT_FALSE(delegate_->internal_uv_was_prompted()); EXPECT_FALSE(delegate_->internal_uv_was_retried());
EXPECT_FALSE(delegate_->internal_uv_was_locked()); EXPECT_FALSE(delegate_->internal_uv_was_locked());
} }
} }
...@@ -305,9 +306,7 @@ TEST_F(AuthTokenRequesterTest, AuthenticatorWithUVTokenSupport) { ...@@ -305,9 +306,7 @@ TEST_F(AuthTokenRequesterTest, AuthenticatorWithUVTokenSupport) {
t.client_pin == ClientPinAvailability::kSupportedAndPinSet && t.client_pin == ClientPinAvailability::kSupportedAndPinSet &&
t.user_verification != t.user_verification !=
UserVerificationAvailability::kSupportedAndConfigured); UserVerificationAvailability::kSupportedAndConfigured);
EXPECT_EQ(delegate_->internal_uv_was_prompted(), EXPECT_FALSE(delegate_->internal_uv_was_retried());
t.user_verification ==
UserVerificationAvailability::kSupportedAndConfigured);
EXPECT_FALSE(delegate_->internal_uv_was_locked()); EXPECT_FALSE(delegate_->internal_uv_was_locked());
} else { } else {
EXPECT_EQ(*delegate_->result(), EXPECT_EQ(*delegate_->result(),
...@@ -315,7 +314,7 @@ TEST_F(AuthTokenRequesterTest, AuthenticatorWithUVTokenSupport) { ...@@ -315,7 +314,7 @@ TEST_F(AuthTokenRequesterTest, AuthenticatorWithUVTokenSupport) {
EXPECT_FALSE(delegate_->response()); EXPECT_FALSE(delegate_->response());
EXPECT_FALSE(delegate_->pin_was_set()); EXPECT_FALSE(delegate_->pin_was_set());
EXPECT_FALSE(delegate_->pin_was_collected()); EXPECT_FALSE(delegate_->pin_was_collected());
EXPECT_FALSE(delegate_->internal_uv_was_prompted()); EXPECT_FALSE(delegate_->internal_uv_was_retried());
EXPECT_FALSE(delegate_->internal_uv_was_locked()); EXPECT_FALSE(delegate_->internal_uv_was_locked());
} }
} }
...@@ -341,7 +340,7 @@ TEST_F(AuthTokenRequesterTest, PINSoftLock) { ...@@ -341,7 +340,7 @@ TEST_F(AuthTokenRequesterTest, PINSoftLock) {
EXPECT_FALSE(delegate_->response()); EXPECT_FALSE(delegate_->response());
EXPECT_FALSE(delegate_->pin_was_set()); EXPECT_FALSE(delegate_->pin_was_set());
EXPECT_TRUE(delegate_->pin_was_collected()); EXPECT_TRUE(delegate_->pin_was_collected());
EXPECT_FALSE(delegate_->internal_uv_was_prompted()); EXPECT_FALSE(delegate_->internal_uv_was_retried());
EXPECT_FALSE(delegate_->internal_uv_was_locked()); EXPECT_FALSE(delegate_->internal_uv_was_locked());
} }
...@@ -365,7 +364,7 @@ TEST_F(AuthTokenRequesterTest, PINHardLock) { ...@@ -365,7 +364,7 @@ TEST_F(AuthTokenRequesterTest, PINHardLock) {
EXPECT_FALSE(delegate_->response()); EXPECT_FALSE(delegate_->response());
EXPECT_FALSE(delegate_->pin_was_set()); EXPECT_FALSE(delegate_->pin_was_set());
EXPECT_FALSE(delegate_->pin_was_collected()); EXPECT_FALSE(delegate_->pin_was_collected());
EXPECT_FALSE(delegate_->internal_uv_was_prompted()); EXPECT_FALSE(delegate_->internal_uv_was_retried());
EXPECT_FALSE(delegate_->internal_uv_was_locked()); EXPECT_FALSE(delegate_->internal_uv_was_locked());
} }
...@@ -374,8 +373,9 @@ TEST_F(AuthTokenRequesterTest, UVLockedPINFallback) { ...@@ -374,8 +373,9 @@ TEST_F(AuthTokenRequesterTest, UVLockedPINFallback) {
config.pin_uv_auth_token_support = true; config.pin_uv_auth_token_support = true;
config.ctap2_versions = {std::begin(kCtap2Versions2_1), config.ctap2_versions = {std::begin(kCtap2Versions2_1),
std::end(kCtap2Versions2_1)}; std::end(kCtap2Versions2_1)};
config.user_verification_succeeds = false;
auto state = base::MakeRefCounted<VirtualFidoDevice::State>(); auto state = base::MakeRefCounted<VirtualFidoDevice::State>();
state->uv_retries = 0; state->uv_retries = 3;
RunTestCase(std::move(config), state, RunTestCase(std::move(config), state,
TestCase{ TestCase{
...@@ -388,7 +388,7 @@ TEST_F(AuthTokenRequesterTest, UVLockedPINFallback) { ...@@ -388,7 +388,7 @@ TEST_F(AuthTokenRequesterTest, UVLockedPINFallback) {
EXPECT_TRUE(delegate_->response()); EXPECT_TRUE(delegate_->response());
EXPECT_FALSE(delegate_->pin_was_set()); EXPECT_FALSE(delegate_->pin_was_set());
EXPECT_TRUE(delegate_->pin_was_collected()); EXPECT_TRUE(delegate_->pin_was_collected());
EXPECT_FALSE(delegate_->internal_uv_was_prompted()); EXPECT_EQ(delegate_->internal_uv_num_retries(), 2u);
EXPECT_TRUE(delegate_->internal_uv_was_locked()); EXPECT_TRUE(delegate_->internal_uv_was_locked());
} }
......
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