From 772244bb90d7403dffebd9a8b171a077a4b8041c Mon Sep 17 00:00:00 2001 From: Gijs Kruitbosch Date: Tue, 6 Oct 2020 15:45:46 +0000 Subject: [PATCH] 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 --- .../migration/SafariProfileMigrator.jsm | 50 ++++++++++++++ .../components/migration/content/migration.js | 67 ++++++++++++++++++- .../migration/content/migration.xhtml | 7 ++ browser/locales/en-US/browser/migration.ftl | 5 ++ 4 files changed, 126 insertions(+), 3 deletions(-) diff --git a/browser/components/migration/SafariProfileMigrator.jsm b/browser/components/migration/SafariProfileMigrator.jsm index 92a8ab5a9857..8cbf3fe29940 100644 --- a/browser/components/migration/SafariProfileMigrator.jsm +++ b/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", diff --git a/browser/components/migration/content/migration.js b/browser/components/migration/content/migration.js index 6f83ef9d90a1..2cf67b73e037 100644 --- a/browser/components/migration/content/migration.js +++ b/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 diff --git a/browser/components/migration/content/migration.xhtml b/browser/components/migration/content/migration.xhtml index 444f25491174..ff2b269c5ef0 100644 --- a/browser/components/migration/content/migration.xhtml +++ b/browser/components/migration/content/migration.xhtml @@ -15,6 +15,7 @@ style="width: 40em;" buttons="accept,cancel"> + @@ -78,6 +79,12 @@ + + + + diff --git a/browser/locales/en-US/browser/migration.ftl b/browser/locales/en-US/browser/migration.ftl index f9b6852283d9..4422812df0c1 100644 --- a/browser/locales/en-US/browser/migration.ftl +++ b/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…