Wire up smart cards with new PIN UI.
The CL introduces a new security token request controller similar to the
parent access controller, which was the only user of the PIN UI until
now.
This CL also adds small bits of functionality to the UI that is required
for the new use-case: Disabling input and clearing the input field.
The new UI applies to login and lock screen, but not in-session or for
OOBE, which use different UIs.
Bug: 1001288
Change-Id: I8c69064a0d944bb6b5a7dac13a2ec2bd968deece
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2020968
Commit-Queue: Fabian Sommer <fabiansommer@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745037}
diff --git a/ash/BUILD.gn b/ash/BUILD.gn
index 953f188..d37e370 100644
--- a/ash/BUILD.gn
+++ b/ash/BUILD.gn
@@ -425,6 +425,8 @@
"login/login_screen_controller.h",
"login/parent_access_controller.cc",
"login/parent_access_controller.h",
+ "login/security_token_request_controller.cc",
+ "login/security_token_request_controller.h",
"login/ui/animated_rounded_image_view.cc",
"login/ui/animated_rounded_image_view.h",
"login/ui/animation_frame.h",
@@ -1819,6 +1821,7 @@
"login/mock_login_screen_client.cc",
"login/mock_login_screen_client.h",
"login/parent_access_controller_unittest.cc",
+ "login/security_token_request_controller_unittest.cc",
"login/ui/fake_login_detachable_base_model.cc",
"login/ui/fake_login_detachable_base_model.h",
"login/ui/lock_contents_view_unittest.cc",
diff --git a/ash/DEPS b/ash/DEPS
index 43ef575..7c7806c 100644
--- a/ash/DEPS
+++ b/ash/DEPS
@@ -57,6 +57,7 @@
"+chromeos/components/multidevice/logging/logging.h",
"+chromeos/components/proximity_auth/public/mojom",
"+chromeos/components/quick_answers",
+ "+chromeos/components/security_token_pin",
"+chromeos/constants",
# TODO(https://crbug.com/940810): Eliminate this.
"+chromeos/dbus/initialize_dbus_client.h",
diff --git a/ash/ash_strings.grd b/ash/ash_strings.grd
index 0816fe2..9ed4108 100644
--- a/ash/ash_strings.grd
+++ b/ash/ash_strings.grd
@@ -1956,6 +1956,12 @@
<message name="IDS_ASH_LOGIN_PIN_REQUEST_NEXT_NUMBER_PROMPT" desc="Accessible prompt read when next access code input field has been focused. Asks user to enter next piece of the access code.">
Next number
</message>
+ <message name="IDS_ASH_LOGIN_SECURITY_TOKEN_REQUEST_DIALOG_TITLE" desc="Title of the PIN dialog to unlock the device with a smart card.">
+ Smart card PIN
+ </message>
+ <message name="IDS_ASH_LOGIN_SECURITY_TOKEN_REQUEST_DIALOG_DESCRIPTION" desc="Message in the PIN dialog to unlock the device with a smart card, asking the user to enter their PIN.">
+ To unlock the device, enter your PIN.
+ </message>
<message name="IDS_ASH_LOGIN_SIGN_IN_REQUIRED_MESSAGE" desc="Message shown in the login screen when online sign-in is required.">
Sign in required
</message>
diff --git a/ash/ash_strings_grd/IDS_ASH_LOGIN_SECURITY_TOKEN_REQUEST_DIALOG_DESCRIPTION.png.sha1 b/ash/ash_strings_grd/IDS_ASH_LOGIN_SECURITY_TOKEN_REQUEST_DIALOG_DESCRIPTION.png.sha1
new file mode 100644
index 0000000..ab47bcf
--- /dev/null
+++ b/ash/ash_strings_grd/IDS_ASH_LOGIN_SECURITY_TOKEN_REQUEST_DIALOG_DESCRIPTION.png.sha1
@@ -0,0 +1 @@
+6adb6ee247ce0cf120b1694a85bdd6185f9dbb36
\ No newline at end of file
diff --git a/ash/ash_strings_grd/IDS_ASH_LOGIN_SECURITY_TOKEN_REQUEST_DIALOG_TITLE.png.sha1 b/ash/ash_strings_grd/IDS_ASH_LOGIN_SECURITY_TOKEN_REQUEST_DIALOG_TITLE.png.sha1
new file mode 100644
index 0000000..ab47bcf
--- /dev/null
+++ b/ash/ash_strings_grd/IDS_ASH_LOGIN_SECURITY_TOKEN_REQUEST_DIALOG_TITLE.png.sha1
@@ -0,0 +1 @@
+6adb6ee247ce0cf120b1694a85bdd6185f9dbb36
\ No newline at end of file
diff --git a/ash/login/login_screen_controller.cc b/ash/login/login_screen_controller.cc
index 1cba4cd..94d3c9f 100644
--- a/ash/login/login_screen_controller.cc
+++ b/ash/login/login_screen_controller.cc
@@ -8,6 +8,7 @@
#include "ash/focus_cycler.h"
#include "ash/login/parent_access_controller.h"
+#include "ash/login/security_token_request_controller.h"
#include "ash/login/ui/lock_screen.h"
#include "ash/login/ui/login_data_dispatcher.h"
#include "ash/public/cpp/ash_pref_names.h"
@@ -210,13 +211,8 @@
return client_->ValidateParentAccessCode(account_id, code, validation_time);
}
-void LoginScreenController::OnSecurityTokenPinRequestCancelledByUser() {
- security_token_pin_request_cancelled_ = true;
- std::move(on_request_security_token_ui_closed_).Run();
-}
-
-bool LoginScreenController::GetSecurityTokenPinRequestCancelled() const {
- return security_token_pin_request_cancelled_;
+bool LoginScreenController::GetSecurityTokenPinRequestCanceled() const {
+ return security_token_request_controller_.request_canceled();
}
void LoginScreenController::HardlockPod(const AccountId& account_id) {
@@ -415,33 +411,11 @@
void LoginScreenController::RequestSecurityTokenPin(
SecurityTokenPinRequest request) {
- if (LockScreen::HasInstance() && !security_token_pin_request_cancelled_) {
- // The caller must ensure that there is no unresolved pin request currently
- // in progress.
- on_request_security_token_ui_closed_ =
- std::move(request.pin_ui_closed_callback);
- // base::Unretained(this) could lead to errors if this controller is
- // destroyed before the callback happens. This will be fixed by
- // crbug.com/1001288 by using a UI owned by the controller.
- request.pin_ui_closed_callback = base::BindOnce(
- &LoginScreenController::OnSecurityTokenPinRequestCancelledByUser,
- base::Unretained(this));
- LockScreen::Get()->RequestSecurityTokenPin(std::move(request));
- } else {
- // The user closed the PIN UI on a previous request that was part of the
- // same smart card login attempt, or the PIN request is made at an
- // inappropriate time, racing with the lock screen showing/hiding.
- std::move(request.pin_ui_closed_callback).Run();
- }
+ security_token_request_controller_.SetPinUiState(std::move(request));
}
void LoginScreenController::ClearSecurityTokenPinRequest() {
- if (!LockScreen::HasInstance()) {
- // Corner case: the request is made at inappropriate time, racing with the
- // lock screen showing/hiding.
- return;
- }
- LockScreen::Get()->ClearSecurityTokenPinRequest();
+ security_token_request_controller_.ClosePinUi();
}
void LoginScreenController::ShowLockScreen() {
@@ -506,9 +480,13 @@
bool success) {
authentication_stage_ = AuthenticationStage::kUserCallback;
std::move(callback).Run(base::make_optional<bool>(success));
- security_token_pin_request_cancelled_ = false;
- on_request_security_token_ui_closed_.Reset();
authentication_stage_ = AuthenticationStage::kIdle;
+
+ // During smart card login flow, multiple security token requests can be made.
+ // If the user cancels one, all others should also be canceled.
+ // At this point, the flow is ending and new security token requests are
+ // displayed again.
+ security_token_request_controller_.ResetRequestCanceled();
}
void LoginScreenController::OnShow() {
diff --git a/ash/login/login_screen_controller.h b/ash/login/login_screen_controller.h
index 772d62ff..96b124c 100644
--- a/ash/login/login_screen_controller.h
+++ b/ash/login/login_screen_controller.h
@@ -8,6 +8,7 @@
#include <vector>
#include "ash/ash_export.h"
+#include "ash/login/security_token_request_controller.h"
#include "ash/login/ui/login_data_dispatcher.h"
#include "ash/public/cpp/kiosk_app_menu.h"
#include "ash/public/cpp/login_screen.h"
@@ -71,8 +72,7 @@
bool ValidateParentAccessCode(const AccountId& account_id,
base::Time validation_time,
const std::string& code);
- void OnSecurityTokenPinRequestCancelledByUser();
- bool GetSecurityTokenPinRequestCancelled() const;
+ bool GetSecurityTokenPinRequestCanceled() const;
void HardlockPod(const AccountId& account_id);
void OnFocusPod(const AccountId& account_id);
void OnNoPodFocused();
@@ -161,15 +161,11 @@
SystemTrayNotifier* system_tray_notifier_;
+ SecurityTokenRequestController security_token_request_controller_;
+
// If set to false, all auth requests will forcibly fail.
ForceFailAuth force_fail_auth_for_debug_overlay_ = ForceFailAuth::kOff;
- // Original OnUiClosed callback of a pending security token pin request.
- // Invoked by OnSecurityTokenPinRequestCancelledByUser.
- SecurityTokenPinRequest::OnUiClosed on_request_security_token_ui_closed_;
-
- bool security_token_pin_request_cancelled_ = false;
-
base::WeakPtrFactory<LoginScreenController> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(LoginScreenController);
diff --git a/ash/login/security_token_request_controller.cc b/ash/login/security_token_request_controller.cc
new file mode 100644
index 0000000..153a192
--- /dev/null
+++ b/ash/login/security_token_request_controller.cc
@@ -0,0 +1,130 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/login/security_token_request_controller.h"
+
+#include <utility>
+
+#include "ash/login/ui/pin_request_widget.h"
+#include "ash/public/cpp/login_types.h"
+#include "ash/strings/grit/ash_strings.h"
+#include "base/bind.h"
+#include "base/i18n/number_formatting.h"
+#include "base/strings/string16.h"
+#include "chromeos/components/security_token_pin/error_generator.h"
+#include "ui/base/l10n/l10n_util.h"
+
+namespace ash {
+
+namespace {
+
+base::string16 GetTitle() {
+ return l10n_util::GetStringUTF16(
+ IDS_ASH_LOGIN_SECURITY_TOKEN_REQUEST_DIALOG_TITLE);
+}
+
+base::string16 GetDescription() {
+ return l10n_util::GetStringUTF16(
+ IDS_ASH_LOGIN_SECURITY_TOKEN_REQUEST_DIALOG_DESCRIPTION);
+}
+
+base::string16 GetAccessibleTitle() {
+ return l10n_util::GetStringUTF16(
+ IDS_ASH_LOGIN_SECURITY_TOKEN_REQUEST_DIALOG_TITLE);
+}
+
+} // namespace
+
+SecurityTokenRequestController::SecurityTokenRequestController() = default;
+
+SecurityTokenRequestController::~SecurityTokenRequestController() {
+ ClosePinUi();
+}
+
+void SecurityTokenRequestController::ResetRequestCanceled() {
+ request_canceled_ = false;
+}
+
+PinRequestView::SubmissionResult SecurityTokenRequestController::OnPinSubmitted(
+ const std::string& code) {
+ if (!on_pin_submitted_.is_null()) {
+ std::move(on_pin_submitted_).Run(code);
+ }
+ return PinRequestView::SubmissionResult::kSubmitPending;
+}
+
+void SecurityTokenRequestController::OnBack() {
+ request_canceled_ = true;
+ if (!on_canceled_by_user_.is_null()) {
+ std::move(on_canceled_by_user_).Run();
+ }
+ ClosePinUi();
+}
+
+void SecurityTokenRequestController::OnHelp(gfx::NativeWindow parent_window) {
+ NOTREACHED();
+}
+
+bool SecurityTokenRequestController::SetPinUiState(
+ SecurityTokenPinRequest request) {
+ // Unable to request a PIN while the PinRequestWidget is already used for
+ // something that is not a SecurityTokenPinRequest.
+ // Also deny the request when the user has just canceled another request: For
+ // example, logging in with smart cards usually requires two requests for the
+ // same PIN. When the user has canceled the first one, we do not show another
+ // right afterwards.
+ if ((PinRequestWidget::Get() && !security_token_request_in_progress_) ||
+ request_canceled_) {
+ std::move(request.pin_ui_closed_callback).Run();
+ return false;
+ }
+
+ on_pin_submitted_ = std::move(request.pin_entered_callback);
+ on_canceled_by_user_ = std::move(request.pin_ui_closed_callback);
+
+ // If this is a new request, open a PIN widget. Otherwise, just update the
+ // existing widget.
+ if (!security_token_request_in_progress_) {
+ security_token_request_in_progress_ = true;
+ PinRequest pin_request;
+ pin_request.on_pin_request_done = base::DoNothing::Once<bool>();
+ pin_request.pin_keyboard_always_enabled = true;
+ pin_request.extra_dimmer = true;
+ pin_request.title = GetTitle();
+ pin_request.description = GetDescription();
+ pin_request.accessible_title = GetAccessibleTitle();
+ PinRequestWidget::Show(std::move(pin_request), this);
+ }
+
+ PinRequestWidget::Get()->ClearInput();
+ PinRequestWidget::Get()->SetPinInputEnabled(request.enable_user_input);
+
+ if (request.error_label == chromeos::security_token_pin::ErrorLabel::kNone) {
+ PinRequestWidget::Get()->UpdateState(PinRequestViewState::kNormal,
+ GetTitle(), GetDescription());
+ } else {
+ PinRequestWidget::Get()->UpdateState(
+ PinRequestViewState::kError,
+ /*title=*/
+ chromeos::security_token_pin::GenerateErrorMessage(
+ request.error_label, request.attempts_left,
+ request.enable_user_input),
+ /*description=*/base::string16());
+ }
+ return true;
+}
+
+void SecurityTokenRequestController::ClosePinUi() {
+ if (!security_token_request_in_progress_)
+ return;
+
+ if (PinRequestWidget::Get()) {
+ PinRequestWidget::Get()->Close(false); // Parameter will be ignored.
+ }
+ on_pin_submitted_.Reset();
+ on_canceled_by_user_.Reset();
+ security_token_request_in_progress_ = false;
+}
+
+} // namespace ash
diff --git a/ash/login/security_token_request_controller.h b/ash/login/security_token_request_controller.h
new file mode 100644
index 0000000..f01125a
--- /dev/null
+++ b/ash/login/security_token_request_controller.h
@@ -0,0 +1,65 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ASH_LOGIN_SECURITY_TOKEN_REQUEST_CONTROLLER_H_
+#define ASH_LOGIN_SECURITY_TOKEN_REQUEST_CONTROLLER_H_
+
+#include "ash/login/ui/pin_request_view.h"
+#include "ash/public/cpp/login_types.h"
+#include "base/memory/weak_ptr.h"
+
+namespace ash {
+
+// SecurityTokenRequestController serves as a single point of access to ask the
+// user for a PIN for a security token request.
+class ASH_EXPORT SecurityTokenRequestController
+ : public PinRequestView::Delegate {
+ public:
+ SecurityTokenRequestController();
+ SecurityTokenRequestController(const SecurityTokenRequestController&) =
+ delete;
+ SecurityTokenRequestController& operator=(
+ const SecurityTokenRequestController&) = delete;
+ ~SecurityTokenRequestController() override;
+
+ bool request_canceled() const { return request_canceled_; }
+ void ResetRequestCanceled();
+
+ // PinRequestView::Delegate interface.
+ PinRequestView::SubmissionResult OnPinSubmitted(
+ const std::string& pin) override;
+ void OnBack() override;
+ void OnHelp(gfx::NativeWindow parent_window) override;
+
+ // Shows the PIN dialog configured by |request|. If there already is a
+ // SecurityTokenPinRequest in progress, keeps the dialog open and updates the
+ // dialog's state.
+ // Returns true if the dialog was opened or updated successfully, false
+ // otherwise. The request will fail if the PIN UI is currently in use for
+ // something other than a SecurityTokenPinRequest.
+ bool SetPinUiState(SecurityTokenPinRequest request);
+
+ // Closes the UI and resets callbacks.
+ void ClosePinUi();
+
+ private:
+ // Called when the user submits the input. Will not be called if the UI is
+ // closed before that happens.
+ SecurityTokenPinRequest::OnPinEntered on_pin_submitted_;
+
+ // Called when the PIN request UI gets closed by the user (back button).
+ SecurityTokenPinRequest::OnUiClosed on_canceled_by_user_;
+
+ // Whether this controller is currently using PinRequestWidget.
+ bool security_token_request_in_progress_ = false;
+
+ // Whether the user has recently canceled a PIN request.
+ bool request_canceled_ = false;
+
+ base::WeakPtrFactory<SecurityTokenRequestController> weak_factory_{this};
+};
+
+} // namespace ash
+
+#endif // ASH_LOGIN_SECURITY_TOKEN_REQUEST_CONTROLLER_H_
diff --git a/ash/login/security_token_request_controller_unittest.cc b/ash/login/security_token_request_controller_unittest.cc
new file mode 100644
index 0000000..e4bd82c
--- /dev/null
+++ b/ash/login/security_token_request_controller_unittest.cc
@@ -0,0 +1,129 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/login/security_token_request_controller.h"
+
+#include <memory>
+
+#include "ash/login/ui/arrow_button_view.h"
+#include "ash/login/ui/login_test_base.h"
+#include "ash/login/ui/pin_request_view.h"
+#include "ash/login/ui/pin_request_widget.h"
+#include "ash/public/cpp/login_types.h"
+#include "base/bind.h"
+#include "base/run_loop.h"
+#include "ui/events/base_event_utils.h"
+#include "ui/events/event.h"
+#include "ui/events/types/event_type.h"
+#include "ui/gfx/geometry/point.h"
+#include "ui/views/controls/button/button.h"
+
+namespace ash {
+
+class SecurityTokenRequestControllerTest : public LoginTestBase {
+ protected:
+ SecurityTokenRequestControllerTest() = default;
+ SecurityTokenRequestControllerTest(
+ const SecurityTokenRequestControllerTest&) = delete;
+ SecurityTokenRequestControllerTest& operator=(
+ const SecurityTokenRequestControllerTest&) = delete;
+ ~SecurityTokenRequestControllerTest() override = default;
+
+ // LoginScreenTest:
+ void SetUp() override {
+ LoginTestBase::SetUp();
+ controller_ = std::make_unique<SecurityTokenRequestController>();
+ }
+
+ void TearDown() override {
+ controller_.reset();
+ LoginTestBase::TearDown();
+ }
+
+ // Simulates mouse press event on a |button|.
+ void SimulateButtonPress(views::Button* button) {
+ ui::MouseEvent event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
+ ui::EventTimeForNow(), 0, 0);
+ view_->ButtonPressed(button, event);
+ }
+
+ void StartRequest(int attempts_left = -1) {
+ SecurityTokenPinRequest request;
+ request.pin_entered_callback =
+ base::BindOnce(&SecurityTokenRequestControllerTest::OnPinEntered,
+ base::Unretained(this));
+ request.pin_ui_closed_callback =
+ base::BindOnce(&SecurityTokenRequestControllerTest::OnUiClosedByUser,
+ base::Unretained(this));
+ request.attempts_left = attempts_left;
+ request.enable_user_input = attempts_left != 0;
+ controller_->SetPinUiState(std::move(request));
+ EXPECT_TRUE(PinRequestWidget::Get());
+ view_ =
+ PinRequestWidget::TestApi(PinRequestWidget::Get()).pin_request_view();
+ }
+
+ // Simulates entering a PIN (012345).
+ void SimulateValidation() {
+ ui::test::EventGenerator* generator = GetEventGenerator();
+ for (int i = 0; i < 6; ++i) {
+ generator->PressKey(ui::KeyboardCode(ui::KeyboardCode::VKEY_0 + i),
+ ui::EF_NONE);
+ }
+ if (PinRequestView::TestApi(view_).submit_button()->GetEnabled())
+ SimulateButtonPress(PinRequestView::TestApi(view_).submit_button());
+ }
+
+ std::unique_ptr<SecurityTokenRequestController> controller_;
+
+ // Number of times a PIN was entered
+ int pin_entered_calls_ = 0;
+
+ // Number of times the UI was closed.
+ int ui_closed_by_user_calls_ = 0;
+
+ PinRequestView* view_ = nullptr; // Owned by test widget view hierarchy.
+
+ private:
+ void OnPinEntered(const std::string& user_input) { ++pin_entered_calls_; }
+ void OnUiClosedByUser() { ++ui_closed_by_user_calls_; }
+};
+
+// Tests successful PIN validation flow.
+TEST_F(SecurityTokenRequestControllerTest, SecurityTokenSuccessfulValidation) {
+ StartRequest();
+ SimulateValidation();
+ EXPECT_EQ(1, pin_entered_calls_);
+ controller_->ClosePinUi();
+ EXPECT_FALSE(PinRequestWidget::Get());
+}
+
+// Tests unsuccessful PIN flow, including cancelling the request.
+TEST_F(SecurityTokenRequestControllerTest,
+ SecurityTokenUnsuccessfulValidation) {
+ StartRequest();
+ SimulateValidation();
+ EXPECT_EQ(1, pin_entered_calls_);
+ EXPECT_TRUE(PinRequestWidget::Get());
+
+ // Simulate wrong PIN response.
+ StartRequest(/*attempts_left=*/1);
+ SimulateValidation();
+ EXPECT_EQ(2, pin_entered_calls_);
+ EXPECT_TRUE(PinRequestWidget::Get());
+
+ // Wrong PIN again. UI should not allow PIN input/submission when there are no
+ // attempts left.
+ StartRequest(/*attempts_left=*/0);
+ SimulateValidation();
+ EXPECT_EQ(2, pin_entered_calls_);
+ EXPECT_TRUE(PinRequestWidget::Get());
+
+ // User should still be able to close the PIN widget.
+ SimulateButtonPress(PinRequestView::TestApi(view_).back_button());
+ EXPECT_EQ(1, ui_closed_by_user_calls_);
+ EXPECT_FALSE(PinRequestWidget::Get());
+}
+
+} // namespace ash
diff --git a/ash/login/ui/login_auth_user_view.cc b/ash/login/ui/login_auth_user_view.cc
index e9581cd..37598ef7 100644
--- a/ash/login/ui/login_auth_user_view.cc
+++ b/ash/login/ui/login_auth_user_view.cc
@@ -1365,12 +1365,12 @@
if (!auth_success.has_value() || !auth_success.value()) {
password_view_->Clear();
password_view_->SetReadOnly(false);
- // If the user cancelled the PIN request during ChallengeResponse,
+ // If the user canceled the PIN request during ChallengeResponse,
// ChallengeResponse will fail with an unknown error. Since this is
// expected, we do not show this error.
if (Shell::Get()
->login_screen_controller()
- ->GetSecurityTokenPinRequestCancelled()) {
+ ->GetSecurityTokenPinRequestCanceled()) {
challenge_response_view_->SetState(
ChallengeResponseView::State::kInitial);
} else {
diff --git a/ash/login/ui/pin_request_view.cc b/ash/login/ui/pin_request_view.cc
index 417ae6a..7c54348 100644
--- a/ash/login/ui/pin_request_view.cc
+++ b/ash/login/ui/pin_request_view.cc
@@ -156,8 +156,10 @@
// Sets the color of the input text.
virtual void SetInputColor(SkColor color) = 0;
- // Enables/disables input.
virtual void SetInputEnabled(bool input_enabled) = 0;
+
+ // Clears the input field(s).
+ virtual void ClearInput() = 0;
};
class PinRequestView::FlexCodeInput : public AccessCodeInput {
@@ -253,6 +255,12 @@
code_field_->SetEnabled(input_enabled);
}
+ // Clears text in input text field.
+ void ClearInput() override {
+ code_field_->SetText(base::string16());
+ on_input_change_.Run(false);
+ }
+
void RequestFocus() override { code_field_->RequestFocus(); }
// views::TextfieldController
@@ -582,10 +590,14 @@
return true;
}
- // Enables/disables input. Currently, there is no use-case the uses this with
- // fixed length PINs.
+ // Enables/disables entering a PIN. Currently, there is no use-case the uses
+ // this with fixed length PINs.
void SetInputEnabled(bool input_enabled) override { NOTIMPLEMENTED(); }
+ // Clears the PIN fields. Currently, there is no use-case the uses this with
+ // fixed length PINs.
+ void ClearInput() override { NOTIMPLEMENTED(); }
+
private:
// Moves focus to the previous input field if it exists.
void FocusPreviousField() {
@@ -785,6 +797,7 @@
// Main view title.
title_label_ = new views::Label(default_title_, views::style::CONTEXT_LABEL,
views::style::STYLE_PRIMARY);
+ title_label_->SetMultiLine(true);
title_label_->SetLineHeight(kTitleLineHeightDp);
title_label_->SetFontList(gfx::FontList().Derive(
kTitleFontSizeDeltaDp, gfx::Font::NORMAL, gfx::Font::Weight::MEDIUM));
@@ -1015,6 +1028,10 @@
}
}
+void PinRequestView::ClearInput() {
+ access_code_view_->ClearInput();
+}
+
void PinRequestView::SetInputEnabled(bool input_enabled) {
access_code_view_->SetInputEnabled(input_enabled);
}
diff --git a/ash/login/ui/pin_request_view.h b/ash/login/ui/pin_request_view.h
index 9fd10aec..2d060010 100644
--- a/ash/login/ui/pin_request_view.h
+++ b/ash/login/ui/pin_request_view.h
@@ -151,9 +151,13 @@
void OnTabletModeEnded() override;
void OnTabletControllerDestroyed() override;
- // Sets whether the user can enter a PIN.
+ // Sets whether the user can enter a PIN. Other buttons (back, submit etc.)
+ // are unaffected.
void SetInputEnabled(bool input_enabled);
+ // Clears previously entered PIN from the PIN input field(s).
+ void ClearInput();
+
// Updates state of the view.
void UpdateState(PinRequestViewState state,
const base::string16& title,
diff --git a/ash/login/ui/pin_request_widget.cc b/ash/login/ui/pin_request_widget.cc
index 72d859e4..86b62184 100644
--- a/ash/login/ui/pin_request_widget.cc
+++ b/ash/login/ui/pin_request_widget.cc
@@ -28,8 +28,7 @@
PinRequestWidget::TestApi::~TestApi() = default;
PinRequestView* PinRequestWidget::TestApi::pin_request_view() {
- return static_cast<PinRequestView*>(
- pin_request_widget_->widget_->widget_delegate());
+ return pin_request_widget_->GetView();
}
// static
@@ -48,8 +47,17 @@
const base::string16& title,
const base::string16& description) {
DCHECK_EQ(instance_, this);
- static_cast<PinRequestView*>(widget_->widget_delegate())
- ->UpdateState(state, title, description);
+ GetView()->UpdateState(state, title, description);
+}
+
+void PinRequestWidget::SetPinInputEnabled(bool enabled) {
+ DCHECK_EQ(instance_, this);
+ GetView()->SetInputEnabled(enabled);
+}
+
+void PinRequestWidget::ClearInput() {
+ DCHECK_EQ(instance_, this);
+ GetView()->ClearInput();
}
void PinRequestWidget::Close(bool success) {
@@ -107,4 +115,8 @@
keyboard_controller->HideKeyboard(HideReason::kSystem);
}
+PinRequestView* PinRequestWidget::GetView() {
+ return static_cast<PinRequestView*>(widget_->widget_delegate());
+}
+
} // namespace ash
diff --git a/ash/login/ui/pin_request_widget.h b/ash/login/ui/pin_request_widget.h
index 01bc81d..cc14f49 100644
--- a/ash/login/ui/pin_request_widget.h
+++ b/ash/login/ui/pin_request_widget.h
@@ -60,6 +60,12 @@
const base::string16& title,
const base::string16& description);
+ // Enables or disables PIN input.
+ void SetPinInputEnabled(bool enabled);
+
+ // Clears previously entered PIN from the PIN input field(s).
+ void ClearInput();
+
// Closes the widget.
// |success| describes whether the validation was successful and is passed to
// |on_pin_request_done_|.
@@ -72,6 +78,9 @@
// Shows the |widget_| and |dimmer_| if applicable.
void Show();
+ // Returns the associated view.
+ PinRequestView* GetView();
+
// Callback invoked when closing the widget.
PinRequest::OnPinRequestDone on_pin_request_done_;
diff --git a/ash/public/cpp/DEPS b/ash/public/cpp/DEPS
index 225f0c4..d352eac 100644
--- a/ash/public/cpp/DEPS
+++ b/ash/public/cpp/DEPS
@@ -1,6 +1,4 @@
include_rules = [
- "+chromeos/components/security_token_pin",
- "+chromeos/constants",
"+chromeos/services",
"+components/arc/mojom",
"+components/prefs",
diff --git a/ash/public/cpp/login_types.h b/ash/public/cpp/login_types.h
index f185bef..f12a3087 100644
--- a/ash/public/cpp/login_types.h
+++ b/ash/public/cpp/login_types.h
@@ -355,7 +355,8 @@
// Called when the user submits the input. Will not be called if the UI is
// closed before that happens.
- base::OnceCallback<void(const std::string& user_input)> pin_entered_callback;
+ using std::string& user_input)>;
+ OnPinEntered pin_entered_callback;
// Called when the PIN request UI gets closed. Will not be called when the
// browser itself requests the UI to be closed.
diff --git a/chrome/browser/chromeos/login/security_token_pin_dialog_host_ash_impl.cc b/chrome/browser/chromeos/login/security_token_pin_dialog_host_ash_impl.cc
index 1fef07620..94f8f3a 100644
--- a/chrome/browser/chromeos/login/security_token_pin_dialog_host_ash_impl.cc
+++ b/chrome/browser/chromeos/login/security_token_pin_dialog_host_ash_impl.cc
@@ -35,17 +35,6 @@
// the PIN has already been entered.
DCHECK(!pin_entered_callback_);
- if (is_request_running() || !enable_user_input) {
- // Don't allow re-requesting the PIN in the same dialog after an error,
- // since the UI doesn't currently handle this in a user-friendly way.
- // TODO(crbug.com/1001288): Remove this after the proper UI error feedback
- // gets implemented in Ash.
- if (is_request_running())
- CloseSecurityTokenPinDialog();
- std::move(pin_dialog_closed_callback).Run();
- return;
- }
-
Reset();
if (!authenticating_user_account_id) {
@@ -68,7 +57,7 @@
base::BindOnce(&SecurityTokenPinDialogHostAshImpl::OnUserInputReceived,
weak_ptr_factory_.GetWeakPtr());
request.pin_ui_closed_callback =
- base::BindOnce(&SecurityTokenPinDialogHostAshImpl::OnClosed,
+ base::BindOnce(&SecurityTokenPinDialogHostAshImpl::OnClosedByUser,
weak_ptr_factory_.GetWeakPtr());
ash::LoginScreen::Get()->RequestSecurityTokenPin(std::move(request));
@@ -89,7 +78,7 @@
std::move(pin_entered_callback_).Run(user_input);
}
-void SecurityTokenPinDialogHostAshImpl::OnClosed() {
+void SecurityTokenPinDialogHostAshImpl::OnClosedByUser() {
DCHECK(is_request_running());
auto closed_callback = std::move(pin_dialog_closed_callback_);
diff --git a/chrome/browser/chromeos/login/security_token_pin_dialog_host_ash_impl.h b/chrome/browser/chromeos/login/security_token_pin_dialog_host_ash_impl.h
index bcd720f..00f8c2d 100644
--- a/chrome/browser/chromeos/login/security_token_pin_dialog_host_ash_impl.h
+++ b/chrome/browser/chromeos/login/security_token_pin_dialog_host_ash_impl.h
@@ -43,7 +43,7 @@
// Screen UI.
void OnUserInputReceived(const std::string& user_input);
// Called when the PIN UI gets closed.
- void OnClosed();
+ void OnClosedByUser();
// Resets the internal state and weak pointers associated with the previously
// started requests.