From 61157e9be98225d7fe5671bcb7c55cb2afe338a0 Mon Sep 17 00:00:00 2001 From: Teppo Kurki Date: Sat, 1 Oct 2022 20:55:09 +0300 Subject: [PATCH 1/8] fix: cors at /signalk/v1/auth/login CORS handling was not working in any of the paths related to authentication, because cors middleware was added after security path handlers were added. This changes the order so that CORS works with these paths also. --- src/index.ts | 2 +- src/security.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index 8b0744ce5..0ed15b055 100644 --- a/src/index.ts +++ b/src/index.ts @@ -72,8 +72,8 @@ class Server { app.logging = require('./logging')(app) app.version = '0.0.1' - startSecurity(app, opts ? opts.securityConfig : null) setupCors(app, getSecurityConfig(app)) + startSecurity(app, opts ? opts.securityConfig : null) require('./serverroutes')(app, saveSecurityConfig, getSecurityConfig) require('./put').start(app) diff --git a/src/security.ts b/src/security.ts index 387719cd0..be860ec24 100644 --- a/src/security.ts +++ b/src/security.ts @@ -229,7 +229,7 @@ export function getSecurityConfig( app: WithConfig & WithSecurityStrategy, forceRead = false ) { - if (!forceRead && app.securityStrategy.configFromArguments) { + if (!forceRead && app.securityStrategy?.configFromArguments) { return app.securityStrategy.securityConfig } else { try { From d2625ec56e46349398a655a85d72f088ac4b09f5 Mon Sep 17 00:00:00 2001 From: Teppo Kurki Date: Sat, 1 Oct 2022 20:57:38 +0300 Subject: [PATCH 2/8] fix: CORS origin check CORS origin check was broken in two ways: - the first configured allowed origin was not allowed - any other origin would be allowed access This fixes the check and changes logging denied requests to debug only. --- src/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index 0ed15b055..9e4f15e88 100644 --- a/src/index.ts +++ b/src/index.ts @@ -571,13 +571,13 @@ function setupCors(app: any, { allowedCorsOrigins }: any) { } if (corsOrigins.length) { corsOptions.origin = (origin: any, cb: any) => { - if (corsOrigins.indexOf(origin)) { + if (origin === undefined || corsOrigins.indexOf(origin) >= 0) { corsDebug(`Found CORS origin ${origin}`) cb(undefined, origin) } else { const errorMsg = `CORS origin not allowed ${origin}` corsDebug(errorMsg) - cb(new Error(errorMsg)) + cb(errorMsg) } } } From e1d2911b23a53029ba580891854af2ffe2514b72 Mon Sep 17 00:00:00 2001 From: Teppo Kurki Date: Sat, 8 Oct 2022 19:25:55 +0300 Subject: [PATCH 3/8] feature: improve CORS origin config Ignore whitespace and trailing slash. --- src/index.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index 9e4f15e88..4f3b08492 100644 --- a/src/index.ts +++ b/src/index.ts @@ -564,7 +564,11 @@ function startInterfaces(app: ServerApp & WithConfig) { function setupCors(app: any, { allowedCorsOrigins }: any) { const corsDebug = createDebug('signalk-server:cors') - const corsOrigins = allowedCorsOrigins ? allowedCorsOrigins.split(',') : [] + const corsOrigins = allowedCorsOrigins + ? allowedCorsOrigins + .split(',') + .map((s: string) => s.trim().replace(/\/*$/, '')) + : [] corsDebug(`corsOrigins:${corsOrigins.toString()}`) const corsOptions: any = { credentials: true From a4633a42b448700eb58ed203187968d88bb37d80 Mon Sep 17 00:00:00 2001 From: Teppo Kurki Date: Sat, 8 Oct 2022 19:26:23 +0300 Subject: [PATCH 4/8] chore: improve CORS debug logging --- src/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index 4f3b08492..f0041889c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -576,10 +576,10 @@ function setupCors(app: any, { allowedCorsOrigins }: any) { if (corsOrigins.length) { corsOptions.origin = (origin: any, cb: any) => { if (origin === undefined || corsOrigins.indexOf(origin) >= 0) { - corsDebug(`Found CORS origin ${origin}`) + corsDebug(`${origin} OK`) cb(undefined, origin) } else { - const errorMsg = `CORS origin not allowed ${origin}` + const errorMsg = `${origin} not allowed` corsDebug(errorMsg) cb(errorMsg) } From aa6c7fec68dd121d95956ef6d07d93a8abad3389 Mon Sep 17 00:00:00 2001 From: Teppo Kurki Date: Sun, 9 Oct 2022 09:34:22 +0300 Subject: [PATCH 5/8] chore: add a page for testing login CORS --- public/examples/loginform.html | 42 ++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 public/examples/loginform.html diff --git a/public/examples/loginform.html b/public/examples/loginform.html new file mode 100644 index 000000000..b0e5b9f05 --- /dev/null +++ b/public/examples/loginform.html @@ -0,0 +1,42 @@ + + + + + + Login form + + + +
+
+
+ +

+
+ + + + \ No newline at end of file From eff11fbfd9fcbefc0eea050c7db56bb2fe67862e Mon Sep 17 00:00:00 2001 From: Teppo Kurki Date: Tue, 8 Nov 2022 19:49:26 +0200 Subject: [PATCH 6/8] feature: align form columns --- packages/server-admin-ui/src/views/security/Settings.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/server-admin-ui/src/views/security/Settings.js b/packages/server-admin-ui/src/views/security/Settings.js index 9b169254c..70de4cfd1 100644 --- a/packages/server-admin-ui/src/views/security/Settings.js +++ b/packages/server-admin-ui/src/views/security/Settings.js @@ -108,7 +108,7 @@ class Settings extends Component { className="form-horizontal" > - + @@ -186,10 +186,10 @@ class Settings extends Component { - + - + - + From eaa210bacc5b3265dc24678d9b940a47ce9b0030 Mon Sep 17 00:00:00 2001 From: Teppo Kurki Date: Tue, 8 Nov 2022 20:21:04 +0200 Subject: [PATCH 7/8] refactor: remove unused imports --- packages/server-admin-ui/src/views/security/Settings.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/server-admin-ui/src/views/security/Settings.js b/packages/server-admin-ui/src/views/security/Settings.js index 70de4cfd1..7bb9539c7 100644 --- a/packages/server-admin-ui/src/views/security/Settings.js +++ b/packages/server-admin-ui/src/views/security/Settings.js @@ -6,16 +6,12 @@ import { CardHeader, CardBody, CardFooter, - InputGroup, - InputGroupAddon, Input, Form, Col, Label, FormGroup, FormText, - Table, - Row, } from 'reactstrap' import EnableSecurity from './EnableSecurity' From d1df4fd1abfa396f6e8a22f8d895200a468095ef Mon Sep 17 00:00:00 2001 From: Teppo Kurki Date: Sat, 12 Nov 2022 21:32:24 +0200 Subject: [PATCH 8/8] feature: add Admin UI cors origin Add special handling for Admin UI CORS origin and a brief explanation of the mechanism in the security configuration form. --- package.json | 1 + .../src/views/security/Settings.js | 15 +++++ src/cors.ts | 60 +++++++++++++++++++ src/index.ts | 28 +-------- src/security.ts | 1 + src/serverroutes.ts | 4 +- 6 files changed, 81 insertions(+), 28 deletions(-) create mode 100644 src/cors.ts diff --git a/package.json b/package.json index 46cd637f4..dffe0c64b 100644 --- a/package.json +++ b/package.json @@ -145,6 +145,7 @@ "@types/busboy": "^1.5.0", "@types/chai": "^4.2.15", "@types/command-exists": "^1.2.0", + "@types/cors": "^2.8.12", "@types/express": "^4.17.1", "@types/lodash": "^4.14.139", "@types/mocha": "^8.2.0", diff --git a/packages/server-admin-ui/src/views/security/Settings.js b/packages/server-admin-ui/src/views/security/Settings.js index 7bb9539c7..0f7320c13 100644 --- a/packages/server-admin-ui/src/views/security/Settings.js +++ b/packages/server-admin-ui/src/views/security/Settings.js @@ -25,6 +25,8 @@ export function fetchSecurityConfig() { }) } +const adminUIOrigin = `${window.location.protocol}//${window.location.host}` + class Settings extends Component { constructor(props) { super(props) @@ -67,6 +69,7 @@ class Settings extends Component { allowNewUserRegistration: this.state.allowNewUserRegistration, allowDeviceAccessRequests: this.state.allowDeviceAccessRequests, allowedCorsOrigins: this.state.allowedCorsOrigins, + adminUIOrigin, } fetch(`${window.serverRoutesPrefix}/security/config`, { method: 'PUT', @@ -197,6 +200,18 @@ class Settings extends Component { + + + + + {' '} diff --git a/src/cors.ts b/src/cors.ts new file mode 100644 index 000000000..ccb6a12eb --- /dev/null +++ b/src/cors.ts @@ -0,0 +1,60 @@ +import { IRouter } from 'express' +import { createDebug } from './debug' +import { SecurityConfig } from './security' +import cors, { CorsOptions } from 'cors' + +type OriginCallback = (err: Error | null, origin?: string) => void + +export function setupCors( + app: IRouter, + { allowedCorsOrigins }: SecurityConfig +) { + const corsDebug = createDebug('signalk-server:cors') + + const corsOrigins = allowedCorsOrigins + ? allowedCorsOrigins + .split(',') + .map((s: string) => s.trim().replace(/\/*$/, '')) + : [] + corsDebug(`corsOrigins:${corsOrigins.toString()}`) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const corsOptions: CorsOptions = { + credentials: true + } + if (corsOrigins.length) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + corsOptions.origin = (origin: string | undefined, cb: OriginCallback) => { + if (origin === undefined || corsOrigins.indexOf(origin) >= 0) { + corsDebug(`${origin} OK`) + cb(null, origin) + } else { + const errorMsg = `${origin} not allowed` + corsDebug(errorMsg) + cb(new Error(errorMsg)) + } + } + } + app.use(cors(corsOptions)) +} + +export const handleAdminUICORSOrigin = ( + securityConfig: SecurityConfig & { adminUIOrigin: string } +) => { + let allowedCorsOrigins: string[] = [] + if ( + securityConfig.adminUIOrigin && + securityConfig.allowedCorsOrigins && + securityConfig.allowedCorsOrigins.length > 0 + ) { + allowedCorsOrigins = securityConfig.allowedCorsOrigins?.split(',') + if (allowedCorsOrigins.indexOf(securityConfig.adminUIOrigin) === -1) { + allowedCorsOrigins.push(securityConfig.adminUIOrigin) + } + } + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { adminUIOrigin, ...configWithoutAdminUIOrigin } = securityConfig + return { + ...configWithoutAdminUIOrigin, + allowedCorsOrigins: allowedCorsOrigins.join(',') + } +} diff --git a/src/index.ts b/src/index.ts index f0041889c..852f19f2f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -45,6 +45,7 @@ import { saveSecurityConfig, startSecurity } from './security.js' +import { setupCors } from './cors' import SubscriptionManager from './subscriptionmanager' import { Delta } from './types' const debug = createDebug('signalk-server') @@ -560,30 +561,3 @@ function startInterfaces(app: ServerApp & WithConfig) { } }) } - -function setupCors(app: any, { allowedCorsOrigins }: any) { - const corsDebug = createDebug('signalk-server:cors') - - const corsOrigins = allowedCorsOrigins - ? allowedCorsOrigins - .split(',') - .map((s: string) => s.trim().replace(/\/*$/, '')) - : [] - corsDebug(`corsOrigins:${corsOrigins.toString()}`) - const corsOptions: any = { - credentials: true - } - if (corsOrigins.length) { - corsOptions.origin = (origin: any, cb: any) => { - if (origin === undefined || corsOrigins.indexOf(origin) >= 0) { - corsDebug(`${origin} OK`) - cb(undefined, origin) - } else { - const errorMsg = `${origin} not allowed` - corsDebug(errorMsg) - cb(errorMsg) - } - } - } - app.use(require('cors')(corsOptions)) -} diff --git a/src/security.ts b/src/security.ts index be860ec24..58e559f88 100644 --- a/src/security.ts +++ b/src/security.ts @@ -98,6 +98,7 @@ export interface SecurityConfig { allow_readonly: boolean allowNewUserRegistration: boolean allowDeviceAccessRequests: boolean + allowedCorsOrigins?: string expiration: string devices: Device[] secretKey: string diff --git a/src/serverroutes.ts b/src/serverroutes.ts index dc7984cca..1bf134ad0 100644 --- a/src/serverroutes.ts +++ b/src/serverroutes.ts @@ -36,6 +36,7 @@ import { writeSettingsFile } from './config/config' import { SERVERROUTESPREFIX } from './constants' +import { handleAdminUICORSOrigin } from './cors' import { createDebug, listKnownDebugs } from './debug' import { getAuthor, Package, restoreModules } from './modules' import { getHttpPort, getSslPort } from './ports' @@ -205,7 +206,8 @@ module.exports = function ( } let config = getSecurityConfig(app) - config = app.securityStrategy.setConfig(config, req.body) + const configToSave = handleAdminUICORSOrigin(req.body) + config = app.securityStrategy.setConfig(config, configToSave) saveSecurityConfig(app, config, (err) => { if (err) { console.log(err)