Skip to content

Commit

Permalink
Merge pull request #1478 from SignalK/fix-cors
Browse files Browse the repository at this point in the history
fix: CORS: security routes and origin handling
  • Loading branch information
tkurki authored Nov 20, 2022
2 parents 8a9e1f5 + d1df4fd commit 775e745
Show file tree
Hide file tree
Showing 7 changed files with 129 additions and 34 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
27 changes: 19 additions & 8 deletions packages/server-admin-ui/src/views/security/Settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,12 @@ import {
CardHeader,
CardBody,
CardFooter,
InputGroup,
InputGroupAddon,
Input,
Form,
Col,
Label,
FormGroup,
FormText,
Table,
Row,
} from 'reactstrap'
import EnableSecurity from './EnableSecurity'

Expand All @@ -29,6 +25,8 @@ export function fetchSecurityConfig() {
})
}

const adminUIOrigin = `${window.location.protocol}//${window.location.host}`

class Settings extends Component {
constructor(props) {
super(props)
Expand Down Expand Up @@ -71,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',
Expand Down Expand Up @@ -108,7 +107,7 @@ class Settings extends Component {
className="form-horizontal"
>
<FormGroup row>
<Col xs="0" md="2">
<Col xs="0" md="3">
<Label>Allow Readonly Access</Label>
</Col>
<Col md="9">
Expand Down Expand Up @@ -186,10 +185,10 @@ class Settings extends Component {
</Col>
</FormGroup>
<FormGroup row>
<Col md="2">
<Col md="3">
<Label htmlFor="text-input">Login Session Timeout</Label>
</Col>
<Col xs="12" md="9">
<Col xs="12" md="2">
<Input
type="text"
name="expiration"
Expand All @@ -202,7 +201,19 @@ class Settings extends Component {
</Col>
</FormGroup>
<FormGroup row>
<Col md="2">
<Col md="12">
<Label>
Simple CORS requests are allowed from all hosts by
default. You can restrict CORS requests to named hosts
by configuring allowed CORS origins below. The host
where this page is loaded from is automatically included
in the allowed CORS origins so that the Admin UI
continues to work.
</Label>
</Col>
</FormGroup>{' '}
<FormGroup row>
<Col md="3">
<Label htmlFor="text-input">Allowed CORS origins</Label>
</Col>
<Col xs="12" md="9">
Expand Down
42 changes: 42 additions & 0 deletions public/examples/loginform.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<!DOCTYPE html>
<html lang="en">

<head>
<meta charset="utf-8">
<title>Login form</title>
</head>

<body>
<form id="loginForm">
<input type="text" id="username" /><br />
<input type="password" id="password" /><br />
<button type="submit">Submit</button>
<p id="result"></p>
</form>
</body>
<script>
window.onload = function () {
document.getElementById('loginForm').onsubmit = function () {
document.getElementById('result').innerHTML = ''
const username = document.getElementById('username').value
const password = document.getElementById('password').value
const body = JSON.stringify({ username, password })
fetch('http://localhost:3000/signalk/v1/auth/login', {
method: 'POST',
withCredentials: true,
headers: {
'Accept': 'application/json',
'Content-Type': 'application/json'
},
body
})
.then(r => r.json())
.then(r => {
document.getElementById('result').innerHTML = JSON.stringify(r)
})
return false;
}
}
</script>

</html>
60 changes: 60 additions & 0 deletions src/cors.ts
Original file line number Diff line number Diff line change
@@ -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(',')
}
}
26 changes: 2 additions & 24 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -72,8 +73,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)
Expand Down Expand Up @@ -560,26 +561,3 @@ function startInterfaces(app: ServerApp & WithConfig) {
}
})
}

function setupCors(app: any, { allowedCorsOrigins }: any) {
const corsDebug = createDebug('signalk-server:cors')

const corsOrigins = allowedCorsOrigins ? allowedCorsOrigins.split(',') : []
corsDebug(`corsOrigins:${corsOrigins.toString()}`)
const corsOptions: any = {
credentials: true
}
if (corsOrigins.length) {
corsOptions.origin = (origin: any, cb: any) => {
if (corsOrigins.indexOf(origin)) {
corsDebug(`Found CORS origin ${origin}`)
cb(undefined, origin)
} else {
const errorMsg = `CORS origin not allowed ${origin}`
corsDebug(errorMsg)
cb(new Error(errorMsg))
}
}
}
app.use(require('cors')(corsOptions))
}
3 changes: 2 additions & 1 deletion src/security.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ export interface SecurityConfig {
allow_readonly: boolean
allowNewUserRegistration: boolean
allowDeviceAccessRequests: boolean
allowedCorsOrigins?: string
expiration: string
devices: Device[]
secretKey: string
Expand Down Expand Up @@ -229,7 +230,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 {
Expand Down
4 changes: 3 additions & 1 deletion src/serverroutes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 775e745

Please sign in to comment.