Sep 16 - Optimize code, part 1

advanced_security_middleware.py

@@ -37,7 +37,8 @@ class SecurityManager:
         self.session_tokens = {}
         
         # Security configuration
-        self.max_failed_attempts = 5
+        self.max_failed_attempts = 5               # per IP + username, 15 minutes
+        self.max_failed_attempts_per_username = 20  # per username from any IP, 15 minutes
         self.lockout_duration = 900  # 15 minutes
         self.session_timeout = 3600  # 1 hour
         
@@ -117,12 +118,13 @@ class SecurityManager:
     
     def get_client_ip(self):
         """Get real client IP address"""
-        # Check for forwarded headers (in case behind proxy/CDN)
-        forwarded_ips = request.headers.getlist('X-Forwarded-For')
-        if forwarded_ips:
-            return forwarded_ips[0].split(',')[0].strip()
+        # request.remote_addr is the real client address: app.py wraps the app in
+        # ProxyFix(x_for=TRUSTED_PROXY_COUNT), which takes it from the entry our
+        # own proxy (Nginx) appended to X-Forwarded-For. The FIRST entry of that
+        # header — used here before — is whatever the client chose to send, so
+        # changing it on every request bypassed the login rate limit.
         
-        return request.headers.get('X-Real-IP') or request.remote_addr
+        return request.remote_addr or 'unknown'
     
     def is_suspicious_request(self):
         """Detect suspicious request patterns"""
@@ -235,30 +237,52 @@ class SecurityManager:
         
         return False
     
-    def is_auth_rate_limited(self):
-        """Check if authentication endpoint is rate limited"""
+    def _login_attempt_keys(self, username=None):
+        """Failed-login counter keys: this IP + username, and the username alone."""
         client_ip = self.get_client_ip()
+        name = (username or '').strip().lower()
+        if not name:
+            return [f"ip:{client_ip}"]
+        return [f"ip:{client_ip}|user:{name}", f"user:{name}"]
+
+    def is_auth_rate_limited(self, username=None):
+        """
+        Should this login attempt be blocked?
+
+        Two counters over lockout_duration (15 minutes):
+          - this IP + username: blocked after max_failed_attempts (5)
+          - this username from ANY IP: blocked after max_failed_attempts_per_username
+            (20), so rotating IP addresses cannot brute-force one account
+        Deliberately not a plain per-IP counter: users behind one shared address
+        (office NAT, a proxy that hides client IPs) must not lock each other out.
+        Counters are in memory per gunicorn worker.
+        """
         current_time = time.time()
-        
-        # Clean old attempts
-        self.failed_attempts[client_ip] = deque([
-            attempt for attempt in self.failed_attempts[client_ip]
-            if current_time - attempt < 900  # Keep attempts from last 15 minutes
-        ], maxlen=10)
-        
-        return len(self.failed_attempts[client_ip]) >= self.max_failed_attempts
-    
+        for key in self._login_attempt_keys(username):
+            self.failed_attempts[key] = deque([
+                attempt for attempt in self.failed_attempts.get(key, ())
+                if current_time - attempt < self.lockout_duration
+            ], maxlen=50)
+            limit = (self.max_failed_attempts_per_username if key.startswith('user:')
+                     else self.max_failed_attempts)
+            if len(self.failed_attempts[key]) >= limit:
+                return True
+        return False
+
     def record_failed_attempt(self, identifier):
-        """Record a failed authentication attempt"""
+        """Record a failed authentication attempt (identifier = the submitted username)"""
         client_ip = self.get_client_ip()
         current_time = time.time()
-        
-        self.failed_attempts[client_ip].append(current_time)
-        
+        keys = self._login_attempt_keys(identifier)
+        for key in keys:
+            if key not in self.failed_attempts:
+                self.failed_attempts[key] = deque(maxlen=50)
+            self.failed_attempts[key].append(current_time)
+
         self.log_security_event('authentication_failure', {
             'ip': client_ip,
             'identifier': identifier,
-            'attempts': len(self.failed_attempts[client_ip])
+            'attempts': len(self.failed_attempts[keys[0]])
         })
     
     def create_secure_session(self, user_id):
@@ -278,10 +302,10 @@ class SecurityManager:
         session['security_token'] = session_token
         session['login_time'] = datetime.utcnow().isoformat()
         
-        # Clear any failed attempts for this IP
+        # Clear the failed-login counters for this IP + username and the username
         client_ip = self.get_client_ip()
-        if client_ip in self.failed_attempts:
-            del self.failed_attempts[client_ip]
+        for key in self._login_attempt_keys(session.get('username')) + [f"ip:{client_ip}"]:
+            self.failed_attempts.pop(key, None)
         
         self.log_security_event('secure_session_created', {
             'user_id': user_id,