fix: return 409 Conflict instead of 500 when creating a duplicate admin or realm

CHANGELOG/fix_admin-realm-create-conflict.md

@@ -0,0 +1,3 @@
+## Bug Fixes
+
+- `POST /admins` and `POST /admins/realms` used to leak a raw `500` with database internals when creating an admin or realm whose ID already existed; they now return a clean `409 Conflict`, matching the fix already applied to `POST /realms/{realm_id}/userpass`.

server/src/database/impls/mysql.rs

@@ -252,7 +252,13 @@ impl Database for MySqlDatabase {
         .bind(realm.session_max_stale_age_seconds)
         .bind(realm.certificate_max_age_seconds)
         .execute(&self.pool)
-        .await?;
+        .await
+        .map_err(|e| {
+            crate::database::AuthDbError::from_insert_error(
+                e,
+                format!("realm '{}' already exists", realm.id),
+            )
+        })?;
 
         Ok(())
     }
@@ -693,7 +699,13 @@ impl Database for MySqlDatabase {
         .bind(&admin.totp_secret)
         .bind(&admin.totp_auth_url)
         .execute(&self.pool)
-        .await?;
+        .await
+        .map_err(|e| {
+            crate::database::AuthDbError::from_insert_error(
+                e,
+                format!("admin '{}' already exists", admin.id),
+            )
+        })?;
 
         // Insert into user_realms join table
         for realm_id in &admin.realms {

server/src/database/impls/postgres.rs

@@ -216,7 +216,13 @@ impl Database for PostgresDatabase {
         .bind(realm.session_max_stale_age_seconds)
         .bind(realm.certificate_max_age_seconds)
         .execute(&self.pool)
-        .await?;
+        .await
+        .map_err(|e| {
+            crate::database::AuthDbError::from_insert_error(
+                e,
+                format!("realm '{}' already exists", realm.id),
+            )
+        })?;
 
         Ok(())
     }
@@ -657,7 +663,13 @@ impl Database for PostgresDatabase {
         .bind(&admin.totp_secret)
         .bind(&admin.totp_auth_url)
         .execute(&self.pool)
-        .await?;
+        .await
+        .map_err(|e| {
+            crate::database::AuthDbError::from_insert_error(
+                e,
+                format!("admin '{}' already exists", admin.id),
+            )
+        })?;
 
         // Insert into user_realms join table
         for realm_id in &admin.realms {

server/src/database/impls/sqlite.rs

@@ -264,7 +264,13 @@ impl Database for SqliteDatabase {
         .bind(realm.session_max_stale_age_seconds)
         .bind(realm.certificate_max_age_seconds)
         .execute(&self.pool)
-        .await?;
+        .await
+        .map_err(|e| {
+            crate::database::AuthDbError::from_insert_error(
+                e,
+                format!("realm '{}' already exists", realm.id),
+            )
+        })?;
 
         Ok(())
     }
@@ -705,7 +711,13 @@ impl Database for SqliteDatabase {
         .bind(&admin.totp_secret)
         .bind(&admin.totp_auth_url)
         .execute(&self.pool)
-        .await?;
+        .await
+        .map_err(|e| {
+            crate::database::AuthDbError::from_insert_error(
+                e,
+                format!("admin '{}' already exists", admin.id),
+            )
+        })?;
 
         // Insert into user_realms join table
         for realm_id in &admin.realms {

server/src/tests/admin_api.rs

@@ -7,7 +7,7 @@
 //! 3. Exercises one scenario against the `/users` scope.
 
 use crate::{
-    AuthResult, AuthenticationNextStep, Realm, RealmAuthParams,
+    AuthError, AuthResult, AuthenticationNextStep, Realm, RealmAuthParams,
     client::AuthClientScheme,
     database::APP_REALM_ADMIN_USERNAME,
     models::ADMIN_REALM,
@@ -75,9 +75,10 @@ async fn test_create_admin() -> AuthResult<()> {
     ctx.stop_server().await
 }
 
-/// Creating a user whose ID already exists must return an error.
+/// Creating an admin whose ID already exists must return a clean `409 Conflict`,
+/// not a `500` leaking the underlying database error.
 #[actix_web::test]
-async fn test_create_duplicate_user_fails() -> AuthResult<()> {
+async fn test_create_duplicate_admin_fails() -> AuthResult<()> {
     init_test_logging(None);
     let ctx = start_default_test_server().await?;
     let client = authenticate_as_admin(&ctx).await?;
@@ -87,14 +88,12 @@ async fn test_create_duplicate_user_fails() -> AuthResult<()> {
         .create_admin_as_super_admin(&test_admin(APP_REALM_ADMIN_USERNAME))
         .await;
 
+    let err = result.expect_err("Expected an error when creating a duplicate admin");
     assert!(
-        result.is_err(),
-        "Expected an error when creating a duplicate user"
-    );
-    info!(
-        "create_duplicate_user returned expected error: {:?}",
-        result
+        matches!(err, AuthError::FailedHttpStatus(ref m) if m.contains("409")),
+        "Expected a 409 Conflict, got: {err:?}"
     );
+    info!("create_duplicate_admin returned expected error: {err:?}");
 
     ctx.stop_server().await
 }

server/src/tests/super_admin_api.rs

@@ -8,7 +8,7 @@
 //!    automatically sent on every call.
 
 use crate::{
-    AuthResult,
+    AuthError, AuthResult,
     models::ADMIN_REALM,
     tests::{
         helpers::{
@@ -180,6 +180,29 @@ async fn test_create_realm() -> AuthResult<()> {
     ctx.stop_server().await
 }
 
+/// Creating a realm whose ID already exists must return a clean `409 Conflict`,
+/// not a `500` leaking the underlying database error.
+#[actix_web::test]
+async fn test_create_duplicate_realm_fails() -> AuthResult<()> {
+    init_test_logging(None);
+    let ctx = start_default_test_server().await?;
+    let client = authenticate_as_admin(&ctx).await?;
+
+    // The `_` admin realm is always seeded — trying to create it again must fail.
+    let result = client
+        .create_realm_as_super_admin(&test_realm(ADMIN_REALM))
+        .await;
+
+    let err = result.expect_err("Expected an error when creating a duplicate realm");
+    assert!(
+        matches!(err, AuthError::FailedHttpStatus(ref m) if m.contains("409")),
+        "Expected a 409 Conflict, got: {err:?}"
+    );
+    info!("create_duplicate_realm returned expected error: {err:?}");
+
+    ctx.stop_server().await
+}
+
 // ── Authorization enforcement ────────────────────────────────────────────────
 
 /// A realm admin (non-super-admin) must not be able to update any realm (HTTP 403).
@@ -372,6 +395,40 @@ async fn test_userpass_crud_by_super_admin() -> AuthResult<()> {
     ctx.stop_server().await
 }
 
+/// Re-provisioning credentials for an existing `(realm, username)` pair must
+/// return a clean `409 Conflict`, not a `500` leaking the underlying database
+/// error — and must not double as a password oracle by distinguishing a
+/// byte-for-byte resubmission from a genuine conflict.
+#[actix_web::test]
+async fn test_create_duplicate_userpass_fails() -> AuthResult<()> {
+    init_test_logging(None);
+    let ctx = start_default_test_server().await?;
+    let client = authenticate_as_admin(&ctx).await?;
+
+    let userpass = create_user(
+        ADMIN_REALM,
+        "duplicate_userpass_test",
+        "initial_pass",
+        false,
+    )?;
+    client
+        .create_admin_credentials_in_realm(ADMIN_REALM, &userpass)
+        .await?;
+
+    let result = client
+        .create_admin_credentials_in_realm(ADMIN_REALM, &userpass)
+        .await;
+
+    let err = result.expect_err("Expected an error when re-provisioning existing credentials");
+    assert!(
+        matches!(err, AuthError::FailedHttpStatus(ref m) if m.contains("409")),
+        "Expected a 409 Conflict, got: {err:?}"
+    );
+    info!("create_duplicate_userpass returned expected error: {err:?}");
+
+    ctx.stop_server().await
+}
+
 // ── list_all_userpass requires super admin ────────────────────────────────────
 
 /// `GET /admins/userpass` is in the `/admins` scope which hosts super-admin-only