diff --git a/app/middleware.js b/app/middleware.js index 3c9d166..6df3205 100644 --- a/app/middleware.js +++ b/app/middleware.js @@ -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), diff --git a/app/socket.js b/app/socket.js index b989c3d..c0798e0 100644 --- a/app/socket.js +++ b/app/socket.js @@ -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 }) diff --git a/app/ssh.js b/app/ssh.js index 5ffe9c2..ec8ce29 100644 --- a/app/ssh.js +++ b/app/ssh.js @@ -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 diff --git a/app/utils.js b/app/utils.js index 0e36dfe..3732a6b 100644 --- a/app/utils.js +++ b/app/utils.js @@ -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 } /** diff --git a/package.json b/package.json index 953dbc1..fdbfe0e 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/tests/ssh.test.js b/tests/ssh.test.js index a297dda..d41400b 100644 --- a/tests/ssh.test.js +++ b/tests/ssh.test.js @@ -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,10 +13,10 @@ 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) { + SSHConnectionError: jest.fn(function (message) { this.message = message }), handleError: jest.fn() @@ -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,96 +76,142 @@ 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 }) ) }) }) - }) - - 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) + it("should fail after max authentication attempts", () => { const mockCreds = { - host: "example.com", + host: "localhost", port: 22, - username: "user" + username: "user", + password: "wrongpass" } - mockSSH2Client.on.mockImplementation((event, callback) => { - if (event === "ready") { - callback() - } + const attempts = DEFAULTS.MAX_AUTH_ATTEMPTS + 1 + mockSSH2Client.connect.mockImplementation(() => { + process.nextTick(() => { + registeredEventHandlers.error(new Error("Authentication failed")) + }) }) - return sshConnection.connect(mockCreds).then(() => { - expect(mockSSH2Client.connect).toHaveBeenCalledWith( - expect.objectContaining({ - privateKey: mockConfig.user.privatekey, - host: mockCreds.host, - port: mockCreds.port, - username: mockCreds.username - }) - ) + 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) }) }) - it("should handle invalid private key format", () => { - mockConfig.user.privatekey = "invalid-key-format" - sshConnection = new SSHConnection(mockConfig) + describe("key authentication", () => { + const validPrivateKey = `-----BEGIN RSA PRIVATE KEY----- +MIIEpTestKeyContentHere +-----END RSA PRIVATE KEY-----` - const mockCreds = { - host: "example.com", - port: 22, - username: "user" - } + it("should try private key first when both password and key are provided", () => { + const mockCreds = { + host: "localhost", + port: 22, + username: "user", + password: "pass", + privatekey: validPrivateKey + } - return sshConnection.connect(mockCreds).catch(error => { - expect(error).toBeInstanceOf(SSHConnectionError) - expect(error.message).toBe("Invalid private key format") + return sshConnection.connect(mockCreds).then(() => { + expect(mockSSH2Client.connect).toHaveBeenCalledWith( + expect.objectContaining({ + privateKey: validPrivateKey, + username: mockCreds.username + }) + ) + }) + }) + + it("should fall back to password after key authentication failure", () => { + const mockCreds = { + host: "localhost", + port: 22, + username: "user", + password: "pass", + privatekey: validPrivateKey + } + + 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") + }) }) }) }) @@ -160,55 +221,52 @@ describe("SSHConnection", () => { 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() }) }) -}) \ No newline at end of file +})