Skip to content

Commit

Permalink
Fix all brave pages are loaded in private window
Browse files Browse the repository at this point in the history
Some brave pages should not be loaded in private window such as
brave://settings.
Instead, they should be redirected to normal window.
Also sync/rewards host are added to not allowed list.
  • Loading branch information
simonhong committed Feb 5, 2019
1 parent 5da1242 commit 205f727
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 3 deletions.
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 &&

0 comments on commit 205f727

Please sign in to comment.