Fix internet gateway IP pool attach for non-fleet users (#6820)

Closes #6813.

nexus/src/app/internet_gateway.rs

@@ -197,23 +197,14 @@ impl super::Nexus {
         let (.., authz_igw, _) =
             lookup.fetch_for(authz::Action::CreateChild).await?;
 
-        let ip_pool_id = match params.ip_pool {
-            NameOrId::Id(id) => id,
-            NameOrId::Name(ref name) => {
-                let name = name.clone().into();
-                LookupPath::new(opctx, &self.db_datastore)
-                    .ip_pool_name(&name)
-                    .fetch()
-                    .await?
-                    .0
-                    .id()
-            }
-        };
+        // need to use this method so it works for non-fleet users
+        let (authz_pool, ..) =
+            self.silo_ip_pool_fetch(&opctx, &params.ip_pool).await?;
 
         let id = Uuid::new_v4();
         let route = db::model::InternetGatewayIpPool::new(
             id,
-            ip_pool_id,
+            authz_pool.id(),
             authz_igw.id(),
             params.identity.clone(),
         );

nexus/src/app/ip_pool.rs

@@ -92,13 +92,18 @@ impl super::Nexus {
         self.db_datastore.silo_ip_pool_list(opctx, &authz_silo, pagparams).await
     }
 
-    // Look up pool by name or ID, but only return it if it's linked to the
-    // current silo
+    /// Look up linked pool by name or ID. 404 on pools that exist but aren't
+    /// linked to the current silo. Special logic to make sure non-fleet users
+    /// can read the pool.
     pub async fn silo_ip_pool_fetch<'a>(
         &'a self,
         opctx: &'a OpContext,
         pool: &'a NameOrId,
-    ) -> LookupResult<(db::model::IpPool, db::model::IpPoolResource)> {
+    ) -> LookupResult<(
+        authz::IpPool,
+        db::model::IpPool,
+        db::model::IpPoolResource,
+    )> {
         let (authz_pool, pool) = self
             .ip_pool_lookup(opctx, pool)?
             // TODO-robustness: https://github.com/oxidecomputer/omicron/issues/3995
@@ -117,7 +122,7 @@ impl super::Nexus {
         // 404 if no link is found in the current silo
         let link = self.db_datastore.ip_pool_fetch_link(opctx, pool.id()).await;
         match link {
-            Ok(link) => Ok((pool, link)),
+            Ok(link) => Ok((authz_pool, pool, link)),
             Err(_) => Err(authz_pool.not_found()),
         }
     }

nexus/src/external_api/http_entrypoints.rs

@@ -1044,7 +1044,7 @@ impl NexusExternalApi for NexusExternalApiImpl {
                 crate::context::op_context_for_external_api(&rqctx).await?;
             let nexus = &apictx.context.nexus;
             let pool_selector = path_params.into_inner().pool;
-            let (pool, silo_link) =
+            let (.., pool, silo_link) =
                 nexus.silo_ip_pool_fetch(&opctx, &pool_selector).await?;
             Ok(HttpResponseOk(views::SiloIpPool {
                 identity: pool.identity(),

nexus/tests/integration_tests/internet_gateway.rs

@@ -2,25 +2,26 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
-use dropshot::test_util::ClientTestContext;
+use dropshot::{test_util::ClientTestContext, ResultsPage};
 use http::{Method, StatusCode};
 use nexus_db_queries::db::fixed_data::silo::DEFAULT_SILO;
 use nexus_test_utils::{
     http_testing::{AuthnMode, NexusRequest},
     resource_helpers::{
         attach_ip_address_to_igw, attach_ip_pool_to_igw, create_floating_ip,
         create_instance_with, create_internet_gateway, create_ip_pool,
-        create_project, create_route, create_router, create_vpc,
-        delete_internet_gateway, detach_ip_address_from_igw,
+        create_local_user, create_project, create_route, create_router,
+        create_vpc, delete_internet_gateway, detach_ip_address_from_igw,
         detach_ip_pool_from_igw, link_ip_pool, objects_list_page_authz,
     },
 };
 use nexus_test_utils_macros::nexus_test;
 use nexus_types::external_api::{
     params::{
-        ExternalIpCreate, InstanceNetworkInterfaceAttachment,
+        self, ExternalIpCreate, InstanceNetworkInterfaceAttachment,
         InstanceNetworkInterfaceCreate,
     },
+    shared::SiloRole,
     views::{InternetGateway, InternetGatewayIpAddress, InternetGatewayIpPool},
 };
 use nexus_types::identity::Resource;
@@ -256,6 +257,90 @@ async fn test_internet_gateway_delete_cascade(ctx: &ControlPlaneTestContext) {
     expect_igw_addresses_not_found(c, PROJECT_NAME, VPC_NAME, IGW_NAME).await;
 }
 
+#[nexus_test]
+async fn test_igw_ip_pool_attach_silo_user(ctx: &ControlPlaneTestContext) {
+    let c = &ctx.external_client;
+    test_setup(c).await;
+
+    // Create a non-admin user
+    let silo_url = format!("/v1/system/silos/{}", DEFAULT_SILO.name());
+    let silo: nexus_types::external_api::views::Silo =
+        nexus_test_utils::resource_helpers::object_get(c, &silo_url).await;
+
+    let user = create_local_user(
+        c,
+        &silo,
+        &"user".parse().unwrap(),
+        params::UserPassword::LoginDisallowed,
+    )
+    .await;
+
+    // Grant the user Collaborator role
+    nexus_test_utils::resource_helpers::grant_iam(
+        c,
+        &silo_url,
+        SiloRole::Collaborator,
+        user.id,
+        AuthnMode::PrivilegedUser,
+    )
+    .await;
+
+    // Create an internet gateway
+    create_internet_gateway(c, PROJECT_NAME, VPC_NAME, IGW_NAME).await;
+
+    // Verify the IP pool is not attached
+    let igw_pools =
+        list_internet_gateway_ip_pools(c, PROJECT_NAME, VPC_NAME, IGW_NAME)
+            .await;
+    assert_eq!(igw_pools.len(), 0, "IP pool should not be attached");
+
+    // Attach an IP pool as non-admin user
+    let url = format!(
+        "/v1/internet-gateway-ip-pools?project={}&vpc={}&gateway={}",
+        PROJECT_NAME, VPC_NAME, IGW_NAME
+    );
+    let params = params::InternetGatewayIpPoolCreate {
+        identity: IdentityMetadataCreateParams {
+            name: IP_POOL_ATTACHMENT_NAME.parse().unwrap(),
+            description: "Test attachment".to_string(),
+        },
+        ip_pool: NameOrId::Name(IP_POOL_NAME.parse().unwrap()),
+    };
+
+    let _result: InternetGatewayIpPool =
+        NexusRequest::objects_post(&c, &url, &params)
+            .authn_as(AuthnMode::SiloUser(user.id))
+            .execute_and_parse_unwrap()
+            .await;
+
+    // Verify the non-admin user can list the attached IP pool
+    let igw_pools: ResultsPage<InternetGatewayIpPool> =
+        NexusRequest::object_get(c, &url)
+            .authn_as(AuthnMode::SiloUser(user.id))
+            .execute_and_parse_unwrap()
+            .await;
+
+    assert_eq!(igw_pools.items.len(), 1);
+    assert_eq!(igw_pools.items[0].identity.name, IP_POOL_ATTACHMENT_NAME);
+
+    // detach doesn't have the authz complication that attach has, but test it anyway
+    let url = format!(
+        "/v1/internet-gateway-ip-pools/{}?project={}&vpc={}&gateway={}&cascade=true",
+        IP_POOL_ATTACHMENT_NAME, PROJECT_NAME, VPC_NAME, IGW_NAME,
+    );
+    NexusRequest::object_delete(&c, &url)
+        .authn_as(AuthnMode::SiloUser(user.id))
+        .execute()
+        .await
+        .unwrap();
+
+    // it's gone
+    let igw_pools =
+        list_internet_gateway_ip_pools(c, PROJECT_NAME, VPC_NAME, IGW_NAME)
+            .await;
+    assert_eq!(igw_pools.len(), 0, "IP pool should not be attached");
+}
+
 async fn test_setup(c: &ClientTestContext) {
     // create a project and vpc to test with
     let _proj = create_project(&c, PROJECT_NAME).await;