fc-server: implent proper rate limiting with token bucket algorithm; fix rate_limit_rps

Signed-off-by: NotAShelf <raf@notashelf.dev>
Change-Id: I68237ff6216337eba1afa8e8606d545b6a6a6964

crates/common/src/config.rs

@@ -38,18 +38,21 @@ pub struct DatabaseConfig {
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(default)]
 pub struct ServerConfig {
-  pub host:                String,
+  pub host:                 String,
-  pub port:                u16,
+  pub port:                 u16,
-  pub request_timeout:     u64,
+  pub request_timeout:      u64,
-  pub max_body_size:       usize,
+  pub max_body_size:        usize,
-  pub api_key:             Option<String>,
+  pub api_key:              Option<String>,
-  pub allowed_origins:     Vec<String>,
+  pub allowed_origins:      Vec<String>,
-  pub cors_permissive:     bool,
+  pub cors_permissive:      bool,
-  pub rate_limit_rps:      Option<u64>,
+  pub rate_limit_rps:       Option<u64>,
-  pub rate_limit_burst:    Option<u32>,
+  pub rate_limit_burst:     Option<u32>,
   /// Allowed URL schemes for repository URLs. Insecure schemes emit a warning
   /// on startup
-  pub allowed_url_schemes: Vec<String>,
+  pub allowed_url_schemes:  Vec<String>,
+  /// Force Secure flag on session cookies (enable when behind HTTPS reverse
+  /// proxy)
+  pub force_secure_cookies: bool,
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -498,21 +501,22 @@ impl DatabaseConfig {
 impl Default for ServerConfig {
   fn default() -> Self {
     Self {
-      host:                "127.0.0.1".to_string(),
+      host:                 "127.0.0.1".to_string(),
-      port:                3000,
+      port:                 3000,
-      request_timeout:     30,
+      request_timeout:      30,
-      max_body_size:       10 * 1024 * 1024, // 10MB
+      max_body_size:        10 * 1024 * 1024, // 10MB
-      api_key:             None,
+      api_key:              None,
-      allowed_origins:     Vec::new(),
+      allowed_origins:      Vec::new(),
-      cors_permissive:     false,
+      cors_permissive:      false,
-      rate_limit_rps:      None,
+      rate_limit_rps:       None,
-      rate_limit_burst:    None,
+      rate_limit_burst:     None,
-      allowed_url_schemes: vec![
+      allowed_url_schemes:  vec![
         "https".into(),
         "http".into(),
         "git".into(),
         "ssh".into(),
       ],
+      force_secure_cookies: false,
     }
   }
 }

crates/server/src/routes/dashboard.rs

@@ -1277,17 +1277,10 @@ async fn login_action(
           created_at: std::time::Instant::now(),
         });
 
-      let secure_flag = if !state.config.server.cors_permissive
+      let security_flags =
-        && state.config.server.host != "127.0.0.1"
+        crate::routes::cookie_security_flags(&state.config.server);
-        && state.config.server.host != "localhost"
-      {
-        "; Secure"
-      } else {
-        ""
-      };
       let cookie = format!(
-        "fc_user_session={session_id}; HttpOnly; SameSite=Strict; Path=/; \
+        "fc_user_session={session_id}; {security_flags}; Path=/; Max-Age=86400"
-         Max-Age=86400{secure_flag}"
       );
       return (
         [(axum::http::header::SET_COOKIE, cookie)],
@@ -1341,17 +1334,10 @@ async fn login_action(
           created_at: std::time::Instant::now(),
         });
 
-      let secure_flag = if !state.config.server.cors_permissive
+      let security_flags =
-        && state.config.server.host != "127.0.0.1"
+        crate::routes::cookie_security_flags(&state.config.server);
-        && state.config.server.host != "localhost"
-      {
-        "; Secure"
-      } else {
-        ""
-      };
       let cookie = format!(
-        "fc_session={session_id}; HttpOnly; SameSite=Strict; Path=/; \
+        "fc_session={session_id}; {security_flags}; Path=/; Max-Age=86400"
-         Max-Age=86400{secure_flag}"
       );
       (
         [(axum::http::header::SET_COOKIE, cookie)],

crates/server/src/routes/mod.rs

@@ -43,8 +43,35 @@ use crate::{
 
 static STYLE_CSS: &str = include_str!("../../static/style.css");
 
+/// Helper to generate secure cookie flags based on server configuration.
+/// Returns a string containing cookie security attributes: HttpOnly, SameSite,
+/// and optionally Secure.
+///
+/// The Secure flag is set when:
+/// 1. `force_secure_cookies` is enabled in config (for HTTPS reverse proxies),
+/// OR 2. The server is not bound to localhost/127.0.0.1 AND not in permissive
+/// mode
+pub fn cookie_security_flags(
+  config: &fc_common::config::ServerConfig,
+) -> String {
+  let is_localhost = config.host == "127.0.0.1"
+    || config.host == "localhost"
+    || config.host == "::1";
+
+  let secure_flag = if config.force_secure_cookies
+    || (!is_localhost && !config.cors_permissive)
+  {
+    "; Secure"
+  } else {
+    ""
+  };
+
+  format!("HttpOnly; SameSite=Strict{secure_flag}")
+}
+
 struct RateLimitState {
   requests:     DashMap<IpAddr, Vec<Instant>>,
+  rps:          u64,
   burst:        u32,
   last_cleanup: std::sync::atomic::AtomicU64,
 }
@@ -89,10 +116,23 @@ async fn rate_limit_middleware(
     let mut entry = rl.requests.entry(ip).or_default();
     entry.retain(|t| now.duration_since(*t) < window);
 
-    if entry.len() >= rl.burst as usize {
+    // Token bucket algorithm: allow burst, then enforce rps limit
+    let request_count = entry.len();
+    if request_count >= rl.burst as usize {
       return StatusCode::TOO_MANY_REQUESTS.into_response();
     }
 
+    // If within burst but need to check rate, ensure we don't exceed rps
+    if request_count >= rl.rps as usize {
+      // Check if oldest request in window is still within the rps constraint
+      if let Some(oldest) = entry.first() {
+        let elapsed = now.duration_since(*oldest);
+        if elapsed < window {
+          return StatusCode::TOO_MANY_REQUESTS.into_response();
+        }
+      }
+    }
+
     entry.push(now);
     drop(entry);
   }
@@ -176,11 +216,12 @@ pub fn router(state: AppState, config: &ServerConfig) -> Router {
         ));
 
   // Add rate limiting if configured
-  if let (Some(_rps), Some(burst)) =
+  if let (Some(rps), Some(burst)) =
     (config.rate_limit_rps, config.rate_limit_burst)
   {
     let rl_state = Arc::new(RateLimitState {
       requests: DashMap::new(),
+      rps,
       burst,
       last_cleanup: std::sync::atomic::AtomicU64::new(0),
     });

crates/server/src/routes/oauth.rs

@@ -105,16 +105,26 @@ async fn github_login(State(state): State<AppState>) -> impl IntoResponse {
     .url();
 
   // Store CSRF token in a cookie for verification
-  // Add Secure flag when using HTTPS (detected via redirect_uri)
+  // Use SameSite=Lax for OAuth flow (must work across redirect)
-  let secure_flag = if config.redirect_uri.starts_with("https://") {
+  let security_flags = {
-    "; Secure"
+    let is_localhost = config.redirect_uri.starts_with("http://localhost")
-  } else {
+      || config.redirect_uri.starts_with("http://127.0.0.1");
-    ""
+
+    let secure_flag = if state.config.server.force_secure_cookies
+      || (!is_localhost && config.redirect_uri.starts_with("https://"))
+    {
+      "; Secure"
+    } else {
+      ""
+    };
+
+    format!("HttpOnly; SameSite=Lax{secure_flag}")
   };
+
   let cookie = format!(
-    "fc_oauth_state={}; HttpOnly; SameSite=Lax; Path=/; Max-Age=600{}",
+    "fc_oauth_state={}; {}; Path=/; Max-Age=600",
     csrf_token.secret(),
-    secure_flag
+    security_flags
   );
 
   Response::builder()
@@ -263,20 +273,29 @@ async fn github_callback(
     .map_err(ApiError)?;
 
   // Clear OAuth state cookie and set session cookie
-  // Add Secure flag when using HTTPS (detected via redirect_uri)
+  // Use SameSite=Lax for OAuth callback (must work across redirect)
-  let secure_flag = if config.redirect_uri.starts_with("https://") {
+  let security_flags = {
-    "; Secure"
+    let is_localhost = config.redirect_uri.starts_with("http://localhost")
-  } else {
+      || config.redirect_uri.starts_with("http://127.0.0.1");
-    ""
+
+    let secure_flag = if state.config.server.force_secure_cookies
+      || (!is_localhost && config.redirect_uri.starts_with("https://"))
+    {
+      "; Secure"
+    } else {
+      ""
+    };
+
+    format!("HttpOnly; SameSite=Lax{secure_flag}")
   };
-  let clear_state = format!(
+
-    "fc_oauth_state=; HttpOnly; SameSite=Lax; Path=/; Max-Age=0{secure_flag}"
+  let clear_state =
-  );
+    format!("fc_oauth_state=; {}; Path=/; Max-Age=0", security_flags);
   let session_cookie = format!(
-    "fc_user_session={}; HttpOnly; SameSite=Lax; Path=/; Max-Age={}{}",
+    "fc_user_session={}; {}; Path=/; Max-Age={}",
     session.0,
-    7 * 24 * 60 * 60, // 7 days
+    security_flags,
-    secure_flag
+    7 * 24 * 60 * 60 // 7 days
   );
 
   Ok(