mirror of
https://github.com/Termix-SSH/Termix.git
synced 2026-08-23 14:36:41 +00:00
fix: SSH-login alerts silently dropped (channel load + auth middleware ordering) (#1083)
* fix: load notification channels on mount in AlertsPanel Channels only loaded when the Channels tab was visited, so opening Edit Alert Rule before ever switching to that tab showed the channel picker as empty even when channels existed. (cherry picked from commit caed913ee91990a853f5a048849c67ed3f7c329e) * fix: register login-alert route before auth middleware Global JWT auth middleware ran before this internal service-to-service route, rejecting it with 401 before its own IP+token check ever ran — silently dropped every SSH-login alert. Also surface non-OK responses instead of swallowing them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test: add coverage for alert-notification fixes Channel-load-on-mount, login-alert non-OK handling, and a source-order guard for the route/auth-middleware regression. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * format AlertsPanel test with prettier --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: ZacharyZcR <zacharyzcr1984@gmail.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
ZacharyZcR
parent
ef958d8076
commit
43e972be84
@@ -842,7 +842,9 @@ app.use((_req, res, next) => {
|
||||
next();
|
||||
});
|
||||
|
||||
// Internal endpoint — only accepts calls from localhost.
|
||||
// Internal endpoint — only accepts calls from localhost. Registered before
|
||||
// the auth middleware since it's a service-to-service call authenticated by
|
||||
// IP + shared secret, not a user JWT.
|
||||
// Used by the main backend to notify the metrics service of SSH login events.
|
||||
app.post("/internal/login-alert", async (req, res) => {
|
||||
const remoteIp = req.socket.remoteAddress;
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import fs from "fs";
|
||||
import path from "path";
|
||||
|
||||
// Regression guard for: the /internal/login-alert route was registered
|
||||
// after the global JWT auth middleware, so every service-to-service login
|
||||
// alert got rejected with 401 before the route's own IP+token check ever
|
||||
// ran. Spinning up the full metrics-service Express app (DB, SSH clients,
|
||||
// polling managers, etc.) just to hit this one route is out of scope, so
|
||||
// this asserts the registration order directly against the source instead.
|
||||
describe("metrics service /internal/login-alert route order", () => {
|
||||
it("is registered before the global auth middleware", () => {
|
||||
const source = fs.readFileSync(
|
||||
path.resolve(__dirname, "../../../hosts/metrics/index.ts"),
|
||||
"utf8",
|
||||
);
|
||||
|
||||
const routeIndex = source.indexOf('app.post("/internal/login-alert"');
|
||||
const authMiddlewareIndex = source.indexOf(
|
||||
"app.use(authManager.createAuthMiddleware())",
|
||||
);
|
||||
|
||||
expect(routeIndex).toBeGreaterThan(-1);
|
||||
expect(authMiddlewareIndex).toBeGreaterThan(-1);
|
||||
expect(routeIndex).toBeLessThan(authMiddlewareIndex);
|
||||
});
|
||||
});
|
||||
@@ -45,4 +45,50 @@ describe("triggerLoginAlert", () => {
|
||||
|
||||
expect(warn).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("sends the internal auth token and login details the metrics service expects", async () => {
|
||||
vi.spyOn(SystemCrypto, "getInstance").mockReturnValue({
|
||||
getInternalAuthToken: vi.fn().mockResolvedValue("internal-token"),
|
||||
} as never);
|
||||
const fetchSpy = vi
|
||||
.spyOn(globalThis, "fetch")
|
||||
.mockResolvedValue(new Response('{"ok":true}', { status: 200 }));
|
||||
|
||||
await triggerLoginAlert(42, "user-1", "root", "10.0.0.5");
|
||||
|
||||
expect(fetchSpy).toHaveBeenCalledWith(
|
||||
"http://localhost:30005/internal/login-alert",
|
||||
expect.objectContaining({
|
||||
method: "POST",
|
||||
headers: expect.objectContaining({
|
||||
"x-internal-auth": "internal-token",
|
||||
}),
|
||||
body: JSON.stringify({
|
||||
hostId: 42,
|
||||
userId: "user-1",
|
||||
sshUser: "root",
|
||||
fromIp: "10.0.0.5",
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("logs a warning if the fetch itself throws, instead of propagating", async () => {
|
||||
vi.spyOn(SystemCrypto, "getInstance").mockReturnValue({
|
||||
getInternalAuthToken: vi.fn().mockResolvedValue("internal-token"),
|
||||
} as never);
|
||||
vi.spyOn(globalThis, "fetch").mockRejectedValue(
|
||||
new Error("connect ECONNREFUSED"),
|
||||
);
|
||||
const warn = vi.spyOn(sshLogger, "warn").mockImplementation(() => {});
|
||||
|
||||
await expect(
|
||||
triggerLoginAlert(1, "user-1", "root", "127.0.0.1"),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
"Failed to trigger login alert",
|
||||
expect.objectContaining({ hostId: 1 }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -96,7 +96,8 @@ export function AlertsPanel() {
|
||||
|
||||
useEffect(() => {
|
||||
loadFirings();
|
||||
}, [loadFirings]);
|
||||
loadChannels();
|
||||
}, [loadFirings, loadChannels]);
|
||||
|
||||
useEffect(() => {
|
||||
if (tab === "rules") loadRules();
|
||||
|
||||
@@ -0,0 +1,131 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import {
|
||||
render,
|
||||
screen,
|
||||
waitFor,
|
||||
cleanup,
|
||||
fireEvent,
|
||||
} from "@testing-library/react";
|
||||
import type { NotificationChannel } from "@/api/alerts-api";
|
||||
|
||||
const alertsApi = vi.hoisted(() => ({
|
||||
getAlertFirings: vi.fn(async () => []),
|
||||
acknowledgeAlertFiring: vi.fn(async () => {}),
|
||||
acknowledgeAllAlertFirings: vi.fn(async () => {}),
|
||||
getAlertRules: vi.fn(async () => []),
|
||||
getNotificationChannels: vi.fn(async () => [] as NotificationChannel[]),
|
||||
deleteAlertRule: vi.fn(async () => {}),
|
||||
deleteNotificationChannel: vi.fn(async () => {}),
|
||||
testNotificationChannel: vi.fn(async () => {}),
|
||||
}));
|
||||
|
||||
vi.mock("@/api/alerts-api", () => alertsApi);
|
||||
|
||||
vi.mock("react-i18next", () => ({
|
||||
useTranslation: () => ({
|
||||
t: (_key: string, fallback?: string) => fallback ?? _key,
|
||||
}),
|
||||
}));
|
||||
|
||||
// A real (non-null) fake so tests can see what channels actually reach the
|
||||
// rule dialog, instead of only asserting the API call count.
|
||||
vi.mock("../../sidebar/AlertRuleDialog", () => ({
|
||||
AlertRuleDialog: ({
|
||||
open,
|
||||
channels,
|
||||
}: {
|
||||
open: boolean;
|
||||
channels: NotificationChannel[];
|
||||
}) =>
|
||||
open ? (
|
||||
<div data-testid="rule-dialog-channels">
|
||||
{channels.length === 0
|
||||
? "no channels"
|
||||
: channels.map((c) => c.name).join(",")}
|
||||
</div>
|
||||
) : null,
|
||||
}));
|
||||
vi.mock("../../sidebar/NotificationChannelDialog", () => ({
|
||||
NotificationChannelDialog: () => null,
|
||||
}));
|
||||
|
||||
import { AlertsPanel } from "../../sidebar/AlertsPanel";
|
||||
|
||||
function channel(
|
||||
overrides: Partial<NotificationChannel> = {},
|
||||
): NotificationChannel {
|
||||
return {
|
||||
id: 1,
|
||||
userId: "user-1",
|
||||
name: "ntfy",
|
||||
type: "ntfy",
|
||||
config: "{}",
|
||||
enabled: true,
|
||||
createdAt: new Date().toISOString(),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
alertsApi.getAlertFirings.mockReset().mockResolvedValue([]);
|
||||
alertsApi.getAlertRules.mockReset().mockResolvedValue([]);
|
||||
alertsApi.getNotificationChannels.mockReset().mockResolvedValue([]);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
});
|
||||
|
||||
describe("AlertsPanel - notification channels", () => {
|
||||
it("loads notification channels on mount, before the Channels tab is ever opened", async () => {
|
||||
alertsApi.getNotificationChannels.mockResolvedValue([channel()]);
|
||||
|
||||
render(<AlertsPanel />);
|
||||
|
||||
// Regression: channels used to only load once the user switched to the
|
||||
// "channels" tab, so editing a rule before ever visiting that tab showed
|
||||
// an empty channel picker even though channels existed.
|
||||
await waitFor(() => {
|
||||
expect(alertsApi.getNotificationChannels).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
it("populates the rule dialog's channel picker without ever visiting the Channels tab", async () => {
|
||||
alertsApi.getNotificationChannels.mockResolvedValue([
|
||||
channel({ id: 1, name: "ntfy-home" }),
|
||||
channel({ id: 2, name: "webhook-slack" }),
|
||||
]);
|
||||
|
||||
render(<AlertsPanel />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(alertsApi.getNotificationChannels).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// Go straight to Rules and open the "new rule" dialog, skipping Channels
|
||||
// entirely, this is exactly the sequence the original bug broke.
|
||||
fireEvent.click(screen.getByText("Rules"));
|
||||
fireEvent.click(screen.getByText("Add"));
|
||||
|
||||
expect(screen.getByTestId("rule-dialog-channels").textContent).toBe(
|
||||
"ntfy-home,webhook-slack",
|
||||
);
|
||||
});
|
||||
|
||||
it("does not refetch channels again when switching to the Rules tab", async () => {
|
||||
render(<AlertsPanel />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(alertsApi.getNotificationChannels).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// Switching tabs only reloads rules here, not channels, since channels
|
||||
// were already fetched on mount by the fix under test.
|
||||
fireEvent.click(screen.getByText("Rules"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(alertsApi.getAlertRules).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
expect(alertsApi.getNotificationChannels).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user