From 15b1d0fdb7a480bfcbbecd461375a1a548fd7cc0 Mon Sep 17 00:00:00 2001 From: Gabriel Kaszewski Date: Fri, 15 May 2026 17:03:48 +0200 Subject: [PATCH] =?UTF-8?q?fix(db):=20map=2023505=20unique=5Fviolation=20t?= =?UTF-8?q?o=20DomainError::Conflict=20(=E2=86=92=20HTTP=20409);=20close?= =?UTF-8?q?=20TOCTOU=20on=20register=20save?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/adapters/postgres/src/db_error.rs | 11 +++- crates/application/src/use_cases/auth.rs | 81 +++++++++++++++++++++++- 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/crates/adapters/postgres/src/db_error.rs b/crates/adapters/postgres/src/db_error.rs index 74bb95f..b30bf44 100644 --- a/crates/adapters/postgres/src/db_error.rs +++ b/crates/adapters/postgres/src/db_error.rs @@ -6,6 +6,15 @@ pub(crate) trait IntoDbResult { impl IntoDbResult for Result { fn into_domain(self) -> Result { - self.map_err(|e| DomainError::Internal(e.to_string())) + self.map_err(|e| { + if let sqlx::Error::Database(ref db) = e { + if db.code().as_deref() == Some("23505") { + return DomainError::Conflict( + db.constraint().unwrap_or("conflict").to_string(), + ); + } + } + DomainError::Internal(e.to_string()) + }) } } diff --git a/crates/application/src/use_cases/auth.rs b/crates/application/src/use_cases/auth.rs index 5ec22c2..c375daf 100644 --- a/crates/application/src/use_cases/auth.rs +++ b/crates/application/src/use_cases/auth.rs @@ -34,7 +34,19 @@ pub async fn register( } let hash = hasher.hash(&input.password).await?; let user = User::new_local(UserId::new(), username, email, hash); - users.save(&user).await?; + users + .save(&user) + .await + .map_err(|e| match e { + DomainError::Conflict(ref c) if c.contains("username") => { + DomainError::Conflict("username taken".into()) + } + DomainError::Conflict(ref c) if c.contains("email") => { + DomainError::Conflict("email taken".into()) + } + DomainError::Conflict(_) => DomainError::Conflict("already exists".into()), + other => other, + })?; events .publish(&DomainEvent::UserRegistered { user_id: user.id.clone(), @@ -90,11 +102,58 @@ mod tests { use domain::{ errors::DomainError, events::DomainEvent, - ports::{AuthService, GeneratedToken, PasswordHasher}, + models::{feed::UserSummary, user::User}, + ports::{AuthService, GeneratedToken, PasswordHasher, UserReader, UserWriter}, testing::{NoOpEventPublisher, TestStore}, - value_objects::{PasswordHash, UserId}, + value_objects::{Email, PasswordHash, UserId, Username}, }; + /// Simulates a concurrent registration that slips past the pre-checks and + /// hits the DB unique constraint — exactly what happens in the TOCTOU window. + struct ConflictOnSaveStore(TestStore); + + #[async_trait] + impl UserReader for ConflictOnSaveStore { + async fn find_by_id(&self, id: &UserId) -> Result, DomainError> { + self.0.find_by_id(id).await + } + async fn find_by_username( + &self, + username: &Username, + ) -> Result, DomainError> { + self.0.find_by_username(username).await + } + async fn find_by_email(&self, email: &Email) -> Result, DomainError> { + self.0.find_by_email(email).await + } + async fn list_with_stats(&self) -> Result, DomainError> { + self.0.list_with_stats().await + } + async fn count(&self) -> Result { + self.0.count().await + } + } + + #[async_trait] + impl UserWriter for ConflictOnSaveStore { + async fn save(&self, _user: &User) -> Result<(), DomainError> { + Err(DomainError::Conflict("users_username_key".into())) + } + async fn update_profile( + &self, + user_id: &UserId, + display_name: Option, + bio: Option, + avatar_url: Option, + header_url: Option, + custom_css: Option, + ) -> Result<(), DomainError> { + self.0 + .update_profile(user_id, display_name, bio, avatar_url, header_url, custom_css) + .await + } + } + struct FakeHasher; #[async_trait] impl PasswordHasher for FakeHasher { @@ -240,4 +299,20 @@ mod tests { .unwrap_err(); assert!(matches!(err, DomainError::Conflict(_))); } + + /// TOCTOU: a concurrent registration slips past the pre-checks and the DB + /// unique constraint fires on save. The map_err must convert it to a + /// human-readable Conflict, not bubble up a raw constraint name. + #[tokio::test] + async fn register_maps_db_conflict_on_username_to_conflict() { + let store = ConflictOnSaveStore(TestStore::default()); + let err = register(&store, &FakeHasher, &FakeAuth, &NoOpEventPublisher, input()) + .await + .unwrap_err(); + assert!( + matches!(err, DomainError::Conflict(ref m) if m == "username taken"), + "expected 'username taken', got: {:?}", + err + ); + } }