From 1983532482b447f9679b6d76328b275a7609c3e8 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Fri, 15 Apr 2022 13:18:03 -0700 Subject: [PATCH] authz: silo endpoints --- nexus/src/authn/external/mod.rs | 4 +- nexus/src/authn/external/spoof.rs | 8 +- nexus/src/authn/mod.rs | 2 +- nexus/src/db/datastore.rs | 132 ++++++++---------- nexus/src/db/fixed_data/mod.rs | 2 +- nexus/src/db/fixed_data/silo.rs | 24 ++++ nexus/src/db/fixed_data/silo_builtin.rs | 11 -- nexus/src/db/fixed_data/user_builtin.rs | 2 +- nexus/src/external_api/http_entrypoints.rs | 2 - nexus/src/nexus.rs | 7 +- nexus/tests/integration_tests/endpoints.rs | 36 +++++ nexus/tests/integration_tests/silos.rs | 2 +- .../output/uncovered-authz-endpoints.txt | 4 - 13 files changed, 133 insertions(+), 103 deletions(-) create mode 100644 nexus/src/db/fixed_data/silo.rs delete mode 100644 nexus/src/db/fixed_data/silo_builtin.rs diff --git a/nexus/src/authn/external/mod.rs b/nexus/src/authn/external/mod.rs index e50c15c0c5..dcda0364a2 100644 --- a/nexus/src/authn/external/mod.rs +++ b/nexus/src/authn/external/mod.rs @@ -189,7 +189,7 @@ mod test { let name1 = authn::SchemeName("grunt1"); let actor1 = authn::Actor { id: "1c91bab2-4841-669f-cc32-de80da5bbf39".parse().unwrap(), - silo_id: *crate::db::fixed_data::silo_builtin::SILO_ID, + silo_id: *crate::db::fixed_data::silo::SILO_ID, }; let grunt1 = Box::new(GruntScheme { name: name1, @@ -204,7 +204,7 @@ mod test { let name2 = authn::SchemeName("grunt2"); let actor2 = authn::Actor { id: "799684af-533a-cb66-b5ac-ab55a791d5ef".parse().unwrap(), - silo_id: *crate::db::fixed_data::silo_builtin::SILO_ID, + silo_id: *crate::db::fixed_data::silo::SILO_ID, }; let grunt2 = Box::new(GruntScheme { name: name2, diff --git a/nexus/src/authn/external/spoof.rs b/nexus/src/authn/external/spoof.rs index da1500ba16..d8d85993b5 100644 --- a/nexus/src/authn/external/spoof.rs +++ b/nexus/src/authn/external/spoof.rs @@ -57,7 +57,7 @@ lazy_static! { /// Actor (id) used for the special "bad credentials" error static ref SPOOF_RESERVED_BAD_CREDS_ACTOR: Actor = Actor { id: "22222222-2222-2222-2222-222222222222".parse().unwrap(), - silo_id: *crate::db::fixed_data::silo_builtin::SILO_ID, + silo_id: *crate::db::fixed_data::silo::SILO_ID, }; /// Complete HTTP header value to trigger the "bad actor" error pub static ref SPOOF_HEADER_BAD_ACTOR: Authorization = @@ -122,10 +122,8 @@ fn authn_spoof(raw_value: Option<&Authorization>) -> SchemeResult { match Uuid::parse_str(str_value).context("parsing header value as UUID") { Ok(id) => { - let actor = Actor { - id, - silo_id: *crate::db::fixed_data::silo_builtin::SILO_ID, - }; + let actor = + Actor { id, silo_id: *crate::db::fixed_data::silo::SILO_ID }; SchemeResult::Authenticated(Details { actor }) } Err(source) => SchemeResult::Failed(Reason::BadFormat { source }), diff --git a/nexus/src/authn/mod.rs b/nexus/src/authn/mod.rs index d3984a2174..af2ef22fc2 100644 --- a/nexus/src/authn/mod.rs +++ b/nexus/src/authn/mod.rs @@ -156,7 +156,7 @@ impl Context { pub fn test_context_for_actor(actor_id: Uuid) -> Context { Context::context_for_actor( actor_id, - *crate::db::fixed_data::silo_builtin::SILO_ID, + *crate::db::fixed_data::silo::SILO_ID, ) } } diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index e9a47a8bbc..e76bd3cd8a 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -31,7 +31,29 @@ use crate::authz::ApiResourceError; use crate::context::OpContext; use crate::db::fixed_data::role_assignment_builtin::BUILTIN_ROLE_ASSIGNMENTS; use crate::db::fixed_data::role_builtin::BUILTIN_ROLES; -use crate::db::fixed_data::silo_builtin::SILO_ID; +use crate::db::fixed_data::silo::{DEFAULT_SILO, SILO_ID}; +use crate::db::lookup::LookupPath; +use crate::db::{ + self, + error::{public_error_from_diesel_pool, ErrorHandler, TransactionError}, + model::{ + ConsoleSession, Dataset, DatasetKind, Disk, DiskRuntimeState, + Generation, IncompleteNetworkInterface, Instance, InstanceRuntimeState, + Name, NetworkInterface, Organization, OrganizationUpdate, OximeterInfo, + ProducerEndpoint, Project, ProjectUpdate, Region, + RoleAssignmentBuiltin, RoleBuiltin, RouterRoute, RouterRouteUpdate, + Silo, SiloUser, Sled, UpdateAvailableArtifact, UserBuiltin, Volume, + Vpc, VpcFirewallRule, VpcRouter, VpcRouterUpdate, VpcSubnet, + VpcSubnetUpdate, VpcUpdate, Zpool, + }, + pagination::paginated, + pagination::paginated_multicolumn, + subnet_allocation::FilterConflictingVpcSubnetRangesQuery, + subnet_allocation::InsertNetworkInterfaceQuery, + subnet_allocation::NetworkInterfaceError, + subnet_allocation::SubnetError, + update_and_check::{UpdateAndCheck, UpdateStatus}, +}; use crate::external_api::params; use async_bb8_diesel::{AsyncConnection, AsyncRunQueryDsl, ConnectionManager}; use chrono::Utc; @@ -59,29 +81,6 @@ use std::net::Ipv6Addr; use std::sync::Arc; use uuid::Uuid; -use crate::db::lookup::LookupPath; -use crate::db::{ - self, - error::{public_error_from_diesel_pool, ErrorHandler, TransactionError}, - model::{ - ConsoleSession, Dataset, DatasetKind, Disk, DiskRuntimeState, - Generation, IncompleteNetworkInterface, Instance, InstanceRuntimeState, - Name, NetworkInterface, Organization, OrganizationUpdate, OximeterInfo, - ProducerEndpoint, Project, ProjectUpdate, Region, - RoleAssignmentBuiltin, RoleBuiltin, RouterRoute, RouterRouteUpdate, - Silo, SiloUser, Sled, UpdateAvailableArtifact, UserBuiltin, Volume, - Vpc, VpcFirewallRule, VpcRouter, VpcRouterUpdate, VpcSubnet, - VpcSubnetUpdate, VpcUpdate, Zpool, - }, - pagination::paginated, - pagination::paginated_multicolumn, - subnet_allocation::FilterConflictingVpcSubnetRangesQuery, - subnet_allocation::InsertNetworkInterfaceQuery, - subnet_allocation::NetworkInterfaceError, - subnet_allocation::SubnetError, - update_and_check::{UpdateAndCheck, UpdateStatus}, -}; - // Number of unique datasets required to back a region. // TODO: This should likely turn into a configuration option. const REGION_REDUNDANCY_THRESHOLD: usize = 3; @@ -2477,22 +2476,21 @@ impl DataStore { &self, opctx: &OpContext, ) -> Result<(), Error> { - debug!(opctx.log, "attempting to create built-in silo"); - - let builtin_silo = Silo::new_with_id( - *SILO_ID, - params::SiloCreate { - identity: IdentityMetadataCreateParams { - name: "fakesilo".parse().unwrap(), - description: "fake silo".to_string(), - }, - discoverable: false, - }, - ); + opctx.authorize(authz::Action::Modify, &authz::DATABASE).await?; - let _create_result = self.silo_create(opctx, builtin_silo).await?; - info!(opctx.log, "created built-in silo"); + debug!(opctx.log, "attempting to create built-in silo"); + use db::schema::silo::dsl; + let count = diesel::insert_into(dsl::silo) + .values(&*DEFAULT_SILO) + .on_conflict(dsl::id) + .do_nothing() + .execute_async(self.pool_authorized(opctx).await?) + .await + .map_err(|e| { + public_error_from_diesel_pool(e, ErrorHandler::Server) + })?; + info!(opctx.log, "created {} built-in silos", count); Ok(()) } @@ -2501,12 +2499,11 @@ impl DataStore { opctx: &OpContext, silo: Silo, ) -> CreateResult { - use db::schema::silo::dsl; - - // TODO opctx.authorize + opctx.authorize(authz::Action::CreateChild, &authz::FLEET).await?; let silo_id = silo.id(); + use db::schema::silo::dsl; diesel::insert_into(dsl::silo) .values(silo) .returning(Silo::as_returning()) @@ -2528,8 +2525,9 @@ impl DataStore { opctx: &OpContext, pagparams: &DataPageParams<'_, Uuid>, ) -> ListResultVec { + opctx.authorize(authz::Action::ListChildren, &authz::FLEET).await?; + use db::schema::silo::dsl; - // TODO opctx.authorize paginated(dsl::silo, dsl::id, pagparams) .filter(dsl::time_deleted.is_null()) .filter(dsl::discoverable.eq(true)) @@ -2544,8 +2542,9 @@ impl DataStore { opctx: &OpContext, pagparams: &DataPageParams<'_, Name>, ) -> ListResultVec { + opctx.authorize(authz::Action::ListChildren, &authz::FLEET).await?; + use db::schema::silo::dsl; - // TODO opctx.authorize paginated(dsl::silo, dsl::name, pagparams) .filter(dsl::time_deleted.is_null()) .filter(dsl::discoverable.eq(true)) @@ -2558,38 +2557,26 @@ impl DataStore { pub async fn silo_delete( &self, opctx: &OpContext, - name: &Name, + authz_silo: &authz::Silo, + db_silo: &db::model::Silo, ) -> DeleteResult { + assert_eq!(authz_silo.id(), db_silo.id()); + opctx.authorize(authz::Action::Delete, authz_silo).await?; + use db::schema::organization; use db::schema::silo; use db::schema::silo_user; - // TODO opctx.authorize - - let (id, rcgen) = silo::dsl::silo - .filter(silo::dsl::time_deleted.is_null()) - .filter(silo::dsl::name.eq(name.clone())) - .select((silo::dsl::id, silo::dsl::rcgen)) - .get_result_async::<(Uuid, Generation)>(self.pool()) - .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ErrorHandler::NotFoundByLookup( - ResourceType::Silo, - LookupType::ByName(name.to_string()), - ), - ) - })?; - // Make sure there are no organizations present within this silo. + let id = authz_silo.id(); + let rcgen = db_silo.rcgen; let org_found = diesel_pool_result_optional( organization::dsl::organization .filter(organization::dsl::silo_id.eq(id)) .filter(organization::dsl::time_deleted.is_null()) .select(organization::dsl::id) .limit(1) - .first_async::(self.pool()) + .first_async::(self.pool_authorized(opctx).await?) .await, ) .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server))?; @@ -2607,15 +2594,12 @@ impl DataStore { .filter(silo::dsl::id.eq(id)) .filter(silo::dsl::rcgen.eq(rcgen)) .set(silo::dsl::time_deleted.eq(now)) - .execute_async(self.pool()) + .execute_async(self.pool_authorized(opctx).await?) .await .map_err(|e| { public_error_from_diesel_pool( e, - ErrorHandler::NotFoundByLookup( - ResourceType::Silo, - LookupType::ById(id), - ), + ErrorHandler::NotFoundByResource(authz_silo), ) })?; @@ -2629,22 +2613,22 @@ impl DataStore { info!(opctx.log, "deleted silo {}", id); // If silo deletion succeeded, delete all silo users + // TODO-correctness This needs to happen in a saga or some other + // mechanism that ensures it happens even if we crash at this point. + // TODO-scalability This needs to happen in batches let updated_rows = diesel::update(silo_user::dsl::silo_user) .filter(silo_user::dsl::silo_id.eq(id)) .set(silo_user::dsl::time_deleted.eq(now)) - .execute_async(self.pool()) + .execute_async(self.pool_authorized(opctx).await?) .await .map_err(|e| { public_error_from_diesel_pool( e, - ErrorHandler::NotFoundByLookup( - ResourceType::Silo, - LookupType::ById(id), - ), + ErrorHandler::NotFoundByResource(authz_silo), ) })?; - info!(opctx.log, "deleted {} silo users for silo {}", updated_rows, id,); + info!(opctx.log, "deleted {} silo users for silo {}", updated_rows, id); Ok(()) } diff --git a/nexus/src/db/fixed_data/mod.rs b/nexus/src/db/fixed_data/mod.rs index 0137eec2ae..57ddf92987 100644 --- a/nexus/src/db/fixed_data/mod.rs +++ b/nexus/src/db/fixed_data/mod.rs @@ -32,7 +32,7 @@ use lazy_static::lazy_static; pub mod role_assignment_builtin; pub mod role_builtin; -pub mod silo_builtin; +pub mod silo; pub mod user_builtin; lazy_static! { diff --git a/nexus/src/db/fixed_data/silo.rs b/nexus/src/db/fixed_data/silo.rs new file mode 100644 index 0000000000..e328da8cf7 --- /dev/null +++ b/nexus/src/db/fixed_data/silo.rs @@ -0,0 +1,24 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// 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::db; +use crate::external_api::params; +use lazy_static::lazy_static; +use omicron_common::api::external::IdentityMetadataCreateParams; + +lazy_static! { + pub static ref SILO_ID: uuid::Uuid = "001de000-5110-4000-8000-000000000000" + .parse() + .expect("invalid uuid for builtin silo id"); + pub static ref DEFAULT_SILO: db::model::Silo = db::model::Silo::new_with_id( + *SILO_ID, + params::SiloCreate { + identity: IdentityMetadataCreateParams { + name: "default-silo".parse().unwrap(), + description: "default silo".to_string(), + }, + discoverable: false, + } + ); +} diff --git a/nexus/src/db/fixed_data/silo_builtin.rs b/nexus/src/db/fixed_data/silo_builtin.rs deleted file mode 100644 index 6610f673c4..0000000000 --- a/nexus/src/db/fixed_data/silo_builtin.rs +++ /dev/null @@ -1,11 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// 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 lazy_static::lazy_static; - -lazy_static! { - pub static ref SILO_ID: uuid::Uuid = "001de000-5110-4000-8000-000000000000" - .parse() - .expect("invalid uuid for builtin silo id"); -} diff --git a/nexus/src/db/fixed_data/user_builtin.rs b/nexus/src/db/fixed_data/user_builtin.rs index 22e83a60a3..f878feb187 100644 --- a/nexus/src/db/fixed_data/user_builtin.rs +++ b/nexus/src/db/fixed_data/user_builtin.rs @@ -3,7 +3,7 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. //! Built-in users -use crate::db::fixed_data::silo_builtin::SILO_ID; +use crate::db::fixed_data::silo::SILO_ID; use lazy_static::lazy_static; use omicron_common::api; use uuid::Uuid; diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index ad24c4b748..90046ff4c8 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -230,8 +230,6 @@ pub fn external_api() -> NexusApiDescription { // clients. Client generators use operationId to name API methods, so changing // a function name is a breaking change from a client perspective. -// TODO authz for silo endpoints - // List all silos (that are discoverable). #[endpoint { method = GET, diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index bd05aee1c5..d6546ce507 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -738,7 +738,12 @@ impl Nexus { opctx: &OpContext, name: &Name, ) -> DeleteResult { - self.db_datastore.silo_delete(opctx, name).await + let (.., authz_silo, db_silo) = + LookupPath::new(opctx, &self.db_datastore) + .silo_name(name) + .fetch_for(authz::Action::Delete) + .await?; + self.db_datastore.silo_delete(opctx, &authz_silo, &db_silo).await } // Organizations diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 0cc99c4b96..c4f8acc3b5 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -23,6 +23,8 @@ use omicron_common::api::external::RouterRouteCreateParams; use omicron_common::api::external::RouterRouteUpdateParams; use omicron_common::api::external::VpcFirewallRuleUpdateParams; use omicron_nexus::authn; +use omicron_nexus::db::fixed_data::silo::DEFAULT_SILO; +use omicron_nexus::db::identity::Resource; use omicron_nexus::external_api::params; use std::net::IpAddr; use std::net::Ipv4Addr; @@ -33,6 +35,19 @@ lazy_static! { pub static ref HARDWARE_SLED_URL: String = format!("/hardware/sleds/{}", SLED_AGENT_UUID); + // Silo used for testing + pub static ref DEFAULT_SILO_NAME: &'static Name = &DEFAULT_SILO.name().0; + pub static ref DEFAULT_SILO_URL: String = + format!("/silos/{}", *DEFAULT_SILO_NAME); + pub static ref DEMO_SILO_CREATE: params::SiloCreate = + params::SiloCreate { + identity: IdentityMetadataCreateParams { + name: DEFAULT_SILO_NAME.clone(), + description: String::from(""), + }, + discoverable: true, + }; + // Organization used for testing pub static ref DEMO_ORG_NAME: Name = "demo-org".parse().unwrap(); pub static ref DEMO_ORG_URL: String = @@ -348,6 +363,27 @@ lazy_static! { /// List of endpoints to be verified pub static ref VERIFY_ENDPOINTS: Vec = vec![ + /* Silos */ + VerifyEndpoint { + url: "/silos", + visibility: Visibility::Public, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_SILO_CREATE).unwrap() + ) + ], + }, + VerifyEndpoint { + url: &*DEFAULT_SILO_URL, + visibility: Visibility::Protected, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Delete, + ], + }, + + /* Organizations */ VerifyEndpoint { diff --git a/nexus/tests/integration_tests/silos.rs b/nexus/tests/integration_tests/silos.rs index 79a7c8991f..ab74ffc923 100644 --- a/nexus/tests/integration_tests/silos.rs +++ b/nexus/tests/integration_tests/silos.rs @@ -110,7 +110,7 @@ async fn test_silos(cptestctx: &ControlPlaneTestContext) { &client, StatusCode::BAD_REQUEST, Method::DELETE, - &"/silos/fakesilo", + &"/silos/default-silo", ) .authn_as(AuthnMode::PrivilegedUser) .execute() diff --git a/nexus/tests/output/uncovered-authz-endpoints.txt b/nexus/tests/output/uncovered-authz-endpoints.txt index 37134686de..48197f499b 100644 --- a/nexus/tests/output/uncovered-authz-endpoints.txt +++ b/nexus/tests/output/uncovered-authz-endpoints.txt @@ -1,8 +1,4 @@ API endpoints with no coverage in authz tests: -silos_delete_silo (delete "/silos/{silo_name}") session_me (get "/session/me") -silos_get (get "/silos") -silos_get_silo (get "/silos/{silo_name}") spoof_login (post "/login") logout (post "/logout") -silos_post (post "/silos")