alvaro/normogen
1
0
Fork 0
Code Issues 2 Pull requests Projects Releases Packages Wiki Activity Actions

fix(backend): P1 — handler unwrap cleanup + rewrite integration tests

Browse source 
P1 items #6 (JWT expiry) and #7 (refresh/logout routes) were already delivered
in the P0 pass. This commit covers the two remaining P1 items:

#9 — Replace dangerous handler-level .unwrap() calls (13 sites):
* handlers/users.rs: 5x ObjectId::parse_str(&claims.sub).unwrap() in
  get_profile/update_profile/delete_account/get_settings/update_settings now
  return 401 on a malformed subject instead of panicking (matches the guard
  already used in change_password).
* handlers/auth.rs: the login user.id.ok_or_else(..).unwrap() was a latent
  panic bug — the crafted 500 response was discarded. Now returns the clean
  500 via match.
* handlers/health_stats.rs: 6x state.health_stats_repo.as_ref().unwrap() now
  return 503 SERVICE_UNAVAILABLE if the feature is unconfigured, mirroring the
  interactions.rs pattern.
* Left untouched: 14 test/boot-time unwraps and 7 infallible ones (header
  literal parsing, infallible TryFrom). The 3 borderline model-layer
  inserted_id unwraps are flagged for later.

#8 — Rewrite the broken integration tests against a real test DB:
* Split the crate into bin+lib: new src/lib.rs + src/app.rs (build_app), with
  main.rs now a thin entrypoint. Tests build the exact production router
  in-process instead of hitting a live server on a hardcoded port.
* tests/common/mod.rs: helpers that connect to Mongo, build a fresh AppState
  against a unique per-process DB (normogen_test_<uuid>), and tear it down.
  A 1s connectivity probe makes tests skip gracefully when Mongo is absent, so
  'cargo test' stays green without Mongo — CI runs them for real.
* Rewrote tests/auth_tests.rs and tests/medication_tests.rs with the ACTUAL
  API contracts (POST /register {email,username,password}; response has token
  + refresh_token, not access_token; register returns 201). Covers register,
  login (right/wrong password), auth enforcement, refresh rotation + reuse
  detection, logout, and password-change invalidating old tokens.
* Added a 'test' CI job with a mongo:7 service container running
  cargo test --all-targets.
* Synced scripts/test-ci-locally.sh: fixed the stale -D warnings (CI is
  non-strict) and the reverted 'Docker Buildx' claims; added unit + integration
  test steps with a Mongo skip note.

Verified: cargo fmt --check clean, build + clippy --all-targets clean.
Full 'cargo test': 18 unit + 9 auth + 4 medication = 31 passed, 0 failed
(integration tests skip cleanly when MongoDB is unreachable).
This commit is contained in:
goose 2026-06-27 14:26:39 -03:00
parent 7ba78a31fb
commit bd1b7c2925
13 changed files with 937 additions and 338 deletions
Download patch file Download diff file Expand all files Collapse all files

104
backend/src/app.rs Normal file
View file

@ -0,0 +1,104 @@
//! Router assembly. Extracted from `main.rs` so integration tests can build the
//! exact same app (routes, middleware, layers) in process against a test
//! database instead of a live server.
use axum::{
routing::{delete, get, post, put},
Router,
};
use tower::ServiceBuilder;
use tower_http::{cors::CorsLayer, trace::TraceLayer};
use crate::config::AppState;
use crate::handlers;
use crate::middleware;
/// Compose the full Axum app: all public + protected routes, the JWT auth
/// layer on protected routes, and the global middleware stack (client-IP
/// resolution, security headers, rate limit, tracing, CORS).
///
/// `main.rs` calls this and wraps it in `into_make_service_with_connect_info`
/// before serving; tests call it and drive it with `oneshot`/`RouterExt`.
pub fn build_app(state: AppState) -> Router {
// Build public routes (no auth required)
let public_routes = Router::new()
.route(
"/health",
get(handlers::health_check).head(handlers::health_check),
)
.route("/ready", get(handlers::ready_check))
.route("/api/auth/register", post(handlers::register))
.route("/api/auth/login", post(handlers::login))
.route("/api/auth/refresh", post(handlers::refresh))
.route("/api/auth/logout", post(handlers::logout))
.route(
"/api/auth/recover-password",
post(handlers::recover_password),
);
// Build protected routes (auth required)
let protected_routes = Router::new()
// User profile management
.route("/api/users/me", get(handlers::get_profile))
.route("/api/users/me", put(handlers::update_profile))
.route("/api/users/me", delete(handlers::delete_account))
.route("/api/users/me/change-password", post(handlers::change_password))
// User settings
.route("/api/users/me/settings", get(handlers::get_settings))
.route("/api/users/me/settings", put(handlers::update_settings))
// Share management
.route("/api/shares", post(handlers::create_share))
.route("/api/shares", get(handlers::list_shares))
.route("/api/shares/:id", put(handlers::update_share))
.route("/api/shares/:id", delete(handlers::delete_share))
// Permission checking
.route("/api/permissions/check", post(handlers::check_permission))
// Session management (Phase 2.6)
.route("/api/sessions", get(handlers::get_sessions))
.route("/api/sessions/:id", delete(handlers::revoke_session))
.route("/api/sessions/all", delete(handlers::revoke_all_sessions))
// Medication management (Phase 2.7)
.route("/api/medications", post(handlers::create_medication))
.route("/api/medications", get(handlers::list_medications))
.route("/api/medications/:id", get(handlers::get_medication))
.route("/api/medications/:id", post(handlers::update_medication))
.route("/api/medications/:id/delete", post(handlers::delete_medication))
.route("/api/medications/:id/log", post(handlers::log_dose))
.route("/api/medications/:id/adherence", get(handlers::get_adherence))
// Health statistics management (Phase 2.7)
.route("/api/health-stats", post(handlers::create_health_stat))
.route("/api/health-stats", get(handlers::list_health_stats))
.route("/api/health-stats/trends", get(handlers::get_health_trends))
.route("/api/health-stats/:id", get(handlers::get_health_stat))
.route("/api/health-stats/:id", put(handlers::update_health_stat))
.route("/api/health-stats/:id", delete(handlers::delete_health_stat))
// Drug interactions (Phase 2.8)
.route("/api/interactions/check", post(handlers::check_interactions))
.route("/api/interactions/check-new", post(handlers::check_new_medication))
.layer(axum::middleware::from_fn_with_state(
state.clone(),
middleware::jwt_auth_middleware,
));
public_routes
.merge(protected_routes)
.with_state(state)
.layer(
ServiceBuilder::new()
// Resolve the client IP once and stash it for handlers/middleware
// (audit logging, account-lockout forensics).
.layer(axum::middleware::from_fn(
middleware::client_ip_middleware,
))
// Security headers (applies to all responses)
.layer(axum::middleware::from_fn(
middleware::security_headers_middleware,
))
// General rate limiting (currently a stub)
.layer(axum::middleware::from_fn(
middleware::general_rate_limit_middleware,
))
.layer(TraceLayer::new_for_http())
.layer(CorsLayer::permissive()),
)
}

16
backend/src/handlers/auth.rs
View file

@ -259,17 +259,21 @@ pub async fn login(
}
};
let user_id = user
.id
.ok_or_else(|| {
(
// A persisted user must have an id; if not, return a clean 500 instead of
// panicking (the previous ok_or_else(...).unwrap() discarded this response).
let user_id = match user.id {
Some(id) => id,
None => {
tracing::error!("Login user record has no id for email: {}", req.email);
return (
StatusCode::INTERNAL_SERVER_ERROR,
Json(serde_json::json!({
"error": "invalid user state"
})),
)
})
.unwrap();
.into_response();
}
};
// Verify password
match user.verify_password(&req.password) {

66
backend/src/handlers/health_stats.rs
View file

@ -38,7 +38,16 @@ pub async fn create_health_stat(
Extension(claims): Extension<Claims>,
Json(req): Json<CreateHealthStatRequest>,
) -> impl IntoResponse {
let repo = state.health_stats_repo.as_ref().unwrap();
let repo = match state.health_stats_repo.as_ref() {
Some(r) => r,
None => {
return (
StatusCode::SERVICE_UNAVAILABLE,
Json(serde_json::json!({ "error": "health stats unavailable" })),
)
.into_response()
}
};
// Convert complex value to f64 or store as string
let value_num = match req.value {
@ -80,7 +89,16 @@ pub async fn list_health_stats(
State(state): State<AppState>,
Extension(claims): Extension<Claims>,
) -> impl IntoResponse {
let repo = state.health_stats_repo.as_ref().unwrap();
let repo = match state.health_stats_repo.as_ref() {
Some(r) => r,
None => {
return (
StatusCode::SERVICE_UNAVAILABLE,
Json(serde_json::json!({ "error": "health stats unavailable" })),
)
.into_response()
}
};
match repo.find_by_user(&claims.sub).await {
Ok(stats) => (StatusCode::OK, Json(stats)).into_response(),
Err(e) => {
@ -99,7 +117,16 @@ pub async fn get_health_stat(
Extension(claims): Extension<Claims>,
Path(id): Path<String>,
) -> impl IntoResponse {
let repo = state.health_stats_repo.as_ref().unwrap();
let repo = match state.health_stats_repo.as_ref() {
Some(r) => r,
None => {
return (
StatusCode::SERVICE_UNAVAILABLE,
Json(serde_json::json!({ "error": "health stats unavailable" })),
)
.into_response()
}
};
let object_id = match ObjectId::parse_str(&id) {
Ok(oid) => oid,
Err(_) => return (StatusCode::BAD_REQUEST, "Invalid ID").into_response(),
@ -130,7 +157,16 @@ pub async fn update_health_stat(
Path(id): Path<String>,
Json(req): Json<UpdateHealthStatRequest>,
) -> impl IntoResponse {
let repo = state.health_stats_repo.as_ref().unwrap();
let repo = match state.health_stats_repo.as_ref() {
Some(r) => r,
None => {
return (
StatusCode::SERVICE_UNAVAILABLE,
Json(serde_json::json!({ "error": "health stats unavailable" })),
)
.into_response()
}
};
let object_id = match ObjectId::parse_str(&id) {
Ok(oid) => oid,
Err(_) => return (StatusCode::BAD_REQUEST, "Invalid ID").into_response(),
@ -182,7 +218,16 @@ pub async fn delete_health_stat(
Extension(claims): Extension<Claims>,
Path(id): Path<String>,
) -> impl IntoResponse {
let repo = state.health_stats_repo.as_ref().unwrap();
let repo = match state.health_stats_repo.as_ref() {
Some(r) => r,
None => {
return (
StatusCode::SERVICE_UNAVAILABLE,
Json(serde_json::json!({ "error": "health stats unavailable" })),
)
.into_response()
}
};
let object_id = match ObjectId::parse_str(&id) {
Ok(oid) => oid,
Err(_) => return (StatusCode::BAD_REQUEST, "Invalid ID").into_response(),
@ -219,7 +264,16 @@ pub async fn get_health_trends(
Extension(claims): Extension<Claims>,
Query(query): Query<HealthTrendsQuery>,
) -> impl IntoResponse {
let repo = state.health_stats_repo.as_ref().unwrap();
let repo = match state.health_stats_repo.as_ref() {
Some(r) => r,
None => {
return (
StatusCode::SERVICE_UNAVAILABLE,
Json(serde_json::json!({ "error": "health stats unavailable" })),
)
.into_response()
}
};
match repo.find_by_user(&claims.sub).await {
Ok(stats) => {
// Filter by stat_type

55
backend/src/handlers/users.rs
View file

@ -43,7 +43,16 @@ pub async fn get_profile(
State(state): State<AppState>,
Extension(claims): Extension<Claims>,
) -> impl IntoResponse {
let user_id = ObjectId::parse_str(&claims.sub).unwrap();
let user_id = match ObjectId::parse_str(&claims.sub) {
Ok(oid) => oid,
Err(_) => {
return (
StatusCode::UNAUTHORIZED,
Json(serde_json::json!({ "error": "invalid token" })),
)
.into_response()
}
};
match state.db.find_user_by_id(&user_id).await {
Ok(Some(user)) => {
@ -86,7 +95,16 @@ pub async fn update_profile(
.into_response();
}
let user_id = ObjectId::parse_str(&claims.sub).unwrap();
let user_id = match ObjectId::parse_str(&claims.sub) {
Ok(oid) => oid,
Err(_) => {
return (
StatusCode::UNAUTHORIZED,
Json(serde_json::json!({ "error": "invalid token" })),
)
.into_response()
}
};
let mut user = match state.db.find_user_by_id(&user_id).await {
Ok(Some(u)) => u,
@ -137,7 +155,16 @@ pub async fn delete_account(
State(state): State<AppState>,
Extension(claims): Extension<Claims>,
) -> impl IntoResponse {
let user_id = ObjectId::parse_str(&claims.sub).unwrap();
let user_id = match ObjectId::parse_str(&claims.sub) {
Ok(oid) => oid,
Err(_) => {
return (
StatusCode::UNAUTHORIZED,
Json(serde_json::json!({ "error": "invalid token" })),
)
.into_response()
}
};
match state.db.delete_user(&user_id).await {
Ok(_) => (StatusCode::NO_CONTENT, ()).into_response(),
@ -314,7 +341,16 @@ pub async fn get_settings(
State(state): State<AppState>,
Extension(claims): Extension<Claims>,
) -> impl IntoResponse {
let user_id = ObjectId::parse_str(&claims.sub).unwrap();
let user_id = match ObjectId::parse_str(&claims.sub) {
Ok(oid) => oid,
Err(_) => {
return (
StatusCode::UNAUTHORIZED,
Json(serde_json::json!({ "error": "invalid token" })),
)
.into_response()
}
};
match state.db.find_user_by_id(&user_id).await {
Ok(Some(user)) => {
@ -351,7 +387,16 @@ pub async fn update_settings(
Extension(claims): Extension<Claims>,
Json(req): Json<UpdateSettingsRequest>,
) -> impl IntoResponse {
let user_id = ObjectId::parse_str(&claims.sub).unwrap();
let user_id = match ObjectId::parse_str(&claims.sub) {
Ok(oid) => oid,
Err(_) => {
return (
StatusCode::UNAUTHORIZED,
Json(serde_json::json!({ "error": "invalid token" })),
)
.into_response()
}
};
let mut user = match state.db.find_user_by_id(&user_id).await {
Ok(Some(u)) => u,

16
backend/src/lib.rs Normal file
View file

@ -0,0 +1,16 @@
//! Library facade so the binary entrypoint and integration tests share a single
//! source of truth for module wiring and router assembly.
//!
//! `main.rs` is a thin wrapper that loads config, connects to Mongo, builds the
//! app via [`app::build_app`], and serves it. Tests build the same app in
//! process against a throwaway database.
pub mod app;
pub mod auth;
pub mod config;
pub mod db;
pub mod handlers;
pub mod middleware;
pub mod models;
pub mod security;
pub mod services;

134
backend/src/main.rs
View file

@ -1,20 +1,19 @@
mod auth;
mod config;
mod db;
mod handlers;
mod middleware;
mod models;
mod security;
mod services;
//! Binary entrypoint: loads config, connects to MongoDB, wires services into
//! [`config::AppState`], builds the router via [`app::build_app`], and serves.
//!
//! All module wiring and route/middleware composition lives in the library
//! (`lib.rs` + `app.rs`) so integration tests can reuse them.
use axum::{
routing::{delete, get, post, put},
Router,
};
use config::Config;
use std::sync::Arc;
use tower::ServiceBuilder;
use tower_http::{cors::CorsLayer, trace::TraceLayer};
use normogen_backend::{
app::build_app,
auth::{JwtService, TokenVersionCache},
config::{self, Config, Environment},
db,
models::{health_stats::HealthStatisticsRepository, refresh_token::RefreshTokenRepository},
security, services,
};
#[tokio::main]
async fn main() -> anyhow::Result<()> {
@ -33,6 +32,11 @@ async fn main() -> anyhow::Result<()> {
// Load configuration
let config = Config::from_env()?;
if config.environment == Environment::Production {
eprintln!("Running in PRODUCTION mode");
} else {
eprintln!("Running in DEVELOPMENT mode");
}
eprintln!("Configuration loaded successfully");
// Connect to MongoDB
@ -65,7 +69,7 @@ async fn main() -> anyhow::Result<()> {
}
// Create JWT service
let jwt_service = auth::JwtService::new(config.jwt.clone());
let jwt_service = JwtService::new(config.jwt.clone());
// Get the underlying MongoDB database for security services
let database = db.get_database();
@ -82,12 +86,10 @@ async fn main() -> anyhow::Result<()> {
// Short-TTL cache of token_version, so the JWT middleware can reject stale
// access tokens without a Mongo lookup per request.
let token_version_cache = std::sync::Arc::new(auth::TokenVersionCache::with_default_ttl());
let token_version_cache = Arc::new(TokenVersionCache::with_default_ttl());
// Persisted refresh tokens (hashed) for rotation/revocation.
let refresh_token_repo = std::sync::Arc::new(
models::refresh_token::RefreshTokenRepository::new(&database),
);
let refresh_token_repo = Arc::new(RefreshTokenRepository::new(&database));
// Initialize security services (Phase 2.6)
let audit_logger = security::AuditLogger::new(&database);
@ -103,7 +105,7 @@ async fn main() -> anyhow::Result<()> {
);
// Initialize health stats repository (Phase 2.7) - using Database pattern
let health_stats_repo = models::health_stats::HealthStatisticsRepository::new(&database);
let health_stats_repo = HealthStatisticsRepository::new(&database);
// Initialize interaction service (Phase 2.8)
let interaction_service = Arc::new(services::InteractionService::new());
@ -125,95 +127,7 @@ async fn main() -> anyhow::Result<()> {
};
eprintln!("Building router with security middleware...");
// Build public routes (no auth required)
let public_routes = Router::new()
.route(
"/health",
get(handlers::health_check).head(handlers::health_check),
)
.route("/ready", get(handlers::ready_check))
.route("/api/auth/register", post(handlers::register))
.route("/api/auth/login", post(handlers::login))
.route("/api/auth/refresh", post(handlers::refresh))
.route("/api/auth/logout", post(handlers::logout))
.route(
"/api/auth/recover-password",
post(handlers::recover_password),
);
// Build protected routes (auth required)
let protected_routes = Router::new()
// User profile management
.route("/api/users/me", get(handlers::get_profile))
.route("/api/users/me", put(handlers::update_profile))
.route("/api/users/me", delete(handlers::delete_account))
.route("/api/users/me/change-password", post(handlers::change_password))
// User settings
.route("/api/users/me/settings", get(handlers::get_settings))
.route("/api/users/me/settings", put(handlers::update_settings))
// Share management
.route("/api/shares", post(handlers::create_share))
.route("/api/shares", get(handlers::list_shares))
.route("/api/shares/:id", put(handlers::update_share))
.route("/api/shares/:id", delete(handlers::delete_share))
// Permission checking
.route("/api/permissions/check", post(handlers::check_permission))
// Session management (Phase 2.6)
.route("/api/sessions", get(handlers::get_sessions))
.route("/api/sessions/:id", delete(handlers::revoke_session))
.route("/api/sessions/all", delete(handlers::revoke_all_sessions))
// Medication management (Phase 2.7)
.route("/api/medications", post(handlers::create_medication))
.route("/api/medications", get(handlers::list_medications))
.route("/api/medications/:id", get(handlers::get_medication))
.route("/api/medications/:id", post(handlers::update_medication))
.route("/api/medications/:id/delete", post(handlers::delete_medication))
.route("/api/medications/:id/log", post(handlers::log_dose))
.route("/api/medications/:id/adherence", get(handlers::get_adherence))
// Health statistics management (Phase 2.7)
.route("/api/health-stats", post(handlers::create_health_stat))
.route("/api/health-stats", get(handlers::list_health_stats))
.route("/api/health-stats/trends", get(handlers::get_health_trends))
.route("/api/health-stats/:id", get(handlers::get_health_stat))
.route("/api/health-stats/:id", put(handlers::update_health_stat))
.route("/api/health-stats/:id", delete(handlers::delete_health_stat))
// Drug interactions (Phase 2.8)
.route("/api/interactions/check", post(handlers::check_interactions))
.route("/api/interactions/check-new", post(handlers::check_new_medication))
.layer(axum::middleware::from_fn_with_state(
state.clone(),
middleware::jwt_auth_middleware
));
let app = public_routes
.merge(protected_routes)
.with_state(state)
.layer(
ServiceBuilder::new()
// Resolve the client IP once and stash it for handlers/middleware
// (audit logging, account-lockout forensics).
.layer(axum::middleware::from_fn(
middleware::client_ip_middleware,
))
// Add security headers first (applies to all responses)
.layer(axum::middleware::from_fn(
middleware::security_headers_middleware
))
// Add general rate limiting
.layer(axum::middleware::from_fn(
middleware::general_rate_limit_middleware
))
.layer(TraceLayer::new_for_http())
.layer(CorsLayer::permissive()),
);
let app = build_app(state);
let addr = format!("{}:{}", config.server.host, config.server.port);
eprintln!("Binding to {}...", addr);