Commit 22f9f166 authored by Yicheng Li's avatar Yicheng Li Committed by Commit Bot

ash: Show website/app name in in-session auth dialog

Fetch the name of the website/app that we are authenticating for from
dbus method call and show it in the dialog. This improves security
and is also the same behavior for WebAuthn on other platforms.

Bug: b:156258540, b:144861739
Change-Id: I05f64e80f555cd7a74e8e8372e54bf3c05483bb0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414624
Commit-Queue: Yicheng Li <yichengli@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824204}
parent a72d2ae0
...@@ -2444,6 +2444,15 @@ This file contains the strings for ash. ...@@ -2444,6 +2444,15 @@ This file contains the strings for ash.
</message> </message>
<!-- In-session auth dialog strings --> <!-- In-session auth dialog strings -->
<message name="IDS_ASH_IN_SESSION_AUTH_TITLE" desc="Text shown in the auth dialog to indicate authentication is needed.">
Verify your identity
</message>
<message name="IDS_ASH_IN_SESSION_AUTH_ORIGIN_NAME_PROMPT" desc="Text shown in the auth dialog to indicate which website / app we are authenticating for.">
<ph name="ORIGIN_NAME">$1<ex>example.com</ex></ph> would like to confirm it's you
</message>
<message name="IDS_ASH_IN_SESSION_AUTH_ACCESSIBLE_TITLE" desc="Accessibility text read by chromevox to indicate which website / app we are authenticating for.">
Verify your identity: <ph name="ORIGIN_NAME">$1<ex>example.com</ex></ph> would like to confirm it's you
</message>
<message name="IDS_ASH_IN_SESSION_AUTH_FINGERPRINT_AVAILABLE" desc="Text shown in the auth dialog to remind user that fingerprint auth is supported"> <message name="IDS_ASH_IN_SESSION_AUTH_FINGERPRINT_AVAILABLE" desc="Text shown in the auth dialog to remind user that fingerprint auth is supported">
Authenticate with fingerprint Authenticate with fingerprint
</message> </message>
......
06bb45e3d732edafb1eab45781c08fdfdddeef78
\ No newline at end of file
06bb45e3d732edafb1eab45781c08fdfdddeef78
\ No newline at end of file
06bb45e3d732edafb1eab45781c08fdfdddeef78
\ No newline at end of file
...@@ -44,9 +44,9 @@ void UserAuthenticationServiceProvider::ShowAuthDialog( ...@@ -44,9 +44,9 @@ void UserAuthenticationServiceProvider::ShowAuthDialog(
dbus::MethodCall* method_call, dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender) { dbus::ExportedObject::ResponseSender response_sender) {
dbus::MessageReader reader(method_call); dbus::MessageReader reader(method_call);
std::string rp_id; std::string origin_name;
if (!reader.PopString(&rp_id)) { if (!reader.PopString(&origin_name)) {
LOG(ERROR) << "Unable to parse rp_id"; LOG(ERROR) << "Unable to parse origin name";
OnAuthFlowComplete(method_call, std::move(response_sender), false); OnAuthFlowComplete(method_call, std::move(response_sender), false);
return; return;
} }
...@@ -74,7 +74,7 @@ void UserAuthenticationServiceProvider::ShowAuthDialog( ...@@ -74,7 +74,7 @@ void UserAuthenticationServiceProvider::ShowAuthDialog(
auto* auth_dialog_controller = InSessionAuthDialogController::Get(); auto* auth_dialog_controller = InSessionAuthDialogController::Get();
auth_dialog_controller->ShowAuthenticationDialog( auth_dialog_controller->ShowAuthenticationDialog(
source_window, source_window, origin_name,
base::BindOnce(&UserAuthenticationServiceProvider::OnAuthFlowComplete, base::BindOnce(&UserAuthenticationServiceProvider::OnAuthFlowComplete,
weak_ptr_factory_.GetWeakPtr(), method_call, weak_ptr_factory_.GetWeakPtr(), method_call,
std::move(response_sender))); std::move(response_sender)));
......
...@@ -34,10 +34,7 @@ ...@@ -34,10 +34,7 @@
namespace ash { namespace ash {
namespace { namespace {
// TODO(b/164195709): Move these strings to a grd file. const int kContainerPreferredWidth = 340;
const char kTitle[] = "Verify it's you";
const int kContainerPreferredWidth = 512;
const int kSpacingAfterAvatar = 18; const int kSpacingAfterAvatar = 18;
const int kSpacingAfterTitle = 16; const int kSpacingAfterTitle = 16;
...@@ -259,9 +256,12 @@ class AuthDialogContentsView::FingerprintView : public views::View { ...@@ -259,9 +256,12 @@ class AuthDialogContentsView::FingerprintView : public views::View {
AuthDialogContentsView::AuthDialogContentsView( AuthDialogContentsView::AuthDialogContentsView(
uint32_t auth_methods, uint32_t auth_methods,
const std::string& origin_name,
const AuthMethodsMetadata& auth_metadata, const AuthMethodsMetadata& auth_metadata,
const UserAvatar& avatar) const UserAvatar& avatar)
: auth_methods_(auth_methods), auth_metadata_(auth_metadata) { : auth_methods_(auth_methods),
origin_name_(origin_name),
auth_metadata_(auth_metadata) {
DCHECK(auth_methods_ & kAuthPassword); DCHECK(auth_methods_ & kAuthPassword);
SetLayoutManager(std::make_unique<views::FillLayout>()); SetLayoutManager(std::make_unique<views::FillLayout>());
...@@ -282,6 +282,8 @@ AuthDialogContentsView::AuthDialogContentsView( ...@@ -282,6 +282,8 @@ AuthDialogContentsView::AuthDialogContentsView(
AddVerticalSpacing(kSpacingAfterAvatar); AddVerticalSpacing(kSpacingAfterAvatar);
AddTitleView(); AddTitleView();
AddVerticalSpacing(kSpacingAfterTitle); AddVerticalSpacing(kSpacingAfterTitle);
AddOriginNameView();
AddVerticalSpacing(kSpacingAfterTitle);
if (auth_methods_ & kAuthPin) { if (auth_methods_ & kAuthPin) {
if (LoginPinInputView::IsAutosubmitSupported( if (LoginPinInputView::IsAutosubmitSupported(
auth_metadata_.autosubmit_pin_length)) { auth_metadata_.autosubmit_pin_length)) {
...@@ -337,13 +339,33 @@ void AuthDialogContentsView::AddTitleView() { ...@@ -337,13 +339,33 @@ void AuthDialogContentsView::AddTitleView() {
title_->SetFontList(base_font_list.Derive(kTitleFontSizeDeltaDp, title_->SetFontList(base_font_list.Derive(kTitleFontSizeDeltaDp,
gfx::Font::FontStyle::NORMAL, gfx::Font::FontStyle::NORMAL,
gfx::Font::Weight::NORMAL)); gfx::Font::Weight::NORMAL));
title_->SetText(base::UTF8ToUTF16(kTitle)); title_->SetText(l10n_util::GetStringUTF16(IDS_ASH_IN_SESSION_AUTH_TITLE));
title_->SetMaximumWidth(kContainerPreferredWidth); title_->SetMaximumWidth(kContainerPreferredWidth);
title_->SetElideBehavior(gfx::ElideBehavior::ELIDE_TAIL); title_->SetElideBehavior(gfx::ElideBehavior::ELIDE_TAIL);
title_->SetPreferredSize( title_->SetPreferredSize(
gfx::Size(kContainerPreferredWidth, title_->height())); gfx::Size(kContainerPreferredWidth, title_->height()));
title_->SetHorizontalAlignment(gfx::ALIGN_LEFT); title_->SetHorizontalAlignment(gfx::ALIGN_CENTER);
}
void AuthDialogContentsView::AddOriginNameView() {
origin_name_view_ =
container_->AddChildView(std::make_unique<views::Label>());
origin_name_view_->SetEnabledColor(SK_ColorBLACK);
origin_name_view_->SetSubpixelRenderingEnabled(false);
origin_name_view_->SetAutoColorReadabilityEnabled(false);
origin_name_view_->SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY);
origin_name_view_->SetText(
l10n_util::GetStringFUTF16(IDS_ASH_IN_SESSION_AUTH_ORIGIN_NAME_PROMPT,
base::UTF8ToUTF16(origin_name_)));
origin_name_view_->SetMaximumWidth(kContainerPreferredWidth);
origin_name_view_->SetMultiLine(true);
origin_name_view_->SetPreferredSize(gfx::Size(
kContainerPreferredWidth,
origin_name_view_->GetHeightForWidth(kContainerPreferredWidth)));
origin_name_view_->SetHorizontalAlignment(gfx::ALIGN_CENTER);
} }
void AuthDialogContentsView::AddPinTextInputView() { void AuthDialogContentsView::AddPinTextInputView() {
...@@ -449,7 +471,9 @@ void AuthDialogContentsView::OnFingerprintAuthComplete( ...@@ -449,7 +471,9 @@ void AuthDialogContentsView::OnFingerprintAuthComplete(
void AuthDialogContentsView::GetAccessibleNodeData(ui::AXNodeData* node_data) { void AuthDialogContentsView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
views::View::GetAccessibleNodeData(node_data); views::View::GetAccessibleNodeData(node_data);
node_data->role = ax::mojom::Role::kDialog; node_data->role = ax::mojom::Role::kDialog;
node_data->SetName(base::UTF8ToUTF16(kTitle)); node_data->SetName(
l10n_util::GetStringFUTF16(IDS_ASH_IN_SESSION_AUTH_ACCESSIBLE_TITLE,
base::UTF8ToUTF16(origin_name_)));
} }
} // namespace ash } // namespace ash
...@@ -42,6 +42,7 @@ class AuthDialogContentsView : public views::View { ...@@ -42,6 +42,7 @@ class AuthDialogContentsView : public views::View {
}; };
AuthDialogContentsView(uint32_t auth_methods, AuthDialogContentsView(uint32_t auth_methods,
const std::string& origin_name,
const AuthMethodsMetadata& auth_metadata, const AuthMethodsMetadata& auth_metadata,
const UserAvatar& avatar); const UserAvatar& avatar);
AuthDialogContentsView(const AuthDialogContentsView&) = delete; AuthDialogContentsView(const AuthDialogContentsView&) = delete;
...@@ -65,8 +66,8 @@ class AuthDialogContentsView : public views::View { ...@@ -65,8 +66,8 @@ class AuthDialogContentsView : public views::View {
// Add a view for dialog title. // Add a view for dialog title.
void AddTitleView(); void AddTitleView();
// Add a view for the prompt message. // Add a view that shows which website/app we are authenticating for.
void AddPromptView(); void AddOriginNameView();
// Add a view for entering PIN (if autosubmit is off). // Add a view for entering PIN (if autosubmit is off).
void AddPinTextInputView(); void AddPinTextInputView();
...@@ -109,7 +110,7 @@ class AuthDialogContentsView : public views::View { ...@@ -109,7 +110,7 @@ class AuthDialogContentsView : public views::View {
views::Label* title_ = nullptr; views::Label* title_ = nullptr;
// Prompt message to the user. // Prompt message to the user.
views::Label* prompt_ = nullptr; views::Label* origin_name_view_ = nullptr;
// Whether PIN can be auto submitted. // Whether PIN can be auto submitted.
bool pin_autosubmit_on_ = false; bool pin_autosubmit_on_ = false;
...@@ -128,6 +129,8 @@ class AuthDialogContentsView : public views::View { ...@@ -128,6 +129,8 @@ class AuthDialogContentsView : public views::View {
// Flags of auth methods that should be visible. // Flags of auth methods that should be visible.
uint32_t auth_methods_ = 0u; uint32_t auth_methods_ = 0u;
const std::string origin_name_;
// Extra parameters to control the UI. // Extra parameters to control the UI.
AuthMethodsMetadata auth_metadata_; AuthMethodsMetadata auth_metadata_;
......
...@@ -49,12 +49,14 @@ std::unique_ptr<views::Widget> CreateAuthDialogWidget( ...@@ -49,12 +49,14 @@ std::unique_ptr<views::Widget> CreateAuthDialogWidget(
InSessionAuthDialog::InSessionAuthDialog( InSessionAuthDialog::InSessionAuthDialog(
uint32_t auth_methods, uint32_t auth_methods,
aura::Window* parent_window, aura::Window* parent_window,
const std::string& origin_name,
const AuthDialogContentsView::AuthMethodsMetadata& auth_metadata, const AuthDialogContentsView::AuthMethodsMetadata& auth_metadata,
const UserAvatar& avatar) const UserAvatar& avatar)
: auth_methods_(auth_methods) { : auth_methods_(auth_methods) {
widget_ = CreateAuthDialogWidget(std::make_unique<AuthDialogContentsView>( widget_ = CreateAuthDialogWidget(
auth_methods, auth_metadata, avatar), std::make_unique<AuthDialogContentsView>(auth_methods, origin_name,
parent_window); auth_metadata, avatar),
parent_window);
gfx::Rect bounds = parent_window->GetBoundsInScreen(); gfx::Rect bounds = parent_window->GetBoundsInScreen();
gfx::Size preferred_size = widget_->GetContentsView()->GetPreferredSize(); gfx::Size preferred_size = widget_->GetContentsView()->GetPreferredSize();
int horizontal_inset_dp = (bounds.width() - preferred_size.width()) / 2; int horizontal_inset_dp = (bounds.width() - preferred_size.width()) / 2;
......
...@@ -30,6 +30,7 @@ class InSessionAuthDialog { ...@@ -30,6 +30,7 @@ class InSessionAuthDialog {
InSessionAuthDialog( InSessionAuthDialog(
uint32_t auth_methods, uint32_t auth_methods,
aura::Window* parent_window, aura::Window* parent_window,
const std::string& origin_name,
const AuthDialogContentsView::AuthMethodsMetadata& auth_metadata, const AuthDialogContentsView::AuthMethodsMetadata& auth_metadata,
const UserAvatar& avatar); const UserAvatar& avatar);
InSessionAuthDialog(const InSessionAuthDialog&) = delete; InSessionAuthDialog(const InSessionAuthDialog&) = delete;
......
...@@ -33,6 +33,7 @@ void InSessionAuthDialogControllerImpl::SetClient( ...@@ -33,6 +33,7 @@ void InSessionAuthDialogControllerImpl::SetClient(
void InSessionAuthDialogControllerImpl::ShowAuthenticationDialog( void InSessionAuthDialogControllerImpl::ShowAuthenticationDialog(
aura::Window* source_window, aura::Window* source_window,
const std::string& origin_name,
FinishCallback finish_callback) { FinishCallback finish_callback) {
DCHECK(client_); DCHECK(client_);
// Concurrent requests are not supported. // Concurrent requests are not supported.
...@@ -51,8 +52,8 @@ void InSessionAuthDialogControllerImpl::ShowAuthenticationDialog( ...@@ -51,8 +52,8 @@ void InSessionAuthDialogControllerImpl::ShowAuthenticationDialog(
account_id, account_id,
base::BindOnce( base::BindOnce(
&InSessionAuthDialogControllerImpl::OnStartFingerprintAuthSession, &InSessionAuthDialogControllerImpl::OnStartFingerprintAuthSession,
weak_factory_.GetWeakPtr(), account_id, auth_methods, weak_factory_.GetWeakPtr(), account_id, auth_methods, source_window,
source_window)); origin_name));
// OnStartFingerprintAuthSession checks PIN availability. // OnStartFingerprintAuthSession checks PIN availability.
return; return;
} }
...@@ -60,13 +61,15 @@ void InSessionAuthDialogControllerImpl::ShowAuthenticationDialog( ...@@ -60,13 +61,15 @@ void InSessionAuthDialogControllerImpl::ShowAuthenticationDialog(
client_->CheckPinAuthAvailability( client_->CheckPinAuthAvailability(
account_id, account_id,
base::BindOnce(&InSessionAuthDialogControllerImpl::OnPinCanAuthenticate, base::BindOnce(&InSessionAuthDialogControllerImpl::OnPinCanAuthenticate,
weak_factory_.GetWeakPtr(), auth_methods, source_window)); weak_factory_.GetWeakPtr(), auth_methods, source_window,
origin_name));
} }
void InSessionAuthDialogControllerImpl::OnStartFingerprintAuthSession( void InSessionAuthDialogControllerImpl::OnStartFingerprintAuthSession(
AccountId account_id, AccountId account_id,
uint32_t auth_methods, uint32_t auth_methods,
aura::Window* source_window, aura::Window* source_window,
const std::string& origin_name,
bool success) { bool success) {
if (success) if (success)
auth_methods |= AuthDialogContentsView::kAuthFingerprint; auth_methods |= AuthDialogContentsView::kAuthFingerprint;
...@@ -74,12 +77,14 @@ void InSessionAuthDialogControllerImpl::OnStartFingerprintAuthSession( ...@@ -74,12 +77,14 @@ void InSessionAuthDialogControllerImpl::OnStartFingerprintAuthSession(
client_->CheckPinAuthAvailability( client_->CheckPinAuthAvailability(
account_id, account_id,
base::BindOnce(&InSessionAuthDialogControllerImpl::OnPinCanAuthenticate, base::BindOnce(&InSessionAuthDialogControllerImpl::OnPinCanAuthenticate,
weak_factory_.GetWeakPtr(), auth_methods, source_window)); weak_factory_.GetWeakPtr(), auth_methods, source_window,
origin_name));
} }
void InSessionAuthDialogControllerImpl::OnPinCanAuthenticate( void InSessionAuthDialogControllerImpl::OnPinCanAuthenticate(
uint32_t auth_methods, uint32_t auth_methods,
aura::Window* source_window, aura::Window* source_window,
const std::string& origin_name,
bool pin_auth_available) { bool pin_auth_available) {
if (pin_auth_available) if (pin_auth_available)
auth_methods |= AuthDialogContentsView::kAuthPin; auth_methods |= AuthDialogContentsView::kAuthPin;
...@@ -113,8 +118,8 @@ void InSessionAuthDialogControllerImpl::OnPinCanAuthenticate( ...@@ -113,8 +118,8 @@ void InSessionAuthDialogControllerImpl::OnPinCanAuthenticate(
user_manager::known_user::GetUserPinLength(account_id); user_manager::known_user::GetUserPinLength(account_id);
window_tracker_.Remove(source_window); window_tracker_.Remove(source_window);
Shell::Get()->focus_controller()->AddObserver(this); Shell::Get()->focus_controller()->AddObserver(this);
dialog_ = std::make_unique<InSessionAuthDialog>(auth_methods, source_window, dialog_ = std::make_unique<InSessionAuthDialog>(
auth_metadata, avatar); auth_methods, source_window, origin_name, auth_metadata, avatar);
} }
void InSessionAuthDialogControllerImpl::DestroyAuthenticationDialog() { void InSessionAuthDialogControllerImpl::DestroyAuthenticationDialog() {
......
...@@ -41,6 +41,7 @@ class InSessionAuthDialogControllerImpl ...@@ -41,6 +41,7 @@ class InSessionAuthDialogControllerImpl
// InSessionAuthDialogController overrides // InSessionAuthDialogController overrides
void SetClient(InSessionAuthDialogClient* client) override; void SetClient(InSessionAuthDialogClient* client) override;
void ShowAuthenticationDialog(aura::Window* source_window, void ShowAuthenticationDialog(aura::Window* source_window,
const std::string& origin_name,
FinishCallback finish_callback) override; FinishCallback finish_callback) override;
void DestroyAuthenticationDialog() override; void DestroyAuthenticationDialog() override;
void AuthenticateUserWithPin(const std::string& pin, void AuthenticateUserWithPin(const std::string& pin,
...@@ -58,9 +59,11 @@ class InSessionAuthDialogControllerImpl ...@@ -58,9 +59,11 @@ class InSessionAuthDialogControllerImpl
void OnStartFingerprintAuthSession(AccountId account_id, void OnStartFingerprintAuthSession(AccountId account_id,
uint32_t auth_methods, uint32_t auth_methods,
aura::Window* source_window, aura::Window* source_window,
const std::string& origin_name,
bool success); bool success);
void OnPinCanAuthenticate(uint32_t auth_methods, void OnPinCanAuthenticate(uint32_t auth_methods,
aura::Window* source_window, aura::Window* source_window,
const std::string& origin_name,
bool pin_auth_available); bool pin_auth_available);
// Callback to execute when auth on ChromeOS side completes. // Callback to execute when auth on ChromeOS side completes.
......
...@@ -32,8 +32,9 @@ class ASH_PUBLIC_EXPORT InSessionAuthDialogController { ...@@ -32,8 +32,9 @@ class ASH_PUBLIC_EXPORT InSessionAuthDialogController {
// Sets the client that will handle authentication. // Sets the client that will handle authentication.
virtual void SetClient(InSessionAuthDialogClient* client) = 0; virtual void SetClient(InSessionAuthDialogClient* client) = 0;
// Displays the authentication dialog. // Displays the authentication dialog for the website/app name in |app_id|.
virtual void ShowAuthenticationDialog(aura::Window* source_window, virtual void ShowAuthenticationDialog(aura::Window* source_window,
const std::string& origin_name,
FinishCallback finish_callback) = 0; FinishCallback finish_callback) = 0;
// Destroys the authentication dialog. // Destroys the authentication dialog.
......
...@@ -41,6 +41,7 @@ class FakeInSessionAuthDialogController ...@@ -41,6 +41,7 @@ class FakeInSessionAuthDialogController
// ash::InSessionAuthDialogController: // ash::InSessionAuthDialogController:
void SetClient(ash::InSessionAuthDialogClient* client) override {} void SetClient(ash::InSessionAuthDialogClient* client) override {}
void ShowAuthenticationDialog(aura::Window* source_window, void ShowAuthenticationDialog(aura::Window* source_window,
const std::string& origin_name,
FinishCallback callback) override {} FinishCallback callback) override {}
void DestroyAuthenticationDialog() override {} void DestroyAuthenticationDialog() override {}
void AuthenticateUserWithPin(const std::string& pin, void AuthenticateUserWithPin(const std::string& pin,
......
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