Skip to content

Commit

Permalink
[MM-46980] Improve screensharing permissions flow (#2556) (#2561)
Browse files Browse the repository at this point in the history 
* Little improvements and simplification

* Improve MacOS screen permissions handling flow

* Add missing error event

* Propagate calls client errors (#2557)

(cherry picked from commit d1f6fc5)

Co-authored-by: Claudio Costa <[email protected]>
  • Loading branch information
@mattermost-build @streamer45
mattermost-build and streamer45 authored Feb 17, 2023
1 parent de62108 commit b6b8e5b
Show file tree
Hide file tree
Showing 8 changed files with 245 additions and 27 deletions.
6 changes: 1 addition & 5 deletions src/main/preload/callsWidget.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
CALLS_ERROR,
DESKTOP_SOURCES_RESULT,
DESKTOP_SOURCES_MODAL_REQUEST,
DISPATCH_GET_DESKTOP_SOURCES,
CALLS_LINK_CLICK,
} from 'common/communication';

Expand All @@ -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;
Expand Down
11 changes: 11 additions & 0 deletions src/main/preload/mattermost.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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', () => {
Expand Down
21 changes: 21 additions & 0 deletions src/main/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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});
}
5 changes: 5 additions & 0 deletions src/main/windows/callsWidgetWindow.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
4 changes: 4 additions & 0 deletions src/main/windows/callsWidgetWindow.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,5 +225,9 @@ export default class CallsWidgetWindow extends EventEmitter {
public getURL() {
return urlUtils.parseURL(this.win.webContents.getURL());
}

public getMainView() {
return this.mainView;
}
}

150 changes: 144 additions & 6 deletions src/main/windows/windowManager.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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',
},
Expand All @@ -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 () => {
Expand All @@ -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,
});
});
});

Expand Down Expand Up @@ -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 = {
Expand Down
Loading

0 comments on commit b6b8e5b

Please sign in to comment.