mirror of
https://github.com/Termix-SSH/Termix.git
synced 2026-10-31 07:35:24 +00:00
fix(hosts): reject or infer a missing authType on host create (#1664)
Creating an SSH host through the API without authType hit the NOT NULL auth_type column and came back as a 500. The create route now infers the type from the credential, key or password in the body, falls back to the host defaults, and otherwise answers 400. A non-string authType is a 400 on create and update.
This commit is contained in:
1 parent
764d034098
commit
65d10b37cf
4 files changed
+242
-2
No files matched your search
@@ -88,6 +88,23 @@ export function keepsStoredSshAuth(authType: unknown): boolean {
|
|||||||
return authType === undefined || authType === null;
|
return authType === undefined || authType === null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function isOptionalAuthType(value: unknown): boolean {
|
||||||
|
return value === undefined || value === null || typeof value === "string";
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The auth type a new SSH host's secrets point to when the body names none,
|
||||||
|
* as an API client creating a host from a credential does.
|
||||||
|
*/
|
||||||
|
export function impliedSshAuthType(
|
||||||
|
body: Record<string, unknown>,
|
||||||
|
): string | undefined {
|
||||||
|
if (body.credentialId) return "credential";
|
||||||
|
if (body.key) return "key";
|
||||||
|
if (body.password) return "password";
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
|
||||||
const FOLDER_PATH_SEPARATOR = " / ";
|
const FOLDER_PATH_SEPARATOR = " / ";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -31,8 +31,10 @@ import {
|
|||||||
import {
|
import {
|
||||||
applyHostKeyTypeUpdate,
|
applyHostKeyTypeUpdate,
|
||||||
containsOwnerPrivateAuthUpdate,
|
containsOwnerPrivateAuthUpdate,
|
||||||
|
impliedSshAuthType,
|
||||||
keepsStoredSshAuth,
|
keepsStoredSshAuth,
|
||||||
isNonEmptyString,
|
isNonEmptyString,
|
||||||
|
isOptionalAuthType,
|
||||||
isOptionalBoolean,
|
isOptionalBoolean,
|
||||||
isValidPort,
|
isValidPort,
|
||||||
normalizeProtocolEnableFields,
|
normalizeProtocolEnableFields,
|
||||||
@@ -259,7 +261,9 @@ router.post(
|
|||||||
!isNonEmptyString(ip) ||
|
!isNonEmptyString(ip) ||
|
||||||
!isValidPort(port) ||
|
!isValidPort(port) ||
|
||||||
!isOptionalBoolean(shareSshAuth) ||
|
!isOptionalBoolean(shareSshAuth) ||
|
||||||
!isOptionalBoolean(enableSsh)
|
!isOptionalBoolean(enableSsh) ||
|
||||||
|
!isOptionalAuthType(authType) ||
|
||||||
|
!isOptionalAuthType(authMethod)
|
||||||
) {
|
) {
|
||||||
sshLogger.warn("Invalid SSH data input validation failed", {
|
sshLogger.warn("Invalid SSH data input validation failed", {
|
||||||
operation: "host_create",
|
operation: "host_create",
|
||||||
@@ -292,7 +296,9 @@ router.post(
|
|||||||
const effectiveAuthType =
|
const effectiveAuthType =
|
||||||
authType ||
|
authType ||
|
||||||
authMethod ||
|
authMethod ||
|
||||||
(effectiveConnectionType !== "ssh" ? "password" : undefined);
|
(effectiveConnectionType !== "ssh"
|
||||||
|
? "password"
|
||||||
|
: impliedSshAuthType(hostData));
|
||||||
const effectiveUsername =
|
const effectiveUsername =
|
||||||
username || firstProtocolUsername(protocolAuthPatch);
|
username || firstProtocolUsername(protocolAuthPatch);
|
||||||
const effectiveName =
|
const effectiveName =
|
||||||
@@ -403,6 +409,13 @@ router.post(
|
|||||||
body: hostData,
|
body: hostData,
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
|
// Host defaults may have supplied it.
|
||||||
|
if (!sshDataObj.authType) {
|
||||||
|
return res.status(400).json({
|
||||||
|
error:
|
||||||
|
"authType is required (password, key, credential, agent or none)",
|
||||||
|
});
|
||||||
|
}
|
||||||
const result = await createCurrentHostRepository().createEncryptedForUser(
|
const result = await createCurrentHostRepository().createEncryptedForUser(
|
||||||
userId,
|
userId,
|
||||||
sshDataObj,
|
sshDataObj,
|
||||||
@@ -827,6 +840,8 @@ router.put(
|
|||||||
!isValidPort(port) ||
|
!isValidPort(port) ||
|
||||||
!isOptionalBoolean(shareSshAuth) ||
|
!isOptionalBoolean(shareSshAuth) ||
|
||||||
!isOptionalBoolean(enableSsh) ||
|
!isOptionalBoolean(enableSsh) ||
|
||||||
|
!isOptionalAuthType(authType) ||
|
||||||
|
!isOptionalAuthType(authMethod) ||
|
||||||
!hostId
|
!hostId
|
||||||
) {
|
) {
|
||||||
sshLogger.warn("Invalid SSH data input validation failed for update", {
|
sshLogger.warn("Invalid SSH data input validation failed for update", {
|
||||||
|
|||||||
@@ -0,0 +1,194 @@
|
|||||||
|
/**
|
||||||
|
* The create and update host routes against a real SQLite database. A body
|
||||||
|
* that leaves out authType must never reach the NOT NULL auth_type column.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import express, {
|
||||||
|
type NextFunction,
|
||||||
|
type Request,
|
||||||
|
type Response,
|
||||||
|
} from "express";
|
||||||
|
import request from "supertest";
|
||||||
|
import { eq, sql } from "drizzle-orm";
|
||||||
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
|
import type { DatabaseContext } from "../../../database/repositories/database-context.js";
|
||||||
|
|
||||||
|
const state = vi.hoisted(() => ({
|
||||||
|
ctx: null as null | { drizzle: unknown },
|
||||||
|
}));
|
||||||
|
|
||||||
|
const USER = "user-1";
|
||||||
|
const pass = (_req: Request, _res: Response, next: NextFunction) => next();
|
||||||
|
|
||||||
|
vi.mock("../../../database/db/index.js", () => ({
|
||||||
|
getDb: () => state.ctx!.drizzle,
|
||||||
|
getSqlite: () => null,
|
||||||
|
}));
|
||||||
|
vi.mock("../../../utils/database-save-trigger.js", () => ({
|
||||||
|
DatabaseSaveTrigger: {
|
||||||
|
triggerSave: vi.fn(),
|
||||||
|
forceSave: vi.fn(async () => {}),
|
||||||
|
batched: <T>(work: T) => work,
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
vi.mock("../../../utils/system-crypto.js", () => ({
|
||||||
|
SystemCrypto: {
|
||||||
|
getInstance: () => ({
|
||||||
|
getEncryptionKey: async () => Buffer.alloc(32, 7),
|
||||||
|
getJWTSecret: async () => "secret",
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
vi.mock("../../../utils/data-crypto.js", () => ({
|
||||||
|
DataCrypto: {
|
||||||
|
getUserDataKey: () => Buffer.alloc(32, 3),
|
||||||
|
validateUserAccess: () => Buffer.alloc(32, 3),
|
||||||
|
encryptRecord: (_table: string, record: unknown) => record,
|
||||||
|
decryptRecord: (_table: string, record: unknown) => record,
|
||||||
|
decryptRecords: (_table: string, records: unknown) => records,
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
vi.mock("../../../utils/auth-manager.js", () => ({
|
||||||
|
AuthManager: {
|
||||||
|
getInstance: () => ({
|
||||||
|
createAuthMiddleware:
|
||||||
|
() => (req: Request, _res: Response, next: NextFunction) => {
|
||||||
|
(req as Request & { userId: string }).userId = USER;
|
||||||
|
next();
|
||||||
|
},
|
||||||
|
createDataAccessMiddleware: () => pass,
|
||||||
|
createAdminMiddleware: () => pass,
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
vi.mock("../../../utils/permission-manager.js", () => ({
|
||||||
|
PermissionManager: {
|
||||||
|
getInstance: () => ({
|
||||||
|
requirePermission: () => pass,
|
||||||
|
canAccessHost: async () => ({ hasAccess: true, isOwner: true }),
|
||||||
|
hasPermission: async () => true,
|
||||||
|
isAdmin: async () => true,
|
||||||
|
getUserPermissions: async () => [],
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
vi.mock("../../../utils/audit-logger.js", () => ({
|
||||||
|
logAudit: async () => {},
|
||||||
|
getAuditUsername: async () => USER,
|
||||||
|
getRequestMeta: () => ({ ipAddress: "127.0.0.1", userAgent: "test" }),
|
||||||
|
}));
|
||||||
|
|
||||||
|
const { TestSqliteDatabase } = await import("../repositories/test-support.js");
|
||||||
|
const { hosts, sshCredentials } =
|
||||||
|
await import("../../../database/db/schema.js");
|
||||||
|
const { saveLevel } = await import("../../../hosts/defaults/service.js");
|
||||||
|
const { default: hostRouter } =
|
||||||
|
await import("../../../database/routes/host.js");
|
||||||
|
|
||||||
|
const app = express();
|
||||||
|
app.use(express.json());
|
||||||
|
app.use("/host", hostRouter);
|
||||||
|
|
||||||
|
let db: InstanceType<typeof TestSqliteDatabase>;
|
||||||
|
let ctx: DatabaseContext;
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
db = new TestSqliteDatabase("sqlite");
|
||||||
|
ctx = await db.connect();
|
||||||
|
state.ctx = ctx;
|
||||||
|
await ctx.drizzle.run(
|
||||||
|
sql`INSERT INTO users (id, username, password_hash, is_admin) VALUES (${USER}, ${USER}, '', 1)`,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(async () => {
|
||||||
|
await db.close();
|
||||||
|
});
|
||||||
|
|
||||||
|
async function addCredential(): Promise<number> {
|
||||||
|
const [row] = await ctx.drizzle
|
||||||
|
.insert(sshCredentials)
|
||||||
|
.values({
|
||||||
|
userId: USER,
|
||||||
|
name: "ops",
|
||||||
|
username: "root",
|
||||||
|
authType: "password",
|
||||||
|
password: "secret",
|
||||||
|
})
|
||||||
|
.returning({ id: sshCredentials.id });
|
||||||
|
return row.id;
|
||||||
|
}
|
||||||
|
|
||||||
|
async function storedAuthType(id: number) {
|
||||||
|
const [row] = await ctx.drizzle
|
||||||
|
.select({ authType: hosts.authType })
|
||||||
|
.from(hosts)
|
||||||
|
.where(eq(hosts.id, id));
|
||||||
|
return row?.authType;
|
||||||
|
}
|
||||||
|
|
||||||
|
const base = { ip: "10.0.0.5", port: 22, username: "root" };
|
||||||
|
|
||||||
|
describe("host create without authType", () => {
|
||||||
|
it("infers credential auth from a credentialId", async () => {
|
||||||
|
const credentialId = await addCredential();
|
||||||
|
const res = await request(app)
|
||||||
|
.post("/host/db/host")
|
||||||
|
.send({ ...base, credentialId });
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(await storedAuthType(res.body.id)).toBe("credential");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("infers password auth from a password", async () => {
|
||||||
|
const res = await request(app)
|
||||||
|
.post("/host/db/host")
|
||||||
|
.send({ ...base, password: "pw" });
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(await storedAuthType(res.body.id)).toBe("password");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects a body that says nothing about authentication", async () => {
|
||||||
|
const res = await request(app).post("/host/db/host").send(base);
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(res.body.error).toMatch(/authType is required/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("takes the auth type from the user's host defaults", async () => {
|
||||||
|
const credentialId = await addCredential();
|
||||||
|
await saveLevel(
|
||||||
|
{ level: "user", userId: USER },
|
||||||
|
{
|
||||||
|
set: new Map([["core.auth", { authType: "credential", credentialId }]]),
|
||||||
|
unset: new Set(),
|
||||||
|
errors: {},
|
||||||
|
},
|
||||||
|
USER,
|
||||||
|
);
|
||||||
|
const res = await request(app).post("/host/db/host").send(base);
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(await storedAuthType(res.body.id)).toBe("credential");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("host update authType", () => {
|
||||||
|
it("keeps the stored auth type when the update leaves it out", async () => {
|
||||||
|
const created = await request(app)
|
||||||
|
.post("/host/db/host")
|
||||||
|
.send({ ...base, authType: "password", password: "pw" });
|
||||||
|
const res = await request(app)
|
||||||
|
.put(`/host/db/host/${created.body.id}`)
|
||||||
|
.send({ ...base, name: "renamed" });
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(await storedAuthType(created.body.id)).toBe("password");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects an authType that is not a string", async () => {
|
||||||
|
const created = await request(app)
|
||||||
|
.post("/host/db/host")
|
||||||
|
.send({ ...base, authType: "password", password: "pw" });
|
||||||
|
const res = await request(app)
|
||||||
|
.put(`/host/db/host/${created.body.id}`)
|
||||||
|
.send({ ...base, authType: { bad: true } });
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -3,6 +3,7 @@ import { setHostProtocolSource } from "../../../hosts/protocol-auth/registry.js"
|
|||||||
import {
|
import {
|
||||||
applyHostKeyTypeUpdate,
|
applyHostKeyTypeUpdate,
|
||||||
containsOwnerPrivateAuthUpdate,
|
containsOwnerPrivateAuthUpdate,
|
||||||
|
impliedSshAuthType,
|
||||||
isNonEmptyString,
|
isNonEmptyString,
|
||||||
keepsStoredSshAuth,
|
keepsStoredSshAuth,
|
||||||
isOptionalBoolean,
|
isOptionalBoolean,
|
||||||
@@ -515,3 +516,16 @@ describe("keepsStoredSshAuth", () => {
|
|||||||
expect(keepsStoredSshAuth("none")).toBe(false);
|
expect(keepsStoredSshAuth("none")).toBe(false);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("impliedSshAuthType", () => {
|
||||||
|
it("reads the auth type from the secret a create body carries", () => {
|
||||||
|
expect(impliedSshAuthType({ credentialId: 2, password: "pw" })).toBe(
|
||||||
|
"credential",
|
||||||
|
);
|
||||||
|
expect(impliedSshAuthType({ key: "-----BEGIN", password: "pw" })).toBe(
|
||||||
|
"key",
|
||||||
|
);
|
||||||
|
expect(impliedSshAuthType({ password: "pw" })).toBe("password");
|
||||||
|
expect(impliedSshAuthType({})).toBeUndefined();
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in new issue
Block a user