Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some webui that doesn't allowed in private window is loaded in private window #1295

Merged
merged 1 commit into from
Feb 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 66 additions & 2 deletions browser/brave_scheme_load_browsertest.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
/* Copyright (c) 2019 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

Expand All @@ -9,14 +10,16 @@
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_list.h"
#include "chrome/browser/ui/browser_list_observer.h"
#include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/common/url_constants.h"
#include "content/public/test/browser_test_utils.h"
#include "net/dns/mock_host_resolver.h"

class BraveSchemeLoadBrowserTest : public InProcessBrowserTest,
public BrowserListObserver {
public BrowserListObserver,
public TabStripModelObserver {
public:
void SetUpOnMainThread() override {
InProcessBrowserTest::SetUpOnMainThread();
Expand All @@ -33,6 +36,16 @@ class BraveSchemeLoadBrowserTest : public InProcessBrowserTest,
// BrowserListObserver overrides:
void OnBrowserAdded(Browser* browser) override { popup_ = browser; }

// TabStripModelObserver overrides:
void OnTabStripModelChanged(
TabStripModel* tab_strip_model,
const TabStripModelChange& change,
const TabStripSelectionChange& selection) override {
if (change.type() == TabStripModelChange::kInserted) {
WaitForLoadStop(active_contents());
}
}

content::WebContents* active_contents() {
return browser()->tab_strip_model()->GetActiveWebContents();
}
Expand All @@ -44,6 +57,35 @@ class BraveSchemeLoadBrowserTest : public InProcessBrowserTest,
return WaitForLoadStop(active_contents());
}

// Check loading |url| in private window is redirected to normal
// window.
void TestURLIsNotLoadedInPrivateWindow(const GURL& url) {
Browser* private_browser = CreateIncognitoBrowser(nullptr);
TabStripModel* private_model = private_browser->tab_strip_model();

// Check normal & private window have one blank tab.
EXPECT_EQ("about:blank",
private_model->GetActiveWebContents()->GetVisibleURL().spec());
EXPECT_EQ(1, private_model->count());
EXPECT_EQ("about:blank", active_contents()->GetVisibleURL().spec());
EXPECT_EQ(1, browser()->tab_strip_model()->count());

browser()->tab_strip_model()->AddObserver(this);

// Load url to private window.
NavigateParams params(private_browser, url, ui::PAGE_TRANSITION_TYPED);
Navigate(&params);

browser()->tab_strip_model()->RemoveObserver(this);

EXPECT_EQ(url, active_contents()->GetVisibleURL());
EXPECT_EQ(2, browser()->tab_strip_model()->count());
// Private window stays as initial state.
EXPECT_EQ("about:blank",
private_model->GetActiveWebContents()->GetVisibleURL().spec());
EXPECT_EQ(1, private_browser->tab_strip_model()->count());
}

Browser* popup_ = nullptr;
};

Expand Down Expand Up @@ -180,3 +222,25 @@ IN_PROC_BROWSER_TEST_F(BraveSchemeLoadBrowserTest,
console_delegate.message(),
"Not allowed to load local resource: brave://settings"));
}

// Some webuis are not allowed to load in private window.
// Allowed url list are checked by IsURLAllowedInIncognito().
// So, corresponding brave scheme url should be filtered as chrome scheme.
// Ex, brave://settings should be loaded only in normal window because
// chrome://settings is not allowed. When tyring to loading brave://settings in
// private window, it should be loaded in normal window instead of private
// window.
IN_PROC_BROWSER_TEST_F(BraveSchemeLoadBrowserTest,
SettingsPageIsNotAllowedInPrivateWindow) {
TestURLIsNotLoadedInPrivateWindow(GURL("brave://settings/"));
}

IN_PROC_BROWSER_TEST_F(BraveSchemeLoadBrowserTest,
SyncPageIsNotAllowedInPrivateWindow) {
TestURLIsNotLoadedInPrivateWindow(GURL("brave://sync/"));
}

IN_PROC_BROWSER_TEST_F(BraveSchemeLoadBrowserTest,
RewardsPageIsNotAllowedInPrivateWindow) {
TestURLIsNotLoadedInPrivateWindow(GURL("brave://rewards/"));
}
22 changes: 21 additions & 1 deletion chromium_src/chrome/browser/ui/browser_navigator.cc
Original file line number Diff line number Diff line change
@@ -1,8 +1,28 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
/* Copyright (c) 2019 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/browser/renderer_host/brave_navigation_ui_data.h"
#include "brave/common/webui_url_constants.h"
#include "chrome/common/webui_url_constants.h"
#include "url/gurl.h"

namespace {
bool IsHostAllowedInIncognitoBraveImpl(std::string* scheme,
const base::StringPiece& host) {
if (*scheme == "brave")
*scheme = content::kChromeUIScheme;

if (host == kRewardsHost ||
host == kBraveUISyncHost ||
host == chrome::kChromeUISyncInternalsHost) {
return false;
}

return true;
}
}

#define ChromeNavigationUIData BraveNavigationUIData
#include "../../../../chrome/browser/ui/browser_navigator.cc"
Expand Down
14 changes: 14 additions & 0 deletions patches/chrome-browser-ui-browser_navigator.cc.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
diff --git a/chrome/browser/ui/browser_navigator.cc b/chrome/browser/ui/browser_navigator.cc
index 57cc96c276c1106079828b5e359a3d88e1c901ce..32d1e9d68577760d78345630204b30ae0bb1bfca 100644
--- a/chrome/browser/ui/browser_navigator.cc
+++ b/chrome/browser/ui/browser_navigator.cc
@@ -700,6 +700,9 @@ void Navigate(NavigateParams* params) {
bool IsHostAllowedInIncognito(const GURL& url) {
std::string scheme = url.scheme();
base::StringPiece host = url.host_piece();
+#if defined(BRAVE_CHROMIUM_BUILD)
+ if (!IsHostAllowedInIncognitoBraveImpl(&scheme, host)) return false;
+#endif
if (scheme == chrome::kChromeSearchScheme) {
return host != chrome::kChromeUIThumbnailHost &&
host != chrome::kChromeUIThumbnailHost2 &&