[MM-46980] Improve screensharing permissions flow (#2556)

* Little improvements and simplification

* Improve MacOS screen permissions handling flow

* Add missing error event

* Propagate calls client errors (#2557)
This commit is contained in:
Claudio Costa
2023-02-17 14:07:29 -06:00
committed by GitHub
parent ec8e0b9cc2
commit d1f6fc5a5f
8 changed files with 245 additions and 27 deletions

View File

@@ -15,7 +15,6 @@ import {
CALLS_ERROR,
DESKTOP_SOURCES_RESULT,
DESKTOP_SOURCES_MODAL_REQUEST,
DISPATCH_GET_DESKTOP_SOURCES,
CALLS_LINK_CLICK,
} from 'common/communication';
@@ -42,16 +41,13 @@ window.addEventListener('message', ({origin, data = {}} = {}) => {
});
break;
}
case 'get-desktop-sources': {
ipcRenderer.send(DISPATCH_GET_DESKTOP_SOURCES, 'widget', message);
break;
}
case DESKTOP_SOURCES_MODAL_REQUEST:
case CALLS_WIDGET_CHANNEL_LINK_CLICK:
case CALLS_LINK_CLICK:
case CALLS_WIDGET_RESIZE:
case CALLS_JOINED_CALL:
case CALLS_POPOUT_FOCUS:
case CALLS_ERROR:
case CALLS_LEAVE_CALL: {
ipcRenderer.send(type, message);
break;

View File

@@ -36,6 +36,7 @@ import {
DESKTOP_SOURCES_MODAL_REQUEST,
CALLS_WIDGET_SHARE_SCREEN,
CLOSE_DOWNLOADS_DROPDOWN,
CALLS_ERROR,
} from 'common/communication';
const UNREAD_COUNT_INTERVAL = 1000;
@@ -343,6 +344,16 @@ ipcRenderer.on(CALLS_JOINED_CALL, (event, message) => {
);
});
ipcRenderer.on(CALLS_ERROR, (event, message) => {
window.postMessage(
{
type: CALLS_ERROR,
message,
},
window.location.origin,
);
});
/* eslint-enable no-magic-numbers */
window.addEventListener('resize', () => {

View File

@@ -5,6 +5,11 @@
import path from 'path';
import fs from 'fs';
import {exec as execOriginal} from 'child_process';
import {promisify} from 'util';
const exec = promisify(execOriginal);
import {app, BrowserWindow} from 'electron';
import {Args} from 'types/args';
@@ -136,3 +141,19 @@ export function shouldIncrementFilename(filepath: string, increment = 0): string
}
return filename;
}
export function resetScreensharePermissionsMacOS() {
if (process.platform !== 'darwin') {
return Promise.resolve();
}
return exec('tccutil reset ScreenCapture Mattermost.Desktop',
{timeout: 1000});
}
export function openScreensharePermissionsSettingsMacOS() {
if (process.platform !== 'darwin') {
return Promise.resolve();
}
return exec('open "x-apple.systempreferences:com.apple.preference.security?Privacy_ScreenCapture"',
{timeout: 1000});
}

View File

@@ -397,5 +397,10 @@ describe('main/windows/callsWidgetWindow', () => {
const widgetWindow = new CallsWidgetWindow(mainWindow, mainView, widgetConfig);
expect(widgetWindow.getURL().toString()).toBe('http://localhost:8065/');
});
it('getMainView', () => {
const widgetWindow = new CallsWidgetWindow(mainWindow, mainView, widgetConfig);
expect(widgetWindow.getMainView()).toEqual(mainView);
});
});
});

View File

@@ -225,5 +225,9 @@ export default class CallsWidgetWindow extends EventEmitter {
public getURL() {
return urlUtils.parseURL(this.win.webContents.getURL());
}
public getMainView() {
return this.mainView;
}
}

View File

@@ -10,7 +10,11 @@ import Config from 'common/config';
import {getTabViewName, TAB_MESSAGING} from 'common/tabs/TabView';
import urlUtils from 'common/utils/url';
import {getAdjustedWindowBoundaries} from 'main/utils';
import {
getAdjustedWindowBoundaries,
resetScreensharePermissionsMacOS,
openScreensharePermissionsSettingsMacOS,
} from 'main/utils';
import {WindowManager} from './windowManager';
import createMainWindow from './mainWindow';
@@ -60,6 +64,8 @@ jest.mock('common/tabs/TabView', () => ({
jest.mock('../utils', () => ({
getAdjustedWindowBoundaries: jest.fn(),
shouldHaveBackBar: jest.fn(),
openScreensharePermissionsSettingsMacOS: jest.fn(),
resetScreensharePermissionsMacOS: jest.fn(),
}));
jest.mock('../views/viewManager', () => ({
ViewManager: jest.fn(),
@@ -1055,18 +1061,67 @@ describe('main/windows/windowManager', () => {
};
beforeEach(() => {
windowManager.viewManager.views = new Map();
windowManager.callsWidgetWindow = new CallsWidgetWindow();
windowManager.callsWidgetWindow.win = {
webContents: {
send: jest.fn(),
},
};
Config.teams = [
{
name: 'server-1',
order: 1,
tabs: [
{
name: 'tab-1',
order: 0,
isOpen: false,
},
{
name: 'tab-2',
order: 2,
isOpen: true,
},
],
}, {
name: 'server-2',
order: 0,
tabs: [
{
name: 'tab-1',
order: 0,
isOpen: false,
},
{
name: 'tab-2',
order: 2,
isOpen: true,
},
],
lastActiveTab: 2,
},
];
const map = Config.teams.reduce((arr, item) => {
item.tabs.forEach((tab) => {
arr.push([`${item.name}_${tab.name}`, {
view: {
webContents: {
send: jest.fn(),
},
},
}]);
});
return arr;
}, []);
windowManager.viewManager.views = new Map(map);
});
afterEach(() => {
jest.resetAllMocks();
Config.teams = [];
windowManager.missingScreensharePermissions = undefined;
});
it('should send sources back', async () => {
@@ -1085,9 +1140,9 @@ describe('main/windows/windowManager', () => {
},
]);
await windowManager.handleGetDesktopSources(null, 'widget', null);
await windowManager.handleGetDesktopSources(null, 'server-1_tab-1', null);
expect(windowManager.callsWidgetWindow.win.webContents.send).toHaveBeenCalledWith('desktop-sources-result', [
expect(windowManager.viewManager.views.get('server-1_tab-1').view.webContents.send).toHaveBeenCalledWith('desktop-sources-result', [
{
id: 'screen0',
},
@@ -1099,10 +1154,14 @@ describe('main/windows/windowManager', () => {
it('should send error with no sources', async () => {
jest.spyOn(desktopCapturer, 'getSources').mockResolvedValue([]);
await windowManager.handleGetDesktopSources(null, 'widget', null);
await windowManager.handleGetDesktopSources(null, 'server-2_tab-1', null);
expect(windowManager.callsWidgetWindow.win.webContents.send).toHaveBeenCalledWith('calls-error', {
err: 'screen-permissions',
});
expect(windowManager.viewManager.views.get('server-2_tab-1').view.webContents.send).toHaveBeenCalledWith('calls-error', {
err: 'screen-permissions',
});
expect(windowManager.callsWidgetWindow.win.webContents.send).toHaveBeenCalledTimes(1);
});
it('should send error with no permissions', async () => {
@@ -1116,12 +1175,55 @@ describe('main/windows/windowManager', () => {
]);
jest.spyOn(systemPreferences, 'getMediaAccessStatus').mockReturnValue('denied');
await windowManager.handleGetDesktopSources(null, 'widget', null);
await windowManager.handleGetDesktopSources(null, 'server-1_tab-1', null);
expect(systemPreferences.getMediaAccessStatus).toHaveBeenCalledWith('screen');
expect(windowManager.callsWidgetWindow.win.webContents.send).toHaveBeenCalledWith('calls-error', {
err: 'screen-permissions',
});
expect(windowManager.viewManager.views.get('server-1_tab-1').view.webContents.send).toHaveBeenCalledWith('calls-error', {
err: 'screen-permissions',
});
expect(windowManager.viewManager.views.get('server-1_tab-1').view.webContents.send).toHaveBeenCalledTimes(1);
expect(windowManager.callsWidgetWindow.win.webContents.send).toHaveBeenCalledTimes(1);
});
it('macos - no permissions', async () => {
const originalPlatform = process.platform;
Object.defineProperty(process, 'platform', {
value: 'darwin',
});
jest.spyOn(desktopCapturer, 'getSources').mockResolvedValue([
{
id: 'screen0',
thumbnail: {
toDataURL: jest.fn(),
},
},
]);
jest.spyOn(systemPreferences, 'getMediaAccessStatus').mockReturnValue('denied');
await windowManager.handleGetDesktopSources(null, 'server-1_tab-1', null);
expect(windowManager.missingScreensharePermissions).toBe(true);
expect(resetScreensharePermissionsMacOS).toHaveBeenCalledTimes(1);
expect(openScreensharePermissionsSettingsMacOS).toHaveBeenCalledTimes(0);
expect(windowManager.callsWidgetWindow.win.webContents.send).toHaveBeenCalledWith('calls-error', {
err: 'screen-permissions',
});
expect(windowManager.viewManager.views.get('server-1_tab-1').view.webContents.send).toHaveBeenCalledWith('calls-error', {
err: 'screen-permissions',
});
await windowManager.handleGetDesktopSources(null, 'server-1_tab-1', null);
expect(resetScreensharePermissionsMacOS).toHaveBeenCalledTimes(2);
expect(openScreensharePermissionsSettingsMacOS).toHaveBeenCalledTimes(1);
Object.defineProperty(process, 'platform', {
value: originalPlatform,
});
});
});
@@ -1267,6 +1369,42 @@ describe('main/windows/windowManager', () => {
});
});
describe('handleCallsError', () => {
const windowManager = new WindowManager();
windowManager.switchServer = jest.fn();
windowManager.mainWindow = {
focus: jest.fn(),
};
beforeEach(() => {
CallsWidgetWindow.mockImplementation(() => {
return {
getServerName: () => 'server-2',
getMainView: jest.fn().mockReturnValue({
view: {
webContents: {
send: jest.fn(),
},
},
}),
};
});
});
afterEach(() => {
jest.resetAllMocks();
Config.teams = [];
});
it('should focus view and propagate error to main view', () => {
windowManager.callsWidgetWindow = new CallsWidgetWindow();
windowManager.handleCallsError(null, {err: 'client-error'});
expect(windowManager.switchServer).toHaveBeenCalledWith('server-2');
expect(windowManager.mainWindow.focus).toHaveBeenCalled();
expect(windowManager.callsWidgetWindow.getMainView().view.webContents.send).toHaveBeenCalledWith('calls-error', {err: 'client-error'});
});
});
describe('handleCallsLinkClick', () => {
const windowManager = new WindowManager();
const view1 = {

View File

@@ -7,7 +7,11 @@ import path from 'path';
import {app, BrowserWindow, nativeImage, systemPreferences, ipcMain, IpcMainEvent, IpcMainInvokeEvent, desktopCapturer} from 'electron';
import log from 'electron-log';
import {CallsJoinCallMessage, CallsLinkClickMessage} from 'types/calls';
import {
CallsJoinCallMessage,
CallsErrorMessage,
CallsLinkClickMessage,
} from 'types/calls';
import {
MAXIMIZE_CHANGE,
@@ -42,7 +46,12 @@ import {getTabViewName, TAB_MESSAGING} from 'common/tabs/TabView';
import {MattermostView} from 'main/views/MattermostView';
import {getAdjustedWindowBoundaries, shouldHaveBackBar} from '../utils';
import {
getAdjustedWindowBoundaries,
shouldHaveBackBar,
resetScreensharePermissionsMacOS,
openScreensharePermissionsSettingsMacOS,
} from '../utils';
import {ViewManager, LoadingScreenState} from '../views/viewManager';
import CriticalErrorHandler from '../CriticalErrorHandler';
@@ -72,6 +81,7 @@ export class WindowManager {
downloadsDropdown?: DownloadsDropdownView;
downloadsDropdownMenu?: DownloadsDropdownMenuView;
currentServerName?: string;
missingScreensharePermissions?: boolean;
constructor() {
this.mainWindowReady = false;
@@ -94,6 +104,7 @@ export class WindowManager {
ipcMain.on(CALLS_LEAVE_CALL, () => this.callsWidgetWindow?.close());
ipcMain.on(DESKTOP_SOURCES_MODAL_REQUEST, this.handleDesktopSourcesModalRequest);
ipcMain.on(CALLS_WIDGET_CHANNEL_LINK_CLICK, this.handleCallsWidgetChannelLinkClick);
ipcMain.on(CALLS_ERROR, this.handleCallsError);
ipcMain.on(CALLS_LINK_CLICK, this.handleCallsLinkClick);
}
@@ -133,7 +144,7 @@ export class WindowManager {
log.debug('WindowManager.handleDesktopSourcesModalRequest');
if (this.callsWidgetWindow) {
this.switchServer(this.callsWidgetWindow?.getServerName());
this.switchServer(this.callsWidgetWindow.getServerName());
this.mainWindow?.focus();
const currentView = this.viewManager?.getCurrentView();
currentView?.view.webContents.send(DESKTOP_SOURCES_MODAL_REQUEST);
@@ -150,6 +161,15 @@ export class WindowManager {
}
}
handleCallsError = (event: IpcMainEvent, msg: CallsErrorMessage) => {
log.debug('WindowManager.handleCallsError', msg);
if (this.callsWidgetWindow) {
this.switchServer(this.callsWidgetWindow.getServerName());
this.mainWindow?.focus();
this.callsWidgetWindow.getMainView().view.webContents.send(CALLS_ERROR, msg);
}
}
handleCallsLinkClick = (_: IpcMainEvent, msg: CallsLinkClickMessage) => {
log.debug('WindowManager.handleCallsLinkClick with linkURL', msg.link);
this.mainWindow?.focus();
@@ -830,15 +850,34 @@ export class WindowManager {
return event.sender.id;
}
handleGetDesktopSources = (event: IpcMainEvent, viewName: string, opts: Electron.SourcesOptions) => {
handleGetDesktopSources = async (event: IpcMainEvent, viewName: string, opts: Electron.SourcesOptions) => {
log.debug('WindowManager.handleGetDesktopSources', {viewName, opts});
const globalWidget = viewName === 'widget' && this.callsWidgetWindow;
const view = this.viewManager?.views.get(viewName);
if (!view && !globalWidget) {
if (!view) {
log.error('WindowManager.handleGetDesktopSources: view not found');
return Promise.resolve();
}
if (process.platform === 'darwin' && systemPreferences.getMediaAccessStatus('screen') === 'denied') {
try {
// If permissions are missing we reset them so that the system
// prompt can be showed.
await resetScreensharePermissionsMacOS();
// We only open the system settings if permissions were already missing since
// on the first attempt to get the sources the OS will correctly show a prompt.
if (this.missingScreensharePermissions) {
await openScreensharePermissionsSettingsMacOS();
}
this.missingScreensharePermissions = true;
} catch (err) {
log.error('failed to reset screen sharing permissions', err);
}
}
const screenPermissionsErrMsg = {err: 'screen-permissions'};
return desktopCapturer.getSources(opts).then((sources) => {
let hasScreenPermissions = true;
if (systemPreferences.getMediaAccessStatus) {
@@ -850,10 +889,10 @@ export class WindowManager {
}
}
if (!hasScreenPermissions || !sources?.length) {
this.callsWidgetWindow?.win.webContents.send(CALLS_ERROR, {
err: 'screen-permissions',
});
if (!hasScreenPermissions || !sources.length) {
log.info('missing screen permissions');
view.view.webContents.send(CALLS_ERROR, screenPermissionsErrMsg);
this.callsWidgetWindow?.win.webContents.send(CALLS_ERROR, screenPermissionsErrMsg);
return;
}
@@ -865,16 +904,14 @@ export class WindowManager {
};
});
if (view) {
if (message.length > 0) {
view.view.webContents.send(DESKTOP_SOURCES_RESULT, message);
} else {
this.callsWidgetWindow?.win.webContents.send(DESKTOP_SOURCES_RESULT, message);
}
}).catch((err) => {
log.error('desktopCapturer.getSources failed', err);
this.callsWidgetWindow?.win.webContents.send(CALLS_ERROR, {
err: 'screen-permissions',
});
view.view.webContents.send(CALLS_ERROR, screenPermissionsErrMsg);
this.callsWidgetWindow?.win.webContents.send(CALLS_ERROR, screenPermissionsErrMsg);
});
}

View File

@@ -29,6 +29,12 @@ export type CallsJoinedCallMessage = {
callID: string;
}
export type CallsErrorMessage = {
err: string;
callID?: string;
errMsg?: string;
}
export type CallsLinkClickMessage = {
link: string | URL;
}