feat: add authentication system and rate limiting for enhanced security#584
feat: add authentication system and rate limiting for enhanced security#584shbhmexe wants to merge 1 commit intoAOSSIE-Org:mainfrom
Conversation
- Implement JWT token and API key authentication - Add rate limiting middleware (60 req/min default) - Fix CORS vulnerability with restricted origins - Add security headers (X-Frame-Options, CSP, HSTS) - Optimize ProcessPoolExecutor worker count - Create comprehensive security documentation - Add authentication tests BREAKING CHANGE: CORS now requires configured origins in .env
|
|
WalkthroughThe pull request introduces comprehensive authentication infrastructure with JWT and API key support, implements rate limiting and security headers, transitions configuration to environment-based loading, and refines CORS policies across both the main backend and sync-microservice components. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Backend
participant Auth Middleware
rect rgba(100, 150, 200, 0.2)
Note over Client,Auth Middleware: API Key Authentication Flow
Client->>Backend: POST /token<br/>{client_id, api_key}
Backend->>Auth Middleware: verify_api_key(x_api_key)
Auth Middleware-->>Backend: ✓ Valid
Backend->>Auth Middleware: create_access_token(data)
Auth Middleware-->>Backend: JWT Token
Backend-->>Client: {access_token, token_type, expires_in}
end
rect rgba(150, 150, 100, 0.2)
Note over Client,Auth Middleware: JWT Authentication Flow
Client->>Backend: GET /protected<br/>Authorization: Bearer {token}
Backend->>Auth Middleware: get_current_user(credentials)
Auth Middleware->>Auth Middleware: verify_token(token)
Auth Middleware-->>Backend: User Payload
Backend-->>Client: Protected Resource
end
rect rgba(100, 150, 100, 0.2)
Note over Client,Auth Middleware: Status Check Flow
Client->>Backend: GET /status<br/>X-API-Key: {key}
Backend->>Auth Middleware: verify_api_key(x_api_key)
Auth Middleware-->>Backend: ✓ Valid
Backend-->>Client: {authenticated: true, auth_method: api_key}
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes introduce a complete authentication system spanning middleware, routes, configuration, and integration. While mostly new code with clear separation of concerns, the JWT/API key logic requires verification of token creation/verification correctness, CORS integration validation, and proper error handling. Test coverage mitigates risk. Scope spreads across 8 files with moderate heterogeneity. Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
backend/requirements.txt (1)
11-12: Configure a shared backend for rate limiting in productionslowapi’s default in-memory storage won’t enforce limits across multiple workers/instances. Use a shared backend (e.g., Redis via limits[redis]) and configure Limiter(storage_uri="redis://...") for consistent enforcement.
Based on learnings
sync-microservice/main.py (1)
9-11: Trim and sanitize ALLOWED_ORIGINSStrip whitespace and ignore empty entries to prevent silent CORS mismatches when env has spaces.
Apply:
-ALLOWED_ORIGINS = os.getenv( - "ALLOWED_ORIGINS", "http://localhost:8000,http://localhost:1420,tauri://localhost" -).split(",") +ALLOWED_ORIGINS = [ + o.strip() + for o in os.getenv( + "ALLOWED_ORIGINS", + "http://localhost:8000,http://localhost:1420,tauri://localhost", + ).split(",") + if o.strip() +]backend/app/config/settings.py (1)
14-17: Sanitize ALLOWED_ORIGINSTrim and ignore empties to avoid CORS mismatches.
-ALLOWED_ORIGINS = os.getenv( - "ALLOWED_ORIGINS", "http://localhost:1420,tauri://localhost" -).split(",") +ALLOWED_ORIGINS = [ + o.strip() + for o in os.getenv("ALLOWED_ORIGINS", "http://localhost:1420,tauri://localhost").split(",") + if o.strip() +]backend/tests/test_auth.py (1)
93-105: Make HSTS assertion environment-awareIf HSTS is later restricted to HTTPS/production (recommended), this test will fail locally. Consider asserting presence conditionally (e.g., only when APP_ENV=production or when scheme is https).
backend/main.py (1)
38-41: Plan for shared rate-limit storage in multi-worker deploymentsDefault in-memory limits won’t be shared across uvicorn/gunicorn workers or instances. Configure Limiter(storage_uri="redis://...") in production.
Based on learnings
backend/app/routes/auth.py (2)
11-13: Remove unused import or reuse centralized verifierverify_api_key is imported but unused. Either remove the import or reuse centralized validation for consistency.
-from app.middleware.auth import create_access_token, verify_api_key +from app.middleware.auth import create_access_token
59-66: Unify API key validation semanticsConsider reusing the centralized verify_api_key logic (adjusted for body vs header) to keep status codes/messages consistent across the app.
backend/app/middleware/auth.py (1)
19-38: Add iat claim for token hygieneInclude issued-at (iat) to aid debugging and validation windows; optionally add nbf if needed.
- to_encode.update({"exp": expire}) + to_encode.update({"exp": expire, "iat": datetime.utcnow()})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/app/config/settings.py(2 hunks)backend/app/middleware/__init__.py(1 hunks)backend/app/middleware/auth.py(1 hunks)backend/app/routes/auth.py(1 hunks)backend/main.py(5 hunks)backend/requirements.txt(1 hunks)backend/tests/test_auth.py(1 hunks)sync-microservice/main.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
backend/app/middleware/__init__.py (1)
backend/app/middleware/auth.py (5)
create_access_token(19-38)verify_token(41-62)verify_api_key(65-91)get_current_user(124-155)get_current_user_optional(94-121)
backend/tests/test_auth.py (2)
backend/app/middleware/auth.py (2)
create_access_token(19-38)verify_token(41-62)backend/tests/test_folders.py (1)
client(76-78)
backend/app/routes/auth.py (1)
backend/app/middleware/auth.py (2)
create_access_token(19-38)verify_api_key(65-91)
backend/app/config/settings.py (1)
frontend/src/config/Backend.ts (1)
SYNC_MICROSERVICE_URL(2-2)
🔇 Additional comments (2)
backend/app/middleware/__init__.py (1)
5-19: Re-exports look goodClear, minimal public surface; names align with auth.py.
sync-microservice/main.py (1)
24-28: No changes needed for CORS handling of Tauri origins
Starlette’s CORSMiddleware accepts custom-scheme origins via literal or regex matching, so including"tauri://localhost"inALLOWED_ORIGINSsuffices.
| SECRET_KEY = os.getenv("SECRET_KEY", "dev-secret-key-change-in-production") | ||
| API_KEY = os.getenv("API_KEY", "dev-api-key-change-in-production") | ||
| ALGORITHM = os.getenv("ALGORITHM", "HS256") | ||
| ACCESS_TOKEN_EXPIRE_MINUTES = int(os.getenv("ACCESS_TOKEN_EXPIRE_MINUTES", "30")) | ||
|
|
There was a problem hiding this comment.
Avoid shipping dev secrets in production
SECRET_KEY/API_KEY have permissive dev defaults. Enforce env presence in prod (fail-fast or warn) to prevent weak secrets in deployments.
Example:
+APP_ENV = os.getenv("APP_ENV", "development").lower()
SECRET_KEY = os.getenv("SECRET_KEY", "dev-secret-key-change-in-production")
API_KEY = os.getenv("API_KEY", "dev-api-key-change-in-production")
@@
-ACCESS_TOKEN_EXPIRE_MINUTES = int(os.getenv("ACCESS_TOKEN_EXPIRE_MINUTES", "30"))
+ACCESS_TOKEN_EXPIRE_MINUTES = int(os.getenv("ACCESS_TOKEN_EXPIRE_MINUTES", "30"))
+if APP_ENV == "production" and (
+ SECRET_KEY == "dev-secret-key-change-in-production"
+ or API_KEY == "dev-api-key-change-in-production"
+):
+ raise RuntimeError("SECRET_KEY/API_KEY must be set in production")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SECRET_KEY = os.getenv("SECRET_KEY", "dev-secret-key-change-in-production") | |
| API_KEY = os.getenv("API_KEY", "dev-api-key-change-in-production") | |
| ALGORITHM = os.getenv("ALGORITHM", "HS256") | |
| ACCESS_TOKEN_EXPIRE_MINUTES = int(os.getenv("ACCESS_TOKEN_EXPIRE_MINUTES", "30")) | |
| APP_ENV = os.getenv("APP_ENV", "development").lower() | |
| SECRET_KEY = os.getenv("SECRET_KEY", "dev-secret-key-change-in-production") | |
| API_KEY = os.getenv("API_KEY", "dev-api-key-change-in-production") | |
| ALGORITHM = os.getenv("ALGORITHM", "HS256") | |
| ACCESS_TOKEN_EXPIRE_MINUTES = int(os.getenv("ACCESS_TOKEN_EXPIRE_MINUTES", "30")) | |
| if APP_ENV == "production" and ( | |
| SECRET_KEY == "dev-secret-key-change-in-production" | |
| or API_KEY == "dev-api-key-change-in-production" | |
| ): | |
| raise RuntimeError("SECRET_KEY/API_KEY must be set in production") |
🤖 Prompt for AI Agents
In backend/app/config/settings.py around lines 8 to 12, SECRET_KEY and API_KEY
currently fall back to permissive dev defaults; remove or avoid silent defaults
and enforce environment presence in production by either reading them without a
default (os.environ["SECRET_KEY"], os.environ["API_KEY"]) or validating after
load and raising an error when running in production (e.g., when ENV/DEBUG
indicates prod). Implement a fail-fast behavior: if running in production and
either SECRET_KEY or API_KEY is missing or empty, raise a clear exception on
startup (or exit) with a message instructing to set the env var; for local/dev
runs keep acknowleged dev-only defaults or explicit debug branch that logs a
warning rather than silently using weak keys.
| # Add security headers middleware | ||
| @app.middleware("http") | ||
| async def add_security_headers(request: Request, call_next): | ||
| """Add security headers to all responses.""" | ||
| response = await call_next(request) | ||
| response.headers["X-Content-Type-Options"] = "nosniff" | ||
| response.headers["X-Frame-Options"] = "DENY" | ||
| response.headers["X-XSS-Protection"] = "1; mode=block" | ||
| response.headers["Strict-Transport-Security"] = "max-age=31536000; includeSubDomains" | ||
| return response |
There was a problem hiding this comment.
Gate HSTS to HTTPS/production; drop obsolete X-XSS-Protection
- Only set Strict-Transport-Security when request.url.scheme == "https" (and preferably behind a proxy) or APP_ENV=production.
- X-XSS-Protection is deprecated; consider removing it.
@app.middleware("http")
async def add_security_headers(request: Request, call_next):
"""Add security headers to all responses."""
response = await call_next(request)
response.headers["X-Content-Type-Options"] = "nosniff"
response.headers["X-Frame-Options"] = "DENY"
- response.headers["X-XSS-Protection"] = "1; mode=block"
- response.headers["Strict-Transport-Security"] = "max-age=31536000; includeSubDomains"
+ # Only set HSTS over HTTPS / in production
+ if request.url.scheme == "https" or os.getenv("APP_ENV", "development").lower() == "production":
+ response.headers["Strict-Transport-Security"] = "max-age=31536000; includeSubDomains"
return response📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Add security headers middleware | |
| @app.middleware("http") | |
| async def add_security_headers(request: Request, call_next): | |
| """Add security headers to all responses.""" | |
| response = await call_next(request) | |
| response.headers["X-Content-Type-Options"] = "nosniff" | |
| response.headers["X-Frame-Options"] = "DENY" | |
| response.headers["X-XSS-Protection"] = "1; mode=block" | |
| response.headers["Strict-Transport-Security"] = "max-age=31536000; includeSubDomains" | |
| return response | |
| # Add security headers middleware | |
| @app.middleware("http") | |
| async def add_security_headers(request: Request, call_next): | |
| """Add security headers to all responses.""" | |
| response = await call_next(request) | |
| response.headers["X-Content-Type-Options"] = "nosniff" | |
| response.headers["X-Frame-Options"] = "DENY" | |
| # Only set HSTS over HTTPS / in production | |
| if request.url.scheme == "https" or os.getenv("APP_ENV", "development").lower() == "production": | |
| response.headers["Strict-Transport-Security"] = "max-age=31536000; includeSubDomains" | |
| return response |
🤖 Prompt for AI Agents
In backend/main.py around lines 132 to 141, the middleware unconditionally sets
HSTS and the obsolete X-XSS-Protection header; update it to (1) remove
X-XSS-Protection entirely and (2) only add Strict-Transport-Security when the
request is actually over HTTPS or when running in production (e.g.,
os.getenv("APP_ENV") == "production"); implement the check using
request.url.scheme == "https" or an APP_ENV guard (and note that when behind a
proxy you must rely on your proxy config / trusted forwarded proto header), then
only set the HSTS header in that branch while leaving the other security headers
as-is.
|
@shbhmexe Looks great — really nice work tightening up the security side Quick question though — since the app mainly runs offline through Tauri (local backend + frontend), are things like API key/JWT auth and rate limiting still needed here? Just trying to understand the reasoning — CORS and the security headers make total sense, but the auth and throttling bits feel more relevant if we ever go online or add multi-user access. |
|
@shbhmexe |
3 participants
🔒 Security Enhancement: Authentication & Rate Limiting Implementation
🎯 Issue Addressed
Fixes critical security vulnerabilities identified in code review:
allow_origins=["*"])max_workers=1🚀 What Was Implemented
1. Authentication System ✨
API Key Authentication: Secure communication between Tauri app and backend
X-API-Key)JWT Token Authentication: Future-ready for web interfaces
POST /auth/token)python-joseFlexible Auth Middleware:
get_current_user()- Required authenticationget_current_user_optional()- Optional authentication2. Rate Limiting 🛡️
slowapifor request throttlingX-RateLimit-LimitX-RateLimit-RemainingX-RateLimit-Reset3. CORS Security 🔒
allow_origins=["*"](CRITICAL vulnerability).envhttp://localhost:1420, tauri://localhost4. Security Headers 🛡️
New middleware adds essential security headers to all responses:
X-Content-Type-Options: nosniff- Prevents MIME sniffingX-Frame-Options: DENY- Prevents clickjackingX-XSS-Protection: 1; mode=block- XSS protectionStrict-Transport-Security- Enforces HTTPS (production)5. Trusted Host Middleware ✅
Hostheaderlocalhost,127.0.0.1,0.0.0.06. Performance Optimization ⚡
max_workers=1(single-threaded)max(1, cpu_count - 1)(multi-threaded)7. Environment Configuration 🔧
Created comprehensive
.env.exampletemplate:8. New API Endpoints 🔌
POST /auth/token- Generate JWT access tokenGET /auth/status- Check authentication statusGET /health- Now returns version info📁 Files Modified
Backend
backend/main.py- Added auth, rate limiting, security middlewarebackend/requirements.txt- Addedpython-jose,slowapibackend/app/config/settings.py- Added security configurationsync-microservice/main.py- Fixed CORS configurationNew Files Created
backend/app/middleware/auth.py- Authentication logicbackend/app/middleware/__init__.py- Middleware packagebackend/app/routes/auth.py- Authentication endpointsbackend/.env.example- Environment templatebackend/SECURITY.md- Comprehensive security guidebackend/MIGRATION_GUIDE.md- Step-by-step migration instructionsbackend/tests/test_auth.py- Authentication test suite🔧 Technical Details
Dependencies Added