Commit 0c4fdc59 authored by Balazs Engedy's avatar Balazs Engedy Committed by Commit Bot

Display relying party ID on WebAuthn UI.

Also, fix the plumbing of |rp_id| out of FidoRequestHandlers into
TransportAvailabilityInfo, and add unittests to verify correctness.

Bug: 866601, 849323
Change-Id: I0e8c1250208c52306501fab7b9c55a2b1a34bbd1
Reviewed-on: https://chromium-review.googlesource.com/1181265
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Reviewed-by: default avatarBalazs Engedy <engedy@chromium.org>
Reviewed-by: default avatarJun Choi <hongjunchoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584526}
parent 4852a5e2
...@@ -100,9 +100,8 @@ class AuthenticatorDialogViewTest : public DialogBrowserTest { ...@@ -100,9 +100,8 @@ class AuthenticatorDialogViewTest : public DialogBrowserTest {
browser()->tab_strip_model()->GetActiveWebContents(); browser()->tab_strip_model()->GetActiveWebContents();
auto dialog_model = std::make_unique<AuthenticatorRequestDialogModel>(); auto dialog_model = std::make_unique<AuthenticatorRequestDialogModel>();
dialog_model->StartFlow( dialog_model->SetCurrentStep(
::device::FidoRequestHandlerBase::TransportAvailabilityInfo(), AuthenticatorRequestDialogModel::Step::kErrorTimedOut);
base::nullopt);
auto dialog = std::make_unique<AuthenticatorRequestDialogView>( auto dialog = std::make_unique<AuthenticatorRequestDialogView>(
web_contents, std::move(dialog_model)); web_contents, std::move(dialog_model));
......
...@@ -19,9 +19,10 @@ class AuthenticatorDialogTest : public DialogBrowserTest { ...@@ -19,9 +19,10 @@ class AuthenticatorDialogTest : public DialogBrowserTest {
// DialogBrowserTest: // DialogBrowserTest:
void ShowUi(const std::string& name) override { void ShowUi(const std::string& name) override {
auto model = std::make_unique<AuthenticatorRequestDialogModel>(); auto model = std::make_unique<AuthenticatorRequestDialogModel>();
model->StartFlow( ::device::FidoRequestHandlerBase::TransportAvailabilityInfo
::device::FidoRequestHandlerBase::TransportAvailabilityInfo(), transport_availability;
base::nullopt); transport_availability.rp_id = "example.com";
model->StartFlow(std::move(transport_availability), base::nullopt);
// The dialog should immediately close as soon as it is displayed. // The dialog should immediately close as soon as it is displayed.
if (name == "completed") { if (name == "completed") {
......
...@@ -76,6 +76,11 @@ void AuthenticatorSheetModelBase::OnCancel() { ...@@ -76,6 +76,11 @@ void AuthenticatorSheetModelBase::OnCancel() {
dialog_model()->Cancel(); dialog_model()->Cancel();
} }
base::string16 AuthenticatorSheetModelBase::GetRelyingPartyIdString() const {
DCHECK(!dialog_model()->transport_availability()->rp_id.empty());
return base::UTF8ToUTF16(dialog_model()->transport_availability()->rp_id);
}
void AuthenticatorSheetModelBase::OnModelDestroyed() { void AuthenticatorSheetModelBase::OnModelDestroyed() {
dialog_model_ = nullptr; dialog_model_ = nullptr;
} }
...@@ -87,11 +92,8 @@ gfx::ImageSkia* AuthenticatorWelcomeSheetModel::GetStepIllustration() const { ...@@ -87,11 +92,8 @@ gfx::ImageSkia* AuthenticatorWelcomeSheetModel::GetStepIllustration() const {
} }
base::string16 AuthenticatorWelcomeSheetModel::GetStepTitle() const { base::string16 AuthenticatorWelcomeSheetModel::GetStepTitle() const {
// TODO(hongjunchoi): Insert actual domain name from model to
// |application_name|.
base::string16 application_name = base::UTF8ToUTF16("example.com");
return l10n_util::GetStringFUTF16(IDS_WEBAUTHN_WELCOME_SCREEN_TITLE, return l10n_util::GetStringFUTF16(IDS_WEBAUTHN_WELCOME_SCREEN_TITLE,
application_name); GetRelyingPartyIdString());
} }
base::string16 AuthenticatorWelcomeSheetModel::GetStepDescription() const { base::string16 AuthenticatorWelcomeSheetModel::GetStepDescription() const {
...@@ -123,11 +125,8 @@ gfx::ImageSkia* AuthenticatorTransportSelectorSheetModel::GetStepIllustration() ...@@ -123,11 +125,8 @@ gfx::ImageSkia* AuthenticatorTransportSelectorSheetModel::GetStepIllustration()
} }
base::string16 AuthenticatorTransportSelectorSheetModel::GetStepTitle() const { base::string16 AuthenticatorTransportSelectorSheetModel::GetStepTitle() const {
// TODO(hongjunchoi): Insert actual domain name from model to
// |application_name|.
base::string16 application_name = base::UTF8ToUTF16("example.com");
return l10n_util::GetStringFUTF16(IDS_WEBAUTHN_TRANSPORT_SELECTION_TITLE, return l10n_util::GetStringFUTF16(IDS_WEBAUTHN_TRANSPORT_SELECTION_TITLE,
application_name); GetRelyingPartyIdString());
} }
base::string16 AuthenticatorTransportSelectorSheetModel::GetStepDescription() base::string16 AuthenticatorTransportSelectorSheetModel::GetStepDescription()
...@@ -150,11 +149,8 @@ AuthenticatorInsertAndActivateUsbSheetModel::GetStepIllustration() const { ...@@ -150,11 +149,8 @@ AuthenticatorInsertAndActivateUsbSheetModel::GetStepIllustration() const {
base::string16 AuthenticatorInsertAndActivateUsbSheetModel::GetStepTitle() base::string16 AuthenticatorInsertAndActivateUsbSheetModel::GetStepTitle()
const { const {
// TODO(hongjunchoi): Insert actual domain name from model to
// |application_name|.
base::string16 application_name = base::UTF8ToUTF16("example.com");
return l10n_util::GetStringFUTF16(IDS_WEBAUTHN_GENERIC_TITLE, return l10n_util::GetStringFUTF16(IDS_WEBAUTHN_GENERIC_TITLE,
application_name); GetRelyingPartyIdString());
} }
base::string16 AuthenticatorInsertAndActivateUsbSheetModel::GetStepDescription() base::string16 AuthenticatorInsertAndActivateUsbSheetModel::GetStepDescription()
...@@ -279,10 +275,8 @@ gfx::ImageSkia* AuthenticatorBlePinEntrySheetModel::GetStepIllustration() ...@@ -279,10 +275,8 @@ gfx::ImageSkia* AuthenticatorBlePinEntrySheetModel::GetStepIllustration()
} }
base::string16 AuthenticatorBlePinEntrySheetModel::GetStepTitle() const { base::string16 AuthenticatorBlePinEntrySheetModel::GetStepTitle() const {
// TODO(hongjunchoi): Insert actual device name from model to |device_name|.
base::string16 device_name = base::UTF8ToUTF16("VHGSHSSN");
return l10n_util::GetStringFUTF16(IDS_WEBAUTHN_BLE_PIN_ENTRY_TITLE, return l10n_util::GetStringFUTF16(IDS_WEBAUTHN_BLE_PIN_ENTRY_TITLE,
device_name); GetRelyingPartyIdString());
} }
base::string16 AuthenticatorBlePinEntrySheetModel::GetStepDescription() const { base::string16 AuthenticatorBlePinEntrySheetModel::GetStepDescription() const {
...@@ -325,11 +319,8 @@ gfx::ImageSkia* AuthenticatorBleActivateSheetModel::GetStepIllustration() ...@@ -325,11 +319,8 @@ gfx::ImageSkia* AuthenticatorBleActivateSheetModel::GetStepIllustration()
} }
base::string16 AuthenticatorBleActivateSheetModel::GetStepTitle() const { base::string16 AuthenticatorBleActivateSheetModel::GetStepTitle() const {
// TODO(hongjunchoi): Insert actual domain name from model to
// |application_name|.
base::string16 application_name = base::UTF8ToUTF16("example.com");
return l10n_util::GetStringFUTF16(IDS_WEBAUTHN_GENERIC_TITLE, return l10n_util::GetStringFUTF16(IDS_WEBAUTHN_GENERIC_TITLE,
application_name); GetRelyingPartyIdString());
} }
base::string16 AuthenticatorBleActivateSheetModel::GetStepDescription() const { base::string16 AuthenticatorBleActivateSheetModel::GetStepDescription() const {
...@@ -349,11 +340,8 @@ gfx::ImageSkia* AuthenticatorTouchIdSheetModel::GetStepIllustration() const { ...@@ -349,11 +340,8 @@ gfx::ImageSkia* AuthenticatorTouchIdSheetModel::GetStepIllustration() const {
base::string16 AuthenticatorTouchIdSheetModel::GetStepTitle() const { base::string16 AuthenticatorTouchIdSheetModel::GetStepTitle() const {
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
// TODO(martinkr): Insert actual domain name from model to
// |application_name|.
base::string16 application_name = base::UTF8ToUTF16("example.com");
return l10n_util::GetStringFUTF16(IDS_WEBAUTHN_TOUCH_ID_TITLE, return l10n_util::GetStringFUTF16(IDS_WEBAUTHN_TOUCH_ID_TITLE,
application_name); GetRelyingPartyIdString());
#else #else
return base::string16(); return base::string16();
#endif // defined(OS_MACOSX) #endif // defined(OS_MACOSX)
......
...@@ -29,6 +29,9 @@ class AuthenticatorSheetModelBase ...@@ -29,6 +29,9 @@ class AuthenticatorSheetModelBase
// loads it in Skia format. // loads it in Skia format.
static gfx::ImageSkia* GetImage(int resource_id); static gfx::ImageSkia* GetImage(int resource_id);
// Returns a string containing the relying party id for this request.
base::string16 GetRelyingPartyIdString() const;
// AuthenticatorRequestSheetModel: // AuthenticatorRequestSheetModel:
bool IsBackButtonVisible() const override; bool IsBackButtonVisible() const override;
bool IsCancelButtonVisible() const override; bool IsCancelButtonVisible() const override;
......
...@@ -97,6 +97,9 @@ class AuthenticatorRequestDialogModel { ...@@ -97,6 +97,9 @@ class AuthenticatorRequestDialogModel {
Step current_step() const { return current_step_; } Step current_step() const { return current_step_; }
TransportListModel* transport_list_model() { return &transport_list_model_; } TransportListModel* transport_list_model() { return &transport_list_model_; }
const TransportAvailabilityInfo* transport_availability() const {
return &transport_availability_;
}
// Starts the UX flow, by either showing the welcome screen, the transport // Starts the UX flow, by either showing the welcome screen, the transport
// selection screen, or the guided flow for them most likely transport. // selection screen, or the guided flow for them most likely transport.
......
...@@ -162,6 +162,17 @@ class FidoGetAssertionHandlerTest : public ::testing::Test { ...@@ -162,6 +162,17 @@ class FidoGetAssertionHandlerTest : public ::testing::Test {
GetTestableTransportProtocols(); GetTestableTransportProtocols();
}; };
TEST_F(FidoGetAssertionHandlerTest, TransportAvailabilityInfo) {
auto request_handler =
CreateGetAssertionHandlerWithRequest(CtapGetAssertionRequest(
test_data::kRelyingPartyId, test_data::kClientDataHash));
EXPECT_EQ(FidoRequestHandlerBase::RequestType::kGetAssertion,
request_handler->transport_availability_info().request_type);
EXPECT_EQ(test_data::kRelyingPartyId,
request_handler->transport_availability_info().rp_id);
}
TEST_F(FidoGetAssertionHandlerTest, CtapRequestOnSingleDevice) { TEST_F(FidoGetAssertionHandlerTest, CtapRequestOnSingleDevice) {
auto request_handler = CreateGetAssertionHandlerCtap(); auto request_handler = CreateGetAssertionHandlerCtap();
discovery()->WaitForCallToStartAndSimulateSuccess(); discovery()->WaitForCallToStartAndSimulateSuccess();
......
...@@ -159,7 +159,7 @@ GetAssertionRequestHandler::GetAssertionRequestHandler( ...@@ -159,7 +159,7 @@ GetAssertionRequestHandler::GetAssertionRequestHandler(
std::move(completion_callback)), std::move(completion_callback)),
request_(std::move(request)), request_(std::move(request)),
weak_factory_(this) { weak_factory_(this) {
transport_availability_info().rp_id = request.rp_id(); transport_availability_info().rp_id = request_.rp_id();
transport_availability_info().request_type = transport_availability_info().request_type =
FidoRequestHandlerBase::RequestType::kGetAssertion; FidoRequestHandlerBase::RequestType::kGetAssertion;
......
...@@ -161,6 +161,15 @@ class FidoMakeCredentialHandlerTest : public ::testing::Test { ...@@ -161,6 +161,15 @@ class FidoMakeCredentialHandlerTest : public ::testing::Test {
GetTestableTransportProtocols(); GetTestableTransportProtocols();
}; };
TEST_F(FidoMakeCredentialHandlerTest, TransportAvailabilityInfo) {
auto request_handler = CreateMakeCredentialHandler();
EXPECT_EQ(FidoRequestHandlerBase::RequestType::kMakeCredential,
request_handler->transport_availability_info().request_type);
EXPECT_EQ(test_data::kRelyingPartyId,
request_handler->transport_availability_info().rp_id);
}
TEST_F(FidoMakeCredentialHandlerTest, TestCtap2MakeCredentialWithFlagEnabled) { TEST_F(FidoMakeCredentialHandlerTest, TestCtap2MakeCredentialWithFlagEnabled) {
auto request_handler = CreateMakeCredentialHandler(); auto request_handler = CreateMakeCredentialHandler();
discovery()->WaitForCallToStartAndSimulateSuccess(); discovery()->WaitForCallToStartAndSimulateSuccess();
......
...@@ -108,7 +108,7 @@ MakeCredentialRequestHandler::MakeCredentialRequestHandler( ...@@ -108,7 +108,7 @@ MakeCredentialRequestHandler::MakeCredentialRequestHandler(
authenticator_selection_criteria_( authenticator_selection_criteria_(
std::move(authenticator_selection_criteria)), std::move(authenticator_selection_criteria)),
weak_factory_(this) { weak_factory_(this) {
transport_availability_info().rp_id = request.rp().rp_id(); transport_availability_info().rp_id = request_parameter_.rp().rp_id();
transport_availability_info().request_type = transport_availability_info().request_type =
FidoRequestHandlerBase::RequestType::kMakeCredential; FidoRequestHandlerBase::RequestType::kMakeCredential;
Start(); Start();
......
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