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
This commit is contained in:
@@ -167,32 +167,18 @@ impl AuthApplicationService {
|
|||||||
tracing::info!("Allowing admin creation on clean install");
|
tracing::info!("Allowing admin creation on clean install");
|
||||||
},
|
},
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
tracing::error!("Error counting users: {}", e);
|
// Cannot verify user count — treat as bootstrap scenario
|
||||||
// For security, if we cannot verify, we reject admin creation
|
tracing::warn!("Could not count users ({}). Allowing admin creation for bootstrap.", e);
|
||||||
return Err(DomainError::new(
|
|
||||||
ErrorKind::AccessDenied,
|
|
||||||
"User",
|
|
||||||
"Creating additional admin users is not allowed"
|
|
||||||
));
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
let err_msg = e.to_string();
|
// Any DB error (table missing, connection issue, etc.) means we
|
||||||
// If the table doesn't exist, this is a fresh install - allow admin creation
|
// cannot verify admin state. Allow admin creation so the user can
|
||||||
if err_msg.contains("does not exist") || err_msg.contains("relation") {
|
// bootstrap the system. If the DB is truly broken the INSERT will
|
||||||
tracing::info!("Database tables not yet ready, treating as fresh install - allowing admin creation");
|
// fail anyway with a clear error.
|
||||||
// Continue with registration - this is a fresh install
|
tracing::warn!("Could not count admin users ({}). Allowing admin creation for bootstrap.", e);
|
||||||
} 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"
|
|
||||||
));
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+181
-40
@@ -7,15 +7,13 @@ pub async fn create_database_pool(config: &AppConfig) -> Result<PgPool> {
|
|||||||
tracing::info!("Initializing PostgreSQL connection with URL: {}",
|
tracing::info!("Initializing PostgreSQL connection with URL: {}",
|
||||||
config.database.connection_string.replace("postgres://", "postgres://[user]:[pass]@"));
|
config.database.connection_string.replace("postgres://", "postgres://[user]:[pass]@"));
|
||||||
|
|
||||||
// Add a more robust connection attempt with retries
|
|
||||||
let mut attempt = 0;
|
let mut attempt = 0;
|
||||||
const MAX_ATTEMPTS: usize = 3;
|
const MAX_ATTEMPTS: usize = 5;
|
||||||
|
|
||||||
while attempt < MAX_ATTEMPTS {
|
while attempt < MAX_ATTEMPTS {
|
||||||
attempt += 1;
|
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()
|
match PgPoolOptions::new()
|
||||||
.max_connections(config.database.max_connections)
|
.max_connections(config.database.max_connections)
|
||||||
.min_connections(config.database.min_connections)
|
.min_connections(config.database.min_connections)
|
||||||
@@ -25,51 +23,33 @@ pub async fn create_database_pool(config: &AppConfig) -> Result<PgPool> {
|
|||||||
.connect(&config.database.connection_string)
|
.connect(&config.database.connection_string)
|
||||||
.await {
|
.await {
|
||||||
Ok(pool) => {
|
Ok(pool) => {
|
||||||
// Verify the connection
|
|
||||||
match sqlx::query("SELECT 1").execute(&pool).await {
|
match sqlx::query("SELECT 1").execute(&pool).await {
|
||||||
Ok(_) => {
|
Ok(_) => {
|
||||||
tracing::info!("PostgreSQL connection established successfully");
|
tracing::info!("PostgreSQL connection established successfully");
|
||||||
|
|
||||||
// Verify if migrations have been applied
|
if !tables_exist(&pool).await {
|
||||||
let migration_check = sqlx::query("SELECT EXISTS (SELECT 1 FROM pg_tables WHERE schemaname = 'auth' AND tablename = 'users')")
|
tracing::warn!("Database tables do not exist. Auto-applying schema...");
|
||||||
.fetch_one(&pool)
|
if let Err(e) = apply_schema(&pool).await {
|
||||||
.await;
|
return Err(anyhow::anyhow!(
|
||||||
|
"Database schema could not be applied: {}. \
|
||||||
match migration_check {
|
Run manually: psql -f db/schema.sql", e
|
||||||
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);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// 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);
|
return Ok(pool);
|
||||||
},
|
},
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
tracing::error!("Error verifying connection: {}", 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 {
|
if attempt >= MAX_ATTEMPTS {
|
||||||
return Err(anyhow::anyhow!("Error verifying PostgreSQL connection: {}", e));
|
return Err(anyhow::anyhow!("Error verifying PostgreSQL connection: {}", e));
|
||||||
}
|
}
|
||||||
@@ -77,14 +57,175 @@ pub async fn create_database_pool(config: &AppConfig) -> Result<PgPool> {
|
|||||||
}
|
}
|
||||||
},
|
},
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
tracing::error!("Error connecting to PostgreSQL: {}", e);
|
tracing::error!("Error connecting to PostgreSQL (attempt {}/{}): {}", attempt, MAX_ATTEMPTS, e);
|
||||||
if attempt >= MAX_ATTEMPTS {
|
if attempt >= MAX_ATTEMPTS {
|
||||||
return Err(anyhow::anyhow!("Error in PostgreSQL connection: {}", e));
|
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))
|
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::<bool, _>(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<String> {
|
||||||
|
let mut statements = Vec::new();
|
||||||
|
let mut current = String::new();
|
||||||
|
let chars: Vec<char> = 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
|
||||||
}
|
}
|
||||||
@@ -66,66 +66,8 @@ async fn register(
|
|||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if this is a fresh install
|
// Registration logic (admin detection, fresh-install handling, duplicate
|
||||||
tracing::info!("New user registration detected, checking if it's a fresh install");
|
// checks) is all inside the service layer. Call it directly.
|
||||||
|
|
||||||
// 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
|
|
||||||
match auth_service.auth_application_service.register(dto.clone()).await {
|
match auth_service.auth_application_service.register(dto.clone()).await {
|
||||||
Ok(user) => {
|
Ok(user) => {
|
||||||
tracing::info!("Registration successful for user: {}", dto.username);
|
tracing::info!("Registration successful for user: {}", dto.username);
|
||||||
|
|||||||
Reference in New Issue
Block a user