Merge commit from fork

* fix(auth): enhance password validation and rate limiting for login attempts

- Added password length validation to prevent CPU DoS attacks, limiting to 128 characters.
- Implemented constant-time password verification to mitigate timing attacks.
- Adjusted rate limit for login attempts from 30 to 10 per 15 minutes for improved security.
- Updated login form validation to reflect new password length constraints.
- Introduced constants for authentication endpoints in the API.

* fixed sample size for timing test

* password validation messages

---------

Co-authored-by: Your Name <you@example.com>
This commit is contained in:
Victor Hugo dos Santos
2025-10-02 06:09:28 -03:00
committed by GitHub
parent 6cf1f49c8e
commit b67177ba55
7 changed files with 441 additions and 15 deletions

View File

@@ -120,7 +120,7 @@ describe("PasswordConfirmationModal", () => {
const confirmButton = screen.getByText("common.confirm");
await user.click(confirmButton);
expect(screen.getByText("String must contain at least 8 character(s)")).toBeInTheDocument();
expect(screen.getByText("Password must be at least 8 characters long")).toBeInTheDocument();
});
test("handles cancel button click and resets form", async () => {

View File

@@ -66,8 +66,21 @@ export const authOptions: NextAuthOptions = {
throw new Error("Invalid credentials");
}
// Validate password length to prevent CPU DoS attacks
// bcrypt processes passwords up to 72 bytes, but we limit to 128 characters for security
if (credentials.password && credentials.password.length > 128) {
if (await shouldLogAuthFailure(identifier)) {
logAuthAttempt("password_too_long", "credentials", "password_validation", UNKNOWN_DATA, credentials?.email);
}
throw new Error("Invalid credentials");
}
// Use a control hash when user doesn't exist to maintain constant timing.
const controlHash = "$2b$12$fzHf9le13Ss9UJ04xzmsjODXpFJxz6vsnupoepF5FiqDECkX2BH5q";
let user;
try {
// Perform database lookup
user = await prisma.user.findUnique({
where: {
email: credentials?.email,
@@ -79,6 +92,12 @@ export const authOptions: NextAuthOptions = {
throw Error("Internal server error. Please try again later");
}
// Always perform password verification to maintain constant timing. This is important to prevent timing attacks for user enumeration.
// Use actual hash if user exists, control hash if user doesn't exist
const hashToVerify = user?.password || controlHash;
const isValid = await verifyPassword(credentials.password, hashToVerify);
// Now check all conditions after constant-time operations are complete
if (!user) {
if (await shouldLogAuthFailure(identifier)) {
logAuthAttempt("user_not_found", "credentials", "user_lookup", UNKNOWN_DATA, credentials?.email);
@@ -96,8 +115,6 @@ export const authOptions: NextAuthOptions = {
throw new Error("Your account is currently inactive. Please contact the organization admin.");
}
const isValid = await verifyPassword(credentials.password, user.password);
if (!isValid) {
if (await shouldLogAuthFailure(user.email)) {
logAuthAttempt("invalid_password", "credentials", "password_validation", user.id, user.email);

View File

@@ -1,5 +1,14 @@
"use client";
import { zodResolver } from "@hookform/resolvers/zod";
import { useTranslate } from "@tolgee/react";
import { signIn } from "next-auth/react";
import Link from "next/dist/client/link";
import { useRouter, useSearchParams } from "next/navigation";
import { useEffect, useMemo, useRef, useState } from "react";
import { FormProvider, SubmitHandler, useForm } from "react-hook-form";
import { toast } from "react-hot-toast";
import { z } from "zod";
import { cn } from "@/lib/cn";
import { FORMBRICKS_LOGGED_IN_WITH_LS } from "@/lib/localStorage";
import { getFormattedErrorMessage } from "@/lib/utils/helper";
@@ -10,19 +19,13 @@ import { TwoFactorBackup } from "@/modules/ee/two-factor-auth/components/two-fac
import { Button } from "@/modules/ui/components/button";
import { FormControl, FormError, FormField, FormItem } from "@/modules/ui/components/form";
import { PasswordInput } from "@/modules/ui/components/password-input";
import { zodResolver } from "@hookform/resolvers/zod";
import { useTranslate } from "@tolgee/react";
import { signIn } from "next-auth/react";
import Link from "next/dist/client/link";
import { useRouter, useSearchParams } from "next/navigation";
import { useEffect, useMemo, useRef, useState } from "react";
import { FormProvider, SubmitHandler, useForm } from "react-hook-form";
import { toast } from "react-hot-toast";
import { z } from "zod";
const ZLoginForm = z.object({
email: z.string().email(),
password: z.string().min(8),
password: z
.string()
.min(8, { message: "Password must be at least 8 characters long" })
.max(128, { message: "Password must be 128 characters or less" }),
totpCode: z.string().optional(),
backupCode: z.string().optional(),
});

View File

@@ -1,7 +1,7 @@
export const rateLimitConfigs = {
// Authentication endpoints - stricter limits for security
auth: {
login: { interval: 900, allowedPerInterval: 30, namespace: "auth:login" }, // 30 per 15 minutes
login: { interval: 900, allowedPerInterval: 10, namespace: "auth:login" }, // 10 per 15 minutes
signup: { interval: 3600, allowedPerInterval: 30, namespace: "auth:signup" }, // 30 per hour
forgotPassword: { interval: 3600, allowedPerInterval: 5, namespace: "auth:forgot" }, // 5 per hour
verifyEmail: { interval: 3600, allowedPerInterval: 10, namespace: "auth:verify" }, // 10 per hour

View File

@@ -0,0 +1,402 @@
import { expect } from "@playwright/test";
import { logger } from "@formbricks/logger";
import { test } from "../../lib/fixtures";
// Authentication endpoints are hardcoded to avoid import issues
test.describe("Authentication Security Tests - Vulnerability Prevention", () => {
let csrfToken: string;
let testUser: { email: string; password: string };
test.beforeEach(async ({ request, users }) => {
// Get CSRF token for authentication requests
const csrfResponse = await request.get("/api/auth/csrf");
const csrfData = await csrfResponse.json();
csrfToken = csrfData.csrfToken;
// Create a test user for "existing user" scenarios with unique email
const uniqueId = Date.now() + Math.random();
const userName = "Security Test User";
const userEmail = `security-test-${uniqueId}@example.com`;
await users.create({
name: userName,
email: userEmail,
});
testUser = {
email: userEmail,
password: userName, // The fixture uses the name as password
};
});
test.describe("DoS Protection - Password Length Limits", () => {
test("should handle extremely long passwords without crashing", async ({ request }) => {
const email = "nonexistent-dos-test@example.com"; // Use non-existent email for DoS test
const extremelyLongPassword = "A".repeat(50000); // 50,000 characters
const start = Date.now();
const response = await request.post("/api/auth/callback/credentials", {
data: {
callbackUrl: "",
email: email,
password: extremelyLongPassword,
redirect: "false",
csrfToken: csrfToken,
json: "true",
},
headers: {
"Content-Type": "application/x-www-form-urlencoded",
},
});
const responseTime = Date.now() - start;
// Should not crash the server (no 500 errors)
expect(response.status()).not.toBe(500);
// Should handle gracefully
expect([200, 400, 401, 429]).toContain(response.status());
logger.info(
`Extremely long password (50k chars) processing time: ${responseTime}ms, status: ${response.status()}`
);
// Verify the security fix is working: long passwords should be rejected quickly
// In production, this should be much faster, but test environment has overhead
if (responseTime < 5000) {
logger.info("✅ Long password rejected quickly - DoS protection working");
} else {
logger.warn("⚠️ Long password took longer than expected - check DoS protection");
}
});
test("should handle password at 128 character limit", async ({ request }) => {
const email = "nonexistent-limit-test@example.com"; // Use non-existent email for limit test
const maxLengthPassword = "A".repeat(128); // Exactly at the 128 character limit
const response = await request.post("/api/auth/callback/credentials", {
data: {
callbackUrl: "",
email: email,
password: maxLengthPassword,
redirect: "false",
csrfToken: csrfToken,
json: "true",
},
headers: {
"Content-Type": "application/x-www-form-urlencoded",
},
});
// Should process normally (not rejected for length)
expect(response.status()).not.toBe(500);
expect([200, 400, 401, 429]).toContain(response.status());
logger.info(`Max length password (128 chars) status: ${response.status()}`);
});
test("should reject passwords over 128 characters", async ({ request }) => {
const email = "nonexistent-overlimit-test@example.com"; // Use non-existent email for over-limit test
const overLimitPassword = "A".repeat(10000); // 10,000 characters (over limit)
const start = Date.now();
const response = await request.post("/api/auth/callback/credentials", {
data: {
callbackUrl: "",
email: email,
password: overLimitPassword,
redirect: "false",
csrfToken: csrfToken,
json: "true",
},
headers: {
"Content-Type": "application/x-www-form-urlencoded",
},
});
const responseTime = Date.now() - start;
// Should not crash
expect(response.status()).not.toBe(500);
logger.info(
`Over-limit password (10k chars) processing time: ${responseTime}ms, status: ${response.status()}`
);
// The key security test: verify it doesn't take exponentially longer than shorter passwords
// This tests the DoS protection is working
});
});
test.describe("Timing Attack Prevention - User Enumeration Protection", () => {
test("should not reveal user existence through response timing differences", async ({ request }) => {
// Test multiple attempts to get reliable timing measurements
const attempts = 50;
const nonExistentTimes: number[] = [];
const existingUserTimes: number[] = [];
// Test non-existent user timing (multiple attempts for statistical reliability)
for (let i = 0; i < attempts; i++) {
const start = process.hrtime.bigint();
const response = await request.post("/api/auth/callback/credentials", {
data: {
callbackUrl: "",
email: `nonexistent-timing-${i}@example.com`,
password: "somepassword",
redirect: "false",
csrfToken: csrfToken,
json: "true",
},
headers: {
"Content-Type": "application/x-www-form-urlencoded",
},
});
const end = process.hrtime.bigint();
const responseTime = Number(end - start) / 1000000; // Convert to milliseconds
nonExistentTimes.push(responseTime);
expect(response.status()).not.toBe(500);
}
// Test existing user with wrong password timing (multiple attempts)
for (let i = 0; i < attempts; i++) {
const start = process.hrtime.bigint();
const response = await request.post("/api/auth/callback/credentials", {
data: {
callbackUrl: "",
email: testUser.email,
password: "wrongpassword123",
redirect: "false",
csrfToken: csrfToken,
json: "true",
},
headers: {
"Content-Type": "application/x-www-form-urlencoded",
},
});
const end = process.hrtime.bigint();
const responseTime = Number(end - start) / 1000000; // Convert to milliseconds
existingUserTimes.push(responseTime);
expect(response.status()).not.toBe(500);
}
// Calculate averages
const avgNonExistent = nonExistentTimes.reduce((a, b) => a + b, 0) / nonExistentTimes.length;
const avgExisting = existingUserTimes.reduce((a, b) => a + b, 0) / existingUserTimes.length;
// Calculate the timing difference percentage
const timingDifference = Math.abs(avgExisting - avgNonExistent);
const timingDifferencePercent = (timingDifference / Math.max(avgExisting, avgNonExistent)) * 100;
logger.info(
`Non-existent user avg: ${avgNonExistent.toFixed(2)}ms (${nonExistentTimes.map((t) => t.toFixed(0)).join(", ")})`
);
logger.info(
`Existing user avg: ${avgExisting.toFixed(2)}ms (${existingUserTimes.map((t) => t.toFixed(0)).join(", ")})`
);
logger.info(
`Timing difference: ${timingDifference.toFixed(2)}ms (${timingDifferencePercent.toFixed(1)}%)`
);
// CRITICAL SECURITY TEST: Timing difference should be minimal
// A large timing difference could allow attackers to enumerate users
// Allow up to 20% difference to account for network/system variance
if (timingDifferencePercent > 20) {
logger.warn(
`⚠️ SECURITY RISK: Timing difference of ${timingDifferencePercent.toFixed(1)}% could allow user enumeration!`
);
logger.warn(`⚠️ Consider implementing constant-time authentication to prevent timing attacks`);
} else {
logger.info(
`✅ Timing attack protection: Only ${timingDifferencePercent.toFixed(1)}% difference between scenarios`
);
}
// Fail the test if timing difference exceeds our security threshold
expect(timingDifferencePercent).toBeLessThan(20); // Fail at our actual security threshold
});
test("should return consistent status codes regardless of user existence", async ({ request }) => {
const scenarios = [
{
email: "nonexistent-status@example.com",
password: "testpassword",
description: "non-existent user",
},
{ email: testUser.email, password: "wrongpassword", description: "existing user, wrong password" },
];
const results: { scenario: string; status: number }[] = [];
for (const scenario of scenarios) {
const response = await request.post("/api/auth/callback/credentials", {
data: {
callbackUrl: "",
email: scenario.email,
password: scenario.password,
redirect: "false",
csrfToken: csrfToken,
json: "true",
},
headers: {
"Content-Type": "application/x-www-form-urlencoded",
},
});
results.push({
scenario: scenario.description,
status: response.status(),
});
expect(response.status()).not.toBe(500);
}
// Log results
results.forEach(({ scenario, status }) => {
logger.info(`Status test - ${scenario}: ${status}`);
});
// CRITICAL: Both scenarios should return the same status code
// Different status codes could reveal user existence
const statuses = results.map((r) => r.status);
const uniqueStatuses = [...new Set(statuses)];
if (uniqueStatuses.length > 1) {
logger.warn(
`⚠️ SECURITY RISK: Different status codes (${uniqueStatuses.join(", ")}) could allow user enumeration!`
);
} else {
logger.info(`✅ Status code consistency: Both scenarios return ${statuses[0]}`);
}
expect(uniqueStatuses.length).toBe(1);
});
});
test.describe("Security Headers and Response Safety", () => {
test("should include security headers in responses", async ({ request }) => {
const response = await request.post("/api/auth/callback/credentials", {
data: {
callbackUrl: "",
email: "nonexistent-headers-test@example.com",
password: "testpassword",
redirect: "false",
csrfToken: csrfToken,
json: "true",
},
headers: {
"Content-Type": "application/x-www-form-urlencoded",
},
});
// Check for important security headers
const headers = response.headers();
// These headers should be present for security
expect(headers["x-frame-options"]).toBeDefined();
expect(headers["x-content-type-options"]).toBe("nosniff");
if (headers["strict-transport-security"]) {
expect(headers["strict-transport-security"]).toContain("max-age");
}
if (headers["content-security-policy"]) {
expect(headers["content-security-policy"]).toContain("default-src");
}
logger.info("✅ Security headers present in authentication responses");
});
test("should not expose sensitive information in error responses", async ({ request }) => {
const response = await request.post("/api/auth/callback/credentials", {
data: {
callbackUrl: "",
email: "nonexistent-disclosure-test@example.com",
password: "A".repeat(10000), // Trigger long password handling
redirect: "false",
csrfToken: csrfToken,
json: "true",
},
headers: {
"Content-Type": "application/x-www-form-urlencoded",
},
});
const responseBody = await response.text();
// Log the actual response for debugging
logger.info(`Response status: ${response.status()}`);
logger.info(`Response body (first 500 chars): ${responseBody.substring(0, 500)}`);
// Check if this is an HTML response (which indicates NextAuth.js is returning a page instead of API response)
const isHtmlResponse =
responseBody.trim().startsWith("<!DOCTYPE html>") || responseBody.includes("<html");
if (isHtmlResponse) {
logger.info(
"✅ NextAuth.js returned HTML page instead of API response - this is expected behavior for security"
);
logger.info("✅ No sensitive technical information exposed in authentication API");
return; // Skip the sensitive information check for HTML responses
}
// Only check for sensitive information in actual API responses (JSON/text)
const sensitiveTerms = [
"password_too_long",
"bcrypt",
"hash",
"redis",
"database",
"prisma",
"stack trace",
"rate limit exceeded",
"authentication failed",
"sql",
"query",
"connection timeout",
"internal error",
];
let foundSensitiveInfo = false;
const foundTerms: string[] = [];
for (const term of sensitiveTerms) {
if (responseBody.toLowerCase().includes(term.toLowerCase())) {
foundSensitiveInfo = true;
foundTerms.push(term);
logger.warn(`Found "${term}" in response`);
}
}
if (foundSensitiveInfo) {
logger.warn(`⚠️ Found sensitive information in response: ${foundTerms.join(", ")}`);
logger.warn(`Full response body: ${responseBody}`);
} else {
logger.info("✅ No sensitive technical information exposed in error responses");
}
// Don't fail the test for generic web responses, only for actual security leaks
expect(foundSensitiveInfo).toBe(false);
});
test("should handle malformed requests gracefully", async ({ request }) => {
// Test with missing CSRF token
const response = await request.post("/api/auth/callback/credentials", {
data: {
callbackUrl: "",
email: "nonexistent-malformed-test@example.com",
password: "testpassword",
redirect: "false",
json: "true",
// Missing csrfToken
},
headers: {
"Content-Type": "application/x-www-form-urlencoded",
},
});
// Should handle gracefully, not crash
expect(response.status()).not.toBe(500);
expect([200, 400, 401, 403, 429]).toContain(response.status());
logger.info(`✅ Malformed request handled gracefully: status ${response.status()}`);
});
});
});

View File

@@ -5,6 +5,10 @@ export const ROLES_API_URL = `/api/v2/roles`;
export const ME_API_URL = `/api/v2/me`;
export const HEALTH_API_URL = `/api/v2/health`;
// Authentication endpoints
export const AUTH_CALLBACK_URL = `/api/auth/callback/credentials`;
export const AUTH_CSRF_URL = `/api/auth/csrf`;
export const TEAMS_API_URL = (organizationId: string) => `/api/v2/organizations/${organizationId}/teams`;
export const PROJECT_TEAMS_API_URL = (organizationId: string) =>
`/api/v2/organizations/${organizationId}/project-teams`;

View File

@@ -31,7 +31,7 @@ export type TUserEmail = z.infer<typeof ZUserEmail>;
export const ZUserPassword = z
.string()
.min(8)
.min(8, { message: "Password must be at least 8 characters long" })
.max(128, { message: "Password must be 128 characters or less" })
.regex(/^(?=.*[A-Z])(?=.*\d).*$/);