Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(113)

Unified Diff: extensions/browser/guest_view/web_view/web_view_guest.cc

Issue 890183002: Allow Signin page to open other chrome:// URLs if login content in <webview> (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Addressed comments Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « extensions/browser/guest_view/web_view/web_view_guest.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: extensions/browser/guest_view/web_view/web_view_guest.cc
diff --git a/extensions/browser/guest_view/web_view/web_view_guest.cc b/extensions/browser/guest_view/web_view/web_view_guest.cc
index fce1b6d0dd7301210ed877a2f3b7f2794649e4fe..eacef42e6fe5200bde185713101f503d660a4bc4 100644
--- a/extensions/browser/guest_view/web_view/web_view_guest.cc
+++ b/extensions/browser/guest_view/web_view/web_view_guest.cc
@@ -875,35 +875,9 @@ void WebViewGuest::NavigateGuest(const std::string& src,
GURL url = ResolveURL(src);
- // Do not allow navigating a guest to schemes other than known safe schemes.
- // This will block the embedder trying to load unwanted schemes, e.g.
- // chrome://settings.
- bool scheme_is_blocked =
- (!content::ChildProcessSecurityPolicy::GetInstance()->IsWebSafeScheme(
- url.scheme()) &&
- !url.SchemeIs(url::kAboutScheme)) ||
- url.SchemeIs(url::kJavaScriptScheme);
- if (scheme_is_blocked || !url.is_valid()) {
- LoadAbort(true /* is_top_level */, url,
- net::ErrorToShortString(net::ERR_ABORTED));
- NavigateGuest(url::kAboutBlankURL, true /* force_navigation */);
- return;
- }
- if (!force_navigation && (src_ == url))
- return;
-
- GURL validated_url(url);
- web_contents()->GetRenderProcessHost()->FilterURL(false, &validated_url);
- // As guests do not swap processes on navigation, only navigations to
- // normal web URLs are supported. No protocol handlers are installed for
- // other schemes (e.g., WebUI or extensions), and no permissions or bindings
- // can be granted to the guest process.
- LoadURLWithParams(validated_url,
- content::Referrer(),
+ LoadURLWithParams(url, content::Referrer(),
ui::PAGE_TRANSITION_AUTO_TOPLEVEL,
- web_contents());
-
- src_ = validated_url;
+ force_navigation);
}
bool WebViewGuest::HandleKeyboardShortcuts(
@@ -1118,6 +1092,23 @@ void WebViewGuest::AddNewContents(content::WebContents* source,
content::WebContents* WebViewGuest::OpenURLFromTab(
content::WebContents* source,
const content::OpenURLParams& params) {
+ // There are two use cases to consider from a security perspective:
+ // 1.) Renderer-initiated navigation to chrome:// must always be blocked even
+ // if the <webview> is in WebUI. This is handled by
+ // WebViewGuest::LoadURLWithParams. WebViewGuest::NavigateGuest will also
+ // call LoadURLWithParams. CreateNewGuestWebViewWindow creates a new
+ // WebViewGuest which will call NavigateGuest in DidInitialize.
+ // 2.) The Language Settings context menu item should always work, both in
+ // Chrome Apps and WebUI. This is a browser initiated request and so
+ // we pass it along to the embedder's WebContentsDelegate to get the
+ // browser to perform the action for the <webview>.
+ if (!params.is_renderer_initiated) {
+ if (!owner_web_contents()->GetDelegate())
+ return nullptr;
+ return owner_web_contents()->GetDelegate()->OpenURLFromTab(
+ owner_web_contents(), params);
+ }
+
// If the guest wishes to navigate away prior to attachment then we save the
// navigation to perform upon attachment. Navigation initializes a lot of
// state that assumes an embedder exists, such as RenderWidgetHostViewGuest.
@@ -1134,12 +1125,25 @@ content::WebContents* WebViewGuest::OpenURLFromTab(
it->second = new_window_info;
return nullptr;
}
+
+ // This code path is taken if RenderFrameImpl::DecidePolicyForNavigation
+ // decides that a fork should happen. At the time of writing this comment,
+ // the only way a well behaving guest could hit this code path is if it
+ // navigates to a URL that's associated with the default search engine.
+ // This list of URLs is generated by chrome::GetSearchURLs. Validity checks
+ // are performed inside LoadURLWithParams such that if the guest attempts
+ // to navigate to a URL that it is not allowed to navigate to, a 'loadabort'
+ // event will fire in the embedder, and the guest will be navigated to
+ // about:blank.
if (params.disposition == CURRENT_TAB) {
- // This can happen for cross-site redirects.
- LoadURLWithParams(params.url, params.referrer, params.transition, source);
- return source;
+ LoadURLWithParams(params.url, params.referrer, params.transition,
+ true /* force_navigation */);
+ return web_contents();
}
+ // This code path is taken if Ctrl+Click, middle click or any of the
+ // keyboard/mouse combinations are used to open a link in a new tab/window.
+ // This code path is also taken on client-side redirects from about:blank.
CreateNewGuestWebViewWindow(params);
return nullptr;
}
@@ -1161,8 +1165,32 @@ void WebViewGuest::WebContentsCreated(WebContents* source_contents,
void WebViewGuest::LoadURLWithParams(const GURL& url,
const content::Referrer& referrer,
ui::PageTransition transition_type,
- content::WebContents* web_contents) {
- content::NavigationController::LoadURLParams load_url_params(url);
+ bool force_navigation) {
+ // Do not allow navigating a guest to schemes other than known safe schemes.
+ // This will block the embedder trying to load unwanted schemes, e.g.
+ // chrome://.
+ bool scheme_is_blocked =
+ (!content::ChildProcessSecurityPolicy::GetInstance()->IsWebSafeScheme(
+ url.scheme()) &&
+ !url.SchemeIs(url::kAboutScheme)) ||
+ url.SchemeIs(url::kJavaScriptScheme);
+ if (scheme_is_blocked || !url.is_valid()) {
+ LoadAbort(true /* is_top_level */, url,
+ net::ErrorToShortString(net::ERR_ABORTED));
+ NavigateGuest(url::kAboutBlankURL, true /* force_navigation */);
+ return;
+ }
+
+ if (!force_navigation && (src_ == url))
+ return;
+
+ GURL validated_url(url);
+ web_contents()->GetRenderProcessHost()->FilterURL(false, &validated_url);
+ // As guests do not swap processes on navigation, only navigations to
+ // normal web URLs are supported. No protocol handlers are installed for
+ // other schemes (e.g., WebUI or extensions), and no permissions or bindings
+ // can be granted to the guest process.
+ content::NavigationController::LoadURLParams load_url_params(validated_url);
load_url_params.referrer = referrer;
load_url_params.transition_type = transition_type;
load_url_params.extra_headers = std::string();
@@ -1170,7 +1198,9 @@ void WebViewGuest::LoadURLWithParams(const GURL& url,
load_url_params.override_user_agent =
content::NavigationController::UA_OVERRIDE_TRUE;
}
- web_contents->GetController().LoadURLWithParams(load_url_params);
+ web_contents()->GetController().LoadURLWithParams(load_url_params);
+
+ src_ = validated_url;
}
void WebViewGuest::RequestNewWindowPermission(
« no previous file with comments | « extensions/browser/guest_view/web_view/web_view_guest.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698