From e2297d276a5fbc1d70b3bd2c826be49dc39548e5 Mon Sep 17 00:00:00 2001 From: Dionisio Date: Thu, 12 Feb 2026 23:20:46 +0100 Subject: [PATCH] fix: resolve admin registration failure on fresh Docker installs (#81) Three bugs caused 403 errors when creating the first admin on fresh Docker deployments (Unraid, Komodo): 1. db.rs: Schema application failures were silently swallowed. The app started with no tables, causing all auth queries to fail. Now the startup aborts if schema cannot be applied, with a fallback statement-by-statement executor that handles dollar-quoted blocks. Retries increased to 5 with 2s intervals. 2. auth_application_service.rs: count_admin_users() used fragile string matching (contains "does not exist")) on multi-layer wrapped errors. count_all_users() rejected admin creation on any DB error. Both now allow admin creation on any error for bootstrap scenarios. 3. auth_handler.rs: Redundant 60-line handler-level admin detection duplicated service-layer logic and generated noisy ERROR logs on fresh installs. Removed entirely - service layer handles it all. Closes #81 --- .../services/auth_application_service.rs | 28 +-- src/infrastructure/db.rs | 221 ++++++++++++++---- src/interfaces/api/handlers/auth_handler.rs | 62 +---- 3 files changed, 190 insertions(+), 121 deletions(-) diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index e2075b99..b4be1181 100644 --- a/src/application/services/auth_application_service.rs +++ b/src/application/services/auth_application_service.rs @@ -167,32 +167,18 @@ impl AuthApplicationService { tracing::info!("Allowing admin creation on clean install"); }, Err(e) => { - tracing::error!("Error counting users: {}", e); - // For security, if we cannot verify, we reject admin creation - return Err(DomainError::new( - ErrorKind::AccessDenied, - "User", - "Creating additional admin users is not allowed" - )); + // Cannot verify user count — treat as bootstrap scenario + tracing::warn!("Could not count users ({}). Allowing admin creation for bootstrap.", e); } } } }, Err(e) => { - let err_msg = e.to_string(); - // If the table doesn't exist, this is a fresh install - allow admin creation - if err_msg.contains("does not exist") || err_msg.contains("relation") { - tracing::info!("Database tables not yet ready, treating as fresh install - allowing admin creation"); - // Continue with registration - this is a fresh install - } else { - tracing::error!("Error counting admin users: {}", e); - // For security, if we cannot verify, we reject admin creation - return Err(DomainError::new( - ErrorKind::AccessDenied, - "User", - "Creating additional admin users is not allowed" - )); - } + // Any DB error (table missing, connection issue, etc.) means we + // cannot verify admin state. Allow admin creation so the user can + // bootstrap the system. If the DB is truly broken the INSERT will + // fail anyway with a clear error. + tracing::warn!("Could not count admin users ({}). Allowing admin creation for bootstrap.", e); } } } diff --git a/src/infrastructure/db.rs b/src/infrastructure/db.rs index e5490c7f..4e088570 100644 --- a/src/infrastructure/db.rs +++ b/src/infrastructure/db.rs @@ -7,15 +7,13 @@ pub async fn create_database_pool(config: &AppConfig) -> Result { tracing::info!("Initializing PostgreSQL connection with URL: {}", config.database.connection_string.replace("postgres://", "postgres://[user]:[pass]@")); - // Add a more robust connection attempt with retries let mut attempt = 0; - const MAX_ATTEMPTS: usize = 3; + const MAX_ATTEMPTS: usize = 5; while attempt < MAX_ATTEMPTS { attempt += 1; - tracing::info!("PostgreSQL connection attempt #{}", attempt); + tracing::info!("PostgreSQL connection attempt #{}/{}", attempt, MAX_ATTEMPTS); - // Create the connection pool with configuration options match PgPoolOptions::new() .max_connections(config.database.max_connections) .min_connections(config.database.min_connections) @@ -25,51 +23,33 @@ pub async fn create_database_pool(config: &AppConfig) -> Result { .connect(&config.database.connection_string) .await { Ok(pool) => { - // Verify the connection match sqlx::query("SELECT 1").execute(&pool).await { Ok(_) => { tracing::info!("PostgreSQL connection established successfully"); - // Verify if migrations have been applied - let migration_check = sqlx::query("SELECT EXISTS (SELECT 1 FROM pg_tables WHERE schemaname = 'auth' AND tablename = 'users')") - .fetch_one(&pool) - .await; - - match migration_check { - Ok(row) => { - let tables_exist: bool = row.get(0); - if !tables_exist { - tracing::warn!("Database tables do not exist. Auto-applying schema..."); - let schema_sql = include_str!("../../db/schema.sql"); - match sqlx::raw_sql(schema_sql).execute(&pool).await { - Ok(_) => { - tracing::info!("Database schema applied successfully"); - }, - Err(e) => { - tracing::error!("Failed to auto-apply database schema: {}. You may need to run: psql -f db/schema.sql", e); - } - } - } - }, - Err(_) => { - tracing::warn!("Could not verify migration status. Attempting to auto-apply schema..."); - let schema_sql = include_str!("../../db/schema.sql"); - match sqlx::raw_sql(schema_sql).execute(&pool).await { - Ok(_) => { - tracing::info!("Database schema applied successfully"); - }, - Err(e) => { - tracing::error!("Failed to auto-apply database schema: {}. You may need to run: psql -f db/schema.sql", e); - } - } + if !tables_exist(&pool).await { + tracing::warn!("Database tables do not exist. Auto-applying schema..."); + if let Err(e) = apply_schema(&pool).await { + return Err(anyhow::anyhow!( + "Database schema could not be applied: {}. \ + Run manually: psql -f db/schema.sql", e + )); } + + // Verify tables were actually created + if !tables_exist(&pool).await { + return Err(anyhow::anyhow!( + "Database schema was applied but tables still missing. \ + Check db/schema.sql for errors." + )); + } + tracing::info!("Database schema applied and verified successfully"); } return Ok(pool); }, Err(e) => { tracing::error!("Error verifying connection: {}", e); - tracing::warn!("The database appears to not be configured. Please run migrations with: cargo run --bin migrate --features migrations"); if attempt >= MAX_ATTEMPTS { return Err(anyhow::anyhow!("Error verifying PostgreSQL connection: {}", e)); } @@ -77,14 +57,175 @@ pub async fn create_database_pool(config: &AppConfig) -> Result { } }, Err(e) => { - tracing::error!("Error connecting to PostgreSQL: {}", e); + tracing::error!("Error connecting to PostgreSQL (attempt {}/{}): {}", attempt, MAX_ATTEMPTS, e); if attempt >= MAX_ATTEMPTS { return Err(anyhow::anyhow!("Error in PostgreSQL connection: {}", e)); } - tokio::time::sleep(Duration::from_secs(1)).await; + tokio::time::sleep(Duration::from_secs(2)).await; } } } Err(anyhow::anyhow!("Could not establish PostgreSQL connection after {} attempts", MAX_ATTEMPTS)) +} + +/// Check whether the core auth tables exist in the database. +async fn tables_exist(pool: &PgPool) -> bool { + sqlx::query("SELECT EXISTS (SELECT 1 FROM pg_tables WHERE schemaname = 'auth' AND tablename = 'users')") + .fetch_one(pool) + .await + .and_then(|row| Ok(row.get::(0))) + .unwrap_or(false) +} + +/// Apply the embedded schema.sql to the database. +/// First tries `raw_sql` (simple query protocol). If that fails, falls back +/// to splitting the SQL into individual statements and executing them one by one. +async fn apply_schema(pool: &PgPool) -> Result<()> { + let schema_sql = include_str!("../../db/schema.sql"); + + // Attempt 1: raw_sql sends the entire script via the simple query protocol + match sqlx::raw_sql(schema_sql).execute(pool).await { + Ok(_) => return Ok(()), + Err(e) => { + tracing::warn!("raw_sql failed ({}), falling back to statement-by-statement execution", e); + } + } + + // Attempt 2: split into individual statements respecting dollar-quoting + let statements = split_sql_statements(schema_sql); + for (i, stmt) in statements.iter().enumerate() { + let trimmed = stmt.trim(); + if trimmed.is_empty() || trimmed == ";" { + continue; + } + if let Err(e) = sqlx::raw_sql(trimmed).execute(pool).await { + let preview = if trimmed.len() > 200 { &trimmed[..200] } else { trimmed }; + tracing::error!("Schema statement {} failed: {}\n--- SQL ---\n{}\n-----------", i + 1, e, preview); + return Err(anyhow::anyhow!("Schema statement {} failed: {}", i + 1, e)); + } + } + + Ok(()) +} + +/// Split a SQL script into individual statements, correctly handling: +/// - Dollar-quoted blocks (`$BODY$...$BODY$`, `$$...$$`) +/// - Single-quoted strings (`'...'`) +/// - Line comments (`-- ...`) +/// - Block comments (`/* ... */`) +fn split_sql_statements(sql: &str) -> Vec { + let mut statements = Vec::new(); + let mut current = String::new(); + let chars: Vec = sql.chars().collect(); + let len = chars.len(); + let mut i = 0; + + while i < len { + // Line comment + if i + 1 < len && chars[i] == '-' && chars[i + 1] == '-' { + while i < len && chars[i] != '\n' { + current.push(chars[i]); + i += 1; + } + continue; + } + + // Block comment + if i + 1 < len && chars[i] == '/' && chars[i + 1] == '*' { + current.push(chars[i]); + current.push(chars[i + 1]); + i += 2; + while i + 1 < len && !(chars[i] == '*' && chars[i + 1] == '/') { + current.push(chars[i]); + i += 1; + } + if i + 1 < len { + current.push(chars[i]); + current.push(chars[i + 1]); + i += 2; + } + continue; + } + + // Single-quoted string + if chars[i] == '\'' { + current.push(chars[i]); + i += 1; + while i < len { + current.push(chars[i]); + if chars[i] == '\'' { + if i + 1 < len && chars[i + 1] == '\'' { + current.push(chars[i + 1]); + i += 2; + } else { + i += 1; + break; + } + } else { + i += 1; + } + } + continue; + } + + // Dollar-quoted string ($tag$...$tag$ or $$...$$) + if chars[i] == '$' { + let _start = i; + i += 1; + let mut tag = String::from("$"); + while i < len && (chars[i].is_alphanumeric() || chars[i] == '_') { + tag.push(chars[i]); + i += 1; + } + if i < len && chars[i] == '$' { + tag.push('$'); + i += 1; + // We have a dollar-quote tag, find the closing tag + current.push_str(&tag); + loop { + if i >= len { + break; + } + if chars[i] == '$' { + let remaining = &sql[i..]; + if remaining.starts_with(&tag) { + current.push_str(&tag); + i += tag.len(); + break; + } + } + current.push(chars[i]); + i += 1; + } + } else { + // Not a valid dollar-quote, push what we consumed + current.push_str(&tag); + } + continue; + } + + // Statement separator + if chars[i] == ';' { + current.push(';'); + let trimmed = current.trim().to_string(); + if !trimmed.is_empty() && trimmed != ";" { + statements.push(trimmed); + } + current.clear(); + i += 1; + continue; + } + + current.push(chars[i]); + i += 1; + } + + // Trailing statement without semicolon + let trimmed = current.trim().to_string(); + if !trimmed.is_empty() && trimmed != ";" { + statements.push(trimmed); + } + + statements } \ No newline at end of file diff --git a/src/interfaces/api/handlers/auth_handler.rs b/src/interfaces/api/handlers/auth_handler.rs index 00da24b2..c7f4df65 100644 --- a/src/interfaces/api/handlers/auth_handler.rs +++ b/src/interfaces/api/handlers/auth_handler.rs @@ -66,66 +66,8 @@ async fn register( )); } - // Check if this is a fresh install - tracing::info!("New user registration detected, checking if it's a fresh install"); - - // Detect if we're in a fresh install with just the default admin user - match auth_service.auth_application_service.count_admin_users().await { - Ok(admin_count) => { - // If we have exactly one admin user (the default one from migrations) - if admin_count == 1 { - tracing::info!("Found one admin user - checking if it's the default admin"); - - // Verify it's truly a fresh install by counting all users - match auth_service.auth_application_service.count_all_users().await { - Ok(user_count) => { - // In a fresh install with only the default admin (and possibly test user) - if user_count <= 2 { // Allow for admin + test user from migrations - tracing::info!("This appears to be a fresh install with just default users"); - - // Check if the user is trying to create an admin user (via role field or username) - let is_admin_registration = - dto.username.to_lowercase() == "admin" || - (dto.role.is_some() && dto.role.as_ref().unwrap().to_lowercase() == "admin"); - - // If we're registering an admin user in a fresh install - if is_admin_registration { - tracing::info!("Admin user registration detected in fresh install"); - - // Remove the default admin user and create the new customized one - match auth_service.auth_application_service.delete_default_admin().await { - Ok(_) => { - tracing::info!("Successfully deleted default admin"); - - // Proceed with normal registration (now that default admin is removed) - // Normal registration will continue below - }, - Err(err) => { - tracing::error!("Failed to delete default admin: {}", err); - // Continue anyway - worst case we'll get an error during registration - // if there's a username conflict - } - } - } else { - // Non-admin user registration in fresh install, proceed normally - tracing::info!("Regular user registration in fresh install, proceeding normally"); - } - } - }, - Err(err) => { - tracing::error!("Error counting users: {}", err); - // Not critical, continue with registration - } - } - } - }, - Err(err) => { - tracing::error!("Error counting admin users: {}", err); - // Not critical, continue with registration - } - } - - // Try the normal registration process + // Registration logic (admin detection, fresh-install handling, duplicate + // checks) is all inside the service layer. Call it directly. match auth_service.auth_application_service.register(dto.clone()).await { Ok(user) => { tracing::info!("Registration successful for user: {}", dto.username);