diff --git a/nexus/src/authn/mod.rs b/nexus/src/authn/mod.rs index af42013f5db..3ecb9a7f2ef 100644 --- a/nexus/src/authn/mod.rs +++ b/nexus/src/authn/mod.rs @@ -27,6 +27,7 @@ pub mod external; pub use crate::db::fixed_data::user_builtin::USER_DB_INIT; +pub use crate::db::fixed_data::user_builtin::USER_INTERNAL_API; pub use crate::db::fixed_data::user_builtin::USER_SAGA_RECOVERY; pub use crate::db::fixed_data::user_builtin::USER_TEST_PRIVILEGED; pub use crate::db::fixed_data::user_builtin::USER_TEST_UNPRIVILEGED; @@ -82,6 +83,11 @@ impl Context { Context { kind: Kind::Unauthenticated, schemes_tried: vec![] } } + /// Returns an authenticated context for handling internal API contexts + pub fn internal_api() -> Context { + Context::context_for_actor(USER_INTERNAL_API.id) + } + /// Returns an authenticated context for saga recovery pub fn internal_saga_recovery() -> Context { Context::context_for_actor(USER_SAGA_RECOVERY.id) @@ -101,15 +107,13 @@ impl Context { } /// Returns an authenticated context for a special testing user - #[cfg(test)] pub fn internal_test_user() -> Context { Context::test_context_for_actor(USER_TEST_PRIVILEGED.id) } /// Returns an authenticated context for a specific user /// - /// This is used for unit testing the authorization rules. - #[cfg(test)] + /// This is used for testing. pub fn test_context_for_actor(actor_id: Uuid) -> Context { Context::context_for_actor(actor_id) } @@ -119,6 +123,7 @@ impl Context { mod test { use super::Context; use super::USER_DB_INIT; + use super::USER_INTERNAL_API; use super::USER_SAGA_RECOVERY; use super::USER_TEST_PRIVILEGED; @@ -142,6 +147,10 @@ mod test { let authn = Context::internal_saga_recovery(); let actor = authn.actor().unwrap(); assert_eq!(actor.0, USER_SAGA_RECOVERY.id); + + let authn = Context::internal_api(); + let actor = authn.actor().unwrap(); + assert_eq!(actor.0, USER_INTERNAL_API.id); } } diff --git a/nexus/src/authz/api_resources.rs b/nexus/src/authz/api_resources.rs index 93196437a61..bafeb11c1bd 100644 --- a/nexus/src/authz/api_resources.rs +++ b/nexus/src/authz/api_resources.rs @@ -432,8 +432,8 @@ pub struct ProjectChild { } impl ProjectChild { - pub fn id(&self) -> &Uuid { - &self.resource_id + pub fn id(&self) -> Uuid { + self.resource_id } } @@ -472,3 +472,6 @@ impl ApiResourceError for ProjectChild { self.lookup_type.clone().into_not_found(self.resource_type) } } + +pub type Disk = ProjectChild; +pub type Instance = ProjectChild; diff --git a/nexus/src/authz/mod.rs b/nexus/src/authz/mod.rs index a77205dbf92..27d25396df6 100644 --- a/nexus/src/authz/mod.rs +++ b/nexus/src/authz/mod.rs @@ -164,11 +164,12 @@ mod actor; mod api_resources; pub use api_resources::ApiResourceError; +pub use api_resources::Disk; pub use api_resources::Fleet; pub use api_resources::FleetChild; +pub use api_resources::Instance; pub use api_resources::Organization; pub use api_resources::Project; -pub use api_resources::ProjectChild; pub use api_resources::FLEET; mod context; diff --git a/nexus/src/context.rs b/nexus/src/context.rs index db2e92066d8..ef7627648d2 100644 --- a/nexus/src/context.rs +++ b/nexus/src/context.rs @@ -43,6 +43,8 @@ pub struct ServerContext { pub log: Logger, /** authenticator for external HTTP requests */ pub external_authn: authn::external::Authenticator>, + /** authentication context used for internal HTTP requests */ + pub internal_authn: Arc, /** authorizer */ pub authz: Arc, /** internal API request latency tracker */ @@ -91,6 +93,7 @@ impl ServerContext { }) .collect(); let external_authn = authn::external::Authenticator::new(nexus_schemes); + let internal_authn = Arc::new(authn::Context::internal_api()); let authz = Arc::new(authz::Authz::new()); let create_tracker = |name: &str| { let target = HttpService { name: name.to_string(), id: config.id }; @@ -145,6 +148,7 @@ impl ServerContext { ), log, external_authn, + internal_authn, authz, internal_latencies, external_latencies, @@ -196,11 +200,12 @@ pub struct OpContext { enum OpKind { /// Handling an external API request ExternalApiRequest, + /// Handling an internal API request + InternalApiRequest, /// Background operations in Nexus Background, - #[cfg(test)] - /// Unit tests - UnitTest, + /// Automated testing (unit tests and integration tests) + Test, } impl OpContext { @@ -220,12 +225,55 @@ impl OpContext { datastore, ); - let request = rqctx.request.lock().await; + let (log, metadata) = + OpContext::log_and_metadata_for_request(rqctx, &authn).await; + + Ok(OpContext { + log, + authz, + authn, + created_instant, + created_walltime, + metadata, + kind: OpKind::ExternalApiRequest, + }) + } + + /// Returns a context suitable for use in handling internal API operations + // TODO-security this should eventually do some kind of authentication + pub async fn for_internal_api( + rqctx: &dropshot::RequestContext>, + ) -> OpContext { + let created_instant = Instant::now(); + let created_walltime = SystemTime::now(); + let apictx = rqctx.context(); + let authn = Arc::clone(&apictx.internal_authn); + let datastore = Arc::clone(apictx.nexus.datastore()); + let authz = authz::Context::new( + Arc::clone(&authn), + Arc::clone(&apictx.authz), + datastore, + ); + + let (log, metadata) = + OpContext::log_and_metadata_for_request(rqctx, &authn).await; + + OpContext { + log, + authz, + authn, + created_instant, + created_walltime, + metadata, + kind: OpKind::InternalApiRequest, + } + } + + async fn log_and_metadata_for_request( + rqctx: &dropshot::RequestContext, + authn: &authn::Context, + ) -> (slog::Logger, BTreeMap) { let mut metadata = BTreeMap::new(); - metadata.insert(String::from("request_id"), rqctx.request_id.clone()); - metadata - .insert(String::from("http_method"), request.method().to_string()); - metadata.insert(String::from("http_uri"), request.uri().to_string()); let log = if let Some(Actor(actor_id)) = authn.actor() { metadata @@ -240,15 +288,13 @@ impl OpContext { rqctx.log.new(o!("authenticated" => false)) }; - Ok(OpContext { - log, - authz, - authn, - created_instant, - created_walltime, - metadata, - kind: OpKind::ExternalApiRequest, - }) + let request = rqctx.request.lock().await; + metadata.insert(String::from("request_id"), rqctx.request_id.clone()); + metadata + .insert(String::from("http_method"), request.method().to_string()); + metadata.insert(String::from("http_uri"), request.uri().to_string()); + + (log, metadata) } /// Returns a context suitable for use in background operations in Nexus @@ -277,10 +323,9 @@ impl OpContext { } } - /// Returns a context suitable for automated unit tests where an OpContext - /// is needed outside of a Dropshot context - #[cfg(test)] - pub fn for_unit_tests( + /// Returns a context suitable for automated tests where an OpContext is + /// needed outside of a Dropshot context + pub fn for_tests( log: slog::Logger, datastore: Arc, ) -> OpContext { @@ -299,7 +344,7 @@ impl OpContext { created_instant, created_walltime, metadata: BTreeMap::new(), - kind: OpKind::UnitTest, + kind: OpKind::Test, } } @@ -384,7 +429,7 @@ mod test { let mut db = test_setup_database(&logctx.log).await; let (_, datastore) = crate::db::datastore::datastore_test(&logctx, &db).await; - let opctx = OpContext::for_unit_tests(logctx.log.new(o!()), datastore); + let opctx = OpContext::for_tests(logctx.log.new(o!()), datastore); // Like in test_background_context(), this is essentially a test of the // authorization policy. The unit tests assume this user can do diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index bae4e4bfcae..cc14293d587 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -561,13 +561,40 @@ impl DataStore { }) } + /// Fetch an [`authz::Organization`] based on its id + pub async fn organization_lookup_by_id( + &self, + organization_id: Uuid, + ) -> LookupResult { + use db::schema::organization::dsl; + // We only do this database lookup to verify that the Organization with + // this id exists and hasn't been deleted. + let _: Uuid = dsl::organization + .filter(dsl::time_deleted.is_null()) + .filter(dsl::id.eq(organization_id)) + .select(dsl::id) + .first_async(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Organization, + LookupType::ById(organization_id), + ), + ) + })?; + Ok(authz::FLEET + .organization(organization_id, LookupType::ById(organization_id))) + } + /// Look up the id for an organization based on its name /// /// Returns an [`authz::Organization`] (which makes the id available). /// /// This function does no authz checks because it is not possible to know /// just by looking up an Organization's id what privileges are required. - pub async fn organization_lookup_path( + pub async fn organization_lookup_by_path( &self, name: &Name, ) -> LookupResult { @@ -701,7 +728,7 @@ impl DataStore { ) -> UpdateResult { use db::schema::organization::dsl; - let authz_org = self.organization_lookup_path(name).await?; + let authz_org = self.organization_lookup_by_path(name).await?; opctx.authorize(authz::Action::Modify, &authz_org).await?; diesel::update(dsl::organization) @@ -789,19 +816,45 @@ impl DataStore { }) } + /// Fetch an [`authz::Project`] based on its id + pub async fn project_lookup_by_id( + &self, + project_id: Uuid, + ) -> LookupResult { + use db::schema::project::dsl; + let organization_id = dsl::project + .filter(dsl::time_deleted.is_null()) + .filter(dsl::id.eq(project_id)) + .select(dsl::organization_id) + .first_async(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Project, + LookupType::ById(project_id), + ), + ) + })?; + let authz_organization = + self.organization_lookup_by_id(organization_id).await?; + Ok(authz_organization.project(project_id, LookupType::ById(project_id))) + } + /// Look up the id for a Project based on its name /// /// Returns an [`authz::Project`] (which makes the id available). /// /// This function does no authz checks because it is not possible to know /// just by looking up an Project's id what privileges are required. - pub async fn project_lookup_path( + pub async fn project_lookup_by_path( &self, organization_name: &Name, project_name: &Name, ) -> LookupResult { let authz_org = - self.organization_lookup_path(organization_name).await?; + self.organization_lookup_by_path(organization_name).await?; self.project_lookup_noauthz(&authz_org, project_name) .await .map(|(p, _)| p) @@ -814,10 +867,10 @@ impl DataStore { authz_org: &authz::Organization, name: &Name, ) -> LookupResult<(authz::Project, Project)> { - let (authz_org, db_org) = + let (authz_project, db_project) = self.project_lookup_noauthz(authz_org, name).await?; - opctx.authorize(authz::Action::Read, &authz_org).await?; - Ok((authz_org, db_org)) + opctx.authorize(authz::Action::Read, &authz_project).await?; + Ok((authz_project, db_project)) } /// Delete a project @@ -1143,6 +1196,114 @@ impl DataStore { * Disks */ + /// Fetches a Disk from the database and returns both the database row + /// and an [`authz::Disk`] for doing authz checks + /// + /// See [`DataStore::organization_lookup_noauthz()`] for intended use cases + /// and caveats. + // TODO-security See the note on organization_lookup_noauthz(). + async fn disk_lookup_noauthz( + &self, + authz_project: &authz::Project, + disk_name: &Name, + ) -> LookupResult<(authz::Disk, Disk)> { + use db::schema::disk::dsl; + dsl::disk + .filter(dsl::time_deleted.is_null()) + .filter(dsl::project_id.eq(authz_project.id())) + .filter(dsl::name.eq(disk_name.clone())) + .select(Disk::as_select()) + .first_async(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Disk, + LookupType::ByName(disk_name.as_str().to_owned()), + ), + ) + }) + .map(|d| { + ( + authz_project.child_generic( + ResourceType::Disk, + d.id(), + LookupType::from(&disk_name.0), + ), + d, + ) + }) + } + + /// Fetch an [`authz::Disk`] based on its id + pub async fn disk_lookup_by_id( + &self, + disk_id: Uuid, + ) -> LookupResult { + use db::schema::disk::dsl; + let project_id = dsl::disk + .filter(dsl::time_deleted.is_null()) + .filter(dsl::id.eq(disk_id)) + .select(dsl::project_id) + .first_async(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Disk, + LookupType::ById(disk_id), + ), + ) + })?; + let authz_project = self.project_lookup_by_id(project_id).await?; + Ok(authz_project.child_generic( + ResourceType::Disk, + disk_id, + LookupType::ById(disk_id), + )) + } + + /// Look up the id for a Disk based on its name + /// + /// Returns an [`authz::Disk`] (which makes the id available). + /// + /// Like the other "lookup_by_path()" functions, this function does no authz + /// checks. + // TODO-security For Containers in the hierarchy (like Organizations and + // Projects), we don't do an authz check in the "lookup_by_path" functions + // because we don't know if the caller has access to do the lookup. For + // leaf resources (like Instances and Disks), though, we do. We could do + // the authz check here, and in disk_lookup_by_id() too. Should we? + pub async fn disk_lookup_by_path( + &self, + organization_name: &Name, + project_name: &Name, + disk_name: &Name, + ) -> LookupResult { + let authz_project = self + .project_lookup_by_path(organization_name, project_name) + .await?; + self.disk_lookup_noauthz(&authz_project, disk_name) + .await + .map(|(d, _)| d) + } + + /// Lookup a Disk by name and return the full database record, along with + /// an [`authz::Disk`] for subsequent authorization checks + pub async fn disk_fetch( + &self, + opctx: &OpContext, + authz_project: &authz::Project, + name: &Name, + ) -> LookupResult<(authz::Disk, Disk)> { + let (authz_disk, db_disk) = + self.disk_lookup_noauthz(authz_project, name).await?; + opctx.authorize(authz::Action::Read, &authz_disk).await?; + Ok((authz_disk, db_disk)) + } + /** * List disks associated with a given instance. */ @@ -1213,19 +1374,23 @@ impl DataStore { .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) } - pub async fn disk_update_runtime( + /// See `disk_update_runtime()`. This version should only be used from + /// sagas, which do not currently have authn contexts. + // TODO-security Like project_delete_disk_no_auth(), this should be removed + // once we have support for authz checks from sagas. + pub async fn disk_update_runtime_no_auth( &self, - disk_id: &Uuid, + authz_disk: &authz::Disk, new_runtime: &DiskRuntimeState, ) -> Result { + let disk_id = authz_disk.id(); use db::schema::disk::dsl; - let updated = diesel::update(dsl::disk) .filter(dsl::time_deleted.is_null()) - .filter(dsl::id.eq(*disk_id)) + .filter(dsl::id.eq(disk_id)) .filter(dsl::state_generation.lt(new_runtime.gen)) .set(new_runtime.clone()) - .check_if_exists::(*disk_id) + .check_if_exists::(disk_id) .execute_and_check(self.pool()) .await .map(|r| match r.status { @@ -1235,57 +1400,61 @@ impl DataStore { .map_err(|e| { public_error_from_diesel_pool( e, - ErrorHandler::NotFoundByLookup( - ResourceType::Disk, - LookupType::ById(*disk_id), - ), + ErrorHandler::NotFoundByResource(authz_disk), ) })?; Ok(updated) } - pub async fn disk_fetch(&self, disk_id: &Uuid) -> LookupResult { - use db::schema::disk::dsl; - - dsl::disk - .filter(dsl::time_deleted.is_null()) - .filter(dsl::id.eq(*disk_id)) - .select(Disk::as_select()) - .get_result_async(self.pool()) - .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ErrorHandler::NotFoundByLookup( - ResourceType::Disk, - LookupType::ById(*disk_id), - ), - ) - }) - } - - pub async fn disk_fetch_by_name( + pub async fn disk_update_runtime( &self, - project_id: &Uuid, - disk_name: &Name, + opctx: &OpContext, + authz_disk: &authz::Disk, + new_runtime: &DiskRuntimeState, + ) -> Result { + // TODO-security This permission might be overloaded here. The way disk + // runtime updates work is that the caller in Nexus first updates the + // Sled Agent to make a change, then updates to the database to reflect + // that change. So by the time we get here, we better have already done + // an authz check, or we will have already made some unauthorized change + // to the system! At the same time, we don't want just anybody to be + // able to modify the database state. So we _do_ still want an authz + // check here. Arguably it's for a different kind of action, but it + // doesn't seem that useful to split it out right now. + opctx.authorize(authz::Action::Modify, authz_disk).await?; + self.disk_update_runtime_no_auth(authz_disk, new_runtime).await + } + + /// Fetches information about a Disk that the caller has previously fetched + /// + /// The principal difference from `disk_fetch` is that this function takes + /// an `authz_disk` and does a lookup by id rather than the full path of + /// names (organization name, project name, and disk name). This could be + /// called `disk_lookup_by_id`, except that you might expect to get back an + /// `authz::Disk` as well. We cannot return you a new `authz::Disk` because + /// we don't know how you looked up the Disk in the first place. However, + /// you must have previously looked it up, which is why we call this + /// `refetch`. + pub async fn disk_refetch( + &self, + opctx: &OpContext, + authz_disk: &authz::Disk, ) -> LookupResult { use db::schema::disk::dsl; + opctx.authorize(authz::Action::Read, authz_disk).await?; + dsl::disk .filter(dsl::time_deleted.is_null()) - .filter(dsl::project_id.eq(*project_id)) - .filter(dsl::name.eq(disk_name.clone())) + .filter(dsl::id.eq(authz_disk.id())) .select(Disk::as_select()) - .get_result_async(self.pool()) + .get_result_async(self.pool_authorized(opctx).await?) .await .map_err(|e| { public_error_from_diesel_pool( e, - ErrorHandler::NotFoundByLookup( - ResourceType::Disk, - LookupType::ByName(disk_name.as_str().to_owned()), - ), + ErrorHandler::NotFoundByResource(authz_disk), ) }) } @@ -2482,6 +2651,7 @@ impl DataStore { let builtin_users = [ // Note: "db_init" is also a builtin user, but that one by necessity // is created with the database. + &*authn::USER_INTERNAL_API, &*authn::USER_SAGA_RECOVERY, &*authn::USER_TEST_PRIVILEGED, &*authn::USER_TEST_UNPRIVILEGED, @@ -2693,7 +2863,7 @@ pub async fn datastore_test( // Create an OpContext with the credentials of "test-privileged" for general // testing. let opctx = - OpContext::for_unit_tests(logctx.log.new(o!()), Arc::clone(&datastore)); + OpContext::for_tests(logctx.log.new(o!()), Arc::clone(&datastore)); (opctx, datastore) } diff --git a/nexus/src/db/fixed_data/role_assignment_builtin.rs b/nexus/src/db/fixed_data/role_assignment_builtin.rs index 07a579b3f18..3f2d4d9281b 100644 --- a/nexus/src/db/fixed_data/role_assignment_builtin.rs +++ b/nexus/src/db/fixed_data/role_assignment_builtin.rs @@ -10,13 +10,26 @@ use crate::db::model::RoleAssignmentBuiltin; use lazy_static::lazy_static; lazy_static! { - // The "test-privileged" user gets the "admin" role on the sole Fleet. - // This will grant them all permissions on all resources. pub static ref BUILTIN_ROLE_ASSIGNMENTS: Vec = - vec![RoleAssignmentBuiltin::new( - user_builtin::USER_TEST_PRIVILEGED.id, - role_builtin::FLEET_ADMIN.resource_type, - *FLEET_ID, - role_builtin::FLEET_ADMIN.role_name, - )]; + vec![ + // The "test-privileged" user gets the "admin" role on the sole + // Fleet. This will grant them all permissions on all resources. + RoleAssignmentBuiltin::new( + user_builtin::USER_TEST_PRIVILEGED.id, + role_builtin::FLEET_ADMIN.resource_type, + *FLEET_ID, + role_builtin::FLEET_ADMIN.role_name, + ), + + // The "internal-api" user gets the "admin" role on the sole Fleet. + // This will grant them all permissions on all resources. + // TODO-security We should scope this down (or, really, figure out a + // better internal authn/authz story). + RoleAssignmentBuiltin::new( + user_builtin::USER_INTERNAL_API.id, + role_builtin::FLEET_ADMIN.resource_type, + *FLEET_ID, + role_builtin::FLEET_ADMIN.role_name, + ), + ]; } diff --git a/nexus/src/db/fixed_data/user_builtin.rs b/nexus/src/db/fixed_data/user_builtin.rs index 4bdb23c2a06..ebe65a295ab 100644 --- a/nexus/src/db/fixed_data/user_builtin.rs +++ b/nexus/src/db/fixed_data/user_builtin.rs @@ -39,6 +39,14 @@ lazy_static! { "used for seeding initial database data", ); + /// Internal user used by Nexus when handling internal API requests + pub static ref USER_INTERNAL_API: UserBuiltinConfig = + UserBuiltinConfig::new_static( + "001de000-05e4-4000-8000-000000000002", + "internal-api", + "used by Nexus when handling internal API requests", + ); + /// Internal user used by Nexus when recovering sagas pub static ref USER_SAGA_RECOVERY: UserBuiltinConfig = UserBuiltinConfig::new_static( @@ -74,6 +82,7 @@ lazy_static! { mod test { use super::super::assert_valid_uuid; use super::USER_DB_INIT; + use super::USER_INTERNAL_API; use super::USER_SAGA_RECOVERY; use super::USER_TEST_PRIVILEGED; use super::USER_TEST_UNPRIVILEGED; @@ -81,6 +90,7 @@ mod test { #[test] fn test_builtin_user_ids_are_valid() { assert_valid_uuid(&USER_DB_INIT.id); + assert_valid_uuid(&USER_INTERNAL_API.id); assert_valid_uuid(&USER_SAGA_RECOVERY.id); assert_valid_uuid(&USER_TEST_PRIVILEGED.id); assert_valid_uuid(&USER_TEST_UNPRIVILEGED.id); diff --git a/nexus/src/db/saga_recovery.rs b/nexus/src/db/saga_recovery.rs index 37193199c4d..584a93f6cbc 100644 --- a/nexus/src/db/saga_recovery.rs +++ b/nexus/src/db/saga_recovery.rs @@ -481,7 +481,7 @@ mod test { let (storage, sec_client, uctx) = create_storage_sec_and_context(&log, db_datastore.clone(), sec_id); let sec_log = log.new(o!("component" => "SEC")); - let opctx = OpContext::for_unit_tests(log, Arc::clone(&db_datastore)); + let opctx = OpContext::for_tests(log, Arc::clone(&db_datastore)); // Create and start a saga. // @@ -555,7 +555,7 @@ mod test { let (storage, sec_client, uctx) = create_storage_sec_and_context(&log, db_datastore.clone(), sec_id); let sec_log = log.new(o!("component" => "SEC")); - let opctx = OpContext::for_unit_tests(log, Arc::clone(&db_datastore)); + let opctx = OpContext::for_tests(log, Arc::clone(&db_datastore)); // Create and start a saga, which we expect to complete successfully. let saga_id = SagaId(Uuid::new_v4()); diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 54ac30f3733..d1a4455d2ad 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -661,8 +661,9 @@ async fn project_disks_get_disk( let project_name = &path.project_name; let disk_name = &path.disk_name; let handler = async { - let (disk, _) = nexus - .project_lookup_disk(&organization_name, &project_name, &disk_name) + let opctx = OpContext::for_external_api(&rqctx).await?; + let disk = nexus + .disk_fetch(&opctx, &organization_name, &project_name, &disk_name) .await?; Ok(HttpResponseOk(disk.into())) }; @@ -1027,8 +1028,10 @@ async fn instance_disks_attach( let project_name = &path.project_name; let instance_name = &path.instance_name; let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; let disk = nexus .instance_attach_disk( + &opctx, &organization_name, &project_name, &instance_name, @@ -1057,8 +1060,10 @@ async fn instance_disks_detach( let project_name = &path.project_name; let instance_name = &path.instance_name; let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; let disk = nexus .instance_detach_disk( + &opctx, &organization_name, &project_name, &instance_name, diff --git a/nexus/src/internal_api/http_entrypoints.rs b/nexus/src/internal_api/http_entrypoints.rs index 92706dc279f..edf97b34273 100644 --- a/nexus/src/internal_api/http_entrypoints.rs +++ b/nexus/src/internal_api/http_entrypoints.rs @@ -2,6 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. +use crate::context::OpContext; /** * Handler functions (entrypoints) for HTTP APIs internal to the control plane */ @@ -206,7 +207,8 @@ async fn cpapi_disks_put( let path = path_params.into_inner(); let new_state = new_runtime_state.into_inner(); let handler = async { - nexus.notify_disk_updated(&path.disk_id, &new_state).await?; + let opctx = OpContext::for_internal_api(&rqctx).await; + nexus.notify_disk_updated(&opctx, path.disk_id, &new_state).await?; Ok(HttpResponseUpdatedNoContent()) }; apictx.internal_latencies.instrument_dropshot_handler(&rqctx, handler).await diff --git a/nexus/src/lib.rs b/nexus/src/lib.rs index e73533d95b1..8ade3e1868b 100644 --- a/nexus/src/lib.rs +++ b/nexus/src/lib.rs @@ -30,6 +30,7 @@ mod sagas; pub use config::Config; pub use context::ServerContext; +pub use crucible_agent_client; use external_api::http_entrypoints::external_api; use internal_api::http_entrypoints::internal_api; pub use nexus::Nexus; diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index bbb739d06f6..d93fc1c3ca6 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -33,7 +33,6 @@ use omicron_common::api::external; use omicron_common::api::external::CreateResult; use omicron_common::api::external::DataPageParams; use omicron_common::api::external::DeleteResult; -use omicron_common::api::external::Disk; use omicron_common::api::external::DiskState; use omicron_common::api::external::Error; use omicron_common::api::external::IdentityMetadataCreateParams; @@ -43,7 +42,6 @@ use omicron_common::api::external::Ipv6Net; use omicron_common::api::external::ListResult; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; -use omicron_common::api::external::LookupType; use omicron_common::api::external::PaginationOrder; use omicron_common::api::external::ResourceType; use omicron_common::api::external::RouteDestination; @@ -549,7 +547,7 @@ impl Nexus { ) -> CreateResult { let org = self .db_datastore - .organization_lookup_path(organization_name) + .organization_lookup_by_path(organization_name) .await?; // Create a project. @@ -591,7 +589,7 @@ impl Nexus { ) -> LookupResult { let authz_org = self .db_datastore - .organization_lookup_path(organization_name) + .organization_lookup_by_path(organization_name) .await?; Ok(self .db_datastore @@ -608,7 +606,7 @@ impl Nexus { ) -> ListResultVec { let authz_org = self .db_datastore - .organization_lookup_path(organization_name) + .organization_lookup_by_path(organization_name) .await?; self.db_datastore .projects_list_by_name(opctx, &authz_org, pagparams) @@ -623,7 +621,7 @@ impl Nexus { ) -> ListResultVec { let authz_org = self .db_datastore - .organization_lookup_path(organization_name) + .organization_lookup_by_path(organization_name) .await?; self.db_datastore .projects_list_by_id(opctx, &authz_org, pagparams) @@ -638,7 +636,7 @@ impl Nexus { ) -> DeleteResult { let authz_project = self .db_datastore - .project_lookup_path(organization_name, project_name) + .project_lookup_by_path(organization_name, project_name) .await?; self.db_datastore.project_delete(opctx, &authz_project).await } @@ -652,7 +650,7 @@ impl Nexus { ) -> UpdateResult { let authz_project = self .db_datastore - .project_lookup_path(organization_name, project_name) + .project_lookup_by_path(organization_name, project_name) .await?; self.db_datastore .project_update(opctx, &authz_project, new_params.clone().into()) @@ -672,7 +670,7 @@ impl Nexus { ) -> ListResultVec { let authz_project = self .db_datastore - .project_lookup_path(organization_name, project_name) + .project_lookup_by_path(organization_name, project_name) .await?; self.db_datastore .project_list_disks(opctx, &authz_project, pagparams) @@ -688,7 +686,7 @@ impl Nexus { ) -> CreateResult { let authz_project = self .db_datastore - .project_lookup_path(organization_name, project_name) + .project_lookup_by_path(organization_name, project_name) .await?; // TODO-security This may need to be revisited once we implement authz @@ -726,29 +724,22 @@ impl Nexus { Ok(disk_created) } - pub async fn project_lookup_disk( + pub async fn disk_fetch( &self, + opctx: &OpContext, organization_name: &Name, project_name: &Name, disk_name: &Name, - ) -> LookupResult<(db::model::Disk, authz::ProjectChild)> { + ) -> LookupResult { let authz_project = self .db_datastore - .project_lookup_path(organization_name, project_name) + .project_lookup_by_path(organization_name, project_name) .await?; - let disk = self + Ok(self .db_datastore - .disk_fetch_by_name(&authz_project.id(), disk_name) - .await?; - let disk_id = disk.id(); - Ok(( - disk, - authz_project.child_generic( - ResourceType::Disk, - disk_id, - LookupType::from(&disk_name.0), - ), - )) + .disk_fetch(opctx, &authz_project, disk_name) + .await? + .1) } pub async fn project_delete_disk( @@ -758,8 +749,9 @@ impl Nexus { project_name: &Name, disk_name: &Name, ) -> DeleteResult { - let (disk, authz_disk) = self - .project_lookup_disk(organization_name, project_name, disk_name) + let authz_disk = self + .db_datastore + .disk_lookup_by_path(organization_name, project_name, disk_name) .await?; // TODO: We need to sort out the authorization checks. @@ -771,7 +763,7 @@ impl Nexus { opctx.authorize(authz::Action::Delete, &authz_disk).await?; let saga_params = - Arc::new(sagas::ParamsDiskDelete { disk_id: disk.id() }); + Arc::new(sagas::ParamsDiskDelete { disk_id: authz_disk.id() }); self.execute_saga( Arc::clone(&sagas::SAGA_DISK_DELETE_TEMPLATE), sagas::SAGA_DISK_DELETE_NAME, @@ -819,7 +811,7 @@ impl Nexus { ) -> ListResultVec { let project_id = self .db_datastore - .project_lookup_path(organization_name, project_name) + .project_lookup_by_path(organization_name, project_name) .await? .id(); self.db_datastore.project_list_instances(&project_id, pagparams).await @@ -833,7 +825,7 @@ impl Nexus { ) -> CreateResult { let authz_project = self .db_datastore - .project_lookup_path(organization_name, project_name) + .project_lookup_by_path(organization_name, project_name) .await?; let saga_params = Arc::new(sagas::ParamsInstanceCreate { @@ -917,7 +909,7 @@ impl Nexus { */ let project_id = self .db_datastore - .project_lookup_path(organization_name, project_name) + .project_lookup_by_path(organization_name, project_name) .await? .id(); let instance = self @@ -936,7 +928,7 @@ impl Nexus { ) -> UpdateResult { let project_id = self .db_datastore - .project_lookup_path(organization_name, project_name) + .project_lookup_by_path(organization_name, project_name) .await? .id(); let instance = self @@ -971,7 +963,7 @@ impl Nexus { ) -> LookupResult { let project_id = self .db_datastore - .project_lookup_path(organization_name, project_name) + .project_lookup_by_path(organization_name, project_name) .await? .id(); self.db_datastore @@ -1271,42 +1263,12 @@ impl Nexus { self.db_datastore.instance_list_disks(&instance.id(), pagparams).await } - /** - * Fetch information about whether this disk is attached to this instance. - */ - pub async fn instance_get_disk( - &self, - organization_name: &Name, - project_name: &Name, - instance_name: &Name, - disk_name: &Name, - ) -> LookupResult { - let instance = self - .project_lookup_instance( - organization_name, - project_name, - instance_name, - ) - .await?; - // TODO: This shouldn't be looking up multiple database entries by name, - // it should resolve names to IDs first. - let (disk, _) = self - .project_lookup_disk(organization_name, project_name, disk_name) - .await?; - if let Some(instance_id) = disk.runtime_state.attach_instance_id { - if instance_id == instance.id() { - return Ok(disk.into()); - } - } - - Err(Error::not_found_by_name(ResourceType::Disk, disk_name)) - } - /** * Attach a disk to an instance. */ pub async fn instance_attach_disk( &self, + opctx: &OpContext, organization_name: &Name, project_name: &Name, instance_name: &Name, @@ -1321,8 +1283,13 @@ impl Nexus { .await?; // TODO: This shouldn't be looking up multiple database entries by name, // it should resolve names to IDs first. - let (disk, _) = self - .project_lookup_disk(organization_name, project_name, disk_name) + let authz_project = self + .db_datastore + .project_lookup_by_path(organization_name, project_name) + .await?; + let (authz_disk, db_disk) = self + .db_datastore + .disk_fetch(opctx, &authz_project, disk_name) .await?; let instance_id = &instance.id(); @@ -1359,12 +1326,14 @@ impl Nexus { Err(Error::InvalidRequest { message }) } - match &disk.state().into() { + match &db_disk.state().into() { /* * If we're already attaching or attached to the requested instance, * there's nothing else to do. + * TODO-security should it be an error if you're not authorized to + * do this and we did not actually have to do anything? */ - DiskState::Attached(id) if id == instance_id => return Ok(disk), + DiskState::Attached(id) if id == instance_id => return Ok(db_disk), /* * If the disk is currently attaching or attached to another @@ -1377,19 +1346,19 @@ impl Nexus { */ DiskState::Attached(id) => { assert_ne!(id, instance_id); - return disk_attachment_error(&disk); + return disk_attachment_error(&db_disk); } DiskState::Detaching(_) => { - return disk_attachment_error(&disk); + return disk_attachment_error(&db_disk); } DiskState::Attaching(id) if id != instance_id => { - return disk_attachment_error(&disk); + return disk_attachment_error(&db_disk); } DiskState::Destroyed => { - return disk_attachment_error(&disk); + return disk_attachment_error(&db_disk); } DiskState::Faulted => { - return disk_attachment_error(&disk); + return disk_attachment_error(&db_disk); } DiskState::Creating => (), @@ -1400,14 +1369,16 @@ impl Nexus { } self.disk_set_runtime( - &disk, + opctx, + &authz_disk, + &db_disk, self.instance_sled(&instance).await?, sled_agent_client::types::DiskStateRequested::Attached( *instance_id, ), ) .await?; - self.db_datastore.disk_fetch(&disk.id()).await + self.db_datastore.disk_refetch(opctx, &authz_disk).await } /** @@ -1415,6 +1386,7 @@ impl Nexus { */ pub async fn instance_detach_disk( &self, + opctx: &OpContext, organization_name: &Name, project_name: &Name, instance_name: &Name, @@ -1429,21 +1401,30 @@ impl Nexus { .await?; // TODO: This shouldn't be looking up multiple database entries by name, // it should resolve names to IDs first. - let (disk, _) = self - .project_lookup_disk(organization_name, project_name, disk_name) + let authz_project = self + .db_datastore + .project_lookup_by_path(organization_name, project_name) + .await?; + let (authz_disk, db_disk) = self + .db_datastore + .disk_fetch(opctx, &authz_project, disk_name) .await?; let instance_id = &instance.id(); - match &disk.state().into() { + match &db_disk.state().into() { /* * This operation is a noop if the disk is not attached or already * detaching from the same instance. + * TODO-security should it be an error if you're not authorized to + * do this and we did not actually have to do anything? */ - DiskState::Creating => return Ok(disk), - DiskState::Detached => return Ok(disk), - DiskState::Destroyed => return Ok(disk), - DiskState::Faulted => return Ok(disk), - DiskState::Detaching(id) if id == instance_id => return Ok(disk), + DiskState::Creating => return Ok(db_disk), + DiskState::Detached => return Ok(db_disk), + DiskState::Destroyed => return Ok(db_disk), + DiskState::Faulted => return Ok(db_disk), + DiskState::Detaching(id) if id == instance_id => { + return Ok(db_disk) + } /* * This operation is not allowed if the disk is attached to some @@ -1471,12 +1452,14 @@ impl Nexus { } self.disk_set_runtime( - &disk, + opctx, + &authz_disk, + &db_disk, self.instance_sled(&instance).await?, sled_agent_client::types::DiskStateRequested::Detached, ) .await?; - Ok(disk) + self.db_datastore.disk_refetch(opctx, &authz_disk).await } /** @@ -1485,19 +1468,23 @@ impl Nexus { */ async fn disk_set_runtime( &self, - disk: &db::model::Disk, + opctx: &OpContext, + authz_disk: &authz::Disk, + db_disk: &db::model::Disk, sa: Arc, requested: sled_agent_client::types::DiskStateRequested, ) -> Result<(), Error> { - let runtime: DiskRuntimeState = disk.runtime().into(); + let runtime: DiskRuntimeState = db_disk.runtime().into(); + + opctx.authorize(authz::Action::Modify, authz_disk).await?; /* - * Ask the SA to begin the state change. Then update the database to - * reflect the new intermediate state. + * Ask the Sled Agent to begin the state change. Then update the + * database to reflect the new intermediate state. */ let new_runtime = sa .disk_put( - &disk.id(), + &authz_disk.id(), &sled_agent_client::types::DiskEnsureBody { initial_runtime: sled_agent_client::types::DiskRuntimeState::from( @@ -1512,7 +1499,7 @@ impl Nexus { let new_runtime: DiskRuntimeState = new_runtime.into(); self.db_datastore - .disk_update_runtime(&disk.id(), &new_runtime.into()) + .disk_update_runtime(opctx, authz_disk, &new_runtime.into()) .await .map(|_| ()) } @@ -1573,7 +1560,7 @@ impl Nexus { ) -> ListResultVec { let project_id = self .db_datastore - .project_lookup_path(organization_name, project_name) + .project_lookup_by_path(organization_name, project_name) .await? .id(); let vpcs = @@ -1589,7 +1576,7 @@ impl Nexus { ) -> CreateResult { let project_id = self .db_datastore - .project_lookup_path(organization_name, project_name) + .project_lookup_by_path(organization_name, project_name) .await? .id(); let vpc_id = Uuid::new_v4(); @@ -1698,7 +1685,7 @@ impl Nexus { ) -> LookupResult { let project_id = self .db_datastore - .project_lookup_path(organization_name, project_name) + .project_lookup_by_path(organization_name, project_name) .await? .id(); Ok(self.db_datastore.vpc_fetch_by_name(&project_id, vpc_name).await?) @@ -1713,7 +1700,7 @@ impl Nexus { ) -> UpdateResult<()> { let project_id = self .db_datastore - .project_lookup_path(organization_name, project_name) + .project_lookup_by_path(organization_name, project_name) .await? .id(); let vpc = @@ -2324,14 +2311,16 @@ impl Nexus { pub async fn notify_disk_updated( &self, - id: &Uuid, + opctx: &OpContext, + id: Uuid, new_state: &DiskRuntimeState, ) -> Result<(), Error> { let log = &self.log; + let authz_disk = self.db_datastore.disk_lookup_by_id(id).await?; let result = self .db_datastore - .disk_update_runtime(id, &new_state.clone().into()) + .disk_update_runtime(opctx, &authz_disk, &new_state.clone().into()) .await; /* TODO-cleanup commonize with notify_instance_updated() */ @@ -2504,8 +2493,14 @@ impl TestInterfaces for Nexus { &self, id: &Uuid, ) -> Result, Error> { - let disk = self.db_datastore.disk_fetch(id).await?; - let instance_id = disk.runtime().attach_instance_id.unwrap(); + let opctx = OpContext::for_tests( + self.log.new(o!()), + Arc::clone(&self.db_datastore), + ); + let authz_disk = self.db_datastore.disk_lookup_by_id(*id).await?; + let db_disk = + self.db_datastore.disk_refetch(&opctx, &authz_disk).await?; + let instance_id = db_disk.runtime().attach_instance_id.unwrap(); let instance = self.db_datastore.instance_fetch(&instance_id).await?; self.instance_sled(&instance).await } diff --git a/nexus/src/sagas.rs b/nexus/src/sagas.rs index 7a30f34cfdb..db5110d4a08 100644 --- a/nexus/src/sagas.rs +++ b/nexus/src/sagas.rs @@ -740,12 +740,19 @@ async fn sdc_finalize_disk_record( ) -> Result<(), ActionError> { let osagactx = sagactx.user_data(); let _params = sagactx.saga_params(); + let datastore = osagactx.datastore(); let disk_id = sagactx.lookup::("disk_id")?; let disk_created = sagactx.lookup::("created_disk")?; - osagactx - .datastore() - .disk_update_runtime(&disk_id, &disk_created.runtime().detach()) + let authz_disk = datastore + .disk_lookup_by_id(disk_id) + .await + .map_err(ActionError::action_failed)?; + datastore + .disk_update_runtime_no_auth( + &authz_disk, + &disk_created.runtime().detach(), + ) .await .map_err(ActionError::action_failed)?; Ok(()) diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index 8f1ba7df714..5973149d075 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -2,6 +2,8 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. +use crate::ControlPlaneTestContext; + use super::http_testing::dropshot_compat::objects_post; use super::http_testing::AuthnMode; use super::http_testing::NexusRequest; @@ -13,8 +15,12 @@ use omicron_common::api::external::ByteCount; use omicron_common::api::external::Disk; use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::VpcRouter; +use omicron_nexus::crucible_agent_client::types::State as RegionState; use omicron_nexus::external_api::params; use omicron_nexus::external_api::views::{Organization, Project, Vpc}; +use omicron_sled_agent::sim::SledAgent; +use std::sync::Arc; +use uuid::Uuid; pub async fn objects_list_page_authz( client: &ClientTestContext, @@ -200,3 +206,38 @@ pub async fn project_get( .parsed_body() .expect("failed to parse Project") } + +pub struct DiskTest { + pub sled_agent: Arc, + pub zpool_id: Uuid, + pub zpool_size: ByteCount, + pub dataset_ids: Vec, +} + +impl DiskTest { + // Creates fake physical storage, an organization, and a project. + pub async fn new(cptestctx: &ControlPlaneTestContext) -> Self { + let sled_agent = cptestctx.sled_agent.sled_agent.clone(); + + // Create a Zpool. + let zpool_id = Uuid::new_v4(); + let zpool_size = ByteCount::from_gibibytes_u32(10); + sled_agent.create_zpool(zpool_id, zpool_size.to_bytes()).await; + + // Create multiple Datasets within that Zpool. + let dataset_count = 3; + let dataset_ids: Vec<_> = + (0..dataset_count).map(|_| Uuid::new_v4()).collect(); + for id in &dataset_ids { + sled_agent.create_crucible_dataset(zpool_id, *id).await; + + // By default, regions are created immediately. + let crucible = sled_agent.get_crucible_dataset(zpool_id, *id).await; + crucible + .set_create_callback(Box::new(|_| RegionState::Created)) + .await; + } + + Self { sled_agent, zpool_id, zpool_size, dataset_ids } + } +} diff --git a/nexus/tests/integration_tests/disks.rs b/nexus/tests/integration_tests/disks.rs index d7af2dd525d..50e0e4d0edf 100644 --- a/nexus/tests/integration_tests/disks.rs +++ b/nexus/tests/integration_tests/disks.rs @@ -14,17 +14,15 @@ use omicron_common::api::external::DiskState; use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::Instance; use omicron_common::api::external::InstanceCpuCount; +use omicron_common::api::external::Name; use omicron_nexus::TestInterfaces as _; use omicron_nexus::{external_api::params, Nexus}; -use omicron_sled_agent::sim::SledAgent; use sled_agent_client::TestInterfaces as _; use std::sync::Arc; use uuid::Uuid; -use dropshot::test_util::object_get; use dropshot::test_util::objects_list_page; use dropshot::test_util::objects_post; -use dropshot::test_util::read_json; use dropshot::test_util::ClientTestContext; use nexus_test_utils::http_testing::AuthnMode; @@ -34,6 +32,7 @@ use nexus_test_utils::identity_eq; use nexus_test_utils::resource_helpers::create_disk; use nexus_test_utils::resource_helpers::create_organization; use nexus_test_utils::resource_helpers::create_project; +use nexus_test_utils::resource_helpers::DiskTest; use nexus_test_utils::ControlPlaneTestContext; use nexus_test_utils_macros::nexus_test; @@ -71,52 +70,13 @@ async fn create_org_and_project(client: &ClientTestContext) -> Uuid { project.identity.id } -struct DiskTest { - sled_agent: Arc, - zpool_id: Uuid, - zpool_size: ByteCount, - dataset_ids: Vec, - project_id: Uuid, -} - -impl DiskTest { - // Creates fake physical storage, an organization, and a project. - async fn new(cptestctx: &ControlPlaneTestContext) -> Self { - let client = &cptestctx.external_client; - let sled_agent = cptestctx.sled_agent.sled_agent.clone(); - - // Create a Zpool. - let zpool_id = Uuid::new_v4(); - let zpool_size = ByteCount::from_gibibytes_u32(10); - sled_agent.create_zpool(zpool_id, zpool_size.to_bytes()).await; - - // Create multiple Datasets within that Zpool. - let dataset_count = 3; - let dataset_ids: Vec<_> = - (0..dataset_count).map(|_| Uuid::new_v4()).collect(); - for id in &dataset_ids { - sled_agent.create_crucible_dataset(zpool_id, *id).await; - - // By default, regions are created immediately. - let crucible = sled_agent.get_crucible_dataset(zpool_id, *id).await; - crucible - .set_create_callback(Box::new(|_| RegionState::Created)) - .await; - } - - // Create a project for testing. - let project_id = create_org_and_project(&client).await; - - Self { sled_agent, zpool_id, zpool_size, dataset_ids, project_id } - } -} - #[nexus_test] async fn test_disk_not_found_before_creation( cptestctx: &ControlPlaneTestContext, ) { let client = &cptestctx.external_client; DiskTest::new(&cptestctx).await; + create_org_and_project(client).await; let disks_url = get_disks_url(); // List disks. There aren't any yet. @@ -155,7 +115,8 @@ async fn test_disk_create_attach_detach_delete( cptestctx: &ControlPlaneTestContext, ) { let client = &cptestctx.external_client; - let test = DiskTest::new(&cptestctx).await; + DiskTest::new(&cptestctx).await; + let project_id = create_org_and_project(client).await; let nexus = &cptestctx.server.apictx.nexus; let disks_url = get_disks_url(); @@ -164,7 +125,7 @@ async fn test_disk_create_attach_detach_delete( let disk = create_disk(&client, ORG_NAME, PROJECT_NAME, DISK_NAME).await; assert_eq!(disk.identity.name, DISK_NAME); assert_eq!(disk.identity.description, "sells rainsticks"); - assert_eq!(disk.project_id, test.project_id); + assert_eq!(disk.project_id, project_id); assert_eq!(disk.snapshot_id, None); assert_eq!(disk.size.to_whole_mebibytes(), 1024); assert_eq!(disk.state, DiskState::Creating); @@ -175,7 +136,7 @@ async fn test_disk_create_attach_detach_delete( let disk = disk_get(&client, &disk_url).await; assert_eq!(disk.identity.name, DISK_NAME); assert_eq!(disk.identity.description, "sells rainsticks"); - assert_eq!(disk.project_id, test.project_id); + assert_eq!(disk.project_id, project_id); assert_eq!(disk.snapshot_id, None); assert_eq!(disk.size.to_whole_mebibytes(), 1024); assert_eq!(disk.state, DiskState::Detached); @@ -215,16 +176,12 @@ async fn test_disk_create_attach_detach_delete( get_disk_detach_url(instance.identity.name.as_str()); // Start attaching the disk to the instance. - let mut response = client - .make_request( - Method::POST, - &url_instance_attach_disk, - Some(params::DiskIdentifier { disk: disk.identity.name.clone() }), - StatusCode::ACCEPTED, - ) - .await - .unwrap(); - let attached_disk: Disk = read_json(&mut response).await; + let attached_disk = disk_post( + client, + &url_instance_attach_disk, + disk.identity.name.clone(), + ) + .await; let instance_id = &instance.identity.id; assert_eq!(attached_disk.identity.name, disk.identity.name); assert_eq!(attached_disk.identity.id, disk.identity.id); @@ -240,28 +197,20 @@ async fn test_disk_create_attach_detach_delete( // Attach the disk to the same instance. This should complete immediately // with no state change. - client - .make_request( - Method::POST, - &url_instance_attach_disk, - Some(params::DiskIdentifier { disk: disk.identity.name }), - StatusCode::ACCEPTED, - ) - .await - .unwrap(); + let disk = + disk_post(client, &url_instance_attach_disk, disk.identity.name).await; + assert_eq!(disk.state, DiskState::Attached(instance_id.clone())); let disk = disk_get(&client, &disk_url).await; assert_eq!(disk.state, DiskState::Attached(instance_id.clone())); // Begin detaching the disk. - client - .make_request( - Method::POST, - &url_instance_detach_disk, - Some(params::DiskIdentifier { disk: disk.identity.name.clone() }), - StatusCode::ACCEPTED, - ) - .await - .unwrap(); + let disk = disk_post( + client, + &url_instance_detach_disk, + disk.identity.name.clone(), + ) + .await; + assert_eq!(disk.state, DiskState::Detaching(instance_id.clone())); let disk: Disk = disk_get(&client, &disk_url).await; assert_eq!(disk.state, DiskState::Detaching(instance_id.clone())); @@ -271,17 +220,15 @@ async fn test_disk_create_attach_detach_delete( assert_eq!(disk.state, DiskState::Detached); // Since detach is idempotent, we can detach it again. - client - .make_request( - Method::POST, - &url_instance_detach_disk, - Some(params::DiskIdentifier { disk: disk.identity.name.clone() }), - StatusCode::ACCEPTED, - ) - .await - .unwrap(); + let disk = disk_post( + client, + &url_instance_detach_disk, + disk.identity.name.clone(), + ) + .await; + assert_eq!(disk.state, DiskState::Detached); - // A priveleged user should be able to delete the disk. + // Delete the disk. NexusRequest::object_delete(client, &disk_url) .authn_as(AuthnMode::PrivilegedUser) .execute() @@ -307,6 +254,7 @@ async fn test_disk_create_disk_that_already_exists_fails( ) { let client = &cptestctx.external_client; DiskTest::new(&cptestctx).await; + create_org_and_project(client).await; let disks_url = get_disks_url(); // Create a disk. @@ -350,6 +298,7 @@ async fn test_disk_move_between_instances(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; let nexus = &cptestctx.server.apictx.nexus; DiskTest::new(&cptestctx).await; + create_org_and_project(&client).await; let disks_url = get_disks_url(); // Create a disk. @@ -386,16 +335,12 @@ async fn test_disk_move_between_instances(cptestctx: &ControlPlaneTestContext) { get_disk_detach_url(instance.identity.name.as_str()); // Start attaching the disk to the instance. - let mut response = client - .make_request( - Method::POST, - &url_instance_attach_disk, - Some(params::DiskIdentifier { disk: disk.identity.name.clone() }), - StatusCode::ACCEPTED, - ) - .await - .unwrap(); - let attached_disk: Disk = read_json(&mut response).await; + let attached_disk = disk_post( + client, + &url_instance_attach_disk, + disk.identity.name.clone(), + ) + .await; let instance_id = &instance.identity.id; assert_eq!(attached_disk.identity.name, disk.identity.name); assert_eq!(attached_disk.identity.id, disk.identity.id); @@ -411,16 +356,8 @@ async fn test_disk_move_between_instances(cptestctx: &ControlPlaneTestContext) { // Attach the disk to the same instance. This should complete immediately // with no state change. - client - .make_request( - Method::POST, - &url_instance_attach_disk, - Some(params::DiskIdentifier { disk: disk.identity.name }), - StatusCode::ACCEPTED, - ) - .await - .unwrap(); - let disk = disk_get(&client, &disk_url).await; + let disk = + disk_post(client, &url_instance_attach_disk, disk.identity.name).await; assert_eq!(disk.state, DiskState::Attached(instance_id.clone())); // Create a second instance and try to attach the disk to that. This should @@ -443,19 +380,24 @@ async fn test_disk_move_between_instances(cptestctx: &ControlPlaneTestContext) { get_disk_attach_url(instance2.identity.name.as_str()); let url_instance2_detach_disk = get_disk_detach_url(instance2.identity.name.as_str()); - let error = client - .make_request_error_body( - Method::POST, - &url_instance2_attach_disk, - params::DiskIdentifier { disk: disk.identity.name.clone() }, - StatusCode::BAD_REQUEST, - ) - .await; + + let error: HttpErrorResponseBody = NexusRequest::new( + RequestBuilder::new(client, Method::POST, &url_instance2_attach_disk) + .body(Some(¶ms::DiskIdentifier { + disk: disk.identity.name.clone(), + })) + .expect_status(Some(StatusCode::BAD_REQUEST)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); assert_eq!( error.message, format!( - "cannot attach disk \"{}\": disk is attached to another \ - instance", + "cannot attach disk \"{}\": disk is attached to another instance", DISK_NAME ) ); @@ -464,65 +406,60 @@ async fn test_disk_move_between_instances(cptestctx: &ControlPlaneTestContext) { assert_eq!(attached_disk.state, DiskState::Attached(instance_id.clone())); // Begin detaching the disk. - client - .make_request( - Method::POST, - &url_instance_detach_disk, - Some(params::DiskIdentifier { disk: disk.identity.name.clone() }), - StatusCode::ACCEPTED, - ) - .await - .unwrap(); - let disk: Disk = disk_get(&client, &disk_url).await; + let disk = + disk_post(client, &url_instance_detach_disk, disk.identity.name).await; + assert_eq!(disk.state, DiskState::Detaching(instance_id.clone())); + let disk = disk_get(&client, &disk_url).await; assert_eq!(disk.state, DiskState::Detaching(instance_id.clone())); // It's still illegal to attach this disk elsewhere. - let error = client - .make_request_error_body( - Method::POST, - &url_instance2_attach_disk, - Some(params::DiskIdentifier { disk: disk.identity.name.clone() }), - StatusCode::BAD_REQUEST, - ) - .await; + let error: HttpErrorResponseBody = NexusRequest::new( + RequestBuilder::new(client, Method::POST, &url_instance2_attach_disk) + .body(Some(¶ms::DiskIdentifier { + disk: disk.identity.name.clone(), + })) + .expect_status(Some(StatusCode::BAD_REQUEST)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); assert_eq!( error.message, format!( - "cannot attach disk \"{}\": disk is attached to another \ - instance", + "cannot attach disk \"{}\": disk is attached to another instance", DISK_NAME ) ); // It's even illegal to attach this disk back to the same instance. - let error = client - .make_request_error_body( - Method::POST, - &url_instance_attach_disk, - Some(params::DiskIdentifier { disk: disk.identity.name.clone() }), - StatusCode::BAD_REQUEST, - ) - .await; - // TODO-debug the error message here is misleading. + let error: HttpErrorResponseBody = NexusRequest::new( + RequestBuilder::new(client, Method::POST, &url_instance_attach_disk) + .body(Some(¶ms::DiskIdentifier { + disk: disk.identity.name.clone(), + })) + .expect_status(Some(StatusCode::BAD_REQUEST)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); assert_eq!( error.message, format!( - "cannot attach disk \"{}\": disk is attached to another \ - instance", + "cannot attach disk \"{}\": disk is attached to another instance", DISK_NAME ) ); // However, there's no problem attempting to detach it again. - client - .make_request( - Method::POST, - &url_instance_detach_disk, - Some(params::DiskIdentifier { disk: disk.identity.name.clone() }), - StatusCode::ACCEPTED, - ) - .await - .unwrap(); + let disk = + disk_post(client, &url_instance_detach_disk, disk.identity.name).await; + assert_eq!(disk.state, DiskState::Detaching(instance_id.clone())); let disk = disk_get(&client, &disk_url).await; assert_eq!(disk.state, DiskState::Detaching(instance_id.clone())); @@ -532,36 +469,20 @@ async fn test_disk_move_between_instances(cptestctx: &ControlPlaneTestContext) { assert_eq!(disk.state, DiskState::Detached); // Since delete is idempotent, we can detach it again -- from either one. - client - .make_request( - Method::POST, - &url_instance_detach_disk, - Some(params::DiskIdentifier { disk: disk.identity.name.clone() }), - StatusCode::ACCEPTED, - ) - .await - .unwrap(); - client - .make_request( - Method::POST, - &url_instance2_detach_disk, - Some(params::DiskIdentifier { disk: disk.identity.name.clone() }), - StatusCode::ACCEPTED, - ) - .await - .unwrap(); + let disk = + disk_post(client, &url_instance_detach_disk, disk.identity.name).await; + assert_eq!(disk.state, DiskState::Detached); + let disk = + disk_post(client, &url_instance2_detach_disk, disk.identity.name).await; + assert_eq!(disk.state, DiskState::Detached); // Now, start attaching it again to the second instance. - let mut response = client - .make_request( - Method::POST, - &url_instance2_attach_disk, - Some(params::DiskIdentifier { disk: disk.identity.name.clone() }), - StatusCode::ACCEPTED, - ) - .await - .unwrap(); - let attached_disk: Disk = read_json(&mut response).await; + let attached_disk = disk_post( + client, + &url_instance2_attach_disk, + disk.identity.name.clone(), + ) + .await; let instance2_id = &instance2.identity.id; assert_eq!(attached_disk.identity.name, disk.identity.name); assert_eq!(attached_disk.identity.id, disk.identity.id); @@ -572,40 +493,44 @@ async fn test_disk_move_between_instances(cptestctx: &ControlPlaneTestContext) { // At this point, it's not legal to attempt to attach it to a different // instance (the first one). - let error = client - .make_request_error_body( - Method::POST, - &url_instance_attach_disk, - Some(params::DiskIdentifier { disk: disk.identity.name.clone() }), - StatusCode::BAD_REQUEST, - ) - .await; + let error: HttpErrorResponseBody = NexusRequest::new( + RequestBuilder::new(client, Method::POST, &url_instance_attach_disk) + .body(Some(¶ms::DiskIdentifier { + disk: disk.identity.name.clone(), + })) + .expect_status(Some(StatusCode::BAD_REQUEST)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); assert_eq!( error.message, format!( - "cannot attach disk \"{}\": disk is attached to another \ - instance", + "cannot attach disk \"{}\": disk is attached to another instance", DISK_NAME ) ); // It's fine to attempt another attachment to the same instance. - client - .make_request( - Method::POST, - &url_instance2_attach_disk, - Some(params::DiskIdentifier { disk: disk.identity.name.clone() }), - StatusCode::ACCEPTED, - ) - .await - .unwrap(); + let disk = disk_post( + client, + &url_instance2_attach_disk, + disk.identity.name.clone(), + ) + .await; + assert_eq!(disk.state, DiskState::Attaching(instance2_id.clone())); let disk = disk_get(&client, &disk_url).await; assert_eq!(disk.state, DiskState::Attaching(instance2_id.clone())); // It's not allowed to delete a disk that's attaching. - let error = NexusRequest::new( - RequestBuilder::new(client, Method::DELETE, &disk_url) - .expect_status(Some(StatusCode::BAD_REQUEST)), + let error = NexusRequest::expect_failure( + client, + StatusCode::BAD_REQUEST, + Method::DELETE, + &disk_url, ) .authn_as(AuthnMode::PrivilegedUser) .execute() @@ -616,22 +541,22 @@ async fn test_disk_move_between_instances(cptestctx: &ControlPlaneTestContext) { assert_eq!(error.message, "disk cannot be deleted in state \"attaching\""); // Now, begin a detach while the disk is still being attached. - client - .make_request( - Method::POST, - &url_instance2_detach_disk, - Some(params::DiskIdentifier { disk: disk.identity.name.clone() }), - StatusCode::ACCEPTED, - ) - .await - .unwrap(); + let disk = disk_post( + client, + &url_instance2_detach_disk, + disk.identity.name.clone(), + ) + .await; + assert_eq!(disk.state, DiskState::Detaching(instance2_id.clone())); let disk: Disk = disk_get(&client, &disk_url).await; assert_eq!(disk.state, DiskState::Detaching(instance2_id.clone())); // It's not allowed to delete a disk that's detaching, either. - let error = NexusRequest::new( - RequestBuilder::new(client, Method::DELETE, &disk_url) - .expect_status(Some(StatusCode::BAD_REQUEST)), + let error = NexusRequest::expect_failure( + client, + StatusCode::BAD_REQUEST, + Method::DELETE, + &disk_url, ) .authn_as(AuthnMode::PrivilegedUser) .execute() @@ -646,6 +571,7 @@ async fn test_disk_move_between_instances(cptestctx: &ControlPlaneTestContext) { let disk = disk_get(&client, &disk_url).await; assert_eq!(disk.state, DiskState::Detached); + // Now we can delete the disk. NexusRequest::object_delete(client, &disk_url) .authn_as(AuthnMode::PrivilegedUser) .execute() @@ -656,9 +582,18 @@ async fn test_disk_move_between_instances(cptestctx: &ControlPlaneTestContext) { assert_eq!(disks_list(&client, &disks_url).await.len(), 0); // We shouldn't find it if we request it explicitly. - let error = client - .make_request_error(Method::GET, &disk_url, StatusCode::NOT_FOUND) - .await; + let error: HttpErrorResponseBody = NexusRequest::expect_failure( + client, + StatusCode::NOT_FOUND, + Method::GET, + &disk_url, + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); assert_eq!( error.message, format!("not found: disk with name \"{}\"", DISK_NAME) @@ -671,6 +606,7 @@ async fn test_disk_creation_region_requested_then_started( ) { let client = &cptestctx.external_client; let test = DiskTest::new(&cptestctx).await; + create_org_and_project(client).await; // Before we create a disk, set the response from the Crucible Agent: // no matter what regions get requested, they'll always *start* as @@ -703,6 +639,7 @@ async fn test_disk_region_creation_failure( ) { let client = &cptestctx.external_client; let test = DiskTest::new(&cptestctx).await; + create_org_and_project(client).await; // Before we create a disk, set the response from the Crucible Agent: // no matter what regions get requested, they'll always fail. @@ -774,7 +711,13 @@ async fn test_disk_region_creation_failure( } async fn disk_get(client: &ClientTestContext, disk_url: &str) -> Disk { - object_get::(client, disk_url).await + NexusRequest::object_get(client, disk_url) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap() } async fn disks_list(client: &ClientTestContext, list_url: &str) -> Vec { @@ -784,6 +727,24 @@ async fn disks_list(client: &ClientTestContext, list_url: &str) -> Vec { .all_items } +async fn disk_post( + client: &ClientTestContext, + url: &str, + disk_name: Name, +) -> Disk { + NexusRequest::new( + RequestBuilder::new(client, Method::POST, url) + .body(Some(¶ms::DiskIdentifier { disk: disk_name })) + .expect_status(Some(StatusCode::ACCEPTED)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap() +} + fn disks_eq(disk1: &Disk, disk2: &Disk) { identity_eq(&disk1.identity, &disk2.identity); assert_eq!(disk1.project_id, disk2.project_id); diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index 7abbdbea957..d41c9ce1600 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -14,11 +14,13 @@ use nexus_test_utils::http_testing::AuthnMode; use nexus_test_utils::http_testing::NexusRequest; use nexus_test_utils::http_testing::RequestBuilder; use nexus_test_utils::http_testing::TestResponse; +use nexus_test_utils::resource_helpers::DiskTest; use nexus_test_utils::ControlPlaneTestContext; use nexus_test_utils_macros::nexus_test; use omicron_common::api::external::ByteCount; use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::IdentityMetadataUpdateParams; +use omicron_common::api::external::InstanceCpuCount; use omicron_common::api::external::Name; use omicron_nexus::authn; use omicron_nexus::authn::external::spoof::HTTP_HEADER_OXIDE_AUTHN_SPOOF; @@ -54,6 +56,7 @@ use omicron_nexus::external_api::params; // from the OpenAPI spec? #[nexus_test] async fn test_unauthorized(cptestctx: &ControlPlaneTestContext) { + DiskTest::new(cptestctx).await; let client = &cptestctx.external_client; let log = &cptestctx.logctx.log; @@ -103,6 +106,16 @@ lazy_static! { url: &*DEMO_ORG_PROJECTS_URL, body: serde_json::to_value(&*DEMO_PROJECT_CREATE).unwrap(), }, + // Create a Disk in the Project + SetupReq { + url: &*DEMO_PROJECT_URL_DISKS, + body: serde_json::to_value(&*DEMO_DISK_CREATE).unwrap(), + }, + // Create an Instance in the Project + SetupReq { + url: &*DEMO_PROJECT_URL_INSTANCES, + body: serde_json::to_value(&*DEMO_INSTANCE_CREATE).unwrap(), + }, ]; // Organization used for testing @@ -125,6 +138,8 @@ lazy_static! { format!("{}/{}", *DEMO_ORG_PROJECTS_URL, *DEMO_PROJECT_NAME); static ref DEMO_PROJECT_URL_DISKS: String = format!("{}/disks", *DEMO_PROJECT_URL); + static ref DEMO_PROJECT_URL_INSTANCES: String = + format!("{}/instances", *DEMO_PROJECT_URL); static ref DEMO_PROJECT_CREATE: params::ProjectCreate = params::ProjectCreate { identity: IdentityMetadataCreateParams { @@ -140,12 +155,33 @@ lazy_static! { static ref DEMO_DISK_CREATE: params::DiskCreate = params::DiskCreate { identity: IdentityMetadataCreateParams { - name: DEMO_PROJECT_NAME.clone(), + name: DEMO_DISK_NAME.clone(), description: "".parse().unwrap(), }, snapshot_id: None, size: ByteCount::from_gibibytes_u32(16), }; + + // Instance used for testing + static ref DEMO_INSTANCE_NAME: Name = "demo-instance".parse().unwrap(); + static ref DEMO_INSTANCE_URL: String = + format!("{}/{}", *DEMO_PROJECT_URL_INSTANCES, *DEMO_INSTANCE_NAME); + static ref DEMO_INSTANCE_DISKS_URL: String = + format!("{}/disks", *DEMO_INSTANCE_URL); + static ref DEMO_INSTANCE_DISKS_ATTACH_URL: String = + format!("{}/attach", *DEMO_INSTANCE_DISKS_URL); + static ref DEMO_INSTANCE_DISKS_DETACH_URL: String = + format!("{}/detach", *DEMO_INSTANCE_DISKS_URL); + static ref DEMO_INSTANCE_CREATE: params::InstanceCreate = + params::InstanceCreate { + identity: IdentityMetadataCreateParams { + name: DEMO_DISK_NAME.clone(), + description: "".parse().unwrap(), + }, + ncpus: InstanceCpuCount(1), + memory: ByteCount::from_gibibytes_u32(16), + hostname: String::from("demo-instance"), + }; } // @@ -262,7 +298,7 @@ lazy_static! { serde_json::to_value(¶ms::OrganizationUpdate { identity: IdentityMetadataUpdateParams { name: None, - description: None, + description: Some("different".to_string()) } }).unwrap() ), @@ -297,7 +333,7 @@ lazy_static! { serde_json::to_value(params::ProjectUpdate{ identity: IdentityMetadataUpdateParams { name: None, - description: None, + description: Some("different".to_string()) }, }).unwrap() ), @@ -315,8 +351,37 @@ lazy_static! { ], }, - // TODO-coverage The single disk endpoint belongs here, but we've only - // implemented authz for DELETE, not GET yet. + VerifyEndpoint { + url: &*DEMO_DISK_URL, + visibility: Visibility::Protected, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Delete, + ], + }, + + VerifyEndpoint { + url: &*DEMO_INSTANCE_DISKS_ATTACH_URL, + visibility: Visibility::Protected, + allowed_methods: vec![ + AllowedMethod::Post( + serde_json::to_value(params::DiskIdentifier { + disk: DEMO_DISK_NAME.clone() + }).unwrap() + ) + ], + }, + VerifyEndpoint { + url: &*DEMO_INSTANCE_DISKS_DETACH_URL, + visibility: Visibility::Protected, + allowed_methods: vec![ + AllowedMethod::Post( + serde_json::to_value(params::DiskIdentifier { + disk: DEMO_DISK_NAME.clone() + }).unwrap() + ) + ], + }, VerifyEndpoint { url: "/roles", @@ -418,12 +483,25 @@ async fn verify_endpoint( // response. Otherwise, the test might later succeed by coincidence. We // might find a 404 because of something that actually doesn't exist rather // than something that's just hidden from unauthorized users. - info!(log, "test: privileged GET"); - NexusRequest::object_get(client, endpoint.url) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .unwrap(); + let get_allowed = endpoint + .allowed_methods + .iter() + .any(|allowed| allowed.http_method() == Method::GET); + let resource_before: Option = if get_allowed { + info!(log, "test: privileged GET"); + Some( + NexusRequest::object_get(client, endpoint.url) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(), + ) + } else { + warn!(log, "test: skipping privileged GET (method not allowed)"); + None + }; // For each of the HTTP methods we use in the API as well as TRACE, we'll // make several requests to this URL and verify the results. @@ -516,6 +594,34 @@ async fn verify_endpoint( .unwrap(); verify_response(&response); } + + // If we fetched the resource earlier, fetch it again and check the state. + // We're trying to catch cases where an endpoint correctly returns an error + // but still applied the result. + // + // This might seem gratuitous but it's an important check for resources like + // disk attachment and detachment, where Nexus reaches out to the Sled Agent + // before making a database change. If Nexus only authorized the request at + // the database query (as is our current emphasis), we could wind up making + // the change to the system even for unauthorized users (and still returning + // an "unauthorized" error)! + // TODO-coverage It would be good to check the ETag here as well, once we + // provide one. + info!(log, "test: compare current resource content with earlier"); + if let Some(resource_before) = resource_before { + let resource_after: serde_json::Value = + NexusRequest::object_get(client, endpoint.url) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); + assert_eq!( + resource_before, resource_after, + "resource changed after making a bunch of failed requests" + ); + } } /// Verifies the body of an HTTP response for status codes 401, 403, 404, or 405 diff --git a/nexus/tests/integration_tests/users_builtin.rs b/nexus/tests/integration_tests/users_builtin.rs index 50f737f4dfa..7796d60ad33 100644 --- a/nexus/tests/integration_tests/users_builtin.rs +++ b/nexus/tests/integration_tests/users_builtin.rs @@ -27,6 +27,8 @@ async fn test_users_builtin(cptestctx: &ControlPlaneTestContext) { let u = users.remove(&authn::USER_DB_INIT.name.to_string()).unwrap(); assert_eq!(u.identity.id, authn::USER_DB_INIT.id); + let u = users.remove(&authn::USER_INTERNAL_API.name.to_string()).unwrap(); + assert_eq!(u.identity.id, authn::USER_INTERNAL_API.id); let u = users.remove(&authn::USER_SAGA_RECOVERY.name.to_string()).unwrap(); assert_eq!(u.identity.id, authn::USER_SAGA_RECOVERY.id); let u = diff --git a/tools/oxapi_demo b/tools/oxapi_demo index 1851f0683c2..2eeb9a13ae1 100755 --- a/tools/oxapi_demo +++ b/tools/oxapi_demo @@ -231,7 +231,7 @@ function cmd_project_create_demo function cmd_project_delete { [[ $# != 2 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME" - do_curl "/organizations/$1/projects/$2" -X DELETE + do_curl_authn "/organizations/$1/projects/$2" -X DELETE } function cmd_project_get @@ -308,14 +308,14 @@ function cmd_instance_attach_disk { [[ $# != 4 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME INSTANCE_NAME DISK_NAME" mkjson disk="$4" | - do_curl "/organizations/$1/projects/$2/instances/$3/disks/attach" -X POST -T - + do_curl_authn "/organizations/$1/projects/$2/instances/$3/disks/attach" -X POST -T - } function cmd_instance_detach_disk { [[ $# != 4 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME INSTANCE_NAME DISK_NAME" mkjson disk="$4" | - do_curl "/organizations/$1/projects/$2/instances/$3/disks/detach" -X POST -T - + do_curl_authn "/organizations/$1/projects/$2/instances/$3/disks/detach" -X POST -T - } function cmd_instance_list_disks @@ -334,7 +334,7 @@ function cmd_disk_create_demo function cmd_disk_get { [[ $# != 3 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME DISK_NAME" - do_curl "/organizations/$1/projects/$2/disks/$3" + do_curl_authn "/organizations/$1/projects/$2/disks/$3" } function cmd_disk_delete