fix: File System API permissions should reset on WebContents destruction (#43048)

fix: active File System API permissions should reset on WebContents destruction

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>

filenames.gni

@@ -388,6 +388,8 @@ filenames = {
     "shell/browser/file_system_access/file_system_access_permission_context.h",
     "shell/browser/file_system_access/file_system_access_permission_context_factory.cc",
     "shell/browser/file_system_access/file_system_access_permission_context_factory.h",
+    "shell/browser/file_system_access/file_system_access_web_contents_helper.cc",
+    "shell/browser/file_system_access/file_system_access_web_contents_helper.h",
     "shell/browser/font_defaults.cc",
     "shell/browser/font_defaults.h",
     "shell/browser/hid/electron_hid_delegate.cc",

shell/browser/file_system_access/file_system_access_permission_context.cc

@@ -93,6 +93,8 @@ const char kPathKey[] = "path";
 const char kPathTypeKey[] = "path-type";
 const char kTimestampKey[] = "timestamp";
 
+constexpr base::TimeDelta kPermissionRevocationTimeout = base::Seconds(5);
+
 #if BUILDFLAG(IS_WIN)
 [[nodiscard]] constexpr bool ContainsInvalidDNSCharacter(
     base::FilePath::StringType hostname) {
@@ -419,6 +421,7 @@ class FileSystemAccessPermissionContext::PermissionGrantImpl
 };
 
 struct FileSystemAccessPermissionContext::OriginState {
+  std::unique_ptr<base::RetainingOneShotTimer> cleanup_timer;
   // Raw pointers, owned collectively by all the handles that reference this
   // grant. When last reference goes away this state is cleared as well by
   // PermissionGrantDestroyed().
@@ -832,7 +835,7 @@ void FileSystemAccessPermissionContext::CheckPathsAgainstEnterprisePolicy(
   std::move(callback).Run(std::move(entries));
 }
 
-void FileSystemAccessPermissionContext::RevokeGrant(
+void FileSystemAccessPermissionContext::RevokeActiveGrants(
     const url::Origin& origin,
     const base::FilePath& file_path) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -882,10 +885,30 @@ bool FileSystemAccessPermissionContext::OriginHasWriteAccess(
   return false;
 }
 
+void FileSystemAccessPermissionContext::NavigatedAwayFromOrigin(
+    const url::Origin& origin) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  auto it = active_permissions_map_.find(origin);
+  // If we have no permissions for the origin, there is nothing to do.
+  if (it == active_permissions_map_.end()) {
+    return;
+  }
+
+  // Start a timer to possibly clean up permissions for this origin.
+  if (!it->second.cleanup_timer) {
+    it->second.cleanup_timer = std::make_unique<base::RetainingOneShotTimer>(
+        FROM_HERE, kPermissionRevocationTimeout,
+        base::BindRepeating(
+            &FileSystemAccessPermissionContext::CleanupPermissions,
+            base::Unretained(this), origin));
+  }
+  it->second.cleanup_timer->Reset();
+}
+
 void FileSystemAccessPermissionContext::CleanupPermissions(
     const url::Origin& origin) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  RevokeGrant(origin);
+  RevokeActiveGrants(origin);
 }
 
 bool FileSystemAccessPermissionContext::AncestorHasActivePermission(

shell/browser/file_system_access/file_system_access_permission_context.h

@@ -112,12 +112,16 @@ class FileSystemAccessPermissionContext
 
   enum class RequestType { kNewPermission, kRestorePermissions };
 
-  void RevokeGrant(const url::Origin& origin,
-                   const base::FilePath& file_path = base::FilePath());
+  void RevokeActiveGrants(const url::Origin& origin,
+                          const base::FilePath& file_path = base::FilePath());
 
   bool OriginHasReadAccess(const url::Origin& origin);
   bool OriginHasWriteAccess(const url::Origin& origin);
 
+  // Called by FileSystemAccessWebContentsHelper when a top-level frame was
+  // navigated away from `origin` to some other origin.
+  void NavigatedAwayFromOrigin(const url::Origin& origin);
+
   content::BrowserContext* browser_context() const { return browser_context_; }
 
  protected:

shell/browser/file_system_access/file_system_access_web_contents_helper.cc

@@ -0,0 +1,56 @@
+// Copyright (c) 2024 Microsoft, GmbH
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "shell/browser/file_system_access/file_system_access_web_contents_helper.h"
+
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/navigation_handle.h"
+#include "shell/browser/file_system_access/file_system_access_permission_context.h"
+#include "shell/browser/file_system_access/file_system_access_permission_context_factory.h"
+
+FileSystemAccessWebContentsHelper::~FileSystemAccessWebContentsHelper() =
+    default;
+
+void FileSystemAccessWebContentsHelper::DidFinishNavigation(
+    content::NavigationHandle* navigation) {
+  // We only care about top-level navigations that actually committed.
+  if (!navigation->IsInPrimaryMainFrame() || !navigation->HasCommitted())
+    return;
+
+  auto src_origin =
+      url::Origin::Create(navigation->GetPreviousPrimaryMainFrameURL());
+  auto dest_origin = url::Origin::Create(navigation->GetURL());
+
+  if (src_origin == dest_origin)
+    return;
+
+  // Navigated away from |src_origin|, tell permission context to check if
+  // permissions need to be revoked.
+  auto* context =
+      electron::FileSystemAccessPermissionContextFactory::GetForBrowserContext(
+          web_contents()->GetBrowserContext());
+  if (context)
+    context->NavigatedAwayFromOrigin(src_origin);
+}
+
+void FileSystemAccessWebContentsHelper::WebContentsDestroyed() {
+  auto src_origin =
+      web_contents()->GetPrimaryMainFrame()->GetLastCommittedOrigin();
+
+  // Navigated away from |src_origin|, tell permission context to check if
+  // permissions need to be revoked.
+  auto* context =
+      electron::FileSystemAccessPermissionContextFactory::GetForBrowserContext(
+          web_contents()->GetBrowserContext());
+  if (context)
+    context->NavigatedAwayFromOrigin(src_origin);
+}
+
+FileSystemAccessWebContentsHelper::FileSystemAccessWebContentsHelper(
+    content::WebContents* web_contents)
+    : content::WebContentsObserver(web_contents),
+      content::WebContentsUserData<FileSystemAccessWebContentsHelper>(
+          *web_contents) {}
+
+WEB_CONTENTS_USER_DATA_KEY_IMPL(FileSystemAccessWebContentsHelper);

shell/browser/file_system_access/file_system_access_web_contents_helper.h

@@ -0,0 +1,34 @@
+// Copyright (c) 2024 Microsoft, GmbH
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef ELECTRON_SHELL_BROWSER_FILE_SYSTEM_ACCESS_FILE_SYSTEM_ACCESS_WEB_CONTENTS_HELPER_H_
+#define ELECTRON_SHELL_BROWSER_FILE_SYSTEM_ACCESS_FILE_SYSTEM_ACCESS_WEB_CONTENTS_HELPER_H_
+
+#include "content/public/browser/web_contents_observer.h"
+#include "content/public/browser/web_contents_user_data.h"
+
+class FileSystemAccessWebContentsHelper
+    : public content::WebContentsObserver,
+      public content::WebContentsUserData<FileSystemAccessWebContentsHelper> {
+ public:
+  FileSystemAccessWebContentsHelper(const FileSystemAccessWebContentsHelper&) =
+      delete;
+  FileSystemAccessWebContentsHelper& operator=(
+      const FileSystemAccessWebContentsHelper&) = delete;
+  ~FileSystemAccessWebContentsHelper() override;
+
+  // content::WebContentsObserver
+  void DidFinishNavigation(
+      content::NavigationHandle* navigation_handle) override;
+  void WebContentsDestroyed() override;
+
+ private:
+  explicit FileSystemAccessWebContentsHelper(
+      content::WebContents* web_contents);
+  friend class content::WebContentsUserData<FileSystemAccessWebContentsHelper>;
+
+  WEB_CONTENTS_USER_DATA_KEY_DECL();
+};
+
+#endif  // ELECTRON_SHELL_BROWSER_FILE_SYSTEM_ACCESS_FILE_SYSTEM_ACCESS_WEB_CONTENTS_HELPER_H_