Skip to content

Commit

Permalink
feat: Allow for users without password (#1715)
Browse files Browse the repository at this point in the history
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 #1714.
  • Loading branch information
rhbvkleef committed Apr 28, 2024
1 parent 9108bc0 commit 4489441
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 25 deletions.
4 changes: 2 additions & 2 deletions src/security.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export interface ACL {
export interface User {
username: string
type: string
password: string
password?: string
}
export interface UserData {
userId: string
Expand Down Expand Up @@ -163,7 +163,7 @@ export interface SecurityStrategy {
getUsers: (theConfig: SecurityConfig) => UserData[]
addUser: (
theConfig: SecurityConfig,
user: UserWithPassword,
user: User,
cb: ICallback<SecurityConfig>
) => void
updateUser: (
Expand Down
45 changes: 29 additions & 16 deletions src/tokensecurity.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) => {
Expand Down
65 changes: 59 additions & 6 deletions test/security.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -17,7 +13,9 @@ const {
WRITE_USER_PASSWORD,
LIMITED_USER_NAME,
LIMITED_USER_PASSWORD,
WsPromiser
WsPromiser,
getToken,
NOPASSWORD_USER_NAME
} = require('./servertestutilities')

const limitedSteeringDelta = {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 () {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 15 additions & 1 deletion test/servertestutilities.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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'
})
}
}

Expand Down

0 comments on commit 4489441

Please sign in to comment.