diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 3e1d553f99..23c98529fa 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/src/app/project.rs b/nexus/src/app/project.rs index e63a14ecf1..e8b50de9ee 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 9de39b4522..39388c179c 100644 --- a/nexus/src/authz/omicron.polar +++ b/nexus/src/authz/omicron.polar @@ -137,23 +137,41 @@ resource Silo { ]; roles = [ "admin", "collaborator", "viewer" ]; + # permissions granted by this resource's roles "list_children" if "viewer"; "read" if "viewer"; + "create_child" if "collaborator"; + "modify" if "admin"; + # roles implied by other roles "viewer" if "collaborator"; - "create_child" if "collaborator"; "collaborator" if "admin"; - "modify" if "admin"; + + # roles implied by relationships with the parent fleet 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) 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 { @@ -180,7 +198,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 +232,9 @@ resource Project { "modify" if "admin"; relations = { parent_organization: Organization }; - "admin" if "admin" on "parent_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/src/db/datastore.rs b/nexus/src/db/datastore.rs index 02f53ad2f0..c80d3c81e8 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/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index 952e2a454a..ff562631d4 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -18,6 +18,8 @@ use omicron_common::api::external::Instance; use omicron_common::api::external::InstanceCpuCount; use omicron_nexus::crucible_agent_client::types::State as RegionState; use omicron_nexus::external_api::params; +use omicron_nexus::external_api::shared; +use omicron_nexus::external_api::shared::IdentityType; use omicron_nexus::external_api::views::{ Organization, Project, Silo, Vpc, VpcRouter, }; @@ -280,6 +282,51 @@ pub async fn create_router( .unwrap() } +/// Grant a role on a resource to a user +/// +/// * `grant_resource_url`: URL of the resource we're granting the role on +/// * `grant_role`: the role we're granting +/// * `grant_user`: the uuid of the user we're granting the role to +/// * `run_as`: the user _doing_ the granting +pub async fn grant_iam( + client: &ClientTestContext, + grant_resource_url: &str, + grant_role: T, + grant_user: Uuid, + run_as: AuthnMode, +) where + T: serde::Serialize + serde::de::DeserializeOwned, +{ + let policy_url = format!("{}/policy", grant_resource_url); + let existing_policy: shared::Policy = + NexusRequest::object_get(client, &policy_url) + .authn_as(run_as.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: grant_user, + role_name: grant_role, + }; + 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 }; + + // TODO-correctness use etag when we have it + NexusRequest::object_put(client, &policy_url, Some(&new_policy)) + .authn_as(run_as) + .execute() + .await + .expect("failed to update policy"); +} + pub async fn project_get( client: &ClientTestContext, project_url: &str, diff --git a/nexus/tests/integration_tests/silos.rs b/nexus/tests/integration_tests/silos.rs index bad2be1294..99a6cee8d5 100644 --- a/nexus/tests/integration_tests/silos.rs +++ b/nexus/tests/integration_tests/silos.rs @@ -3,17 +3,20 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use nexus_test_utils::http_testing::{AuthnMode, NexusRequest}; -use omicron_nexus::external_api::views::{Organization, Silo}; +use omicron_common::api::external::{IdentityMetadataCreateParams, Name}; +use omicron_nexus::external_api::views::{self, Organization, Silo}; use omicron_nexus::TestInterfaces as _; use http::method::Method; use http::StatusCode; use nexus_test_utils::resource_helpers::{ - create_organization, create_silo, objects_list_page_authz, + create_organization, create_silo, grant_iam, objects_list_page_authz, }; use nexus_test_utils::ControlPlaneTestContext; use nexus_test_utils_macros::nexus_test; +use omicron_nexus::authz::SiloRoles; +use omicron_nexus::external_api::params; #[nexus_test] async fn test_silos(cptestctx: &ControlPlaneTestContext) { @@ -74,6 +77,16 @@ async fn test_silos(cptestctx: &ControlPlaneTestContext) { .await .unwrap(); + // Grant the user "admin" privileges on that Silo. + grant_iam( + client, + "/silos/discoverable", + SiloRoles::Admin, + new_silo_user_id, + AuthnMode::PrivilegedUser, + ) + .await; + // TODO-coverage, TODO-security: Add test for Silo-local session // when we can use users in another Silo. @@ -81,7 +94,45 @@ async fn test_silos(cptestctx: &ControlPlaneTestContext) { // Create organization with built-in user auth // Note: this currently goes to the built-in silo! - create_organization(&client, "someorg").await; + let org_name: Name = "someorg".parse().unwrap(); + let new_org_in_default_silo = + create_organization(&client, org_name.as_str()).await; + + // Create an Organization of the same name in a different Silo to verify + // that's possible. + let new_org_in_our_silo = NexusRequest::objects_post( + client, + "/organizations", + ¶ms::OrganizationCreate { + identity: IdentityMetadataCreateParams { + name: org_name.clone(), + description: String::new(), + }, + }, + ) + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to create same-named Organization in a different Silo") + .parsed_body::() + .expect("failed to parse new Organization"); + assert_eq!( + new_org_in_default_silo.identity.name, + new_org_in_our_silo.identity.name + ); + assert_ne!( + new_org_in_default_silo.identity.id, + new_org_in_our_silo.identity.id + ); + // Delete it so that we can delete the Silo later. + NexusRequest::object_delete( + client, + &format!("/organizations/{}", org_name), + ) + .authn_as(AuthnMode::SiloUser(new_silo_user_id)) + .execute() + .await + .expect("failed to delete test Organization"); // Verify GET /organizations works with built-in user auth let organizations =