Fleet privileges should not cascade into Silos

nexus/src/app/silo.rs

@@ -31,7 +31,14 @@ impl super::Nexus {
         opctx: &OpContext,
         new_silo_params: params::SiloCreate,
     ) -> CreateResult<db::model::Silo> {
-        self.datastore().silo_create(&opctx, new_silo_params).await
+        // Silo group creation happens as Nexus's "external authn" context,
+        // not the user's context here.  The user may not have permission to
+        // create arbitrary groups in the Silo, but we allow them to create
+        // this one in this case.
+        let external_authn_opctx = self.opctx_external_authn();
+        self.datastore()
+            .silo_create(&opctx, &external_authn_opctx, new_silo_params)
+            .await
     }
 
     pub async fn silos_list_by_name(

nexus/src/authz/omicron.polar

@@ -61,7 +61,7 @@ has_role(actor: AuthenticatedActor, role: String, resource: Resource)
 #
 # - fleet.admin           (superuser for the whole system)
 # - fleet.collaborator    (can manage Silos)
-# - fleet.viewer          (can read most resources in the system)
+# - fleet.viewer          (can read most non-siloed resources in the system)
 # - silo.admin            (superuser for the silo)
 # - silo.collaborator     (can create and own Organizations)
 # - silo.viewer           (can read most resources within the Silo)
@@ -132,10 +132,16 @@ resource Silo {
 	"create_child" if "collaborator";
 	"modify" if "admin";
 
-	# Roles implied by roles on this resource's parent (Fleet)
+	# Permissions implied by roles on this resource's parent (Fleet).  Fleet
+	# privileges allow a user to see and potentially administer the Silo,
+	# but they do not give anyone permission to look at anything inside the
+	# Silo.  To achieve this, we use permission rules here.  (If we granted
+	# Fleet administrators _roles_ on the Silo, then those would cascade
+	# into the Silo as well.)
 	relations = { parent_fleet: Fleet };
-	"admin" if "collaborator" on "parent_fleet";
-	"viewer" if "viewer" on "parent_fleet";
+	"read" if "viewer" on "parent_fleet";
+	"list_identity_providers" if "viewer" on "parent_fleet";
+	"modify" if "collaborator" on "parent_fleet";
 
 	# external authenticator has to create silo users
 	"list_children" if "external-authenticator" on "parent_fleet";

nexus/src/authz/policy_test/mod.rs

@@ -14,14 +14,19 @@ mod coverage;
 mod resource_builder;
 mod resources;
 
+use super::SiloRole;
 use crate::authn;
 use crate::authz;
 use crate::context::OpContext;
 use crate::db;
+use authn::USER_TEST_PRIVILEGED;
 use coverage::Coverage;
 use futures::StreamExt;
 use nexus_test_utils::db::test_setup_database;
+use nexus_types::external_api::shared;
+use nexus_types::identity::Asset;
 use omicron_common::api::external::Error;
+use omicron_common::api::external::LookupType;
 use omicron_test_utils::dev;
 use resource_builder::DynAuthorizedResource;
 use resource_builder::ResourceBuilder;
@@ -55,13 +60,34 @@ async fn test_iam_roles_behavior() {
     let mut db = test_setup_database(&logctx.log).await;
     let (opctx, datastore) = db::datastore::datastore_test(&logctx, &db).await;
 
+    // Before we can create the resources, users, and role assignments that we
+    // need, we must grant the "test-privileged" user privileges to fetch and
+    // modify policies inside the "main" Silo (the one we create users in).
+    let main_silo_id = Uuid::new_v4();
+    let main_silo = authz::Silo::new(
+        authz::FLEET,
+        main_silo_id,
+        LookupType::ById(main_silo_id),
+    );
+    datastore
+        .role_assignment_replace_visible(
+            &opctx,
+            &main_silo,
+            &[shared::RoleAssignment {
+                identity_type: shared::IdentityType::SiloUser,
+                identity_id: USER_TEST_PRIVILEGED.id(),
+                role_name: SiloRole::Admin,
+            }],
+        )
+        .await
+        .unwrap();
+
     // Assemble the list of resources that we'll use for testing.  As we create
     // these resources, create the users and role assignments needed for the
     // exhaustive test.  `Coverage` is used to help verify that all resources
     // are tested or explicitly opted out.
     let exemptions = resources::exempted_authz_classes();
     let mut coverage = Coverage::new(&logctx.log, exemptions);
-    let main_silo_id = Uuid::new_v4();
     let builder =
         ResourceBuilder::new(&opctx, &datastore, &mut coverage, main_silo_id);
     let test_resources = resources::make_resources(builder, main_silo_id).await;

nexus/src/authz/policy_test/resources.rs

@@ -98,28 +98,23 @@ async fn make_silo(
     silo_id: Uuid,
     first_branch: bool,
 ) {
-    let silo1 = authz::Silo::new(
+    let silo = authz::Silo::new(
         authz::FLEET,
         silo_id,
         LookupType::ByName(silo_name.to_string()),
     );
     if first_branch {
-        builder.new_resource_with_users(silo1.clone()).await;
+        builder.new_resource_with_users(silo.clone()).await;
     } else {
-        builder.new_resource(silo1.clone());
+        builder.new_resource(silo.clone());
     }
 
     let norganizations = if first_branch { 2 } else { 1 };
     for i in 0..norganizations {
         let organization_name = format!("{}-org{}", silo_name, i + 1);
         let org_first_branch = first_branch && i == 0;
-        make_organization(
-            builder,
-            &silo1,
-            &organization_name,
-            org_first_branch,
-        )
-        .await;
+        make_organization(builder, &silo, &organization_name, org_first_branch)
+            .await;
     }
 }
 

nexus/src/db/datastore/silo.rs

@@ -71,6 +71,7 @@ impl DataStore {
     pub async fn silo_create(
         &self,
         opctx: &OpContext,
+        group_opctx: &OpContext,
         new_silo_params: params::SiloCreate,
     ) -> CreateResult<Silo> {
         let silo_id = Uuid::new_v4();
@@ -90,7 +91,7 @@ impl DataStore {
         {
             let silo_admin_group_ensure_query =
                 DataStore::silo_group_ensure_query(
-                    opctx,
+                    &group_opctx,
                     &authz_silo,
                     db::model::SiloGroup::new(
                         silo_group_id,

nexus/src/db/fixed_data/role_assignment.rs

@@ -14,7 +14,7 @@ lazy_static! {
     pub static ref BUILTIN_ROLE_ASSIGNMENTS: Vec<RoleAssignment> =
         vec![
             // The "internal-api" user gets the "admin" role on the sole Fleet.
-            // This will grant them (nearly) all permissions on all resources.
+            // This is a pretty elevated privilege.
             // TODO-security We should scope this down (or, really, figure out a
             // better internal authn/authz story).
             RoleAssignment::new(
@@ -24,13 +24,6 @@ lazy_static! {
                 *FLEET_ID,
                 role_builtin::FLEET_ADMIN.role_name,
             ),
-            RoleAssignment::new(
-                IdentityType::UserBuiltin,
-                user_builtin::USER_SERVICE_BALANCER.id,
-                role_builtin::FLEET_ADMIN.resource_type,
-                *FLEET_ID,
-                role_builtin::FLEET_ADMIN.role_name,
-            ),
 
             // The "internal-read" user gets the "viewer" role on the sole
             // Fleet.  This will grant them the ability to read various control

nexus/src/db/fixed_data/role_builtin.rs

@@ -41,6 +41,11 @@ lazy_static! {
             role_name: "collaborator",
             description: "Organization Collaborator",
         };
+    pub static ref SILO_ADMIN: RoleBuiltinConfig = RoleBuiltinConfig {
+        resource_type: api::external::ResourceType::Silo,
+        role_name: "admin",
+        description: "Silo Administrator",
+    };
     pub static ref BUILTIN_ROLES: Vec<RoleBuiltinConfig> = vec![
         FLEET_ADMIN.clone(),
         FLEET_AUTHENTICATOR.clone(),
@@ -50,11 +55,7 @@ lazy_static! {
             role_name: "collaborator",
             description: "Fleet Collaborator",
         },
-        RoleBuiltinConfig {
-            resource_type: api::external::ResourceType::Silo,
-            role_name: "admin",
-            description: "Silo Administrator",
-        },
+        SILO_ADMIN.clone(),
         RoleBuiltinConfig {
             resource_type: api::external::ResourceType::Silo,
             role_name: "collaborator",

nexus/src/db/fixed_data/silo_user.rs

@@ -26,14 +26,22 @@ lazy_static! {
     pub static ref ROLE_ASSIGNMENTS_PRIVILEGED:
         Vec<db::model::RoleAssignment> = vec![
             // The "test-privileged" user gets the "admin" role on the sole
-            // Fleet.  This will grant them all permissions on all resources.
+            // Fleet as well as the default Silo.
             db::model::RoleAssignment::new(
                 db::model::IdentityType::SiloUser,
                 USER_TEST_PRIVILEGED.id(),
                 role_builtin::FLEET_ADMIN.resource_type,
                 *db::fixed_data::FLEET_ID,
                 role_builtin::FLEET_ADMIN.role_name,
             ),
+
+            db::model::RoleAssignment::new(
+                db::model::IdentityType::SiloUser,
+                USER_TEST_PRIVILEGED.id(),
+                role_builtin::SILO_ADMIN.resource_type,
+                *db::fixed_data::silo::SILO_ID,
+                role_builtin::SILO_ADMIN.role_name,
+            ),
         ];
 
     /// Test user that's granted no privileges, used for automated testing

nexus/tests/integration_tests/saml.rs

@@ -14,12 +14,17 @@ use omicron_nexus::TestInterfaces;
 
 use http::method::Method;
 use http::StatusCode;
-use nexus_test_utils::resource_helpers::{create_silo, object_create};
+use nexus_test_utils::resource_helpers::{
+    create_silo, grant_iam, object_create,
+};
 
 use nexus_test_utils::ControlPlaneTestContext;
 use nexus_test_utils_macros::nexus_test;
 
 use httptest::{matchers::*, responders::*, Expectation, Server};
+use omicron_nexus::authn::USER_TEST_PRIVILEGED;
+use omicron_nexus::authz::SiloRole;
+use omicron_nexus::db::identity::Asset;
 
 // Valid SAML IdP entity descriptor from https://en.wikipedia.org/wiki/SAML_metadata#Identity_provider_metadata
 // note: no signing keys
@@ -34,6 +39,14 @@ async fn test_create_a_saml_idp(cptestctx: &ControlPlaneTestContext) {
     const SILO_NAME: &str = "saml-silo";
     create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Fixed)
         .await;
+    grant_iam(
+        &client,
+        &format!("/silos/{}", SILO_NAME),
+        SiloRole::Admin,
+        USER_TEST_PRIVILEGED.id(),
+        AuthnMode::PrivilegedUser,
+    )
+    .await;
 
     let silo: Silo =
         NexusRequest::object_get(&client, &format!("/silos/{}", SILO_NAME,))
@@ -152,6 +165,14 @@ async fn test_create_a_saml_idp_invalid_descriptor_truncated(
     const SILO_NAME: &str = "saml-silo";
     create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Fixed)
         .await;
+    grant_iam(
+        &client,
+        &format!("/silos/{}", SILO_NAME),
+        SiloRole::Admin,
+        USER_TEST_PRIVILEGED.id(),
+        AuthnMode::PrivilegedUser,
+    )
+    .await;
 
     let saml_idp_descriptor = {
         let mut saml_idp_descriptor = SAML_IDP_DESCRIPTOR.to_string();
@@ -212,6 +233,14 @@ async fn test_create_a_saml_idp_invalid_descriptor_no_redirect_binding(
     const SILO_NAME: &str = "saml-silo";
     create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Fixed)
         .await;
+    grant_iam(
+        &client,
+        &format!("/silos/{}", SILO_NAME),
+        SiloRole::Admin,
+        USER_TEST_PRIVILEGED.id(),
+        AuthnMode::PrivilegedUser,
+    )
+    .await;
 
     let saml_idp_descriptor = {
         let saml_idp_descriptor = SAML_IDP_DESCRIPTOR.to_string();
@@ -282,6 +311,14 @@ async fn test_create_a_hidden_silo_saml_idp(
 
     create_silo(&client, "hidden", false, shared::UserProvisionType::Fixed)
         .await;
+    grant_iam(
+        &client,
+        "/silos/hidden",
+        SiloRole::Admin,
+        USER_TEST_PRIVILEGED.id(),
+        AuthnMode::PrivilegedUser,
+    )
+    .await;
 
     // Valid IdP descriptor
     let saml_idp_descriptor = SAML_IDP_DESCRIPTOR.to_string();
@@ -351,6 +388,14 @@ async fn test_saml_idp_metadata_url_404(cptestctx: &ControlPlaneTestContext) {
     const SILO_NAME: &str = "saml-silo";
     create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Fixed)
         .await;
+    grant_iam(
+        &client,
+        &format!("/silos/{}", SILO_NAME),
+        SiloRole::Admin,
+        USER_TEST_PRIVILEGED.id(),
+        AuthnMode::PrivilegedUser,
+    )
+    .await;
 
     let server = Server::run();
     server.expect(
@@ -405,6 +450,14 @@ async fn test_saml_idp_metadata_url_invalid(
     const SILO_NAME: &str = "saml-silo";
     create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Fixed)
         .await;
+    grant_iam(
+        &client,
+        &format!("/silos/{}", SILO_NAME),
+        SiloRole::Admin,
+        USER_TEST_PRIVILEGED.id(),
+        AuthnMode::PrivilegedUser,
+    )
+    .await;
 
     NexusRequest::new(
         RequestBuilder::new(
@@ -561,6 +614,14 @@ async fn test_saml_idp_rsa_keypair_ok(cptestctx: &ControlPlaneTestContext) {
     const SILO_NAME: &str = "saml-silo";
     create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Fixed)
         .await;
+    grant_iam(
+        &client,
+        &format!("/silos/{}", SILO_NAME),
+        SiloRole::Admin,
+        USER_TEST_PRIVILEGED.id(),
+        AuthnMode::PrivilegedUser,
+    )
+    .await;
 
     NexusRequest::new(
         RequestBuilder::new(
@@ -932,6 +993,14 @@ async fn test_post_saml_response(cptestctx: &ControlPlaneTestContext) {
 
     const SILO_NAME: &str = "saml-silo";
     create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Jit).await;
+    grant_iam(
+        &client,
+        &format!("/silos/{}", SILO_NAME),
+        SiloRole::Admin,
+        USER_TEST_PRIVILEGED.id(),
+        AuthnMode::PrivilegedUser,
+    )
+    .await;
 
     let _silo_saml_idp: views::SamlIdentityProvider = object_create(
         client,
@@ -1025,6 +1094,14 @@ async fn test_post_saml_response_with_relay_state(
 
     const SILO_NAME: &str = "saml-silo";
     create_silo(&client, SILO_NAME, true, shared::UserProvisionType::Jit).await;
+    grant_iam(
+        &client,
+        &format!("/silos/{}", SILO_NAME),
+        SiloRole::Admin,
+        USER_TEST_PRIVILEGED.id(),
+        AuthnMode::PrivilegedUser,
+    )
+    .await;
 
     let _silo_saml_idp: views::SamlIdentityProvider = object_create(
         client,