refactor: add DomainError::Forbidden + centralize error-to-HTTP mapping

Ownership checks (delete_review, confirm/dismiss watch events) now
return Forbidden instead of Unauthorized. Presentation layer maps
DomainError→StatusCode via domain_error_response helper, replacing
verbose per-handler match arms.
This commit is contained in:
2026-06-02 21:00:22 +02:00
parent 28170c95d4
commit 4067dedb28
8 changed files with 61 additions and 127 deletions

View File

@@ -17,7 +17,7 @@ pub async fn execute(ctx: &AppContext, cmd: DeleteReviewCommand) -> Result<(), D
.ok_or_else(|| DomainError::NotFound(format!("review {}", cmd.review_id)))?; .ok_or_else(|| DomainError::NotFound(format!("review {}", cmd.review_id)))?;
if review.user_id() != &requesting_user_id { if review.user_id() != &requesting_user_id {
return Err(DomainError::Unauthorized("not your review".into())); return Err(DomainError::Forbidden("not your review".into()));
} }
let movie_id = review.movie_id().clone(); let movie_id = review.movie_id().clone();

View File

@@ -25,7 +25,7 @@ pub async fn execute(ctx: &AppContext, cmd: ConfirmWatchEventsCommand) -> Result
.ok_or_else(|| DomainError::NotFound(format!("WatchEvent {}", c.watch_event_id)))?; .ok_or_else(|| DomainError::NotFound(format!("WatchEvent {}", c.watch_event_id)))?;
if event.user_id() != &user_id { if event.user_id() != &user_id {
return Err(DomainError::Unauthorized("not your watch event".into())); return Err(DomainError::Forbidden("not your watch event".into()));
} }
let input = if let Some(movie_id) = event.movie_id() { let input = if let Some(movie_id) = event.movie_id() {

View File

@@ -27,7 +27,7 @@ pub async fn execute(ctx: &AppContext, cmd: DismissWatchEventsCommand) -> Result
} }
for event in &events { for event in &events {
if event.user_id() != &user_id { if event.user_id() != &user_id {
return Err(DomainError::Unauthorized("not your watch event".into())); return Err(DomainError::Forbidden("not your watch event".into()));
} }
} }

View File

@@ -16,4 +16,7 @@ pub enum DomainError {
#[error("Unauthorized: {0}")] #[error("Unauthorized: {0}")]
Unauthorized(String), Unauthorized(String),
#[error("Forbidden: {0}")]
Forbidden(String),
} }

View File

@@ -4,6 +4,28 @@ use axum::{
}; };
use domain::errors::DomainError; use domain::errors::DomainError;
pub fn domain_error_status(e: &DomainError) -> StatusCode {
match e {
DomainError::InvalidRating { .. } | DomainError::ValidationError(_) => {
StatusCode::BAD_REQUEST
}
DomainError::NotFound(_) => StatusCode::NOT_FOUND,
DomainError::Unauthorized(_) => StatusCode::UNAUTHORIZED,
DomainError::Forbidden(_) => StatusCode::FORBIDDEN,
DomainError::InfrastructureError(_) => StatusCode::INTERNAL_SERVER_ERROR,
}
}
pub fn domain_error_response(e: DomainError) -> Response {
match &e {
DomainError::InfrastructureError(_) => {
tracing::error!("Internal error: {:?}", e);
StatusCode::INTERNAL_SERVER_ERROR.into_response()
}
_ => (domain_error_status(&e), e.to_string()).into_response(),
}
}
pub struct ApiError(pub DomainError); pub struct ApiError(pub DomainError);
impl From<DomainError> for ApiError { impl From<DomainError> for ApiError {
@@ -14,20 +36,6 @@ impl From<DomainError> for ApiError {
impl IntoResponse for ApiError { impl IntoResponse for ApiError {
fn into_response(self) -> Response { fn into_response(self) -> Response {
let (status, error_message) = match self.0 { domain_error_response(self.0)
DomainError::InvalidRating { .. } => (StatusCode::BAD_REQUEST, self.0.to_string()),
DomainError::ValidationError(msg) => (StatusCode::BAD_REQUEST, msg),
DomainError::NotFound(msg) => (StatusCode::NOT_FOUND, msg),
DomainError::Unauthorized(msg) => (StatusCode::UNAUTHORIZED, msg),
DomainError::InfrastructureError(_) => {
tracing::error!("Internal Infrastructure Error: {:?}", self.0);
(
StatusCode::INTERNAL_SERVER_ERROR,
"Internal server error".to_string(),
)
}
};
(status, error_message).into_response()
} }
} }

View File

@@ -37,7 +37,6 @@ use application::{
}, },
}; };
use domain::{ use domain::{
errors::DomainError,
models::{ExportFormat, PersonId, collections::PageParams}, models::{ExportFormat, PersonId, collections::PageParams},
services::review_history::Trend, services::review_history::Trend,
value_objects::UserId, value_objects::UserId,
@@ -257,20 +256,13 @@ pub async fn delete_review(
State(state): State<AppState>, State(state): State<AppState>,
AuthenticatedUser(user_id): AuthenticatedUser, AuthenticatedUser(user_id): AuthenticatedUser,
Path(review_id): Path<Uuid>, Path(review_id): Path<Uuid>,
) -> impl IntoResponse { ) -> Result<StatusCode, ApiError> {
let cmd = DeleteReviewCommand { let cmd = DeleteReviewCommand {
review_id, review_id,
requesting_user_id: user_id.value(), requesting_user_id: user_id.value(),
}; };
match delete_review::execute(&state.app_ctx, cmd).await { delete_review::execute(&state.app_ctx, cmd).await?;
Ok(()) => StatusCode::NO_CONTENT.into_response(), Ok(StatusCode::NO_CONTENT)
Err(DomainError::NotFound(_)) => StatusCode::NOT_FOUND.into_response(),
Err(DomainError::Unauthorized(_)) => StatusCode::FORBIDDEN.into_response(),
Err(e) => {
tracing::error!("delete_review error: {:?}", e);
StatusCode::INTERNAL_SERVER_ERROR.into_response()
}
}
} }
#[utoipa::path( #[utoipa::path(
@@ -395,10 +387,7 @@ pub async fn get_movie_profile(
}) })
.into_response(), .into_response(),
Ok(None) => StatusCode::NOT_FOUND.into_response(), Ok(None) => StatusCode::NOT_FOUND.into_response(),
Err(e) => { Err(e) => crate::errors::domain_error_response(e),
tracing::error!("get_movie_profile: {:?}", e);
StatusCode::INTERNAL_SERVER_ERROR.into_response()
}
} }
} }
@@ -414,27 +403,19 @@ pub async fn get_movie_profile(
pub async fn get_profile( pub async fn get_profile(
State(state): State<AppState>, State(state): State<AppState>,
AuthenticatedUser(user_id): AuthenticatedUser, AuthenticatedUser(user_id): AuthenticatedUser,
) -> impl IntoResponse { ) -> Result<Json<ProfileResponse>, ApiError> {
match application::users::get_current_profile::execute( let profile = application::users::get_current_profile::execute(
&state.app_ctx, &state.app_ctx,
application::users::queries::GetCurrentProfileQuery { application::users::queries::GetCurrentProfileQuery {
user_id: user_id.value(), user_id: user_id.value(),
}, },
) )
.await .await?;
{ Ok(Json(ProfileResponse {
Ok(profile) => Json(ProfileResponse {
username: profile.username, username: profile.username,
bio: profile.bio, bio: profile.bio,
avatar_url: profile.avatar_url, avatar_url: profile.avatar_url,
}) }))
.into_response(),
Err(DomainError::NotFound(_)) => StatusCode::NOT_FOUND.into_response(),
Err(e) => {
tracing::error!("get_profile error: {:?}", e);
StatusCode::INTERNAL_SERVER_ERROR.into_response()
}
}
} }
#[utoipa::path( #[utoipa::path(
@@ -513,14 +494,7 @@ pub async fn update_profile_handler(
match update_profile::execute(&state.app_ctx, cmd).await { match update_profile::execute(&state.app_ctx, cmd).await {
Ok(()) => StatusCode::NO_CONTENT.into_response(), Ok(()) => StatusCode::NO_CONTENT.into_response(),
Err(domain::errors::DomainError::ValidationError(msg)) => { Err(e) => crate::errors::domain_error_response(e),
tracing::warn!("update_profile validation: {}", msg);
StatusCode::BAD_REQUEST.into_response()
}
Err(e) => {
tracing::error!("update_profile error: {:?}", e);
StatusCode::INTERNAL_SERVER_ERROR.into_response()
}
} }
} }
@@ -561,13 +535,7 @@ pub async fn update_profile_fields_handler(
match update_profile_fields::execute(&state.app_ctx, cmd).await { match update_profile_fields::execute(&state.app_ctx, cmd).await {
Ok(()) => StatusCode::NO_CONTENT.into_response(), Ok(()) => StatusCode::NO_CONTENT.into_response(),
Err(domain::errors::DomainError::ValidationError(msg)) => { Err(e) => crate::errors::domain_error_response(e),
(StatusCode::BAD_REQUEST, msg).into_response()
}
Err(e) => {
tracing::error!("update_profile_fields error: {:?}", e);
StatusCode::INTERNAL_SERVER_ERROR.into_response()
}
} }
} }
@@ -1034,8 +1002,7 @@ pub async fn get_user_profile(
Ok(Some(u)) => u, Ok(Some(u)) => u,
Ok(None) => return StatusCode::NOT_FOUND.into_response(), Ok(None) => return StatusCode::NOT_FOUND.into_response(),
Err(e) => { Err(e) => {
tracing::error!("user lookup: {:?}", e); return crate::errors::domain_error_response(e);
return StatusCode::INTERNAL_SERVER_ERROR.into_response();
} }
}; };
@@ -1054,10 +1021,7 @@ pub async fn get_user_profile(
.await .await
{ {
Ok(p) => p, Ok(p) => p,
Err(e) => { Err(e) => return crate::errors::domain_error_response(e),
tracing::error!("profile: {:?}", e);
return StatusCode::INTERNAL_SERVER_ERROR.into_response();
}
}; };
let entries = profile.entries.map(|p| DiaryResponse { let entries = profile.entries.map(|p| DiaryResponse {
@@ -1236,10 +1200,7 @@ pub async fn get_search(
}, },
}) })
.into_response(), .into_response(),
Err(e) => { Err(e) => crate::errors::domain_error_response(e),
tracing::error!("search failed: {e}");
StatusCode::INTERNAL_SERVER_ERROR.into_response()
}
} }
} }
@@ -1266,10 +1227,7 @@ pub async fn get_person_handler(
}) })
.into_response(), .into_response(),
Ok(None) => StatusCode::NOT_FOUND.into_response(), Ok(None) => StatusCode::NOT_FOUND.into_response(),
Err(e) => { Err(e) => crate::errors::domain_error_response(e),
tracing::error!("get_person failed: {e}");
StatusCode::INTERNAL_SERVER_ERROR.into_response()
}
} }
} }
@@ -1320,11 +1278,7 @@ pub async fn get_person_credits_handler(
.collect(), .collect(),
}) })
.into_response(), .into_response(),
Err(DomainError::NotFound(_)) => StatusCode::NOT_FOUND.into_response(), Err(e) => crate::errors::domain_error_response(e),
Err(e) => {
tracing::error!("get_person_credits failed: {e}");
StatusCode::INTERNAL_SERVER_ERROR.into_response()
}
} }
} }

View File

@@ -301,12 +301,7 @@ pub async fn post_delete_review(
.unwrap_or_else(|| "/".to_string()); .unwrap_or_else(|| "/".to_string());
Redirect::to(&redirect_url).into_response() Redirect::to(&redirect_url).into_response()
} }
Err(DomainError::NotFound(_)) => StatusCode::NOT_FOUND.into_response(), Err(e) => crate::errors::domain_error_response(e),
Err(DomainError::Unauthorized(_)) => StatusCode::FORBIDDEN.into_response(),
Err(e) => {
tracing::error!("delete_review html error: {:?}", e);
StatusCode::INTERNAL_SERVER_ERROR.into_response()
}
} }
} }
@@ -341,11 +336,7 @@ pub async fn get_export(
bytes, bytes,
) )
.into_response(), .into_response(),
Err(DomainError::Unauthorized(_)) => StatusCode::FORBIDDEN.into_response(), Err(e) => crate::errors::domain_error_response(e),
Err(e) => {
tracing::error!("export error: {:?}", e);
StatusCode::INTERNAL_SERVER_ERROR.into_response()
}
} }
} }
@@ -408,7 +399,7 @@ pub async fn get_activity_feed(
}) })
.into_response() .into_response()
} }
Err(e) => (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()).into_response(), Err(e) => crate::errors::domain_error_response(e),
} }
} }
@@ -445,7 +436,7 @@ pub async fn get_users_list(
}) })
.into_response() .into_response()
} }
Err(e) => (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()).into_response(), Err(e) => crate::errors::domain_error_response(e),
} }
} }
@@ -521,8 +512,8 @@ pub async fn get_user_profile(
.await .await
{ {
Ok(Some(u)) => u, Ok(Some(u)) => u,
Ok(None) => return (StatusCode::NOT_FOUND, "User not found").into_response(), Ok(None) => return StatusCode::NOT_FOUND.into_response(),
Err(e) => return (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()).into_response(), Err(e) => return crate::errors::domain_error_response(e),
}; };
let display_name = profile_user.username().value(); let display_name = profile_user.username().value();
@@ -650,7 +641,7 @@ pub async fn get_user_profile(
}) })
.into_response() .into_response()
} }
Err(e) => (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()).into_response(), Err(e) => crate::errors::domain_error_response(e),
} }
} }
@@ -993,12 +984,7 @@ pub async fn get_movie_detail(
) )
.await .await
{ {
Err(DomainError::NotFound(_)) => StatusCode::NOT_FOUND.into_response(), Err(e) => crate::errors::domain_error_response(e),
Err(DomainError::ValidationError(_)) => StatusCode::BAD_REQUEST.into_response(),
Err(e) => {
tracing::error!("movie detail error: {:?}", e);
StatusCode::INTERNAL_SERVER_ERROR.into_response()
}
Ok(result) => { Ok(result) => {
let histogram_max = result let histogram_max = result
.stats .stats
@@ -1062,10 +1048,7 @@ pub async fn get_watchlist_page(
.await .await
{ {
Ok(r) => r, Ok(r) => r,
Err(e) => { Err(e) => return crate::errors::domain_error_response(e),
tracing::error!("watchlist error: {:?}", e);
return StatusCode::INTERNAL_SERVER_ERROR.into_response();
}
}; };
render_page(WatchlistTemplate { render_page(WatchlistTemplate {
@@ -1151,10 +1134,7 @@ pub async fn post_watchlist_add(
let url = format!("{}{}error={}", redirect_base, sep, encode_error(&msg)); let url = format!("{}{}error={}", redirect_base, sep, encode_error(&msg));
Redirect::to(&url).into_response() Redirect::to(&url).into_response()
} }
Err(e) => { Err(e) => crate::errors::domain_error_response(e),
tracing::error!("watchlist add error: {:?}", e);
StatusCode::INTERNAL_SERVER_ERROR.into_response()
}
} }
} }
@@ -1184,10 +1164,7 @@ pub async fn post_watchlist_remove(
.unwrap_or_else(|| "/".to_string()); .unwrap_or_else(|| "/".to_string());
Redirect::to(&redirect_url).into_response() Redirect::to(&redirect_url).into_response()
} }
Err(e) => { Err(e) => crate::errors::domain_error_response(e),
tracing::error!("watchlist remove error: {:?}", e);
StatusCode::INTERNAL_SERVER_ERROR.into_response()
}
} }
} }
@@ -1209,10 +1186,7 @@ pub async fn get_profile_settings(
let user = match state.app_ctx.repos.user.find_by_id(&user_id).await { let user = match state.app_ctx.repos.user.find_by_id(&user_id).await {
Ok(Some(u)) => u, Ok(Some(u)) => u,
Ok(None) => return StatusCode::NOT_FOUND.into_response(), Ok(None) => return StatusCode::NOT_FOUND.into_response(),
Err(e) => { Err(e) => return crate::errors::domain_error_response(e),
tracing::error!("get_profile_settings user lookup: {:?}", e);
return StatusCode::INTERNAL_SERVER_ERROR.into_response();
}
}; };
let base_url = &state.app_ctx.config.base_url; let base_url = &state.app_ctx.config.base_url;

View File

@@ -128,12 +128,7 @@ async fn run_ingest(
) -> StatusCode { ) -> StatusCode {
match ingest_watch_event::execute(&state.app_ctx, cmd, parser).await { match ingest_watch_event::execute(&state.app_ctx, cmd, parser).await {
Ok(()) => StatusCode::OK, Ok(()) => StatusCode::OK,
Err(domain::errors::DomainError::Unauthorized(_)) => StatusCode::UNAUTHORIZED, Err(e) => crate::errors::domain_error_status(&e),
Err(domain::errors::DomainError::ValidationError(_)) => StatusCode::BAD_REQUEST,
Err(e) => {
tracing::error!("webhook ingestion failed: {e:?}");
StatusCode::INTERNAL_SERVER_ERROR
}
} }
} }