From af73790306711ddc489a8fdb5be8419eecaa3970 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Mon, 9 Mar 2020 15:45:07 -0700 Subject: [PATCH 1/5] macOS: request camera and microphone access Note `audio-input` and `camera` were already in `entitlements.plist` Supporting changes: - Add `allow-jit` entitlement since documentation says it's needed. - Only use sandbox for MAS build, not for non-MAS macOS build. NOTE: both still use the hardened runtime, as required on Catalina. - Remove `entitlements.inherit.plist` since it matches default settings. - Add to `electron-builder.yaml` English descriptions for why the app requests access to the microphone and camera. I'm not yet sure if there's a way to localize these. - Minor tweaks in `electron-builder.yaml`. --- README.md | 14 ++++++++++++ buildResources/entitlements.inherit.plist | 12 ---------- ...itlements.plist => entitlements.mac.plist} | 2 +- buildResources/entitlements.mas.plist | 22 +++++++++++++++++++ electron-builder.yaml | 12 +++++++--- src/main/index.js | 6 ++++- 6 files changed, 51 insertions(+), 17 deletions(-) delete mode 100644 buildResources/entitlements.inherit.plist rename buildResources/{entitlements.plist => entitlements.mac.plist} (92%) create mode 100644 buildResources/entitlements.mas.plist diff --git a/README.md b/README.md index b678d97..7abefa5 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,20 @@ To generate a signed NSIS installer: 4. Build the NSIS installer only: building the APPX installer will fail if these environment variables are set. - `npm run dist -- -w nsis` +#### Workaround for code signing issue in macOS + +Sometimes the macOS build process will result in a build which crashes on startup. If this happens, check in `Console` +for an entry similar to this: + +```text +failed to parse entitlements for Scratch Desktop[12345]: OSUnserializeXML: syntax error near line 1 +``` + +This appears to be an issue with `codesign` itself. Rebooting your computer and trying to build again might help. Yes, +really. + +See this issue for more detail: + ### Make a semi-packaged build This will simulate a packaged build without actually packaging it: instead the files will be copied to a subdirectory diff --git a/buildResources/entitlements.inherit.plist b/buildResources/entitlements.inherit.plist deleted file mode 100644 index a7a925c..0000000 --- a/buildResources/entitlements.inherit.plist +++ /dev/null @@ -1,12 +0,0 @@ - - - - -com.apple.security.app-sandbox - -com.apple.security.cs.allow-unsigned-executable-memory - -com.apple.security.inherit - - - diff --git a/buildResources/entitlements.plist b/buildResources/entitlements.mac.plist similarity index 92% rename from buildResources/entitlements.plist rename to buildResources/entitlements.mac.plist index 44d4e94..2e144be 100644 --- a/buildResources/entitlements.plist +++ b/buildResources/entitlements.mac.plist @@ -2,7 +2,7 @@ -com.apple.security.app-sandbox +com.apple.security.cs.allow-jit com.apple.security.cs.allow-unsigned-executable-memory diff --git a/buildResources/entitlements.mas.plist b/buildResources/entitlements.mas.plist new file mode 100644 index 0000000..0d80102 --- /dev/null +++ b/buildResources/entitlements.mas.plist @@ -0,0 +1,22 @@ + + + + +com.apple.security.app-sandbox + +com.apple.security.cs.allow-jit + +com.apple.security.cs.allow-unsigned-executable-memory + +com.apple.security.device.audio-input + +com.apple.security.device.camera + +com.apple.security.files.user-selected.read-only + +com.apple.security.files.user-selected.read-write + +com.apple.security.network.client + + + diff --git a/electron-builder.yaml b/electron-builder.yaml index 8ff6017..0bfb5ea 100644 --- a/electron-builder.yaml +++ b/electron-builder.yaml @@ -6,17 +6,23 @@ productName: "Scratch Desktop" afterSign: "scripts/afterSign.js" mac: category: public.app-category.education + entitlements: buildResources/entitlements.mac.plist + extendInfo: + NSCameraUsageDescription: >- + This app requires camera access when taking a photo in the paint editor or using the video sensing blocks. + NSMicrophoneUsageDescription: >- + This app requires microphone access when recording sounds or detecting loudness. + gatekeeperAssess: true hardenedRuntime: true icon: buildResources/ScratchDesktop.icns provisioningProfile: embedded.provisionprofile target: - dmg - mas -mas: type: distribution +mas: category: public.app-category.education - entitlements: buildResources/entitlements.plist - entitlementsInherit: buildResources/entitlements.inherit.plist + entitlements: buildResources/entitlements.mas.plist icon: buildResources/ScratchDesktop.icns win: icon: buildResources/ScratchDesktop.ico diff --git a/src/main/index.js b/src/main/index.js index e7bac4a..a3a9424 100644 --- a/src/main/index.js +++ b/src/main/index.js @@ -1,4 +1,4 @@ -import {BrowserWindow, Menu, app, dialog, ipcMain} from 'electron'; +import {BrowserWindow, Menu, app, dialog, ipcMain, systemPreferences} from 'electron'; import fs from 'fs'; import path from 'path'; import {format as formatUrl} from 'url'; @@ -140,6 +140,10 @@ const createMainWindow = () => { if (process.platform === 'darwin') { const osxMenu = Menu.buildFromTemplate(MacOSMenu(app)); Menu.setApplicationMenu(osxMenu); + (async () => { + await systemPreferences.askForMediaAccess('microphone'); + await systemPreferences.askForMediaAccess('camera'); + })(); } else { // disable menu for other platforms Menu.setApplicationMenu(null); From b26c0b6bd3c6d1d87bc13b65e3022854bffb5c21 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Thu, 12 Mar 2020 14:23:35 -0700 Subject: [PATCH 2/5] fix npm run dist:dir --- scripts/electron-builder-wrapper.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scripts/electron-builder-wrapper.js b/scripts/electron-builder-wrapper.js index f5aa406..a6fb573 100644 --- a/scripts/electron-builder-wrapper.js +++ b/scripts/electron-builder-wrapper.js @@ -50,9 +50,10 @@ const runBuilder = function (targetGroup) { throw new Error(`NSIS build requires CSC_LINK or WIN_CSC_LINK`); } const platformFlag = getPlatformFlag(); - const command = `electron-builder ${platformFlag} ${targetGroup}`; - console.log(`running: ${command}`); - const result = spawnSync(command, { + const customArgs = process.argv.slice(2); // remove `node` and `this-script.js` + const allArgs = [platformFlag, targetGroup, ...customArgs]; + console.log(`running electron-builder with arguments: ${allArgs}`); + const result = spawnSync('electron-builder', allArgs, { env: childEnvironment, shell: true, stdio: 'inherit' From 74968704c8ed8bf81f2c650553f0d8a086c61852 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Thu, 12 Mar 2020 17:17:56 -0700 Subject: [PATCH 3/5] improve user experience around mic/camera permission - ask for permission when trying to use a feature, not on startup - if permission is denied, explain the consequence and provide a hint for fixing it --- src/main/index.js | 130 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 111 insertions(+), 19 deletions(-) diff --git a/src/main/index.js b/src/main/index.js index a3a9424..2b83665 100644 --- a/src/main/index.js +++ b/src/main/index.js @@ -18,22 +18,48 @@ const isDevelopment = process.env.NODE_ENV !== 'production'; // global window references prevent them from being garbage-collected const _windows = {}; -const createWindow = ({search = null, url = 'index.html', ...browserWindowOptions}) => { - const window = new BrowserWindow({ - useContentSize: true, - show: false, - webPreferences: { - nodeIntegration: true - }, - ...browserWindowOptions - }); - const webContents = window.webContents; - - if (isDevelopment) { - webContents.openDevTools({mode: 'detach', activate: true}); +const displayPermissionDeniedWarning = (browserWindow, permissionType) => { + let title; + let message; + switch (permissionType) { + case 'camera': + title = 'Camera Permission Denied'; + message = 'Permission to use the camera has been denied. ' + + 'Scratch will not be able to take a photo or use video sensing blocks.'; + break; + case 'microphone': + title = 'Microphone Permission Denied'; + message = 'Permission to use the microphone has been denied. ' + + 'Scratch will not be able to record sounds or detect loudness.'; + break; + default: // shouldn't ever happen... + title = 'Permission Denied'; + message = 'A permission has been denied.'; } - const fullUrl = formatUrl(isDevelopment ? + let instructions; + switch (process.platform) { + case 'darwin': + instructions = 'To change Scratch permissions, please check "Security & Privacy" in System Preferences.'; + break; + default: + instructions = 'To change Scratch permissions, please check your system settings and restart Scratch.'; + break; + } + message = `${message}\n\n${instructions}`; + + dialog.showMessageBox(browserWindow, {type: 'warning', title, message}); +}; + +/** + * Build an absolute URL from a relative one, optionally adding search query parameters. + * The base of the URL will depend on whether or not the application is running in development mode. + * @param {string} url - the relative URL, like 'index.html' + * @param {*} search - the optional "search" parameters (the part of the URL after '?'), like "route=about" + * @returns {string} - an absolute URL as a string + */ +const makeFullUrl = (url, search = null) => + encodeURI(formatUrl(isDevelopment ? { // Webpack Dev Server hostname: 'localhost', pathname: url, @@ -47,7 +73,77 @@ const createWindow = ({search = null, url = 'index.html', ...browserWindowOption search, slashes: true } - ); + )); + +const handlePermissionRequest = async (webContents, permission, callback, details) => { + if (webContents !== _windows.main.webContents) { + // deny: request came from somewhere other than the main window's web contents + return callback(false); + } + if (!details.isMainFrame) { + // deny: request came from a subframe of the main window, not the main frame + return callback(false); + } + if (permission !== 'media') { + // deny: request is for some other kind of access like notifications or pointerLock + return callback(false); + } + const requiredBase = makeFullUrl('/'); + if (details.requestingUrl.indexOf(requiredBase) !== 0) { + // deny: request came from a URL outside of our "sandbox" + return callback(false); + } + let askForMicrophone = false; + let askForCamera = false; + for (const mediaType of details.mediaTypes) { + switch (mediaType) { + case 'audio': + askForMicrophone = true; + break; + case 'video': + askForCamera = true; + break; + default: + // deny: unhandled media type + return callback(false); + } + } + const parentWindow = _windows.main; // if we ever allow media in non-main windows we'll also need to change this + if (askForMicrophone) { + const microphoneResult = await systemPreferences.askForMediaAccess('microphone'); + if (!microphoneResult) { + displayPermissionDeniedWarning(parentWindow, 'microphone'); + return callback(false); + } + } + if (askForCamera) { + const cameraResult = await systemPreferences.askForMediaAccess('camera'); + if (!cameraResult) { + displayPermissionDeniedWarning(parentWindow, 'camera'); + return callback(false); + } + } + return callback(true); +}; + +const createWindow = ({search = null, url = 'index.html', ...browserWindowOptions}) => { + const window = new BrowserWindow({ + useContentSize: true, + show: false, + webPreferences: { + nodeIntegration: true + }, + ...browserWindowOptions + }); + const webContents = window.webContents; + + webContents.session.setPermissionRequestHandler(handlePermissionRequest); + + if (isDevelopment) { + webContents.openDevTools({mode: 'detach', activate: true}); + } + + const fullUrl = makeFullUrl(url, search); window.loadURL(fullUrl); return window; @@ -140,10 +236,6 @@ const createMainWindow = () => { if (process.platform === 'darwin') { const osxMenu = Menu.buildFromTemplate(MacOSMenu(app)); Menu.setApplicationMenu(osxMenu); - (async () => { - await systemPreferences.askForMediaAccess('microphone'); - await systemPreferences.askForMediaAccess('camera'); - })(); } else { // disable menu for other platforms Menu.setApplicationMenu(null); From 6e1bfc33f3d95da60b57bd95d7e3eb9a5ca8af88 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Tue, 17 Mar 2020 16:15:46 -0700 Subject: [PATCH 4/5] only call systemPreferences.askForMediaAccess if it exists --- src/main/index.js | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/main/index.js b/src/main/index.js index 2b83665..eaa132f 100644 --- a/src/main/index.js +++ b/src/main/index.js @@ -75,6 +75,23 @@ const makeFullUrl = (url, search = null) => } )); +/** + * Prompt in a platform-specific way for permission to access the microphone or camera, if Electron supports doing so. + * Any application-level checks, such as whether or not a particular frame or document should be allowed to ask, + * should be done before calling this function. + * + * @param {string} mediaType - one of Electron's media types, like 'microphone' or 'camera' + * @returns {boolean} - true if permission granted, false otherwise. + */ +const askForMediaAccess = async mediaType => { + if (systemPreferences.askForMediaAccess) { + // Electron currently only implements this on macOS + return await systemPreferences.askForMediaAccess(mediaType); + } + // For other platforms we can't reasonably do anything other than assume we have access. + return true; +}; + const handlePermissionRequest = async (webContents, permission, callback, details) => { if (webContents !== _windows.main.webContents) { // deny: request came from somewhere other than the main window's web contents @@ -110,14 +127,14 @@ const handlePermissionRequest = async (webContents, permission, callback, detail } const parentWindow = _windows.main; // if we ever allow media in non-main windows we'll also need to change this if (askForMicrophone) { - const microphoneResult = await systemPreferences.askForMediaAccess('microphone'); + const microphoneResult = await askForMediaAccess('microphone'); if (!microphoneResult) { displayPermissionDeniedWarning(parentWindow, 'microphone'); return callback(false); } } if (askForCamera) { - const cameraResult = await systemPreferences.askForMediaAccess('camera'); + const cameraResult = await askForMediaAccess('camera'); if (!cameraResult) { displayPermissionDeniedWarning(parentWindow, 'camera'); return callback(false); From 7ee12dcdd8cc69b419f9ce181252738eed97578d Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Tue, 17 Mar 2020 18:59:03 -0700 Subject: [PATCH 5/5] don't `return await` --- src/main/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/index.js b/src/main/index.js index eaa132f..fb8b311 100644 --- a/src/main/index.js +++ b/src/main/index.js @@ -86,7 +86,7 @@ const makeFullUrl = (url, search = null) => const askForMediaAccess = async mediaType => { if (systemPreferences.askForMediaAccess) { // Electron currently only implements this on macOS - return await systemPreferences.askForMediaAccess(mediaType); + return systemPreferences.askForMediaAccess(mediaType); } // For other platforms we can't reasonably do anything other than assume we have access. return true;