Skip to content

Commit 91f7c5a

Browse files
tassoevantassoevan
authored
fix: Server validation (#1756)
* Remove server version check on preload script * Uses normalized URL to check server version * Tighten ServerUrlResolutionResult * Expand main process tests path resolution * Add more rules for server URL resolution * Lower required server version range * Use server URL resolution in deep links handling
1 parent 0f513d4 commit 91f7c5a

6 files changed

Lines changed: 134 additions & 58 deletions

File tree

‎jest.config.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@ module.exports = {
2020
errorOnDeprecated: true,
2121
runner: '@jest-runner/electron/main',
2222
testEnvironment: 'node',
23-
testMatch: ['<rootDir>/src/*/main/**/*.(spec|test).{js,ts,tsx}'],
23+
testMatch: [
24+
'<rootDir>/src/*/main/**/*.(spec|test).{js,ts,tsx}',
25+
'<rootDir>/src/**/main.(spec|test).{js,ts,tsx}',
26+
],
2427
globals: {
2528
'ts-jest': {
2629
tsConfig: {

‎src/deepLinks/main.ts‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { URL } from 'url';
33
import { app, WebContents } from 'electron';
44

55
import { ServerUrlResolutionStatus } from '../servers/common';
6-
import { normalizeServerUrl, resolveServerUrl } from '../servers/main';
6+
import { resolveServerUrl } from '../servers/main';
77
import { select, dispatch } from '../store';
88
import { askForServerAddition, warnAboutInvalidServerUrl } from '../ui/main/dialogs';
99
import { getRootWindow } from '../ui/main/rootWindow';
@@ -19,18 +19,28 @@ const isRocketChatUrl = (parsedUrl: URL): boolean =>
1919
const isGoRocketChatUrl = (parsedUrl: URL): boolean =>
2020
parsedUrl.protocol === 'https:' && parsedUrl.hostname === 'go.rocket.chat';
2121

22-
const parseDeepLink = (deepLink: string): { action: string, args: URLSearchParams } => {
23-
const parsedUrl = new URL(deepLink);
22+
const parseDeepLink = (input: string): { action: string, args: URLSearchParams } | null => {
23+
if (/^--/.test(input)) { // input is a CLI flag
24+
return null;
25+
}
26+
27+
let url: URL;
28+
29+
try {
30+
url = new URL(input);
31+
} catch (error) {
32+
return null;
33+
}
2434

25-
if (isRocketChatUrl(parsedUrl)) {
26-
const action = parsedUrl.hostname;
27-
const args = parsedUrl.searchParams;
35+
if (isRocketChatUrl(url)) {
36+
const action = url.hostname;
37+
const args = url.searchParams;
2838
return { action, args };
2939
}
3040

31-
if (isGoRocketChatUrl(parsedUrl)) {
32-
const action = parsedUrl.pathname;
33-
const args = parsedUrl.searchParams;
41+
if (isGoRocketChatUrl(url)) {
42+
const action = url.pathname;
43+
const args = url.searchParams;
3444
return { action, args };
3545
}
3646

@@ -58,11 +68,10 @@ type InviteParams = {
5868
};
5969

6070
const performOnServer = async (url: string, action: (serverUrl: string) => Promise<void>): Promise<void> => {
61-
let serverUrl: string;
71+
const [serverUrl, status, error] = await resolveServerUrl(url);
6272

63-
try {
64-
serverUrl = normalizeServerUrl(url);
65-
} catch (error) {
73+
if (status !== ServerUrlResolutionStatus.OK) {
74+
await warnAboutInvalidServerUrl(serverUrl, error.message);
6675
return;
6776
}
6877

@@ -80,18 +89,11 @@ const performOnServer = async (url: string, action: (serverUrl: string) => Promi
8089
return;
8190
}
8291

83-
const [normalizedServerUrl, result, error] = await resolveServerUrl(serverUrl);
84-
85-
if (result !== ServerUrlResolutionStatus.OK) {
86-
await warnAboutInvalidServerUrl(normalizedServerUrl, error.message);
87-
return;
88-
}
89-
9092
dispatch({
9193
type: DEEP_LINKS_SERVER_ADDED,
92-
payload: normalizedServerUrl,
94+
payload: serverUrl,
9395
});
94-
await action(normalizedServerUrl);
96+
await action(serverUrl);
9597
};
9698

9799
const getWebContents = (serverUrl: string): Promise<WebContents> =>

‎src/preload.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import fs from 'fs';
22
import path from 'path';
33

44
import { contextBridge, ipcRenderer, webFrame } from 'electron';
5-
import { satisfies, coerce } from 'semver';
65

76
import { setupRendererErrorHandling } from './errors';
87
import { JitsiMeetElectron, JitsiMeetElectronAPI } from './jitsi/preload';
@@ -42,7 +41,7 @@ const start = async (): Promise<void> => {
4241
);
4342
await webFrame.executeJavaScript(injectedCode);
4443

45-
if (!satisfies(coerce(serverInfo?.version), '>=3.0.x')) {
44+
if (!serverInfo) {
4645
return;
4746
}
4847

‎src/servers/common.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ export const enum ServerUrlResolutionStatus {
1919
}
2020

2121
export type ServerUrlResolutionResult = (
22-
[resolvedServerUrl: string, result: ServerUrlResolutionStatus.OK]
22+
[resolvedServerUrl: Server['url'], result: ServerUrlResolutionStatus.OK]
2323
| [
24-
resolvedServerUrl: string,
24+
resolvedServerUrl: Server['url'],
2525
result: Exclude<ServerUrlResolutionStatus, 'OK'>,
2626
error: Error,
2727
]

‎src/servers/main.spec.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { convertToURL } from './main';
2+
3+
describe('convertToUrl', () => {
4+
it('transforms localhost into URL(https://localhost/)', () => {
5+
const input = 'localhost';
6+
const result = convertToURL(input);
7+
8+
expect(result.href).toBe('https://localhost/');
9+
});
10+
11+
it('transforms localhost:3000 into URL(https://localhost:3000/)', () => {
12+
const input = 'localhost:3000';
13+
const result = convertToURL(input);
14+
15+
expect(result.href).toBe('https://localhost:3000/');
16+
});
17+
18+
it('transforms https://localhost into URL(https://localhost/)', () => {
19+
const input = 'https://localhost';
20+
const result = convertToURL(input);
21+
22+
expect(result.href).toBe('https://localhost/');
23+
});
24+
25+
it('transforms http://localhost into URL(http://localhost/)', () => {
26+
const input = 'http://localhost';
27+
const result = convertToURL(input);
28+
29+
expect(result.href).toBe('http://localhost/');
30+
});
31+
32+
it('transforms https://localhost/ into URL(https://localhost/)', () => {
33+
const input = 'https://localhost/';
34+
const result = convertToURL(input);
35+
36+
expect(result.href).toBe('https://localhost/');
37+
});
38+
39+
it('transforms http://localhost/ into URL(http://localhost/)', () => {
40+
const input = 'http://localhost/';
41+
const result = convertToURL(input);
42+
43+
expect(result.href).toBe('http://localhost/');
44+
});
45+
46+
it('transforms https://localhost/subdir into URL(https://localhost/subdir/)', () => {
47+
const input = 'https://localhost/subdir';
48+
const result = convertToURL(input);
49+
50+
expect(result.href).toBe('https://localhost/subdir/');
51+
});
52+
53+
it('transforms http://localhost:80 into URL(http://localhost/)', () => {
54+
const input = 'http://localhost:80';
55+
const result = convertToURL(input);
56+
57+
expect(result.href).toBe('http://localhost/');
58+
});
59+
60+
it('transforms https://localhost:443 into URL(https://localhost/)', () => {
61+
const input = 'https://localhost:443';
62+
const result = convertToURL(input);
63+
64+
expect(result.href).toBe('https://localhost/');
65+
});
66+
});

‎src/servers/main.ts‎

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -20,39 +20,39 @@ import {
2020
} from './actions';
2121
import { ServerUrlResolutionStatus, Server, ServerUrlResolutionResult } from './common';
2222

23-
export const normalizeServerUrl = (input: string): string => {
24-
if (typeof input !== 'string') {
25-
throw new TypeError('server URL is not a string');
26-
}
23+
const REQUIRED_SERVER_VERSION_RANGE = '>=2.4.0';
2724

28-
let parsedUrl: URL;
25+
export const convertToURL = (input: string): URL => {
26+
let url: URL;
2927

30-
try {
31-
parsedUrl = new URL(input);
32-
} catch (error) {
33-
parsedUrl = new URL(`https://${ input }`);
28+
if (/^https?:\/\//.test(input)) {
29+
url = new URL(input);
30+
} else {
31+
url = new URL(`https://${ input }`);
3432
}
3533

36-
const { protocol, username, password, hostname, port, pathname } = parsedUrl;
34+
const { protocol, username, password, hostname, port, pathname } = url;
3735
return Object.assign(new URL('https://0.0.0.0'), {
3836
protocol,
3937
username,
4038
password,
4139
hostname,
42-
port,
43-
pathname,
44-
}).href;
40+
port: (protocol === 'http' && port === '80' && undefined)
41+
|| (protocol === 'https' && port === '443' && undefined)
42+
|| port,
43+
pathname: /\/$/.test(pathname) ? pathname : `${ pathname }/`,
44+
});
4545
};
4646

47-
export const getServerVersion = async (serverUrl: string): Promise<string> => {
48-
const { username, password, href } = new URL(serverUrl);
47+
const fetchServerInformation = async (url: URL): Promise<[finalURL: URL, version: string]> => {
48+
const { username, password } = url;
4949
const headers: HeadersInit = [];
5050

5151
if (username && password) {
5252
headers.push(['Authorization', `Basic ${ Buffer.from(`${ username }:${ password }`).toString('base64') }`]);
5353
}
5454

55-
const endpoint = new URL('api/info', href);
55+
const endpoint = new URL('api/info', url);
5656

5757
const controller = new AbortController();
5858

@@ -80,37 +80,43 @@ export const getServerVersion = async (serverUrl: string): Promise<string> => {
8080
throw new Error();
8181
}
8282

83-
return responseBody.version;
83+
return [new URL('/', convertToURL(response.url)), responseBody.version];
8484
};
8585

86-
export const resolveServerUrl = async (serverUrl: string): Promise<ServerUrlResolutionResult> => {
87-
let normalizedServerUrl: string;
86+
export const resolveServerUrl = async (input: string): Promise<ServerUrlResolutionResult> => {
87+
let url: URL;
8888

8989
try {
90-
normalizedServerUrl = normalizeServerUrl(serverUrl);
90+
url = convertToURL(input);
9191
} catch (error) {
92-
return [serverUrl, ServerUrlResolutionStatus.INVALID_URL, error];
92+
return [input, ServerUrlResolutionStatus.INVALID_URL, error];
9393
}
9494

95-
try {
96-
const version = await getServerVersion(serverUrl);
95+
let version: string;
9796

98-
if (!satisfies(coerce(version), '>=3.0.x')) {
99-
throw new Error(`incompatible server version (${ version }, expected >=3.0.x)`);
100-
}
97+
try {
98+
[url, version] = await fetchServerInformation(url);
10199
} catch (error) {
102-
if (!/(^https?:\/\/)|(\.)|(^([^:]+:[^@]+@)?localhost(:\d+)?$)/.test(serverUrl)) {
103-
return resolveServerUrl(`https://${ serverUrl }.rocket.chat`);
100+
if (!/(^https?:\/\/)|(\.)|(^([^:]+:[^@]+@)?localhost(:\d+)?$)/.test(input)) {
101+
return resolveServerUrl(`https://${ input }.rocket.chat`);
104102
}
105103

106104
if (error.name === 'AbortError') {
107-
return [normalizedServerUrl, ServerUrlResolutionStatus.TIMEOUT, error];
105+
return [url.href, ServerUrlResolutionStatus.TIMEOUT, error];
108106
}
109107

110-
return [normalizedServerUrl, ServerUrlResolutionStatus.INVALID, error];
108+
return [url.href, ServerUrlResolutionStatus.INVALID, error];
109+
}
110+
111+
if (!satisfies(coerce(version), REQUIRED_SERVER_VERSION_RANGE)) {
112+
return [
113+
url.href,
114+
ServerUrlResolutionStatus.INVALID,
115+
new Error(`incompatible server version (${ version }, expected ${ REQUIRED_SERVER_VERSION_RANGE })`),
116+
];
111117
}
112118

113-
return [normalizedServerUrl, ServerUrlResolutionStatus.OK];
119+
return [url.href, ServerUrlResolutionStatus.OK];
114120
};
115121

116122
const loadAppServers = async (): Promise<Record<string, string>> => {

0 commit comments

Comments
 (0)