From e62adbd0d3f9c2b8ef598e7043ebf02eec9b3a57 Mon Sep 17 00:00:00 2001 From: Matt Loft Date: Mon, 15 Aug 2022 15:01:25 +1000 Subject: [PATCH 1/5] fix: WebAuthn Discoverable Credential (Resident Credential) #33353 Enables support for Webauthn discoverable credentials (aka resident credentials). This allows users to authenticate without first having to select or type a username. To decide if discoverable credentials are supported, the class 'AuthenticatorCommon', in the chrome content code, indirectly calls the method 'context::WebAuthenticationDelegate.SupportsResidentKeys(..)'. The default implementation of this returns false, leaving it up to specific implementations to override. This change adds a new class 'ElectronWebAuthenticationDelegate' to subclass 'WebAuthenticationDelegate' and override the behaviour of the 'SupportsResidentKeys' method to return true. The implementation is copied from the Chrome browser equivalent 'ChromeWebAuthenticationDelegate', though the chrome class includes other methods that don't seem to be required for this functionality. The 'ElectronContentClient' class was also updated to store an instance of 'ElectronWebAuthenticationDelegate', and to provide an accessor method, GetWebAuthenticationDelegate(). --- filenames.gni | 2 + shell/browser/electron_browser_client.cc | 10 +++ shell/browser/electron_browser_client.h | 5 ++ ...electron_authenticator_request_delegate.cc | 23 +++++++ .../electron_authenticator_request_delegate.h | 64 +++++++++++++++++++ 5 files changed, 104 insertions(+) create mode 100644 shell/browser/webauthn/electron_authenticator_request_delegate.cc create mode 100644 shell/browser/webauthn/electron_authenticator_request_delegate.h diff --git a/filenames.gni b/filenames.gni index 9fda05b41f983..f6fade5495814 100644 --- a/filenames.gni +++ b/filenames.gni @@ -519,6 +519,8 @@ filenames = { "shell/browser/web_view_guest_delegate.h", "shell/browser/web_view_manager.cc", "shell/browser/web_view_manager.h", + "shell/browser/webauthn/electron_authenticator_request_delegate.cc", + "shell/browser/webauthn/electron_authenticator_request_delegate.h", "shell/browser/window_list.cc", "shell/browser/window_list.h", "shell/browser/window_list_observer.h", diff --git a/shell/browser/electron_browser_client.cc b/shell/browser/electron_browser_client.cc index ca46535c69055..d85bc5b67312f 100644 --- a/shell/browser/electron_browser_client.cc +++ b/shell/browser/electron_browser_client.cc @@ -103,6 +103,7 @@ #include "shell/browser/ui/devtools_manager_delegate.h" #include "shell/browser/web_contents_permission_helper.h" #include "shell/browser/web_contents_preferences.h" +#include "shell/browser/webauthn/electron_authenticator_request_delegate.h" #include "shell/browser/window_list.h" #include "shell/common/api/api.mojom.h" #include "shell/common/application_info.h" @@ -1841,4 +1842,13 @@ content::HidDelegate* ElectronBrowserClient::GetHidDelegate() { return hid_delegate_.get(); } +content::WebAuthenticationDelegate* +ElectronBrowserClient::GetWebAuthenticationDelegate() { + if (!web_authentication_delegate_) { + web_authentication_delegate_ = + std::make_unique(); + } + return web_authentication_delegate_.get(); +} + } // namespace electron diff --git a/shell/browser/electron_browser_client.h b/shell/browser/electron_browser_client.h index 597eb58360ef9..252c61bf150e5 100644 --- a/shell/browser/electron_browser_client.h +++ b/shell/browser/electron_browser_client.h @@ -38,6 +38,7 @@ namespace electron { class ElectronBrowserMainParts; class NotificationPresenter; class PlatformNotificationService; +class ElectronWebAuthenticationDelegate; class ElectronBrowserClient : public content::ContentBrowserClient, public content::RenderProcessHostObserver { @@ -102,6 +103,8 @@ class ElectronBrowserClient : public content::ContentBrowserClient, content::HidDelegate* GetHidDelegate() override; + content::WebAuthenticationDelegate* GetWebAuthenticationDelegate() override; + device::GeolocationManager* GetGeolocationManager() override; content::PlatformNotificationService* GetPlatformNotificationService(); @@ -330,6 +333,8 @@ class ElectronBrowserClient : public content::ContentBrowserClient, std::unique_ptr serial_delegate_; std::unique_ptr bluetooth_delegate_; std::unique_ptr hid_delegate_; + std::unique_ptr + web_authentication_delegate_; #if BUILDFLAG(IS_MAC) ElectronBrowserMainParts* browser_main_parts_ = nullptr; diff --git a/shell/browser/webauthn/electron_authenticator_request_delegate.cc b/shell/browser/webauthn/electron_authenticator_request_delegate.cc new file mode 100644 index 0000000000000..7f90c23a8d95b --- /dev/null +++ b/shell/browser/webauthn/electron_authenticator_request_delegate.cc @@ -0,0 +1,23 @@ +// Copyright 2018 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 "shell/browser/webauthn/electron_authenticator_request_delegate.h" + +namespace electron { +// --------------------------------------------------------------------- +// ElectronWebAuthenticationDelegate +// --------------------------------------------------------------------- + +ElectronWebAuthenticationDelegate::~ElectronWebAuthenticationDelegate() = + default; + +#if !BUILDFLAG(IS_ANDROID) +bool ElectronWebAuthenticationDelegate::SupportsResidentKeys( + content::RenderFrameHost* render_frame_host) { + return true; +} + +#endif // !IS_ANDROID + +} // namespace electron \ No newline at end of file diff --git a/shell/browser/webauthn/electron_authenticator_request_delegate.h b/shell/browser/webauthn/electron_authenticator_request_delegate.h new file mode 100644 index 0000000000000..b5d120c6feec0 --- /dev/null +++ b/shell/browser/webauthn/electron_authenticator_request_delegate.h @@ -0,0 +1,64 @@ +// Copyright 2018 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 ELECTRON_BROWSER_WEBAUTHN_CHROME_AUTHENTICATOR_REQUEST_DELEGATE_H_ +#define ELECTRON_BROWSER_WEBAUTHN_CHROME_AUTHENTICATOR_REQUEST_DELEGATE_H_ + +#include "content/public/browser/authenticator_request_client_delegate.h" + +namespace electron { +// ElectronWebAuthenticationDelegate is the //Electron layer implementation of +// content::WebAuthenticationDelegate. +class ElectronWebAuthenticationDelegate + : public content::WebAuthenticationDelegate { + public: +#if BUILDFLAG(IS_MAC) + // Returns a configuration struct for instantiating the macOS WebAuthn + // platform authenticator for the given Profile. + static TouchIdAuthenticatorConfig TouchIdAuthenticatorConfigForProfile( + Profile* profile); +#endif // BUILDFLAG(IS_MAC) + + ~ElectronWebAuthenticationDelegate() override; + +#if !BUILDFLAG(IS_ANDROID) + // content::WebAuthenticationDelegate: + // bool OverrideCallerOriginAndRelyingPartyIdValidation( + // content::BrowserContext* browser_context, + // const url::Origin& caller_origin, + // const std::string& relying_party_id) override; + // bool OriginMayUseRemoteDesktopClientOverride( + // content::BrowserContext* browser_context, + // const url::Origin& caller_origin) override; + // absl::optional MaybeGetRelyingPartyIdOverride( + // const std::string& claimed_relying_party_id, + // const url::Origin& caller_origin) override; + // bool ShouldPermitIndividualAttestation( + // content::BrowserContext* browser_context, + // const url::Origin& caller_origin, + // const std::string& relying_party_id) override; + bool SupportsResidentKeys( + content::RenderFrameHost* render_frame_host) override; + // bool IsFocused(content::WebContents* web_contents) override; + // absl::optional IsUserVerifyingPlatformAuthenticatorAvailableOverride( + // content::RenderFrameHost* render_frame_host) override; + // content::WebAuthenticationRequestProxy* MaybeGetRequestProxy( + // content::BrowserContext* browser_context) override; +#endif + // #if BUILDFLAG(IS_WIN) + // void OperationSucceeded(content::BrowserContext* browser_context, + // bool used_win_api) override; + // #endif + // #if BUILDFLAG(IS_MAC) + // absl::optional GetTouchIdAuthenticatorConfig( + // content::BrowserContext* browser_context) override; + // #endif // BUILDFLAG(IS_MAC) + // #if BUILDFLAG(IS_CHROMEOS) + // ChromeOSGenerateRequestIdCallback GetGenerateRequestIdCallback( + // content::RenderFrameHost* render_frame_host) override; + // #endif // BUILDFLAG(IS_CHROMEOS) +}; + +} // namespace electron +#endif \ No newline at end of file From cde30499fbad4979d88f8604de16d90fba683659 Mon Sep 17 00:00:00 2001 From: Matt Loft Date: Tue, 16 Aug 2022 10:54:45 +1000 Subject: [PATCH 2/5] Remove redundant, commented-out code --- .../electron_authenticator_request_delegate.h | 40 +------------------ 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/shell/browser/webauthn/electron_authenticator_request_delegate.h b/shell/browser/webauthn/electron_authenticator_request_delegate.h index b5d120c6feec0..b4ee725f27306 100644 --- a/shell/browser/webauthn/electron_authenticator_request_delegate.h +++ b/shell/browser/webauthn/electron_authenticator_request_delegate.h @@ -10,54 +10,16 @@ namespace electron { // ElectronWebAuthenticationDelegate is the //Electron layer implementation of // content::WebAuthenticationDelegate. +// Based on Chrome browser class ChromeWebAuthenticationDelegate class ElectronWebAuthenticationDelegate : public content::WebAuthenticationDelegate { public: -#if BUILDFLAG(IS_MAC) - // Returns a configuration struct for instantiating the macOS WebAuthn - // platform authenticator for the given Profile. - static TouchIdAuthenticatorConfig TouchIdAuthenticatorConfigForProfile( - Profile* profile); -#endif // BUILDFLAG(IS_MAC) - ~ElectronWebAuthenticationDelegate() override; #if !BUILDFLAG(IS_ANDROID) - // content::WebAuthenticationDelegate: - // bool OverrideCallerOriginAndRelyingPartyIdValidation( - // content::BrowserContext* browser_context, - // const url::Origin& caller_origin, - // const std::string& relying_party_id) override; - // bool OriginMayUseRemoteDesktopClientOverride( - // content::BrowserContext* browser_context, - // const url::Origin& caller_origin) override; - // absl::optional MaybeGetRelyingPartyIdOverride( - // const std::string& claimed_relying_party_id, - // const url::Origin& caller_origin) override; - // bool ShouldPermitIndividualAttestation( - // content::BrowserContext* browser_context, - // const url::Origin& caller_origin, - // const std::string& relying_party_id) override; bool SupportsResidentKeys( content::RenderFrameHost* render_frame_host) override; - // bool IsFocused(content::WebContents* web_contents) override; - // absl::optional IsUserVerifyingPlatformAuthenticatorAvailableOverride( - // content::RenderFrameHost* render_frame_host) override; - // content::WebAuthenticationRequestProxy* MaybeGetRequestProxy( - // content::BrowserContext* browser_context) override; #endif - // #if BUILDFLAG(IS_WIN) - // void OperationSucceeded(content::BrowserContext* browser_context, - // bool used_win_api) override; - // #endif - // #if BUILDFLAG(IS_MAC) - // absl::optional GetTouchIdAuthenticatorConfig( - // content::BrowserContext* browser_context) override; - // #endif // BUILDFLAG(IS_MAC) - // #if BUILDFLAG(IS_CHROMEOS) - // ChromeOSGenerateRequestIdCallback GetGenerateRequestIdCallback( - // content::RenderFrameHost* render_frame_host) override; - // #endif // BUILDFLAG(IS_CHROMEOS) }; } // namespace electron From c009f7f27a4bcd4b1f7d0055c0475be34e78301a Mon Sep 17 00:00:00 2001 From: Matt Loft Date: Tue, 16 Aug 2022 11:25:47 +1000 Subject: [PATCH 3/5] style: comment cleanup --- shell/browser/webauthn/electron_authenticator_request_delegate.h | 1 + 1 file changed, 1 insertion(+) diff --git a/shell/browser/webauthn/electron_authenticator_request_delegate.h b/shell/browser/webauthn/electron_authenticator_request_delegate.h index b4ee725f27306..df355fdfbf8b5 100644 --- a/shell/browser/webauthn/electron_authenticator_request_delegate.h +++ b/shell/browser/webauthn/electron_authenticator_request_delegate.h @@ -8,6 +8,7 @@ #include "content/public/browser/authenticator_request_client_delegate.h" namespace electron { + // ElectronWebAuthenticationDelegate is the //Electron layer implementation of // content::WebAuthenticationDelegate. // Based on Chrome browser class ChromeWebAuthenticationDelegate From 10e20e22926eb6020b7b7235e5448f04b44462e5 Mon Sep 17 00:00:00 2001 From: Matt Loft Date: Tue, 23 Aug 2022 17:43:32 +1000 Subject: [PATCH 4/5] style: updated comments and formatting based on pull request review --- .../electron_authenticator_request_delegate.cc | 12 +++--------- .../electron_authenticator_request_delegate.h | 16 ++++++---------- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/shell/browser/webauthn/electron_authenticator_request_delegate.cc b/shell/browser/webauthn/electron_authenticator_request_delegate.cc index 7f90c23a8d95b..598cfd98daf3d 100644 --- a/shell/browser/webauthn/electron_authenticator_request_delegate.cc +++ b/shell/browser/webauthn/electron_authenticator_request_delegate.cc @@ -1,23 +1,17 @@ -// Copyright 2018 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be +// Copyright (c) 2022 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be // found in the LICENSE file. #include "shell/browser/webauthn/electron_authenticator_request_delegate.h" namespace electron { -// --------------------------------------------------------------------- -// ElectronWebAuthenticationDelegate -// --------------------------------------------------------------------- ElectronWebAuthenticationDelegate::~ElectronWebAuthenticationDelegate() = default; -#if !BUILDFLAG(IS_ANDROID) bool ElectronWebAuthenticationDelegate::SupportsResidentKeys( content::RenderFrameHost* render_frame_host) { return true; } -#endif // !IS_ANDROID - -} // namespace electron \ No newline at end of file +} // namespace electron diff --git a/shell/browser/webauthn/electron_authenticator_request_delegate.h b/shell/browser/webauthn/electron_authenticator_request_delegate.h index df355fdfbf8b5..88aa05bbb1e8c 100644 --- a/shell/browser/webauthn/electron_authenticator_request_delegate.h +++ b/shell/browser/webauthn/electron_authenticator_request_delegate.h @@ -1,27 +1,23 @@ -// Copyright 2018 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be +// Copyright (c) 2022 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be // found in the LICENSE file. -#ifndef ELECTRON_BROWSER_WEBAUTHN_CHROME_AUTHENTICATOR_REQUEST_DELEGATE_H_ -#define ELECTRON_BROWSER_WEBAUTHN_CHROME_AUTHENTICATOR_REQUEST_DELEGATE_H_ +#ifndef ELECTRON_BROWSER_WEBAUTHN_AUTHENTICATOR_REQUEST_DELEGATE_H_ +#define ELECTRON_BROWSER_WEBAUTHN_AUTHENTICATOR_REQUEST_DELEGATE_H_ #include "content/public/browser/authenticator_request_client_delegate.h" namespace electron { -// ElectronWebAuthenticationDelegate is the //Electron layer implementation of -// content::WebAuthenticationDelegate. -// Based on Chrome browser class ChromeWebAuthenticationDelegate +// Modified from chrome/browser/webauthn/chrome_authenticator_request_delegate.h class ElectronWebAuthenticationDelegate : public content::WebAuthenticationDelegate { public: ~ElectronWebAuthenticationDelegate() override; -#if !BUILDFLAG(IS_ANDROID) bool SupportsResidentKeys( content::RenderFrameHost* render_frame_host) override; -#endif }; } // namespace electron -#endif \ No newline at end of file +#endif From 6b0deb092ffaa0ec429fc3b27728ecf4969dec1a Mon Sep 17 00:00:00 2001 From: Matt Loft Date: Wed, 24 Aug 2022 22:21:06 +1000 Subject: [PATCH 5/5] style: fix lint error on header guard clause --- .../webauthn/electron_authenticator_request_delegate.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shell/browser/webauthn/electron_authenticator_request_delegate.h b/shell/browser/webauthn/electron_authenticator_request_delegate.h index 88aa05bbb1e8c..217e7b4667aa4 100644 --- a/shell/browser/webauthn/electron_authenticator_request_delegate.h +++ b/shell/browser/webauthn/electron_authenticator_request_delegate.h @@ -2,8 +2,8 @@ // Use of this source code is governed by the MIT license that can be // found in the LICENSE file. -#ifndef ELECTRON_BROWSER_WEBAUTHN_AUTHENTICATOR_REQUEST_DELEGATE_H_ -#define ELECTRON_BROWSER_WEBAUTHN_AUTHENTICATOR_REQUEST_DELEGATE_H_ +#ifndef ELECTRON_SHELL_BROWSER_WEBAUTHN_ELECTRON_AUTHENTICATOR_REQUEST_DELEGATE_H_ +#define ELECTRON_SHELL_BROWSER_WEBAUTHN_ELECTRON_AUTHENTICATOR_REQUEST_DELEGATE_H_ #include "content/public/browser/authenticator_request_client_delegate.h" @@ -20,4 +20,4 @@ class ElectronWebAuthenticationDelegate }; } // namespace electron -#endif +#endif // ELECTRON_SHELL_BROWSER_WEBAUTHN_ELECTRON_AUTHENTICATOR_REQUEST_DELEGATE_H_