Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 27 additions & 10 deletions app/src/app/(auth)/login/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,27 @@ function LoginForm() {
setError("");

const formData = new FormData(e.currentTarget);
const result = await signIn("credentials", {
email: formData.get("email"),
password: formData.get("password"),
redirect: false,
});
try {
const result = await signIn("credentials", {
email: formData.get("email"),
password: formData.get("password"),
redirect: false,
});

if (result?.error) {
setError("Invalid email or password");
if (result?.error) {
setError("Invalid email or password");
setLoading(false);
} else if (result) {
router.push(callbackUrl);
} else {
// Nothing returned → server unreachable
setError("Unable to sign in. Please try again.");
setLoading(false);
}
Comment on lines +46 to +50

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

NextAuth v5 signIn function return value with redirect false

💡 Result:

In NextAuth.js (Auth.js) v5, when calling the client-side signIn function with { redirect: false }, it returns a Promise object containing { ok: boolean, error?: string, code?: string }. This allows checking if sign-in succeeded without automatic redirection: if (response.ok) { // success, manually redirect or update UI } else { // handle error: response.error }. The exact shape differs slightly from v4 (which had { ok, error, status, url }), and v5 beta versions have reported inconsistencies where it sometimes returns a URL string instead of the object, or ok: true despite errors (ongoing issues #9465, #6040). Official docs confirm SignInResponse has ok, error, code for client use with redirect: false. Use .then or await to access it, as one example showed: await signIn(...).then(({ ok, error }) => ...). For server actions, signIn throws AuthError on failure instead of returning response.

Citations:


🏁 Script executed:

# Find and check the login page file
find . -name "page.tsx" | grep -i login | head -5

Repository: alfredo1996/neoboard

Length of output: 99


🏁 Script executed:

# Check NextAuth version in package files
find . -name "package.json" -o -name "package-lock.json" -o -name "yarn.lock" | head -3

Repository: alfredo1996/neoboard

Length of output: 130


🏁 Script executed:

# Get NextAuth version
cat app/package.json 2>/dev/null | grep -A 2 -B 2 '"next-auth"' || \
cat package.json 2>/dev/null | grep -A 2 -B 2 '"next-auth"'

Repository: alfredo1996/neoboard

Length of output: 200


🏁 Script executed:

# Read the login page to see the full context around lines 46-50
cat -n app/src/app/\(auth\)/login/page.tsx | sed -n '35,60p'

Repository: alfredo1996/neoboard

Length of output: 987


🏁 Script executed:

# Double-check if there are any other signIn calls with different patterns in the codebase
rg "signIn.*redirect.*false" app/src --type ts --type tsx -A 3 -B 3

Repository: alfredo1996/neoboard

Length of output: 91


🏁 Script executed:

# Search for signIn calls with redirect false
rg "signIn.*redirect.*false" app/src -A 3 -B 3

Repository: alfredo1996/neoboard

Length of output: 46


The else branch is unreachable — but the real issue is the unsafe condition above it.

NextAuth v5 signIn("credentials", { redirect: false }) always returns a SignInResponse object { ok, error, code } on both success and failure. It only throws on network errors (caught by your catch block). The else branch will never execute.

However, the bigger problem is else if (result) at line 44 — it allows redirect even when result.ok is false and result.error is undefined. Change to else if (result?.ok) to safely verify success before redirecting:

♻️ Suggested fix
      if (result?.error) {
         setError("Invalid email or password");
         setLoading(false);
-      } else if (result) {
+      } else if (result?.ok) {
         router.push(callbackUrl);
       } else {
         setError("Unable to sign in. Please try again.");
         setLoading(false);
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/app/`(auth)/login/page.tsx around lines 46 - 50, The current sign-in
handling uses else if (result) which is unsafe because NextAuth v5 always
returns a SignInResponse; update the check to explicitly test success (e.g., use
result?.ok) so you only redirect on true, and remove or simplify the
now-unreachable else branch that assumes a null result; adjust the logic in the
submit/handleSignIn function where signIn("credentials", ...) is awaited and
result is inspected to use result?.ok and keep the existing catch for network
errors.

} catch {
// Network error or server unreachable
setError("Unable to reach server. Please check your connection.");
setLoading(false);
} else {
router.push(callbackUrl);
}
}

Expand All @@ -59,14 +69,21 @@ function LoginForm() {
id="email"
name="email"
type="email"
autoComplete="email"
required
placeholder="you@example.com"
/>
</div>

<div className="space-y-2">
<Label htmlFor="password">Password</Label>
<PasswordInput id="password" name="password" required minLength={6} />
<PasswordInput
id="password"
name="password"
autoComplete="current-password"
required
minLength={6}
/>
</div>

<LoadingButton
Expand Down
12 changes: 10 additions & 2 deletions app/src/app/api/connections/[id]/test/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ import { decryptJson } from "@/lib/crypto";
import { testConnection } from "@/lib/query-executor";
import type { ConnectionCredentials, DbType } from "@/lib/query-executor";
import { apiSuccess } from "@/lib/api-response";
import { notFound, handleRouteError } from "@/lib/api-utils";
import {
notFound,
handleRouteError,
sanitizeErrorMessage,
} from "@/lib/api-utils";

export async function POST(
_request: Request,
Expand Down Expand Up @@ -40,10 +44,14 @@ export async function POST(
...(!success ? { error: "Connection check returned false" } : {}),
});
} catch (testError) {
const message =
const rawMessage =
testError instanceof Error
? testError.message
: "Connection test failed";
const message = sanitizeErrorMessage(
rawMessage,
"Connection test failed",
);
return apiSuccess({ success: false, error: message });
}
} catch (error) {
Expand Down
12 changes: 10 additions & 2 deletions app/src/app/api/connections/test-inline/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { testConnection } from "@/lib/query-executor";
import type { DbType } from "@/lib/query-executor";
import { testInlineSchema } from "@/lib/schemas";
import { apiSuccess } from "@/lib/api-response";
import { handleRouteError, validateBody } from "@/lib/api-utils";
import {
handleRouteError,
validateBody,
sanitizeErrorMessage,
} from "@/lib/api-utils";

export async function POST(request: Request) {
try {
Expand Down Expand Up @@ -36,10 +40,14 @@ export async function POST(request: Request) {
...(!success ? { error: "Connection check returned false" } : {}),
});
} catch (testError) {
const message =
const rawMessage =
testError instanceof Error
? testError.message
: "Connection test failed";
const message = sanitizeErrorMessage(
rawMessage,
"Connection test failed",
);
return apiSuccess({ success: false, error: message });
}
} catch (error) {
Expand Down
41 changes: 41 additions & 0 deletions app/src/lib/__tests__/api-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
serverError,
handleRouteError,
validateBody,
sanitizeErrorMessage,
} from "../api-utils";
import { UnauthorizedError, ForbiddenError } from "../auth/errors";
import { z } from "zod";
Expand Down Expand Up @@ -116,3 +117,43 @@ describe("validateBody", () => {
}
});
});

describe("sanitizeErrorMessage", () => {
it("returns user-readable message as-is", () => {
expect(sanitizeErrorMessage("Connection refused")).toBe(
"Connection refused",
);
});

it("strips Turbopack internal paths", () => {
const raw =
"(0 , __TURBOPACK__imported__module__$5b$project$5d2f$app$2f$src$2f$lib.ts__$5b$app$2d$route$5d$.createConnectionModule) is not a function";
expect(sanitizeErrorMessage(raw)).toBe(
"Internal server error — check server logs",
);
});

it("strips webpack internal paths", () => {
const raw = "__webpack_require__ is not defined";
expect(sanitizeErrorMessage(raw)).toBe(
"Internal server error — check server logs",
);
});

it("strips encoded module paths with $XX$ pattern", () => {
const raw = "Error at $5b$module$5d$ resolution";
expect(sanitizeErrorMessage(raw)).toBe(
"Internal server error — check server logs",
);
});

it("uses custom fallback", () => {
expect(sanitizeErrorMessage("__TURBOPACK__foo", "Connection failed")).toBe(
"Connection failed",
);
});

it("preserves short error messages", () => {
expect(sanitizeErrorMessage("ECONNREFUSED")).toBe("ECONNREFUSED");
});
});
222 changes: 222 additions & 0 deletions app/src/lib/__tests__/chart-plugin-registry.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
import { describe, it, expect, beforeEach } from "vitest";
import {
defineChartPlugin,
createPluginRegistry,
type ChartPlugin,
} from "../chart-plugin-registry";

// ---------------------------------------------------------------------------
// Fixtures
// ---------------------------------------------------------------------------

const fakeComponent = () => null;

const makePlugin = (overrides: Partial<ChartPlugin> = {}): ChartPlugin =>
defineChartPlugin({
type: "bar",
label: "Bar Chart",
component: fakeComponent,
transform: (rows) => rows,
...overrides,
});

// ---------------------------------------------------------------------------
// defineChartPlugin — config normalization
// ---------------------------------------------------------------------------

describe("defineChartPlugin", () => {
it("creates a plugin with minimal required fields", () => {
const plugin = defineChartPlugin({
type: "bar",
label: "Bar Chart",
component: fakeComponent,
transform: (rows) => rows,
});
expect(plugin.type).toBe("bar");
expect(plugin.label).toBe("Bar Chart");
expect(plugin.component).toBe(fakeComponent);
});

it("applies sensible defaults to capabilities", () => {
const plugin = makePlugin();
expect(plugin.capabilities.supportsClickAction).toBe(true);
expect(plugin.capabilities.supportsStyling).toBe(false);
expect(plugin.capabilities.requiresQuery).toBe(true);
expect(plugin.capabilities.isECharts).toBe(false);
});

it("enables supportsStyling when stylingTargets is provided", () => {
const plugin = makePlugin({
stylingTargets: [{ value: "color", label: "Color" }],
});
expect(plugin.capabilities.supportsStyling).toBe(true);
});

it("allows explicit capability overrides", () => {
const plugin = makePlugin({
capabilities: {
supportsClickAction: false,
requiresQuery: false,
supportsStyling: false,
isECharts: false,
},
});
expect(plugin.capabilities.supportsClickAction).toBe(false);
expect(plugin.capabilities.requiresQuery).toBe(false);
});

it("preserves options when provided", () => {
const plugin = makePlugin({
options: [
{
key: "stacked",
label: "Stacked",
type: "boolean",
default: false,
category: "Layout",
},
],
});
expect(plugin.options).toHaveLength(1);
expect(plugin.options?.[0].key).toBe("stacked");
});

it("defaults options to empty array when not provided", () => {
const plugin = makePlugin();
expect(plugin.options).toEqual([]);
});

it("preserves compatibleWith list", () => {
const plugin = makePlugin({ compatibleWith: ["neo4j"] });
expect(plugin.compatibleWith).toEqual(["neo4j"]);
});

it("preserves queryHint", () => {
const plugin = makePlugin({ queryHint: "Return label, value" });
expect(plugin.queryHint).toBe("Return label, value");
});
});

// ---------------------------------------------------------------------------
// Registry — register / lookup
// ---------------------------------------------------------------------------

describe("createPluginRegistry", () => {
let registry: ReturnType<typeof createPluginRegistry>;

beforeEach(() => {
registry = createPluginRegistry();
});

it("registers a plugin and retrieves it by type", () => {
const plugin = makePlugin({ type: "bar" });
registry.register(plugin);
expect(registry.get("bar")).toBe(plugin);
});

it("returns undefined for unknown type", () => {
expect(registry.get("unknown-chart")).toBeUndefined();
});

it("throws on duplicate registration of same type", () => {
registry.register(makePlugin({ type: "bar" }));
expect(() => registry.register(makePlugin({ type: "bar" }))).toThrow(
/already registered/i,
);
});

it("allows multiple distinct plugins", () => {
registry.register(makePlugin({ type: "bar", label: "Bar" }));
registry.register(makePlugin({ type: "line", label: "Line" }));
registry.register(makePlugin({ type: "pie", label: "Pie" }));
expect(registry.getAll()).toHaveLength(3);
});

it("lists all registered plugins in registration order", () => {
registry.register(makePlugin({ type: "bar" }));
registry.register(makePlugin({ type: "line" }));
const all = registry.getAll();
expect(all.map((p) => p.type)).toEqual(["bar", "line"]);
});

it("has() returns true for registered types", () => {
registry.register(makePlugin({ type: "bar" }));
expect(registry.has("bar")).toBe(true);
expect(registry.has("line")).toBe(false);
});

it("getTypes() returns all registered type names", () => {
registry.register(makePlugin({ type: "bar" }));
registry.register(makePlugin({ type: "pie" }));
expect(registry.getTypes()).toEqual(["bar", "pie"]);
});

it("unregister() removes a plugin", () => {
registry.register(makePlugin({ type: "bar" }));
registry.unregister("bar");
expect(registry.get("bar")).toBeUndefined();
expect(registry.has("bar")).toBe(false);
});

it("unregister() is safe on unknown types", () => {
expect(() => registry.unregister("nope")).not.toThrow();
});

it("filters plugins by compatible connector type", () => {
registry.register(makePlugin({ type: "graph", compatibleWith: ["neo4j"] }));
registry.register(
makePlugin({ type: "bar", compatibleWith: ["neo4j", "postgresql"] }),
);
registry.register(makePlugin({ type: "pie" })); // no compatibleWith = all

const neo4jPlugins = registry.getCompatibleWith("neo4j");
expect(neo4jPlugins.map((p) => p.type).sort()).toEqual([
"bar",
"graph",
"pie",
]);

const pgPlugins = registry.getCompatibleWith("postgresql");
expect(pgPlugins.map((p) => p.type).sort()).toEqual(["bar", "pie"]);
});
});

// ---------------------------------------------------------------------------
// Validation — defineChartPlugin guards invalid configs
// ---------------------------------------------------------------------------

describe("defineChartPlugin validation", () => {
it("throws when type is empty", () => {
expect(() =>
defineChartPlugin({
type: "",
label: "Empty",
component: fakeComponent,
transform: (r) => r,
}),
).toThrow(/type.*required/i);
});

it("throws when label is empty", () => {
expect(() =>
defineChartPlugin({
type: "bar",
label: "",
component: fakeComponent,
transform: (r) => r,
}),
).toThrow(/label.*required/i);
});

it("throws when transform is not a function", () => {
expect(() =>
defineChartPlugin({
type: "bar",
label: "Bar",
component: fakeComponent,
// @ts-expect-error -- deliberately invalid
transform: "not a function",
}),
).toThrow(/transform.*function/i);
});
});
Loading
Loading