improve authz

nexus/src/app/sagas/instance_create.rs

@@ -628,7 +628,7 @@ async fn sic_allocate_instance_snat_ip(
     let ip_id = sagactx.lookup::<Uuid>("snat_ip_id")?;
 
     let (.., pool) = datastore
-        .ip_pools_lookup_by_name_no_auth(&opctx, "default")
+        .ip_pools_fetch_default_for(&opctx, authz::Action::CreateChild)
         .await
         .map_err(ActionError::action_failed)?;
     let pool_id = pool.identity.id;

nexus/src/db/datastore/external_ip.rs

@@ -5,6 +5,7 @@
 //! [`DataStore`] methods on [`ExternalIp`]s.
 
 use super::DataStore;
+use crate::authz;
 use crate::context::OpContext;
 use crate::db;
 use crate::db::error::public_error_from_diesel_pool;
@@ -23,6 +24,8 @@ use nexus_types::identity::Resource;
 use omicron_common::api::external::CreateResult;
 use omicron_common::api::external::Error;
 use omicron_common::api::external::LookupResult;
+use omicron_common::api::external::Name as ExternalName;
+use std::str::FromStr;
 use uuid::Uuid;
 
 impl DataStore {
@@ -50,22 +53,12 @@ impl DataStore {
         instance_id: Uuid,
         pool_name: Option<Name>,
     ) -> CreateResult<ExternalIp> {
-        let name = if let Some(name) = pool_name {
-            name.as_str().to_string()
-        } else {
-            "default".to_string()
-        };
-
-        // We'd like to add authz checks here, and use the `LookupPath`
-        // methods on the project-scoped view of this resource. It's not
-        // entirely clear how that'll work in the API, so see RFD 288 and
-        // https://github.com/oxidecomputer/omicron/issues/1470 for more
-        // details.
-        //
-        // For now, we just ensure that the pool is either unreserved, or
-        // reserved for the instance's project.
-        let (.., pool) =
-            self.ip_pools_lookup_by_name_no_auth(opctx, &name).await?;
+        let name = pool_name.unwrap_or_else(|| {
+            Name(ExternalName::from_str("default").unwrap())
+        });
+        let (.., pool) = self
+            .ip_pools_fetch_for(opctx, authz::Action::CreateChild, &name)
+            .await?;
         let pool_id = pool.identity.id;
 
         let data =

nexus/src/db/datastore/ip_pool.rs

@@ -6,6 +6,7 @@
 
 use super::DataStore;
 use crate::authz;
+use crate::authz::ApiResource;
 use crate::context::OpContext;
 use crate::db;
 use crate::db::collection_insert::AsyncInsertError;
@@ -14,6 +15,7 @@ use crate::db::error::diesel_pool_result_optional;
 use crate::db::error::public_error_from_diesel_pool;
 use crate::db::error::ErrorHandler;
 use crate::db::identity::Resource;
+use crate::db::lookup::LookupPath;
 use crate::db::model::IpPool;
 use crate::db::model::IpPoolRange;
 use crate::db::model::IpPoolUpdate;
@@ -33,8 +35,10 @@ use omicron_common::api::external::Error;
 use omicron_common::api::external::ListResultVec;
 use omicron_common::api::external::LookupResult;
 use omicron_common::api::external::LookupType;
+use omicron_common::api::external::Name as ExternalName;
 use omicron_common::api::external::ResourceType;
 use omicron_common::api::external::UpdateResult;
+use std::str::FromStr;
 use uuid::Uuid;
 
 impl DataStore {
@@ -76,32 +80,35 @@ impl DataStore {
             .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server))
     }
 
+    /// Looks up the default IP pool by name.
+    pub(crate) async fn ip_pools_fetch_default_for(
+        &self,
+        opctx: &OpContext,
+        action: authz::Action,
+    ) -> LookupResult<(authz::IpPool, IpPool)> {
+        self.ip_pools_fetch_for(
+            opctx,
+            action,
+            &Name(ExternalName::from_str("default").unwrap()),
+        )
+        .await
+    }
+
     /// Looks up an IP pool by name.
-    pub(crate) async fn ip_pools_lookup_by_name_no_auth(
+    pub(crate) async fn ip_pools_fetch_for(
         &self,
         opctx: &OpContext,
-        name: &str,
+        action: authz::Action,
+        name: &Name,
     ) -> LookupResult<(authz::IpPool, IpPool)> {
-        use db::schema::ip_pool::dsl;
+        let (.., authz_pool, pool) = LookupPath::new(opctx, &self)
+            .ip_pool_name(&name)
+            .fetch_for(action)
+            .await?;
+        if pool.internal {
+            return Err(authz_pool.not_found());
+        }
 
-        let (authz_pool, pool) = dsl::ip_pool
-            .filter(dsl::internal.eq(false))
-            .filter(dsl::time_deleted.is_null())
-            .filter(dsl::name.eq(name.to_string()))
-            .select(IpPool::as_select())
-            .get_result_async(self.pool_authorized(opctx).await?)
-            .await
-            .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server))
-            .map(|ip_pool| {
-                (
-                    authz::IpPool::new(
-                        authz::FLEET,
-                        ip_pool.id(),
-                        LookupType::ByName(name.to_string()),
-                    ),
-                    ip_pool,
-                )
-            })?;
         Ok((authz_pool, pool))
     }
 

nexus/src/db/queries/external_ip.rs

@@ -673,7 +673,10 @@ mod tests {
         async fn default_pool_id(&self) -> Uuid {
             let (.., pool) = self
                 .db_datastore
-                .ip_pools_lookup_by_name_no_auth(&self.opctx, "default")
+                .ip_pools_fetch_default_for(
+                    &self.opctx,
+                    crate::authz::Action::ListChildren,
+                )
                 .await
                 .expect("Failed to lookup default ip pool");
             pool.identity.id