[Native File System] Consider write guard setting

This change updates the Native File System API permissions model to
consider the write guard content setting when determining permissions.
The write guard determines whether sites are allowed to ask for
permission to save to the original files selected by the user through
the Native File System API.

Bug: 985101
Change-Id: I3ee844ee634ed55b93a6b1e08cd9b8bdad67239b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1707430Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Kamila Hasanbega <hkamila@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Ovidio de Jesús Ruiz-Henríquez <odejesush@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683447}

chrome/android/javatests/src/org/chromium/chrome/browser/preferences/website/WebsitePermissionsFetcherTest.java

@@ -292,7 +292,7 @@ public class WebsitePermissionsFetcherTest {
         // If the CONTENT_SETTINGS_NUM_TYPES value changes *and* a new value has been exposed on
         // Android, then please update this code block to include a test for your new type.
         // Otherwise, just update count in the assert.
-        Assert.assertEquals(52, ContentSettingsType.CONTENT_SETTINGS_NUM_TYPES);
+        Assert.assertEquals(53, ContentSettingsType.CONTENT_SETTINGS_NUM_TYPES);
         websitePreferenceBridge.addContentSettingException(
                 new ContentSettingException(ContentSettingsType.CONTENT_SETTINGS_TYPE_COOKIES,
                         googleOrigin, ContentSettingValues.DEFAULT, preferenceSource));

chrome/browser/native_file_system/chrome_native_file_system_permission_context.h

@@ -6,12 +6,17 @@
 #define CHROME_BROWSER_NATIVE_FILE_SYSTEM_CHROME_NATIVE_FILE_SYSTEM_PERMISSION_CONTEXT_H_
 
 #include <map>
+#include <vector>
 
 #include "base/sequence_checker.h"
+#include "chrome/browser/permissions/permission_util.h"
+#include "components/content_settings/core/common/content_settings_types.h"
 #include "components/keyed_service/core/refcounted_keyed_service.h"
 #include "content/public/browser/native_file_system_permission_context.h"
 #include "third_party/blink/public/mojom/permissions/permission_status.mojom.h"
 
+class HostContentSettingsMap;
+
 namespace content {
 class BrowserContext;
 }  // namespace content
@@ -40,6 +45,85 @@ class ChromeNativeFileSystemPermissionContext
   explicit ChromeNativeFileSystemPermissionContext(
       content::BrowserContext* context);
 
+  class WritePermissionGrantImpl
+      : public content::NativeFileSystemPermissionGrant {
+   public:
+    // In the current implementation permission grants are scoped to the frame
+    // they are requested in. Within a frame we only want to have one grant per
+    // path. The Key struct contains these fields. Keys are comparable so they
+    // can be used with sorted containers like std::map and std::set.
+    // TODO(https://crbug.com/984769): Eliminate process_id and frame_id and
+    // replace usage of this struct with just a file path when grants stop being
+    // scoped to a frame.
+    struct Key {
+      base::FilePath path;
+      int process_id = 0;
+      int frame_id = 0;
+
+      bool operator==(const Key& rhs) const;
+      bool operator<(const Key& rhs) const;
+    };
+
+    WritePermissionGrantImpl(
+        scoped_refptr<ChromeNativeFileSystemPermissionContext> context,
+        ContentSettingsType content_settings_guard_type,
+        const url::Origin& origin,
+        const Key& key,
+        bool is_directory);
+
+    // content::NativeFileSystemPermissionGrant implementation:
+    PermissionStatus GetStatus() override;
+    void RequestPermission(int process_id,
+                           int frame_id,
+                           base::OnceClosure callback) override;
+
+    const url::Origin& origin() const {
+      DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+      return origin_;
+    }
+
+    bool is_directory() const {
+      DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+      return is_directory_;
+    }
+
+    const base::FilePath& path() const {
+      DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+      return key_.path;
+    }
+
+    const Key& key() const {
+      DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+      return key_;
+    }
+
+    // Returns true if the |content_setting_guard_type_| has not been blocked.
+    bool CanRequestPermission();
+
+    void SetStatus(PermissionStatus status);
+
+   protected:
+    ~WritePermissionGrantImpl() override;
+
+   private:
+    void OnPermissionRequestComplete(base::OnceClosure callback,
+                                     PermissionAction result);
+
+    SEQUENCE_CHECKER(sequence_checker_);
+
+    scoped_refptr<ChromeNativeFileSystemPermissionContext> const context_;
+    const ContentSettingsType write_guard_content_setting_type_;
+    const url::Origin origin_;
+    const Key key_;
+    const bool is_directory_;
+
+    // This member should only be updated via SetStatus(), to make sure
+    // observers are properly notified about any change in status.
+    PermissionStatus status_ = PermissionStatus::ASK;
+
+    DISALLOW_COPY_AND_ASSIGN(WritePermissionGrantImpl);
+  };
+
   // content::NativeFileSystemPermissionContext:
   scoped_refptr<content::NativeFileSystemPermissionGrant>
   GetReadPermissionGrant(const url::Origin& origin,
@@ -47,6 +131,8 @@ class ChromeNativeFileSystemPermissionContext
                          bool is_directory,
                          int process_id,
                          int frame_id) override;
+
+  bool CanRequestWritePermission(WritePermissionGrantImpl* grant);
   scoped_refptr<content::NativeFileSystemPermissionGrant>
   GetWritePermissionGrant(const url::Origin& origin,
                           const base::FilePath& path,
@@ -111,12 +197,13 @@ class ChromeNativeFileSystemPermissionContext
   // RefcountedKeyedService:
   void ShutdownOnUIThread() override;
 
+  HostContentSettingsMap* content_settings() { return content_settings_.get(); }
+
  private:
   // Destructor is private because this class is refcounted.
   ~ChromeNativeFileSystemPermissionContext() override;
 
-  class PermissionGrantImpl;
-  void PermissionGrantDestroyed(PermissionGrantImpl* grant);
+  void PermissionGrantDestroyed(WritePermissionGrantImpl* grant);
 
   void DidConfirmSensitiveDirectoryAccess(
       const url::Origin& origin,
@@ -132,6 +219,8 @@ class ChromeNativeFileSystemPermissionContext
   struct OriginState;
   std::map<url::Origin, OriginState> origins_;
 
+  scoped_refptr<HostContentSettingsMap> content_settings_;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeNativeFileSystemPermissionContext);
 };
 

chrome/browser/native_file_system/native_file_system_permission_context_factory.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/native_file_system/native_file_system_permission_context_factory.h"
 
 #include "base/no_destructor.h"
+#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/native_file_system/chrome_native_file_system_permission_context.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
@@ -36,7 +37,9 @@ NativeFileSystemPermissionContextFactory::
     NativeFileSystemPermissionContextFactory()
     : RefcountedBrowserContextKeyedServiceFactory(
           "NativeFileSystemPermissionContext",
-          BrowserContextDependencyManager::GetInstance()) {}
+          BrowserContextDependencyManager::GetInstance()) {
+  DependsOn(HostContentSettingsMapFactory::GetInstance());
+}
 
 NativeFileSystemPermissionContextFactory::
     ~NativeFileSystemPermissionContextFactory() = default;

chrome/browser/ui/webui/site_settings_helper.cc

@@ -90,6 +90,8 @@ const ContentSettingsTypeNameEntry kContentSettingsTypeGroupNames[] = {
     {CONTENT_SETTINGS_TYPE_BLUETOOTH_SCANNING, "bluetooth-scanning"},
     {CONTENT_SETTINGS_TYPE_HID_GUARD, "hid-devices"},
     {CONTENT_SETTINGS_TYPE_HID_CHOOSER_DATA, kHidChooserDataGroupType},
+    {CONTENT_SETTINGS_TYPE_NATIVE_FILE_SYSTEM_WRITE_GUARD,
+     "native-file-system-write"},
 
     // Add new content settings here if a corresponding Javascript string
     // representation for it is not required. Note some exceptions do have UI in

components/content_settings/core/browser/content_settings_registry.cc

@@ -491,6 +491,16 @@ void ContentSettingsRegistry::Init() {
            ContentSettingsInfo::INHERIT_IF_LESS_PERMISSIVE,
            ContentSettingsInfo::PERSISTENT,
            ContentSettingsInfo::EXCEPTIONS_ON_SECURE_ORIGINS_ONLY);
+
+  Register(CONTENT_SETTINGS_TYPE_NATIVE_FILE_SYSTEM_WRITE_GUARD,
+           "native-file-system-write-guard", CONTENT_SETTING_ASK,
+           WebsiteSettingsInfo::UNSYNCABLE, WhitelistedSchemes(),
+           ValidSettings(CONTENT_SETTING_ASK, CONTENT_SETTING_BLOCK),
+           WebsiteSettingsInfo::SINGLE_ORIGIN_ONLY_SCOPE,
+           WebsiteSettingsRegistry::DESKTOP,
+           ContentSettingsInfo::INHERIT_IF_LESS_PERMISSIVE,
+           ContentSettingsInfo::PERSISTENT,
+           ContentSettingsInfo::EXCEPTIONS_ON_SECURE_ORIGINS_ONLY);
 }
 
 void ContentSettingsRegistry::Register(

components/content_settings/core/common/content_settings.cc

@@ -78,6 +78,7 @@ constexpr HistogramValue kHistogramValue[] = {
     {CONTENT_SETTINGS_TYPE_WAKE_LOCK_SCREEN, 54},
     {CONTENT_SETTINGS_TYPE_WAKE_LOCK_SYSTEM, 55},
     {CONTENT_SETTINGS_TYPE_LEGACY_COOKIE_ACCESS, 56},
+    {CONTENT_SETTINGS_TYPE_NATIVE_FILE_SYSTEM_WRITE_GUARD, 57},
 };
 
 }  // namespace

components/content_settings/core/common/content_settings_types.h

@@ -167,6 +167,11 @@ enum ContentSettingsType {
   // in cookie handling are introduced.
   CONTENT_SETTINGS_TYPE_LEGACY_COOKIE_ACCESS,
 
+  // Content settings which stores whether to allow sites to ask for permission
+  // to save changes to an original file selected by the user through the Native
+  // File System API.
+  CONTENT_SETTINGS_TYPE_NATIVE_FILE_SYSTEM_WRITE_GUARD,
+
   CONTENT_SETTINGS_NUM_TYPES,
 };
 

tools/metrics/histograms/enums.xml

@@ -9787,6 +9787,7 @@ Called by update_net_error_codes.py.-->
   <int value="54" label="Screen wake lock"/>
   <int value="55" label="System wake lock"/>
   <int value="56" label="Legacy cookie access"/>
+  <int value="57" label="Native file system write guard"/>
 </enum>
 
 <enum name="ContentTypeParseableResult">