From f59cdf9ad5443a7526df4cef6d3cd136676589d3 Mon Sep 17 00:00:00 2001 From: Mohamed Marrouchi Date: Sun, 1 Dec 2024 08:22:47 +0100 Subject: [PATCH 1/5] fix: webchannel cors check --- .../channels/web/base-web-channel.ts | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/api/src/extensions/channels/web/base-web-channel.ts b/api/src/extensions/channels/web/base-web-channel.ts index b1544ca3..389ec00d 100644 --- a/api/src/extensions/channels/web/base-web-channel.ts +++ b/api/src/extensions/channels/web/base-web-channel.ts @@ -298,12 +298,27 @@ export default abstract class BaseWebChannelHandler< if (req.headers && req.headers.origin) { // Get the allowed origins const origins: string[] = settings.allowed_domains.split(','); - const foundOrigin = origins.some((origin: string) => { - origin = origin.trim(); - // If we find a whitelisted origin, send the Access-Control-Allow-Origin header - // to greenlight the request. - return origin == req.headers.origin || origin == '*'; - }); + const foundOrigin = origins + .map((origin) => { + try { + return new URL(origin.trim()).origin; + } catch (error) { + this.logger.error( + `Invalid URL in allowed domains: ${origin}`, + error, + ); + return null; + } + }) + .filter( + (normalizedOrigin): normalizedOrigin is string => + normalizedOrigin !== null, + ) + .some((origin: string) => { + // If we find a whitelisted origin, send the Access-Control-Allow-Origin header + // to greenlight the request. + return origin === req.headers.origin || origin === '*'; + }); if (!foundOrigin) { // For HTTP requests, set the Access-Control-Allow-Origin header to '', which the browser will From 778bd59f918c8083426f0e3fcf3102cd5672831c Mon Sep 17 00:00:00 2001 From: Mohamed Marrouchi Date: Sun, 1 Dec 2024 08:29:03 +0100 Subject: [PATCH 2/5] test: add unit tests --- .../channels/web/__test__/index.spec.ts | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/api/src/extensions/channels/web/__test__/index.spec.ts b/api/src/extensions/channels/web/__test__/index.spec.ts index 4ad78dbe..4038f632 100644 --- a/api/src/extensions/channels/web/__test__/index.spec.ts +++ b/api/src/extensions/channels/web/__test__/index.spec.ts @@ -134,6 +134,59 @@ describe('WebChannelHandler', () => { expect(handler.getName()).toEqual('web-channel'); }); + it('should allow the request if the origin is in the allowed domains', async () => { + const req = { + headers: { + origin: 'https://example.com', + }, + method: 'GET', + } as unknown as Request; + + const res = { + set: jest.fn(), + } as any; + + jest.spyOn(handler, 'getSettings').mockResolvedValue({ + allowed_domains: + 'https://example.com/,https://test.com,http://invalid-url', + }); + + await expect(handler['validateCors'](req, res)).resolves.not.toThrow(); + + expect(res.set).toHaveBeenCalledWith( + 'Access-Control-Allow-Origin', + 'https://example.com', + ); + expect(res.set).toHaveBeenCalledWith( + 'Access-Control-Allow-Credentials', + 'true', + ); + }); + + it('should reject the request if the origin is not in the allowed domains', async () => { + const req = { + headers: { + origin: 'https://notallowed.com', + }, + method: 'GET', + } as unknown as Request; + + jest.spyOn(handler, 'getSettings').mockResolvedValue({ + allowed_domains: + 'https://example.com/,https://test.com,http://invalid-url', + }); + + const res = { + set: jest.fn(), + } as any; + + await expect(handler['validateCors'](req, res)).rejects.toThrow( + 'CORS - Domain not allowed!', + ); + + expect(res.set).toHaveBeenCalledWith('Access-Control-Allow-Origin', ''); + }); + it('should format text properly', () => { const formatted = handler._textFormat(textMessage, {}); expect(formatted).toEqual(webText); From e7109ba036b15feda17180a9411f69945f1023e7 Mon Sep 17 00:00:00 2001 From: Mohamed Marrouchi Date: Sun, 1 Dec 2024 08:37:06 +0100 Subject: [PATCH 3/5] fix: include wildcard support --- api/src/extensions/channels/web/base-web-channel.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/extensions/channels/web/base-web-channel.ts b/api/src/extensions/channels/web/base-web-channel.ts index 389ec00d..0797d3c6 100644 --- a/api/src/extensions/channels/web/base-web-channel.ts +++ b/api/src/extensions/channels/web/base-web-channel.ts @@ -317,10 +317,10 @@ export default abstract class BaseWebChannelHandler< .some((origin: string) => { // If we find a whitelisted origin, send the Access-Control-Allow-Origin header // to greenlight the request. - return origin === req.headers.origin || origin === '*'; + return origin === req.headers.origin; }); - if (!foundOrigin) { + if (!foundOrigin && !origins.includes('*')) { // For HTTP requests, set the Access-Control-Allow-Origin header to '', which the browser will // interpret as, 'no way Jose.' res.set('Access-Control-Allow-Origin', ''); From 5c21212a96587bac5e4f8b423fbc64a2e5b64f98 Mon Sep 17 00:00:00 2001 From: Mohamed Marrouchi Date: Sun, 1 Dec 2024 08:38:16 +0100 Subject: [PATCH 4/5] fix: adjust log message --- api/src/extensions/channels/web/base-web-channel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/extensions/channels/web/base-web-channel.ts b/api/src/extensions/channels/web/base-web-channel.ts index 0797d3c6..e9c1497f 100644 --- a/api/src/extensions/channels/web/base-web-channel.ts +++ b/api/src/extensions/channels/web/base-web-channel.ts @@ -304,7 +304,7 @@ export default abstract class BaseWebChannelHandler< return new URL(origin.trim()).origin; } catch (error) { this.logger.error( - `Invalid URL in allowed domains: ${origin}`, + `Web Channel Handler : Invalid URL in allowed domains: ${origin}`, error, ); return null; From 2407ce86b92bf3da612bec2c3ea26dab1720ccf4 Mon Sep 17 00:00:00 2001 From: Mohamed Marrouchi Date: Mon, 2 Dec 2024 16:21:36 +0100 Subject: [PATCH 5/5] fix: handle unsupported protocols --- .../channels/web/base-web-channel.ts | 105 ++++++++++-------- 1 file changed, 56 insertions(+), 49 deletions(-) diff --git a/api/src/extensions/channels/web/base-web-channel.ts b/api/src/extensions/channels/web/base-web-channel.ts index e9c1497f..bdf3629f 100644 --- a/api/src/extensions/channels/web/base-web-channel.ts +++ b/api/src/extensions/channels/web/base-web-channel.ts @@ -294,57 +294,64 @@ export default abstract class BaseWebChannelHandler< res: Response | SocketResponse, ) { const settings = await this.getSettings(); - // If we have an origin header... - if (req.headers && req.headers.origin) { - // Get the allowed origins - const origins: string[] = settings.allowed_domains.split(','); - const foundOrigin = origins - .map((origin) => { - try { - return new URL(origin.trim()).origin; - } catch (error) { - this.logger.error( - `Web Channel Handler : Invalid URL in allowed domains: ${origin}`, - error, - ); - return null; - } - }) - .filter( - (normalizedOrigin): normalizedOrigin is string => - normalizedOrigin !== null, - ) - .some((origin: string) => { - // If we find a whitelisted origin, send the Access-Control-Allow-Origin header - // to greenlight the request. - return origin === req.headers.origin; - }); - if (!foundOrigin && !origins.includes('*')) { - // For HTTP requests, set the Access-Control-Allow-Origin header to '', which the browser will - // interpret as, 'no way Jose.' - res.set('Access-Control-Allow-Origin', ''); - this.logger.debug( - 'Web Channel Handler : No origin found ', - req.headers.origin, - ); - throw new Error('CORS - Domain not allowed!'); - } else { - res.set('Access-Control-Allow-Origin', req.headers.origin); - } - // Determine whether or not to allow cookies to be passed cross-origin - res.set('Access-Control-Allow-Credentials', 'true'); - // This header lets a server whitelist headers that browsers are allowed to access - res.set('Access-Control-Expose-Headers', ''); - // Handle preflight requests - if (req.method == 'OPTIONS') { - res.set('Access-Control-Allow-Methods', 'GET, POST'); - res.set('Access-Control-Allow-Headers', 'content-type'); - } - return; + // Check if we have an origin header... + if (!req.headers?.origin) { + this.logger.debug('Web Channel Handler : No origin ', req.headers); + throw new Error('CORS - No origin provided!'); + } + + const originUrl = new URL(req.headers.origin); + const allowedProtocols = new Set(['http:', 'https:']); + if (!allowedProtocols.has(originUrl.protocol)) { + throw new Error('CORS - Invalid origin!'); + } + + // Get the allowed origins + const origins: string[] = settings.allowed_domains.split(','); + const foundOrigin = origins + .map((origin) => { + try { + return new URL(origin.trim()).origin; + } catch (error) { + this.logger.error( + `Web Channel Handler : Invalid URL in allowed domains: ${origin}`, + error, + ); + return null; + } + }) + .filter( + (normalizedOrigin): normalizedOrigin is string => + normalizedOrigin !== null, + ) + .some((origin: string) => { + // If we find a whitelisted origin, send the Access-Control-Allow-Origin header + // to greenlight the request. + return origin === originUrl.origin; + }); + + if (!foundOrigin && !origins.includes('*')) { + // For HTTP requests, set the Access-Control-Allow-Origin header to '', which the browser will + // interpret as, 'no way Jose.' + res.set('Access-Control-Allow-Origin', ''); + this.logger.debug( + 'Web Channel Handler : No origin found ', + req.headers.origin, + ); + throw new Error('CORS - Domain not allowed!'); + } else { + res.set('Access-Control-Allow-Origin', originUrl.origin); + } + // Determine whether or not to allow cookies to be passed cross-origin + res.set('Access-Control-Allow-Credentials', 'true'); + // This header lets a server whitelist headers that browsers are allowed to access + res.set('Access-Control-Expose-Headers', ''); + // Handle preflight requests + if (req.method == 'OPTIONS') { + res.set('Access-Control-Allow-Methods', 'GET, POST'); + res.set('Access-Control-Allow-Headers', 'content-type'); } - this.logger.debug('Web Channel Handler : No origin ', req.headers); - throw new Error('CORS - No origin provided!'); } /**