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

Add origin-based permission lifetime support (uplift to 1.24.x) #8585

Merged
merged 1 commit into from
Apr 23, 2021
Merged
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
6 changes: 6 additions & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion browser/ephemeral_storage/ephemeral_storage_tab_helper.cc
Original file line number Diff line number Diff line change
@@ -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;
2 changes: 1 addition & 1 deletion browser/ephemeral_storage/ephemeral_storage_tab_helper.h
Original file line number Diff line number Diff line change
@@ -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>;
195 changes: 189 additions & 6 deletions browser/permissions/permission_lifetime_manager_browsertest.cc
Original file line number Diff line number Diff line change
@@ -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"
@@ -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());
@@ -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);
@@ -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);
@@ -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);
@@ -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
@@ -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*
@@ -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()
Loading