Merge pull request #198 from NodeDB-Lab/fix/trust-superuser-identity

fix(auth): materialize trust superuser identity

Author: farhan-syah

nodedb-test-support/src/pgwire_harness/multicore.rs

@@ -13,7 +13,6 @@ use nodedb::config::auth::AuthMode;
 use nodedb::control::server::pgwire::listener::PgListener;
 use nodedb::control::state::SharedState;
 use nodedb::event::{EventPlane, create_event_bus};
-use nodedb::types::TenantId;
 use nodedb::wal::WalManager;
 
 use super::support::{bind_http_listener, bind_native_listener, init_test_memory_governor};
@@ -41,12 +40,9 @@ impl TestServer {
             nodedb::control::security::credential::store::CredentialStore::open(&catalog_path)
                 .unwrap(),
         );
-        let _ = credentials.create_user(
-            "nodedb",
-            "nodedb",
-            TenantId::new(1),
-            vec![nodedb::control::security::identity::Role::Superuser],
-        );
+        credentials
+            .bootstrap_trust_superuser("nodedb")
+            .expect("bootstrap trust superuser");
         let mut shared =
             SharedState::new_with_credentials(dispatcher, Arc::clone(&wal), credentials)
                 .expect("build shared state");

nodedb-test-support/src/pgwire_harness/restart.rs

@@ -13,7 +13,6 @@ use nodedb::config::auth::AuthMode;
 use nodedb::control::server::pgwire::listener::PgListener;
 use nodedb::control::state::SharedState;
 use nodedb::event::{EventPlane, create_event_bus};
-use nodedb::types::TenantId;
 use nodedb::wal::WalManager;
 
 use super::support::{bind_http_listener, bind_native_listener, init_test_memory_governor};
@@ -121,6 +120,15 @@ impl TestServer {
         (server, data_dir)
     }
 
+    /// Reopen a credential store in trust mode without provisioning the normal
+    /// harness superuser. The returned server has no preconnected harness client;
+    /// callers can exercise the production bootstrap path explicitly.
+    pub async fn open_on_path_empty_store_trust(dir: TestDataDir) -> (Self, TestDataDir) {
+        let data_dir = TestDataDir(dir.0);
+        let server = Self::start_on_dir_ref(data_dir.path(), None, AuthMode::Trust, false).await;
+        (server, data_dir)
+    }
+
     /// Reopen an empty credential store in password mode without provisioning
     /// the normal harness superuser. The returned server has no preconnected
     /// harness client; callers must open the connection under test themselves.
@@ -175,12 +183,14 @@ impl TestServer {
                 .unwrap(),
         );
         if provision_superuser {
-            let _ = credentials.create_user(
-                "nodedb",
-                "nodedb",
-                TenantId::new(1),
-                vec![nodedb::control::security::identity::Role::Superuser],
-            );
+            match auth_mode {
+                AuthMode::Trust => credentials
+                    .bootstrap_trust_superuser("nodedb")
+                    .expect("bootstrap trust superuser"),
+                _ => credentials
+                    .bootstrap_superuser("nodedb", "nodedb")
+                    .expect("bootstrap password superuser"),
+            }
         }
         let mut shared =
             SharedState::new_with_credentials(dispatcher, Arc::clone(&wal), credentials)

nodedb-test-support/src/pgwire_harness/start.rs

@@ -161,17 +161,17 @@ impl TestServer {
             credential_store.set_lockout_policy(max_failed, lockout_secs, 0);
         }
         let credentials = Arc::new(credential_store);
-        // Provision the harness superuser `nodedb` so Trust-mode strict
-        // identity resolution accepts the default test connection. The
-        // bootstrap exception in the handler only fires when the store
-        // is empty, which would break as soon as any DDL creates a user.
+        // Mirror production bootstrap so trust-mode listeners resolve a durable
+        // configured principal rather than fabricating an ephemeral identity.
         if cfg.provision_superuser {
-            let _ = credentials.create_user(
-                "nodedb",
-                "nodedb",
-                nodedb::types::TenantId::new(1),
-                vec![nodedb::control::security::identity::Role::Superuser],
-            );
+            match cfg.auth_mode {
+                AuthMode::Trust => credentials
+                    .bootstrap_trust_superuser("nodedb")
+                    .expect("bootstrap trust superuser"),
+                _ => credentials
+                    .bootstrap_superuser("nodedb", "nodedb")
+                    .expect("bootstrap password superuser"),
+            }
         }
         // Ensure the built-in `default` database (id 0) is present in the
         // catalog so `USE DATABASE default` and `\c default` work in tests.

nodedb/src/bootstrap/credentials.rs

@@ -43,36 +43,42 @@ pub fn replay_surrogate_wal(shared: &Arc<SharedState>, wal_records: &Arc<[WalRec
     }
 }
 
-/// Bootstrap the superuser credential from config / data dir, or warn about trust mode.
+/// Materialize the configured superuser before any listener starts.
 pub fn bootstrap_superuser(shared: &Arc<SharedState>, config: &ServerConfig) -> anyhow::Result<()> {
-    let auth_mode = config.auth.mode.clone();
-    match config
-        .auth
-        .resolve_superuser_password(&config.server.data_dir)
-    {
-        Ok(Some(password)) => {
+    match &config.auth.mode {
+        crate::config::auth::AuthMode::Trust => {
             shared
                 .credentials
-                .bootstrap_superuser(&config.auth.superuser_name, &password)?;
+                .bootstrap_trust_superuser(&config.auth.superuser_name)?;
             info!(
                 user = config.auth.superuser_name,
-                mode = ?auth_mode,
-                "superuser bootstrapped"
+                mode = ?config.auth.mode,
+                "trust-mode superuser bootstrapped"
             );
-        }
-        Ok(None) => {
-            // Trust mode — no credentials needed, but operators must opt in explicitly.
             warn!("╔══════════════════════════════════════════════════════════════╗");
             warn!("║  WARNING: NodeDB is running in TRUST mode.                  ║");
-            warn!("║  ALL connections are accepted WITHOUT credentials.           ║");
+            warn!("║  Password credentials are NOT verified.                    ║");
             warn!("║  This is UNSAFE for any environment beyond local dev/CI.    ║");
             warn!("║  Set auth.mode = \"password\" (or \"certificate\") to require   ║");
             warn!("║  credentials. Trust mode must be an explicit operator       ║");
             warn!("║  opt-in — it is never the NodeDB default.                   ║");
             warn!("╚══════════════════════════════════════════════════════════════╝");
         }
-        Err(e) => {
-            return Err(e.into());
+        crate::config::auth::AuthMode::Password | crate::config::auth::AuthMode::Certificate => {
+            let password = config
+                .auth
+                .resolve_superuser_password(&config.server.data_dir)?
+                .ok_or_else(|| crate::Error::Config {
+                    detail: "superuser password resolution returned no credential".into(),
+                })?;
+            shared
+                .credentials
+                .bootstrap_superuser(&config.auth.superuser_name, &password)?;
+            info!(
+                user = config.auth.superuser_name,
+                mode = ?config.auth.mode,
+                "superuser bootstrapped"
+            );
         }
     }
     Ok(())

nodedb/src/control/security/catalog/system_catalog.rs

@@ -20,6 +20,8 @@ use super::types::*;
 #[derive(Clone)]
 pub struct SystemCatalog {
     pub(super) db: Arc<Database>,
+    #[cfg(test)]
+    pub(super) fail_next_user_counter_write: Arc<std::sync::atomic::AtomicBool>,
 }
 
 impl SystemCatalog {
@@ -37,7 +39,11 @@ impl SystemCatalog {
             info!(path = %path.display(), "system catalog opened");
         }
 
-        Ok(Self { db: Arc::new(db) })
+        Ok(Self {
+            db: Arc::new(db),
+            #[cfg(test)]
+            fail_next_user_counter_write: Arc::new(std::sync::atomic::AtomicBool::new(false)),
+        })
     }
 
     /// Open a non-persistent system catalog backed by an in-memory redb
@@ -48,7 +54,17 @@ impl SystemCatalog {
             .create_with_backend(redb::backends::InMemoryBackend::new())
             .map_err(|e| catalog_err("open in-memory", e))?;
         Self::ensure_bootstrapped(&db)?;
-        Ok(Self { db: Arc::new(db) })
+        Ok(Self {
+            db: Arc::new(db),
+            #[cfg(test)]
+            fail_next_user_counter_write: Arc::new(std::sync::atomic::AtomicBool::new(false)),
+        })
+    }
+
+    #[cfg(test)]
+    pub(crate) fn fail_next_user_counter_write_for_test(&self) {
+        self.fail_next_user_counter_write
+            .store(true, std::sync::atomic::Ordering::SeqCst);
     }
 
     /// Bootstrap every `_system.*` table from the canonical registry —

nodedb/src/control/security/catalog/users.rs

@@ -2,7 +2,7 @@
 
 //! User CRUD operations for the system catalog.
 
-use super::types::{StoredUser, SystemCatalog, USERS, catalog_err};
+use super::types::{METADATA, StoredUser, SystemCatalog, USERS, catalog_err};
 
 impl SystemCatalog {
     /// Load all active users from the catalog.
@@ -71,6 +71,46 @@ impl SystemCatalog {
         Ok(())
     }
 
+    /// Atomically write a newly allocated user and advance the durable ID counter.
+    pub fn put_user_with_next_user_id(
+        &self,
+        user: &StoredUser,
+        next_user_id: u64,
+    ) -> crate::Result<()> {
+        let bytes = zerompk::to_msgpack_vec(user).map_err(|e| catalog_err("serialize user", e))?;
+        let write_txn = self
+            .db
+            .begin_write()
+            .map_err(|e| catalog_err("write txn", e))?;
+        {
+            let mut users = write_txn
+                .open_table(USERS)
+                .map_err(|e| catalog_err("open users", e))?;
+            users
+                .insert(user.username.as_str(), bytes.as_slice())
+                .map_err(|e| catalog_err("insert user", e))?;
+        }
+        {
+            let mut metadata = write_txn
+                .open_table(METADATA)
+                .map_err(|e| catalog_err("open metadata", e))?;
+            metadata
+                .insert("next_user_id", next_user_id.to_le_bytes().as_slice())
+                .map_err(|e| catalog_err("insert next_user_id", e))?;
+        }
+        #[cfg(test)]
+        if self
+            .fail_next_user_counter_write
+            .swap(false, std::sync::atomic::Ordering::SeqCst)
+        {
+            return Err(crate::Error::Storage {
+                engine: "catalog".into(),
+                detail: "injected user/counter transaction failure".into(),
+            });
+        }
+        write_txn.commit().map_err(|e| catalog_err("commit", e))
+    }
+
     /// Delete a user record from the catalog.
     pub fn delete_user(&self, username: &str) -> crate::Result<()> {
         let write_txn = self

nodedb/src/control/security/credential/lockout.rs

@@ -10,7 +10,7 @@ use crate::control::security::audit::{AuditEmitContext, AuditEmitter, AuditEvent
 use crate::control::security::catalog::StoredLockoutRecord;
 use crate::types::TenantId;
 
-use super::store::{CredentialStore, read_lock, write_lock};
+use super::store::core::{CredentialStore, read_lock, write_lock};
 
 /// Tracks failed login attempts for lockout enforcement.
 #[derive(Debug, Clone)]

nodedb/src/control/security/credential/record.rs

@@ -138,3 +138,68 @@ impl UserRecord {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn backward_compat_stored_user_defaults() {
+        let created_at_epoch: u64 = 1_700_000_000;
+        let mut buf = Vec::new();
+        rmpv::encode::write_value(
+            &mut buf,
+            &rmpv::Value::Map(vec![
+                (
+                    rmpv::Value::String("user_id".into()),
+                    rmpv::Value::Integer(rmpv::Integer::from(42u64)),
+                ),
+                (
+                    rmpv::Value::String("username".into()),
+                    rmpv::Value::String("legacy_user".into()),
+                ),
+                (
+                    rmpv::Value::String("tenant_id".into()),
+                    rmpv::Value::Integer(rmpv::Integer::from(1u32)),
+                ),
+                (
+                    rmpv::Value::String("password_hash".into()),
+                    rmpv::Value::String("$argon2id$fake_hash".into()),
+                ),
+                (
+                    rmpv::Value::String("scram_salt".into()),
+                    rmpv::Value::Binary(vec![1, 2, 3]),
+                ),
+                (
+                    rmpv::Value::String("scram_salted_password".into()),
+                    rmpv::Value::Binary(vec![4, 5, 6]),
+                ),
+                (
+                    rmpv::Value::String("roles".into()),
+                    rmpv::Value::Array(vec![rmpv::Value::String("read_write".into())]),
+                ),
+                (
+                    rmpv::Value::String("is_superuser".into()),
+                    rmpv::Value::Boolean(false),
+                ),
+                (
+                    rmpv::Value::String("is_active".into()),
+                    rmpv::Value::Boolean(true),
+                ),
+                (
+                    rmpv::Value::String("created_at".into()),
+                    rmpv::Value::Integer(rmpv::Integer::from(created_at_epoch)),
+                ),
+            ]),
+        )
+        .expect("encode legacy StoredUser");
+
+        let stored: StoredUser = zerompk::from_msgpack(&buf).expect("decode legacy StoredUser");
+        assert!(!stored.must_change_password);
+        assert_eq!(stored.password_changed_at, 0);
+
+        let record = UserRecord::from_stored(stored);
+        assert!(!record.must_change_password);
+        assert_eq!(record.password_changed_at, created_at_epoch);
+    }
+}