-
Notifications
You must be signed in to change notification settings - Fork 34
Feature: Implement Sign-Up Rate Limiting #134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e788636
97d71d1
2aeb968
a6f4fa9
1c8e1ed
89caac5
424358e
228ae45
87fc2c3
2a92b5a
b17f560
ada193f
a950aa5
b9adb0f
3f7d1d4
9fbb68b
2ac2a5c
2010cc0
b837c7c
5b7ad4b
0f99957
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,7 @@ | |
"@types/mdx": "^2.0.13", | ||
"@types/micromatch": "^4.0.9", | ||
"@vercel/analytics": "^1.5.0", | ||
"@vercel/functions": "^3.1.0", | ||
"@vercel/kv": "^3.0.0", | ||
"@vercel/otel": "^1.13.0", | ||
"@vercel/speed-insights": "^1.2.0", | ||
|
@@ -1166,8 +1167,12 @@ | |
|
||
"@vercel/analytics": ["@vercel/[email protected]", "", { "peerDependencies": { "@remix-run/react": "^2", "@sveltejs/kit": "^1 || ^2", "next": ">= 13", "react": "^18 || ^19 || ^19.0.0-rc", "svelte": ">= 4", "vue": "^3", "vue-router": "^4" }, "optionalPeers": ["@remix-run/react", "@sveltejs/kit", "next", "react", "svelte", "vue", "vue-router"] }, "sha512-MYsBzfPki4gthY5HnYN7jgInhAZ7Ac1cYDoRWFomwGHWEX7odTEzbtg9kf/QSo7XEsEAqlQugA6gJ2WS2DEa3g=="], | ||
|
||
"@vercel/functions": ["@vercel/[email protected]", "", { "dependencies": { "@vercel/oidc": "3.0.0" }, "peerDependencies": { "@aws-sdk/credential-provider-web-identity": "*" }, "optionalPeers": ["@aws-sdk/credential-provider-web-identity"] }, "sha512-V+p8dO+sg1VjiJJUO5rYPp1KG17SzDcR74OWwW7Euyde6L8U5wuTMe9QfEOfLTiWPUPzN1MXZvLcYxqSYhKc4Q=="], | ||
|
||
"@vercel/kv": ["@vercel/[email protected]", "", { "dependencies": { "@upstash/redis": "^1.34.0" } }, "sha512-pKT8fRnfyYk2MgvyB6fn6ipJPCdfZwiKDdw7vB+HL50rjboEBHDVBEcnwfkEpVSp2AjNtoaOUH7zG+bVC/rvSg=="], | ||
|
||
"@vercel/oidc": ["@vercel/[email protected]", "", { "dependencies": { "@types/ms": "2.1.0", "ms": "2.1.3" } }, "sha512-XOoUcf/1VfGArUAfq0ELxk6TD7l4jGcrOsWjQibj4wYM74uNihzZ9gA46ywWegoqKWWdph4y5CKxGI9823deoA=="], | ||
|
||
"@vercel/otel": ["@vercel/[email protected]", "", { "peerDependencies": { "@opentelemetry/api": ">=1.7.0 <2.0.0", "@opentelemetry/api-logs": ">=0.46.0 <0.200.0", "@opentelemetry/instrumentation": ">=0.46.0 <0.200.0", "@opentelemetry/resources": ">=1.19.0 <2.0.0", "@opentelemetry/sdk-logs": ">=0.46.0 <0.200.0", "@opentelemetry/sdk-metrics": ">=1.19.0 <2.0.0", "@opentelemetry/sdk-trace-base": ">=1.19.0 <2.0.0" } }, "sha512-esRkt470Y2jRK1B1g7S1vkt4Csu44gp83Zpu8rIyPoqy2BKgk4z7ik1uSMswzi45UogLHFl6yR5TauDurBQi4Q=="], | ||
|
||
"@vercel/speed-insights": ["@vercel/[email protected]", "", { "peerDependencies": { "@sveltejs/kit": "^1 || ^2", "next": ">= 13", "react": "^18 || ^19 || ^19.0.0-rc", "svelte": ">= 4", "vue": "^3", "vue-router": "^4" }, "optionalPeers": ["@sveltejs/kit", "next", "react", "svelte", "vue", "vue-router"] }, "sha512-y9GVzrUJ2xmgtQlzFP2KhVRoCglwfRQgjyfY607aU0hh0Un6d0OUyrJkjuAlsV18qR4zfoFPs/BiIj9YDS6Wzw=="], | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,10 @@ | ||
export const ALLOW_SEO_INDEXING = process.env.ALLOW_SEO_INDEXING === '1' | ||
export const VERBOSE = process.env.NEXT_PUBLIC_VERBOSE === '1' | ||
export const INCLUDE_BILLING = process.env.NEXT_PUBLIC_INCLUDE_BILLING === '1' | ||
export const ENABLE_SIGN_UP_RATE_LIMITING = | ||
process.env.ENABLE_SIGN_UP_RATE_LIMITING === '1' && | ||
process.env.KV_REST_API_URL && | ||
process.env.KV_REST_API_TOKEN | ||
export const USE_MOCK_DATA = | ||
process.env.VERCEL_ENV !== 'production' && | ||
process.env.NEXT_PUBLIC_MOCK_DATA === '1' |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
const DEFAULT_SIGN_UP_LIMIT_PER_WINDOW = 3 | ||
const DEFAULT_SIGN_UP_WINDOW_HOURS = 24 | ||
|
||
export { DEFAULT_SIGN_UP_LIMIT_PER_WINDOW, DEFAULT_SIGN_UP_WINDOW_HOURS } |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
import 'server-cli-only' | ||
|
||
import { KV_KEYS } from '@/configs/keys' | ||
import { | ||
DEFAULT_SIGN_UP_LIMIT_PER_WINDOW, | ||
DEFAULT_SIGN_UP_WINDOW_HOURS, | ||
} from '@/configs/limits' | ||
import { kv } from '@vercel/kv' | ||
|
||
// we need to ensure the limits are valid numbers | ||
// if parseInt returns NaN, we fallback to the default | ||
const SIGN_UP_LIMIT_PER_WINDOW = | ||
parseInt( | ||
process.env.SIGN_UP_LIMIT_PER_WINDOW || | ||
DEFAULT_SIGN_UP_LIMIT_PER_WINDOW.toString() | ||
) || DEFAULT_SIGN_UP_LIMIT_PER_WINDOW | ||
|
||
const SIGN_UP_WINDOW_HOURS = | ||
parseInt( | ||
process.env.SIGN_UP_WINDOW_HOURS || DEFAULT_SIGN_UP_WINDOW_HOURS.toString() | ||
) || DEFAULT_SIGN_UP_WINDOW_HOURS | ||
|
||
/** | ||
* Increments the sign-up attempt counter and checks if the rate limit has been reached. | ||
* Uses a Lua script for atomic execution to avoid race conditions. | ||
* | ||
* The script: | ||
* 1. Increments the counter | ||
* 2. Sets TTL on first increment | ||
* 3. Returns the new count | ||
* | ||
* @param identifier - The unique identifier (e.g., IP address) to track rate limit for | ||
* @returns Promise<boolean> - Returns true if the rate limit has been exceeded (no more attempts allowed), | ||
* false if more attempts are available | ||
*/ | ||
export async function incrementAndCheckSignUpRateLimit( | ||
identifier: string | ||
): Promise<boolean> { | ||
const key = KV_KEYS.RATE_LIMIT_SIGN_UP(identifier) | ||
const windowSeconds = SIGN_UP_WINDOW_HOURS * 60 * 60 | ||
|
||
// executes atomically on redis server | ||
// we ensure TTL exists once per window, even if expire would fail on first increment for some reason | ||
const luaScript = ` | ||
local count = redis.call('INCR', KEYS[1]) | ||
if count == 1 then | ||
redis.call('EXPIRE', KEYS[1], ARGV[1]) | ||
else | ||
if redis.call('TTL', KEYS[1]) == -1 then | ||
redis.call('EXPIRE', KEYS[1], ARGV[1]) | ||
end | ||
end | ||
return count | ||
` | ||
|
||
const count = await kv.eval<string[], number>( | ||
luaScript, | ||
[key], | ||
[windowSeconds.toString()] | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Incorrect Type Parameters in Rate Limit FunctionsThe Additional Locations (1) |
||
|
||
// return true if limit exceeded (rate limited) | ||
return count > SIGN_UP_LIMIT_PER_WINDOW | ||
} | ||
ben-fornefeld marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/** | ||
* Decrements the sign-up attempt counter when a sign-up fails. | ||
* Uses a Lua script for atomic execution to avoid race conditions. | ||
* | ||
* The script only decrements if: | ||
* 1. Key exists | ||
* 2. Current count > 0 | ||
* | ||
* This allows the user to retry since no account was actually created. | ||
* | ||
* @param identifier - The unique identifier whose rate limit should be decremented | ||
*/ | ||
export async function decrementSignUpRateLimit(identifier: string) { | ||
const key = KV_KEYS.RATE_LIMIT_SIGN_UP(identifier) | ||
|
||
// executes atomically on redis server | ||
const luaScript = ` | ||
local current = redis.call('GET', KEYS[1]) | ||
if current and tonumber(current) > 0 then | ||
return redis.call('DECR', KEYS[1]) | ||
end | ||
return current | ||
` | ||
|
||
await kv.eval<string[], number>(luaScript, [key], []) | ||
} | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Rate Limiting Flag Evaluates Incorrectly
The
ENABLE_SIGN_UP_RATE_LIMITING
flag's logic is flawed. It evaluates to a non-boolean string (e.g.,KV_REST_API_TOKEN
's value or an empty string) when enabled, which may cause unexpected conditional behavior. It also treats empty strings forKV_REST_API_URL
andKV_REST_API_TOKEN
as valid, potentially leading to silent rate limiting failures.