Bug 1493103 - allow users to import bookmarks from Safari without granting full disk access, r=jaws,Mardak,fluent-reviewers,flod

Differential Revision: https://phabricator.services.mozilla.com/D91485

browser/components/migration/SafariProfileMigrator.jsm

@@ -449,6 +449,56 @@ SafariProfileMigrator.prototype.getLastUsedDate = function SM_getLastUsedDate()
   });
 };
 
+SafariProfileMigrator.prototype.hasPermissions = async function SM_hasPermissions() {
+  if (this._hasPermissions) {
+    return true;
+  }
+  // Check if we have access:
+  let target = FileUtils.getDir(
+    "ULibDir",
+    ["Safari", "Bookmarks.plist"],
+    false
+  );
+  try {
+    // 'stat' is always allowed, but reading is somehow not, if the user hasn't
+    // allowed it:
+    await IOUtils.read(target.path, { maxBytes: 1 });
+    this._hasPermissions = true;
+    return true;
+  } catch (ex) {
+    return false;
+  }
+};
+
+SafariProfileMigrator.prototype.getPermissions = async function SM_getPermissions(
+  win
+) {
+  // Keep prompting the user until they pick a file that grants us access,
+  // or they cancel out of the file open panel.
+  while (!(await this.hasPermissions())) {
+    let fp = Cc["@mozilla.org/filepicker;1"].createInstance(Ci.nsIFilePicker);
+    // The title (second arg) is not displayed on macOS, so leave it blank.
+    fp.init(win, "", Ci.nsIFilePicker.modeOpen);
+    // This is a little weird. You'd expect that it matters which file
+    // the user picks, but it doesn't really, as long as it's in this
+    // directory. Anyway, let's not confuse the user: the sensible idea
+    // here is to ask for permissions for Bookmarks.plist, and we'll
+    // silently accept whatever input as long as we can then read the plist.
+    fp.appendFilter("plist", "*.plist");
+    fp.filterIndex = 1;
+    fp.displayDirectory = FileUtils.getDir("ULibDir", ["Safari"], false);
+    // Now wait for the filepicker to open and close. If the user picks
+    // any file in this directory, macOS will grant us read access, so
+    // we don't need to check or do anything else with the file returned
+    // by the filepicker.
+    let result = await new Promise(resolve => fp.open(resolve));
+    // Bail if the user cancels the dialog:
+    if (result == Ci.nsIFilePicker.returnCancel) {
+      return false;
+    }
+  }
+};
+
 Object.defineProperty(
   SafariProfileMigrator.prototype,
   "mainPreferencesPropertyList",

browser/components/migration/content/migration.js

@@ -8,6 +8,9 @@ const kIMig = Ci.nsIBrowserProfileMigrator;
 const kIPStartup = Ci.nsIProfileStartup;
 
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
+const { AppConstants } = ChromeUtils.import(
+  "resource://gre/modules/AppConstants.jsm"
+);
 const { MigrationUtils } = ChromeUtils.import(
   "resource:///modules/MigrationUtils.jsm"
 );
@@ -37,6 +40,7 @@ var MigrationWizard = {
   _wiz: null,
   _migrator: null,
   _autoMigrate: null,
+  _receivedPermissions: new Set(),
 
   init() {
     let os = Services.obs;
@@ -138,6 +142,11 @@ var MigrationWizard = {
       .addEventListener("pageadvanced", function() {
         MigrationWizard.onImportItemsPageAdvanced();
       });
+    document
+      .getElementById("importPermissions")
+      .addEventListener("pageadvanced", function(e) {
+        MigrationWizard.onImportPermissionsPageAdvanced(e);
+      });
     document
       .getElementById("importSource")
       .addEventListener("pageadvanced", function(e) {
@@ -286,12 +295,12 @@ var MigrationWizard = {
     // check for more than one source profile
     var sourceProfiles = this.spinResolve(this._migrator.getSourceProfiles());
     if (this._skipImportSourcePage) {
-      this._wiz.currentPage.next = "migrating";
+      this._updateNextPageForPermissions();
     } else if (sourceProfiles && sourceProfiles.length > 1) {
       this._wiz.currentPage.next = "selectProfile";
     } else {
       if (this._autoMigrate) {
-        this._wiz.currentPage.next = "migrating";
+        this._updateNextPageForPermissions();
       } else {
         this._wiz.currentPage.next = "importItems";
       }
@@ -351,7 +360,7 @@ var MigrationWizard = {
 
     // If we're automigrating or just doing bookmarks don't show the item selection page
     if (this._autoMigrate) {
-      this._wiz.currentPage.next = "migrating";
+      this._updateNextPageForPermissions();
     }
   },
 
@@ -398,6 +407,8 @@ var MigrationWizard = {
         this._itemsFlags |= parseInt(checkbox.id);
       }
     }
+
+    this._updateNextPageForPermissions();
   },
 
   onImportItemCommand() {
@@ -413,6 +424,56 @@ var MigrationWizard = {
     }
 
     this._wiz.canAdvance = oneChecked;
+
+    this._updateNextPageForPermissions();
+  },
+
+  _updateNextPageForPermissions() {
+    // We would like to just go straight to work:
+    this._wiz.currentPage.next = "migrating";
+    // If we already have permissions, this is easy:
+    if (this._receivedPermissions.has(this._source)) {
+      return;
+    }
+
+    // Otherwise, if we're on mojave or later and importing from
+    // Safari, prompt for the bookmarks file.
+    // We may add other browser/OS combos here in future.
+    if (
+      this._source == "safari" &&
+      AppConstants.isPlatformAndVersionAtLeast("macosx", "18") &&
+      this._itemsFlags & MigrationUtils.resourceTypes.BOOKMARKS
+    ) {
+      let migrator = this._migrator.wrappedJSObject;
+      let havePermissions = this.spinResolve(migrator.hasPermissions());
+
+      if (!havePermissions) {
+        this._wiz.currentPage.next = "importPermissions";
+      }
+    }
+  },
+
+  // 3b: permissions. This gets invoked when the user clicks "Next"
+  async onImportPermissionsPageAdvanced(event) {
+    // We're done if we have permission:
+    if (this._receivedPermissions.has(this._source)) {
+      return;
+    }
+    // The wizard helper is sync, and we need to check some stuff, so just stop
+    // advancing for now and prompt the user, then advance the wizard if everything
+    // worked.
+    event.preventDefault();
+
+    let migrator = this._migrator.wrappedJSObject;
+    await migrator.getPermissions(window);
+    if (await migrator.hasPermissions()) {
+      this._receivedPermissions.add(this._source);
+      // Re-enter (we'll then allow the advancement through the early return above)
+      this._wiz.advance();
+    }
+    // if we didn't have permissions after the `getPermissions` call, the user
+    // cancelled the dialog. Just no-op out now; the user can re-try by clicking
+    // the 'Continue' button again, or go back and pick a different browser.
   },
 
   // 4 - Migrating

browser/components/migration/content/migration.xhtml

@@ -15,6 +15,7 @@
         style="width: 40em;"
         buttons="accept,cancel">
 <linkset>
+  <html:link rel="localization" href="branding/brand.ftl"/>
   <html:link rel="localization" href="toolkit/global/wizard.ftl"/>
   <html:link rel="localization" href="browser/migration.ftl"/>
 </linkset>
@@ -78,6 +79,12 @@
     <vbox id="dataSources" style="overflow: auto; appearance: auto; -moz-default-appearance: listbox" align="start" flex="1" role="group"/>
   </wizardpage>
 
+  <wizardpage id="importPermissions" pageid="importPermissions"
+              data-header-label-id="import-permissions-page-title"
+              next="migrating">
+    <description data-l10n-id="import-permissions-description"></description>
+  </wizardpage>
+
   <wizardpage id="migrating" pageid="migrating"
               data-header-label-id="import-migrating-page-title"
               next="done">

browser/locales/en-US/browser/migration.ftl

@@ -59,6 +59,11 @@ import-items-page-title = Items to Import
 
 import-items-description = Select which items to import:
 
+import-permissions-page-title = Please give { -brand-short-name } permissions
+
+# Do not translate "Bookmarks.plist"; the file name is the same everywhere.
+import-permissions-description = macOS requires you to explicitly allow { -brand-short-name } to access Safari’s bookmarks. Click “Continue” and select the “Bookmarks.plist” file in the File Open panel that appears.
+
 import-migrating-page-title = Importing…
 
 import-migrating-description = The following items are currently being imported…