From 34e8d0f2f75e691cdeec6fee0ded7d36e8fc3595 Mon Sep 17 00:00:00 2001 From: mkarolin Date: Fri, 5 Jun 2020 14:49:37 -0400 Subject: [PATCH] Content Settings updated to Utilize Time/Scope Restrictions. Chromium change: https://chromium.googlesource.com/chromium/src/+/f62a1294a4b54626abf1275e47057e5564a7bb3a commit f62a1294a4b54626abf1275e47057e5564a7bb3a Author: Brandon Maslen Date: Wed Apr 29 07:37:10 2020 +0000 Update Content Settings to Utilize Time/Scope Restrictions to Get/Set Settings. Currently all content/website settings are not bounded or scoped aside from possibly being ephemeral and held in memory only. In order to support scenarios for the Storage Access API and future permission time-boxing efforts the concept of scoping settings is needed. This change introduces the notion of a ContentSettingConstraints struct that can be optionally provided when setting a content or website setting. The constraint consists of an optional base::Time to specify the expiration that a setting should use,as well as a SessionModel to specify the lifetime model of the setting. The following initial SessionModel have been specified; however, future options such as TabOnly may be added to accommodate permissions scoping: Durable: Settings persist forever and are bounded only by an expiry date, if set. Will be persisted to disk to survive session restarts. UserSession: Settings will persist no longer than a user session regardless of expiry date, if set. All types of scopes may be constrained by an upper bound duration. This allows settings to expire after a set period of time has passed. The majority of this new behaviour is contained within the HostContentSettingsMap and associated Providers and supporting classes. New constraint parameters have been added as optional to setters and unless opted in by new callers will behave as they did prior to this change. The getters have been updated so that a direct retrieval of a setting will only return a value if a currently un-expired setting exists. If an array of values is retrieved for future use like with GetSettingsForOneType the expiry date and convenience method IsExpired() have been added to ensure consumers can accurately determine if a setting is relevant. In order to cleanup expired Durable entries we will validate the expiration time on entries, if set, when we read our settings from prefs. If present any expired entries will be cleaned up at this time. New test cases have been added to validate the HCSM behaviour changes as well as the PrefProvider and supporting classes. In addition to this the first consumer of these changes has been added in the Storage Access API permissions context code. This ensures that the API can both set either a UserSession or Durable grant with an associated duration as well as determine the number of existing grants that are UserSession only as required. Bug: 989663 --- .../brave_browsing_data_remover_delegate.cc | 2 +- ...ave_content_settings_ephemeral_provider.cc | 5 ++- ...rave_content_settings_ephemeral_provider.h | 12 +++--- .../brave_content_settings_pref_provider.cc | 43 ++++++++++--------- .../brave_content_settings_pref_provider.h | 12 +++--- ...content_settings_pref_provider_unittest.cc | 4 +- 6 files changed, 42 insertions(+), 36 deletions(-) diff --git a/browser/browsing_data/brave_browsing_data_remover_delegate.cc b/browser/browsing_data/brave_browsing_data_remover_delegate.cc index b6a7828661c..959faead37c 100644 --- a/browser/browsing_data/brave_browsing_data_remover_delegate.cc +++ b/browser/browsing_data/brave_browsing_data_remover_delegate.cc @@ -66,7 +66,7 @@ void BraveBrowsingDataRemoverDelegate::ClearShieldsSettings( (last_modified < end_time || end_time.is_null())) { provider->SetWebsiteSetting(setting.primary_pattern, setting.secondary_pattern, content_type, - resource_id, nullptr); + resource_id, nullptr, {}); } } } diff --git a/components/content_settings/core/browser/brave_content_settings_ephemeral_provider.cc b/components/content_settings/core/browser/brave_content_settings_ephemeral_provider.cc index ecae97834bc..d701c527970 100644 --- a/components/content_settings/core/browser/brave_content_settings_ephemeral_provider.cc +++ b/components/content_settings/core/browser/brave_content_settings_ephemeral_provider.cc @@ -17,7 +17,8 @@ bool BraveEphemeralProvider::SetWebsiteSetting( const ContentSettingsPattern& secondary_pattern, ContentSettingsType content_type, const ResourceIdentifier& resource_identifier, - std::unique_ptr&& in_value) { + std::unique_ptr&& in_value, + const ContentSettingConstraints& constraint) { // Prevent this handle shields configuration. if (content_type == ContentSettingsType::PLUGINS && IsShieldsResourceID(resource_identifier)) { @@ -29,7 +30,7 @@ bool BraveEphemeralProvider::SetWebsiteSetting( return EphemeralProvider::SetWebsiteSetting( primary_pattern, secondary_pattern, content_type, resource_identifier, - std::move(in_value)); + std::move(in_value), constraint); } } // namespace content_settings diff --git a/components/content_settings/core/browser/brave_content_settings_ephemeral_provider.h b/components/content_settings/core/browser/brave_content_settings_ephemeral_provider.h index c63f16550ca..b805c9e6bb1 100644 --- a/components/content_settings/core/browser/brave_content_settings_ephemeral_provider.h +++ b/components/content_settings/core/browser/brave_content_settings_ephemeral_provider.h @@ -20,11 +20,13 @@ class BraveEphemeralProvider : public EphemeralProvider { private: // EphemeralProvider overrides: - bool SetWebsiteSetting(const ContentSettingsPattern& primary_pattern, - const ContentSettingsPattern& secondary_pattern, - ContentSettingsType content_type, - const ResourceIdentifier& resource_identifier, - std::unique_ptr&& value) override; + bool SetWebsiteSetting( + const ContentSettingsPattern& primary_pattern, + const ContentSettingsPattern& secondary_pattern, + ContentSettingsType content_type, + const ResourceIdentifier& resource_identifier, + std::unique_ptr&& value, + const ContentSettingConstraints& constraints = {}) override; DISALLOW_COPY_AND_ASSIGN(BraveEphemeralProvider); }; diff --git a/components/content_settings/core/browser/brave_content_settings_pref_provider.cc b/components/content_settings/core/browser/brave_content_settings_pref_provider.cc index 90f282f47eb..ee81539f6a6 100644 --- a/components/content_settings/core/browser/brave_content_settings_pref_provider.cc +++ b/components/content_settings/core/browser/brave_content_settings_pref_provider.cc @@ -54,9 +54,8 @@ Rule CloneRule(const Rule& rule, bool reverse_patterns = false) { } } - return Rule(primary_pattern, - secondary_pattern, - rule.value.Clone()); + return Rule(primary_pattern, secondary_pattern, rule.value.Clone(), + rule.expiration, rule.session_model); } class BraveShieldsRuleIterator : public RuleIterator { @@ -206,7 +205,7 @@ void BravePrefProvider::MigrateShieldsSettingsV1ToV2ForOneType( new_rules.emplace_back( new_primary_pattern.value_or(rule.primary_pattern), new_secondary_pattern.value_or(rule.secondary_pattern), - rule.value.Clone()); + rule.value.Clone(), rule.expiration, rule.session_model); } } rule_iterator.reset(); @@ -217,12 +216,13 @@ void BravePrefProvider::MigrateShieldsSettingsV1ToV2ForOneType( // Remove current setting. PrefProvider::SetWebsiteSetting( old_rules[i].first, old_rules[i].second, content_type, resource_id, - ContentSettingToValue(CONTENT_SETTING_DEFAULT)); + ContentSettingToValue(CONTENT_SETTING_DEFAULT), {}); // Add new setting. PrefProvider::SetWebsiteSetting( new_rules[i].primary_pattern, new_rules[i].secondary_pattern, content_type, resource_id, - ContentSettingToValue(ValueToContentSetting(&(new_rules[i].value)))); + ContentSettingToValue(ValueToContentSetting(&(new_rules[i].value))), + {new_rules[i].expiration, new_rules[i].session_model}); } } @@ -231,7 +231,8 @@ bool BravePrefProvider::SetWebsiteSetting( const ContentSettingsPattern& secondary_pattern, ContentSettingsType content_type, const ResourceIdentifier& resource_identifier, - std::unique_ptr&& in_value) { + std::unique_ptr&& in_value, + const ContentSettingConstraints& constraints) { // Flash's setting shouldn't be reached here. // Its content type is plugin and id is empty string. // One excpetion is default setting. It can be persisted. @@ -264,17 +265,16 @@ bool BravePrefProvider::SetWebsiteSetting( } // change to type PLUGINS - return PrefProvider::SetWebsiteSetting(plugin_primary_pattern, - plugin_secondary_pattern, - ContentSettingsType::PLUGINS, - brave_shields::kCookies, - std::move(in_value)); + return PrefProvider::SetWebsiteSetting( + plugin_primary_pattern, plugin_secondary_pattern, + ContentSettingsType::PLUGINS, brave_shields::kCookies, + std::move(in_value), constraints); } } return PrefProvider::SetWebsiteSetting(primary_pattern, secondary_pattern, content_type, resource_identifier, - std::move(in_value)); + std::move(in_value), constraints); } std::unique_ptr BravePrefProvider::GetRuleIterator( @@ -325,7 +325,8 @@ void BravePrefProvider::UpdateCookieRules(ContentSettingsType content_type, ContentSettingsPattern::FromString(kGoogleAuthPattern), ContentSettingsPattern::Wildcard(), base::Value::FromUniquePtrValue( - ContentSettingToValue(CONTENT_SETTING_ALLOW))); + ContentSettingToValue(CONTENT_SETTING_ALLOW)), + base::Time(), SessionModel::Durable); rules.emplace_back(CloneRule(google_auth_rule)); brave_cookie_rules_[incognito].emplace_back(CloneRule(google_auth_rule)); @@ -333,7 +334,8 @@ void BravePrefProvider::UpdateCookieRules(ContentSettingsType content_type, ContentSettingsPattern::FromString(kFirebasePattern), ContentSettingsPattern::Wildcard(), base::Value::FromUniquePtrValue( - ContentSettingToValue(CONTENT_SETTING_ALLOW))); + ContentSettingToValue(CONTENT_SETTING_ALLOW)), + base::Time(), SessionModel::Durable); rules.emplace_back(CloneRule(firebase_rule)); brave_cookie_rules_[incognito].emplace_back(CloneRule(firebase_rule)); } @@ -390,12 +392,14 @@ void BravePrefProvider::UpdateCookieRules(ContentSettingsType content_type, Rule(ContentSettingsPattern::Wildcard(), shield_rule.primary_pattern, base::Value::FromUniquePtrValue( - ContentSettingToValue(CONTENT_SETTING_ALLOW)))); + ContentSettingToValue(CONTENT_SETTING_ALLOW)), + base::Time(), SessionModel::Durable)); brave_cookie_rules_[incognito].emplace_back( Rule(ContentSettingsPattern::Wildcard(), shield_rule.primary_pattern, base::Value::FromUniquePtrValue( - ContentSettingToValue(CONTENT_SETTING_ALLOW)))); + ContentSettingToValue(CONTENT_SETTING_ALLOW)), + base::Time(), SessionModel::Durable)); } } @@ -431,9 +435,8 @@ void BravePrefProvider::UpdateCookieRules(ContentSettingsType content_type, }); if (match == brave_cookie_rules_[incognito].end()) { brave_cookie_updates.emplace_back( - Rule(old_rule.primary_pattern, - old_rule.secondary_pattern, - base::Value())); + Rule(old_rule.primary_pattern, old_rule.secondary_pattern, + base::Value(), old_rule.expiration, old_rule.session_model)); } } diff --git a/components/content_settings/core/browser/brave_content_settings_pref_provider.h b/components/content_settings/core/browser/brave_content_settings_pref_provider.h index d0f30d1de52..af09c9136fd 100644 --- a/components/content_settings/core/browser/brave_content_settings_pref_provider.h +++ b/components/content_settings/core/browser/brave_content_settings_pref_provider.h @@ -37,12 +37,12 @@ class BravePrefProvider : public PrefProvider, // content_settings::PrefProvider overrides: void ShutdownOnUIThread() override; - bool SetWebsiteSetting( - const ContentSettingsPattern& primary_pattern, - const ContentSettingsPattern& secondary_pattern, - ContentSettingsType content_type, - const ResourceIdentifier& resource_identifier, - std::unique_ptr&& value) override; + bool SetWebsiteSetting(const ContentSettingsPattern& primary_pattern, + const ContentSettingsPattern& secondary_pattern, + ContentSettingsType content_type, + const ResourceIdentifier& resource_identifier, + std::unique_ptr&& value, + const ContentSettingConstraints& constraints) override; std::unique_ptr GetRuleIterator( ContentSettingsType content_type, const ResourceIdentifier& resource_identifier, diff --git a/components/content_settings/core/browser/brave_content_settings_pref_provider_unittest.cc b/components/content_settings/core/browser/brave_content_settings_pref_provider_unittest.cc index ab207f4f9e7..83fd41a811b 100644 --- a/components/content_settings/core/browser/brave_content_settings_pref_provider_unittest.cc +++ b/components/content_settings/core/browser/brave_content_settings_pref_provider_unittest.cc @@ -50,7 +50,7 @@ class ShieldsSetting { provider_->SetWebsiteSetting( pattern, SecondaryUrlToPattern(url_source.first), ContentSettingsType::PLUGINS, url_source.second, - ContentSettingToValue(setting)); + ContentSettingToValue(setting), {}); } } @@ -134,7 +134,7 @@ class ShieldsScriptSetting : public ShieldsSetting { ContentSetting setting) override { provider_->SetWebsiteSetting(pattern, ContentSettingsPattern::Wildcard(), ContentSettingsType::JAVASCRIPT, "", - ContentSettingToValue(setting)); + ContentSettingToValue(setting), {}); } private: