Skip to content

fix(security): resolve rate limit bypass for unauthenticated requests#209

Merged
DenizAltunkapan merged 4 commits intoVault-Web:mainfrom
aryankshl:fix/issue-206-registration-rate-limit
Apr 1, 2026
Merged

fix(security): resolve rate limit bypass for unauthenticated requests#209
DenizAltunkapan merged 4 commits intoVault-Web:mainfrom
aryankshl:fix/issue-206-registration-rate-limit

Conversation

@aryankshl
Copy link
Copy Markdown
Contributor

Summary

This PR fixes a critical logic flaw in the RateLimitAspect that allowed unauthenticated users (such as those on the /register and /login endpoints) to bypass rate limiting entirely.

  • Deterministic Key Generation: Refactored getRateLimitKey to consistently use the client's IP address for unauthenticated users.
  • Proxy-Aware IP Detection: Enhanced getClientIpAddress to check the X-Forwarded-For header, ensuring rate limiting works correctly behind Docker or Nginx proxies.
  • Fallback Logic: Guaranteed that even if IP detection fails, the key remains stable rather than randomized.

Linked issue

Closes #206

How to test

  • Start the environment: Run docker compose up -d and ./mvnw spring-boot:run
  • Execute a "Spam" Script: Run the following bash loop to simulate a registration burst:
for i in {1..10}; do 
  curl -s -o /dev/null -w "%{http_code}\n" -X POST http://localhost:8080/api/auth/register \
  -H "Content-Type: application/json" \
  -d '{"username":"test_user_'$i'", "password":"Password123!"}'
done 

Notes / Risk

  • Risk: Users sharing a single Public IP (like a large office or university) will share the same rate-limit bucket. However, given the current limit of 5 requests per minute for registration, this is a standard and acceptable security trade-off.
  • Dependency: Requires Bucket4j and Caffeine which are already present in the project.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens backend rate limiting by ensuring unauthenticated requests are deterministically bucketed (primarily by client IP), including better support for deployments behind proxies via X-Forwarded-For.

Changes:

  • Refactors rate-limit key generation to avoid randomized fallback keys for anonymous requests.
  • Adds X-Forwarded-For support in client IP detection to improve correctness behind reverse proxies.
  • Adds key prefixes (IP:, USER:, ANON:) to reduce key-space ambiguity.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@DenizAltunkapan
Copy link
Copy Markdown
Member

@aryankshl please fix the failing workflow

Copy link
Copy Markdown
Member

@DenizAltunkapan DenizAltunkapan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aryankshl Thank you for your contribution

@DenizAltunkapan DenizAltunkapan merged commit ecd38bf into Vault-Web:main Apr 1, 2026
2 checks passed
@aryankshl aryankshl deleted the fix/issue-206-registration-rate-limit branch April 1, 2026 11:26
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.

[Bug]: Missing rate limiting on registration endpoint

3 participants