From 92153bf9aa1b5a3fc09f7740d16efbd9430fd1c4 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Feb 2026 01:26:17 +0300 Subject: [PATCH] crates/server: enhance auth middleware and error responses Signed-off-by: NotAShelf Change-Id: I48a780779d884c4a7730347f920b91216a6a6964 --- crates/server/src/auth_middleware.rs | 79 ++++++++-------- crates/server/src/error.rs | 56 +++++++++-- crates/server/src/main.rs | 7 +- crates/server/src/routes/builds.rs | 39 +------- crates/server/src/routes/cache.rs | 8 +- crates/server/src/routes/dashboard.rs | 52 +++++++++- crates/server/src/routes/evaluations.rs | 121 +++++++++++++++++++++++- crates/server/src/routes/logs.rs | 5 +- 8 files changed, 270 insertions(+), 97 deletions(-) diff --git a/crates/server/src/auth_middleware.rs b/crates/server/src/auth_middleware.rs index 768c7bb..7c18708 100644 --- a/crates/server/src/auth_middleware.rs +++ b/crates/server/src/auth_middleware.rs @@ -9,8 +9,8 @@ use sha2::{Digest, Sha256}; use crate::state::AppState; -/// Extract and validate an API key from the Authorization header. -/// Keys use the format: `Bearer fc_xxxx`. +/// Extract and validate an API key from the Authorization header or session cookie. +/// Keys use the format: `Bearer fc_xxxx`. Session cookies use `fc_session=`. /// Write endpoints (POST/PUT/DELETE/PATCH) require a valid key. /// Read endpoints (GET/HEAD/OPTIONS) try to extract optionally (for dashboard admin UI). pub async fn require_api_key( @@ -33,42 +33,44 @@ pub async fn require_api_key( .as_deref() .and_then(|h| h.strip_prefix("Bearer ")); - match token { - Some(token) => { - let mut hasher = Sha256::new(); - hasher.update(token.as_bytes()); - let key_hash = hex::encode(hasher.finalize()); + // Try Bearer token first + if let Some(token) = token { + let mut hasher = Sha256::new(); + hasher.update(token.as_bytes()); + let key_hash = hex::encode(hasher.finalize()); - match fc_common::repo::api_keys::get_by_hash(&state.pool, &key_hash).await { - Ok(Some(api_key)) => { - // Touch last_used_at (fire and forget) - let pool = state.pool.clone(); - let key_id = api_key.id; - tokio::spawn(async move { - let _ = fc_common::repo::api_keys::touch_last_used(&pool, key_id).await; - }); + if let Ok(Some(api_key)) = + fc_common::repo::api_keys::get_by_hash(&state.pool, &key_hash).await + { + let pool = state.pool.clone(); + let key_id = api_key.id; + tokio::spawn(async move { + let _ = fc_common::repo::api_keys::touch_last_used(&pool, key_id).await; + }); - request.extensions_mut().insert(api_key); - Ok(next.run(request).await) - } - _ => { - if is_read { - // Invalid token on read is still allowed, just no ApiKey in extensions - Ok(next.run(request).await) - } else { - Err(StatusCode::UNAUTHORIZED) - } - } - } - } - None => { - if is_read { - Ok(next.run(request).await) - } else { - Err(StatusCode::UNAUTHORIZED) - } + request.extensions_mut().insert(api_key); + return Ok(next.run(request).await); } } + + // Fall back to session cookie (so dashboard JS fetches work) + if let Some(cookie_header) = request + .headers() + .get("cookie") + .and_then(|v| v.to_str().ok()) + && let Some(session_id) = parse_cookie(cookie_header, "fc_session") + && let Some(session) = state.sessions.get(&session_id) + && session.created_at.elapsed() < std::time::Duration::from_secs(24 * 60 * 60) { + request.extensions_mut().insert(session.api_key.clone()); + return Ok(next.run(request).await); + } + + // No valid auth found + if is_read { + Ok(next.run(request).await) + } else { + Err(StatusCode::UNAUTHORIZED) + } } /// Extractor that requires an authenticated admin user. @@ -129,9 +131,8 @@ pub async fn extract_session( .headers() .get("cookie") .and_then(|v| v.to_str().ok()) - { - if let Some(session_id) = parse_cookie(cookie_header, "fc_session") { - if let Some(session) = state.sessions.get(&session_id) { + && let Some(session_id) = parse_cookie(cookie_header, "fc_session") + && let Some(session) = state.sessions.get(&session_id) { // Check session expiry (24 hours) if session.created_at.elapsed() < std::time::Duration::from_secs(24 * 60 * 60) { request.extensions_mut().insert(session.api_key.clone()); @@ -141,12 +142,10 @@ pub async fn extract_session( state.sessions.remove(&session_id); } } - } - } next.run(request).await } -fn parse_cookie<'a>(header: &'a str, name: &str) -> Option { +fn parse_cookie(header: &str, name: &str) -> Option { header .split(';') .filter_map(|pair| { diff --git a/crates/server/src/error.rs b/crates/server/src/error.rs index d116764..6503a4f 100644 --- a/crates/server/src/error.rs +++ b/crates/server/src/error.rs @@ -22,18 +22,60 @@ impl IntoResponse for ApiError { CiError::Timeout(msg) => (StatusCode::REQUEST_TIMEOUT, "TIMEOUT", msg.clone()), CiError::Unauthorized(msg) => (StatusCode::UNAUTHORIZED, "UNAUTHORIZED", msg.clone()), CiError::Forbidden(msg) => (StatusCode::FORBIDDEN, "FORBIDDEN", msg.clone()), - CiError::Database(_) => ( - StatusCode::INTERNAL_SERVER_ERROR, - "DATABASE_ERROR", - "Internal database error".to_string(), + CiError::NixEval(msg) => ( + StatusCode::UNPROCESSABLE_ENTITY, + "NIX_EVAL_ERROR", + msg.clone(), ), - _ => ( + CiError::Build(msg) => (StatusCode::UNPROCESSABLE_ENTITY, "BUILD_ERROR", msg.clone()), + CiError::Config(msg) => ( StatusCode::INTERNAL_SERVER_ERROR, - "INTERNAL_ERROR", - "Internal server error".to_string(), + "CONFIG_ERROR", + msg.clone(), ), + CiError::Database(e) => { + tracing::error!(error = %e, "Database error in API handler"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + "DATABASE_ERROR", + "Internal database error".to_string(), + ) + } + CiError::Git(e) => { + tracing::error!(error = %e, "Git error in API handler"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + "GIT_ERROR", + format!("Git operation failed: {e}"), + ) + } + CiError::Serialization(e) => { + tracing::error!(error = %e, "Serialization error in API handler"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + "SERIALIZATION_ERROR", + format!("Data serialization error: {e}"), + ) + } + CiError::Io(e) => { + tracing::error!(error = %e, "IO error in API handler"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + "IO_ERROR", + format!("IO error: {e}"), + ) + } }; + if status.is_server_error() { + tracing::warn!( + status = %status, + code = code, + "API error response: {}", + message + ); + } + let body = axum::Json(json!({ "error": message, "error_code": code })); (status, body).into_response() } diff --git a/crates/server/src/main.rs b/crates/server/src/main.rs index e11e1c7..9a9bf24 100644 --- a/crates/server/src/main.rs +++ b/crates/server/src/main.rs @@ -45,16 +45,19 @@ async fn shutdown_signal() { #[tokio::main] async fn main() -> anyhow::Result<()> { - tracing_subscriber::fmt::init(); + let config = Config::load()?; + fc_common::init_tracing(&config.tracing); let cli = Cli::parse(); - let config = Config::load()?; let host = cli.host.unwrap_or(config.server.host.clone()); let port = cli.port.unwrap_or(config.server.port); let db = Database::new(config.database.clone()).await?; + // Bootstrap declarative projects, jobsets, and API keys from config + fc_common::bootstrap::run(db.pool(), &config.declarative).await?; + let state = AppState { pool: db.pool().clone(), config: config.clone(), diff --git a/crates/server/src/routes/builds.rs b/crates/server/src/routes/builds.rs index 3fa171d..26f3011 100644 --- a/crates/server/src/routes/builds.rs +++ b/crates/server/src/routes/builds.rs @@ -6,9 +6,7 @@ use axum::{ response::{IntoResponse, Response}, routing::{get, post}, }; -use fc_common::{ - Build, BuildProduct, BuildStatus, BuildStep, CreateBuild, PaginatedResponse, PaginationParams, -}; +use fc_common::{Build, BuildProduct, BuildStep, PaginatedResponse, PaginationParams}; use serde::Deserialize; use uuid::Uuid; @@ -155,44 +153,17 @@ async fn restart_build( Path(id): Path, ) -> Result, ApiError> { check_role(&extensions, &["restart-jobs"])?; - let original = fc_common::repo::builds::get(&state.pool, id) + let build = fc_common::repo::builds::restart(&state.pool, id) .await .map_err(ApiError)?; - // Can only restart completed or failed builds - if original.status != BuildStatus::Failed - && original.status != BuildStatus::Completed - && original.status != BuildStatus::Cancelled - { - return Err(ApiError(fc_common::CiError::Validation( - "Can only restart failed, completed, or cancelled builds".to_string(), - ))); - } - - // Create a new build with the same parameters - let new_build = fc_common::repo::builds::create( - &state.pool, - CreateBuild { - evaluation_id: original.evaluation_id, - job_name: original.job_name.clone(), - drv_path: original.drv_path.clone(), - system: original.system.clone(), - outputs: original.outputs.clone(), - is_aggregate: Some(original.is_aggregate), - constituents: original.constituents.clone(), - }, - ) - .await - .map_err(ApiError)?; - tracing::info!( - original_id = %id, - new_id = %new_build.id, - job = %original.job_name, + build_id = %id, + job = %build.job_name, "Build restarted" ); - Ok(Json(new_build)) + Ok(Json(build)) } async fn bump_build( diff --git a/crates/server/src/routes/cache.rs b/crates/server/src/routes/cache.rs index a5194df..47a4d3a 100644 --- a/crates/server/src/routes/cache.rs +++ b/crates/server/src/routes/cache.rs @@ -170,9 +170,9 @@ async fn sign_narinfo(narinfo: &str, key_file: &std::path::Path) -> String { .output() .await; - if let Ok(o) = re_output { - if let Ok(parsed) = serde_json::from_slice::(&o.stdout) { - if let Some(sigs) = parsed + if let Ok(o) = re_output + && let Ok(parsed) = serde_json::from_slice::(&o.stdout) + && let Some(sigs) = parsed .as_array() .and_then(|a| a.first()) .and_then(|e| e.get("signatures")) @@ -187,8 +187,6 @@ async fn sign_narinfo(narinfo: &str, key_file: &std::path::Path) -> String { return format!("{narinfo}{}\n", sig_lines.join("\n")); } } - } - } narinfo.to_string() } _ => narinfo.to_string(), diff --git a/crates/server/src/routes/dashboard.rs b/crates/server/src/routes/dashboard.rs index 99283cd..8dd5fa7 100644 --- a/crates/server/src/routes/dashboard.rs +++ b/crates/server/src/routes/dashboard.rs @@ -316,6 +316,14 @@ struct AdminTemplate { auth_name: String, } +#[derive(Template)] +#[template(path = "project_setup.html")] +#[allow(dead_code)] +struct ProjectSetupTemplate { + is_admin: bool, + auth_name: String, +} + #[derive(Template)] #[template(path = "login.html")] struct LoginTemplate { @@ -353,14 +361,13 @@ async fn home(State(state): State, extensions: Extensions) -> Html le.evaluation_time) + .is_none_or(|le| e.evaluation_time > le.evaluation_time) { last_eval = Some(e); } - } } let (status, class, time) = match &last_eval { Some(e) => { @@ -884,6 +891,19 @@ async fn admin_page(State(state): State, extensions: Extensions) -> Ht ) } +// --- Setup Wizard --- + +async fn project_setup_page(extensions: Extensions) -> Html { + let tmpl = ProjectSetupTemplate { + is_admin: is_admin(&extensions), + auth_name: auth_name(&extensions), + }; + Html( + tmpl.render() + .unwrap_or_else(|e| format!("Template error: {e}")), + ) +} + // --- Login / Logout --- async fn login_page() -> Html { @@ -950,7 +970,28 @@ async fn login_action(State(state): State, Form(form): Form } } -async fn logout_action() -> Response { +async fn logout_action(State(state): State, request: axum::extract::Request) -> Response { + // Remove server-side session + if let Some(cookie_header) = request + .headers() + .get("cookie") + .and_then(|v| v.to_str().ok()) + && let Some(session_id) = cookie_header + .split(';') + .filter_map(|pair| { + let pair = pair.trim(); + let (k, v) = pair.split_once('=')?; + if k.trim() == "fc_session" { + Some(v.trim().to_string()) + } else { + None + } + }) + .next() + { + state.sessions.remove(&session_id); + } + let cookie = "fc_session=; HttpOnly; SameSite=Strict; Path=/; Max-Age=0"; ( [(axum::http::header::SET_COOKIE, cookie.to_string())], @@ -966,6 +1007,7 @@ pub fn router(state: AppState) -> Router { .route("/logout", axum::routing::post(logout_action)) .route("/", get(home)) .route("/projects", get(projects_page)) + .route("/projects/new", get(project_setup_page)) .route("/project/{id}", get(project_page)) .route("/jobset/{id}", get(jobset_page)) .route("/evaluations", get(evaluations_page)) diff --git a/crates/server/src/routes/evaluations.rs b/crates/server/src/routes/evaluations.rs index 53dba3b..5b50121 100644 --- a/crates/server/src/routes/evaluations.rs +++ b/crates/server/src/routes/evaluations.rs @@ -5,7 +5,8 @@ use axum::{ routing::{get, post}, }; use fc_common::{CreateEvaluation, Evaluation, PaginatedResponse, PaginationParams, Validate}; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; use uuid::Uuid; use crate::auth_middleware::RequireRoles; @@ -85,9 +86,127 @@ async fn trigger_evaluation( Ok(Json(evaluation)) } +#[derive(Debug, Deserialize)] +struct CompareParams { + to: Uuid, +} + +#[derive(Debug, Serialize)] +struct EvalComparison { + from_id: Uuid, + to_id: Uuid, + new_jobs: Vec, + removed_jobs: Vec, + changed_jobs: Vec, + unchanged_count: usize, +} + +#[derive(Debug, Serialize)] +struct JobDiff { + job_name: String, + system: Option, + drv_path: String, + status: String, +} + +#[derive(Debug, Serialize)] +struct JobChange { + job_name: String, + system: Option, + old_drv: String, + new_drv: String, + old_status: String, + new_status: String, +} + +async fn compare_evaluations( + State(state): State, + Path(id): Path, + Query(params): Query, +) -> Result, ApiError> { + // Verify both evaluations exist + let _from_eval = fc_common::repo::evaluations::get(&state.pool, id) + .await + .map_err(ApiError)?; + let _to_eval = fc_common::repo::evaluations::get(&state.pool, params.to) + .await + .map_err(ApiError)?; + + let from_builds = fc_common::repo::builds::list_for_evaluation(&state.pool, id) + .await + .map_err(ApiError)?; + let to_builds = fc_common::repo::builds::list_for_evaluation(&state.pool, params.to) + .await + .map_err(ApiError)?; + + let from_map: HashMap<&str, &fc_common::Build> = from_builds + .iter() + .map(|b| (b.job_name.as_str(), b)) + .collect(); + let to_map: HashMap<&str, &fc_common::Build> = + to_builds.iter().map(|b| (b.job_name.as_str(), b)).collect(); + + let mut new_jobs = Vec::new(); + let mut removed_jobs = Vec::new(); + let mut changed_jobs = Vec::new(); + let mut unchanged_count = 0; + + // Jobs in `to` but not in `from` are new + for (name, build) in &to_map { + if !from_map.contains_key(name) { + new_jobs.push(JobDiff { + job_name: name.to_string(), + system: build.system.clone(), + drv_path: build.drv_path.clone(), + status: format!("{:?}", build.status), + }); + } + } + + // Jobs in `from` but not in `to` are removed + for (name, build) in &from_map { + if !to_map.contains_key(name) { + removed_jobs.push(JobDiff { + job_name: name.to_string(), + system: build.system.clone(), + drv_path: build.drv_path.clone(), + status: format!("{:?}", build.status), + }); + } + } + + // Jobs in both: compare derivation paths + for (name, from_build) in &from_map { + if let Some(to_build) = to_map.get(name) { + if from_build.drv_path != to_build.drv_path { + changed_jobs.push(JobChange { + job_name: name.to_string(), + system: to_build.system.clone(), + old_drv: from_build.drv_path.clone(), + new_drv: to_build.drv_path.clone(), + old_status: format!("{:?}", from_build.status), + new_status: format!("{:?}", to_build.status), + }); + } else { + unchanged_count += 1; + } + } + } + + Ok(Json(EvalComparison { + from_id: id, + to_id: params.to, + new_jobs, + removed_jobs, + changed_jobs, + unchanged_count, + })) +} + pub fn router() -> Router { Router::new() .route("/evaluations", get(list_evaluations)) .route("/evaluations/{id}", get(get_evaluation)) + .route("/evaluations/{id}/compare", get(compare_evaluations)) .route("/evaluations/trigger", post(trigger_evaluation)) } diff --git a/crates/server/src/routes/logs.rs b/crates/server/src/routes/logs.rs index 30fd6d0..e2034cf 100644 --- a/crates/server/src/routes/logs.rs +++ b/crates/server/src/routes/logs.rs @@ -96,13 +96,12 @@ async fn stream_build_log( consecutive_empty += 1; if consecutive_empty > 5 { // Check build status - if let Ok(b) = fc_common::repo::builds::get(&pool, build_id).await { - if b.status != fc_common::models::BuildStatus::Running + if let Ok(b) = fc_common::repo::builds::get(&pool, build_id).await + && b.status != fc_common::models::BuildStatus::Running && b.status != fc_common::models::BuildStatus::Pending { yield Ok(Event::default().event("done").data("Build completed")); return; } - } consecutive_empty = 0; } tokio::time::sleep(std::time::Duration::from_millis(500)).await;