Skip to content

feat: brute-force protection for the login endpoint#104

Open
christianromeni wants to merge 1 commit into
mainfrom
feat/login-rate-limit
Open

feat: brute-force protection for the login endpoint#104
christianromeni wants to merge 1 commit into
mainfrom
feat/login-rate-limit

Conversation

@christianromeni

Copy link
Copy Markdown
Contributor

Problem

POST /api/v1/auth/login had no rate limiting and no account lockout. bcrypt slows a single attempt, but a wordlist attack against a known account was practical.

Fix

New LoginThrottle (in-memory, internal/auth/login_throttle.go) with two mechanisms:

  • Per-IP limit: 10 attempts per minute per client IP (every attempt counts, success or failure).
  • Per-account lockout: 15 minutes after 5 consecutive failures for the same (normalised) email. A successful login resets the counter.

Integration in the login handler:

  • Allow() is checked before the bcrypt comparison.
  • On lockout, the handler still runs a bcrypt dummy comparison before returning 429, so response timing does not reveal whether an account is locked.
  • The 429 message is generic (too many login attempts, try again later) - it does not disclose whether the IP limit or the account lockout fired, nor whether the account exists.
  • The existing dummy-hash timing protection against user enumeration is preserved.

Memory bounds

Eviction is hooked into the existing 5-minute ticker (no new goroutine). It drops expired IP windows and idle account entries (no active lockout, last failure older than 30 minutes), so spraying random emails cannot grow the maps unbounded.

Known tradeoffs (accepted for this iteration)

  • Account-lockout DoS: an attacker can lock a known account for 15 min with 5 failed attempts - the standard tradeoff of account lockout. State is per-process, so a restart clears it.
  • TOCTOU under concurrency: a few attempts may exceed the threshold under heavy parallel load; bounded in practice by the per-IP limit and bcrypt cost.
  • Reverse-proxy deployments: c.IP() uses the TCP peer IP (no X-Forwarded-For trust configured), so behind a proxy without trusted_proxies set, all clients share one IP bucket. This is a pre-existing, deployment-wide concern (also affects audit logs) and should be documented in the deployment guide.
  • In-memory / single-instance: consistent with the documented single-instance constraint; distributed throttling will come with Redis.

Tests

  • 8 unit tests (IP limit, window rollover, lockout set/expiry, success reset, email normalisation, idle eviction, concurrency smoke) using an injected clock.
  • 4 handler integration tests (lockout after 5 failures, correct password still throttled while locked, success resets, nil-throttle no-op).
  • go test -race ./internal/auth/ ./internal/api/... clean.

POST /api/v1/auth/login had no rate limiting or lockout, so a wordlist
attack against a known account was practical. LoginThrottle adds two
in-memory mechanisms: a per-IP limit (10 attempts/minute) and a
per-account lockout (15 minutes after 5 consecutive failures). A locked
account also burns a bcrypt comparison before returning 429 so the
response timing does not reveal the lockout state.

Eviction runs on the existing 5-minute ticker and also drops idle
account entries (no lockout, last failure older than 30 minutes) so
spraying random emails cannot grow the maps unbounded.

In-memory state is per-process and does not survive restarts, matching
the documented single-instance constraint; distributed throttling will
follow with Redis support.
@snyk-io

snyk-io Bot commented Jun 10, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.27451% with 14 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/auth/login_throttle.go 90.47% 5 Missing and 3 partials ⚠️
internal/app/app.go 0.00% 4 Missing ⚠️
internal/api/admin/auth.go 85.71% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant