Skip to content

Commit

Permalink
[MM-61821] Automatically allow permission checks for supported permis…
Browse files Browse the repository at this point in the history
…sion types through for GPO configured servers (#3231)

* [MM-61821] Automatically allow permission checks for supported permission types through for GPO configured servers

* Fix lint

* Fix tsc
  • Loading branch information
devinbinnie authored Dec 3, 2024
1 parent b73d68c commit 1894d8a
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 2 deletions.
24 changes: 23 additions & 1 deletion src/main/permissionsManager.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import {dialog, systemPreferences} from 'electron';

import Config from 'common/config';
import {parseURL, isTrustedURL} from 'common/utils/url';
import ViewManager from 'main/views/viewManager';
import CallsWidgetWindow from 'main/windows/callsWidgetWindow';
Expand Down Expand Up @@ -37,6 +38,12 @@ jest.mock('common/utils/url', () => ({
isTrustedURL: jest.fn(),
}));

jest.mock('common/config', () => ({
registryData: {
servers: [],
},
}));

jest.mock('main/i18nManager', () => ({
localizeMessage: jest.fn(),
}));
Expand Down Expand Up @@ -72,6 +79,9 @@ describe('main/PermissionsManager', () => {
if (id === 2) {
return {view: {server: {url: new URL('http://anyurl.com')}}};
}
if (id === 4) {
return {view: {server: {url: new URL('http://gposerver.com')}}};
}

return null;
});
Expand All @@ -84,6 +94,11 @@ describe('main/PermissionsManager', () => {
}
});
isTrustedURL.mockImplementation((url, baseURL) => url.toString().startsWith(baseURL.toString()));
Config.registryData.servers = [
{
url: 'http://gposerver.com',
},
];
});

afterEach(() => {
Expand Down Expand Up @@ -115,10 +130,17 @@ describe('main/PermissionsManager', () => {
it('should deny if the server URL can not be found', async () => {
const permissionsManager = new PermissionsManager('anyfile.json');
const cb = jest.fn();
await permissionsManager.handlePermissionRequest({id: 4}, 'media', cb, {securityOrigin: 'http://anyurl.com'});
await permissionsManager.handlePermissionRequest({id: 5}, 'media', cb, {securityOrigin: 'http://anyurl.com'});
expect(cb).toHaveBeenCalledWith(false);
});

it('should allow if the URL is a GPO configured server', async () => {
const permissionsManager = new PermissionsManager('anyfile.json');
const cb = jest.fn();
await permissionsManager.handlePermissionRequest({id: 4}, 'media', cb, {securityOrigin: 'http://gposerver.com'});
expect(cb).toHaveBeenCalledWith(true);
});

it('should deny if the URL is not trusted', async () => {
const permissionsManager = new PermissionsManager('anyfile.json');
const cb = jest.fn();
Expand Down
9 changes: 8 additions & 1 deletion src/main/permissionsManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
OPEN_WINDOWS_MICROPHONE_PREFERENCES,
UPDATE_PATHS,
} from 'common/communication';
import Config from 'common/config';
import JsonFileManager from 'common/JsonFileManager';
import {Logger} from 'common/log';
import type {MattermostServer} from 'common/servers/MattermostServer';
Expand Down Expand Up @@ -141,7 +142,7 @@ export class PermissionsManager extends JsonFileManager<PermissionsByOrigin> {
return false;
}

let serverURL;
let serverURL: URL | undefined;
if (CallsWidgetWindow.isCallsWidget(webContentsId)) {
serverURL = CallsWidgetWindow.getViewURL();
} else {
Expand All @@ -152,6 +153,12 @@ export class PermissionsManager extends JsonFileManager<PermissionsByOrigin> {
return false;
}

// For GPO servers, we always allow permissions since they are trusted
const serverHref = serverURL.href;
if (Config.registryData?.servers?.some((s) => parseURL(s.url)?.href === serverHref)) {
return true;
}

// Exception for embedded videos such as YouTube
// We still want to ask permission to do this though
const isExternalFullscreen = permission === 'fullscreen' && parsedURL.origin !== serverURL.origin;
Expand Down

0 comments on commit 1894d8a

Please sign in to comment.