Security/rate limiting proxy support#1478
Conversation
- Detects real IP from CF-Connecting-IP, X-Forwarded-For, X-Real-IP - Validates IP format to prevent header injection - Filters private IPs (configurable) - Generates rate limit keys with IP+Email combination Closes OpenSID#1410
- Add configurable trusted proxies via env variable - Support Cloudflare, reverse proxy, load balancer Closes OpenSID#1410
- Login rate limiter: 10 attempts per minute per IP+Email - OTP rate limiter: 3 attempts per minute per IP+Email - Use IpAddress helper for real IP detection Closes OpenSID#1410
- Add throttle:login to POST /login - Add throttle:login to 2FA verify-login - Add throttle:otp to OTP request/resend routes Closes OpenSID#1410
- TRUST_PROXIES for trusted proxy configuration - RATE_LIMIT_LOGIN_MAX and _DECAY - RATE_LIMIT_OTP_MAX and _DECAY Closes OpenSID#1410
- IP detection tests (8 tests) - Rate limit key generation tests (3 tests) - Login rate limiting tests (3 tests) - OTP rate limiting tests (2 tests) - Security validation tests (2 tests) - Priority order tests (2 tests) Total: 20 tests Closes OpenSID#1410
|
Banyak sekali test yang gagal
Bandingkan dengan test ketika rilis v2603 https://github.com/OpenSID/OpenDK/actions/runs/22567803576/job/65367793792
|
|
mas @Revanza1106 mohon di perbaiki biar bisa di gabung . |
Baik mas,maaf baru bisa handle sekarang 🙏 |
🔒 Security ReviewTotal Temuan: 5 isu (2 Critical, 2 High, 1 Medium)
|
|
📍 [CRITICAL] 🔒 Security: Dangerous Default Configuration - Trust All Proxies Masalah: File Kode: TRUST_PROXIES=*Risiko:
Cara Reproduksi: # 1. Developer deploy aplikasi baru dengan .env dari .env.example
# 2. TRUST_PROXIES=* masuk ke production
# 3. Attacker exploit:
# Bypass rate limiting dengan rotating fake IPs
for i in {1..1000}; do
curl -X POST https://newapp.com/login \
-H "X-Forwarded-For: 10.0.$((i/256)).$((i%256))" \
-d "email=admin@newapp.com&password=attempt$i"
done
# Semua 1000 request dianggap dari IP berbeda
# Rate limiting 10 attempts/minute menjadi tidak berguna
# Frame user lain untuk audit log
curl -X POST https://newapp.com/api/delete-data \
-H "X-Forwarded-For: 192.168.1.100" \
-H "Authorization: Bearer attacker_token" \
-d "id=sensitive_data"
# Log akan menunjukkan IP 192.168.1.100 (IP admin) yang melakukan deleteFix: # .env.example
# ⚠️ SECURITY WARNING: JANGAN gunakan TRUST_PROXIES=*
# Hanya trust IP proxy yang benar-benar Anda kontrol
#
# Untuk Cloudflare, gunakan IP ranges resmi:
# https://www.cloudflare.com/ips/
#
# Contoh untuk Cloudflare + Load Balancer internal:
# TRUST_PROXIES=173.245.48.0/20,103.21.244.0/22,10.0.1.5
#
# Untuk development lokal (HANYA untuk dev, JANGAN production):
# TRUST_PROXIES=127.0.0.1,::1
#
# Untuk production tanpa proxy (recommended jika tidak pakai CDN):
TRUST_PROXIES=
# Rate Limiting Configuration
RATE_LIMIT_LOGIN_MAX=10
RATE_LIMIT_LOGIN_DECAY=1
RATE_LIMIT_OTP_MAX=3
RATE_LIMIT_OTP_DECAY=1Tambahan - Dokumentasi di README: ## Security Configuration
### Trusted Proxies
⚠️ **CRITICAL**: Never use `TRUST_PROXIES=*` in production!
If you're behind Cloudflare:
1. Get current Cloudflare IP ranges: https://www.cloudflare.com/ips/
2. Set in .env: `TRUST_PROXIES=173.245.48.0/20,103.21.244.0/22,...`
If you're NOT behind a proxy/CDN:
- Leave empty: `TRUST_PROXIES=`
- Or remove the line entirely
If you're behind a load balancer:
- Only trust your load balancer's IP: `TRUST_PROXIES=10.0.1.5` |
|
📍 [HIGH] 🔒 Security: Mass Assignment Risk - Unvalidated Email Input in Rate Limit Key Masalah: Method Kode: // Line 52-54
$email = $request->input('email')
?? $request->input('username')
?? $request->input('identity');
$key = IpAddress::getRateLimitKey($request, $email);Risiko:
Cara Reproduksi: # Scenario 1: DoS dengan string panjang
curl -X POST https://target.com/login \
-H "Content-Type: application/json" \
-d "{\"email\":\"$(python3 -c 'print("a"*10000)')@example.com\",\"password\":\"test\"}"
# Scenario 2: Cache key collision attack
# Kirim banyak request dengan email yang berbeda sedikit
for i in {1..1000}; do
curl -X POST https://target.com/login \
-d "email=user$i@example.com&password=test" &
done
# Jika cache tidak handle dengan baik, bisa memory exhaustion
# Scenario 3: Bypass rate limiting dengan special characters
curl -X POST https://target.com/login \
-d "email=test@example.com%00admin&password=test"
curl -X POST https://target.com/login \
-d "email=test@example.com%0aadmin&password=test"
# Scenario 4: Array injection (jika tidak di-handle)
curl -X POST https://target.com/login \
-H "Content-Type: application/json" \
-d "{\"email\":[\"user1@example.com\",\"user2@example.com\"],\"password\":\"test\"}"
# Scenario 5: Null byte injection
curl -X POST https://target.com/login \
-d "email=attacker@evil.com%00victim@example.com&password=test"Fix: <?php
namespace App\Providers;
use Illuminate\Cache\RateLimiting\Limit;
use Illuminate\Foundation\Support\Providers\RouteServiceProvider as ServiceProvider;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\RateLimiter;
use Illuminate\Support\Facades\Route;
use App\Helpers\IpAddress;
class RouteServiceProvider extends ServiceProvider
{
public const HOME = '/home';
public function boot(): void
{
$this->configureRateLimiting();
$this->routes(function () {
Route::middleware('api')
->prefix('api')
->group(base_path('routes/api.php'));
Route::middleware('web')
->group(base_path('routes/web.php'));
});
}
protected function configureRateLimiting(): void
{
RateLimiter::for('api', function (Request $request) {
return Limit::perMinute(60)
->by($request->user()?->id ?: IpAddress::getRealIp($request));
});
RateLimiter::for('login', function (Request $request) {
// Validasi dan sanitasi email dengan ketat
$email = $this->extractAndValidateEmail($request);
$key = IpAddress::getRateLimitKey($request, $email);
$maxAttempts = (int) env('RATE_LIMIT_LOGIN_MAX', 10);
$decayMinutes = (int) env('RATE_LIMIT_LOGIN_DECAY', 1);
return Limit::perMinutes($decayMinutes, $maxAttempts)
->by($key)
->response(function () {
return response()->json([
'message' => 'Terlalu banyak percobaan login. Silakan coba lagi dalam beberapa menit.',
'error' => 'too_many_attempts',
], 429);
});
});
RateLimiter::for('otp', function (Request $request) {
$email = $this->extractAndValidateEmail($request);
$key = IpAddress::getRateLimitKey($request, $email);
$maxAttempts = (int) env('RATE_LIMIT_OTP_MAX', 3);
$decayMinutes = (int) env('RATE_LIMIT_OTP_DECAY', 1);
return Limit::perMinutes($decayMinutes, $maxAttempts)
->by($key)
->response(function () {
return response()->json([
'message' => 'Terlalu banyak permintaan OTP. Silakan coba lagi dalam beberapa menit.',
'error' => 'too_many_otp_attempts',
], 429);
});
});
}
/**
* Extract dan validasi email dari request dengan ketat
*/
private function extractAndValidateEmail(Request $request): ?string
{
// Ambil email dari berbagai field
$email = $request->input('email')
?? $request->input('username')
?? $request->input('identity');
// Return null jika tidak ada
if (empty($email)) {
return null;
}
// PENTING: Validasi tipe data - harus string
if (!is_string($email)) {
\Log::warning('Non-string email in rate limiter', [
'type' => gettype($email),
'value' => $email
]);
return null;
}
// Batasi panjang maksimal (RFC 5321: 320 characters)
if (strlen($email) > 320) {
\Log::warning('Email too long in rate limiter', [
'length' => strlen($email)
]);
return substr($email, 0, 320);
}
// Sanitasi: hapus null bytes dan control characters
$email = preg_replace('/[\x00-\x1F\x7F]/', '', $email);
// Validasi format email (basic)
// Tidak perlu validasi ketat karena ini hanya untuk rate limiting
// Tapi pastikan tidak ada karakter berbahaya
$email = filter_var($email, FILTER_SANITIZE_EMAIL);
// Trim whitespace
$email = trim($email);
return $email ?: null;
}
} |
⚡ Performance ReviewTotal Temuan: 0 isu (0 Critical, 0 High) Status: ✅ TIDAK ADA ISU HIGH/CRITICAL DITEMUKAN Analisis menyeluruh terhadap 6 file yang berubah tidak menemukan masalah performa HIGH atau CRITICAL. PR ini fokus pada security enhancement dengan implementasi yang sudah optimal:
Catatan: |
📝 Code Quality ReviewTotal Temuan: 5 isu (0 Critical, 5 High)
|
|
📍 [HIGH] 📝 Code Quality: Missing Input Validation Kategori: Architecture Kode: $email = $request->input('email')
?? $request->input('username')
?? $request->input('identity');
$key = IpAddress::getRateLimitKey($request, $email);Fix: Tambahkan validasi format email sebelum digunakan: $email = $request->input('email')
?? $request->input('username')
?? $request->input('identity');
// Validate email format if provided
if ($email && !filter_var($email, FILTER_VALIDATE_EMAIL)) {
// Use only IP for invalid email format
$email = null;
}
$key = IpAddress::getRateLimitKey($request, $email);Atau lebih baik, buat method helper: private function getValidatedIdentifier(Request $request): ?string
{
$identifier = $request->input('email')
?? $request->input('username')
?? $request->input('identity');
if (!$identifier) {
return null;
}
// Validate email format
if (filter_var($identifier, FILTER_VALIDATE_EMAIL)) {
return $identifier;
}
// Validate username format (alphanumeric + underscore/dash)
if (preg_match('/^[a-zA-Z0-9_-]{3,50}$/', $identifier)) {
return $identifier;
}
return null;
} |
|
📍 [HIGH] 📝 Code Quality: Inconsistent Middleware Application Kategori: Architecture Kode: Route::post('/login', 'LoginController@login')
->middleware('throttle:login')
->name('login.post');
// Auth::routes() lainnya tidak di-overrideFix: Override semua authentication routes yang perlu rate limiting: // Override login dengan rate limiting
Route::post('/login', 'LoginController@login')
->middleware('throttle:login')
->name('login.post');
// Override register dengan rate limiting
Route::post('/register', 'RegisterController@register')
->middleware('throttle:login') // Gunakan limiter yang sama atau buat 'register' limiter
->name('register.post');
// Override password reset dengan rate limiting
Route::post('/password/email', 'ForgotPasswordController@sendResetLinkEmail')
->middleware('throttle:otp') // Lebih strict untuk password reset
->name('password.email');
Route::post('/password/reset', 'ResetPasswordController@reset')
->middleware('throttle:login')
->name('password.update');Atau lebih baik, buat rate limiter group: Route::middleware(['guest', 'throttle:auth'])->group(function () {
Route::post('/login', 'LoginController@login')->name('login.post');
Route::post('/register', 'RegisterController@register')->name('register.post');
Route::post('/password/email', 'ForgotPasswordController@sendResetLinkEmail')->name('password.email');
Route::post('/password/reset', 'ResetPasswordController@reset')->name('password.update');
});Dan tambahkan limiter 'auth' di RateLimiter::for('auth', function (Request $request) {
$identifier = $request->input('email')
?? $request->input('username')
?? $request->input('identity');
$key = IpAddress::getRateLimitKey($request, $identifier);
return Limit::perMinute(env('RATE_LIMIT_AUTH_MAX', 10))
->by($key)
->response(function () {
return response()->json([
'message' => 'Terlalu banyak percobaan. Silakan coba lagi nanti.',
'error' => 'too_many_attempts',
], 429);
});
}); |
🐛 Bug Detection ReviewTotal Temuan: 6 isu (2 Critical, 4 High)
|
🤖 AI Code Review — Selesai📋 Ringkasan Semua Review
Total inline comments: 11 |
…D#1478 - CRITICAL: Remove dangerous TRUST_PROXIES=* default, secure by default (null) - CRITICAL: Create IpAddress helper with safe proxy IP detection - Proper IPv6 port handling ([::1]:8080 -> ::1) - preg_replace null return handling - Header injection prevention (newline sanitization) - Return type hints for all methods - HIGH: Use IpAddress helper in RouteServiceProvider for rate limiting - HIGH: Add null check for preg_replace in resolveRateLimitIdentifier - HIGH: Remove unused makeRateLimitKey method - Update .env.example with secure default and warning about IP spoofing Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…#1478 CRITICAL fixes: - TrustProxies: reject TRUST_PROXIES=* from env, return null instead - TrustProxies: validate IP/CIDR format before trusting proxies - IpAddress: fix IPv6 port removal logic (handle bracketed format) - IpAddress: fix preg_replace null return handling in cleanIpAddress - IpAddress: add null check before isValidIpAddress call HIGH fixes: - IpAddress: prevent HTTP header injection via control character removal - IpAddress: add sanitizeIdentifier method for rate limit key safety - IpAddress: add return type hints to all public methods - RouteServiceProvider: add extractAndValidateIdentifier with type checking - RouteServiceProvider: validate identifier is string, max 320 chars - RouteServiceProvider: remove null bytes and control characters from input MEDIUM fixes: - IpAddress: proper IPv6 validation before port stripping - IpAddress: handle IPv4-mapped IPv6 addresses correctly Code quality: - Refactor IpAddress into smaller single-responsibility methods - Add parseFirstIp helper for X-Forwarded-For parsing - Update .env.example with secure defaults and spoofing warnings Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Halo mas @pandigresik dan @vickyrolanda , Saya sudah push fix untuk semua temuan dari security & code review. Berikut ringkasan perubahan: CRITICAL
HIGH
MEDIUM
Code Quality
Semua file sudah pass PHP lint dan Laravel Pint style check. Terkait CI
|


Deskripsi
Menambahkan rate limiting pada endpoint login dan OTP untuk mencegah brute force attack. Implementasi mendukung aplikasi yang berada di belakang Cloudflare Proxy atau Reverse Proxy dengan deteksi IP asli dari header proxy.
Fitur Utama
CF-Connecting-IP,X-Forwarded-For,X-Real-IPPerubahan File
app/Helpers/IpAddress.php(baru) - Helper deteksi IP proxyapp/Http/Middleware/TrustProxies.php- Konfigurasi trusted proxyapp/Providers/RouteServiceProvider.php- Rate limiter login/OTProutes/web.php- Apply throttle middleware.env.example- Konfigurasi env variablestests/Feature/RateLimitingTest.php- Test suite (20 test cases)Masalah Terkait (Related Issue)
Langkah untuk mereproduksi (Steps to Reproduce)
Sebelum Perubahan (Bug Reproduction)
/loginSetelah Perubahan (Verification)
/loginCF-Connecting-IP: IP terdeteksi dengan benarManual Test Command
Hasil Test
Automated Test (Pest)
Test Coverage Summary
Daftar Periksa (Checklist)
Tangkapan Layar (Screenshot)
Test Results
Konfigurasi Environment
Tambahkan ke
.env:Commit Breakdown