From 6769d871ffbb0d448ef3f7b9b2396ef64c122b00 Mon Sep 17 00:00:00 2001 From: Scott Bender Date: Sat, 16 Jun 2018 10:58:56 -0400 Subject: [PATCH 1/2] feature: remove possible conflicts with third party apps Specifically done to enable security for node red --- lib/interfaces/rest.js | 2 +- lib/put.js | 4 +- lib/tokensecurity.js | 122 ++++++++++++++++++++++++----------------- 3 files changed, 74 insertions(+), 54 deletions(-) diff --git a/lib/interfaces/rest.js b/lib/interfaces/rest.js index e3f71b0c9..5d0c7c318 100644 --- a/lib/interfaces/rest.js +++ b/lib/interfaces/rest.js @@ -73,7 +73,7 @@ module.exports = function (app) { } } - var last = app.deltaCache.buildFull(req.user, path) + var last = app.deltaCache.buildFull(req.skUser, path) if (last) { for (var i in path) { diff --git a/lib/put.js b/lib/put.js index 4d31dd48f..e329c764d 100644 --- a/lib/put.js +++ b/lib/put.js @@ -69,8 +69,8 @@ module.exports = { } else if (actionResult.state === State.completed) { res.status(actionResult.resultStatus || 200) } else if (actionResult.state === State.pending) { - if (req.user) { - actions[actionResult.action.id].user = req.user.id + if (req.skUser) { + actions[actionResult.action.id].user = req.skUser.id } res.status(202) diff --git a/lib/tokensecurity.js b/lib/tokensecurity.js index 92eab854a..d241afbba 100644 --- a/lib/tokensecurity.js +++ b/lib/tokensecurity.js @@ -91,6 +91,30 @@ module.exports = function (app, config) { } } + function adminAuthenticationMiddleware (redirect) { + return function (req, res, next) { + if (!getIsEnabled()) { + return next() + } + + if (req.skIsAuthenticated && req.skUser) { + if (req.skUser.type == 'admin') { + return next() + } else if (req.skUser.id === 'AUTO' && redirect) { + res.redirect('/@signalk/server-admin-ui/#/login') + } else { + res.status(401) + res.send('Permission Denied') + } + } else if (redirect) { + res.redirect('/@signalk/server-admin-ui/#/login') + } else { + res.status(401) + res.send('Permission Denied') + } + } + } + function setupApp () { app.use(require('body-parser').urlencoded({ extended: true })) @@ -138,11 +162,11 @@ module.exports = function (app, config) { } }) - var do_redir = http_authorize(true) + var do_redir = http_authorize(false) - app.use('/apps', http_authorize(true)) - app.use('/appstore', http_authorize(true)) - app.use('/plugins', http_authorize(true)) + app.use('/apps', http_authorize(false)) + app.use('/appstore', http_authorize(false)) + app.use('/plugins', http_authorize(false)) app.use('/restart', http_authorize(false)) app.use('/security', http_authorize(false)) app.use('/vessel', http_authorize(false)) @@ -155,32 +179,15 @@ module.exports = function (app, config) { res.send('Logout OK') }) - function adminAuthenticationMiddleware (redirect) { - return function (req, res, next) { - if (!getIsEnabled()) { - return next() - } - - debug('isAuthenticated: ' + req.isAuthenticated) - if (req.isAuthenticated) { - if (req.user.type == 'admin') { - return next() - } - } - res.status(401) - res.send('Permission Denied') - } - } - function writeAuthenticationMiddleware (redirect) { return function (req, res, next) { if (!getIsEnabled()) { return next() } - debug('isAuthenticated: ' + req.isAuthenticated) - if (req.isAuthenticated) { - if (req.user.type === 'admin' || req.user.type === 'readwrite') { + debug('skIsAuthenticated: ' + req.skIsAuthenticated) + if (req.skIsAuthenticated) { + if (req.skUser.type === 'admin' || req.skUser.type === 'readwrite') { return next() } } @@ -194,11 +201,11 @@ module.exports = function (app, config) { if (!getIsEnabled()) { return next() } - debug('isAuthenticated: ' + req.isAuthenticated) - if (req.isAuthenticated) { + debug('skIsAuthenticated: ' + req.skIsAuthenticated) + if (req.skIsAuthenticated) { if ( ['admin', 'readonly', 'readwrite'].find( - type => req.user.type == type + type => req.skUser.type == type ) ) { return next() @@ -211,7 +218,7 @@ module.exports = function (app, config) { app.use('/restart', adminAuthenticationMiddleware(false)) app.use('/plugins', adminAuthenticationMiddleware(true)) - app.use('/appstore', adminAuthenticationMiddleware(true)) + app.use('/appstore', adminAuthenticationMiddleware(false)) app.use('/security', adminAuthenticationMiddleware(false)) app.use('/settings', adminAuthenticationMiddleware(false)) app.use('/providers', adminAuthenticationMiddleware(false)) @@ -226,6 +233,11 @@ module.exports = function (app, config) { app.put('/signalk/v1/*', writeAuthenticationMiddleware(false)) } + strategy.addAdminMiddleware = function (path) { + app.use(path, http_authorize(true)) + app.use(path, adminAuthenticationMiddleware(true)) + } + strategy.generateToken = function (req, res, next, id, expiration) { var configuration = getConfiguration() var payload = { id: id } @@ -235,24 +247,29 @@ module.exports = function (app, config) { res.send(token) } + strategy.allowReadOnly = function () { + var configuration = getConfiguration() + return configuration.allow_readonly + } + strategy.allowRestart = function (req) { - return req.isAuthenticated && req.user.type == 'admin' + return req.skIsAuthenticated && req.skUser.type == 'admin' } strategy.allowConfigure = function (req) { - return req.isAuthenticated && req.user.type == 'admin' + return req.skIsAuthenticated && req.skUser.type == 'admin' } strategy.getLoginStatus = function (req) { var configuration = getConfiguration() var result = { - status: req.isAuthenticated ? 'loggedIn' : 'notLoggedIn', + status: req.skIsAuthenticated ? 'loggedIn' : 'notLoggedIn', readOnlyAccess: configuration.allow_readonly, authenticationRequired: true } - if (req.isAuthenticated) { - result.userLevel = req.user.type - result.username = req.user.id + if (req.skIsAuthenticated) { + result.userLevel = req.skUser.type + result.username = req.skUser.id } if (configuration.users.length == 0) { result.noUsers = true @@ -366,8 +383,8 @@ module.exports = function (app, config) { strategy.shouldAllowWrite = function (req, delta) { if ( - req.user && - (req.user.type === 'admin' || req.user.type === 'readwrite') + req.skUser && + (req.skUser.type === 'admin' || req.skUser.type === 'readwrite') ) { var context = delta.context === app.selfContext ? 'vessels.self' : delta.context @@ -380,7 +397,7 @@ module.exports = function (app, config) { return update.values.find(valuePath => { return ( strategy.checkACL( - req.user.id, + req.skUser.id, context, valuePath.path, source, @@ -398,12 +415,12 @@ module.exports = function (app, config) { strategy.shouldAllowPut = function (req, context, source, path) { if ( - req.user && - (req.user.type === 'admin' || req.user.type === 'readwrite') + req.skUser && + (req.skUser.type === 'admin' || req.skUser.type === 'readwrite') ) { var context = context === app.selfContext ? 'vessels.self' : context - return strategy.checkACL(req.user.id, context, path, source, 'put') + return strategy.checkACL(req.skUser.id, context, path, source, 'put') } return false } @@ -498,7 +515,7 @@ module.exports = function (app, config) { if (!token || error) { if (configuration.allow_readonly) { - req.user = { id: 'AUTO', type: 'readonly' } + req.skUser = { id: 'AUTO', type: 'readonly' } return } else { if (!error) { @@ -520,9 +537,9 @@ module.exports = function (app, config) { throw error } - req.user = payload - req.user.type = user.type - req.isAuthenticated = true + req.skUser = payload + req.skUser.type = user.type + req.skIsAuthenticated = true } strategy.checkACL = (id, context, path, source, operation) => { @@ -632,9 +649,9 @@ module.exports = function (app, config) { ) if (user) { debug('authorized') - req.user = decoded - req.user.type = user.type - req.isAuthenticated = true + req.skUser = decoded + req.skUser.type = user.type + req.skIsAuthenticated = true req.userLoggedIn = true next() return @@ -646,7 +663,7 @@ module.exports = function (app, config) { } if (configuration.allow_readonly) { - req.isAuthenticated = false + req.skIsAuthenticated = false next() } else { res.clearCookie('JAUTHENTICATION') @@ -657,14 +674,17 @@ module.exports = function (app, config) { debug('no token') if (configuration.allow_readonly && !forLoginStatus) { - req.user = { id: 'AUTO', type: 'readonly' } - req.isAuthenticated = true + req.skUser = { id: 'AUTO', type: 'readonly' } + req.skIsAuthenticated = true return next() } else { - req.isAuthenticated = false + req.skIsAuthenticated = false if (forLoginStatus) { next() + } else if (redirect) { + debug('redirecting to login') + res.redirect('/@signalk/server-admin-ui/#/login') } else { res.status(401).send('Unauthorized') } From 64d0d9243a405a86f40b443e7d3be3ac3e0ae308 Mon Sep 17 00:00:00 2001 From: Scott Bender Date: Fri, 22 Jun 2018 15:11:15 -0400 Subject: [PATCH 2/2] fix: ws security failing because of user property name change --- lib/interfaces/ws.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/interfaces/ws.js b/lib/interfaces/ws.js index 1dc118a0e..fe0f8a98f 100644 --- a/lib/interfaces/ws.js +++ b/lib/interfaces/ws.js @@ -115,7 +115,7 @@ module.exports = function (app) { var aclFilter = delta => { var filtered = app.securityStrategy.filterReadDelta( - spark.request.user, + spark.request.skUser, delta ) if (filtered) { @@ -189,7 +189,7 @@ module.exports = function (app) { } } : spark.write.bind(this), - spark.request.user + spark.request.skUser ) } if ( @@ -255,13 +255,13 @@ module.exports = function (app) { if (!spark.query.subscribe || spark.query.subscribe === 'self') { app.deltaCache .getCachedDeltas( - spark.request.user, + spark.request.skUser, delta => delta.context === app.selfContext ) .forEach(delta => spark.write(delta)) } else if (spark.query.subscribe === 'all') { app.deltaCache - .getCachedDeltas(spark.request.user, delta => true) + .getCachedDeltas(spark.request.skUser, delta => true) .forEach(delta => spark.write(delta)) }