From 8991e2000ee88c5a09c34be8298e52d44f182537 Mon Sep 17 00:00:00 2001 From: Purvesh Date: Sat, 24 Oct 2026 16:37:18 +1300 Subject: [PATCH 1/3] remove unwrap --- src/api/rest_api_routes.rs | 2 +- src/core/domain/entities/workflow.rs | 26 ++++++++++------- src/error.rs | 31 ++++++++++++++++++++ src/infrastructure/workflow_dispatcher.rs | 15 ++++++---- src/interfaces/web/pages/auth/login_page.rs | 32 ++++++++++++++------- src/main.rs | 7 ++--- 6 files changed, 81 insertions(+), 32 deletions(-) diff --git a/src/api/rest_api_routes.rs b/src/api/rest_api_routes.rs index 7810d45e..742c25e0 100644 --- a/src/api/rest_api_routes.rs +++ b/src/api/rest_api_routes.rs @@ -61,7 +61,7 @@ pub fn rest_api_routes(state: AppState) -> crate::error::Result { let env_str_allowed_cors = state.config.cors_allowed_app_url.clone(); for origin in &env_str_allowed_cors { - origins.push(HeaderValue::from_str(origin).unwrap()); + origins.push(HeaderValue::from_str(origin)?); } let cors = CorsLayer::new() diff --git a/src/core/domain/entities/workflow.rs b/src/core/domain/entities/workflow.rs index 34fe2d3d..86fb4ac0 100644 --- a/src/core/domain/entities/workflow.rs +++ b/src/core/domain/entities/workflow.rs @@ -58,28 +58,31 @@ pub struct WorkflowExecutionModel { #[cfg(test)] mod tests { + use crate::error::Result; use super::{WorkflowAction, WorkflowEvent, WorkflowExecutionStatus}; #[test] - fn workflow_events_use_stable_snake_case_names() { + fn workflow_events_use_stable_snake_case_names() -> Result<()> { assert_eq!( - serde_json::to_value(WorkflowEvent::Created).unwrap(), + serde_json::to_value(WorkflowEvent::Created)?, serde_json::json!("created") ); assert_eq!( - serde_json::to_value(WorkflowEvent::Updated).unwrap(), + serde_json::to_value(WorkflowEvent::Updated)?, serde_json::json!("updated") ); + + Ok(()) } #[test] - fn workflow_actions_use_a_discriminated_ordered_action_shape() { + fn workflow_actions_use_a_discriminated_ordered_action_shape() -> Result<()> { let action = WorkflowAction::Email { template_id: "welcome".to_string(), recipient: "admin@example.test".to_string(), }; - let encoded = serde_json::to_value(&action).unwrap(); + let encoded = serde_json::to_value(&action)?; assert_eq!( encoded, serde_json::json!({ @@ -89,24 +92,27 @@ mod tests { }) ); assert_eq!( - serde_json::from_value::(encoded).unwrap(), + serde_json::from_value::(encoded)?, action ); + + Ok(()) } #[test] - fn workflow_execution_status_names_are_stable() { + fn workflow_execution_status_names_are_stable() -> Result<()> { assert_eq!( - serde_json::to_value(WorkflowExecutionStatus::Running).unwrap(), + serde_json::to_value(WorkflowExecutionStatus::Running)?, serde_json::json!("running") ); assert_eq!( - serde_json::to_value(WorkflowExecutionStatus::Success).unwrap(), + serde_json::to_value(WorkflowExecutionStatus::Success)?, serde_json::json!("success") ); assert_eq!( - serde_json::to_value(WorkflowExecutionStatus::Failed).unwrap(), + serde_json::to_value(WorkflowExecutionStatus::Failed)?, serde_json::json!("failed") ); + Ok(()) } } diff --git a/src/error.rs b/src/error.rs index 4f7a1812..7cafed85 100644 --- a/src/error.rs +++ b/src/error.rs @@ -69,6 +69,37 @@ impl From for Error { } } +impl From for Error { + fn from(actual_error: leptos::config::errors::LeptosConfigError) -> Self { + error!("leptos config error: {actual_error:?}"); + Self::Generic(format!("there is an issue with leptos config")) + } +} + +impl From for Error { + fn from(actual_error: std::io::Error) -> Self { + error!("IO error: {actual_error:?}"); + Self::Generic(format!("there is an issue with IO")) + } +} + +impl From for Error { + fn from(actual_error: reqwest::header::InvalidHeaderValue) -> Self { + error!("Invalid header value error: {actual_error:?}"); + Self::Generic(format!("there is an issue with the header value")) + } +} + + +impl From for Error { + fn from(actual_error: std::net::AddrParseError) -> Self { + error!("Address parse error: {actual_error:?}"); + Self::Generic(format!("there is an issue with the address")) + } +} + + + impl IntoResponse for Error { fn into_response(self) -> Response { match self { diff --git a/src/infrastructure/workflow_dispatcher.rs b/src/infrastructure/workflow_dispatcher.rs index 3838ef64..43700ea3 100644 --- a/src/infrastructure/workflow_dispatcher.rs +++ b/src/infrastructure/workflow_dispatcher.rs @@ -257,6 +257,7 @@ mod tests { use super::{HttpWebhookExecutor, WorkflowActionExecutor}; use crate::core::domain::entities::WorkflowAction; use serde_json::json; + use crate::error::Result; #[test] fn rejects_non_public_ipv4_addresses() { @@ -278,13 +279,13 @@ mod tests { } #[test] - fn rejects_non_public_ipv6_addresses() { + fn rejects_non_public_ipv6_addresses() -> Result<()> { for address in [ Ipv6Addr::LOCALHOST, - "fc00::1".parse().unwrap(), - "fe80::1".parse().unwrap(), - "2001:db8::1".parse().unwrap(), - "2002::1".parse().unwrap(), + "fc00::1".parse()?, + "fe80::1".parse()?, + "2001:db8::1".parse()?, + "2002::1".parse()?, ] { assert!( !is_public_ip(IpAddr::V6(address)), @@ -292,8 +293,10 @@ mod tests { ); } assert!(is_public_ip(IpAddr::V6( - "2606:4700:4700::1111".parse().unwrap() + "2606:4700:4700::1111".parse()? ))); + + Ok(()) } #[tokio::test] diff --git a/src/interfaces/web/pages/auth/login_page.rs b/src/interfaces/web/pages/auth/login_page.rs index 4addfecf..18373a6e 100644 --- a/src/interfaces/web/pages/auth/login_page.rs +++ b/src/interfaces/web/pages/auth/login_page.rs @@ -40,6 +40,23 @@ pub struct ApiValidationError { pub message: String, } +async fn send_login_request( + login_request: LoginRequest, +) -> Result { + let request_body = serde_json::to_string(&login_request) + .map_err(|err| format!("Failed to serialize login request: {err}"))?; + + let request = gloo_net::http::Request::post("/api/auth/login") + .header("Content-Type", "application/json") + .body(request_body) + .map_err(|err| format!("Failed to build login request: {err}"))?; + + request + .send() + .await + .map_err(|err| format!("Unable to reach login API: {err}")) +} + #[component] pub fn LoginPage() -> impl IntoView { let email = RwSignal::new("admin@avored.com".to_string()); @@ -67,18 +84,13 @@ pub fn LoginPage() -> impl IntoView { field_errors.set(HashMap::new()); leptos::task::spawn_local(async move { - let request_body = serde_json::to_string(&LoginRequest { + let response = send_login_request(LoginRequest { email: email_value, password: password_value, }) - .unwrap_or_default(); + .await; - let request = gloo_net::http::Request::post("/api/auth/login") - .header("Content-Type", "application/json") - .body(request_body) - .unwrap(); - - match request.send().await { + match response { Ok(response) if response.ok() => match response.json::().await { Ok(login_response) => { if login_response.authenticated { @@ -126,9 +138,7 @@ pub fn LoginPage() -> impl IntoView { } } } - Err(err) => { - error.set(format!("Unable to reach login API: {err}")); - } + Err(err) => error.set(err), } submitting.set(false); diff --git a/src/main.rs b/src/main.rs index ef56fe22..64f93163 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,7 +4,7 @@ use avored_rust_cms::{api::rest_api_routes::rest_api_routes, avored_state::AppSt #[tokio::main] async fn main() -> error::Result<()> { - let conf = leptos::config::get_configuration(None).unwrap(); + let conf = leptos::config::get_configuration(None)?; let leptos_options = conf.leptos_options; let state = AppState::new(leptos_options).await?; @@ -12,10 +12,9 @@ async fn main() -> error::Result<()> { let app: axum::Router = rest_api_routes(state)?; println!("listening on http://{}", 3000); - let listener = tokio::net::TcpListener::bind("0.0.0.0:3000").await.unwrap(); + let listener = tokio::net::TcpListener::bind("0.0.0.0:3000").await?; axum::serve(listener, app.into_make_service()) - .await - .unwrap(); + .await?; Ok(()) } From d8c90d51f5027e0e0d9ec304ff4a3152d4e0631e Mon Sep 17 00:00:00 2001 From: Purvesh Date: Sat, 24 Oct 2026 16:47:51 +1300 Subject: [PATCH 2/3] remove all unwrap --- src/error.rs | 20 +++++- .../middleware/auth_middleware.rs | 69 ++++++++---------- tests/workflow_api_integration_test.rs | 72 ++++++++++++++++++- 3 files changed, 119 insertions(+), 42 deletions(-) diff --git a/src/error.rs b/src/error.rs index 7cafed85..56dcdf96 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,5 +1,6 @@ use axum::http::{self, StatusCode}; use axum::response::{IntoResponse, Response}; +use axum::Json; use rust_i18n::t; use tracing::error; @@ -9,6 +10,22 @@ use crate::core::domain::entities::ErrorMessageResponse; /// This is custom Result type for the application. pub type Result = core::result::Result; +#[derive(Debug, serde::Serialize, Default)] +pub struct AuthErrorResponse { + pub status: bool, + pub message: String, +} + +pub fn unauthorized(message: impl Into) -> (StatusCode, Json) { + ( + StatusCode::UNAUTHORIZED, + Json(AuthErrorResponse { + status: false, + message: message.into(), + }), + ) +} + /// This is the custom error type for the application. #[derive(Debug, Clone)] pub enum Error { @@ -90,7 +107,6 @@ impl From for Error { } } - impl From for Error { fn from(actual_error: std::net::AddrParseError) -> Self { error!("Address parse error: {actual_error:?}"); @@ -98,8 +114,6 @@ impl From for Error { } } - - impl IntoResponse for Error { fn into_response(self) -> Response { match self { diff --git a/src/infrastructure/middleware/auth_middleware.rs b/src/infrastructure/middleware/auth_middleware.rs index 6f7c8fbf..30d9dbf5 100644 --- a/src/infrastructure/middleware/auth_middleware.rs +++ b/src/infrastructure/middleware/auth_middleware.rs @@ -1,22 +1,11 @@ use crate::avored_state::AppState; use crate::core::domain::entities::user::TokenClaims; -use crate::error::Error; +use crate::error::{unauthorized, AuthErrorResponse as ErrorResponse}; use axum::body::Body; use axum::http::StatusCode; use axum::response::IntoResponse; use axum::{http::Request, middleware::Next, Json}; use jsonwebtoken::{decode, DecodingKey, Validation}; -use serde::Serialize; -use std::borrow::ToOwned; - -#[derive(Debug, Serialize, Default)] -/// error response struct -pub struct ErrorResponse { - /// status of the response - pub status: bool, - /// message of the response - pub message: String, -} /// Middleware to require JWT authentication for incoming requests pub async fn check_auth( @@ -25,12 +14,20 @@ pub async fn check_auth( next: Next, ) -> Result)> { if let Some(t) = req.headers().get("authorization") { - let auth_value = t.to_str().map_err(|_e| Error::Authentication).unwrap(); + let Ok(auth_value) = t.to_str() else { + return Err(unauthorized( + "Authentication failed: Invalid authorization header", + )); + }; let jwt_token = &state.config.jwt_secret_key; - let token = auth_value.strip_prefix("Bearer ").map(ToOwned::to_owned); + let Some(token) = auth_value.strip_prefix("Bearer ") else { + return Err(unauthorized( + "Authentication failed: Invalid authorization header", + )); + }; let claims = match decode::( - &token.unwrap_or_default(), + token, &DecodingKey::from_secret(jwt_token.as_ref()), &Validation::default(), ) { @@ -43,11 +40,9 @@ pub async fn check_auth( _ => "Token validation failed", }; - let json_error = ErrorResponse { - status: false, - message: format!("Authentication failed: {error_message}"), - }; - return Err((StatusCode::UNAUTHORIZED, Json(json_error))); + return Err(unauthorized(format!( + "Authentication failed: {error_message}" + ))); } }; @@ -55,11 +50,7 @@ pub async fn check_auth( Ok(next.run(req).await) } else { - let json_error = ErrorResponse { - status: false, - message: "You are not logged in, please provide token".to_string(), - }; - Err((StatusCode::UNAUTHORIZED, Json(json_error))) + Err(unauthorized("You are not logged in, please provide token")) } } @@ -70,12 +61,20 @@ pub async fn check_customer_auth( next: Next, ) -> Result)> { if let Some(t) = req.headers().get("authorization") { - let auth_value = t.to_str().map_err(|_e| Error::Authentication).unwrap(); + let Ok(auth_value) = t.to_str() else { + return Err(unauthorized( + "Authentication failed: Invalid authorization header", + )); + }; let jwt_token = &state.config.jwt_secret_key; - let token = auth_value.strip_prefix("Bearer ").map(ToOwned::to_owned); + let Some(token) = auth_value.strip_prefix("Bearer ") else { + return Err(unauthorized( + "Authentication failed: Invalid authorization header", + )); + }; let claims = match decode::( - &token.unwrap_or_default(), + token, &DecodingKey::from_secret(jwt_token.as_ref()), &Validation::default(), ) { @@ -88,11 +87,9 @@ pub async fn check_customer_auth( _ => "Token validation failed", }; - let json_error = ErrorResponse { - status: false, - message: format!("Authentication failed: {error_message}"), - }; - return Err((StatusCode::UNAUTHORIZED, Json(json_error))); + return Err(unauthorized(format!( + "Authentication failed: {error_message}" + ))); } }; @@ -100,10 +97,6 @@ pub async fn check_customer_auth( Ok(next.run(req).await) } else { - let json_error = ErrorResponse { - status: false, - message: "You are not logged in, please provide token".to_string(), - }; - Err((StatusCode::UNAUTHORIZED, Json(json_error))) + Err(unauthorized("You are not logged in, please provide token")) } } diff --git a/tests/workflow_api_integration_test.rs b/tests/workflow_api_integration_test.rs index cf4a6353..66acc6a9 100644 --- a/tests/workflow_api_integration_test.rs +++ b/tests/workflow_api_integration_test.rs @@ -11,7 +11,7 @@ use avored_rust_cms::{ }; use axum::{ body::{to_bytes, Body}, - http::{Request, StatusCode}, + http::{header::AUTHORIZATION, HeaderValue, Request, StatusCode}, routing::{delete, get, patch, post, put}, Router, }; @@ -57,6 +57,16 @@ fn workflow_api(state: avored_rust_cms::avored_state::AppState) -> Router { .with_state(state) } +fn customer_auth_probe(state: avored_rust_cms::avored_state::AppState) -> Router { + Router::new() + .route("/api/workflows", get(|| async { StatusCode::OK })) + .layer(axum::middleware::from_fn_with_state( + state.clone(), + avored_rust_cms::infrastructure::middleware::auth_middleware::check_customer_auth, + )) + .with_state(state) +} + fn request(method: &str, uri: &str, body: Option<&str>, jwt: Option<&str>) -> Request { let mut builder = Request::builder().method(method).uri(uri); if let Some(jwt) = jwt { @@ -74,6 +84,26 @@ fn request(method: &str, uri: &str, body: Option<&str>, jwt: Option<&str>) -> Re .expect("request should be valid") } +fn malformed_authorization_request() -> Request { + let mut request = Request::builder() + .uri("/api/workflows") + .body(Body::empty()) + .expect("request should be valid"); + request.headers_mut().insert( + AUTHORIZATION, + HeaderValue::from_bytes(&[0xff]).expect("header value should accept opaque bytes"), + ); + request +} + +fn authorization_request(value: &str) -> Request { + Request::builder() + .uri("/api/workflows") + .header(AUTHORIZATION, value) + .body(Body::empty()) + .expect("request should be valid") +} + async fn response_json(response: axum::response::Response) -> serde_json::Value { let body = to_bytes(response.into_body(), usize::MAX) .await @@ -93,6 +123,46 @@ async fn workflow_api_requires_authentication() { assert_eq!(response.status(), StatusCode::UNAUTHORIZED); } +#[tokio::test] +async fn auth_middlewares_reject_malformed_authorization_headers_without_panicking() { + let state = test_avored_state().await; + let apps = [workflow_api(state.clone()), customer_auth_probe(state)]; + + for app in apps { + let response = app + .oneshot(malformed_authorization_request()) + .await + .expect("request should complete"); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + let error = response_json(response).await; + assert_eq!(error["status"], false); + assert_eq!( + error["message"], + "Authentication failed: Invalid authorization header" + ); + } +} + +#[tokio::test] +async fn auth_middlewares_reject_authorization_headers_without_bearer_prefix() { + let state = test_avored_state().await; + let apps = [workflow_api(state.clone()), customer_auth_probe(state)]; + + for app in apps { + let response = app + .oneshot(authorization_request("Basic abc123")) + .await + .expect("request should complete"); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + let error = response_json(response).await; + assert_eq!(error["status"], false); + assert_eq!( + error["message"], + "Authentication failed: Invalid authorization header" + ); + } +} + #[tokio::test] async fn workflow_routes_are_registered_on_the_application_router() { let state = test_avored_state().await; From fa620e0156e283571b2332e6bd5d802f197daa6d Mon Sep 17 00:00:00 2001 From: Purvesh Date: Sat, 24 Oct 2026 17:46:08 +1300 Subject: [PATCH 3/3] fixed the unit test --- tests/workflow_entity_dispatch_test.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/workflow_entity_dispatch_test.rs b/tests/workflow_entity_dispatch_test.rs index 1765a2b1..d5960dc0 100644 --- a/tests/workflow_entity_dispatch_test.rs +++ b/tests/workflow_entity_dispatch_test.rs @@ -9,10 +9,15 @@ use avored_rust_cms::{ WorkflowAction, WorkflowEvent, WorkflowExecutionStatus, WorkflowTrigger, }, }, + interfaces::api::api_manager::{ + create_entity_record_handler::create_entity_record_handler, + update_entity_record_handler::update_entity_record_handler, + }, }; use axum::{ body::{to_bytes, Body}, http::{Request, StatusCode}, + routing::{patch, post}, Router, }; use chrono::Utc; @@ -116,8 +121,14 @@ async fn entity_create_and_update_dispatch_matching_workflows_without_changing_r .expect("update-trigger workflow should be configured"); let token = auth_token(&state); - let app: Router = avored_rust_cms::api::rest_api_routes::rest_api_routes(state.clone()) - .expect("application routes should be configured"); + let app: Router = Router::new() + .route("/api/v1/entities", post(create_entity_record_handler)) + .route("/api/v1/entities/{id}", patch(update_entity_record_handler)) + .layer(axum::middleware::from_fn_with_state( + state.clone(), + avored_rust_cms::infrastructure::middleware::auth_middleware::check_auth, + )) + .with_state(state.clone()); let create_response = app .clone()