fix(security): write Traefik basic-auth middleware after DB commit on update

apps/dokploy/__test__/services/security-update.test.ts

@@ -0,0 +1,202 @@
+import { db } from "@dokploy/server/db";
+import { updateSecurityById } from "@dokploy/server/services/security";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const OLD_ROW = {
+	securityId: "sec-1",
+	username: "alice",
+	password: "oldpass",
+	applicationId: "app-1",
+	createdAt: "2024-01-01",
+};
+
+const NEW_ROW = {
+	securityId: "sec-1",
+	username: "bob",
+	password: "newpass",
+	applicationId: "app-1",
+	createdAt: "2024-01-01",
+};
+
+const APP = {
+	applicationId: "app-1",
+	appName: "myapp",
+	serverId: null,
+};
+
+const mocks = vi.hoisted(() => {
+	const returningMock = vi.fn();
+	const chain = {
+		set: vi.fn(),
+		where: vi.fn(),
+		returning: returningMock,
+	};
+	chain.set.mockReturnValue(chain);
+	chain.where.mockReturnValue(chain);
+	const tx = {
+		update: vi.fn(),
+	};
+	tx.update.mockReturnValue(chain);
+	return {
+		returningMock,
+		chain,
+		tx,
+		transaction: vi.fn(),
+		findFirst: vi.fn(),
+		findApplicationById: vi.fn(),
+		createSecurityMiddleware: vi.fn(),
+		removeSecurityMiddleware: vi.fn(),
+		replaceSecurityMiddlewareUser: vi.fn(),
+	};
+});
+
+vi.mock("@dokploy/server/db", () => ({
+	db: {
+		transaction: mocks.transaction,
+		query: {
+			security: {
+				findFirst: mocks.findFirst,
+			},
+		},
+	},
+}));
+
+vi.mock("@dokploy/server/services/application", () => ({
+	findApplicationById: mocks.findApplicationById,
+}));
+
+vi.mock("@dokploy/server/utils/traefik/security", () => ({
+	createSecurityMiddleware: mocks.createSecurityMiddleware,
+	removeSecurityMiddleware: mocks.removeSecurityMiddleware,
+	replaceSecurityMiddlewareUser: mocks.replaceSecurityMiddlewareUser,
+}));
+
+describe("updateSecurityById", () => {
+	beforeEach(() => {
+		mocks.findFirst.mockReset();
+		mocks.findApplicationById.mockReset();
+		mocks.returningMock.mockReset();
+		mocks.replaceSecurityMiddlewareUser.mockReset();
+		mocks.removeSecurityMiddleware.mockReset();
+		mocks.createSecurityMiddleware.mockReset();
+		mocks.transaction.mockReset();
+		mocks.tx.update.mockReset();
+		mocks.chain.set.mockReset();
+		mocks.chain.where.mockReset();
+
+		mocks.chain.set.mockReturnValue(mocks.chain);
+		mocks.chain.where.mockReturnValue(mocks.chain);
+		mocks.tx.update.mockReturnValue(mocks.chain);
+		mocks.transaction.mockImplementation(
+			async (cb: (tx: typeof mocks.tx) => unknown) => cb(mocks.tx),
+		);
+
+		mocks.findFirst.mockResolvedValue(OLD_ROW);
+		mocks.findApplicationById.mockResolvedValue(APP);
+		mocks.returningMock.mockResolvedValue([NEW_ROW]);
+		mocks.replaceSecurityMiddlewareUser.mockResolvedValue(undefined);
+		mocks.removeSecurityMiddleware.mockResolvedValue(undefined);
+		mocks.createSecurityMiddleware.mockResolvedValue(undefined);
+	});
+
+	it("performs a single middleware write after the DB transaction commits with old + new security", async () => {
+		await updateSecurityById("sec-1", { username: "bob", password: "newpass" });
+
+		expect(mocks.transaction).toHaveBeenCalledTimes(1);
+		expect(mocks.findFirst).toHaveBeenCalledTimes(1);
+		expect(mocks.findApplicationById).toHaveBeenCalledWith("app-1");
+		expect(mocks.tx.update).toHaveBeenCalledTimes(1);
+
+		expect(mocks.replaceSecurityMiddlewareUser).toHaveBeenCalledTimes(1);
+		expect(mocks.replaceSecurityMiddlewareUser).toHaveBeenCalledWith(
+			APP,
+			OLD_ROW,
+			NEW_ROW,
+		);
+		expect(mocks.removeSecurityMiddleware).not.toHaveBeenCalled();
+		expect(mocks.createSecurityMiddleware).not.toHaveBeenCalled();
+	});
+
+	it("does NOT touch the middleware file when tx.update throws (unique violation rollback)", async () => {
+		mocks.returningMock.mockRejectedValue(
+			new Error(
+				'duplicate key value violates unique constraint "security_username_applicationId_unique"',
+			),
+		);
+
+		await expect(
+			updateSecurityById("sec-1", { username: "bob" }),
+		).rejects.toThrow();
+
+		expect(mocks.replaceSecurityMiddlewareUser).not.toHaveBeenCalled();
+		expect(mocks.removeSecurityMiddleware).not.toHaveBeenCalled();
+		expect(mocks.createSecurityMiddleware).not.toHaveBeenCalled();
+		expect(mocks.tx.update).toHaveBeenCalledTimes(1);
+	});
+
+	it("rethrows a TRPCError BAD_REQUEST wrapping the underlying error on tx.update failure", async () => {
+		mocks.returningMock.mockRejectedValue(
+			new Error(
+				'duplicate key value violates unique constraint "security_username_applicationId_unique"',
+			),
+		);
+
+		await expect(
+			updateSecurityById("sec-1", { username: "bob" }),
+		).rejects.toMatchObject({
+			code: "BAD_REQUEST",
+		});
+	});
+
+	it("does NOT touch the middleware file when the security row is missing", async () => {
+		mocks.findFirst.mockResolvedValue(undefined);
+
+		await expect(updateSecurityById("missing", {})).rejects.toThrow();
+		expect(mocks.replaceSecurityMiddlewareUser).not.toHaveBeenCalled();
+		expect(mocks.removeSecurityMiddleware).not.toHaveBeenCalled();
+		expect(mocks.createSecurityMiddleware).not.toHaveBeenCalled();
+		expect(mocks.tx.update).not.toHaveBeenCalled();
+	});
+
+	it("commits the DB row even if the post-commit middleware write fails (stale-credential, not lockout)", async () => {
+		mocks.returningMock.mockResolvedValue([NEW_ROW]);
+		mocks.replaceSecurityMiddlewareUser.mockRejectedValue(
+			new Error("ssh failure"),
+		);
+
+		await expect(
+			updateSecurityById("sec-1", { username: "bob", password: "newpass" }),
+		).rejects.toThrow();
+
+		expect(mocks.tx.update).toHaveBeenCalledTimes(1);
+		expect(mocks.replaceSecurityMiddlewareUser).toHaveBeenCalledTimes(1);
+		expect(mocks.removeSecurityMiddleware).not.toHaveBeenCalled();
+		expect(mocks.createSecurityMiddleware).not.toHaveBeenCalled();
+	});
+
+	it("runs the middleware write only after the transaction callback returns (post-commit)", async () => {
+		let txReturned = false;
+		mocks.transaction.mockImplementation(
+			async (cb: (tx: typeof mocks.tx) => unknown) => {
+				await cb(mocks.tx);
+				txReturned = true;
+				return { oldSecurity: OLD_ROW, newSecurity: NEW_ROW, application: APP };
+			},
+		);
+		mocks.replaceSecurityMiddlewareUser.mockImplementation(async () => {
+			expect(txReturned).toBe(true);
+		});
+
+		await updateSecurityById("sec-1", { username: "bob" });
+
+		expect(mocks.replaceSecurityMiddlewareUser).toHaveBeenCalledTimes(1);
+	});
+});
+
+describe("updateSecurityById db mock sanity", () => {
+	it("exposes the mocked db used by the service under test", () => {
+		expect((db as unknown as { transaction: unknown }).transaction).toBe(
+			mocks.transaction,
+		);
+	});
+});

apps/dokploy/__test__/traefik/security-middleware.test.ts

@@ -0,0 +1,306 @@
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import type { ApplicationNested } from "@dokploy/server";
+import type { Security } from "@dokploy/server/services/security";
+import type { FileConfig } from "@dokploy/server/utils/traefik/file-types";
+import { replaceSecurityMiddlewareUser } from "@dokploy/server/utils/traefik/security";
+import * as bcrypt from "bcrypt";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { parse, stringify } from "yaml";
+
+const remoteMocks = vi.hoisted(() => ({
+	execAsyncRemote: vi.fn(),
+	writeFileRemote: vi.fn(),
+}));
+
+vi.mock("@dokploy/server/utils/process/execAsync", async (importOriginal) => {
+	const actual =
+		await importOriginal<
+			typeof import("@dokploy/server/utils/process/execAsync")
+		>();
+	return {
+		...actual,
+		execAsyncRemote: remoteMocks.execAsyncRemote,
+		writeFileRemote: remoteMocks.writeFileRemote,
+	};
+});
+
+const OLD: Security = {
+	securityId: "sec-1",
+	username: "alice",
+	password: "oldpass",
+	applicationId: "app-1",
+	createdAt: "2024-01-01",
+};
+
+const initialMiddlewares = (users: string[]): FileConfig => ({
+	http: {
+		middlewares: {
+			"auth-myapp": {
+				basicAuth: { removeHeader: true, users },
+			},
+		},
+	},
+});
+
+const initialAppConfig = (): FileConfig => ({
+	http: {
+		routers: {
+			"myapp-router-1": {
+				rule: "Host(`x`)",
+				service: "myapp-service",
+				middlewares: ["auth-myapp"],
+			},
+		},
+		services: {
+			"myapp-service": {
+				loadBalancer: {
+					servers: [{ url: "http://myapp:3000" }],
+					passHostHeader: true,
+				},
+			},
+		},
+	},
+});
+
+type BasicAuthMW = {
+	basicAuth?: { removeHeader?: boolean; users?: string[] };
+};
+
+const hashOf = (line: string): string => line.split(":")[1] ?? "";
+
+const readUsers = (file: string): string[] => {
+	const parsed = parse(fs.readFileSync(file, "utf8")) as FileConfig;
+	const mw = parsed.http?.middlewares?.["auth-myapp"] as
+		| BasicAuthMW
+		| undefined;
+	return mw?.basicAuth?.users ?? [];
+};
+
+describe("replaceSecurityMiddlewareUser (local filesystem)", () => {
+	let cwd: string;
+	let dynamicPath: string;
+	let middlewaresPath: string;
+	let appConfigPath: string;
+
+	const APP_LOCAL = {
+		applicationId: "app-1",
+		appName: "myapp",
+		serverId: null,
+	} as unknown as ApplicationNested;
+
+	beforeEach(() => {
+		cwd = fs.mkdtempSync(path.join(os.tmpdir(), "dokploy-security-"));
+		dynamicPath = path.join(cwd, ".docker", "traefik", "dynamic");
+		fs.mkdirSync(dynamicPath, { recursive: true });
+		middlewaresPath = path.join(dynamicPath, "middlewares.yml");
+		appConfigPath = path.join(dynamicPath, "myapp.yml");
+		vi.spyOn(process, "cwd").mockReturnValue(cwd);
+		vi.clearAllMocks();
+	});
+
+	afterEach(() => {
+		vi.restoreAllMocks();
+		fs.rmSync(cwd, { recursive: true, force: true });
+	});
+
+	it("replaces the old username's hash with the new one in a single write (rename)", async () => {
+		fs.writeFileSync(
+			middlewaresPath,
+			stringify(initialMiddlewares(["alice:oldhash", "carol:carolhash"])),
+			"utf8",
+		);
+		fs.writeFileSync(appConfigPath, stringify(initialAppConfig()), "utf8");
+
+		const newSecurity: Security = {
+			...OLD,
+			username: "bob",
+			password: "newpass",
+		};
+
+		await replaceSecurityMiddlewareUser(APP_LOCAL, OLD, newSecurity);
+
+		const users = readUsers(middlewaresPath);
+		expect(users).not.toContain("alice:oldhash");
+		expect(users).toContain("carol:carolhash");
+		const bobLine = users.find((u) => u.startsWith("bob:"));
+		expect(bobLine).toBeDefined();
+		const [name, hash = ""] = bobLine!.split(":");
+		expect(name).toBe("bob");
+		expect(await bcrypt.compare("newpass", hash)).toBe(true);
+		expect(await bcrypt.compare("oldpass", hash)).toBe(false);
+
+		const appCfg = parse(fs.readFileSync(appConfigPath, "utf8")) as FileConfig;
+		expect(appCfg.http?.routers?.["myapp-router-1"]?.middlewares).toContain(
+			"auth-myapp",
+		);
+	});
+
+	it("replaces only the hash when the username is unchanged (password rotation)", async () => {
+		fs.writeFileSync(
+			middlewaresPath,
+			stringify(initialMiddlewares(["alice:oldhash", "carol:carolhash"])),
+			"utf8",
+		);
+		fs.writeFileSync(appConfigPath, stringify(initialAppConfig()), "utf8");
+
+		const newSecurity: Security = {
+			...OLD,
+			username: "alice",
+			password: "newpass",
+		};
+
+		await replaceSecurityMiddlewareUser(APP_LOCAL, OLD, newSecurity);
+
+		const users = readUsers(middlewaresPath);
+		expect(users).not.toContain("alice:oldhash");
+		expect(users).toContain("carol:carolhash");
+		const aliceLine = users.find((u) => u.startsWith("alice:"));
+		expect(aliceLine).toBeDefined();
+		expect(await bcrypt.compare("newpass", hashOf(aliceLine!))).toBe(true);
+		expect(await bcrypt.compare("oldpass", hashOf(aliceLine!))).toBe(false);
+		expect(users).toHaveLength(2);
+	});
+
+	it("creates the basicAuth middleware entry if it does not yet exist", async () => {
+		const otherMiddlewares: FileConfig = {
+			http: {
+				middlewares: {
+					"redirect-other": { redirectScheme: { scheme: "https" } },
+				},
+			},
+		};
+		fs.writeFileSync(middlewaresPath, stringify(otherMiddlewares), "utf8");
+		fs.writeFileSync(appConfigPath, stringify(initialAppConfig()), "utf8");
+
+		const newSecurity: Security = {
+			...OLD,
+			username: "bob",
+			password: "newpass",
+		};
+
+		await replaceSecurityMiddlewareUser(APP_LOCAL, OLD, newSecurity);
+
+		const parsed = parse(
+			fs.readFileSync(middlewaresPath, "utf8"),
+		) as FileConfig;
+		expect(parsed.http?.middlewares?.["redirect-other"]).toBeDefined();
+		const users = readUsers(middlewaresPath);
+		expect(users).toHaveLength(1);
+		const bobLine = users.find((u) => u.startsWith("bob:"));
+		expect(bobLine).toBeDefined();
+		expect(await bcrypt.compare("newpass", hashOf(bobLine!))).toBe(true);
+	});
+
+	it("attaches the middleware to routers that did not reference it yet", async () => {
+		fs.writeFileSync(
+			middlewaresPath,
+			stringify(initialMiddlewares(["alice:oldhash"])),
+			"utf8",
+		);
+		const appConfigNoMiddleware: FileConfig = {
+			http: {
+				routers: {
+					"myapp-router-1": {
+						rule: "Host(`x`)",
+						service: "myapp-service",
+					},
+				},
+				services: {
+					"myapp-service": {
+						loadBalancer: {
+							servers: [{ url: "http://myapp:3000" }],
+							passHostHeader: true,
+						},
+					},
+				},
+			},
+		};
+		fs.writeFileSync(appConfigPath, stringify(appConfigNoMiddleware), "utf8");
+
+		const newSecurity: Security = {
+			...OLD,
+			username: "alice",
+			password: "newpass",
+		};
+
+		await replaceSecurityMiddlewareUser(APP_LOCAL, OLD, newSecurity);
+
+		const appCfg = parse(fs.readFileSync(appConfigPath, "utf8")) as FileConfig;
+		expect(appCfg.http?.routers?.["myapp-router-1"]?.middlewares).toContain(
+			"auth-myapp",
+		);
+		const users = readUsers(middlewaresPath);
+		expect(users).not.toContain("alice:oldhash");
+		expect(await bcrypt.compare("newpass", hashOf(users[0]!))).toBe(true);
+	});
+});
+
+describe("replaceSecurityMiddlewareUser (remote serverId)", () => {
+	const APP_REMOTE = {
+		applicationId: "app-1",
+		appName: "myapp",
+		serverId: "server-1",
+	} as unknown as ApplicationNested;
+
+	beforeEach(() => {
+		vi.clearAllMocks();
+	});
+
+	afterEach(() => {
+		vi.restoreAllMocks();
+	});
+
+	it("writes the remote middlewares.yml exactly once with the old user replaced", async () => {
+		remoteMocks.execAsyncRemote.mockImplementation(
+			async (_serverId, command) => {
+				if (command.includes("middlewares.yml")) {
+					return {
+						stdout: stringify(
+							initialMiddlewares(["alice:oldhash", "carol:carolhash"]),
+						),
+						stderr: "",
+					};
+				}
+				if (command.includes("myapp.yml")) {
+					return { stdout: stringify(initialAppConfig()), stderr: "" };
+				}
+				return { stdout: "", stderr: "" };
+			},
+		);
+		remoteMocks.writeFileRemote.mockResolvedValue(undefined);
+
+		const newSecurity: Security = {
+			...OLD,
+			username: "bob",
+			password: "newpass",
+		};
+
+		await replaceSecurityMiddlewareUser(APP_REMOTE, OLD, newSecurity);
+
+		const mwWrites = remoteMocks.writeFileRemote.mock.calls.filter(([, p]) =>
+			(p as string).includes("middlewares.yml"),
+		);
+		expect(mwWrites).toHaveLength(1);
+
+		const [, , content] = mwWrites[0]!;
+		const written = parse(content as string) as FileConfig;
+		const mw = written.http?.middlewares?.["auth-myapp"] as
+			| BasicAuthMW
+			| undefined;
+		const users = mw?.basicAuth?.users ?? [];
+		expect(users).not.toContain("alice:oldhash");
+		expect(users).toContain("carol:carolhash");
+		const bobLine = users.find((u) => u.startsWith("bob:"));
+		expect(bobLine).toBeDefined();
+		expect(await bcrypt.compare("newpass", hashOf(bobLine!))).toBe(true);
+
+		const appWrites = remoteMocks.writeFileRemote.mock.calls.filter(([, p]) =>
+			(p as string).includes("myapp.yml"),
+		);
+		expect(appWrites).toHaveLength(1);
+
+		expect(remoteMocks.execAsyncRemote).toHaveBeenCalledTimes(2);
+	});
+});

packages/server/src/services/security.ts

@@ -3,6 +3,7 @@ import { type apiCreateSecurity, security } from "@dokploy/server/db/schema";
 import {
 	createSecurityMiddleware,
 	removeSecurityMiddleware,
+	replaceSecurityMiddlewareUser,
 } from "@dokploy/server/utils/traefik/security";
 import { TRPCError } from "@trpc/server";
 import { eq } from "drizzle-orm";
@@ -91,41 +92,38 @@ export const updateSecurityById = async (
 	data: Partial<Security>,
 ) => {
 	try {
-		await db.transaction(async (tx) => {
-			const securityResponse = await findSecurityById(securityId);
+		const { oldSecurity, newSecurity, application } = await db.transaction(
+			async (tx) => {
+				const oldSecurity = await findSecurityById(securityId);
 
-			const application = await findApplicationById(
-				securityResponse.applicationId,
-			);
+				const application = await findApplicationById(
+					oldSecurity.applicationId,
+				);
 
-			await removeSecurityMiddleware(application, securityResponse);
+				const response = await tx
+					.update(security)
+					.set({
+						...data,
+					})
+					.where(eq(security.securityId, securityId))
+					.returning()
+					.then((res) => res[0]);
 
-			const response = await tx
-				.update(security)
-				.set({
-					...data,
-				})
-				.where(eq(security.securityId, securityId))
-				.returning()
-				.then((res) => res[0]);
+				if (!response) {
+					throw new TRPCError({
+						code: "NOT_FOUND",
+						message: "Security not found",
+					});
+				}
 
-			if (!response) {
-				throw new TRPCError({
-					code: "NOT_FOUND",
-					message: "Security not found",
-				});
-			}
+				return { oldSecurity, newSecurity: response, application };
+			},
+		);
 
-			await createSecurityMiddleware(application, response);
-
-			return response;
-		});
+		await replaceSecurityMiddlewareUser(application, oldSecurity, newSecurity);
 	} catch (error) {
 		const message =
 			error instanceof Error ? error.message : "Error updating this security";
-		throw new TRPCError({
-			code: "BAD_REQUEST",
-			message,
-		});
+		throw new TRPCError({ code: "BAD_REQUEST", message });
 	}
 };

packages/server/src/utils/traefik/security.ts

@@ -121,6 +121,62 @@ export const removeSecurityMiddleware = async (
 	}
 };
 
+export const replaceSecurityMiddlewareUser = async (
+	application: ApplicationNested,
+	oldSecurity: Security,
+	newSecurity: Security,
+) => {
+	const { appName, serverId } = application;
+	let config: FileConfig;
+
+	if (serverId) {
+		config = await loadRemoteMiddlewares(serverId);
+	} else {
+		config = loadMiddlewares<FileConfig>();
+	}
+	const middlewareName = `auth-${appName}`;
+
+	const newUser = `${newSecurity.username}:${await bcrypt.hash(
+		newSecurity.password,
+		10,
+	)}`;
+
+	if (config.http?.middlewares) {
+		const currentMiddleware = config.http.middlewares[middlewareName];
+		if (isBasicAuthMiddleware(currentMiddleware)) {
+			const users =
+				currentMiddleware.basicAuth.users?.filter((user) => {
+					const [username] = user.split(":");
+					return username !== oldSecurity.username;
+				}) || [];
+			currentMiddleware.basicAuth.users = [...users, newUser];
+		} else {
+			config.http.middlewares[middlewareName] = {
+				basicAuth: {
+					removeHeader: true,
+					users: [newUser],
+				},
+			};
+		}
+	}
+
+	let appConfig: FileConfig;
+
+	if (serverId) {
+		appConfig = await loadOrCreateConfigRemote(serverId, appName);
+	} else {
+		appConfig = loadOrCreateConfig(appName);
+	}
+	addMiddleware(appConfig, middlewareName);
+
+	if (serverId) {
+		await writeTraefikConfigRemote(config, "middlewares", serverId);
+	} else {
+		writeMiddleware(config);
+	}
+	await writeAppTraefikConfig(appConfig, appName, serverId);
+};
+
 const isBasicAuthMiddleware = (
 	middleware: HttpMiddleware | undefined,
 ): middleware is { basicAuth: BasicAuthMiddleware } => {