fix: return login errors to form instead of throwing (#474)

Throwing data() in React Router v7 actions goes to ErrorBoundary instead
of returning to the component. Changed validation errors to return
directly so the login form can display them properly.

app/routes/auth/login/action.ts

@@ -1,4 +1,4 @@
-import { data, redirect } from "react-router";
+import { redirect } from "react-router";
 
 import { isDataWithApiError } from "~/server/headscale/api/error-client";
 import log from "~/utils/log";
@@ -15,7 +15,10 @@ export async function loginAction({ request, context }: Route.LoaderArgs) {
       "auth",
       "If this is unexpected, ensure your reverse proxy (if applicable) is configured correctly",
     );
-    throw data("Missing `api_key`", { status: 400 });
+    return {
+      success: false,
+      message: "Missing API key. Please enter your API key.",
+    };
   }
 
   if (apiKey.length === 0) {
@@ -24,13 +27,15 @@ export async function loginAction({ request, context }: Route.LoaderArgs) {
       "auth",
       "If this is unexpected, ensure your reverse proxy (if applicable) is configured correctly",
     );
-    throw data("Received an empty `api_key`", { status: 400 });
+    return {
+      success: false,
+      message: "API key cannot be empty. Please enter a valid API key.",
+    };
   }
 
   const api = context.hsApi.getRuntimeClient(apiKey);
   try {
     const apiKeys = await api.getApiKeys();
-    console.log(apiKeys);
 
     // We don't need to check for 0 API keys because this request cannot
     // be authenticated correctly without an API key
@@ -47,7 +52,10 @@ export async function loginAction({ request, context }: Route.LoaderArgs) {
 
     if (lookup.expiration === null || lookup.expiration === undefined) {
       log.error("auth", "Got an API key without an expiration");
-      throw data("API key is malformed", { status: 500 });
+      return {
+        success: false,
+        message: "API key is malformed (missing expiration). Please generate a new API key.",
+      };
     }
 
     const expiry = new Date(lookup.expiration);

tests/unit/auth/login-action.test.ts

@@ -0,0 +1,192 @@
+import { describe, expect, test, vi } from "vitest";
+
+// Helper to create a mock FormData with optional api_key
+function mockFormData(apiKey?: string): FormData {
+  const formData = new FormData();
+  if (apiKey !== undefined) {
+    formData.set("api_key", apiKey);
+  }
+  return formData;
+}
+
+// Helper to create mock request
+function mockRequest(formData: FormData): Request {
+  return {
+    formData: () => Promise.resolve(formData),
+  } as unknown as Request;
+}
+
+// Types for test clarity
+interface LoginResult {
+  success: boolean;
+  message: string;
+}
+
+interface MockApiKey {
+  prefix: string;
+  expiration: string | null;
+}
+
+// Mock the log module to avoid console spam during tests
+vi.mock("~/utils/log", () => ({
+  default: {
+    warn: vi.fn(),
+    error: vi.fn(),
+    debug: vi.fn(),
+  },
+}));
+
+describe("Login action validation", () => {
+  test("returns error when api_key field is missing", async () => {
+    const { loginAction } = await import("~/routes/auth/login/action");
+    const formData = mockFormData(); // no api_key
+    const request = mockRequest(formData);
+
+    const mockContext = {
+      hsApi: { getRuntimeClient: vi.fn() },
+      sessions: { createSession: vi.fn() },
+    };
+
+    const result = (await loginAction({
+      request,
+      context: mockContext,
+      params: {},
+    } as any)) as LoginResult;
+
+    expect(result.success).toBe(false);
+    expect(result.message).toContain("Missing");
+  });
+
+  test("returns error when api_key is empty string", async () => {
+    const { loginAction } = await import("~/routes/auth/login/action");
+    const formData = mockFormData("");
+    const request = mockRequest(formData);
+
+    const mockContext = {
+      hsApi: { getRuntimeClient: vi.fn() },
+      sessions: { createSession: vi.fn() },
+    };
+
+    const result = (await loginAction({
+      request,
+      context: mockContext,
+      params: {},
+    } as any)) as LoginResult;
+
+    expect(result.success).toBe(false);
+    expect(result.message).toContain("empty");
+  });
+
+  test("returns error when api key not found in database", async () => {
+    const { loginAction } = await import("~/routes/auth/login/action");
+    const formData = mockFormData("some-invalid-key-12345");
+    const request = mockRequest(formData);
+
+    const mockGetApiKeys = vi
+      .fn()
+      .mockResolvedValue([{ prefix: "other-prefix", expiration: "2030-01-01T00:00:00Z" }]);
+
+    const mockContext = {
+      hsApi: {
+        getRuntimeClient: () => ({ getApiKeys: mockGetApiKeys }),
+      },
+      sessions: { createSession: vi.fn() },
+    };
+
+    const result = (await loginAction({
+      request,
+      context: mockContext,
+      params: {},
+    } as any)) as LoginResult;
+
+    expect(result.success).toBe(false);
+    expect(result.message).toContain("not found");
+  });
+
+  test("returns error when api key has expired", async () => {
+    const { loginAction } = await import("~/routes/auth/login/action");
+    const expiredKey = "expired-key-prefix.secret";
+    const formData = mockFormData(expiredKey);
+    const request = mockRequest(formData);
+
+    const mockGetApiKeys = vi
+      .fn()
+      .mockResolvedValue([{ prefix: "expired-key-prefix", expiration: "2020-01-01T00:00:00Z" }]);
+
+    const mockContext = {
+      hsApi: {
+        getRuntimeClient: () => ({ getApiKeys: mockGetApiKeys }),
+      },
+      sessions: { createSession: vi.fn() },
+    };
+
+    const result = (await loginAction({
+      request,
+      context: mockContext,
+      params: {},
+    } as any)) as LoginResult;
+
+    expect(result.success).toBe(false);
+    expect(result.message).toContain("expired");
+  });
+
+  test("returns error when api key has no expiration field", async () => {
+    const { loginAction } = await import("~/routes/auth/login/action");
+    const keyWithoutExpiry = "malformed-key.secret";
+    const formData = mockFormData(keyWithoutExpiry);
+    const request = mockRequest(formData);
+
+    const mockGetApiKeys = vi
+      .fn()
+      .mockResolvedValue([{ prefix: "malformed-key", expiration: null } as MockApiKey]);
+
+    const mockContext = {
+      hsApi: {
+        getRuntimeClient: () => ({ getApiKeys: mockGetApiKeys }),
+      },
+      sessions: { createSession: vi.fn() },
+    };
+
+    const result = (await loginAction({
+      request,
+      context: mockContext,
+      params: {},
+    } as any)) as LoginResult;
+
+    expect(result.success).toBe(false);
+    expect(result.message).toContain("malformed");
+  });
+
+  test("handles asterisks in api key prefix from headscale 0.28+", async () => {
+    const { loginAction } = await import("~/routes/auth/login/action");
+    const apiKey = "my-key-prefix.the-secret-part";
+    const formData = mockFormData(apiKey);
+    const request = mockRequest(formData);
+
+    const futureDate = new Date();
+    futureDate.setFullYear(futureDate.getFullYear() + 1);
+
+    const mockGetApiKeys = vi
+      .fn()
+      .mockResolvedValue([{ prefix: "my-***-prefix", expiration: futureDate.toISOString() }]);
+
+    const mockCreateSession = vi.fn().mockResolvedValue("session-cookie");
+
+    const mockContext = {
+      hsApi: {
+        getRuntimeClient: () => ({ getApiKeys: mockGetApiKeys }),
+      },
+      sessions: { createSession: mockCreateSession },
+    };
+
+    const result = await loginAction({
+      request,
+      context: mockContext,
+      params: {},
+    } as any);
+
+    // Should match despite asterisks in stored prefix
+    // Result will be a redirect (Response) on success, not our error object
+    expect(result).toBeDefined();
+  });
+});