From a0848d2cb07679543734b7199a0255de9897ebc6 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Fri, 20 May 2022 16:45:50 -0700 Subject: [PATCH 01/11] initial work --- common/src/sql/dbinit.sql | 1 + nexus/Cargo.toml | 2 +- nexus/src/authz/api_resources.rs | 18 +- nexus/tests/integration_tests/authz_roles.rs | 764 +++++++++++++++++++ nexus/tests/integration_tests/mod.rs | 1 + 5 files changed, 779 insertions(+), 7 deletions(-) create mode 100644 nexus/tests/integration_tests/authz_roles.rs diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 3e1d553f999..23c98529fab 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -273,6 +273,7 @@ CREATE TABLE omicron.public.organization ( ); CREATE UNIQUE INDEX ON omicron.public.organization ( + silo_id, name ) WHERE time_deleted IS NULL; diff --git a/nexus/Cargo.toml b/nexus/Cargo.toml index 7393c0eb58f..e97371ad38e 100644 --- a/nexus/Cargo.toml +++ b/nexus/Cargo.toml @@ -47,6 +47,7 @@ serde_with = "1.13.0" sled-agent-client = { path = "../sled-agent-client" } slog-dtrace = "0.2" structopt = "0.3" +strum = { version = "0.23", features = [ "derive" ] } tempfile = "3.3" thiserror = "1.0" toml = "0.5.9" @@ -127,7 +128,6 @@ regex = "1.5.5" subprocess = "0.2.9" term = "0.7" httptest = "0.15.4" -strum = { version = "0.23", features = [ "derive" ] } [dev-dependencies.openapi-lint] git = "https://github.com/oxidecomputer/openapi-lint" diff --git a/nexus/src/authz/api_resources.rs b/nexus/src/authz/api_resources.rs index 915b98b19b5..3ef2b6d0412 100644 --- a/nexus/src/authz/api_resources.rs +++ b/nexus/src/authz/api_resources.rs @@ -50,7 +50,6 @@ use parse_display::Display; use parse_display::FromStr; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -#[cfg(test)] use strum::EnumIter; use uuid::Uuid; @@ -206,9 +205,16 @@ impl ApiResourceWithRolesType for Fleet { } #[derive( - Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema, + Clone, + Copy, + Debug, + Deserialize, + EnumIter, + Eq, + PartialEq, + Serialize, + JsonSchema, )] -#[cfg_attr(test, derive(EnumIter))] #[serde(rename_all = "snake_case")] pub enum FleetRoles { Admin, @@ -379,13 +385,13 @@ impl ApiResourceWithRolesType for Organization { Debug, Deserialize, Display, + EnumIter, Eq, FromStr, PartialEq, Serialize, JsonSchema, )] -#[cfg_attr(test, derive(EnumIter))] #[display(style = "kebab-case")] #[serde(rename_all = "snake_case")] pub enum OrganizationRoles { @@ -433,13 +439,13 @@ impl ApiResourceWithRolesType for Project { Debug, Deserialize, Display, + EnumIter, Eq, FromStr, PartialEq, Serialize, JsonSchema, )] -#[cfg_attr(test, derive(EnumIter))] #[display(style = "kebab-case")] #[serde(rename_all = "snake_case")] pub enum ProjectRoles { @@ -579,13 +585,13 @@ impl ApiResourceWithRolesType for Silo { Debug, Deserialize, Display, + EnumIter, Eq, FromStr, PartialEq, Serialize, JsonSchema, )] -#[cfg_attr(test, derive(EnumIter))] #[display(style = "kebab-case")] #[serde(rename_all = "snake_case")] pub enum SiloRoles { diff --git a/nexus/tests/integration_tests/authz_roles.rs b/nexus/tests/integration_tests/authz_roles.rs new file mode 100644 index 00000000000..e92f5a171ab --- /dev/null +++ b/nexus/tests/integration_tests/authz_roles.rs @@ -0,0 +1,764 @@ +// 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/. + +//! (Fairly) comprehensive test of authz by creating a hierarchy of resources +//! and a group of users with various roles on these resources and verifying +//! that each role grants the privileges we expect. + +use anyhow::anyhow; +use dropshot::test_util::ClientTestContext; +use futures::future::join_all; +use http::Method; +use http::StatusCode; +use lazy_static::lazy_static; +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::resource_helpers; +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_nexus::app::test_interfaces::TestInterfaces; +use omicron_nexus::authz; +use omicron_nexus::external_api::params; +use omicron_nexus::external_api::params::InstanceNetworkInterfaceAttachment; +use omicron_nexus::external_api::shared; +use omicron_nexus::external_api::shared::IdentityType; +use std::collections::BTreeMap; +use std::convert::AsRef; +use strum::AsRefStr; +use uuid::Uuid; + +struct World { + users: BTreeMap<(String, String), Uuid>, + resources: Vec<&'static Resource>, +} + +#[nexus_test] +async fn test_authz_roles(cptestctx: &ControlPlaneTestContext) { + let world = setup_hierarchy(cptestctx).await; + + // For each user that we've created, for each resource, for each possible + // action, attempt the action. + let results = test_operations(cptestctx, &world).await; + + let mut stream = std::io::Cursor::new(Vec::new()); + dump_results(&mut stream, &results) + .expect("failed to write to in-memory buffer"); + let output = stream.into_inner(); + print!("{}", String::from_utf8_lossy(&output)); + + // XXX-dap add expectorate +} + +#[derive(Debug)] +struct Resource { + resource_type: ResourceType, +} + +impl Resource { + fn full_name(&self) -> String { + match self.resource_type { + ResourceType::Fleet => String::from("fleet"), + ResourceType::Silo { name } => format!("{}", name), + ResourceType::Organization { name, parent_silo } => { + format!("{}{}", parent_silo, name) + } + ResourceType::Project { name, parent_org, parent_silo } => { + format!("{}{}{}", parent_silo, parent_org, name) + } + ResourceType::Instance { + name, + parent_project, + parent_org, + parent_silo, + } => format!( + "{}{}{}{}", + parent_silo, parent_org, parent_project, name + ), + } + } + + fn parent_full_name(&self) -> String { + match self.resource_type { + ResourceType::Fleet => unimplemented!(), + ResourceType::Silo { .. } => unimplemented!(), + ResourceType::Organization { parent_silo, .. } => { + format!("{}", parent_silo) + } + ResourceType::Project { parent_org, parent_silo, .. } => { + format!("{}{}", parent_silo, parent_org) + } + ResourceType::Instance { + parent_project, + parent_org, + parent_silo, + .. + } => format!("{}{}{}", parent_silo, parent_org, parent_project), + } + } + + fn create_url(&self) -> String { + match self.resource_type { + ResourceType::Fleet => unimplemented!(), + ResourceType::Silo { .. } => format!("/silos"), + ResourceType::Organization { .. } => { + format!("/organizations") + } + ResourceType::Project { parent_org, .. } => { + format!("/organizations/{}/projects", parent_org) + } + ResourceType::Instance { parent_project, parent_org, .. } => { + format!( + "/organizations/{}/projects/{}/instances", + parent_org, parent_project + ) + } + } + } + + fn policy_url(&self) -> String { + match self.resource_type { + ResourceType::Fleet => format!("/policy"), + ResourceType::Silo { name } => format!("/silos/{}/policy", name), + ResourceType::Organization { name, .. } => { + format!("/organizations/{}/policy", name) + } + ResourceType::Project { name, parent_org, .. } => format!( + "/organizations/{}/projects/{}/policy", + parent_org, name + ), + ResourceType::Instance { .. } => unimplemented!(), + } + } + + fn test_operations<'a>( + &self, + client: &'a ClientTestContext, + ) -> Vec> { + match self.resource_type { + // XXX-dap TODO + ResourceType::Fleet => vec![], + // XXX-dap TODO + ResourceType::Silo { .. } => vec![], + // XXX-dap TODO + ResourceType::Organization { name, .. } => { + let resource_url = format!("{}/{}", self.create_url(), name); + vec![ + TestOperation { + label: "Fetch", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &resource_url, + )), + }, + TestOperation { + label: "ModifyDescription", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::PUT, + &resource_url, + ) + .body(Some( + ¶ms::OrganizationUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some(String::from( + "updated!", + )), + }, + }, + )), + ), + }, + ] + } + // XXX-dap TODO + ResourceType::Project { .. } => vec![], + // XXX-dap TODO + ResourceType::Instance { .. } => vec![], + } + } +} + +#[derive(Debug, AsRefStr)] +enum ResourceType { + Fleet, + Silo { + name: &'static str, + }, + Organization { + name: &'static str, + parent_silo: &'static str, + }, + Project { + name: &'static str, + parent_org: &'static str, + parent_silo: &'static str, + }, + Instance { + name: &'static str, + parent_project: &'static str, + parent_org: &'static str, + parent_silo: &'static str, + }, +} + +lazy_static! { + // Hierarchy: + // fleet + // fleet/s1 + // fleet/s1/o1 + // fleet/s1/o1/p1 + // fleet/s1/o1/p1/i1 + // fleet/s1/o1/p2 + // fleet/s1/o1/p2/i1 + // fleet/s1/o2 + // fleet/s1/o2/p1 + // fleet/s1/o2/p1/i1 + // fleet/s2 + // fleet/s2/o1 + // fleet/s2/o1/p1 + // fleet/s2/o1/p1/i1 + + static ref FLEET: Resource = Resource { resource_type: ResourceType::Fleet }; + + static ref SILO1: Resource = + Resource { resource_type: ResourceType::Silo { name: "s1" } }; + static ref SILO1_ORG1: Resource = Resource { + resource_type: ResourceType::Organization { + name: "o1", + parent_silo: "s1", + }, + }; + static ref SILO1_ORG1_PROJ1: Resource = Resource { + resource_type: ResourceType::Project { + name: "p1", + parent_org: "o1", + parent_silo: "s1", + }, + }; + static ref SILO1_ORG1_PROJ1_INST: Resource = Resource { + resource_type: ResourceType::Instance { + name: "i1", + parent_project: "p1", + parent_org: "o1", + parent_silo: "s1", + }, + }; + static ref SILO1_ORG1_PROJ2: Resource = Resource { + resource_type: ResourceType::Project { + name: "p2", + parent_org: "o1", + parent_silo: "s1", + }, + }; + static ref SILO1_ORG1_PROJ2_INST: Resource = Resource { + resource_type: ResourceType::Instance { + name: "i1", + parent_project: "p2", + parent_org: "o1", + parent_silo: "s1", + }, + }; + static ref SILO1_ORG2: Resource = Resource { + resource_type: ResourceType::Organization { + name: "o2", + parent_silo: "s1", + }, + }; + static ref SILO1_ORG2_PROJ1: Resource = Resource { + resource_type: ResourceType::Project { + name: "p1", + parent_org: "o2", + parent_silo: "s1", + }, + }; + static ref SILO1_ORG2_PROJ1_INST: Resource = Resource { + resource_type: ResourceType::Instance { + name: "i1", + parent_project: "p1", + parent_org: "o2", + parent_silo: "s1", + }, + }; + + static ref SILO2: Resource = + Resource { resource_type: ResourceType::Silo { name: "s2" } }; + static ref SILO2_ORG1: Resource = Resource { + resource_type: ResourceType::Organization { + name: "o1", + parent_silo: "s2", + }, + }; + static ref SILO2_ORG1_PROJ1: Resource = Resource { + resource_type: ResourceType::Project { + name: "p1", + parent_org: "o1", + parent_silo: "s2", + }, + }; + static ref SILO2_ORG1_PROJ1_INST: Resource = Resource { + resource_type: ResourceType::Instance { + name: "i1", + parent_project: "p1", + parent_org: "o1", + parent_silo: "s2", + }, + }; + + // XXX-dap TODO-doc These are the resources for which: for each role, we + // will create a user having that role on this resource. We will later test + // that this user _can_ access the corresponding thing in the corresponding + // way, and that they _cannot_ access any _other_ resources in any other + // way. + // Silos are skipped because we always create their users. + static ref RESOURCES_WITH_USERS: Vec<&'static Resource> = vec![ + &*FLEET, + &*SILO1_ORG1, + &*SILO1_ORG1_PROJ1, + ]; + + static ref ALL_RESOURCES: Vec<&'static Resource> = vec![ + &*FLEET, + &*SILO1, + &*SILO1_ORG1, + &*SILO1_ORG1_PROJ1, + &*SILO1_ORG1_PROJ1_INST, + &*SILO1_ORG1_PROJ2, + &*SILO1_ORG1_PROJ2_INST, + &*SILO1_ORG2, + &*SILO1_ORG2_PROJ1, + &*SILO1_ORG2_PROJ1_INST, + &*SILO2, + &*SILO2_ORG1, + &*SILO2_ORG1_PROJ1, + &*SILO2_ORG1_PROJ1_INST, + ]; +} + +async fn setup_hierarchy(testctx: &ControlPlaneTestContext) -> World { + let client = &testctx.external_client; + let nexus = &*testctx.server.apictx.nexus; + let log = &testctx.logctx.log.new(o!("component" => "SetupHierarchy")); + + // Mapping: (our resource name, role name) -> user id + // where "user id" is the id of a user having role "role name" on resource + // "our resource name". + // + // "our resource name" is a string like s1o1 (SILO1_ORG1). + let mut users: BTreeMap<(String, String), Uuid> = BTreeMap::new(); + + // Mapping: silo name -> silo id + let mut silos: BTreeMap = BTreeMap::new(); + + // We can't use the helpers in `resource_helpers` to create most of these + // resources because they always use the "test-privileged" user in the + // default Silo. But in order to create things in other Silos, we need to + // use different users. + // XXX-dap don't bother creating users except for RESOURCES_WITH_USERS + for resource in &*ALL_RESOURCES { + let resource = *resource; + debug!(log, "creating resource"; "resource" => ?resource); + match resource.resource_type { + ResourceType::Fleet => (), + ResourceType::Silo { name } => { + let silo = + resource_helpers::create_silo(client, name, false).await; + silos.insert(name.to_string(), silo.identity.id); + // We have to create the Silo users here so that we can use the + // admin to create the other resources. + create_users::( + log, + nexus, + client, + name, + silo.identity.id, + &resource.policy_url(), + &mut users, + None, + ) + .await; + } + ResourceType::Organization { name, parent_silo, .. } => { + let caller_id = user_id(&users, &parent_silo, "admin"); + let full_name = resource.full_name(); + NexusRequest::objects_post( + client, + &resource.create_url(), + ¶ms::OrganizationCreate { + identity: IdentityMetadataCreateParams { + name: name + .parse() + .expect("generated name was invalid"), + description: full_name.clone(), + }, + }, + ) + .authn_as(AuthnMode::SiloUser(caller_id)) + .execute() + .await + .unwrap_or_else(|_| panic!("failed to create {}", full_name)); + } + ResourceType::Project { name, parent_silo, .. } => { + let caller_id = user_id(&users, &parent_silo, "admin"); + let full_name = resource.full_name(); + NexusRequest::objects_post( + client, + &resource.create_url(), + ¶ms::ProjectCreate { + identity: IdentityMetadataCreateParams { + name: name + .parse() + .expect("generated name was invalid"), + description: full_name.clone(), + }, + }, + ) + .authn_as(AuthnMode::SiloUser(caller_id)) + .execute() + .await + .unwrap_or_else(|_| panic!("failed to create {}", full_name)); + } + ResourceType::Instance { name, parent_silo, .. } => { + let caller_id = user_id(&users, &parent_silo, "admin"); + let full_name = resource.full_name(); + NexusRequest::objects_post( + client, + &resource.create_url(), + ¶ms::InstanceCreate { + identity: IdentityMetadataCreateParams { + name: name + .parse() + .expect("generated name was invalid"), + description: full_name.clone(), + }, + ncpus: InstanceCpuCount(1), + memory: ByteCount::from_gibibytes_u32(1), + hostname: full_name.clone(), + user_data: vec![], + network_interfaces: + InstanceNetworkInterfaceAttachment::Default, + disks: vec![], + }, + ) + .authn_as(AuthnMode::SiloUser(caller_id)) + .execute() + .await + .unwrap_or_else(|_| panic!("failed to create {}", full_name)); + } + } + } + + for resource in &*RESOURCES_WITH_USERS { + match resource.resource_type { + // XXX-dap we should be testing fleet users too + // We don't create users for Instances. We already created users + // for Silos. + ResourceType::Instance { .. } | ResourceType::Silo { .. } => { + unimplemented!() + } + ResourceType::Fleet => { + create_users::( + log, + nexus, + client, + &resource.full_name(), + *omicron_nexus::db::fixed_data::silo::SILO_ID, + &resource.policy_url(), + &mut users, + None, + ) + .await; + } + ResourceType::Organization { parent_silo, .. } => { + let silo_id = silos.get(parent_silo).expect("missing silo"); + let caller_id = + user_id(&users, &resource.parent_full_name(), "admin"); + create_users::( + log, + nexus, + client, + &resource.full_name(), + *silo_id, + &resource.policy_url(), + &mut users, + Some(caller_id), + ) + .await; + } + ResourceType::Project { parent_silo, .. } => { + let silo_id = silos.get(parent_silo).expect("missing silo"); + let caller_id = + user_id(&users, &resource.parent_full_name(), "admin"); + create_users::( + log, + nexus, + client, + &resource.full_name(), + *silo_id, + &resource.policy_url(), + &mut users, + Some(caller_id), + ) + .await; + } + } + } + + World { users, resources: ALL_RESOURCES.clone() } +} + +fn user_id( + users: &BTreeMap<(String, String), Uuid>, + resource_full_name: &str, + role_name: &str, +) -> Uuid { + *users + .get(&(resource_full_name.to_owned(), role_name.to_owned())) + .unwrap_or_else(|| { + panic!( + "expected user to be created with role {:?} on {:?}", + role_name, resource_full_name, + ) + }) +} + +/// For a given resource, for each supported role, create a new user with that +/// role on this resource. +/// +/// - `nexus`: needed to use the private interface for creating silo users +/// - `resource_name`: *our* identifier for the resource (e.g., s1o1). This is +/// used as the basename of the user's name +/// - `policy_url`: the URL for the resource's IAM policy +/// - `users`: a map into which we'll insert the uuids of created users +/// - `run_as`: executes requests to update resource policy using the given silo +/// user id. This lets us, say, create an Organization with a user having +/// "admin" on the parent Silo, and create a Project with a user having +/// "admin" on the parent Organization, etc. +async fn create_users< + T: strum::IntoEnumIterator + + serde::Serialize + + serde::de::DeserializeOwned + + omicron_nexus::db::model::DatabaseString, +>( + log: &slog::Logger, + nexus: &dyn TestInterfaces, + client: &ClientTestContext, + resource_name: &str, + silo_id: Uuid, + policy_url: &str, + users: &mut BTreeMap<(String, String), Uuid>, + run_as: Option, +) { + for variant in T::iter() { + let role_name = variant.to_database_string(); + // TODO when silo users get user names, this should go into it. + let username = format!("{}-{}", resource_name, role_name); + let user_id = Uuid::new_v4(); + debug!( + log, + "creating user"; + "username" => &username, + "user_id" => user_id.to_string() + ); + nexus + .silo_user_create(silo_id, user_id) + .await + .unwrap_or_else(|_| panic!("failed to create user {:?}", username)); + users.insert((resource_name.to_owned(), role_name.to_owned()), user_id); + + debug!( + log, + "adding role"; + "username" => username, + "user_id" => user_id.to_string(), + "role" => role_name, + ); + let authn_mode = run_as + .map(|caller_id| AuthnMode::SiloUser(caller_id)) + .unwrap_or(AuthnMode::PrivilegedUser); + + let existing_policy: shared::Policy = + NexusRequest::object_get(client, policy_url) + .authn_as(authn_mode.clone()) + .execute() + .await + .expect("failed to fetch policy") + .parsed_body() + .expect("failed to parse policy"); + + let new_role_assignment = shared::RoleAssignment { + identity_type: IdentityType::SiloUser, + identity_id: user_id, + role_name: variant, + }; + let new_role_assignments = existing_policy + .role_assignments + .into_iter() + .chain(std::iter::once(new_role_assignment)) + .collect(); + + let new_policy = + shared::Policy { role_assignments: new_role_assignments }; + + NexusRequest::object_put(client, policy_url, Some(&new_policy)) + .authn_as(authn_mode) + .execute() + .await + .expect("failed to update policy"); + } +} + +struct TestOperation<'a> { + label: &'static str, + template: NexusRequest<'a>, +} + +enum OperationResult { + Success, + Denied, + UnexpectedError(anyhow::Error), +} + +struct ResourceResults { + resource: &'static Resource, + test_labels: Vec<&'static str>, + results: Vec, +} + +struct ResourceUserResults { + username: String, + results: Vec, +} + +async fn test_operations( + cptestctx: &ControlPlaneTestContext, + world: &World, +) -> Vec { + let log = &cptestctx.logctx.log; + let client = &cptestctx.external_client; + let mut results = Vec::with_capacity(world.resources.len()); + for resource in &world.resources { + let mut user_results = Vec::with_capacity(world.users.len()); + let mut test_labels: Option> = None; + for ((user_resource_name, user_role_name), user_id) in &world.users { + let username = format!("{}-{}", user_resource_name, user_role_name); + let log = log.new(o!( + "resource" => resource.full_name(), + "user" => username.clone(), + )); + + let operations = resource.test_operations(client); + if test_labels.is_none() { + test_labels = + Some(operations.iter().map(|t| t.label).collect()); + } + user_results.push(ResourceUserResults { + username, + results: join_all(operations.into_iter().map( + |test_operation| { + run_test_operation(&log, test_operation, *user_id) + }, + )) + .await, + }); + } + + let result = if let Some(test_labels) = test_labels { + assert!(!user_results.is_empty()); + ResourceResults { resource, test_labels, results: user_results } + } else { + assert!(user_results.is_empty()); + ResourceResults { + resource, + test_labels: Vec::new(), + results: Vec::new(), + } + }; + results.push(result); + } + + results +} + +async fn run_test_operation<'a>( + log: &slog::Logger, + to: TestOperation<'a>, + user_id: Uuid, +) -> OperationResult { + info!(log, "test operation"; "operation" => to.label); + let request = to.template; + let response = request + .authn_as(AuthnMode::SiloUser(user_id)) + .execute() + .await + .expect("failed to execute request"); + if matches!(response.status, StatusCode::NOT_FOUND | StatusCode::FORBIDDEN) + { + OperationResult::Denied + } else if response.status.is_success() { + OperationResult::Success + } else { + let status = response.status; + let error_response: dropshot::HttpErrorResponseBody = + response.parsed_body().expect("failed to parse error response"); + OperationResult::UnexpectedError(anyhow!( + "unexpected response: status code {}, message {:?}", + status, + error_response.message + )) + } +} + +fn dump_results( + mut out: W, + results: &[ResourceResults], +) -> std::io::Result<()> { + for resource_result in results { + write!( + out, + "resource: {} {:?}", + resource_result.resource.resource_type.as_ref(), + resource_result.resource.full_name() + )?; + + if resource_result.test_labels.is_empty() { + write!(out, ": no tests defined\n\n")?; + continue; + } + + write!( + out, + "\n actions: {}\n\n", + resource_result.test_labels.join(", ") + )?; + + write!(out, "{:20} {}\n", "USER", "RESULTS FOR EACH ACTION")?; + for user_result in &resource_result.results { + write!(out, "{:20}", user_result.username)?; + for op_result in &user_result.results { + write!( + out, + " {}", + match op_result { + OperationResult::Success => '\u{2713}', + OperationResult::Denied => '\u{2717}', + OperationResult::UnexpectedError(_) => '\u{26a0}', + } + )?; + } + write!(out, "\n")?; + } + + write!(out, "\n")?; + } + + Ok(()) +} diff --git a/nexus/tests/integration_tests/mod.rs b/nexus/tests/integration_tests/mod.rs index de5de9679bd..fb1e53447e1 100644 --- a/nexus/tests/integration_tests/mod.rs +++ b/nexus/tests/integration_tests/mod.rs @@ -4,6 +4,7 @@ //! the way it is. mod authn_http; +mod authz_roles; mod basic; mod commands; mod console_api; From 600211eb21a7c7b348382f6011c799b3a5adc50a Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Fri, 20 May 2022 16:57:06 -0700 Subject: [PATCH 02/11] fixes --- nexus/tests/integration_tests/authz_roles.rs | 46 ++++++++++++++------ 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/nexus/tests/integration_tests/authz_roles.rs b/nexus/tests/integration_tests/authz_roles.rs index e92f5a171ab..e3e534da8c2 100644 --- a/nexus/tests/integration_tests/authz_roles.rs +++ b/nexus/tests/integration_tests/authz_roles.rs @@ -140,6 +140,20 @@ impl Resource { &self, client: &'a ClientTestContext, ) -> Vec> { + // XXX-dap TODO examples: + // - list children (various kinds) + // - modify + // - fetch + // - delete? this one seems hard to test because it might succeed! + // - start/stop/halt -- hard to test because it's async + // - migrate? + // - disk attach/detach? + // This is getting a little nuts. In the limit, we kind of want to run + // every test as a user with every role we think should grant that test + // the privileges it needs (to make sure they grant enough). Then we + // kind of want to run every test again as every other role to make sure + // it fails at some point. Then we want to check coverage of the + // endpoints/role combinations. This seems really hard! match self.resource_type { // XXX-dap TODO ResourceType::Fleet => vec![], @@ -223,9 +237,14 @@ lazy_static! { // fleet/s1/o2/p1 // fleet/s1/o2/p1/i1 // fleet/s2 - // fleet/s2/o1 - // fleet/s2/o1/p1 - // fleet/s2/o1/p1/i1 + // fleet/s2/o3 + // fleet/s2/o3/p1 + // fleet/s2/o3/p1/i1 + // + // Silo 2's organization is called "o3" because otherwise it appears as + // though users from Silo 1 are successfully accessing it. + // TODO-security TODO-coverage when we add support for looking up resources + // by id, we should update this test to operate on things by-id as well. static ref FLEET: Resource = Resource { resource_type: ResourceType::Fleet }; @@ -291,24 +310,24 @@ lazy_static! { static ref SILO2: Resource = Resource { resource_type: ResourceType::Silo { name: "s2" } }; - static ref SILO2_ORG1: Resource = Resource { + static ref SILO2_ORG3: Resource = Resource { resource_type: ResourceType::Organization { - name: "o1", + name: "o3", parent_silo: "s2", }, }; - static ref SILO2_ORG1_PROJ1: Resource = Resource { + static ref SILO2_ORG3_PROJ1: Resource = Resource { resource_type: ResourceType::Project { name: "p1", - parent_org: "o1", + parent_org: "o3", parent_silo: "s2", }, }; - static ref SILO2_ORG1_PROJ1_INST: Resource = Resource { + static ref SILO2_ORG3_PROJ1_INST: Resource = Resource { resource_type: ResourceType::Instance { name: "i1", parent_project: "p1", - parent_org: "o1", + parent_org: "o3", parent_silo: "s2", }, }; @@ -337,9 +356,9 @@ lazy_static! { &*SILO1_ORG2_PROJ1, &*SILO1_ORG2_PROJ1_INST, &*SILO2, - &*SILO2_ORG1, - &*SILO2_ORG1_PROJ1, - &*SILO2_ORG1_PROJ1_INST, + &*SILO2_ORG3, + &*SILO2_ORG3_PROJ1, + &*SILO2_ORG3_PROJ1_INST, ]; } @@ -362,7 +381,6 @@ async fn setup_hierarchy(testctx: &ControlPlaneTestContext) -> World { // resources because they always use the "test-privileged" user in the // default Silo. But in order to create things in other Silos, we need to // use different users. - // XXX-dap don't bother creating users except for RESOURCES_WITH_USERS for resource in &*ALL_RESOURCES { let resource = *resource; debug!(log, "creating resource"; "resource" => ?resource); @@ -465,7 +483,7 @@ async fn setup_hierarchy(testctx: &ControlPlaneTestContext) -> World { unimplemented!() } ResourceType::Fleet => { - create_users::( + create_users::( log, nexus, client, From 10827f5100c21eb9a307cc9cf9389f7af5eed3d7 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 23 May 2022 09:56:21 -0700 Subject: [PATCH 03/11] record bug --- nexus/tests/integration_tests/authz_roles.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nexus/tests/integration_tests/authz_roles.rs b/nexus/tests/integration_tests/authz_roles.rs index e3e534da8c2..6e0545df52c 100644 --- a/nexus/tests/integration_tests/authz_roles.rs +++ b/nexus/tests/integration_tests/authz_roles.rs @@ -5,6 +5,8 @@ //! (Fairly) comprehensive test of authz by creating a hierarchy of resources //! and a group of users with various roles on these resources and verifying //! that each role grants the privileges we expect. +//! XXX-dap it looks like a bug that fleet admins can't see Organizations. Is +//! silo.fleet not set up right in Polar? use anyhow::anyhow; use dropshot::test_util::ClientTestContext; From 7e169d2b90404eed86476c32d2b4aa111614c6f2 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 23 May 2022 10:25:45 -0700 Subject: [PATCH 04/11] add some more actions --- nexus/tests/integration_tests/authz_roles.rs | 63 +++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/nexus/tests/integration_tests/authz_roles.rs b/nexus/tests/integration_tests/authz_roles.rs index 6e0545df52c..58c3b7224e3 100644 --- a/nexus/tests/integration_tests/authz_roles.rs +++ b/nexus/tests/integration_tests/authz_roles.rs @@ -141,6 +141,7 @@ impl Resource { fn test_operations<'a>( &self, client: &'a ClientTestContext, + username: &str, ) -> Vec> { // XXX-dap TODO examples: // - list children (various kinds) @@ -164,6 +165,15 @@ impl Resource { // XXX-dap TODO ResourceType::Organization { name, .. } => { let resource_url = format!("{}/{}", self.create_url(), name); + let projects_url = format!("{}/projects", &resource_url); + let new_project_name = username.parse().expect( + "invalid test project name (tried to use a username \ + that we generated)", + ); + let new_project_description = + format!("created by {}", username); + let project_url = + format!("{}/{}", &projects_url, new_project_name); vec![ TestOperation { label: "Fetch", @@ -172,6 +182,38 @@ impl Resource { Method::GET, &resource_url, )), + on_success: None, + }, + TestOperation { + label: "ListProjects", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &projects_url, + )), + on_success: None, + }, + TestOperation { + label: "CreateProject", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::POST, + &projects_url, + ) + .body(Some( + ¶ms::ProjectCreate { + identity: IdentityMetadataCreateParams { + name: new_project_name, + description: new_project_description, + }, + }, + )), + ), + on_success: Some(NexusRequest::object_delete( + client, + &project_url, + )), }, TestOperation { label: "ModifyDescription", @@ -192,6 +234,7 @@ impl Resource { }, )), ), + on_success: None, }, ] } @@ -346,6 +389,14 @@ lazy_static! { &*SILO1_ORG1_PROJ1, ]; + // XXX-dap One idea to support testing DELETE: + // - every resource has a "recreate()" function that recreates it (including + // the role assignments) + // - this list is guaranteed to be topo-sorted + // - we explicitly delete every resource when we're done testing it + // + // Then we can test these in reverse order. By the time we get to any + // resource, its children should be gone. static ref ALL_RESOURCES: Vec<&'static Resource> = vec![ &*FLEET, &*SILO1, @@ -639,6 +690,7 @@ async fn create_users< struct TestOperation<'a> { label: &'static str, template: NexusRequest<'a>, + on_success: Option>, } enum OperationResult { @@ -675,7 +727,7 @@ async fn test_operations( "user" => username.clone(), )); - let operations = resource.test_operations(client); + let operations = resource.test_operations(client, &username); if test_labels.is_none() { test_labels = Some(operations.iter().map(|t| t.label).collect()); @@ -724,6 +776,15 @@ async fn run_test_operation<'a>( { OperationResult::Denied } else if response.status.is_success() { + if let Some(request) = to.on_success { + debug!(log, "on_success operation"); + request + .authn_as(AuthnMode::SiloUser(user_id)) + .execute() + .await + .expect("failed to execute on-success request"); + } + OperationResult::Success } else { let status = response.status; From 974c126b713cfe52e586d8c98cf6d658636c3bdc Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 23 May 2022 10:39:39 -0700 Subject: [PATCH 05/11] test VPCs instead of Instances --- nexus/tests/integration_tests/authz_roles.rs | 134 ++++++++++++++----- 1 file changed, 102 insertions(+), 32 deletions(-) diff --git a/nexus/tests/integration_tests/authz_roles.rs b/nexus/tests/integration_tests/authz_roles.rs index 58c3b7224e3..35567ee5060 100644 --- a/nexus/tests/integration_tests/authz_roles.rs +++ b/nexus/tests/integration_tests/authz_roles.rs @@ -20,14 +20,12 @@ use nexus_test_utils::http_testing::RequestBuilder; use nexus_test_utils::resource_helpers; 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::app::test_interfaces::TestInterfaces; use omicron_nexus::authz; use omicron_nexus::external_api::params; -use omicron_nexus::external_api::params::InstanceNetworkInterfaceAttachment; use omicron_nexus::external_api::shared; use omicron_nexus::external_api::shared::IdentityType; use std::collections::BTreeMap; @@ -73,7 +71,7 @@ impl Resource { ResourceType::Project { name, parent_org, parent_silo } => { format!("{}{}{}", parent_silo, parent_org, name) } - ResourceType::Instance { + ResourceType::Vpc { name, parent_project, parent_org, @@ -95,7 +93,7 @@ impl Resource { ResourceType::Project { parent_org, parent_silo, .. } => { format!("{}{}", parent_silo, parent_org) } - ResourceType::Instance { + ResourceType::Vpc { parent_project, parent_org, parent_silo, @@ -114,9 +112,9 @@ impl Resource { ResourceType::Project { parent_org, .. } => { format!("/organizations/{}/projects", parent_org) } - ResourceType::Instance { parent_project, parent_org, .. } => { + ResourceType::Vpc { parent_project, parent_org, .. } => { format!( - "/organizations/{}/projects/{}/instances", + "/organizations/{}/projects/{}/vpcs", parent_org, parent_project ) } @@ -134,7 +132,7 @@ impl Resource { "/organizations/{}/projects/{}/policy", parent_org, name ), - ResourceType::Instance { .. } => unimplemented!(), + ResourceType::Vpc { .. } => unimplemented!(), } } @@ -162,7 +160,7 @@ impl Resource { ResourceType::Fleet => vec![], // XXX-dap TODO ResourceType::Silo { .. } => vec![], - // XXX-dap TODO + ResourceType::Organization { name, .. } => { let resource_url = format!("{}/{}", self.create_url(), name); let projects_url = format!("{}/projects", &resource_url); @@ -238,10 +236,85 @@ impl Resource { }, ] } + // XXX-dap TODO - ResourceType::Project { .. } => vec![], + ResourceType::Project { name, .. } => { + let resource_url = format!("{}/{}", self.create_url(), name); + let vpcs_url = format!("{}/vpcs", &resource_url); + let new_vpc_name: Name = username.parse().expect( + "invalid test VPC name (tried to use a username \ + that we generated)", + ); + let new_vpc_description = format!("created by {}", username); + let vpc_url = format!("{}/{}", &vpcs_url, new_vpc_name); + vec![ + TestOperation { + label: "Fetch", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &resource_url, + )), + on_success: None, + }, + TestOperation { + label: "ListVpcs", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &vpcs_url, + )), + on_success: None, + }, + TestOperation { + label: "CreateVpc", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::POST, + &vpcs_url, + ) + .body(Some( + ¶ms::VpcCreate { + identity: IdentityMetadataCreateParams { + name: new_vpc_name.clone(), + description: new_vpc_description, + }, + ipv6_prefix: None, + dns_name: new_vpc_name, + }, + )), + ), + on_success: Some(NexusRequest::object_delete( + client, &vpc_url, + )), + }, + TestOperation { + label: "ModifyDescription", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::PUT, + &resource_url, + ) + .body(Some( + ¶ms::ProjectUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some(String::from( + "updated!", + )), + }, + }, + )), + ), + on_success: None, + }, + ] + } + // XXX-dap TODO - ResourceType::Instance { .. } => vec![], + ResourceType::Vpc { .. } => vec![], } } } @@ -261,7 +334,7 @@ enum ResourceType { parent_org: &'static str, parent_silo: &'static str, }, - Instance { + Vpc { name: &'static str, parent_project: &'static str, parent_org: &'static str, @@ -309,8 +382,8 @@ lazy_static! { }, }; static ref SILO1_ORG1_PROJ1_INST: Resource = Resource { - resource_type: ResourceType::Instance { - name: "i1", + resource_type: ResourceType::Vpc { + name: "v1", parent_project: "p1", parent_org: "o1", parent_silo: "s1", @@ -324,8 +397,8 @@ lazy_static! { }, }; static ref SILO1_ORG1_PROJ2_INST: Resource = Resource { - resource_type: ResourceType::Instance { - name: "i1", + resource_type: ResourceType::Vpc { + name: "v1", parent_project: "p2", parent_org: "o1", parent_silo: "s1", @@ -345,8 +418,8 @@ lazy_static! { }, }; static ref SILO1_ORG2_PROJ1_INST: Resource = Resource { - resource_type: ResourceType::Instance { - name: "i1", + resource_type: ResourceType::Vpc { + name: "v1", parent_project: "p1", parent_org: "o2", parent_silo: "s1", @@ -369,8 +442,8 @@ lazy_static! { }, }; static ref SILO2_ORG3_PROJ1_INST: Resource = Resource { - resource_type: ResourceType::Instance { - name: "i1", + resource_type: ResourceType::Vpc { + name: "v1", parent_project: "p1", parent_org: "o3", parent_silo: "s2", @@ -497,26 +570,23 @@ async fn setup_hierarchy(testctx: &ControlPlaneTestContext) -> World { .await .unwrap_or_else(|_| panic!("failed to create {}", full_name)); } - ResourceType::Instance { name, parent_silo, .. } => { + ResourceType::Vpc { name, parent_silo, .. } => { let caller_id = user_id(&users, &parent_silo, "admin"); let full_name = resource.full_name(); NexusRequest::objects_post( client, &resource.create_url(), - ¶ms::InstanceCreate { + ¶ms::VpcCreate { identity: IdentityMetadataCreateParams { name: name .parse() .expect("generated name was invalid"), description: full_name.clone(), }, - ncpus: InstanceCpuCount(1), - memory: ByteCount::from_gibibytes_u32(1), - hostname: full_name.clone(), - user_data: vec![], - network_interfaces: - InstanceNetworkInterfaceAttachment::Default, - disks: vec![], + ipv6_prefix: None, + dns_name: full_name.parse().expect( + "expected resource name to be a valid DNS name", + ), }, ) .authn_as(AuthnMode::SiloUser(caller_id)) @@ -530,9 +600,9 @@ async fn setup_hierarchy(testctx: &ControlPlaneTestContext) -> World { for resource in &*RESOURCES_WITH_USERS { match resource.resource_type { // XXX-dap we should be testing fleet users too - // We don't create users for Instances. We already created users - // for Silos. - ResourceType::Instance { .. } | ResourceType::Silo { .. } => { + // We don't create users for Vpcs. We already created users for + // Silos. + ResourceType::Vpc { .. } | ResourceType::Silo { .. } => { unimplemented!() } ResourceType::Fleet => { From 3a7544c271fad7e0cb9d923860ba86dcd5a22267 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 23 May 2022 11:32:08 -0700 Subject: [PATCH 06/11] add tests for Vpc --- nexus/tests/integration_tests/authz_roles.rs | 83 +++++++++++++++++++- 1 file changed, 81 insertions(+), 2 deletions(-) diff --git a/nexus/tests/integration_tests/authz_roles.rs b/nexus/tests/integration_tests/authz_roles.rs index 35567ee5060..f611dd1077e 100644 --- a/nexus/tests/integration_tests/authz_roles.rs +++ b/nexus/tests/integration_tests/authz_roles.rs @@ -22,6 +22,7 @@ use nexus_test_utils::ControlPlaneTestContext; use nexus_test_utils_macros::nexus_test; use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::IdentityMetadataUpdateParams; +use omicron_common::api::external::Ipv4Net; use omicron_common::api::external::Name; use omicron_nexus::app::test_interfaces::TestInterfaces; use omicron_nexus::authz; @@ -53,6 +54,7 @@ async fn test_authz_roles(cptestctx: &ControlPlaneTestContext) { print!("{}", String::from_utf8_lossy(&output)); // XXX-dap add expectorate + panic!("blowing up to preserve results"); } #[derive(Debug)] @@ -313,8 +315,85 @@ impl Resource { ] } - // XXX-dap TODO - ResourceType::Vpc { .. } => vec![], + ResourceType::Vpc { name, .. } => { + let resource_url = format!("{}/{}", self.create_url(), name); + let subnets_url = format!("{}/subnets", &resource_url); + let new_subnet_name: Name = username.parse().expect( + "invalid test VPC name (tried to use a username \ + that we generated)", + ); + let new_subnet_description = format!("created by {}", username); + let subnet_url = + format!("{}/{}", &subnets_url, new_subnet_name); + vec![ + TestOperation { + label: "Fetch", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &resource_url, + )), + on_success: None, + }, + TestOperation { + label: "ListSubnets", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &subnets_url, + )), + on_success: None, + }, + TestOperation { + label: "CreateSubnet", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::POST, + &subnets_url, + ) + .body(Some( + ¶ms::VpcSubnetCreate { + identity: IdentityMetadataCreateParams { + name: new_subnet_name.clone(), + description: new_subnet_description, + }, + ipv4_block: Ipv4Net( + "192.168.1.0/24".parse().unwrap(), + ), + ipv6_block: None, + }, + )), + ), + on_success: Some(NexusRequest::object_delete( + client, + &subnet_url, + )), + }, + TestOperation { + label: "ModifyDescription", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::PUT, + &resource_url, + ) + .body(Some( + ¶ms::VpcUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some(String::from( + "updated!", + )), + }, + dns_name: None, + }, + )), + ), + on_success: None, + }, + ] + } } } } From 484d968131a8a19089b250f9a7c95ef381bf217c Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 23 May 2022 14:28:12 -0700 Subject: [PATCH 07/11] a bunch of work --- nexus/src/authz/omicron.polar | 15 +- nexus/src/db/datastore.rs | 2 +- nexus/tests/integration_tests/authz_roles.rs | 747 ++++++++++++------- nexus/tests/output/authz-roles-test.txt | 280 +++++++ 4 files changed, 777 insertions(+), 267 deletions(-) create mode 100644 nexus/tests/output/authz-roles-test.txt diff --git a/nexus/src/authz/omicron.polar b/nexus/src/authz/omicron.polar index 9de39b4522d..80d763a9197 100644 --- a/nexus/src/authz/omicron.polar +++ b/nexus/src/authz/omicron.polar @@ -137,16 +137,15 @@ resource Silo { ]; roles = [ "admin", "collaborator", "viewer" ]; - "list_children" if "viewer"; "read" if "viewer"; + "list_children" if "collaborator"; + "create_child" if "collaborator"; + "modify" if "admin"; "viewer" if "collaborator"; - "create_child" if "collaborator"; "collaborator" if "admin"; - "modify" if "admin"; relations = { parent_fleet: Fleet }; - "admin" if "admin" on "parent_fleet"; - "collaborator" if "collaborator" on "parent_fleet"; + "admin" if "collaborator" on "parent_fleet"; "viewer" if "viewer" on "parent_fleet"; } has_relation(fleet: Fleet, "parent_fleet", silo: Silo) @@ -180,7 +179,9 @@ resource Organization { "modify" if "admin"; relations = { parent_silo: Silo }; - "admin" if "admin" on "parent_silo"; + "admin" if "collaborator" on "parent_silo"; + "read" if "viewer" on "parent_silo"; + "list_children" if "viewer" on "parent_silo"; } has_relation(silo: Silo, "parent_silo", organization: Organization) if organization.silo = silo; @@ -212,7 +213,7 @@ resource Project { "modify" if "admin"; relations = { parent_organization: Organization }; - "admin" if "admin" on "parent_organization"; + "admin" if "collaborator" on "parent_organization"; } has_relation(organization: Organization, "parent_organization", project: Project) if project.organization = organization; diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 02f53ad2f00..c80d3c81e89 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -676,7 +676,7 @@ impl DataStore { pagparams: &DataPageParams<'_, Name>, ) -> ListResultVec { let authz_silo = opctx.authn.silo_required()?; - opctx.authorize(authz::Action::ListChildren, &authz::FLEET).await?; + opctx.authorize(authz::Action::ListChildren, &authz_silo).await?; use db::schema::organization::dsl; paginated(dsl::organization, dsl::name, pagparams) diff --git a/nexus/tests/integration_tests/authz_roles.rs b/nexus/tests/integration_tests/authz_roles.rs index f611dd1077e..13be5d40d94 100644 --- a/nexus/tests/integration_tests/authz_roles.rs +++ b/nexus/tests/integration_tests/authz_roles.rs @@ -5,8 +5,12 @@ //! (Fairly) comprehensive test of authz by creating a hierarchy of resources //! and a group of users with various roles on these resources and verifying //! that each role grants the privileges we expect. -//! XXX-dap it looks like a bug that fleet admins can't see Organizations. Is -//! silo.fleet not set up right in Polar? +//! XXX-dap bugs found / confusing behavior: +//! - it shouldn't be the case that you can create a Silo that you cannot +//! delete. either we should create an initial role assignment for you or we +//! should have a role that you get "admin" on Silo if you have "collaborator" +//! on parent_fleet? +//! - why is it that everyone who can create an Organization can also delete it? use anyhow::anyhow; use dropshot::test_util::ClientTestContext; @@ -31,6 +35,7 @@ use omicron_nexus::external_api::shared; use omicron_nexus::external_api::shared::IdentityType; use std::collections::BTreeMap; use std::convert::AsRef; +use std::io::Write; use strum::AsRefStr; use uuid::Uuid; @@ -45,16 +50,20 @@ async fn test_authz_roles(cptestctx: &ControlPlaneTestContext) { // For each user that we've created, for each resource, for each possible // action, attempt the action. - let results = test_operations(cptestctx, &world).await; - - let mut stream = std::io::Cursor::new(Vec::new()); - dump_results(&mut stream, &results) - .expect("failed to write to in-memory buffer"); - let output = stream.into_inner(); - print!("{}", String::from_utf8_lossy(&output)); + let mut cursor = std::io::Cursor::new(Vec::new()); + { + let mut stdout = std::io::stdout(); + let mut stream = DumbTee::new(vec![&mut cursor, &mut stdout]); + test_all_operations(cptestctx, &world, &mut stream) + .await + .expect("failed to write output"); + } - // XXX-dap add expectorate - panic!("blowing up to preserve results"); + let output = cursor.into_inner(); + expectorate::assert_contents( + "tests/output/authz-roles-test.txt", + &String::from_utf8_lossy(&output), + ); } #[derive(Debug)] @@ -142,7 +151,7 @@ impl Resource { &self, client: &'a ClientTestContext, username: &str, - ) -> Vec> { + ) -> ResourceTestOperations<'a> { // XXX-dap TODO examples: // - list children (various kinds) // - modify @@ -157,11 +166,176 @@ impl Resource { // kind of want to run every test again as every other role to make sure // it fails at some point. Then we want to check coverage of the // endpoints/role combinations. This seems really hard! + // TODO-coverage what other interesting cases aren't covered? + // - fetch policy (all resources) + // - update policy (how do we do this?!) match self.resource_type { - // XXX-dap TODO - ResourceType::Fleet => vec![], - // XXX-dap TODO - ResourceType::Silo { .. } => vec![], + ResourceType::Fleet => { + let silos_url = "/silos"; + let new_silo_name = username.parse().expect( + "invalid test Silo name (tried to use a username \ + that we generated)", + ); + let new_silo_description = format!("created by {}", username); + let silo_url = format!("{}/{}", &silos_url, new_silo_name); + ResourceTestOperations { + comment: "Fleet-level roles apply to everything in \ + the system. However, even fleet admins have no \ + way to refer to resources in Silos other than \ + the ones they exist in. This test creates fleet \ + admins in Silo \"s1\".", + operations: vec![ + TestOperation { + label: "FetchPolicy", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + "/policy", + )), + on_success: None, + }, + TestOperation { + label: "ListSagas", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + "/sagas", + )), + on_success: None, + }, + TestOperation { + label: "ListSleds", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + "/hardware/sleds", + )), + on_success: None, + }, + TestOperation { + label: "ListRacks", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + "/hardware/racks", + )), + on_success: None, + }, + TestOperation { + label: "ListBuiltinUsers", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + "/users", + )), + on_success: None, + }, + TestOperation { + label: "ListBuiltinRoles", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + "/roles", + )), + on_success: None, + }, + TestOperation { + label: "ListSilos", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + "/silos", + )), + on_success: None, + }, + TestOperation { + label: "CreateSilo", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::POST, + "/silos", + ) + .body(Some( + ¶ms::SiloCreate { + identity: + IdentityMetadataCreateParams { + name: new_silo_name, + description: + new_silo_description, + }, + discoverable: true, + }, + )), + ), + on_success: Some(NexusRequest::object_delete( + client, &silo_url, + )), + }, + ], + } + } + + ResourceType::Silo { name } => { + let resource_url = format!("{}/{}", self.create_url(), name); + let orgs_url = "/organizations"; + let new_org_name = username.parse().expect( + "invalid test organization name (tried to use a username \ + that we generated)", + ); + let new_org_description = format!("created by {}", username); + let org_url = format!("{}/{}", orgs_url, new_org_name); + ResourceTestOperations { + comment: "All users can view their own Silo. Roles are \ + required to list and create Organizations. Note \ + that administrators on \"s2\" appear to be able to \ + list and create Organizations in the output below, \ + but those are Organizations in \"s2\".", + operations: vec![ + TestOperation { + label: "Fetch", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &resource_url, + )), + on_success: None, + }, + TestOperation { + label: "ListOrganizations", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + orgs_url, + )), + on_success: None, + }, + TestOperation { + label: "CreateOrganization", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::POST, + orgs_url, + ) + .body(Some( + ¶ms::OrganizationCreate { + identity: + IdentityMetadataCreateParams { + name: new_org_name, + description: + new_org_description, + }, + }, + )), + ), + on_success: Some(NexusRequest::object_delete( + client, &org_url, + )), + }, + ], + } + } ResourceType::Organization { name, .. } => { let resource_url = format!("{}/{}", self.create_url(), name); @@ -174,72 +348,77 @@ impl Resource { format!("created by {}", username); let project_url = format!("{}/{}", &projects_url, new_project_name); - vec![ - TestOperation { - label: "Fetch", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - &resource_url, - )), - on_success: None, - }, - TestOperation { - label: "ListProjects", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - &projects_url, - )), - on_success: None, - }, - TestOperation { - label: "CreateProject", - template: NexusRequest::new( - RequestBuilder::new( + ResourceTestOperations { + comment: "", + operations: vec![ + TestOperation { + label: "Fetch", + template: NexusRequest::new(RequestBuilder::new( client, - Method::POST, - &projects_url, - ) - .body(Some( - ¶ms::ProjectCreate { - identity: IdentityMetadataCreateParams { - name: new_project_name, - description: new_project_description, - }, - }, + Method::GET, + &resource_url, )), - ), - on_success: Some(NexusRequest::object_delete( - client, - &project_url, - )), - }, - TestOperation { - label: "ModifyDescription", - template: NexusRequest::new( - RequestBuilder::new( + on_success: None, + }, + TestOperation { + label: "ListProjects", + template: NexusRequest::new(RequestBuilder::new( client, - Method::PUT, - &resource_url, - ) - .body(Some( - ¶ms::OrganizationUpdate { - identity: IdentityMetadataUpdateParams { - name: None, - description: Some(String::from( - "updated!", - )), + Method::GET, + &projects_url, + )), + on_success: None, + }, + TestOperation { + label: "CreateProject", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::POST, + &projects_url, + ) + .body(Some( + ¶ms::ProjectCreate { + identity: + IdentityMetadataCreateParams { + name: new_project_name, + description: + new_project_description, + }, }, - }, + )), + ), + on_success: Some(NexusRequest::object_delete( + client, + &project_url, )), - ), - on_success: None, - }, - ] + }, + TestOperation { + label: "ModifyDescription", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::PUT, + &resource_url, + ) + .body(Some( + ¶ms::OrganizationUpdate { + identity: + IdentityMetadataUpdateParams { + name: None, + description: Some( + String::from("updated!"), + ), + }, + }, + )), + ), + on_success: None, + }, + ], + } } - // XXX-dap TODO ResourceType::Project { name, .. } => { let resource_url = format!("{}/{}", self.create_url(), name); let vpcs_url = format!("{}/vpcs", &resource_url); @@ -249,70 +428,76 @@ impl Resource { ); let new_vpc_description = format!("created by {}", username); let vpc_url = format!("{}/{}", &vpcs_url, new_vpc_name); - vec![ - TestOperation { - label: "Fetch", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - &resource_url, - )), - on_success: None, - }, - TestOperation { - label: "ListVpcs", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - &vpcs_url, - )), - on_success: None, - }, - TestOperation { - label: "CreateVpc", - template: NexusRequest::new( - RequestBuilder::new( + ResourceTestOperations { + comment: "", + operations: vec![ + TestOperation { + label: "Fetch", + template: NexusRequest::new(RequestBuilder::new( client, - Method::POST, - &vpcs_url, - ) - .body(Some( - ¶ms::VpcCreate { - identity: IdentityMetadataCreateParams { - name: new_vpc_name.clone(), - description: new_vpc_description, - }, - ipv6_prefix: None, - dns_name: new_vpc_name, - }, + Method::GET, + &resource_url, )), - ), - on_success: Some(NexusRequest::object_delete( - client, &vpc_url, - )), - }, - TestOperation { - label: "ModifyDescription", - template: NexusRequest::new( - RequestBuilder::new( + on_success: None, + }, + TestOperation { + label: "ListVpcs", + template: NexusRequest::new(RequestBuilder::new( client, - Method::PUT, - &resource_url, - ) - .body(Some( - ¶ms::ProjectUpdate { - identity: IdentityMetadataUpdateParams { - name: None, - description: Some(String::from( - "updated!", - )), + Method::GET, + &vpcs_url, + )), + on_success: None, + }, + TestOperation { + label: "CreateVpc", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::POST, + &vpcs_url, + ) + .body(Some( + ¶ms::VpcCreate { + identity: + IdentityMetadataCreateParams { + name: new_vpc_name.clone(), + description: + new_vpc_description, + }, + ipv6_prefix: None, + dns_name: new_vpc_name, }, - }, + )), + ), + on_success: Some(NexusRequest::object_delete( + client, &vpc_url, )), - ), - on_success: None, - }, - ] + }, + TestOperation { + label: "ModifyDescription", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::PUT, + &resource_url, + ) + .body(Some( + ¶ms::ProjectUpdate { + identity: + IdentityMetadataUpdateParams { + name: None, + description: Some( + String::from("updated!"), + ), + }, + }, + )), + ), + on_success: None, + }, + ], + } } ResourceType::Vpc { name, .. } => { @@ -325,74 +510,81 @@ impl Resource { let new_subnet_description = format!("created by {}", username); let subnet_url = format!("{}/{}", &subnets_url, new_subnet_name); - vec![ - TestOperation { - label: "Fetch", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - &resource_url, - )), - on_success: None, - }, - TestOperation { - label: "ListSubnets", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - &subnets_url, - )), - on_success: None, - }, - TestOperation { - label: "CreateSubnet", - template: NexusRequest::new( - RequestBuilder::new( + + ResourceTestOperations { + comment: "", + operations: vec![ + TestOperation { + label: "Fetch", + template: NexusRequest::new(RequestBuilder::new( client, - Method::POST, - &subnets_url, - ) - .body(Some( - ¶ms::VpcSubnetCreate { - identity: IdentityMetadataCreateParams { - name: new_subnet_name.clone(), - description: new_subnet_description, - }, - ipv4_block: Ipv4Net( - "192.168.1.0/24".parse().unwrap(), - ), - ipv6_block: None, - }, + Method::GET, + &resource_url, )), - ), - on_success: Some(NexusRequest::object_delete( - client, - &subnet_url, - )), - }, - TestOperation { - label: "ModifyDescription", - template: NexusRequest::new( - RequestBuilder::new( + on_success: None, + }, + TestOperation { + label: "ListSubnets", + template: NexusRequest::new(RequestBuilder::new( client, - Method::PUT, - &resource_url, - ) - .body(Some( - ¶ms::VpcUpdate { - identity: IdentityMetadataUpdateParams { - name: None, - description: Some(String::from( - "updated!", - )), + Method::GET, + &subnets_url, + )), + on_success: None, + }, + TestOperation { + label: "CreateSubnet", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::POST, + &subnets_url, + ) + .body(Some( + ¶ms::VpcSubnetCreate { + identity: + IdentityMetadataCreateParams { + name: new_subnet_name.clone(), + description: + new_subnet_description, + }, + ipv4_block: Ipv4Net( + "192.168.1.0/24".parse().unwrap(), + ), + ipv6_block: None, }, - dns_name: None, - }, + )), + ), + on_success: Some(NexusRequest::object_delete( + client, + &subnet_url, )), - ), - on_success: None, - }, - ] + }, + TestOperation { + label: "ModifyDescription", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::PUT, + &resource_url, + ) + .body(Some( + ¶ms::VpcUpdate { + identity: + IdentityMetadataUpdateParams { + name: None, + description: Some( + String::from("updated!"), + ), + }, + dns_name: None, + }, + )), + ), + on_success: None, + }, + ], + } } } } @@ -588,6 +780,7 @@ async fn setup_hierarchy(testctx: &ControlPlaneTestContext) -> World { // use different users. for resource in &*ALL_RESOURCES { let resource = *resource; + println!("creating resource: {:?}", resource); debug!(log, "creating resource"; "resource" => ?resource); match resource.resource_type { ResourceType::Fleet => (), @@ -678,19 +871,19 @@ async fn setup_hierarchy(testctx: &ControlPlaneTestContext) -> World { for resource in &*RESOURCES_WITH_USERS { match resource.resource_type { - // XXX-dap we should be testing fleet users too // We don't create users for Vpcs. We already created users for // Silos. ResourceType::Vpc { .. } | ResourceType::Silo { .. } => { unimplemented!() } ResourceType::Fleet => { + let silo_id = silos.get("s1").expect("missing silo \"s1\""); create_users::( log, nexus, client, &resource.full_name(), - *omicron_nexus::db::fixed_data::silo::SILO_ID, + *silo_id, &resource.policy_url(), &mut users, None, @@ -836,6 +1029,11 @@ async fn create_users< } } +struct ResourceTestOperations<'a> { + comment: &'static str, + operations: Vec>, +} + struct TestOperation<'a> { label: &'static str, template: NexusRequest<'a>, @@ -848,27 +1046,29 @@ enum OperationResult { UnexpectedError(anyhow::Error), } -struct ResourceResults { - resource: &'static Resource, - test_labels: Vec<&'static str>, - results: Vec, -} - struct ResourceUserResults { username: String, results: Vec, } -async fn test_operations( +async fn test_all_operations( cptestctx: &ControlPlaneTestContext, world: &World, -) -> Vec { + mut out: W, +) -> std::io::Result<()> { let log = &cptestctx.logctx.log; let client = &cptestctx.external_client; - let mut results = Vec::with_capacity(world.resources.len()); for resource in &world.resources { + write!( + out, + "resource: {} {:?}", + resource.resource_type.as_ref(), + resource.full_name() + )?; + let mut user_results = Vec::with_capacity(world.users.len()); let mut test_labels: Option> = None; + let mut comment: Option<&'static str> = None; for ((user_resource_name, user_role_name), user_id) in &world.users { let username = format!("{}-{}", user_resource_name, user_role_name); let log = log.new(o!( @@ -876,11 +1076,15 @@ async fn test_operations( "user" => username.clone(), )); - let operations = resource.test_operations(client, &username); + let resource_operations = + resource.test_operations(client, &username); + let resource_comment = resource_operations.comment; + let operations = resource_operations.operations; if test_labels.is_none() { test_labels = Some(operations.iter().map(|t| t.label).collect()); } + comment = Some(resource_comment); user_results.push(ResourceUserResults { username, results: join_all(operations.into_iter().map( @@ -892,21 +1096,37 @@ async fn test_operations( }); } - let result = if let Some(test_labels) = test_labels { - assert!(!user_results.is_empty()); - ResourceResults { resource, test_labels, results: user_results } - } else { - assert!(user_results.is_empty()); - ResourceResults { - resource, - test_labels: Vec::new(), - results: Vec::new(), + if test_labels.is_none() { + write!(out, ": no tests defined\n\n")?; + continue; + } + + write!(out, "\n actions: {}\n", test_labels.unwrap().join(", "))?; + if let Some(comment) = comment { + write!(out, " note: {}\n\n", comment)?; + } + + write!(out, "{:20} {}\n", "USER", "RESULTS FOR EACH ACTION")?; + for user_result in &user_results { + write!(out, "{:20}", user_result.username)?; + for op_result in &user_result.results { + write!( + out, + " {}", + match op_result { + OperationResult::Success => '\u{2713}', + OperationResult::Denied => '\u{2717}', + OperationResult::UnexpectedError(_) => '\u{26a0}', + } + )?; } - }; - results.push(result); + write!(out, "\n")?; + } + + write!(out, "\n")?; } - results + Ok(()) } async fn run_test_operation<'a>( @@ -914,17 +1134,31 @@ async fn run_test_operation<'a>( to: TestOperation<'a>, user_id: Uuid, ) -> OperationResult { - info!(log, "test operation"; "operation" => to.label); + let log = log.new(o!("operation" => to.label)); + trace!(log, "test operation"); let request = to.template; let response = request .authn_as(AuthnMode::SiloUser(user_id)) .execute() .await .expect("failed to execute request"); + let req_id = response + .headers + .get(dropshot::HEADER_REQUEST_ID) + .unwrap() + .to_str() + .unwrap() + .to_owned(); + let log = log.new(o!( + "status_code" => response.status.to_string(), + "req_id" => req_id, + )); if matches!(response.status, StatusCode::NOT_FOUND | StatusCode::FORBIDDEN) { + info!(log, "test operation result"; "result" => "denied"); OperationResult::Denied } else if response.status.is_success() { + info!(log, "test operation result"; "result" => "success"); if let Some(request) = to.on_success { debug!(log, "on_success operation"); request @@ -939,6 +1173,12 @@ async fn run_test_operation<'a>( let status = response.status; let error_response: dropshot::HttpErrorResponseBody = response.parsed_body().expect("failed to parse error response"); + info!( + log, + "test operation result"; + "result" => "unexpected failure", + "message" => error_response.message.clone(), + ); OperationResult::UnexpectedError(anyhow!( "unexpected response: status code {}, message {:?}", status, @@ -947,48 +1187,37 @@ async fn run_test_operation<'a>( } } -fn dump_results( - mut out: W, - results: &[ResourceResults], -) -> std::io::Result<()> { - for resource_result in results { - write!( - out, - "resource: {} {:?}", - resource_result.resource.resource_type.as_ref(), - resource_result.resource.full_name() - )?; +struct DumbTee<'a> { + sinks: Vec<&'a mut dyn Write>, +} - if resource_result.test_labels.is_empty() { - write!(out, ": no tests defined\n\n")?; - continue; +impl<'a> DumbTee<'a> { + fn new(sinks: Vec<&mut dyn Write>) -> DumbTee { + DumbTee { sinks } + } +} + +impl<'a> Write for DumbTee<'a> { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + for sink in &mut self.sinks { + let size = sink + .write(buf) + .expect("one side of the tee failed unexpectedly"); + assert_eq!( + size, + buf.len(), + "tee can only be used with streams that always accept all data" + ); } - write!( - out, - "\n actions: {}\n\n", - resource_result.test_labels.join(", ") - )?; + Ok(buf.len()) + } - write!(out, "{:20} {}\n", "USER", "RESULTS FOR EACH ACTION")?; - for user_result in &resource_result.results { - write!(out, "{:20}", user_result.username)?; - for op_result in &user_result.results { - write!( - out, - " {}", - match op_result { - OperationResult::Success => '\u{2713}', - OperationResult::Denied => '\u{2717}', - OperationResult::UnexpectedError(_) => '\u{26a0}', - } - )?; - } - write!(out, "\n")?; + fn flush(&mut self) -> std::io::Result<()> { + for sink in &mut self.sinks { + sink.flush().expect("one side of the tee failed to flush"); } - write!(out, "\n")?; + Ok(()) } - - Ok(()) } diff --git a/nexus/tests/output/authz-roles-test.txt b/nexus/tests/output/authz-roles-test.txt new file mode 100644 index 00000000000..13f6465dd26 --- /dev/null +++ b/nexus/tests/output/authz-roles-test.txt @@ -0,0 +1,280 @@ +resource: Fleet "fleet" + actions: FetchPolicy, ListSagas, ListSleds, ListRacks, ListBuiltinUsers, ListBuiltinRoles, ListSilos, CreateSilo + note: Fleet-level roles apply to everything in the system. However, even fleet admins have no way to refer to resources in Silos other than the ones they exist in. This test creates fleet admins in Silo "s1". + +USER RESULTS FOR EACH ACTION +fleet-admin ✓ ✓ ✓ ✓ ✓ ✓ ✓ ✓ +fleet-collaborator ✓ ✓ ✓ ✓ ✓ ✓ ✓ ✓ +fleet-viewer ✓ ✓ ✓ ✓ ✓ ✓ ✓ ✗ +s1-admin ✗ ✗ ✗ ✗ ✗ ✗ ✗ ✗ +s1-collaborator ✗ ✗ ✗ ✗ ✗ ✗ ✗ ✗ +s1-viewer ✗ ✗ ✗ ✗ ✗ ✗ ✗ ✗ +s1o1-admin ✗ ✗ ✗ ✗ ✗ ✗ ✗ ✗ +s1o1-collaborator ✗ ✗ ✗ ✗ ✗ ✗ ✗ ✗ +s1o1p1-admin ✗ ✗ ✗ ✗ ✗ ✗ ✗ ✗ +s1o1p1-collaborator ✗ ✗ ✗ ✗ ✗ ✗ ✗ ✗ +s1o1p1-viewer ✗ ✗ ✗ ✗ ✗ ✗ ✗ ✗ +s2-admin ✗ ✗ ✗ ✗ ✗ ✗ ✗ ✗ +s2-collaborator ✗ ✗ ✗ ✗ ✗ ✗ ✗ ✗ +s2-viewer ✗ ✗ ✗ ✗ ✗ ✗ ✗ ✗ + +resource: Silo "s1" + actions: Fetch, ListOrganizations, CreateOrganization + note: All users can view their own Silo. Roles are required to list and create Organizations. Note that administrators on "s2" appear to be able to list and create Organizations in the output below, but those are Organizations in "s2". + +USER RESULTS FOR EACH ACTION +fleet-admin ✓ ✓ ✓ +fleet-collaborator ✓ ✓ ✓ +fleet-viewer ✓ ✗ ✗ +s1-admin ✓ ✓ ✓ +s1-collaborator ✓ ✓ ✓ +s1-viewer ✓ ✗ ✗ +s1o1-admin ✓ ✗ ✗ +s1o1-collaborator ✓ ✗ ✗ +s1o1p1-admin ✓ ✗ ✗ +s1o1p1-collaborator ✓ ✗ ✗ +s1o1p1-viewer ✓ ✗ ✗ +s2-admin ✗ ✓ ✓ +s2-collaborator ✗ ✓ ✓ +s2-viewer ✗ ✗ ✗ + +resource: Organization "s1o1" + actions: Fetch, ListProjects, CreateProject, ModifyDescription + note: + +USER RESULTS FOR EACH ACTION +fleet-admin ✓ ✓ ✓ ✓ +fleet-collaborator ✓ ✓ ✓ ✓ +fleet-viewer ✓ ✗ ✗ ✗ +s1-admin ✓ ✓ ✓ ✓ +s1-collaborator ✓ ✓ ✓ ✓ +s1-viewer ✓ ✗ ✗ ✗ +s1o1-admin ✓ ✓ ✓ ✓ +s1o1-collaborator ✓ ✓ ✓ ✗ +s1o1p1-admin ✓ ✗ ✗ ✗ +s1o1p1-collaborator ✓ ✗ ✗ ✗ +s1o1p1-viewer ✓ ✗ ✗ ✗ +s2-admin ✗ ✗ ✗ ✗ +s2-collaborator ✗ ✗ ✗ ✗ +s2-viewer ✗ ✗ ✗ ✗ + +resource: Project "s1o1p1" + actions: Fetch, ListVpcs, CreateVpc, ModifyDescription + note: + +USER RESULTS FOR EACH ACTION +fleet-admin ✓ ✓ ✓ ✓ +fleet-collaborator ✓ ✓ ✓ ✓ +fleet-viewer ✗ ✗ ✗ ✗ +s1-admin ✓ ✓ ✓ ✓ +s1-collaborator ✓ ✓ ✓ ✓ +s1-viewer ✗ ✗ ✗ ✗ +s1o1-admin ✓ ✓ ✓ ✓ +s1o1-collaborator ✓ ✓ ✓ ✓ +s1o1p1-admin ✓ ✓ ✓ ✓ +s1o1p1-collaborator ✓ ✓ ✓ ✗ +s1o1p1-viewer ✓ ✓ ✗ ✗ +s2-admin ✗ ✗ ✗ ✗ +s2-collaborator ✗ ✗ ✗ ✗ +s2-viewer ✗ ✗ ✗ ✗ + +resource: Vpc "s1o1p1v1" + actions: Fetch, ListSubnets, CreateSubnet, ModifyDescription + note: + +USER RESULTS FOR EACH ACTION +fleet-admin ✓ ✓ ✓ ✓ +fleet-collaborator ✓ ✓ ✓ ✓ +fleet-viewer ✗ ✗ ✗ ✗ +s1-admin ✓ ✓ ✓ ✓ +s1-collaborator ✓ ✓ ✓ ✓ +s1-viewer ✗ ✗ ✗ ✗ +s1o1-admin ✓ ✓ ✓ ✓ +s1o1-collaborator ✓ ✓ ✓ ✓ +s1o1p1-admin ✓ ✓ ✓ ✓ +s1o1p1-collaborator ✓ ✓ ✓ ✓ +s1o1p1-viewer ✓ ✓ ✗ ✗ +s2-admin ✗ ✗ ✗ ✗ +s2-collaborator ✗ ✗ ✗ ✗ +s2-viewer ✗ ✗ ✗ ✗ + +resource: Project "s1o1p2" + actions: Fetch, ListVpcs, CreateVpc, ModifyDescription + note: + +USER RESULTS FOR EACH ACTION +fleet-admin ✓ ✓ ✓ ✓ +fleet-collaborator ✓ ✓ ✓ ✓ +fleet-viewer ✗ ✗ ✗ ✗ +s1-admin ✓ ✓ ✓ ✓ +s1-collaborator ✓ ✓ ✓ ✓ +s1-viewer ✗ ✗ ✗ ✗ +s1o1-admin ✓ ✓ ✓ ✓ +s1o1-collaborator ✓ ✓ ✓ ✓ +s1o1p1-admin ✗ ✗ ✗ ✗ +s1o1p1-collaborator ✗ ✗ ✗ ✗ +s1o1p1-viewer ✗ ✗ ✗ ✗ +s2-admin ✗ ✗ ✗ ✗ +s2-collaborator ✗ ✗ ✗ ✗ +s2-viewer ✗ ✗ ✗ ✗ + +resource: Vpc "s1o1p2v1" + actions: Fetch, ListSubnets, CreateSubnet, ModifyDescription + note: + +USER RESULTS FOR EACH ACTION +fleet-admin ✓ ✓ ✓ ✓ +fleet-collaborator ✓ ✓ ✓ ✓ +fleet-viewer ✗ ✗ ✗ ✗ +s1-admin ✓ ✓ ✓ ✓ +s1-collaborator ✓ ✓ ✓ ✓ +s1-viewer ✗ ✗ ✗ ✗ +s1o1-admin ✓ ✓ ✓ ✓ +s1o1-collaborator ✓ ✓ ✓ ✓ +s1o1p1-admin ✗ ✗ ✗ ✗ +s1o1p1-collaborator ✗ ✗ ✗ ✗ +s1o1p1-viewer ✗ ✗ ✗ ✗ +s2-admin ✗ ✗ ✗ ✗ +s2-collaborator ✗ ✗ ✗ ✗ +s2-viewer ✗ ✗ ✗ ✗ + +resource: Organization "s1o2" + actions: Fetch, ListProjects, CreateProject, ModifyDescription + note: + +USER RESULTS FOR EACH ACTION +fleet-admin ✓ ✓ ✓ ✓ +fleet-collaborator ✓ ✓ ✓ ✓ +fleet-viewer ✓ ✗ ✗ ✗ +s1-admin ✓ ✓ ✓ ✓ +s1-collaborator ✓ ✓ ✓ ✓ +s1-viewer ✓ ✗ ✗ ✗ +s1o1-admin ✓ ✗ ✗ ✗ +s1o1-collaborator ✓ ✗ ✗ ✗ +s1o1p1-admin ✓ ✗ ✗ ✗ +s1o1p1-collaborator ✓ ✗ ✗ ✗ +s1o1p1-viewer ✓ ✗ ✗ ✗ +s2-admin ✗ ✗ ✗ ✗ +s2-collaborator ✗ ✗ ✗ ✗ +s2-viewer ✗ ✗ ✗ ✗ + +resource: Project "s1o2p1" + actions: Fetch, ListVpcs, CreateVpc, ModifyDescription + note: + +USER RESULTS FOR EACH ACTION +fleet-admin ✓ ✓ ✓ ✓ +fleet-collaborator ✓ ✓ ✓ ✓ +fleet-viewer ✗ ✗ ✗ ✗ +s1-admin ✓ ✓ ✓ ✓ +s1-collaborator ✓ ✓ ✓ ✓ +s1-viewer ✗ ✗ ✗ ✗ +s1o1-admin ✗ ✗ ✗ ✗ +s1o1-collaborator ✗ ✗ ✗ ✗ +s1o1p1-admin ✗ ✗ ✗ ✗ +s1o1p1-collaborator ✗ ✗ ✗ ✗ +s1o1p1-viewer ✗ ✗ ✗ ✗ +s2-admin ✗ ✗ ✗ ✗ +s2-collaborator ✗ ✗ ✗ ✗ +s2-viewer ✗ ✗ ✗ ✗ + +resource: Vpc "s1o2p1v1" + actions: Fetch, ListSubnets, CreateSubnet, ModifyDescription + note: + +USER RESULTS FOR EACH ACTION +fleet-admin ✓ ✓ ✓ ✓ +fleet-collaborator ✓ ✓ ✓ ✓ +fleet-viewer ✗ ✗ ✗ ✗ +s1-admin ✓ ✓ ✓ ✓ +s1-collaborator ✓ ✓ ✓ ✓ +s1-viewer ✗ ✗ ✗ ✗ +s1o1-admin ✗ ✗ ✗ ✗ +s1o1-collaborator ✗ ✗ ✗ ✗ +s1o1p1-admin ✗ ✗ ✗ ✗ +s1o1p1-collaborator ✗ ✗ ✗ ✗ +s1o1p1-viewer ✗ ✗ ✗ ✗ +s2-admin ✗ ✗ ✗ ✗ +s2-collaborator ✗ ✗ ✗ ✗ +s2-viewer ✗ ✗ ✗ ✗ + +resource: Silo "s2" + actions: Fetch, ListOrganizations, CreateOrganization + note: All users can view their own Silo. Roles are required to list and create Organizations. Note that administrators on "s2" appear to be able to list and create Organizations in the output below, but those are Organizations in "s2". + +USER RESULTS FOR EACH ACTION +fleet-admin ✓ ✓ ✓ +fleet-collaborator ✓ ✓ ✓ +fleet-viewer ✓ ✗ ✗ +s1-admin ✗ ✓ ✓ +s1-collaborator ✗ ✓ ✓ +s1-viewer ✗ ✗ ✗ +s1o1-admin ✗ ✗ ✗ +s1o1-collaborator ✗ ✗ ✗ +s1o1p1-admin ✗ ✗ ✗ +s1o1p1-collaborator ✗ ✗ ✗ +s1o1p1-viewer ✗ ✗ ✗ +s2-admin ✓ ✓ ✓ +s2-collaborator ✓ ✓ ✓ +s2-viewer ✓ ✗ ✗ + +resource: Organization "s2o3" + actions: Fetch, ListProjects, CreateProject, ModifyDescription + note: + +USER RESULTS FOR EACH ACTION +fleet-admin ✗ ✗ ✗ ✗ +fleet-collaborator ✗ ✗ ✗ ✗ +fleet-viewer ✗ ✗ ✗ ✗ +s1-admin ✗ ✗ ✗ ✗ +s1-collaborator ✗ ✗ ✗ ✗ +s1-viewer ✗ ✗ ✗ ✗ +s1o1-admin ✗ ✗ ✗ ✗ +s1o1-collaborator ✗ ✗ ✗ ✗ +s1o1p1-admin ✗ ✗ ✗ ✗ +s1o1p1-collaborator ✗ ✗ ✗ ✗ +s1o1p1-viewer ✗ ✗ ✗ ✗ +s2-admin ✓ ✓ ✓ ✓ +s2-collaborator ✓ ✓ ✓ ✓ +s2-viewer ✓ ✗ ✗ ✗ + +resource: Project "s2o3p1" + actions: Fetch, ListVpcs, CreateVpc, ModifyDescription + note: + +USER RESULTS FOR EACH ACTION +fleet-admin ✗ ✗ ✗ ✗ +fleet-collaborator ✗ ✗ ✗ ✗ +fleet-viewer ✗ ✗ ✗ ✗ +s1-admin ✗ ✗ ✗ ✗ +s1-collaborator ✗ ✗ ✗ ✗ +s1-viewer ✗ ✗ ✗ ✗ +s1o1-admin ✗ ✗ ✗ ✗ +s1o1-collaborator ✗ ✗ ✗ ✗ +s1o1p1-admin ✗ ✗ ✗ ✗ +s1o1p1-collaborator ✗ ✗ ✗ ✗ +s1o1p1-viewer ✗ ✗ ✗ ✗ +s2-admin ✓ ✓ ✓ ✓ +s2-collaborator ✓ ✓ ✓ ✓ +s2-viewer ✗ ✗ ✗ ✗ + +resource: Vpc "s2o3p1v1" + actions: Fetch, ListSubnets, CreateSubnet, ModifyDescription + note: + +USER RESULTS FOR EACH ACTION +fleet-admin ✗ ✗ ✗ ✗ +fleet-collaborator ✗ ✗ ✗ ✗ +fleet-viewer ✗ ✗ ✗ ✗ +s1-admin ✗ ✗ ✗ ✗ +s1-collaborator ✗ ✗ ✗ ✗ +s1-viewer ✗ ✗ ✗ ✗ +s1o1-admin ✗ ✗ ✗ ✗ +s1o1-collaborator ✗ ✗ ✗ ✗ +s1o1p1-admin ✗ ✗ ✗ ✗ +s1o1p1-collaborator ✗ ✗ ✗ ✗ +s1o1p1-viewer ✗ ✗ ✗ ✗ +s2-admin ✓ ✓ ✓ ✓ +s2-collaborator ✓ ✓ ✓ ✓ +s2-viewer ✗ ✗ ✗ ✗ + From 6f2cfb612117a22af038729eb3e0dbbeaba24905 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 23 May 2022 14:42:58 -0700 Subject: [PATCH 08/11] output could be more real-time still --- nexus/tests/integration_tests/authz_roles.rs | 63 ++++++-------------- 1 file changed, 18 insertions(+), 45 deletions(-) diff --git a/nexus/tests/integration_tests/authz_roles.rs b/nexus/tests/integration_tests/authz_roles.rs index 13be5d40d94..00a416e264a 100644 --- a/nexus/tests/integration_tests/authz_roles.rs +++ b/nexus/tests/integration_tests/authz_roles.rs @@ -5,16 +5,9 @@ //! (Fairly) comprehensive test of authz by creating a hierarchy of resources //! and a group of users with various roles on these resources and verifying //! that each role grants the privileges we expect. -//! XXX-dap bugs found / confusing behavior: -//! - it shouldn't be the case that you can create a Silo that you cannot -//! delete. either we should create an initial role assignment for you or we -//! should have a role that you get "admin" on Silo if you have "collaborator" -//! on parent_fleet? -//! - why is it that everyone who can create an Organization can also delete it? use anyhow::anyhow; use dropshot::test_util::ClientTestContext; -use futures::future::join_all; use http::Method; use http::StatusCode; use lazy_static::lazy_static; @@ -925,6 +918,7 @@ async fn setup_hierarchy(testctx: &ControlPlaneTestContext) -> World { } } + println!("setup done\n"); World { users, resources: ALL_RESOURCES.clone() } } @@ -975,6 +969,7 @@ async fn create_users< // TODO when silo users get user names, this should go into it. let username = format!("{}-{}", resource_name, role_name); let user_id = Uuid::new_v4(); + println!("creating user: {}", &username); debug!( log, "creating user"; @@ -987,6 +982,7 @@ async fn create_users< .unwrap_or_else(|_| panic!("failed to create user {:?}", username)); users.insert((resource_name.to_owned(), role_name.to_owned()), user_id); + println!("adding role {} for user {}", role_name, &username); debug!( log, "adding role"; @@ -1046,11 +1042,6 @@ enum OperationResult { UnexpectedError(anyhow::Error), } -struct ResourceUserResults { - username: String, - results: Vec, -} - async fn test_all_operations( cptestctx: &ControlPlaneTestContext, world: &World, @@ -1061,14 +1052,12 @@ async fn test_all_operations( for resource in &world.resources { write!( out, - "resource: {} {:?}", + "resource: {} {:?}\n", resource.resource_type.as_ref(), resource.full_name() )?; - let mut user_results = Vec::with_capacity(world.users.len()); - let mut test_labels: Option> = None; - let mut comment: Option<&'static str> = None; + let mut first = true; for ((user_resource_name, user_role_name), user_id) in &world.users { let username = format!("{}-{}", user_resource_name, user_role_name); let log = log.new(o!( @@ -1078,38 +1067,21 @@ async fn test_all_operations( let resource_operations = resource.test_operations(client, &username); - let resource_comment = resource_operations.comment; + let comment = resource_operations.comment; let operations = resource_operations.operations; - if test_labels.is_none() { - test_labels = - Some(operations.iter().map(|t| t.label).collect()); + if first { + let test_labels = + operations.iter().map(|t| t.label).collect::>(); + write!(out, " actions: {}\n", test_labels.join(", "))?; + write!(out, " note: {}\n\n", comment)?; + write!(out, "{:20} {}\n", "USER", "RESULTS FOR EACH ACTION")?; + first = false; } - comment = Some(resource_comment); - user_results.push(ResourceUserResults { - username, - results: join_all(operations.into_iter().map( - |test_operation| { - run_test_operation(&log, test_operation, *user_id) - }, - )) - .await, - }); - } - if test_labels.is_none() { - write!(out, ": no tests defined\n\n")?; - continue; - } - - write!(out, "\n actions: {}\n", test_labels.unwrap().join(", "))?; - if let Some(comment) = comment { - write!(out, " note: {}\n\n", comment)?; - } - - write!(out, "{:20} {}\n", "USER", "RESULTS FOR EACH ACTION")?; - for user_result in &user_results { - write!(out, "{:20}", user_result.username)?; - for op_result in &user_result.results { + write!(out, "{:20}", username)?; + for test_operation in operations.into_iter() { + let op_result = + run_test_operation(&log, test_operation, *user_id).await; write!( out, " {}", @@ -1120,6 +1092,7 @@ async fn test_all_operations( } )?; } + write!(out, "\n")?; } From 311da19e3fe8a424697a7d2a9789c6ba33411df0 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 23 May 2022 15:51:52 -0700 Subject: [PATCH 09/11] fix some bugs --- nexus/src/app/project.rs | 4 +- nexus/src/authz/omicron.polar | 28 +- nexus/tests/integration_tests/authz_roles.rs | 660 +++++++++---------- nexus/tests/output/authz-roles-test.txt | 94 +-- 4 files changed, 367 insertions(+), 419 deletions(-) diff --git a/nexus/src/app/project.rs b/nexus/src/app/project.rs index e63a14ecf10..e8b50de9ee4 100644 --- a/nexus/src/app/project.rs +++ b/nexus/src/app/project.rs @@ -96,7 +96,7 @@ impl super::Nexus { ) -> ListResultVec { let (.., authz_org) = LookupPath::new(opctx, &self.db_datastore) .organization_name(organization_name) - .lookup_for(authz::Action::CreateChild) + .lookup_for(authz::Action::ListChildren) .await?; self.db_datastore .projects_list_by_name(opctx, &authz_org, pagparams) @@ -111,7 +111,7 @@ impl super::Nexus { ) -> ListResultVec { let (.., authz_org) = LookupPath::new(opctx, &self.db_datastore) .organization_name(organization_name) - .lookup_for(authz::Action::CreateChild) + .lookup_for(authz::Action::ListChildren) .await?; self.db_datastore .projects_list_by_id(opctx, &authz_org, pagparams) diff --git a/nexus/src/authz/omicron.polar b/nexus/src/authz/omicron.polar index 80d763a9197..fe8112b86cf 100644 --- a/nexus/src/authz/omicron.polar +++ b/nexus/src/authz/omicron.polar @@ -137,22 +137,42 @@ resource Silo { ]; roles = [ "admin", "collaborator", "viewer" ]; + # permissions granted by this resource's roles + "list_children" if "viewer"; "read" if "viewer"; - "list_children" if "collaborator"; "create_child" if "collaborator"; "modify" if "admin"; + # roles implied by other roles "viewer" if "collaborator"; "collaborator" if "admin"; + + # roles implied by relationships with the parent fleet relations = { parent_fleet: Fleet }; "admin" if "collaborator" on "parent_fleet"; "viewer" if "viewer" on "parent_fleet"; + "list_children" if "viewer" on "parent_fleet"; } has_relation(fleet: Fleet, "parent_fleet", silo: Silo) if silo.fleet = fleet; -has_role(actor: AuthenticatedActor, "viewer", silo: Silo) + +# All authenticated users can read their own Silo. That's not quite the same as +# having the "viewer" role. For example, they cannot list Organizations in the +# Silo. +# +# One reason this is necessary is because if an unprivileged user tries to +# create an Organization using "POST /organizations", they should get back a 403 +# (which implies they're able to see /organizations, which is essentially seeing +# the Silo itself) rather than a 404. This behavior isn't a hard constraint +# (i.e., you could reasonably get a 404 for an API you're not allowed to call). +# Nor is the implementation (i.e., we could special-case this endpoint somehow). +# But granting this permission is the simplest way to keep this endpoint's +# behavior consistent with the rest of the API. +# +# It's unclear what else would break if users couldn't see their own Silo. +has_permission(actor: AuthenticatedActor, "read", silo: Silo) # TODO-security TODO-coverage We should have a test that exercises this - # case. + # syntax. if silo in actor.silo; resource Organization { @@ -214,6 +234,8 @@ resource Project { relations = { parent_organization: Organization }; "admin" if "collaborator" on "parent_organization"; + + "viewer" if "list_children" on "parent_organization"; } has_relation(organization: Organization, "parent_organization", project: Project) if project.organization = organization; diff --git a/nexus/tests/integration_tests/authz_roles.rs b/nexus/tests/integration_tests/authz_roles.rs index 00a416e264a..ba5f75522f2 100644 --- a/nexus/tests/integration_tests/authz_roles.rs +++ b/nexus/tests/integration_tests/authz_roles.rs @@ -52,6 +52,17 @@ async fn test_authz_roles(cptestctx: &ControlPlaneTestContext) { .expect("failed to write output"); } + // Some general notes if you're debugging a failure in this test: + // + // Fleet-level roles apply to everything in the system. However, even fleet + // admins have no way to refer to resources in Silos other than the ones + // they exist in. This test creates fleet admins in Silo "s1". So they can + // do all the usual things on the resources in "s1", but not in "s2". + // + // Administrators on "s2" appear to be able to list and create Organizations + // in Silo "s1" in the test's output. But that just means they can list and + // create things under /organizations -- that's listing and creating + // Organizations in "s2", not "s1". let output = cursor.into_inner(); expectorate::assert_contents( "tests/output/authz-roles-test.txt", @@ -144,7 +155,7 @@ impl Resource { &self, client: &'a ClientTestContext, username: &str, - ) -> ResourceTestOperations<'a> { + ) -> Vec> { // XXX-dap TODO examples: // - list children (various kinds) // - modify @@ -171,102 +182,87 @@ impl Resource { ); let new_silo_description = format!("created by {}", username); let silo_url = format!("{}/{}", &silos_url, new_silo_name); - ResourceTestOperations { - comment: "Fleet-level roles apply to everything in \ - the system. However, even fleet admins have no \ - way to refer to resources in Silos other than \ - the ones they exist in. This test creates fleet \ - admins in Silo \"s1\".", - operations: vec![ - TestOperation { - label: "FetchPolicy", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - "/policy", - )), - on_success: None, - }, - TestOperation { - label: "ListSagas", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - "/sagas", - )), - on_success: None, - }, - TestOperation { - label: "ListSleds", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - "/hardware/sleds", - )), - on_success: None, - }, - TestOperation { - label: "ListRacks", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - "/hardware/racks", - )), - on_success: None, - }, - TestOperation { - label: "ListBuiltinUsers", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - "/users", - )), - on_success: None, - }, - TestOperation { - label: "ListBuiltinRoles", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - "/roles", - )), - on_success: None, - }, - TestOperation { - label: "ListSilos", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - "/silos", - )), - on_success: None, - }, - TestOperation { - label: "CreateSilo", - template: NexusRequest::new( - RequestBuilder::new( - client, - Method::POST, - "/silos", - ) - .body(Some( - ¶ms::SiloCreate { - identity: - IdentityMetadataCreateParams { - name: new_silo_name, - description: - new_silo_description, - }, - discoverable: true, + vec![ + TestOperation { + label: "FetchPolicy", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + "/policy", + )), + on_success: None, + }, + TestOperation { + label: "ListSagas", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + "/sagas", + )), + on_success: None, + }, + TestOperation { + label: "ListSleds", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + "/hardware/sleds", + )), + on_success: None, + }, + TestOperation { + label: "ListRacks", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + "/hardware/racks", + )), + on_success: None, + }, + TestOperation { + label: "ListBuiltinUsers", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + "/users", + )), + on_success: None, + }, + TestOperation { + label: "ListBuiltinRoles", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + "/roles", + )), + on_success: None, + }, + TestOperation { + label: "ListSilos", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + "/silos", + )), + on_success: None, + }, + TestOperation { + label: "CreateSilo", + template: NexusRequest::new( + RequestBuilder::new(client, Method::POST, "/silos") + .body(Some(¶ms::SiloCreate { + identity: IdentityMetadataCreateParams { + name: new_silo_name, + description: new_silo_description, }, - )), - ), - on_success: Some(NexusRequest::object_delete( - client, &silo_url, - )), - }, - ], - } + discoverable: true, + })), + ), + on_success: Some(NexusRequest::object_delete( + client, &silo_url, + )), + }, + ] } ResourceType::Silo { name } => { @@ -278,56 +274,41 @@ impl Resource { ); let new_org_description = format!("created by {}", username); let org_url = format!("{}/{}", orgs_url, new_org_name); - ResourceTestOperations { - comment: "All users can view their own Silo. Roles are \ - required to list and create Organizations. Note \ - that administrators on \"s2\" appear to be able to \ - list and create Organizations in the output below, \ - but those are Organizations in \"s2\".", - operations: vec![ - TestOperation { - label: "Fetch", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - &resource_url, - )), - on_success: None, - }, - TestOperation { - label: "ListOrganizations", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - orgs_url, - )), - on_success: None, - }, - TestOperation { - label: "CreateOrganization", - template: NexusRequest::new( - RequestBuilder::new( - client, - Method::POST, - orgs_url, - ) - .body(Some( - ¶ms::OrganizationCreate { - identity: - IdentityMetadataCreateParams { - name: new_org_name, - description: - new_org_description, - }, + vec![ + TestOperation { + label: "Fetch", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &resource_url, + )), + on_success: None, + }, + TestOperation { + label: "ListOrganizations", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + orgs_url, + )), + on_success: None, + }, + TestOperation { + label: "CreateOrganization", + template: NexusRequest::new( + RequestBuilder::new(client, Method::POST, orgs_url) + .body(Some(¶ms::OrganizationCreate { + identity: IdentityMetadataCreateParams { + name: new_org_name, + description: new_org_description, }, - )), - ), - on_success: Some(NexusRequest::object_delete( - client, &org_url, - )), - }, - ], - } + })), + ), + on_success: Some(NexusRequest::object_delete( + client, &org_url, + )), + }, + ] } ResourceType::Organization { name, .. } => { @@ -341,75 +322,69 @@ impl Resource { format!("created by {}", username); let project_url = format!("{}/{}", &projects_url, new_project_name); - ResourceTestOperations { - comment: "", - operations: vec![ - TestOperation { - label: "Fetch", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - &resource_url, - )), - on_success: None, - }, - TestOperation { - label: "ListProjects", - template: NexusRequest::new(RequestBuilder::new( + vec![ + TestOperation { + label: "Fetch", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &resource_url, + )), + on_success: None, + }, + TestOperation { + label: "ListProjects", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &projects_url, + )), + on_success: None, + }, + TestOperation { + label: "CreateProject", + template: NexusRequest::new( + RequestBuilder::new( client, - Method::GET, + Method::POST, &projects_url, - )), - on_success: None, - }, - TestOperation { - label: "CreateProject", - template: NexusRequest::new( - RequestBuilder::new( - client, - Method::POST, - &projects_url, - ) - .body(Some( - ¶ms::ProjectCreate { - identity: - IdentityMetadataCreateParams { - name: new_project_name, - description: - new_project_description, - }, + ) + .body(Some( + ¶ms::ProjectCreate { + identity: IdentityMetadataCreateParams { + name: new_project_name, + description: new_project_description, }, - )), - ), - on_success: Some(NexusRequest::object_delete( - client, - &project_url, + }, )), - }, - TestOperation { - label: "ModifyDescription", - template: NexusRequest::new( - RequestBuilder::new( - client, - Method::PUT, - &resource_url, - ) - .body(Some( - ¶ms::OrganizationUpdate { - identity: - IdentityMetadataUpdateParams { - name: None, - description: Some( - String::from("updated!"), - ), - }, + ), + on_success: Some(NexusRequest::object_delete( + client, + &project_url, + )), + }, + TestOperation { + label: "ModifyDescription", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::PUT, + &resource_url, + ) + .body(Some( + ¶ms::OrganizationUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some(String::from( + "updated!", + )), }, - )), - ), - on_success: None, - }, - ], - } + }, + )), + ), + on_success: None, + }, + ] } ResourceType::Project { name, .. } => { @@ -421,76 +396,70 @@ impl Resource { ); let new_vpc_description = format!("created by {}", username); let vpc_url = format!("{}/{}", &vpcs_url, new_vpc_name); - ResourceTestOperations { - comment: "", - operations: vec![ - TestOperation { - label: "Fetch", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - &resource_url, - )), - on_success: None, - }, - TestOperation { - label: "ListVpcs", - template: NexusRequest::new(RequestBuilder::new( + vec![ + TestOperation { + label: "Fetch", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &resource_url, + )), + on_success: None, + }, + TestOperation { + label: "ListVpcs", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &vpcs_url, + )), + on_success: None, + }, + TestOperation { + label: "CreateVpc", + template: NexusRequest::new( + RequestBuilder::new( client, - Method::GET, + Method::POST, &vpcs_url, - )), - on_success: None, - }, - TestOperation { - label: "CreateVpc", - template: NexusRequest::new( - RequestBuilder::new( - client, - Method::POST, - &vpcs_url, - ) - .body(Some( - ¶ms::VpcCreate { - identity: - IdentityMetadataCreateParams { - name: new_vpc_name.clone(), - description: - new_vpc_description, - }, - ipv6_prefix: None, - dns_name: new_vpc_name, + ) + .body(Some( + ¶ms::VpcCreate { + identity: IdentityMetadataCreateParams { + name: new_vpc_name.clone(), + description: new_vpc_description, }, - )), - ), - on_success: Some(NexusRequest::object_delete( - client, &vpc_url, + ipv6_prefix: None, + dns_name: new_vpc_name, + }, )), - }, - TestOperation { - label: "ModifyDescription", - template: NexusRequest::new( - RequestBuilder::new( - client, - Method::PUT, - &resource_url, - ) - .body(Some( - ¶ms::ProjectUpdate { - identity: - IdentityMetadataUpdateParams { - name: None, - description: Some( - String::from("updated!"), - ), - }, + ), + on_success: Some(NexusRequest::object_delete( + client, &vpc_url, + )), + }, + TestOperation { + label: "ModifyDescription", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::PUT, + &resource_url, + ) + .body(Some( + ¶ms::ProjectUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some(String::from( + "updated!", + )), }, - )), - ), - on_success: None, - }, - ], - } + }, + )), + ), + on_success: None, + }, + ] } ResourceType::Vpc { name, .. } => { @@ -504,80 +473,74 @@ impl Resource { let subnet_url = format!("{}/{}", &subnets_url, new_subnet_name); - ResourceTestOperations { - comment: "", - operations: vec![ - TestOperation { - label: "Fetch", - template: NexusRequest::new(RequestBuilder::new( - client, - Method::GET, - &resource_url, - )), - on_success: None, - }, - TestOperation { - label: "ListSubnets", - template: NexusRequest::new(RequestBuilder::new( + vec![ + TestOperation { + label: "Fetch", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &resource_url, + )), + on_success: None, + }, + TestOperation { + label: "ListSubnets", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &subnets_url, + )), + on_success: None, + }, + TestOperation { + label: "CreateSubnet", + template: NexusRequest::new( + RequestBuilder::new( client, - Method::GET, + Method::POST, &subnets_url, - )), - on_success: None, - }, - TestOperation { - label: "CreateSubnet", - template: NexusRequest::new( - RequestBuilder::new( - client, - Method::POST, - &subnets_url, - ) - .body(Some( - ¶ms::VpcSubnetCreate { - identity: - IdentityMetadataCreateParams { - name: new_subnet_name.clone(), - description: - new_subnet_description, - }, - ipv4_block: Ipv4Net( - "192.168.1.0/24".parse().unwrap(), - ), - ipv6_block: None, + ) + .body(Some( + ¶ms::VpcSubnetCreate { + identity: IdentityMetadataCreateParams { + name: new_subnet_name.clone(), + description: new_subnet_description, }, - )), - ), - on_success: Some(NexusRequest::object_delete( - client, - &subnet_url, + ipv4_block: Ipv4Net( + "192.168.1.0/24".parse().unwrap(), + ), + ipv6_block: None, + }, )), - }, - TestOperation { - label: "ModifyDescription", - template: NexusRequest::new( - RequestBuilder::new( - client, - Method::PUT, - &resource_url, - ) - .body(Some( - ¶ms::VpcUpdate { - identity: - IdentityMetadataUpdateParams { - name: None, - description: Some( - String::from("updated!"), - ), - }, - dns_name: None, + ), + on_success: Some(NexusRequest::object_delete( + client, + &subnet_url, + )), + }, + TestOperation { + label: "ModifyDescription", + template: NexusRequest::new( + RequestBuilder::new( + client, + Method::PUT, + &resource_url, + ) + .body(Some( + ¶ms::VpcUpdate { + identity: IdentityMetadataUpdateParams { + name: None, + description: Some(String::from( + "updated!", + )), }, - )), - ), - on_success: None, - }, - ], - } + dns_name: None, + }, + )), + ), + on_success: None, + }, + ] } } } @@ -1025,11 +988,6 @@ async fn create_users< } } -struct ResourceTestOperations<'a> { - comment: &'static str, - operations: Vec>, -} - struct TestOperation<'a> { label: &'static str, template: NexusRequest<'a>, @@ -1065,15 +1023,11 @@ async fn test_all_operations( "user" => username.clone(), )); - let resource_operations = - resource.test_operations(client, &username); - let comment = resource_operations.comment; - let operations = resource_operations.operations; + let operations = resource.test_operations(client, &username); if first { let test_labels = operations.iter().map(|t| t.label).collect::>(); write!(out, " actions: {}\n", test_labels.join(", "))?; - write!(out, " note: {}\n\n", comment)?; write!(out, "{:20} {}\n", "USER", "RESULTS FOR EACH ACTION")?; first = false; } diff --git a/nexus/tests/output/authz-roles-test.txt b/nexus/tests/output/authz-roles-test.txt index 13f6465dd26..7bae2e5d80d 100644 --- a/nexus/tests/output/authz-roles-test.txt +++ b/nexus/tests/output/authz-roles-test.txt @@ -1,7 +1,5 @@ resource: Fleet "fleet" actions: FetchPolicy, ListSagas, ListSleds, ListRacks, ListBuiltinUsers, ListBuiltinRoles, ListSilos, CreateSilo - note: Fleet-level roles apply to everything in the system. However, even fleet admins have no way to refer to resources in Silos other than the ones they exist in. This test creates fleet admins in Silo "s1". - USER RESULTS FOR EACH ACTION fleet-admin ✓ ✓ ✓ ✓ ✓ ✓ ✓ ✓ fleet-collaborator ✓ ✓ ✓ ✓ ✓ ✓ ✓ ✓ @@ -20,15 +18,13 @@ s2-viewer ✗ ✗ ✗ ✗ ✗ ✗ ✗ ✗ resource: Silo "s1" actions: Fetch, ListOrganizations, CreateOrganization - note: All users can view their own Silo. Roles are required to list and create Organizations. Note that administrators on "s2" appear to be able to list and create Organizations in the output below, but those are Organizations in "s2". - USER RESULTS FOR EACH ACTION fleet-admin ✓ ✓ ✓ fleet-collaborator ✓ ✓ ✓ -fleet-viewer ✓ ✗ ✗ +fleet-viewer ✓ ✓ ✗ s1-admin ✓ ✓ ✓ s1-collaborator ✓ ✓ ✓ -s1-viewer ✓ ✗ ✗ +s1-viewer ✓ ✓ ✗ s1o1-admin ✓ ✗ ✗ s1o1-collaborator ✓ ✗ ✗ s1o1p1-admin ✓ ✗ ✗ @@ -36,39 +32,35 @@ s1o1p1-collaborator ✓ ✗ ✗ s1o1p1-viewer ✓ ✗ ✗ s2-admin ✗ ✓ ✓ s2-collaborator ✗ ✓ ✓ -s2-viewer ✗ ✗ ✗ +s2-viewer ✗ ✓ ✗ resource: Organization "s1o1" actions: Fetch, ListProjects, CreateProject, ModifyDescription - note: - USER RESULTS FOR EACH ACTION fleet-admin ✓ ✓ ✓ ✓ fleet-collaborator ✓ ✓ ✓ ✓ -fleet-viewer ✓ ✗ ✗ ✗ +fleet-viewer ✓ ✓ ✗ ✗ s1-admin ✓ ✓ ✓ ✓ s1-collaborator ✓ ✓ ✓ ✓ -s1-viewer ✓ ✗ ✗ ✗ +s1-viewer ✓ ✓ ✗ ✗ s1o1-admin ✓ ✓ ✓ ✓ s1o1-collaborator ✓ ✓ ✓ ✗ -s1o1p1-admin ✓ ✗ ✗ ✗ -s1o1p1-collaborator ✓ ✗ ✗ ✗ -s1o1p1-viewer ✓ ✗ ✗ ✗ +s1o1p1-admin ✗ ✗ ✗ ✗ +s1o1p1-collaborator ✗ ✗ ✗ ✗ +s1o1p1-viewer ✗ ✗ ✗ ✗ s2-admin ✗ ✗ ✗ ✗ s2-collaborator ✗ ✗ ✗ ✗ s2-viewer ✗ ✗ ✗ ✗ resource: Project "s1o1p1" actions: Fetch, ListVpcs, CreateVpc, ModifyDescription - note: - USER RESULTS FOR EACH ACTION fleet-admin ✓ ✓ ✓ ✓ fleet-collaborator ✓ ✓ ✓ ✓ -fleet-viewer ✗ ✗ ✗ ✗ +fleet-viewer ✓ ✓ ✗ ✗ s1-admin ✓ ✓ ✓ ✓ s1-collaborator ✓ ✓ ✓ ✓ -s1-viewer ✗ ✗ ✗ ✗ +s1-viewer ✓ ✓ ✗ ✗ s1o1-admin ✓ ✓ ✓ ✓ s1o1-collaborator ✓ ✓ ✓ ✓ s1o1p1-admin ✓ ✓ ✓ ✓ @@ -80,15 +72,13 @@ s2-viewer ✗ ✗ ✗ ✗ resource: Vpc "s1o1p1v1" actions: Fetch, ListSubnets, CreateSubnet, ModifyDescription - note: - USER RESULTS FOR EACH ACTION fleet-admin ✓ ✓ ✓ ✓ fleet-collaborator ✓ ✓ ✓ ✓ -fleet-viewer ✗ ✗ ✗ ✗ +fleet-viewer ✓ ✓ ✗ ✗ s1-admin ✓ ✓ ✓ ✓ s1-collaborator ✓ ✓ ✓ ✓ -s1-viewer ✗ ✗ ✗ ✗ +s1-viewer ✓ ✓ ✗ ✗ s1o1-admin ✓ ✓ ✓ ✓ s1o1-collaborator ✓ ✓ ✓ ✓ s1o1p1-admin ✓ ✓ ✓ ✓ @@ -100,15 +90,13 @@ s2-viewer ✗ ✗ ✗ ✗ resource: Project "s1o1p2" actions: Fetch, ListVpcs, CreateVpc, ModifyDescription - note: - USER RESULTS FOR EACH ACTION fleet-admin ✓ ✓ ✓ ✓ fleet-collaborator ✓ ✓ ✓ ✓ -fleet-viewer ✗ ✗ ✗ ✗ +fleet-viewer ✓ ✓ ✗ ✗ s1-admin ✓ ✓ ✓ ✓ s1-collaborator ✓ ✓ ✓ ✓ -s1-viewer ✗ ✗ ✗ ✗ +s1-viewer ✓ ✓ ✗ ✗ s1o1-admin ✓ ✓ ✓ ✓ s1o1-collaborator ✓ ✓ ✓ ✓ s1o1p1-admin ✗ ✗ ✗ ✗ @@ -120,15 +108,13 @@ s2-viewer ✗ ✗ ✗ ✗ resource: Vpc "s1o1p2v1" actions: Fetch, ListSubnets, CreateSubnet, ModifyDescription - note: - USER RESULTS FOR EACH ACTION fleet-admin ✓ ✓ ✓ ✓ fleet-collaborator ✓ ✓ ✓ ✓ -fleet-viewer ✗ ✗ ✗ ✗ +fleet-viewer ✓ ✓ ✗ ✗ s1-admin ✓ ✓ ✓ ✓ s1-collaborator ✓ ✓ ✓ ✓ -s1-viewer ✗ ✗ ✗ ✗ +s1-viewer ✓ ✓ ✗ ✗ s1o1-admin ✓ ✓ ✓ ✓ s1o1-collaborator ✓ ✓ ✓ ✓ s1o1p1-admin ✗ ✗ ✗ ✗ @@ -140,35 +126,31 @@ s2-viewer ✗ ✗ ✗ ✗ resource: Organization "s1o2" actions: Fetch, ListProjects, CreateProject, ModifyDescription - note: - USER RESULTS FOR EACH ACTION fleet-admin ✓ ✓ ✓ ✓ fleet-collaborator ✓ ✓ ✓ ✓ -fleet-viewer ✓ ✗ ✗ ✗ +fleet-viewer ✓ ✓ ✗ ✗ s1-admin ✓ ✓ ✓ ✓ s1-collaborator ✓ ✓ ✓ ✓ -s1-viewer ✓ ✗ ✗ ✗ -s1o1-admin ✓ ✗ ✗ ✗ -s1o1-collaborator ✓ ✗ ✗ ✗ -s1o1p1-admin ✓ ✗ ✗ ✗ -s1o1p1-collaborator ✓ ✗ ✗ ✗ -s1o1p1-viewer ✓ ✗ ✗ ✗ +s1-viewer ✓ ✓ ✗ ✗ +s1o1-admin ✗ ✗ ✗ ✗ +s1o1-collaborator ✗ ✗ ✗ ✗ +s1o1p1-admin ✗ ✗ ✗ ✗ +s1o1p1-collaborator ✗ ✗ ✗ ✗ +s1o1p1-viewer ✗ ✗ ✗ ✗ s2-admin ✗ ✗ ✗ ✗ s2-collaborator ✗ ✗ ✗ ✗ s2-viewer ✗ ✗ ✗ ✗ resource: Project "s1o2p1" actions: Fetch, ListVpcs, CreateVpc, ModifyDescription - note: - USER RESULTS FOR EACH ACTION fleet-admin ✓ ✓ ✓ ✓ fleet-collaborator ✓ ✓ ✓ ✓ -fleet-viewer ✗ ✗ ✗ ✗ +fleet-viewer ✓ ✓ ✗ ✗ s1-admin ✓ ✓ ✓ ✓ s1-collaborator ✓ ✓ ✓ ✓ -s1-viewer ✗ ✗ ✗ ✗ +s1-viewer ✓ ✓ ✗ ✗ s1o1-admin ✗ ✗ ✗ ✗ s1o1-collaborator ✗ ✗ ✗ ✗ s1o1p1-admin ✗ ✗ ✗ ✗ @@ -180,15 +162,13 @@ s2-viewer ✗ ✗ ✗ ✗ resource: Vpc "s1o2p1v1" actions: Fetch, ListSubnets, CreateSubnet, ModifyDescription - note: - USER RESULTS FOR EACH ACTION fleet-admin ✓ ✓ ✓ ✓ fleet-collaborator ✓ ✓ ✓ ✓ -fleet-viewer ✗ ✗ ✗ ✗ +fleet-viewer ✓ ✓ ✗ ✗ s1-admin ✓ ✓ ✓ ✓ s1-collaborator ✓ ✓ ✓ ✓ -s1-viewer ✗ ✗ ✗ ✗ +s1-viewer ✓ ✓ ✗ ✗ s1o1-admin ✗ ✗ ✗ ✗ s1o1-collaborator ✗ ✗ ✗ ✗ s1o1p1-admin ✗ ✗ ✗ ✗ @@ -200,15 +180,13 @@ s2-viewer ✗ ✗ ✗ ✗ resource: Silo "s2" actions: Fetch, ListOrganizations, CreateOrganization - note: All users can view their own Silo. Roles are required to list and create Organizations. Note that administrators on "s2" appear to be able to list and create Organizations in the output below, but those are Organizations in "s2". - USER RESULTS FOR EACH ACTION fleet-admin ✓ ✓ ✓ fleet-collaborator ✓ ✓ ✓ -fleet-viewer ✓ ✗ ✗ +fleet-viewer ✓ ✓ ✗ s1-admin ✗ ✓ ✓ s1-collaborator ✗ ✓ ✓ -s1-viewer ✗ ✗ ✗ +s1-viewer ✗ ✓ ✗ s1o1-admin ✗ ✗ ✗ s1o1-collaborator ✗ ✗ ✗ s1o1p1-admin ✗ ✗ ✗ @@ -216,12 +194,10 @@ s1o1p1-collaborator ✗ ✗ ✗ s1o1p1-viewer ✗ ✗ ✗ s2-admin ✓ ✓ ✓ s2-collaborator ✓ ✓ ✓ -s2-viewer ✓ ✗ ✗ +s2-viewer ✓ ✓ ✗ resource: Organization "s2o3" actions: Fetch, ListProjects, CreateProject, ModifyDescription - note: - USER RESULTS FOR EACH ACTION fleet-admin ✗ ✗ ✗ ✗ fleet-collaborator ✗ ✗ ✗ ✗ @@ -236,12 +212,10 @@ s1o1p1-collaborator ✗ ✗ ✗ ✗ s1o1p1-viewer ✗ ✗ ✗ ✗ s2-admin ✓ ✓ ✓ ✓ s2-collaborator ✓ ✓ ✓ ✓ -s2-viewer ✓ ✗ ✗ ✗ +s2-viewer ✓ ✓ ✗ ✗ resource: Project "s2o3p1" actions: Fetch, ListVpcs, CreateVpc, ModifyDescription - note: - USER RESULTS FOR EACH ACTION fleet-admin ✗ ✗ ✗ ✗ fleet-collaborator ✗ ✗ ✗ ✗ @@ -256,12 +230,10 @@ s1o1p1-collaborator ✗ ✗ ✗ ✗ s1o1p1-viewer ✗ ✗ ✗ ✗ s2-admin ✓ ✓ ✓ ✓ s2-collaborator ✓ ✓ ✓ ✓ -s2-viewer ✗ ✗ ✗ ✗ +s2-viewer ✓ ✓ ✗ ✗ resource: Vpc "s2o3p1v1" actions: Fetch, ListSubnets, CreateSubnet, ModifyDescription - note: - USER RESULTS FOR EACH ACTION fleet-admin ✗ ✗ ✗ ✗ fleet-collaborator ✗ ✗ ✗ ✗ @@ -276,5 +248,5 @@ s1o1p1-collaborator ✗ ✗ ✗ ✗ s1o1p1-viewer ✗ ✗ ✗ ✗ s2-admin ✓ ✓ ✓ ✓ s2-collaborator ✓ ✓ ✓ ✓ -s2-viewer ✗ ✗ ✗ ✗ +s2-viewer ✓ ✓ ✗ ✗ From d17fad952558426516121d23d0c63a14e71707b5 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 23 May 2022 16:13:19 -0700 Subject: [PATCH 10/11] update TODOs --- nexus/tests/integration_tests/authz_roles.rs | 49 ++++++++++---------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/nexus/tests/integration_tests/authz_roles.rs b/nexus/tests/integration_tests/authz_roles.rs index ba5f75522f2..0a877ba6e53 100644 --- a/nexus/tests/integration_tests/authz_roles.rs +++ b/nexus/tests/integration_tests/authz_roles.rs @@ -6,6 +6,30 @@ //! and a group of users with various roles on these resources and verifying //! that each role grants the privileges we expect. +// XXX-dap TODO +// - implement tests for DELETE. One idea: +// - every resource has a "recreate()" function that recreates it (including +// the role assignments) +// - this list is guaranteed to be topo-sorted +// - we explicitly delete every resource when we're done testing it +// +// Then we can test these in reverse order. By the time we get to any +// resource, its children should be gone. +// - figure out how to implement tests for fetching/modifying the policy. Maybe +// have a dummy user that we grant/remove privileges for? +// - do we want to look into performance at all? It takes a long time. +// - this test needs a lot of documentation +// - document limitations / figure out how to get better coverage. What this +// test really verifies is that the roles probably grant what we expect, and +// that the specific APIs tested check the privileges we expect. It doesn't +// test that any other APIs check the right privileges. That's a lot harder +// -- we basically need a whole coverage test version of this (which seems +// harder than the "unauthorized" test). Alternatively, we could modify the +// basic functionality tests to use minimum-privileged users, which would +// ensure that those minimum privileges are _sufficient_ to make the +// corresponding API calls. But that wouldn't test that _other_ privileges +// _don't_ grant those permissions. That seems a lot harder. + use anyhow::anyhow; use dropshot::test_util::ClientTestContext; use http::Method; @@ -156,23 +180,6 @@ impl Resource { client: &'a ClientTestContext, username: &str, ) -> Vec> { - // XXX-dap TODO examples: - // - list children (various kinds) - // - modify - // - fetch - // - delete? this one seems hard to test because it might succeed! - // - start/stop/halt -- hard to test because it's async - // - migrate? - // - disk attach/detach? - // This is getting a little nuts. In the limit, we kind of want to run - // every test as a user with every role we think should grant that test - // the privileges it needs (to make sure they grant enough). Then we - // kind of want to run every test again as every other role to make sure - // it fails at some point. Then we want to check coverage of the - // endpoints/role combinations. This seems really hard! - // TODO-coverage what other interesting cases aren't covered? - // - fetch policy (all resources) - // - update policy (how do we do this?!) match self.resource_type { ResourceType::Fleet => { let silos_url = "/silos"; @@ -689,14 +696,6 @@ lazy_static! { &*SILO1_ORG1_PROJ1, ]; - // XXX-dap One idea to support testing DELETE: - // - every resource has a "recreate()" function that recreates it (including - // the role assignments) - // - this list is guaranteed to be topo-sorted - // - we explicitly delete every resource when we're done testing it - // - // Then we can test these in reverse order. By the time we get to any - // resource, its children should be gone. static ref ALL_RESOURCES: Vec<&'static Resource> = vec![ &*FLEET, &*SILO1, From b8abf6b0e58b4a6a46911eb93d5601fe7b2b3bc4 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Tue, 24 May 2022 10:28:22 -0700 Subject: [PATCH 11/11] add FetchPolicy test (not tested) --- nexus/tests/integration_tests/authz_roles.rs | 40 +++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/nexus/tests/integration_tests/authz_roles.rs b/nexus/tests/integration_tests/authz_roles.rs index 0a877ba6e53..6e8e299ccd8 100644 --- a/nexus/tests/integration_tests/authz_roles.rs +++ b/nexus/tests/integration_tests/authz_roles.rs @@ -15,7 +15,7 @@ // // Then we can test these in reverse order. By the time we get to any // resource, its children should be gone. -// - figure out how to implement tests for fetching/modifying the policy. Maybe +// - figure out how to implement tests for modifying the policy. Maybe // have a dummy user that we grant/remove privileges for? // - do we want to look into performance at all? It takes a long time. // - this test needs a lot of documentation @@ -195,7 +195,7 @@ impl Resource { template: NexusRequest::new(RequestBuilder::new( client, Method::GET, - "/policy", + &self.policy_url() )), on_success: None, }, @@ -291,6 +291,15 @@ impl Resource { )), on_success: None, }, + TestOperation { + label: "FetchPolicy", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &self.policy_url() + )), + on_success: None, + }, TestOperation { label: "ListOrganizations", template: NexusRequest::new(RequestBuilder::new( @@ -339,6 +348,15 @@ impl Resource { )), on_success: None, }, + TestOperation { + label: "FetchPolicy", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &self.policy_url() + )), + on_success: None, + }, TestOperation { label: "ListProjects", template: NexusRequest::new(RequestBuilder::new( @@ -413,6 +431,15 @@ impl Resource { )), on_success: None, }, + TestOperation { + label: "FetchPolicy", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &self.policy_url() + )), + on_success: None, + }, TestOperation { label: "ListVpcs", template: NexusRequest::new(RequestBuilder::new( @@ -490,6 +517,15 @@ impl Resource { )), on_success: None, }, + TestOperation { + label: "FetchPolicy", + template: NexusRequest::new(RequestBuilder::new( + client, + Method::GET, + &self.policy_url() + )), + on_success: None, + }, TestOperation { label: "ListSubnets", template: NexusRequest::new(RequestBuilder::new(