Browse Source

feat: emit an event when accessing restricted path in File System Access API (#42994)

* fix: show a dialog when accessing restricted path in File System Access API

Co-authored-by: Shelley Vohr <[email protected]>

* fix: allow overriding initial blocked paths

Co-authored-by: Shelley Vohr <[email protected]>

* docs: fix doc

Co-authored-by: Shelley Vohr <[email protected]>

* Update docs/api/session.md

Co-authored-by: Erick Zhao <[email protected]>

Co-authored-by: Shelley Vohr <[email protected]>

* fix: change block to deny for consistency

Co-authored-by: Shelley Vohr <[email protected]>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>
trop[bot] 9 months ago
parent
commit
9e14f8d828

+ 65 - 0
docs/api/session.md

@@ -143,6 +143,71 @@ Returns:
 Emitted after an extension is loaded and all necessary browser state is
 initialized to support the start of the extension's background page.
 
+#### Event: 'file-system-access-restricted'
+
+Returns:
+
+* `event` Event
+* `details` Object
+  * `origin` string - The origin that initiated access to the blocked path.
+  * `isDirectory` boolean - Whether or not the path is a directory.
+  * `path` string - The blocked path attempting to be accessed.
+* `callback` Function
+  * `action` string - The action to take as a result of the restricted path access attempt.
+    * `allow` - This will allow `path` to be accessed despite restricted status.
+    * `deny` - This will block the access request and trigger an [`AbortError`](https://developer.mozilla.org/en-US/docs/Web/API/AbortController/abort).
+    * `tryAgain` - This will open a new file picker and allow the user to choose another path.
+
+```js
+const { app, dialog, BrowserWindow, session } = require('electron')
+
+async function createWindow () {
+  const mainWindow = new BrowserWindow()
+
+  await mainWindow.loadURL('https://buzzfeed.com')
+
+  session.defaultSession.on('file-system-access-restricted', async (e, details, callback) => {
+    const { origin, path } = details
+    const { response } = await dialog.showMessageBox({
+      message: `Are you sure you want ${origin} to open restricted path ${path}?`,
+      title: 'File System Access Restricted',
+      buttons: ['Choose a different folder', 'Allow', 'Cancel'],
+      cancelId: 2
+    })
+
+    if (response === 0) {
+      callback('tryAgain')
+    } else if (response === 1) {
+      callback('allow')
+    } else {
+      callback('deny')
+    }
+  })
+
+  mainWindow.webContents.executeJavaScript(`
+    window.showDirectoryPicker({
+      id: 'electron-demo',
+      mode: 'readwrite',
+      startIn: 'downloads',
+    }).catch(e => {
+      console.log(e)
+    })`, true
+  )
+}
+
+app.whenReady().then(() => {
+  createWindow()
+
+  app.on('activate', () => {
+    if (BrowserWindow.getAllWindows().length === 0) createWindow()
+  })
+})
+
+app.on('window-all-closed', function () {
+  if (process.platform !== 'darwin') app.quit()
+})
+```
+
 #### Event: 'preconnect'
 
 Returns:

+ 68 - 10
shell/browser/file_system_access/file_system_access_permission_context.cc

@@ -11,6 +11,7 @@
 #include "base/files/file_path.h"
 #include "base/json/values_util.h"
 #include "base/path_service.h"
+#include "base/task/bind_post_task.h"
 #include "base/task/thread_pool.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
@@ -26,18 +27,52 @@
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
+#include "gin/data_object_builder.h"
+#include "shell/browser/api/electron_api_session.h"
 #include "shell/browser/electron_permission_manager.h"
 #include "shell/browser/web_contents_permission_helper.h"
+#include "shell/common/gin_converters/callback_converter.h"
 #include "shell/common/gin_converters/file_path_converter.h"
 #include "third_party/blink/public/mojom/file_system_access/file_system_access_manager.mojom.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "url/origin.h"
 
+namespace gin {
+
+template <>
+struct Converter<
+    ChromeFileSystemAccessPermissionContext::SensitiveEntryResult> {
+  static bool FromV8(
+      v8::Isolate* isolate,
+      v8::Local<v8::Value> val,
+      ChromeFileSystemAccessPermissionContext::SensitiveEntryResult* out) {
+    std::string type;
+    if (!ConvertFromV8(isolate, val, &type))
+      return false;
+    if (type == "allow")
+      *out = ChromeFileSystemAccessPermissionContext::SensitiveEntryResult::
+          kAllowed;
+    else if (type == "tryAgain")
+      *out = ChromeFileSystemAccessPermissionContext::SensitiveEntryResult::
+          kTryAgain;
+    else if (type == "deny")
+      *out =
+          ChromeFileSystemAccessPermissionContext::SensitiveEntryResult::kAbort;
+    else
+      return false;
+    return true;
+  }
+};
+
+}  // namespace gin
+
 namespace {
 
 using BlockType = ChromeFileSystemAccessPermissionContext::BlockType;
 using HandleType = content::FileSystemAccessPermissionContext::HandleType;
 using GrantType = electron::FileSystemAccessPermissionContext::GrantType;
+using SensitiveEntryResult =
+    ChromeFileSystemAccessPermissionContext::SensitiveEntryResult;
 using blink::mojom::PermissionStatus;
 
 // Dictionary keys for the FILE_SYSTEM_LAST_PICKED_DIRECTORY website setting.
@@ -527,11 +562,11 @@ void FileSystemAccessPermissionContext::ConfirmSensitiveEntryAccess(
     content::GlobalRenderFrameHostId frame_id,
     base::OnceCallback<void(SensitiveEntryResult)> callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  callback_ = std::move(callback);
 
   auto after_blocklist_check_callback = base::BindOnce(
       &FileSystemAccessPermissionContext::DidCheckPathAgainstBlocklist,
-      GetWeakPtr(), origin, path, handle_type, user_action, frame_id,
-      std::move(callback));
+      GetWeakPtr(), origin, path, handle_type, user_action, frame_id);
   CheckPathAgainstBlocklist(path_type, path, handle_type,
                             std::move(after_blocklist_check_callback));
 }
@@ -570,31 +605,54 @@ void FileSystemAccessPermissionContext::PerformAfterWriteChecks(
   std::move(callback).Run(AfterWriteCheckResult::kAllow);
 }
 
+void FileSystemAccessPermissionContext::RunRestrictedPathCallback(
+    SensitiveEntryResult result) {
+  if (callback_)
+    std::move(callback_).Run(result);
+}
+
+void FileSystemAccessPermissionContext::OnRestrictedPathResult(
+    gin::Arguments* args) {
+  SensitiveEntryResult result = SensitiveEntryResult::kAbort;
+  args->GetNext(&result);
+  RunRestrictedPathCallback(result);
+}
+
 void FileSystemAccessPermissionContext::DidCheckPathAgainstBlocklist(
     const url::Origin& origin,
     const base::FilePath& path,
     HandleType handle_type,
     UserAction user_action,
     content::GlobalRenderFrameHostId frame_id,
-    base::OnceCallback<void(SensitiveEntryResult)> callback,
     bool should_block) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   if (user_action == UserAction::kNone) {
-    std::move(callback).Run(should_block ? SensitiveEntryResult::kAbort
-                                         : SensitiveEntryResult::kAllowed);
+    RunRestrictedPathCallback(should_block ? SensitiveEntryResult::kAbort
+                                           : SensitiveEntryResult::kAllowed);
     return;
   }
 
-  // Chromium opens a dialog here, but in Electron's case we log and abort.
   if (should_block) {
-    LOG(INFO) << path.value()
-              << " is blocked by the blocklis and cannot be accessed";
-    std::move(callback).Run(SensitiveEntryResult::kAbort);
+    auto* session =
+        electron::api::Session::FromBrowserContext(browser_context());
+    v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
+    v8::HandleScope scope(isolate);
+    v8::Local<v8::Object> details =
+        gin::DataObjectBuilder(isolate)
+            .Set("origin", origin.GetURL().spec())
+            .Set("isDirectory", handle_type == HandleType::kDirectory)
+            .Set("path", path)
+            .Build();
+    session->Emit(
+        "file-system-access-restricted", details,
+        base::BindRepeating(
+            &FileSystemAccessPermissionContext::OnRestrictedPathResult,
+            weak_factory_.GetWeakPtr()));
     return;
   }
 
-  std::move(callback).Run(SensitiveEntryResult::kAllowed);
+  RunRestrictedPathCallback(SensitiveEntryResult::kAllowed);
 }
 
 void FileSystemAccessPermissionContext::MaybeEvictEntries(

+ 16 - 8
shell/browser/file_system_access/file_system_access_permission_context.h

@@ -21,6 +21,10 @@
 
 class GURL;
 
+namespace gin {
+class Arguments;
+}  // namespace gin
+
 namespace base {
 class FilePath;
 }  // namespace base
@@ -128,14 +132,16 @@ class FileSystemAccessPermissionContext
                                  const base::FilePath& path,
                                  HandleType handle_type,
                                  base::OnceCallback<void(bool)> callback);
-  void DidCheckPathAgainstBlocklist(
-      const url::Origin& origin,
-      const base::FilePath& path,
-      HandleType handle_type,
-      UserAction user_action,
-      content::GlobalRenderFrameHostId frame_id,
-      base::OnceCallback<void(SensitiveEntryResult)> callback,
-      bool should_block);
+  void DidCheckPathAgainstBlocklist(const url::Origin& origin,
+                                    const base::FilePath& path,
+                                    HandleType handle_type,
+                                    UserAction user_action,
+                                    content::GlobalRenderFrameHostId frame_id,
+                                    bool should_block);
+
+  void RunRestrictedPathCallback(SensitiveEntryResult result);
+
+  void OnRestrictedPathResult(gin::Arguments* args);
 
   void MaybeEvictEntries(base::Value::Dict& dict);
 
@@ -159,6 +165,8 @@ class FileSystemAccessPermissionContext
 
   std::map<url::Origin, base::Value::Dict> id_pathinfo_map_;
 
+  base::OnceCallback<void(SensitiveEntryResult)> callback_;
+
   base::WeakPtrFactory<FileSystemAccessPermissionContext> weak_factory_{this};
 };