From 448944169a7bcb09ef3ca2bbc7be2b041481ad5d Mon Sep 17 00:00:00 2001 From: Rolf van Kleef Date: Sun, 28 Apr 2024 06:41:43 +0200 Subject: [PATCH] feat: Allow for users without password (#1715) This commit updates the security code to allow for users to be configured in `security.json` without a password value. These users can then only be logged in with using the `signalk-generate-token` script. Fixes https://github.com/SignalK/signalk-server/issues/1714. --- src/security.ts | 4 +-- src/tokensecurity.js | 45 ++++++++++++++++--------- test/security.js | 65 +++++++++++++++++++++++++++++++++---- test/servertestutilities.js | 16 ++++++++- 4 files changed, 105 insertions(+), 25 deletions(-) diff --git a/src/security.ts b/src/security.ts index 0154c24e2..a2f9a8782 100644 --- a/src/security.ts +++ b/src/security.ts @@ -63,7 +63,7 @@ export interface ACL { export interface User { username: string type: string - password: string + password?: string } export interface UserData { userId: string @@ -163,7 +163,7 @@ export interface SecurityStrategy { getUsers: (theConfig: SecurityConfig) => UserData[] addUser: ( theConfig: SecurityConfig, - user: UserWithPassword, + user: User, cb: ICallback ) => void updateUser: ( diff --git a/src/tokensecurity.js b/src/tokensecurity.js index c1f506547..a059ce422 100644 --- a/src/tokensecurity.js +++ b/src/tokensecurity.js @@ -277,6 +277,10 @@ module.exports = function (app, config) { resolve({ statusCode: 401, message: LOGIN_FAILED_MESSAGE }) return } + if (!user.password) { + resolve({ statusCode: 401, message: LOGIN_FAILED_MESSAGE }) + return + } bcrypt.compare(password, user.password, (err, matches) => { if (err) { @@ -411,23 +415,32 @@ module.exports = function (app, config) { function addUser(theConfig, user, callback) { assertConfigImmutability() - bcrypt.hash(user.password, passwordSaltRounds, (err, hash) => { - if (err) { - callback(err) - } else { - const newuser = { - username: user.userId, - type: user.type, - password: hash - } - if (!theConfig.users) { - theConfig.users = [] - } - theConfig.users.push(newuser) - options = theConfig - callback(err, theConfig) + const newUser = { + username: user.userId, + type: user.type + } + + function finish(newUser, err) { + if (!theConfig.users) { + theConfig.users = [] } - }) + theConfig.users.push(newUser) + options = theConfig + callback(err, theConfig) + } + + if (user.password) { + bcrypt.hash(user.password, passwordSaltRounds, (err, hash) => { + if (err) { + callback(err) + } else { + newUser.password = hash + finish(newUser, err) + } + }) + } else { + finish(newUser, undefined) + } } strategy.updateUser = (theConfig, username, updates, callback) => { diff --git a/test/security.js b/test/security.js index fa2f6e2c0..616d23aa0 100644 --- a/test/security.js +++ b/test/security.js @@ -2,12 +2,8 @@ const chai = require('chai') chai.Should() chai.use(require('chai-things')) const freeport = require('freeport-promise') -const Server = require('../lib') const fetch = require('node-fetch') -const http = require('http') -const promisify = require('util').promisify const WebSocket = require('ws') -const _ = require('lodash') const { startServerP, getReadOnlyToken, @@ -17,7 +13,9 @@ const { WRITE_USER_PASSWORD, LIMITED_USER_NAME, LIMITED_USER_PASSWORD, - WsPromiser + WsPromiser, + getToken, + NOPASSWORD_USER_NAME } = require('./servertestutilities') const limitedSteeringDelta = { @@ -70,7 +68,7 @@ const metaDelta = { } describe('Security', () => { - let server, url, port, readToken, writeToken, adminToken + let server, url, port, readToken, writeToken, adminToken, noPasswordToken before(async function () { this.timeout(5000) @@ -118,6 +116,7 @@ describe('Security', () => { readToken = await getReadOnlyToken(server) writeToken = await getWriteToken(server) adminToken = await getAdminToken(server) + noPasswordToken = await getToken(server, NOPASSWORD_USER_NAME) }) after(async function () { @@ -162,6 +161,33 @@ describe('Security', () => { result.status.should.equal(401) }) + it('login without password to user without password fails', async function () { + const result = await fetch(`${url}/signalk/v1/auth/login`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ + username: NOPASSWORD_USER_NAME, + }) + }) + result.status.should.equal(401) + }) + + it('login with incorrect password to user without password fails', async function () { + const result = await fetch(`${url}/signalk/v1/auth/login`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ + username: NOPASSWORD_USER_NAME, + password: 'incorrect', + }) + }) + result.status.should.equal(401) + }) + it('login works', async function () { const writeUserToken = await login(WRITE_USER_NAME, WRITE_USER_PASSWORD) writeUserToken.length.should.equal(151) @@ -199,6 +225,33 @@ describe('Security', () => { result.status.should.equal(200) }) + it('authorized read works for user without password', async function () { + const result = await fetch(`${url}/signalk/v1/api/vessels/self`, { + headers: { + Cookie: `JAUTHENTICATION=${noPasswordToken}` + } + }) + result.status.should.equal(200) + }) + + it('authorized read with Authorization header works for user without password', async function () { + const result = await fetch(`${url}/signalk/v1/api/vessels/self`, { + headers: { + Authorization: `JWT ${noPasswordToken}` + } + }) + result.status.should.equal(200) + }) + + it('authorized read with X-Authorization header works for user without password', async function () { + const result = await fetch(`${url}/signalk/v1/api/vessels/self`, { + headers: { + 'X-Authorization': `JWT ${noPasswordToken}` + } + }) + result.status.should.equal(200) + }) + it('admin request fails', async function () { const result = await fetch(`${url}/skServer/plugins`) result.status.should.equal(401) diff --git a/test/servertestutilities.js b/test/servertestutilities.js index fea17ab4b..708ece9c3 100755 --- a/test/servertestutilities.js +++ b/test/servertestutilities.js @@ -2,6 +2,7 @@ const WebSocket = require('ws') const _ = require('lodash') const promisify = require('util').promisify const fetch = require('node-fetch') +const jwt = require('jsonwebtoken') // Connects to the url via ws // and provides Promises that are either resolved within @@ -105,6 +106,7 @@ const LIMITED_USER_NAME = 'testuser' const LIMITED_USER_PASSWORD = 'verylimited' const ADMIN_USER_NAME = 'adminuser' const ADMIN_USER_PASSWORD = 'admin' +const NOPASSWORD_USER_NAME = 'nopassword' const serverTestConfigDirectory = () => require('path').join( __dirname, @@ -157,7 +159,11 @@ module.exports = { userId: ADMIN_USER_NAME, type: 'admin', password: ADMIN_USER_PASSWORD - }) + }), + promisify(s.app.securityStrategy.addUser)(props.securityConfig, { + userId: NOPASSWORD_USER_NAME, + type: 'admin', + }), ]) .then(() => { resolve(s) @@ -181,6 +187,14 @@ module.exports = { WRITE_USER_PASSWORD, getAdminToken: server => { return login(server, ADMIN_USER_NAME, ADMIN_USER_PASSWORD) + }, + NOPASSWORD_USER_NAME, + getToken: (server, username) => { + return jwt.sign({ + id: username + }, server.app.securityStrategy.securityConfig.secretKey, { + expiresIn: '1h' + }) } }