Skip to content

fix(auth): add rate limiting for authentication endpoints#1940

Open
Ridanshi wants to merge 4 commits into
Priyanshu-byte-coder:mainfrom
Ridanshi:fix/cron-auth-bypass
Open

fix(auth): add rate limiting for authentication endpoints#1940
Ridanshi wants to merge 4 commits into
Priyanshu-byte-coder:mainfrom
Ridanshi:fix/cron-auth-bypass

Conversation

@Ridanshi
Copy link
Copy Markdown
Contributor

@Ridanshi Ridanshi commented Jun 3, 2026

Closes #1303

Security analysis

DevTrack authenticates exclusively via GitHub OAuth — there is no password, email, OTP, or magic-link authentication. Despite this, the NextAuth endpoints responsible for initiating and completing the OAuth handshake were completely unprotected:

Endpoint Risk
POST /api/auth/signin/github OAuth initiation — flooding exhausts GitHub token-exchange quota
GET /api/auth/callback/github Code exchange — flooding can probe for valid OAuth codes
GET /api/auth/link-github Secondary account link initiation
GET /api/auth/link-github/callback Secondary account link code exchange

Endpoints intentionally excluded from rate limiting (they are called on every page render and are not attack surfaces):

  • GET /api/auth/session
  • GET /api/auth/csrf
  • GET /api/auth/signout

Authentication flows discovered

GitHub OAuth only. No password, email/OTP, or magic-link flows exist in this codebase.

Rate limiting strategy

5 requests per IP per 15-minute fixed window (matching issue #1303).

A complete GitHub OAuth sign-in consumes 2 requests (initiation + callback), so the limit allows two full sign-in attempts plus one spare before throttling. In NODE_ENV=development the limit is relaxed to 1 000 so test suites and local sign-in flows are never blocked.

The implementation uses the shared createMemoryFixedWindowRateLimiter factory already present in the codebase (@/lib/rate-limit), keeping behaviour consistent with the metrics and contact rate limiters. The key namespace auth:<ip> is isolated from metrics-rate-limit:* to prevent any interference.

429 responses include X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, and Retry-After headers. Response body:

{ "error": "Too many authentication attempts. Please try again later." }

Files changed

src/lib/auth-rate-limit.ts (new)
Standalone auth rate limiter module. Exports checkAuthRateLimit(), isAuthSensitivePath(), AUTH_LIMIT, AUTH_WINDOW_MS, and AUTH_SENSITIVE_PREFIXES. No new dependencies.

src/middleware.ts (modified)

  • Imports checkAuthRateLimit, isAuthSensitivePath, AUTH_LIMIT from @/lib/auth-rate-limit.
  • Inserts an auth rate-limit block after the protected-route check and before the general metrics limiter.
  • Adds the four auth-sensitive paths to the config.matcher array.

Tests added

test/auth-rate-limit.test.ts — 22 tests:

  • isAuthSensitivePath (9 tests) — correct classification of protected paths (signin, callback, link-github) and excluded paths (session, csrf, signout, unrelated API routes); consistency with AUTH_SENSITIVE_PREFIXES.
  • Basic behaviour (5 tests) — first request allowed; remaining decrements; exactly AUTH_LIMIT requests then blocks; 429-compatible status when blocked; continued blocking after limit hit.
  • Window expiry (2 tests) — counter resets after AUTH_WINDOW_MS, does not reset before window ends.
  • IP isolation (2 tests) — independent counters per IP; blocking one IP does not affect another.
  • Custom limit override (2 tests) — higher limit (dev/test mode); limit of 1 blocks after one request.
  • Reset timestamp (2 tests) — reset points to window boundary; all requests in same window share the same reset epoch.

Verification

tsc --noEmit    # clean (no errors in changed files)
next lint       # warnings only (all pre-existing)
vitest run test/auth-rate-limit.test.ts  # 22/22 passed

Ridanshi added 2 commits June 3, 2026 16:50
Closes Priyanshu-byte-coder#1657

Three cron/sync endpoints bypassed authentication when NODE_ENV was
development. The condition used was:

  if (authHeader !== Bearer-secret AND process.env.NODE_ENV !== development)

This made the authorization check a no-op in dev mode, allowing any
unauthenticated caller to trigger bulk operations: sponsors sync,
WakaTime stats sync, and Discord notifications.

Root cause: a developer-convenience shortcut never removed before deploy.

Fix: created src/lib/cron-auth.ts with validateCronRequest() that
enforces a consistent, environment-independent fail-closed check:
  - CRON_SECRET not configured -> 500
  - Authorization header missing or wrong -> 401
  - Correct secret -> null (proceed)
No NODE_ENV bypass in any environment.

Affected routes (all fixed):
  src/app/api/wakatime/sync/route.ts
  src/app/api/sponsors/sync/route.ts
  src/app/api/notifications/discord-sync/route.ts

Already-correct (no change needed):
  src/app/api/cron/weekly-digest/route.ts

Local development: set CRON_SECRET in .env.local and supply the
matching Authorization header when calling endpoints manually.
Documented in .env.example.

Tests added:
  test/cron-auth.test.ts (17 tests) - unit tests for validateCronRequest
    and integration regression tests for discord-sync covering all cases
    including development environment enforcement
  test/wakatime-sync-auth.test.ts (12 tests) - replaced bypass test
    with enforcement tests for dev environment
  test/sponsors-sync.test.ts (12 tests) - auth and dev enforcement
Closes Priyanshu-byte-coder#1303

Security analysis
-----------------
DevTrack authenticates exclusively via GitHub OAuth; there is no
password, email, or OTP flow. Despite this, the NextAuth endpoints
responsible for initiating and completing the OAuth handshake were
entirely unprotected:

  POST /api/auth/signin/github         - initiates the OAuth redirect
  GET  /api/auth/callback/github       - exchanges the code for a token
  GET  /api/auth/link-github           - secondary account link initiation
  GET  /api/auth/link-github/callback  - secondary account link callback

Flooding these routes can exhaust GitHub token-exchange quota for the
application, probe for valid OAuth codes, or generate high server load.

Endpoints deliberately excluded from rate limiting:
  GET /api/auth/session  - polled on every page render
  GET /api/auth/csrf     - CSRF token fetch, not an attack surface
  GET /api/auth/signout  - termination, not initiation

Authentication flows discovered
---------------------------------
GitHub OAuth only. No password, email/OTP, or magic-link flows exist.

Rate limiting strategy
----------------------
5 requests per IP per 15-minute fixed window (issue Priyanshu-byte-coder#1303).
Allows two complete sign-in flows (initiation + callback) plus one
spare before throttling. In development the limit is raised to 1000.

Uses the shared createMemoryFixedWindowRateLimiter factory from
@/lib/rate-limit. Key namespace auth:<ip> is isolated from the
metrics-rate-limit:* namespace to prevent interference.

429 responses carry X-RateLimit-* and Retry-After headers. Body:
  { error: 'Too many authentication attempts. Please try again later.' }

Protected endpoints
-------------------
Four auth-sensitive paths added to the Next.js middleware matcher and
handled by the auth rate limiter before the general metrics gate.

Files changed
-------------
src/lib/auth-rate-limit.ts  (new)
  Exports checkAuthRateLimit(), isAuthSensitivePath(), AUTH_LIMIT,
  AUTH_WINDOW_MS, AUTH_SENSITIVE_PREFIXES. No new dependencies.

src/middleware.ts  (modified)
  Imports auth limiter; inserts auth rate-limit block after the
  protected-route check; adds auth paths to the matcher config.

Tests added
-----------
test/auth-rate-limit.test.ts - 22 tests:
  isAuthSensitivePath (9) - protected vs excluded path classification
  basic behaviour (5) - allow/block, remaining, 429-compatible status
  window expiry (2) - counter resets after window, not before
  IP isolation (2) - independent counters per IP
  custom limit override (2) - dev/test relaxation
  reset timestamp (2) - window-boundary reset, shared within window

Verification
------------
  tsc --noEmit  clean (no errors in changed files)
  next lint     warnings only (all pre-existing)
  vitest run    22/22 tests passed
@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 3, 2026

@Ridanshi is attempting to deploy a commit to the PRIYANSHU DOSHI's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added gssoc26 GSSoC 2026 contribution type:bug GSSoC type bonus: bug fix type:feature GSSoC type bonus: new feature type:security GSSoC type bonus: security (+20 pts) type:testing GSSoC type bonus: tests (+10 pts) labels Jun 3, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

GSSoC Label Checklist 🏷️

@Priyanshu-byte-coder — please apply the appropriate labels before merging:

Difficulty (pick one):

  • level:beginner — 20 pts
  • level:intermediate — 35 pts
  • level:advanced — 55 pts
  • level:critical — 80 pts

Quality (optional):

  • quality:clean — ×1.2 multiplier
  • quality:exceptional — ×1.5 multiplier

Validation (required to score):

  • gssoc:approved — counts for points
  • gssoc:invalid / gssoc:spam / gssoc:ai-slop — does not score

Type labels (type:*) are auto-detected from files and title. Review and adjust if needed.
Points formula: (difficulty × quality_multiplier) + type_bonus

@Priyanshu-byte-coder
Copy link
Copy Markdown
Owner

This PR has merge conflicts with main. Please rebase:

git fetch origin
git rebase origin/main
git push --force-with-lease

Ridanshi added 2 commits June 3, 2026 22:18
Upstream rewrote the in-memory rate-limiter: replaced the external
createMemoryFixedWindowRateLimiter factory with inline memoryBuckets,
added pruneMemoryBuckets, and inlined getIp header extraction.  This
PR added per-IP rate limiting on OAuth auth endpoints using
checkAuthRateLimit / isAuthSensitivePath / AUTH_LIMIT.  Both edits
touched the same lines in src/middleware.ts.

Resolution keeps all upstream improvements:
  - inline memoryBuckets / pruneMemoryBuckets / checkMemoryLimit
  - getIp reads headers directly (no external getClientIp)
  - removes createMemoryFixedWindowRateLimiter dependency from middleware

And preserves all PR auth rate-limiting functionality:
  - checkAuthRateLimit / isAuthSensitivePath / AUTH_LIMIT imports
  - runtime = nodejs
  - isAuthSensitivePath block intercepting OAuth endpoints
  - /api/:path* matcher so auth paths are covered
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gssoc26 GSSoC 2026 contribution type:bug GSSoC type bonus: bug fix type:feature GSSoC type bonus: new feature type:security GSSoC type bonus: security (+20 pts) type:testing GSSoC type bonus: tests (+10 pts)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security]: No Rate Limiting on Authentication — App Vulnerable to Brute Force Attacks

2 participants