Fix forward auth security vulnerabilities found during pentest
- Fix broken rate limiting: add registerFailedAttempt/resetAttempts calls - Remove raw session token from exchange table; generate fresh token at redemption - Fix TOCTOU race: atomic UPDATE...WHERE used=false for exchange redemption - Delete exchange records immediately after redemption - Change bcrypt.compareSync to async bcrypt.compare to prevent event loop blocking - Fix IP extraction: prefer x-real-ip, fall back to last x-forwarded-for entry - Restrict redirect URI scheme to http/https only - Add Origin header CSRF check on login and session-login endpoints - Remove admin auto-access bypass from checkHostAccess (deny-by-default for all) - Revoke forward auth sessions when user status changes away from active - Validate portal domain against registered forward-auth hosts to prevent phishing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
import { auth } from "@/src/lib/auth";
|
||||
import { getEnabledOAuthProviders } from "@/src/lib/config";
|
||||
import { isForwardAuthDomain } from "@/src/lib/models/forward-auth";
|
||||
import PortalLoginForm from "./PortalLoginForm";
|
||||
|
||||
interface PortalPageProps {
|
||||
@@ -10,10 +11,16 @@ export default async function PortalPage({ searchParams }: PortalPageProps) {
|
||||
const params = await searchParams;
|
||||
const redirectUri = params.rd ?? "";
|
||||
|
||||
// Only display the target domain if it's a genuine forward-auth-protected host.
|
||||
// This prevents attackers from using the portal to phish with arbitrary domain names
|
||||
// and avoids leaking the list of configured proxies (we only confirm/deny a specific domain).
|
||||
let targetDomain = "";
|
||||
try {
|
||||
if (redirectUri) {
|
||||
targetDomain = new URL(redirectUri).hostname;
|
||||
const hostname = new URL(redirectUri).hostname;
|
||||
if (await isForwardAuthDomain(hostname)) {
|
||||
targetDomain = hostname;
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// invalid URL — will be caught by the login endpoint
|
||||
|
||||
@@ -8,7 +8,7 @@ import {
|
||||
checkHostAccessByDomain
|
||||
} from "@/src/lib/models/forward-auth";
|
||||
import { logAuditEvent } from "@/src/lib/audit";
|
||||
import { isRateLimited } from "@/src/lib/rate-limit";
|
||||
import { isRateLimited, registerFailedAttempt, resetAttempts } from "@/src/lib/rate-limit";
|
||||
|
||||
/**
|
||||
* Forward auth login endpoint — validates credentials and starts the exchange flow.
|
||||
@@ -16,6 +16,13 @@ import { isRateLimited } from "@/src/lib/rate-limit";
|
||||
*/
|
||||
export async function POST(request: NextRequest) {
|
||||
try {
|
||||
// CSRF: verify the request originates from the CPM portal
|
||||
const origin = request.headers.get("origin");
|
||||
const baseOrigin = new URL(config.baseUrl).origin;
|
||||
if (!origin || origin !== baseOrigin) {
|
||||
return NextResponse.json({ error: "Forbidden" }, { status: 403 });
|
||||
}
|
||||
|
||||
const body = await request.json();
|
||||
const username = typeof body.username === "string" ? body.username.trim() : "";
|
||||
const password = typeof body.password === "string" ? body.password : "";
|
||||
@@ -28,16 +35,22 @@ export async function POST(request: NextRequest) {
|
||||
return NextResponse.json({ error: "Redirect URI is required" }, { status: 400 });
|
||||
}
|
||||
|
||||
// Validate redirect URI
|
||||
// Validate redirect URI — only allow http/https schemes
|
||||
let targetUrl: URL;
|
||||
try {
|
||||
targetUrl = new URL(redirectUri);
|
||||
} catch {
|
||||
return NextResponse.json({ error: "Invalid redirect URI" }, { status: 400 });
|
||||
}
|
||||
if (targetUrl.protocol !== "https:" && targetUrl.protocol !== "http:") {
|
||||
return NextResponse.json({ error: "Invalid redirect URI scheme" }, { status: 400 });
|
||||
}
|
||||
|
||||
// Rate limiting
|
||||
const ip = request.headers.get("x-forwarded-for")?.split(",")[0]?.trim() ?? "unknown";
|
||||
// Rate limiting — prefer x-real-ip (set by reverse proxy) over x-forwarded-for
|
||||
const ip =
|
||||
request.headers.get("x-real-ip")?.trim() ||
|
||||
request.headers.get("x-forwarded-for")?.split(",").pop()?.trim() ||
|
||||
"unknown";
|
||||
const rateLimitResult = isRateLimited(ip);
|
||||
if (rateLimitResult.blocked) {
|
||||
return NextResponse.json(
|
||||
@@ -53,6 +66,7 @@ export async function POST(request: NextRequest) {
|
||||
});
|
||||
|
||||
if (!user || user.status !== "active" || !user.passwordHash) {
|
||||
registerFailedAttempt(ip);
|
||||
logAuditEvent({
|
||||
userId: null,
|
||||
action: "forward_auth_login_failed",
|
||||
@@ -62,8 +76,9 @@ export async function POST(request: NextRequest) {
|
||||
return NextResponse.json({ error: "Invalid credentials" }, { status: 401 });
|
||||
}
|
||||
|
||||
const isValid = bcrypt.compareSync(password, user.passwordHash);
|
||||
const isValid = await bcrypt.compare(password, user.passwordHash);
|
||||
if (!isValid) {
|
||||
registerFailedAttempt(ip);
|
||||
logAuditEvent({
|
||||
userId: user.id,
|
||||
action: "forward_auth_login_failed",
|
||||
@@ -74,6 +89,9 @@ export async function POST(request: NextRequest) {
|
||||
return NextResponse.json({ error: "Invalid credentials" }, { status: 401 });
|
||||
}
|
||||
|
||||
// Successful credential check — reset rate limiter for this IP
|
||||
resetAttempts(ip);
|
||||
|
||||
// Check if user has access to the target host
|
||||
const { hasAccess } = await checkHostAccessByDomain(user.id, targetUrl.hostname);
|
||||
if (!hasAccess) {
|
||||
@@ -90,8 +108,8 @@ export async function POST(request: NextRequest) {
|
||||
}
|
||||
|
||||
// Create session and exchange code
|
||||
const { rawToken, session } = await createForwardAuthSession(user.id);
|
||||
const { rawCode } = await createExchangeCode(session.id, rawToken, redirectUri);
|
||||
const { session } = await createForwardAuthSession(user.id);
|
||||
const { rawCode } = await createExchangeCode(session.id, redirectUri);
|
||||
|
||||
logAuditEvent({
|
||||
userId: user.id,
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { NextRequest, NextResponse } from "next/server";
|
||||
import { auth } from "@/src/lib/auth";
|
||||
import { config } from "@/src/lib/config";
|
||||
import {
|
||||
createForwardAuthSession,
|
||||
createExchangeCode,
|
||||
@@ -13,6 +14,13 @@ import { logAuditEvent } from "@/src/lib/audit";
|
||||
*/
|
||||
export async function POST(request: NextRequest) {
|
||||
try {
|
||||
// CSRF: verify the request originates from the CPM portal
|
||||
const origin = request.headers.get("origin");
|
||||
const baseOrigin = new URL(config.baseUrl).origin;
|
||||
if (!origin || origin !== baseOrigin) {
|
||||
return NextResponse.json({ error: "Forbidden" }, { status: 403 });
|
||||
}
|
||||
|
||||
const session = await auth();
|
||||
if (!session?.user?.id) {
|
||||
return NextResponse.json({ error: "Not authenticated" }, { status: 401 });
|
||||
@@ -31,6 +39,9 @@ export async function POST(request: NextRequest) {
|
||||
} catch {
|
||||
return NextResponse.json({ error: "Invalid redirect URI" }, { status: 400 });
|
||||
}
|
||||
if (targetUrl.protocol !== "https:" && targetUrl.protocol !== "http:") {
|
||||
return NextResponse.json({ error: "Invalid redirect URI scheme" }, { status: 400 });
|
||||
}
|
||||
|
||||
const userId = Number(session.user.id);
|
||||
|
||||
@@ -50,8 +61,8 @@ export async function POST(request: NextRequest) {
|
||||
}
|
||||
|
||||
// Create forward auth session and exchange code
|
||||
const { rawToken, session: faSession } = await createForwardAuthSession(userId);
|
||||
const { rawCode } = await createExchangeCode(faSession.id, rawToken, redirectUri);
|
||||
const { session: faSession } = await createForwardAuthSession(userId);
|
||||
const { rawCode } = await createExchangeCode(faSession.id, redirectUri);
|
||||
|
||||
logAuditEvent({
|
||||
userId,
|
||||
|
||||
@@ -10,7 +10,7 @@ import {
|
||||
groups,
|
||||
proxyHosts
|
||||
} from "../db/schema";
|
||||
import { eq, inArray, lt } from "drizzle-orm";
|
||||
import { and, eq, gt, inArray, lt } from "drizzle-orm";
|
||||
|
||||
const DEFAULT_SESSION_TTL = 7 * 24 * 60 * 60; // 7 days in seconds
|
||||
const EXCHANGE_CODE_TTL = 60; // 60 seconds
|
||||
@@ -96,7 +96,6 @@ export async function deleteUserForwardAuthSessions(userId: number): Promise<voi
|
||||
|
||||
export async function createExchangeCode(
|
||||
sessionId: number,
|
||||
rawSessionToken: string,
|
||||
redirectUri: string
|
||||
): Promise<{ rawCode: string }> {
|
||||
const rawCode = randomBytes(32).toString("hex");
|
||||
@@ -107,7 +106,7 @@ export async function createExchangeCode(
|
||||
await db.insert(forwardAuthExchanges).values({
|
||||
sessionId,
|
||||
codeHash,
|
||||
sessionToken: rawSessionToken,
|
||||
sessionToken: "[pending]", // placeholder — fresh token generated at redemption
|
||||
redirectUri,
|
||||
expiresAt,
|
||||
used: false,
|
||||
@@ -121,25 +120,42 @@ export async function redeemExchangeCode(
|
||||
rawCode: string
|
||||
): Promise<{ sessionId: number; redirectUri: string; rawSessionToken: string } | null> {
|
||||
const codeHash = hashToken(rawCode);
|
||||
const now = nowIso();
|
||||
|
||||
const exchange = await db.query.forwardAuthExchanges.findFirst({
|
||||
where: (table, operators) => operators.eq(table.codeHash, codeHash)
|
||||
});
|
||||
|
||||
if (!exchange) return null;
|
||||
if (exchange.used) return null;
|
||||
if (new Date(exchange.expiresAt) <= new Date()) return null;
|
||||
|
||||
// Mark as used atomically
|
||||
await db
|
||||
// Atomic claim: only succeeds if the exchange exists, is unused, and not expired
|
||||
const claimed = await db
|
||||
.update(forwardAuthExchanges)
|
||||
.set({ used: true })
|
||||
.where(
|
||||
and(
|
||||
eq(forwardAuthExchanges.codeHash, codeHash),
|
||||
eq(forwardAuthExchanges.used, false),
|
||||
gt(forwardAuthExchanges.expiresAt, now)
|
||||
)
|
||||
)
|
||||
.returning();
|
||||
|
||||
if (claimed.length === 0) return null;
|
||||
const exchange = claimed[0];
|
||||
|
||||
// Generate a fresh session token (never stored in the exchange table)
|
||||
const rawToken = randomBytes(32).toString("hex");
|
||||
const tokenHash = hashToken(rawToken);
|
||||
|
||||
await db
|
||||
.update(forwardAuthSessions)
|
||||
.set({ tokenHash })
|
||||
.where(eq(forwardAuthSessions.id, exchange.sessionId));
|
||||
|
||||
// Delete the redeemed exchange immediately
|
||||
await db
|
||||
.delete(forwardAuthExchanges)
|
||||
.where(eq(forwardAuthExchanges.id, exchange.id));
|
||||
|
||||
return {
|
||||
sessionId: exchange.sessionId,
|
||||
redirectUri: exchange.redirectUri,
|
||||
rawSessionToken: exchange.sessionToken
|
||||
rawSessionToken: rawToken
|
||||
};
|
||||
}
|
||||
|
||||
@@ -157,7 +173,6 @@ export async function checkHostAccess(
|
||||
userId: number,
|
||||
proxyHostId: number
|
||||
): Promise<boolean> {
|
||||
// Admins always have access
|
||||
const user = await db.query.users.findFirst({
|
||||
where: (table, operators) => operators.eq(table.id, userId)
|
||||
});
|
||||
@@ -276,6 +291,35 @@ export async function setForwardAuthAccess(
|
||||
return getForwardAuthAccessForHost(proxyHostId);
|
||||
}
|
||||
|
||||
// ── Domain Validation ────────────────────────────────────────────────
|
||||
|
||||
export async function isForwardAuthDomain(host: string): Promise<boolean> {
|
||||
const allHosts = await db.query.proxyHosts.findMany({
|
||||
where: (table, operators) => operators.eq(table.enabled, true)
|
||||
});
|
||||
|
||||
for (const ph of allHosts) {
|
||||
let domains: string[] = [];
|
||||
try {
|
||||
domains = JSON.parse(ph.domains);
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
if (domains.some((d) => d.toLowerCase() === host.toLowerCase())) {
|
||||
// Check that this host actually has forward auth enabled
|
||||
let meta: Record<string, unknown> = {};
|
||||
try {
|
||||
meta = ph.meta ? JSON.parse(ph.meta) : {};
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
const fa = meta.cpm_forward_auth as Record<string, unknown> | undefined;
|
||||
if (fa?.enabled) return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
// ── Cleanup ──────────────────────────────────────────────────────────
|
||||
|
||||
export async function cleanupExpiredSessions(): Promise<number> {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import db, { nowIso, toIso } from "../db";
|
||||
import { users } from "../db/schema";
|
||||
import { and, count, eq } from "drizzle-orm";
|
||||
import { deleteUserForwardAuthSessions } from "./forward-auth";
|
||||
|
||||
export type User = {
|
||||
id: number;
|
||||
@@ -160,6 +161,12 @@ export async function updateUserStatus(userId: number, status: string): Promise<
|
||||
.set({ status, updatedAt: now })
|
||||
.where(eq(users.id, userId))
|
||||
.returning();
|
||||
|
||||
// Revoke all forward auth sessions when user is deactivated
|
||||
if (status !== "active") {
|
||||
await deleteUserForwardAuthSessions(userId);
|
||||
}
|
||||
|
||||
return updated ? parseDbUser(updated) : null;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user