authz: check project list endpoints (#617)

Author: davepacheco

nexus/src/db/datastore.rs

@@ -607,45 +607,40 @@ impl DataStore {
 
     pub async fn projects_list_by_id(
         &self,
-        organization_id: &Uuid,
+        opctx: &OpContext,
+        authz_org: &authz::Organization,
         pagparams: &DataPageParams<'_, Uuid>,
     ) -> ListResultVec<Project> {
         use db::schema::project::dsl;
+
+        opctx.authorize(authz::Action::ListChildren, authz_org).await?;
+
         paginated(dsl::project, dsl::id, pagparams)
-            .filter(dsl::organization_id.eq(*organization_id))
+            .filter(dsl::organization_id.eq(authz_org.id()))
             .filter(dsl::time_deleted.is_null())
             .select(Project::as_select())
-            .load_async(self.pool())
+            .load_async(self.pool_authorized(opctx).await?)
             .await
-            .map_err(|e| {
-                public_error_from_diesel_pool(
-                    e,
-                    ResourceType::Project,
-                    LookupType::Other("Listing All".to_string()),
-                )
-            })
+            .map_err(public_error_from_diesel_pool_shouldnt_fail)
     }
 
     pub async fn projects_list_by_name(
         &self,
-        organization_id: &Uuid,
+        opctx: &OpContext,
+        authz_org: &authz::Organization,
         pagparams: &DataPageParams<'_, Name>,
     ) -> ListResultVec<Project> {
         use db::schema::project::dsl;
 
+        opctx.authorize(authz::Action::ListChildren, authz_org).await?;
+
         paginated(dsl::project, dsl::name, &pagparams)
-            .filter(dsl::organization_id.eq(*organization_id))
+            .filter(dsl::organization_id.eq(authz_org.id()))
             .filter(dsl::time_deleted.is_null())
             .select(Project::as_select())
-            .load_async(self.pool())
+            .load_async(self.pool_authorized(opctx).await?)
             .await
-            .map_err(|e| {
-                public_error_from_diesel_pool(
-                    e,
-                    ResourceType::Project,
-                    LookupType::Other("Listing All".to_string()),
-                )
-            })
+            .map_err(public_error_from_diesel_pool_shouldnt_fail)
     }
 
     /// Updates a project by name (clobbering update -- no etag)

nexus/src/external_api/http_entrypoints.rs

@@ -381,13 +381,18 @@ async fn organization_projects_get(
     let organization_name = &path.organization_name;
 
     let handler = async {
+        let opctx = OpContext::for_external_api(&rqctx).await?;
         let params = ScanByNameOrId::from_query(&query)?;
         let field = pagination_field_for_scan_params(params);
         let projects = match field {
             PagField::Id => {
                 let page_selector = data_page_params_nameid_id(&rqctx, &query)?;
                 nexus
-                    .projects_list_by_id(&organization_name, &page_selector)
+                    .projects_list_by_id(
+                        &opctx,
+                        &organization_name,
+                        &page_selector,
+                    )
                     .await?
             }
 
@@ -396,7 +401,11 @@ async fn organization_projects_get(
                     data_page_params_nameid_name(&rqctx, &query)?
                         .map_name(|n| Name::ref_cast(n));
                 nexus
-                    .projects_list_by_name(&organization_name, &page_selector)
+                    .projects_list_by_name(
+                        &opctx,
+                        &organization_name,
+                        &page_selector,
+                    )
                     .await?
             }
         }

nexus/src/nexus.rs

@@ -590,30 +590,28 @@ impl Nexus {
 
     pub async fn projects_list_by_name(
         &self,
+        opctx: &OpContext,
         organization_name: &Name,
         pagparams: &DataPageParams<'_, Name>,
     ) -> ListResultVec<db::model::Project> {
-        let organization_id = self
-            .db_datastore
-            .organization_lookup_id(organization_name)
-            .await?
-            .id();
+        let authz_org =
+            self.db_datastore.organization_lookup_id(organization_name).await?;
         self.db_datastore
-            .projects_list_by_name(&organization_id, pagparams)
+            .projects_list_by_name(opctx, &authz_org, pagparams)
             .await
     }
 
     pub async fn projects_list_by_id(
         &self,
+        opctx: &OpContext,
         organization_name: &Name,
         pagparams: &DataPageParams<'_, Uuid>,
     ) -> ListResultVec<db::model::Project> {
-        let organization_id = self
-            .db_datastore
-            .organization_lookup_id(organization_name)
-            .await?
-            .id();
-        self.db_datastore.projects_list_by_id(&organization_id, pagparams).await
+        let authz_org =
+            self.db_datastore.organization_lookup_id(organization_name).await?;
+        self.db_datastore
+            .projects_list_by_id(opctx, &authz_org, pagparams)
+            .await
     }
 
     pub async fn project_delete(

nexus/tests/integration_tests/basic.rs

@@ -9,7 +9,6 @@
  * TODO-coverage add test for racks, sleds
  */
 
-use dropshot::test_util::iter_collection;
 use dropshot::test_util::object_get;
 use dropshot::test_util::objects_list_page;
 use dropshot::test_util::read_json;
@@ -177,11 +176,32 @@ async fn test_projects_basic(cptestctx: &ControlPlaneTestContext) {
 
     let org_name = "test-org";
     create_organization(&client, &org_name).await;
+    let projects_url = "/organizations/test-org/projects";
+
+    /* Unauthenticated and unauthorized users cannot list projects. */
+    NexusRequest::expect_failure(
+        client,
+        http::StatusCode::NOT_FOUND,
+        http::Method::GET,
+        projects_url,
+    )
+    .execute()
+    .await
+    .expect("failed to make request");
+    NexusRequest::expect_failure(
+        client,
+        http::StatusCode::NOT_FOUND,
+        http::Method::GET,
+        projects_url,
+    )
+    .authn_as(AuthnMode::UnprivilegedUser)
+    .execute()
+    .await
+    .expect("failed to make request");
 
     /*
      * Verify that there are no projects to begin with.
      */
-    let projects_url = "/organizations/test-org/projects";
     let projects = projects_list(&client, &projects_url).await;
     assert_eq!(0, projects.len());
 
@@ -564,9 +584,15 @@ async fn test_projects_list(cptestctx: &ControlPlaneTestContext) {
      * increasing order of name.
      */
     let found_projects_by_name =
-        iter_collection::<Project>(&client, projects_url, "", projects_subset)
-            .await
-            .0;
+        NexusRequest::iter_collection_authn::<Project>(
+            &client,
+            projects_url,
+            "",
+            projects_subset,
+        )
+        .await
+        .expect("failed to list projects")
+        .all_items;
     assert_eq!(found_projects_by_name.len(), project_names_by_name.len());
     assert_eq!(
         project_names_by_name,
@@ -580,14 +606,16 @@ async fn test_projects_list(cptestctx: &ControlPlaneTestContext) {
      * Page through all the projects in ascending order by name, which should be
      * the same as above.
      */
-    let found_projects_by_name = iter_collection::<Project>(
-        &client,
-        projects_url,
-        "sort_by=name-ascending",
-        projects_subset,
-    )
-    .await
-    .0;
+    let found_projects_by_name =
+        NexusRequest::iter_collection_authn::<Project>(
+            &client,
+            projects_url,
+            "sort_by=name-ascending",
+            projects_subset,
+        )
+        .await
+        .expect("failed to list projects")
+        .all_items;
     assert_eq!(found_projects_by_name.len(), project_names_by_name.len());
     assert_eq!(
         project_names_by_name,
@@ -601,14 +629,16 @@ async fn test_projects_list(cptestctx: &ControlPlaneTestContext) {
      * Page through all the projects in descending order by name, which should be
      * the reverse of the above.
      */
-    let mut found_projects_by_name = iter_collection::<Project>(
-        &client,
-        projects_url,
-        "sort_by=name-descending",
-        projects_subset,
-    )
-    .await
-    .0;
+    let mut found_projects_by_name =
+        NexusRequest::iter_collection_authn::<Project>(
+            &client,
+            projects_url,
+            "sort_by=name-descending",
+            projects_subset,
+        )
+        .await
+        .expect("failed to list projects")
+        .all_items;
     assert_eq!(found_projects_by_name.len(), project_names_by_name.len());
     found_projects_by_name.reverse();
     assert_eq!(
@@ -622,14 +652,15 @@ async fn test_projects_list(cptestctx: &ControlPlaneTestContext) {
     /*
      * Page through the projects in ascending order by id.
      */
-    let found_projects_by_id = iter_collection::<Project>(
+    let found_projects_by_id = NexusRequest::iter_collection_authn::<Project>(
         &client,
         projects_url,
         "sort_by=id-ascending",
         projects_subset,
     )
     .await
-    .0;
+    .expect("failed to list projects")
+    .all_items;
     assert_eq!(found_projects_by_id.len(), project_names_by_id.len());
     assert_eq!(
         project_names_by_id,
@@ -679,7 +710,10 @@ async fn projects_list(
     client: &ClientTestContext,
     projects_url: &str,
 ) -> Vec<Project> {
-    objects_list_page::<Project>(client, projects_url).await.items
+    NexusRequest::iter_collection_authn(client, projects_url, "", 10)
+        .await
+        .expect("failed to list projects")
+        .all_items
 }
 
 async fn project_get(client: &ClientTestContext, project_url: &str) -> Project {

nexus/tests/integration_tests/projects.rs

@@ -2,10 +2,11 @@
 // 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 nexus_test_utils::http_testing::AuthnMode;
+use nexus_test_utils::http_testing::NexusRequest;
 use omicron_nexus::external_api::views::Project;
 
 use dropshot::test_util::object_get;
-use dropshot::test_util::objects_list_page;
 
 use nexus_test_utils::resource_helpers::{create_organization, create_project};
 use nexus_test_utils::ControlPlaneTestContext;
@@ -34,12 +35,41 @@ async fn test_projects(cptestctx: &ControlPlaneTestContext) {
     let project: Project = object_get(&client, &p2_url).await;
     assert_eq!(project.identity.name, p2_name);
 
-    let projects = objects_list_page::<Project>(
-        client,
-        &format!("/organizations/{}/projects", org_name),
+    /*
+     * Unauthenticated and unauthorized users should not be able to list
+     * Projects.
+     */
+    let projects_url = format!("/organizations/{}/projects", org_name);
+    NexusRequest::expect_failure(
+        &client,
+        http::StatusCode::NOT_FOUND,
+        http::Method::GET,
+        &projects_url,
     )
+    .execute()
     .await
-    .items;
+    .expect("failed to make request");
+    NexusRequest::expect_failure(
+        &client,
+        http::StatusCode::NOT_FOUND,
+        http::Method::GET,
+        &projects_url,
+    )
+    .authn_as(AuthnMode::UnprivilegedUser)
+    .execute()
+    .await
+    .expect("failed to make request");
+
+    /* Verify the list of Projects. */
+    let projects = NexusRequest::iter_collection_authn::<Project>(
+        &client,
+        &projects_url,
+        "",
+        10,
+    )
+    .await
+    .expect("failed to list projects")
+    .all_items;
     assert_eq!(projects.len(), 2);
     // alphabetical order for now
     assert_eq!(projects[0].identity.name, p2_name);
@@ -54,12 +84,15 @@ async fn test_projects(cptestctx: &ControlPlaneTestContext) {
     assert_ne!(org_p1_id, org2_p1_id);
 
     // Make sure the list projects results for the new org make sense
-    let projects = objects_list_page::<Project>(
-        client,
+    let projects = NexusRequest::iter_collection_authn::<Project>(
+        &client,
         &format!("/organizations/{}/projects", org2_name),
+        "",
+        10,
     )
     .await
-    .items;
+    .expect("failed to list projects")
+    .all_items;
     assert_eq!(projects.len(), 1);
     assert_eq!(projects[0].identity.name, p1_name);
 }

tools/oxapi_demo

@@ -217,7 +217,7 @@ function cmd_organization_get
 function cmd_projects_list
 {
 	[[ $# != 1 ]] && usage "expected ORGANIZATION_NAME"
-	do_curl "/organizations/$1/projects"
+	do_curl_authn "/organizations/$1/projects"
 }
 
 function cmd_project_create_demo