diff --git a/crates/common/src/config.rs b/crates/common/src/config.rs index 71a283c..283348d 100644 --- a/crates/common/src/config.rs +++ b/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 port: u16, - pub request_timeout: u64, - pub max_body_size: usize, - pub api_key: Option, - pub allowed_origins: Vec, - pub cors_permissive: bool, - pub rate_limit_rps: Option, - pub rate_limit_burst: Option, + pub host: String, + pub port: u16, + pub request_timeout: u64, + pub max_body_size: usize, + pub api_key: Option, + pub allowed_origins: Vec, + pub cors_permissive: bool, + pub rate_limit_rps: Option, + pub rate_limit_burst: Option, /// Allowed URL schemes for repository URLs. Insecure schemes emit a warning /// on startup - pub allowed_url_schemes: Vec, + pub allowed_url_schemes: Vec, + /// 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(), - port: 3000, - request_timeout: 30, - max_body_size: 10 * 1024 * 1024, // 10MB - api_key: None, - allowed_origins: Vec::new(), - cors_permissive: false, - rate_limit_rps: None, - rate_limit_burst: None, - allowed_url_schemes: vec![ + host: "127.0.0.1".to_string(), + port: 3000, + request_timeout: 30, + max_body_size: 10 * 1024 * 1024, // 10MB + api_key: None, + allowed_origins: Vec::new(), + cors_permissive: false, + rate_limit_rps: None, + rate_limit_burst: None, + allowed_url_schemes: vec![ "https".into(), "http".into(), "git".into(), "ssh".into(), ], + force_secure_cookies: false, } } } diff --git a/crates/server/src/routes/dashboard.rs b/crates/server/src/routes/dashboard.rs index a2de337..7971fef 100644 --- a/crates/server/src/routes/dashboard.rs +++ b/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 - && state.config.server.host != "127.0.0.1" - && state.config.server.host != "localhost" - { - "; Secure" - } else { - "" - }; + let security_flags = + crate::routes::cookie_security_flags(&state.config.server); let cookie = format!( - "fc_user_session={session_id}; HttpOnly; SameSite=Strict; Path=/; \ - Max-Age=86400{secure_flag}" + "fc_user_session={session_id}; {security_flags}; Path=/; Max-Age=86400" ); 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 - && state.config.server.host != "127.0.0.1" - && state.config.server.host != "localhost" - { - "; Secure" - } else { - "" - }; + let security_flags = + crate::routes::cookie_security_flags(&state.config.server); let cookie = format!( - "fc_session={session_id}; HttpOnly; SameSite=Strict; Path=/; \ - Max-Age=86400{secure_flag}" + "fc_session={session_id}; {security_flags}; Path=/; Max-Age=86400" ); ( [(axum::http::header::SET_COOKIE, cookie)], diff --git a/crates/server/src/routes/mod.rs b/crates/server/src/routes/mod.rs index 298da90..0f70437 100644 --- a/crates/server/src/routes/mod.rs +++ b/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>, + 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), }); diff --git a/crates/server/src/routes/oauth.rs b/crates/server/src/routes/oauth.rs index f2d4467..9514aa3 100644 --- a/crates/server/src/routes/oauth.rs +++ b/crates/server/src/routes/oauth.rs @@ -105,16 +105,26 @@ async fn github_login(State(state): State) -> impl IntoResponse { .url(); // Store CSRF token in a cookie for verification - // Add Secure flag when using HTTPS (detected via redirect_uri) - let secure_flag = if config.redirect_uri.starts_with("https://") { - "; Secure" - } else { - "" + // Use SameSite=Lax for OAuth flow (must work across redirect) + let security_flags = { + let is_localhost = config.redirect_uri.starts_with("http://localhost") + || 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) - let secure_flag = if config.redirect_uri.starts_with("https://") { - "; Secure" - } else { - "" + // Use SameSite=Lax for OAuth callback (must work across redirect) + let security_flags = { + let is_localhost = config.redirect_uri.starts_with("http://localhost") + || 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 - secure_flag + security_flags, + 7 * 24 * 60 * 60 // 7 days ); Ok(