Skip to content

Commit

Permalink
Merge pull request #8378 from /issues/14126-2
Browse files Browse the repository at this point in the history
Add origin-based permission lifetime support
  • Loading branch information
goodov authored Apr 13, 2021
2 parents 3a78d66 + 58370b1 commit 72c9338
Show file tree
Hide file tree
Showing 23 changed files with 1,143 additions and 174 deletions.
6 changes: 6 additions & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ components/weekly_storage/ @iefremov
# Perf predictor
components/brave_perf_predictor/ @iefremov

# Permissions
browser/permissions/**/*expiration* @goodov
browser/permissions/**/*lifetime* @goodov
components/permissions/**/*expiration* @goodov
components/permissions/**/*lifetime* @goodov

# Java patching
build/android/bytecode/ @samartnik
patches/*.java.patch @samartnik
Expand Down
2 changes: 1 addition & 1 deletion browser/ephemeral_storage/ephemeral_storage_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ void EphemeralStorageTabHelper::ReadyToCommitNavigation(
}

void EphemeralStorageTabHelper::CreateEphemeralStorageAreasForDomainAndURL(
std::string new_domain,
const std::string& new_domain,
const GURL& new_url) {
if (new_url.is_empty())
return;
Expand Down
2 changes: 1 addition & 1 deletion browser/ephemeral_storage/ephemeral_storage_tab_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class EphemeralStorageTabHelper
content::NavigationHandle* navigation_handle) override;

private:
void CreateEphemeralStorageAreasForDomainAndURL(std::string new_domain,
void CreateEphemeralStorageAreasForDomainAndURL(const std::string& new_domain,
const GURL& new_url);

friend class content::WebContentsUserData<EphemeralStorageTabHelper>;
Expand Down
195 changes: 189 additions & 6 deletions browser/permissions/permission_lifetime_manager_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@
#include "chrome/test/base/ui_test_utils.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"
#include "components/content_settings/core/common/content_settings_types.h"
#include "components/network_session_configurator/common/network_switches.h"
#include "components/permissions/features.h"
#include "components/permissions/request_type.h"
#include "components/prefs/pref_service.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "net/base/features.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "url/gurl.h"
Expand All @@ -49,22 +51,31 @@ const char kPreTestDataFileName[] = "pre_test_data";

class PermissionLifetimeManagerBrowserTest : public InProcessBrowserTest {
public:
PermissionLifetimeManagerBrowserTest() {
PermissionLifetimeManagerBrowserTest()
: https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {
scoped_feature_list_.InitAndEnableFeature(features::kPermissionLifetime);
}

~PermissionLifetimeManagerBrowserTest() override = default;

void SetUpCommandLine(base::CommandLine* command_line) override {
InProcessBrowserTest::SetUpCommandLine(command_line);
command_line->AppendSwitch(switches::kIgnoreCertificateErrors);
}

void SetUpOnMainThread() override {
PermissionRequestManager* manager = GetPermissionRequestManager();
prompt_factory_.reset(new MockPermissionLifetimePromptFactory(manager));

host_resolver()->AddRule("*", "127.0.0.1");
ASSERT_TRUE(embedded_test_server()->Start());
https_server()->ServeFilesFromDirectory(GetChromeTestDataDir());
ASSERT_TRUE(https_server()->Start());
}

void TearDownOnMainThread() override { prompt_factory_.reset(); }

net::EmbeddedTestServer* https_server() { return &https_server_; }

PermissionRequestManager* GetPermissionRequestManager() {
return PermissionRequestManager::FromWebContents(
browser()->tab_strip_model()->GetActiveWebContents());
Expand Down Expand Up @@ -120,12 +131,13 @@ class PermissionLifetimeManagerBrowserTest : public InProcessBrowserTest {

protected:
base::test::ScopedFeatureList scoped_feature_list_;
net::test_server::EmbeddedTestServer https_server_;
std::unique_ptr<MockPermissionLifetimePromptFactory> prompt_factory_;
base::Value pre_test_data_{base::Value::Type::DICTIONARY};
};

IN_PROC_BROWSER_TEST_F(PermissionLifetimeManagerBrowserTest, ExpirationSmoke) {
const GURL& url = embedded_test_server()->GetURL("/empty.html");
const GURL& url = https_server()->GetURL("/empty.html");
ui_test_utils::NavigateToURL(browser(), url);
prompt_factory_->set_response_type(
PermissionRequestManager::AutoResponseType::ACCEPT_ALL);
Expand Down Expand Up @@ -169,7 +181,7 @@ IN_PROC_BROWSER_TEST_F(PermissionLifetimeManagerBrowserTest, ExpirationSmoke) {

IN_PROC_BROWSER_TEST_F(PermissionLifetimeManagerBrowserTest,
PRE_PermissionExpiredAfterRestart) {
const GURL& url = embedded_test_server()->GetURL("/empty.html");
const GURL& url = https_server()->GetURL("/empty.html");
ui_test_utils::NavigateToURL(browser(), url);
prompt_factory_->set_response_type(
PermissionRequestManager::AutoResponseType::ACCEPT_ALL);
Expand Down Expand Up @@ -227,7 +239,7 @@ IN_PROC_BROWSER_TEST_F(PermissionLifetimeManagerBrowserTest,

IN_PROC_BROWSER_TEST_F(PermissionLifetimeManagerBrowserTest,
ExpirationRemovedAfterManualReset) {
const GURL& url = embedded_test_server()->GetURL("/empty.html");
const GURL& url = https_server()->GetURL("/empty.html");
ui_test_utils::NavigateToURL(browser(), url);
prompt_factory_->set_response_type(
PermissionRequestManager::AutoResponseType::ACCEPT_ALL);
Expand Down Expand Up @@ -257,4 +269,175 @@ IN_PROC_BROWSER_TEST_F(PermissionLifetimeManagerBrowserTest,
EXPECT_TRUE(GetExpirationsPrefValue()->DictEmpty());
}

} // namespace permissions
class PermissionLifetimeManagerWithOriginMonitorBrowserTest
: public PermissionLifetimeManagerBrowserTest {
public:
PermissionLifetimeManagerWithOriginMonitorBrowserTest() {
scoped_feature_list_.InitAndEnableFeature(
net::features::kBraveEphemeralStorage);
}

protected:
base::test::ScopedFeatureList scoped_feature_list_;
};

IN_PROC_BROWSER_TEST_F(PermissionLifetimeManagerWithOriginMonitorBrowserTest,
DomainPermissionReset) {
const GURL& url = https_server()->GetURL("host.com", "/empty.html");
ui_test_utils::NavigateToURL(browser(), url);
prompt_factory_->set_response_type(
PermissionRequestManager::AutoResponseType::ACCEPT_ALL);

EXPECT_CALL(*prompt_factory_, OnPermissionPromptCreated(_))
.WillOnce(testing::Invoke([](MockPermissionLifetimePrompt* prompt) {
prompt->delegate()->Requests()[0]->SetLifetime(base::TimeDelta());
}));
content::ExecuteScriptAsync(
GetActiveMainFrame(),
"navigator.geolocation.getCurrentPosition(function(){});");
prompt_factory_->WaitForPermissionBubble();

EXPECT_EQ(1, prompt_factory_->show_count());
EXPECT_FALSE(permission_lifetime_timer().IsRunning());
EXPECT_FALSE(GetExpirationsPrefValue()->DictEmpty());

EXPECT_EQ(host_content_settings_map()->GetContentSetting(
url, url, ContentSettingsType::GEOLOCATION),
ContentSetting::CONTENT_SETTING_ALLOW);

// Navigate to another domain. It should reset the permission.
const GURL& other_url =
https_server()->GetURL("other_host.com", "/empty.html");
ui_test_utils::NavigateToURL(browser(), other_url);
EXPECT_EQ(host_content_settings_map()->GetContentSetting(
url, url, ContentSettingsType::GEOLOCATION),
ContentSetting::CONTENT_SETTING_ASK);
EXPECT_TRUE(GetExpirationsPrefValue()->DictEmpty());
}

IN_PROC_BROWSER_TEST_F(PermissionLifetimeManagerWithOriginMonitorBrowserTest,
FriendlyDomainPermissionKept) {
const GURL& url = https_server()->GetURL("example.com", "/empty.html");
ui_test_utils::NavigateToURL(browser(), url);
prompt_factory_->set_response_type(
PermissionRequestManager::AutoResponseType::ACCEPT_ALL);

EXPECT_CALL(*prompt_factory_, OnPermissionPromptCreated(_))
.WillOnce(testing::Invoke([](MockPermissionLifetimePrompt* prompt) {
prompt->delegate()->Requests()[0]->SetLifetime(base::TimeDelta());
}));
content::ExecuteScriptAsync(
GetActiveMainFrame(),
"navigator.geolocation.getCurrentPosition(function(){});");
prompt_factory_->WaitForPermissionBubble();

EXPECT_EQ(1, prompt_factory_->show_count());
EXPECT_FALSE(permission_lifetime_timer().IsRunning());
EXPECT_FALSE(GetExpirationsPrefValue()->DictEmpty());

EXPECT_EQ(host_content_settings_map()->GetContentSetting(
url, url, ContentSettingsType::GEOLOCATION),
ContentSetting::CONTENT_SETTING_ALLOW);

// Navigate to a subdomain, permission should be kept.
const GURL& sub_url =
https_server()->GetURL("sub.example.com", "/empty.html");
ui_test_utils::NavigateToURL(browser(), sub_url);
EXPECT_EQ(host_content_settings_map()->GetContentSetting(
url, url, ContentSettingsType::GEOLOCATION),
ContentSetting::CONTENT_SETTING_ALLOW);
EXPECT_FALSE(GetExpirationsPrefValue()->DictEmpty());

// Navigate to another domain. It should reset the permission.
const GURL& other_url =
https_server()->GetURL("other_host.com", "/empty.html");
ui_test_utils::NavigateToURL(browser(), other_url);
EXPECT_EQ(host_content_settings_map()->GetContentSetting(
url, url, ContentSettingsType::GEOLOCATION),
ContentSetting::CONTENT_SETTING_ASK);
EXPECT_TRUE(GetExpirationsPrefValue()->DictEmpty());
}

IN_PROC_BROWSER_TEST_F(PermissionLifetimeManagerWithOriginMonitorBrowserTest,
PublicSuffixListDomainPermissionReset) {
const GURL& url = https_server()->GetURL("user.github.io", "/empty.html");
ui_test_utils::NavigateToURL(browser(), url);
prompt_factory_->set_response_type(
PermissionRequestManager::AutoResponseType::ACCEPT_ALL);

EXPECT_CALL(*prompt_factory_, OnPermissionPromptCreated(_))
.WillOnce(testing::Invoke([](MockPermissionLifetimePrompt* prompt) {
prompt->delegate()->Requests()[0]->SetLifetime(base::TimeDelta());
}));
content::ExecuteScriptAsync(
GetActiveMainFrame(),
"navigator.geolocation.getCurrentPosition(function(){});");
prompt_factory_->WaitForPermissionBubble();

EXPECT_EQ(1, prompt_factory_->show_count());
EXPECT_FALSE(permission_lifetime_timer().IsRunning());
EXPECT_FALSE(GetExpirationsPrefValue()->DictEmpty());

EXPECT_EQ(host_content_settings_map()->GetContentSetting(
url, url, ContentSettingsType::GEOLOCATION),
ContentSetting::CONTENT_SETTING_ALLOW);

// Navigate to a subdomain, permission should be kept.
const GURL& sub_url =
https_server()->GetURL("sub.user.github.io", "/empty.html");
ui_test_utils::NavigateToURL(browser(), sub_url);
EXPECT_EQ(host_content_settings_map()->GetContentSetting(
url, url, ContentSettingsType::GEOLOCATION),
ContentSetting::CONTENT_SETTING_ALLOW);
EXPECT_FALSE(GetExpirationsPrefValue()->DictEmpty());

// Navigate to another domain in PSL. It should reset the permission.
const GURL& other_url =
https_server()->GetURL("user2.github.io", "/empty.html");
ui_test_utils::NavigateToURL(browser(), other_url);
EXPECT_EQ(host_content_settings_map()->GetContentSetting(
url, url, ContentSettingsType::GEOLOCATION),
ContentSetting::CONTENT_SETTING_ASK);
EXPECT_TRUE(GetExpirationsPrefValue()->DictEmpty());
}

IN_PROC_BROWSER_TEST_F(PermissionLifetimeManagerWithOriginMonitorBrowserTest,
PRE_DomainPermissionResetAfterRestart) {
const GURL& url = https_server()->GetURL("example.com", "/empty.html");
ui_test_utils::NavigateToURL(browser(), url);
prompt_factory_->set_response_type(
PermissionRequestManager::AutoResponseType::ACCEPT_ALL);

EXPECT_CALL(*prompt_factory_, OnPermissionPromptCreated(_))
.WillOnce(testing::Invoke([](MockPermissionLifetimePrompt* prompt) {
prompt->delegate()->Requests()[0]->SetLifetime(base::TimeDelta());
}));
content::ExecuteScriptAsync(
GetActiveMainFrame(),
"navigator.geolocation.getCurrentPosition(function(){});");
prompt_factory_->WaitForPermissionBubble();

EXPECT_EQ(1, prompt_factory_->show_count());
EXPECT_FALSE(permission_lifetime_timer().IsRunning());
EXPECT_FALSE(GetExpirationsPrefValue()->DictEmpty());

EXPECT_EQ(host_content_settings_map()->GetContentSetting(
url, url, ContentSettingsType::GEOLOCATION),
ContentSetting::CONTENT_SETTING_ALLOW);

pre_test_data_.SetStringKey("url", url.spec());
WritePreTestData();
}

IN_PROC_BROWSER_TEST_F(PermissionLifetimeManagerWithOriginMonitorBrowserTest,
DomainPermissionResetAfterRestart) {
ReadPreTestData();
const GURL url(*pre_test_data_.FindStringKey("url"));

EXPECT_EQ(host_content_settings_map()->GetContentSetting(
url, url, ContentSettingsType::GEOLOCATION),
ContentSetting::CONTENT_SETTING_ASK);
EXPECT_TRUE(GetExpirationsPrefValue()->DictEmpty());
}

} // namespace permissions
15 changes: 14 additions & 1 deletion browser/permissions/permission_lifetime_manager_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,18 @@

#include "brave/browser/permissions/permission_lifetime_manager_factory.h"

#include <memory>
#include <utility>

#include "brave/components/permissions/permission_lifetime_manager.h"
#include "brave/components/permissions/permission_origin_lifetime_monitor_impl.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/permissions/permission_manager_factory.h"
#include "chrome/browser/profiles/incognito_helpers.h"
#include "chrome/browser/profiles/profile.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/permissions/features.h"
#include "net/base/features.h"

// static
permissions::PermissionLifetimeManager*
Expand Down Expand Up @@ -43,10 +48,18 @@ KeyedService* PermissionLifetimeManagerFactory::BuildServiceInstanceFor(
permissions::features::kPermissionLifetime)) {
return nullptr;
}
std::unique_ptr<permissions::PermissionOriginLifetimeMonitor>
permission_origin_lifetime_monitor;
if (base::FeatureList::IsEnabled(net::features::kBraveEphemeralStorage)) {
permission_origin_lifetime_monitor =
std::make_unique<permissions::PermissionOriginLifetimeMonitorImpl>(
context);
}
auto* profile = Profile::FromBrowserContext(context);
return new permissions::PermissionLifetimeManager(
HostContentSettingsMapFactory::GetForProfile(context),
profile->IsOffTheRecord() ? nullptr : profile->GetPrefs());
profile->IsOffTheRecord() ? nullptr : profile->GetPrefs(),
std::move(permission_origin_lifetime_monitor));
}

bool PermissionLifetimeManagerFactory::ServiceIsCreatedWithBrowserContext()
Expand Down
Loading

0 comments on commit 72c9338

Please sign in to comment.