From 99b5ab5978d98008081fcda25abe996bd8facb16 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 13 Mar 2026 17:39:28 -0500 Subject: [PATCH 1/2] make omdb not rely on USER_TEST_PRIVILEGED --- dev-tools/omdb/src/bin/omdb/db.rs | 2 +- dev-tools/omdb/src/bin/omdb/nexus.rs | 6 ++--- nexus/auth/src/authn/mod.rs | 6 +++++ nexus/auth/src/authz/actor.rs | 10 ++++++++ nexus/auth/src/authz/omicron.polar | 9 +++++++ nexus/auth/src/context.rs | 24 +++++++++++++++++++ nexus/db-fixed-data/src/user_builtin.rs | 12 ++++++++++ .../db-queries/src/db/datastore/silo_user.rs | 1 + .../tests/integration_tests/users_builtin.rs | 3 +++ 9 files changed, 69 insertions(+), 4 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/db.rs b/dev-tools/omdb/src/bin/omdb/db.rs index 913efe5acd5..03793e64bb7 100644 --- a/dev-tools/omdb/src/bin/omdb/db.rs +++ b/dev-tools/omdb/src/bin/omdb/db.rs @@ -322,7 +322,7 @@ impl DbUrlOptions { Fut: Future>, { let datastore = self.connect(omdb, log).await?; - let opctx = OpContext::for_tests(log.clone(), datastore.clone()); + let opctx = OpContext::for_omdb(log.clone(), datastore.clone()); let result = f(opctx, datastore.clone()).await; datastore.terminate().await; result diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index 6657e273541..b78aa2c5324 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -4401,7 +4401,7 @@ async fn cmd_nexus_sled_expunge_with_datastore( // most recent inventory collection use nexus_db_queries::context::OpContext; - let opctx = OpContext::for_tests(log.clone(), datastore.clone()); + let opctx = OpContext::for_omdb(log.clone(), datastore.clone()); let opctx = &opctx; // First, we need to look up the sled so we know its serial number. @@ -4511,7 +4511,7 @@ async fn cmd_nexus_sled_expunge_disk_with_datastore( ) -> Result<(), anyhow::Error> { use nexus_db_queries::context::OpContext; - let opctx = OpContext::for_tests(log.clone(), datastore.clone()); + let opctx = OpContext::for_omdb(log.clone(), datastore.clone()); let opctx = &opctx; // First, we need to look up the disk so we can lookup identity information. @@ -4732,7 +4732,7 @@ async fn cmd_nexus_trust_quorum_remove_sled_with_datastore( _destruction_token: DestructiveOperationToken, ) -> Result<(), anyhow::Error> { use nexus_db_queries::context::OpContext; - let opctx = OpContext::for_tests(log.clone(), datastore.clone()); + let opctx = OpContext::for_omdb(log.clone(), datastore.clone()); let opctx = &opctx; // First, we need to look up the sled so we know its serial number. diff --git a/nexus/auth/src/authn/mod.rs b/nexus/auth/src/authn/mod.rs index a133d571643..9c4668ef09d 100644 --- a/nexus/auth/src/authn/mod.rs +++ b/nexus/auth/src/authn/mod.rs @@ -34,6 +34,7 @@ pub use nexus_db_fixed_data::user_builtin::USER_DB_INIT; pub use nexus_db_fixed_data::user_builtin::USER_EXTERNAL_AUTHN; pub use nexus_db_fixed_data::user_builtin::USER_INTERNAL_API; pub use nexus_db_fixed_data::user_builtin::USER_INTERNAL_READ; +pub use nexus_db_fixed_data::user_builtin::USER_OMDB; pub use nexus_db_fixed_data::user_builtin::USER_SAGA_RECOVERY; pub use nexus_db_fixed_data::user_builtin::USER_SERVICE_BALANCER; @@ -242,6 +243,11 @@ impl Context { Context::context_for_builtin_user(USER_SERVICE_BALANCER.id) } + /// Returns an authenticated context for omdb + pub fn omdb() -> Context { + Context::context_for_builtin_user(USER_OMDB.id) + } + fn context_for_builtin_user(user_builtin_id: BuiltInUserUuid) -> Context { Context { kind: Kind::Authenticated( diff --git a/nexus/auth/src/authz/actor.rs b/nexus/auth/src/authz/actor.rs index 9eaaacaff98..0279b4bde5b 100644 --- a/nexus/auth/src/authz/actor.rs +++ b/nexus/auth/src/authz/actor.rs @@ -122,6 +122,16 @@ impl oso::PolarClass for AuthenticatedActor { }, "USER_INTERNAL_API", ) + .add_constant( + AuthenticatedActor { + actor: authn::Actor::UserBuiltin { + user_builtin_id: authn::USER_OMDB.id, + }, + roles: RoleSet::new(), + silo_policy: None, + }, + "USER_OMDB", + ) // This is meant to guard against the SCIM actor being able to see // the full resource hierarchy due to implicit grants in the Polar // file. There are "if actor.is_user" guards to prevent this. diff --git a/nexus/auth/src/authz/omicron.polar b/nexus/auth/src/authz/omicron.polar index 35e29206700..f4ff98caa35 100644 --- a/nexus/auth/src/authz/omicron.polar +++ b/nexus/auth/src/authz/omicron.polar @@ -804,6 +804,15 @@ has_role(USER_DB_INIT: AuthenticatedActor, "admin", _silo: Silo); # Allow the internal API admin permissions on all silos. has_role(USER_INTERNAL_API: AuthenticatedActor, "admin", _silo: Silo); +# USER_OMDB is the identity omdb uses when constructing an OpContext for +# datastore calls. This identity is only assumed in-process by the omdb binary +# itself — it cannot be presented over HTTP or any external authentication +# scheme. omdb itself connects directly to CockroachDB, so the authz checks are +# not really a security boundary; they happen only because omdb calls datastore +# functions and those contain authz checks. We grant fleet admin because some +# omdb commands require it. +has_role(USER_OMDB: AuthenticatedActor, "admin", _fleet: Fleet); + resource WebhookSecret { permissions = [ "read", "modify" ]; relations = { parent_alert_receiver: AlertReceiver }; diff --git a/nexus/auth/src/context.rs b/nexus/auth/src/context.rs index 797f80c2778..94c8bb77a0c 100644 --- a/nexus/auth/src/context.rs +++ b/nexus/auth/src/context.rs @@ -229,6 +229,30 @@ impl OpContext { } } + /// Returns a context for use by omdb (the operator debug tool). + pub fn for_omdb( + log: slog::Logger, + datastore: Arc, + ) -> OpContext { + let created_instant = Instant::now(); + let created_walltime = SystemTime::now(); + let authn = Arc::new(authn::Context::omdb()); + let authz = authz::Context::new( + Arc::clone(&authn), + Arc::new(authz::Authz::new(&log)), + Arc::clone(&datastore), + ); + OpContext { + log, + authz, + authn, + created_instant, + created_walltime, + metadata: BTreeMap::new(), + kind: OpKind::Test, + } + } + /// Creates a new `OpContext` with extra metadata (including log metadata) /// /// This is intended for cases where you want an OpContext that's diff --git a/nexus/db-fixed-data/src/user_builtin.rs b/nexus/db-fixed-data/src/user_builtin.rs index 1194fe23a53..c3e5c5527e9 100644 --- a/nexus/db-fixed-data/src/user_builtin.rs +++ b/nexus/db-fixed-data/src/user_builtin.rs @@ -94,6 +94,16 @@ pub static USER_EXTERNAL_AUTHN: LazyLock = ) }); +/// Built-in user for omdb (the Oxide operator debug tool) +pub static USER_OMDB: LazyLock = LazyLock::new(|| { + UserBuiltinConfig::new_static( + // "00db" for "omdb" + "001de000-05e4-4000-8000-0000000000db", + "omdb", + "used by the omdb operator debug tool", + ) +}); + #[cfg(test)] mod test { use super::super::assert_valid_typed_uuid; @@ -101,6 +111,7 @@ mod test { use super::USER_EXTERNAL_AUTHN; use super::USER_INTERNAL_API; use super::USER_INTERNAL_READ; + use super::USER_OMDB; use super::USER_SAGA_RECOVERY; use super::USER_SERVICE_BALANCER; @@ -112,5 +123,6 @@ mod test { assert_valid_typed_uuid(&USER_EXTERNAL_AUTHN.id); assert_valid_typed_uuid(&USER_INTERNAL_READ.id); assert_valid_typed_uuid(&USER_SAGA_RECOVERY.id); + assert_valid_typed_uuid(&USER_OMDB.id); } } diff --git a/nexus/db-queries/src/db/datastore/silo_user.rs b/nexus/db-queries/src/db/datastore/silo_user.rs index d42b0e6d152..64ce4ddc2f3 100644 --- a/nexus/db-queries/src/db/datastore/silo_user.rs +++ b/nexus/db-queries/src/db/datastore/silo_user.rs @@ -884,6 +884,7 @@ impl DataStore { &authn::USER_INTERNAL_READ, &authn::USER_EXTERNAL_AUTHN, &authn::USER_SAGA_RECOVERY, + &authn::USER_OMDB, ] .iter() .map(|u| { diff --git a/nexus/tests/integration_tests/users_builtin.rs b/nexus/tests/integration_tests/users_builtin.rs index 0f87e19ca41..c5315305436 100644 --- a/nexus/tests/integration_tests/users_builtin.rs +++ b/nexus/tests/integration_tests/users_builtin.rs @@ -54,6 +54,9 @@ async fn test_users_builtin(cptestctx: &ControlPlaneTestContext) { let u = users.remove(&authn::USER_SAGA_RECOVERY.name.to_string()).unwrap(); assert_eq!(u.identity.id, authn::USER_SAGA_RECOVERY.id.into_untyped_uuid()); + let u = users.remove(&authn::USER_OMDB.name.to_string()).unwrap(); + assert_eq!(u.identity.id, authn::USER_OMDB.id.into_untyped_uuid()); + assert!(users.is_empty(), "found unexpected built-in users"); // TODO-coverage add test for fetching individual users, including invalid From 647d12d1ff9b5f7cfe7d361622a7d138acc0ba86 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 17 Mar 2026 13:22:37 -0500 Subject: [PATCH 2/2] dave's suggestions: comments + internal role assignment approach --- nexus/auth/src/authn/mod.rs | 5 +++++ nexus/auth/src/authz/actor.rs | 10 ---------- nexus/auth/src/authz/omicron.polar | 9 --------- nexus/auth/src/context.rs | 2 +- nexus/db-fixed-data/src/role_assignment.rs | 8 ++++++++ nexus/db-fixed-data/src/user_builtin.rs | 4 ++-- 6 files changed, 16 insertions(+), 22 deletions(-) diff --git a/nexus/auth/src/authn/mod.rs b/nexus/auth/src/authn/mod.rs index 9c4668ef09d..557f287162e 100644 --- a/nexus/auth/src/authn/mod.rs +++ b/nexus/auth/src/authn/mod.rs @@ -372,6 +372,7 @@ mod test { use super::USER_DB_INIT; use super::USER_INTERNAL_API; use super::USER_INTERNAL_READ; + use super::USER_OMDB; use super::USER_SAGA_RECOVERY; use super::USER_SERVICE_BALANCER; use super::USER_TEST_PRIVILEGED; @@ -419,6 +420,10 @@ mod test { let authn = Context::internal_api(); let actor = authn.actor().unwrap(); assert_eq!(actor.built_in_user_id(), Some(USER_INTERNAL_API.id)); + + let authn = Context::omdb(); + let actor = authn.actor().unwrap(); + assert_eq!(actor.built_in_user_id(), Some(USER_OMDB.id)); } } diff --git a/nexus/auth/src/authz/actor.rs b/nexus/auth/src/authz/actor.rs index 0279b4bde5b..9eaaacaff98 100644 --- a/nexus/auth/src/authz/actor.rs +++ b/nexus/auth/src/authz/actor.rs @@ -122,16 +122,6 @@ impl oso::PolarClass for AuthenticatedActor { }, "USER_INTERNAL_API", ) - .add_constant( - AuthenticatedActor { - actor: authn::Actor::UserBuiltin { - user_builtin_id: authn::USER_OMDB.id, - }, - roles: RoleSet::new(), - silo_policy: None, - }, - "USER_OMDB", - ) // This is meant to guard against the SCIM actor being able to see // the full resource hierarchy due to implicit grants in the Polar // file. There are "if actor.is_user" guards to prevent this. diff --git a/nexus/auth/src/authz/omicron.polar b/nexus/auth/src/authz/omicron.polar index f4ff98caa35..35e29206700 100644 --- a/nexus/auth/src/authz/omicron.polar +++ b/nexus/auth/src/authz/omicron.polar @@ -804,15 +804,6 @@ has_role(USER_DB_INIT: AuthenticatedActor, "admin", _silo: Silo); # Allow the internal API admin permissions on all silos. has_role(USER_INTERNAL_API: AuthenticatedActor, "admin", _silo: Silo); -# USER_OMDB is the identity omdb uses when constructing an OpContext for -# datastore calls. This identity is only assumed in-process by the omdb binary -# itself — it cannot be presented over HTTP or any external authentication -# scheme. omdb itself connects directly to CockroachDB, so the authz checks are -# not really a security boundary; they happen only because omdb calls datastore -# functions and those contain authz checks. We grant fleet admin because some -# omdb commands require it. -has_role(USER_OMDB: AuthenticatedActor, "admin", _fleet: Fleet); - resource WebhookSecret { permissions = [ "read", "modify" ]; relations = { parent_alert_receiver: AlertReceiver }; diff --git a/nexus/auth/src/context.rs b/nexus/auth/src/context.rs index 94c8bb77a0c..62e8c856e93 100644 --- a/nexus/auth/src/context.rs +++ b/nexus/auth/src/context.rs @@ -229,7 +229,7 @@ impl OpContext { } } - /// Returns a context for use by omdb (the operator debug tool). + /// Returns a context for use by omdb (the debugging tool). pub fn for_omdb( log: slog::Logger, datastore: Arc, diff --git a/nexus/db-fixed-data/src/role_assignment.rs b/nexus/db-fixed-data/src/role_assignment.rs index aae06d4b7ae..ced2a05b035 100644 --- a/nexus/db-fixed-data/src/role_assignment.rs +++ b/nexus/db-fixed-data/src/role_assignment.rs @@ -53,5 +53,13 @@ pub static BUILTIN_ROLE_ASSIGNMENTS: LazyLock> = *FLEET_ID, "external-authenticator", ), + // The "omdb" user gets the "admin" role on the sole Fleet. + // It has all privileges for debugging purposes. + RoleAssignment::new_for_builtin_user( + user_builtin::USER_OMDB.id, + ResourceType::Fleet, + *FLEET_ID, + "admin", + ), ] }); diff --git a/nexus/db-fixed-data/src/user_builtin.rs b/nexus/db-fixed-data/src/user_builtin.rs index c3e5c5527e9..f95e53d6e74 100644 --- a/nexus/db-fixed-data/src/user_builtin.rs +++ b/nexus/db-fixed-data/src/user_builtin.rs @@ -94,13 +94,13 @@ pub static USER_EXTERNAL_AUTHN: LazyLock = ) }); -/// Built-in user for omdb (the Oxide operator debug tool) +/// Built-in user for omdb (the Omicron debugger) pub static USER_OMDB: LazyLock = LazyLock::new(|| { UserBuiltinConfig::new_static( // "00db" for "omdb" "001de000-05e4-4000-8000-0000000000db", "omdb", - "used by the omdb operator debug tool", + "used by the omdb debug tool", ) });