chore: finish up feat #379

This commit is contained in:
Bill Church 2024-11-29 22:28:56 +00:00
parent 44ba13cf67
commit 8fa1631196
No known key found for this signature in database
6 changed files with 180 additions and 88 deletions

View file

@ -28,25 +28,36 @@ const { HTTP } = require("./constants")
* If the authentication fails, the function will send a 401 Unauthorized response
* with the appropriate WWW-Authenticate header.
*/
// eslint-disable-next-line consistent-return
function createAuthMiddleware(config) {
// eslint-disable-next-line consistent-return
return (req, res, next) => {
if (config.user.name && config.user.password) {
// Check if username and either password or private key is configured
if (config.user.name && (config.user.password || config.user.privatekey)) {
req.session.sshCredentials = {
username: config.user.name,
password: config.user.password
username: config.user.name
}
// Add credentials based on what's available
if (config.user.privatekey) {
req.session.sshCredentials.privatekey = config.user.privatekey
}
if (config.user.password) {
req.session.sshCredentials.password = config.user.password
}
req.session.usedBasicAuth = true
return next()
}
// Scenario 2: Basic Auth
// If no configured credentials, fall back to Basic Auth
debug("auth: Basic Auth")
const credentials = basicAuth(req)
if (!credentials) {
res.setHeader(HTTP.AUTHENTICATE, HTTP.REALM)
return res.status(HTTP.UNAUTHORIZED).send(HTTP.AUTH_REQUIRED)
}
// Validate and sanitize credentials
req.session.sshCredentials = {
username: validator.escape(credentials.name),

View file

@ -117,12 +117,17 @@ class WebSSH2Socket extends EventEmitter {
}
initializeConnection(creds) {
// const self = this
debug(
`initializeConnection: ${this.socket.id}, INITIALIZING SSH CONNECTION: Host: ${creds.host}, creds: %O`,
maskSensitiveData(creds)
)
// Add private key from config if available
if (this.config.user.privatekey && !creds.privatekey) {
// eslint-disable-next-line no-param-reassign
creds.privatekey = this.config.user.privatekey
}
this.ssh
.connect(creds)
.then(() => {
@ -130,6 +135,7 @@ class WebSSH2Socket extends EventEmitter {
authenticated: true,
username: creds.username,
password: creds.password,
privatekey: creds.privatekey,
host: creds.host,
port: creds.port
})

View file

@ -93,7 +93,7 @@ class SSHConnection extends EventEmitter {
// Check if this is an authentication error and we haven't exceeded max attempts
if (this.authAttempts < DEFAULTS.MAX_AUTH_ATTEMPTS) {
this.authAttempts++
this.authAttempts += 1
debug(
`Authentication attempt ${this.authAttempts} failed, trying password authentication`
)
@ -172,13 +172,14 @@ class SSHConnection extends EventEmitter {
debug: createNamespacedDebug("ssh2")
}
// If useKey is true and we have a private key, use it
if (useKey && this.config.user.privatekey) {
// Try private key first if available and useKey is true
if (useKey && (creds.privatekey || this.config.user.privatekey)) {
debug("Using private key authentication")
if (!this.validatePrivateKey(this.config.user.privatekey)) {
const privateKey = creds.privatekey || this.config.user.privatekey
if (!this.validatePrivateKey(privateKey)) {
throw new SSHConnectionError("Invalid private key format")
}
config.privateKey = this.config.user.privatekey
config.privateKey = privateKey
} else if (creds.password) {
debug("Using password authentication")
config.password = creds.password

View file

@ -81,22 +81,39 @@ function getValidatedPort(portInput) {
/**
* Checks if the provided credentials object is valid.
* Valid credentials must have:
* - username (string)
* - host (string)
* - port (number)
* AND either:
* - password (string) OR
* - privatekey (string)
*
* @param {Object} creds - The credentials object.
* @param {string} creds.username - The username.
* @param {string} creds.password - The password.
* @param {string} [creds.password] - The password.
* @param {string} [creds.privatekey] - The private key.
* @param {string} creds.host - The host.
* @param {number} creds.port - The port.
* @returns {boolean} - Returns true if the credentials are valid, otherwise false.
*/
function isValidCredentials(creds) {
return !!(
const hasRequiredFields = !!(
creds &&
typeof creds.username === "string" &&
typeof creds.password === "string" &&
typeof creds.host === "string" &&
typeof creds.port === "number"
)
if (!hasRequiredFields) {
return false
}
// Must have either password or privatekey
const hasPassword = typeof creds.password === "string"
const hasPrivateKey = typeof creds.privatekey === "string"
return hasPassword || hasPrivateKey
}
/**

View file

@ -49,7 +49,7 @@
"start": "node index.js",
"lint": "eslint app",
"lint:fix": "eslint app --fix",
"watch": "NODE_ENV=development DEBUG=webssh* nodemon index.js -w app/ -w index.js -w config.json -w package.json",
"watch": "NODE_ENV=development DEBUG=webssh*,-webssh2:ssh2 nodemon index.js -w app/ -w index.js -w config.json -w package.json",
"test": "jest",
"release": "standard-version -a -s --release-as patch --commit-all",
"release:dry-run": "standard-version -a -s --release-as patch --dry-run",

View file

@ -1,10 +1,11 @@
/* eslint-disable jest/no-conditional-expect */
// server
// tests/ssh.test.js
const SSH2 = require("ssh2")
const SSHConnection = require("../app/ssh")
const { SSHConnectionError } = require("../app/errors")
const { maskSensitiveData } = require("../app/utils")
const { DEFAULTS } = require("../app/constants")
jest.mock("ssh2")
jest.mock("../app/logger", () => ({
@ -12,7 +13,7 @@ jest.mock("../app/logger", () => ({
logError: jest.fn()
}))
jest.mock("../app/utils", () => ({
maskSensitiveData: jest.fn(data => data)
maskSensitiveData: jest.fn((data) => data)
}))
jest.mock("../app/errors", () => ({
SSHConnectionError: jest.fn(function (message) {
@ -25,8 +26,11 @@ describe("SSHConnection", () => {
let sshConnection
let mockConfig
let mockSSH2Client
let registeredEventHandlers
beforeEach(() => {
registeredEventHandlers = {}
mockConfig = {
ssh: {
algorithms: {
@ -46,14 +50,25 @@ describe("SSHConnection", () => {
privatekey: null
}
}
sshConnection = new SSHConnection(mockConfig)
mockSSH2Client = {
on: jest.fn(),
connect: jest.fn(),
on: jest.fn((event, handler) => {
registeredEventHandlers[event] = handler
}),
connect: jest.fn(() => {
process.nextTick(() => {
// By default, emit ready event unless test modifies this behavior
if (registeredEventHandlers.ready) {
registeredEventHandlers.ready()
}
})
}),
shell: jest.fn(),
end: jest.fn()
}
SSH2.Client.mockImplementation(() => mockSSH2Client)
sshConnection = new SSHConnection(mockConfig)
})
afterEach(() => {
@ -61,154 +76,197 @@ describe("SSHConnection", () => {
})
describe("connect", () => {
it("should handle connection errors", () => {
it("should handle immediate connection errors", () => {
const mockCreds = {
host: "example.com",
host: "localhost",
port: 22,
username: "user",
password: "pass"
}
mockSSH2Client.on.mockImplementation((event, callback) => {
if (event === "error") {
callback(new Error("Connection failed"))
}
// Mock the connect method to throw an error immediately
mockSSH2Client.connect.mockImplementation(() => {
throw new Error("Spooky Error") // Immediate error
})
return sshConnection.connect(mockCreds).catch(error => {
return sshConnection.connect(mockCreds).catch((error) => {
expect(error).toBeInstanceOf(SSHConnectionError)
expect(error.message).toBe("Connection failed")
expect(error.message).toBe("Connection failed: Spooky Error")
})
})
it("should connect successfully with password", () => {
const mockCreds = {
host: "example.com",
host: "localhost",
port: 22,
username: "user",
password: "pass"
}
mockSSH2Client.on.mockImplementation((event, callback) => {
if (event === "ready") {
callback()
}
})
return sshConnection.connect(mockCreds).then(() => {
expect(mockSSH2Client.connect).toHaveBeenCalledWith(
expect.objectContaining({
host: "example.com",
port: 22,
username: "user",
password: "pass",
host: mockCreds.host,
port: mockCreds.port,
username: mockCreds.username,
password: mockCreds.password,
tryKeyboard: true
})
)
})
})
it("should fail after max authentication attempts", () => {
const mockCreds = {
host: "localhost",
port: 22,
username: "user",
password: "wrongpass"
}
const attempts = DEFAULTS.MAX_AUTH_ATTEMPTS + 1
mockSSH2Client.connect.mockImplementation(() => {
process.nextTick(() => {
registeredEventHandlers.error(new Error("Authentication failed"))
})
})
return sshConnection.connect(mockCreds).catch((error) => {
expect(error).toBeInstanceOf(SSHConnectionError)
expect(error.message).toBe("All authentication methods failed")
expect(mockSSH2Client.connect.mock.calls.length).toBe(attempts)
})
})
describe("key authentication", () => {
it("should use privateKey when provided in config", () => {
mockConfig.user.privatekey = "-----BEGIN RSA PRIVATE KEY-----\nMIIEpA...etc\n-----END RSA PRIVATE KEY-----"
sshConnection = new SSHConnection(mockConfig)
const validPrivateKey = `-----BEGIN RSA PRIVATE KEY-----
MIIEpTestKeyContentHere
-----END RSA PRIVATE KEY-----`
it("should try private key first when both password and key are provided", () => {
const mockCreds = {
host: "example.com",
host: "localhost",
port: 22,
username: "user"
username: "user",
password: "pass",
privatekey: validPrivateKey
}
mockSSH2Client.on.mockImplementation((event, callback) => {
if (event === "ready") {
callback()
}
})
return sshConnection.connect(mockCreds).then(() => {
expect(mockSSH2Client.connect).toHaveBeenCalledWith(
expect.objectContaining({
privateKey: mockConfig.user.privatekey,
host: mockCreds.host,
port: mockCreds.port,
privateKey: validPrivateKey,
username: mockCreds.username
})
)
})
})
it("should handle invalid private key format", () => {
mockConfig.user.privatekey = "invalid-key-format"
sshConnection = new SSHConnection(mockConfig)
it("should fall back to password after key authentication failure", () => {
const mockCreds = {
host: "example.com",
host: "localhost",
port: 22,
username: "user"
username: "user",
password: "pass",
privatekey: validPrivateKey
}
return sshConnection.connect(mockCreds).catch(error => {
let authAttempts = 0
mockSSH2Client.connect
.mockImplementationOnce(() => {
process.nextTick(() => {
authAttempts += 1
registeredEventHandlers.error(
new Error("Key authentication failed")
)
})
})
.mockImplementationOnce(() => {
process.nextTick(() => {
authAttempts += 1
registeredEventHandlers.ready()
})
})
return sshConnection.connect(mockCreds).then(() => {
expect(authAttempts).toBe(2)
expect(mockSSH2Client.connect).toHaveBeenCalledTimes(2)
// Verify second attempt used password
expect(mockSSH2Client.connect).toHaveBeenLastCalledWith(
expect.objectContaining({
password: mockCreds.password
})
)
})
})
it("should reject invalid private key format", () => {
const mockCreds = {
host: "localhost",
port: 22,
username: "user",
privatekey: "invalid-key-format"
}
return sshConnection.connect(mockCreds).catch((error) => {
expect(error).toBeInstanceOf(SSHConnectionError)
expect(error.message).toBe("Invalid private key format")
})
})
})
})
describe("shell", () => {
beforeEach(() => {
sshConnection.conn = mockSSH2Client
})
it("should open a shell successfully", () => {
it("should open shell successfully", () => {
const mockStream = {
on: jest.fn(),
stderr: { on: jest.fn() }
}
mockSSH2Client.shell.mockImplementation((options, callback) => {
callback(null, mockStream)
process.nextTick(() => callback(null, mockStream))
})
return sshConnection.shell().then(result => {
return sshConnection.shell().then((result) => {
expect(result).toBe(mockStream)
expect(sshConnection.stream).toBe(mockStream)
})
})
it("should handle shell errors", () => {
it("should handle shell creation errors", () => {
mockSSH2Client.shell.mockImplementation((options, callback) => {
callback(new Error("Shell error"))
process.nextTick(() => callback(new Error("Shell error")))
})
return sshConnection.shell().catch(error => {
return sshConnection.shell().catch((error) => {
expect(error.message).toBe("Shell error")
})
})
})
describe("resizeTerminal", () => {
it("should resize the terminal if stream exists", () => {
it("should resize terminal if stream exists", () => {
const mockStream = {
setWindow: jest.fn()
}
sshConnection.stream = mockStream
sshConnection.resizeTerminal(80, 24)
expect(mockStream.setWindow).toHaveBeenCalledWith(80, 24)
})
it("should not resize if stream does not exist", () => {
it("should do nothing if stream does not exist", () => {
sshConnection.stream = null
sshConnection.resizeTerminal(80, 24)
// No error should be thrown
expect(() => sshConnection.resizeTerminal(80, 24)).not.toThrow()
})
})
describe("end", () => {
it("should end the stream and connection", () => {
it("should close stream and connection", () => {
const mockStream = {
end: jest.fn()
}
@ -223,12 +281,11 @@ describe("SSHConnection", () => {
expect(sshConnection.conn).toBeNull()
})
it("should handle ending when stream and connection do not exist", () => {
it("should handle cleanup when no stream or connection exists", () => {
sshConnection.stream = null
sshConnection.conn = null
sshConnection.end()
// No error should be thrown
expect(() => sshConnection.end()).not.toThrow()
})
})
})