fix: session validation fixes

This commit is contained in:
Michael Freno
2026-01-11 11:23:32 -05:00
parent 842a6075f9
commit 1fb8f45705
5 changed files with 166 additions and 37 deletions

View File

@@ -1261,7 +1261,7 @@ function ActiveSessions(props: { userId: string }) {
</Show> </Show>
<div> <div>
Last active:{" "} Last active:{" "}
{formatDate(session.lastRotatedAt || session.createdAt)} {formatDate(session.lastActiveAt || session.createdAt)}
</div> </div>
<Show when={session.expiresAt}> <Show when={session.expiresAt}>
<div class="text-xs"> <div class="text-xs">

View File

@@ -419,11 +419,11 @@ export const userRouter = createTRPCRouter({
const conn = ConnectionFactory(); const conn = ConnectionFactory();
const res = await conn.execute({ const res = await conn.execute({
sql: `SELECT session_id, token_family, created_at, expires_at, last_rotated_at, sql: `SELECT session_id, token_family, created_at, expires_at, last_active_at,
rotation_count, client_ip, user_agent rotation_count, ip_address, user_agent
FROM Session FROM Session
WHERE user_id = ? AND revoked = 0 AND expires_at > datetime('now') WHERE user_id = ? AND revoked = 0 AND expires_at > datetime('now')
ORDER BY last_rotated_at DESC`, ORDER BY last_active_at DESC`,
args: [userId] args: [userId]
}); });
@@ -435,9 +435,9 @@ export const userRouter = createTRPCRouter({
tokenFamily: row.token_family, tokenFamily: row.token_family,
createdAt: row.created_at, createdAt: row.created_at,
expiresAt: row.expires_at, expiresAt: row.expires_at,
lastRotatedAt: row.last_rotated_at, lastActiveAt: row.last_active_at,
rotationCount: row.rotation_count, rotationCount: row.rotation_count,
clientIp: row.client_ip, clientIp: row.ip_address,
userAgent: row.user_agent, userAgent: row.user_agent,
isCurrent: currentSession?.sessionId === row.session_id isCurrent: currentSession?.sessionId === row.session_id
})); }));

View File

@@ -349,19 +349,33 @@ export const RATE_LIMITS = CONFIG_RATE_LIMITS;
/** /**
* Rate limiting middleware for login operations * Rate limiting middleware for login operations
* In development, skips IP rate limiting to avoid self-DoS
* For unknown IPs in production, uses stricter shared limits
*/ */
export async function rateLimitLogin( export async function rateLimitLogin(
email: string, email: string,
clientIP: string, clientIP: string,
event?: H3Event event?: H3Event
): Promise<void> { ): Promise<void> {
// In development, skip IP rate limiting to avoid self-DoS
if (env.NODE_ENV !== "development") {
const isUnknownIP = clientIP === "unknown";
const ipIdentifier = isUnknownIP
? `login:unknown-ip`
: `login:ip:${clientIP}`;
const ipLimit = isUnknownIP
? { maxAttempts: 3, windowMs: RATE_LIMITS.LOGIN_IP.windowMs } // Stricter for unknown IPs
: RATE_LIMITS.LOGIN_IP;
await checkRateLimit( await checkRateLimit(
`login:ip:${clientIP}`, ipIdentifier,
RATE_LIMITS.LOGIN_IP.maxAttempts, ipLimit.maxAttempts,
RATE_LIMITS.LOGIN_IP.windowMs, ipLimit.windowMs,
event event
); );
}
// Always rate limit by email in all environments
await checkRateLimit( await checkRateLimit(
`login:email:${email}`, `login:email:${email}`,
RATE_LIMITS.LOGIN_EMAIL.maxAttempts, RATE_LIMITS.LOGIN_EMAIL.maxAttempts,
@@ -372,48 +386,87 @@ export async function rateLimitLogin(
/** /**
* Rate limiting middleware for password reset * Rate limiting middleware for password reset
* In development, skips IP rate limiting to avoid self-DoS
* For unknown IPs in production, uses stricter shared limits
*/ */
export async function rateLimitPasswordReset( export async function rateLimitPasswordReset(
clientIP: string, clientIP: string,
event?: H3Event event?: H3Event
): Promise<void> { ): Promise<void> {
// In development, skip IP rate limiting to avoid self-DoS
if (env.NODE_ENV !== "development") {
const isUnknownIP = clientIP === "unknown";
const ipIdentifier = isUnknownIP
? `password-reset:unknown-ip`
: `password-reset:ip:${clientIP}`;
const ipLimit = isUnknownIP
? { maxAttempts: 2, windowMs: RATE_LIMITS.PASSWORD_RESET_IP.windowMs } // Stricter for unknown IPs
: RATE_LIMITS.PASSWORD_RESET_IP;
await checkRateLimit( await checkRateLimit(
`password-reset:ip:${clientIP}`, ipIdentifier,
RATE_LIMITS.PASSWORD_RESET_IP.maxAttempts, ipLimit.maxAttempts,
RATE_LIMITS.PASSWORD_RESET_IP.windowMs, ipLimit.windowMs,
event event
); );
} }
}
/** /**
* Rate limiting middleware for registration * Rate limiting middleware for registration
* In development, skips IP rate limiting to avoid self-DoS
* For unknown IPs in production, uses stricter shared limits
*/ */
export async function rateLimitRegistration( export async function rateLimitRegistration(
clientIP: string, clientIP: string,
event?: H3Event event?: H3Event
): Promise<void> { ): Promise<void> {
// In development, skip IP rate limiting to avoid self-DoS
if (env.NODE_ENV !== "development") {
const isUnknownIP = clientIP === "unknown";
const ipIdentifier = isUnknownIP
? `registration:unknown-ip`
: `registration:ip:${clientIP}`;
const ipLimit = isUnknownIP
? { maxAttempts: 2, windowMs: RATE_LIMITS.REGISTRATION_IP.windowMs } // Stricter for unknown IPs
: RATE_LIMITS.REGISTRATION_IP;
await checkRateLimit( await checkRateLimit(
`registration:ip:${clientIP}`, ipIdentifier,
RATE_LIMITS.REGISTRATION_IP.maxAttempts, ipLimit.maxAttempts,
RATE_LIMITS.REGISTRATION_IP.windowMs, ipLimit.windowMs,
event event
); );
} }
}
/** /**
* Rate limiting middleware for email verification * Rate limiting middleware for email verification
* In development, skips IP rate limiting to avoid self-DoS
* For unknown IPs in production, uses stricter shared limits
*/ */
export async function rateLimitEmailVerification( export async function rateLimitEmailVerification(
clientIP: string, clientIP: string,
event?: H3Event event?: H3Event
): Promise<void> { ): Promise<void> {
// In development, skip IP rate limiting to avoid self-DoS
if (env.NODE_ENV !== "development") {
const isUnknownIP = clientIP === "unknown";
const ipIdentifier = isUnknownIP
? `email-verification:unknown-ip`
: `email-verification:ip:${clientIP}`;
const ipLimit = isUnknownIP
? { maxAttempts: 3, windowMs: RATE_LIMITS.EMAIL_VERIFICATION_IP.windowMs } // Stricter for unknown IPs
: RATE_LIMITS.EMAIL_VERIFICATION_IP;
await checkRateLimit( await checkRateLimit(
`email-verification:ip:${clientIP}`, ipIdentifier,
RATE_LIMITS.EMAIL_VERIFICATION_IP.maxAttempts, ipLimit.maxAttempts,
RATE_LIMITS.EMAIL_VERIFICATION_IP.windowMs, ipLimit.windowMs,
event event
); );
} }
}
export const ACCOUNT_LOCKOUT = CONFIG_ACCOUNT_LOCKOUT; export const ACCOUNT_LOCKOUT = CONFIG_ACCOUNT_LOCKOUT;
export async function checkAccountLockout(userId: string): Promise<{ export async function checkAccountLockout(userId: string): Promise<{

View File

@@ -416,6 +416,79 @@ describe("Rate Limiting", () => {
}); });
}); });
describe("Unknown IP Handling", () => {
it("should handle 'unknown' IP in development without rate limiting", async () => {
// In development, IP rate limits should be skipped
// This test assumes NODE_ENV is 'development' or 'test'
const unknownIP = "unknown";
const email = `test-${Date.now()}@example.com`;
// Should allow many login attempts in development with unknown IP
// (only email rate limit applies)
for (let i = 0; i < RATE_LIMITS.LOGIN_EMAIL.maxAttempts; i++) {
const testEmail = `test-${Date.now()}-${i}@example.com`;
await rateLimitLogin(testEmail, unknownIP);
}
// Should be able to continue with different emails (no IP limit in dev)
await rateLimitLogin(`final-${Date.now()}@example.com`, unknownIP);
});
it("should still enforce email rate limits with unknown IP", async () => {
const unknownIP = "unknown";
const email = `test-${Date.now()}@example.com`;
// Use up email rate limit
for (let i = 0; i < RATE_LIMITS.LOGIN_EMAIL.maxAttempts; i++) {
await rateLimitLogin(email, unknownIP);
}
// Next attempt should fail due to email limit
try {
await rateLimitLogin(email, unknownIP);
expect.unreachable("Should have thrown");
} catch (error) {
expect(error).toBeInstanceOf(TRPCError);
}
});
it("should handle unknown IP in password reset", async () => {
const unknownIP = "unknown";
// In development, should allow many attempts (no IP limit)
for (let i = 0; i < 10; i++) {
await rateLimitPasswordReset(unknownIP);
}
// Should not throw in development
expect(true).toBe(true);
});
it("should handle unknown IP in registration", async () => {
const unknownIP = "unknown";
// In development, should allow many attempts (no IP limit)
for (let i = 0; i < 10; i++) {
await rateLimitRegistration(unknownIP);
}
// Should not throw in development
expect(true).toBe(true);
});
it("should handle unknown IP in email verification", async () => {
const unknownIP = "unknown";
// In development, should allow many attempts (no IP limit)
for (let i = 0; i < 10; i++) {
await rateLimitEmailVerification(unknownIP);
}
// Should not throw in development
expect(true).toBe(true);
});
});
describe("Rate Limit Configuration", () => { describe("Rate Limit Configuration", () => {
it("should have reasonable limits configured", () => { it("should have reasonable limits configured", () => {
// Login should be more permissive than registration // Login should be more permissive than registration

View File

@@ -185,19 +185,22 @@ export async function getAuthSession(
// In SSR contexts where headers may already be sent, use unsealSession directly // In SSR contexts where headers may already be sent, use unsealSession directly
if (skipUpdate) { if (skipUpdate) {
const { unsealSession } = await import("vinxi/http"); const { unsealSession } = await import("vinxi/http");
const cookieValue = getCookie(event, sessionConfig.cookieName); const cookieName = sessionConfig.name || "session";
const cookieValue = getCookie(event, cookieName);
if (!cookieValue) { if (!cookieValue) {
return null; return null;
} }
try { try {
const data = await unsealSession<SessionData>( // unsealSession returns Partial<Session<T>>, not T directly
event, const session = await unsealSession(event, sessionConfig, cookieValue);
sessionConfig,
cookieValue
);
if (!data || !data.userId || !data.sessionId) { if (!session?.data || typeof session.data !== "object") {
return null;
}
const data = session.data as SessionData;
if (!data.userId || !data.sessionId) {
return null; return null;
} }