authz: add checks for project update and delete

nexus/src/db/datastore.rs

@@ -806,26 +806,23 @@ impl DataStore {
      */
     pub async fn project_delete(
         &self,
-        organization_id: &Uuid,
-        name: &Name,
+        opctx: &OpContext,
+        authz_project: &authz::Project,
     ) -> DeleteResult {
+        opctx.authorize(authz::Action::Delete, authz_project).await?;
+
         use db::schema::project::dsl;
 
         let now = Utc::now();
         diesel::update(dsl::project)
             .filter(dsl::time_deleted.is_null())
-            .filter(dsl::organization_id.eq(*organization_id))
-            .filter(dsl::name.eq(name.clone()))
+            .filter(dsl::id.eq(authz_project.id()))
             .set(dsl::time_deleted.eq(now))
             .returning(Project::as_returning())
-            .get_result_async(self.pool())
+            .get_result_async(self.pool_authorized(opctx).await?)
             .await
             .map_err(|e| {
-                public_error_from_diesel_pool_lookup(
-                    e,
-                    ResourceType::Project,
-                    LookupType::ByName(name.as_str().to_owned()),
-                )
+                public_error_from_diesel_pool_authz(e, authz_project)
             })?;
         Ok(())
     }
@@ -868,30 +865,24 @@ impl DataStore {
             .map_err(public_error_from_diesel_pool_shouldnt_fail)
     }
 
-    /// Updates a project by name (clobbering update -- no etag)
+    /// Updates a project (clobbering update -- no etag)
     pub async fn project_update(
         &self,
-        organization_id: &Uuid,
-        name: &Name,
+        opctx: &OpContext,
+        authz_project: &authz::Project,
         updates: ProjectUpdate,
     ) -> UpdateResult<Project> {
-        use db::schema::project::dsl;
+        opctx.authorize(authz::Action::Modify, authz_project).await?;
 
+        use db::schema::project::dsl;
         diesel::update(dsl::project)
             .filter(dsl::time_deleted.is_null())
-            .filter(dsl::organization_id.eq(*organization_id))
-            .filter(dsl::name.eq(name.clone()))
+            .filter(dsl::id.eq(authz_project.id()))
             .set(updates)
             .returning(Project::as_returning())
-            .get_result_async(self.pool())
+            .get_result_async(self.pool_authorized(opctx).await?)
             .await
-            .map_err(|e| {
-                public_error_from_diesel_pool_lookup(
-                    e,
-                    ResourceType::Project,
-                    LookupType::ByName(name.as_str().to_owned()),
-                )
-            })
+            .map_err(|e| public_error_from_diesel_pool_authz(e, authz_project))
     }
 
     /*

nexus/src/external_api/http_entrypoints.rs

@@ -506,7 +506,8 @@ async fn organization_projects_delete_project(
     let organization_name = &params.organization_name;
     let project_name = &params.project_name;
     let handler = async {
-        nexus.project_delete(&organization_name, &project_name).await?;
+        let opctx = OpContext::for_external_api(&rqctx).await?;
+        nexus.project_delete(&opctx, &organization_name, &project_name).await?;
         Ok(HttpResponseDeleted())
     };
     apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await
@@ -538,8 +539,10 @@ async fn organization_projects_put_project(
     let organization_name = &path.organization_name;
     let project_name = &path.project_name;
     let handler = async {
+        let opctx = OpContext::for_external_api(&rqctx).await?;
         let newproject = nexus
             .project_update(
+                &opctx,
                 &organization_name,
                 &project_name,
                 &updated_project.into_inner(),

nexus/src/nexus.rs

@@ -632,34 +632,30 @@ impl Nexus {
 
     pub async fn project_delete(
         &self,
+        opctx: &OpContext,
         organization_name: &Name,
         project_name: &Name,
     ) -> DeleteResult {
-        let organization_id = self
+        let authz_project = self
             .db_datastore
-            .organization_lookup_path(organization_name)
-            .await?
-            .id();
-        self.db_datastore.project_delete(&organization_id, project_name).await
+            .project_lookup_path(organization_name, project_name)
+            .await?;
+        self.db_datastore.project_delete(opctx, &authz_project).await
     }
 
     pub async fn project_update(
         &self,
+        opctx: &OpContext,
         organization_name: &Name,
         project_name: &Name,
         new_params: &params::ProjectUpdate,
     ) -> UpdateResult<db::model::Project> {
-        let organization_id = self
+        let authz_project = self
             .db_datastore
-            .organization_lookup_path(organization_name)
-            .await?
-            .id();
+            .project_lookup_path(organization_name, project_name)
+            .await?;
         self.db_datastore
-            .project_update(
-                &organization_id,
-                project_name,
-                new_params.clone().into(),
-            )
+            .project_update(opctx, &authz_project, new_params.clone().into())
             .await
     }
 

nexus/test-utils/src/http_testing.rs

@@ -479,6 +479,22 @@ impl<'a> NexusRequest<'a> {
         )
     }
 
+    /// Returns a new `NexusRequest` suitable for `PUT $uri`
+    pub fn object_put<B>(
+        testctx: &'a ClientTestContext,
+        uri: &str,
+        body: Option<&B>,
+    ) -> Self
+    where
+        B: serde::Serialize,
+    {
+        NexusRequest::new(
+            RequestBuilder::new(testctx, http::Method::PUT, uri)
+                .body(body)
+                .expect_status(Some(http::StatusCode::OK)),
+        )
+    }
+
     /// Convenience constructor for failure cases
     pub fn expect_failure(
         testctx: &'a ClientTestContext,

nexus/tests/integration_tests/basic.rs

@@ -10,7 +10,6 @@
  */
 
 use dropshot::test_util::objects_list_page;
-use dropshot::test_util::read_json;
 use dropshot::test_util::ClientTestContext;
 use dropshot::HttpErrorResponseBody;
 use http::method::Method;
@@ -312,46 +311,49 @@ async fn test_projects_basic(cptestctx: &ControlPlaneTestContext) {
      * Delete "simproject2".  We'll make sure that's reflected in the other
      * requests.
      */
-    client
-        .make_request_no_body(
-            Method::DELETE,
-            "/organizations/test-org/projects/simproject2",
-            StatusCode::NO_CONTENT,
-        )
-        .await
-        .expect("expected success");
+    NexusRequest::object_delete(
+        client,
+        "/organizations/test-org/projects/simproject2",
+    )
+    .authn_as(AuthnMode::PrivilegedUser)
+    .execute()
+    .await
+    .expect("expected request to fail");
 
     /*
      * Having deleted "simproject2", verify "GET", "PUT", and "DELETE" on
      * "/organizations/test-org/projects/simproject2".
      */
-    client
-        .make_request_error(
-            Method::GET,
-            "/organizations/test-org/projects/simproject2",
+    for method in [Method::GET, Method::DELETE] {
+        NexusRequest::expect_failure(
+            client,
             StatusCode::NOT_FOUND,
-        )
-        .await;
-    client
-        .make_request_error(
-            Method::DELETE,
+            method,
             "/organizations/test-org/projects/simproject2",
-            StatusCode::NOT_FOUND,
         )
-        .await;
-    client
-        .make_request_error_body(
+        .authn_as(AuthnMode::PrivilegedUser)
+        .execute()
+        .await
+        .expect("failed to make request");
+    }
+    NexusRequest::new(
+        RequestBuilder::new(
+            client,
             Method::PUT,
             "/organizations/test-org/projects/simproject2",
-            params::ProjectUpdate {
-                identity: IdentityMetadataUpdateParams {
-                    name: None,
-                    description: None,
-                },
-            },
-            StatusCode::NOT_FOUND,
         )
-        .await;
+        .body(Some(&params::ProjectUpdate {
+            identity: IdentityMetadataUpdateParams {
+                name: None,
+                description: None,
+            },
+        }))
+        .expect_status(Some(StatusCode::NOT_FOUND)),
+    )
+    .authn_as(AuthnMode::PrivilegedUser)
+    .execute()
+    .await
+    .expect("failed to make request");
 
     /*
      * Similarly, verify "GET /organizations/test-org/projects"
@@ -383,25 +385,61 @@ async fn test_projects_basic(cptestctx: &ControlPlaneTestContext) {
     );
 
     /*
-     * Update "simproject3".  We'll make sure that's reflected in the other
-     * requests.
+     * Unprivileged users should not be able to update a Project.
      */
     let project_update = params::ProjectUpdate {
         identity: IdentityMetadataUpdateParams {
             name: None,
-            description: Some("Li'l lightnin'".to_string()),
+            description: None,
         },
     };
-    let mut response = client
-        .make_request(
+    NexusRequest::new(
+        RequestBuilder::new(
+            client,
             Method::PUT,
             "/organizations/test-org/projects/simproject3",
-            Some(project_update),
-            StatusCode::OK,
         )
-        .await
-        .expect("expected success");
-    let project: Project = read_json(&mut response).await;
+        .body(Some(&project_update))
+        .expect_status(Some(StatusCode::NOT_FOUND)),
+    )
+    .execute()
+    .await
+    .expect("failed to make request");
+    NexusRequest::new(
+        RequestBuilder::new(
+            client,
+            Method::PUT,
+            "/organizations/test-org/projects/simproject3",
+        )
+        .body(Some(&project_update))
+        .expect_status(Some(StatusCode::NOT_FOUND)),
+    )
+    .authn_as(AuthnMode::UnprivilegedUser)
+    .execute()
+    .await
+    .expect("failed to make request");
+
+    /*
+     * Update "simproject3".  We'll make sure that's reflected in the other
+     * requests.
+     */
+    let project_update = params::ProjectUpdate {
+        identity: IdentityMetadataUpdateParams {
+            name: None,
+            description: Some("Li'l lightnin'".to_string()),
+        },
+    };
+    let project = NexusRequest::object_put(
+        client,
+        "/organizations/test-org/projects/simproject3",
+        Some(&project_update),
+    )
+    .authn_as(AuthnMode::PrivilegedUser)
+    .execute()
+    .await
+    .expect("expected success")
+    .parsed_body::<Project>()
+    .expect("failed to parse Project from PUT response");
     assert_eq!(project.identity.id, new_project_ids[2]);
     assert_eq!(project.identity.name, "simproject3");
     assert_eq!(project.identity.description, "Li'l lightnin'");
@@ -425,27 +463,31 @@ async fn test_projects_basic(cptestctx: &ControlPlaneTestContext) {
             description: Some("little lightning".to_string()),
         },
     };
-    let mut response = client
-        .make_request(
-            Method::PUT,
-            "/organizations/test-org/projects/simproject3",
-            Some(project_update),
-            StatusCode::OK,
-        )
-        .await
-        .expect("failed to make request to server");
-    let project: Project = read_json(&mut response).await;
+    let project = NexusRequest::object_put(
+        client,
+        "/organizations/test-org/projects/simproject3",
+        Some(&project_update),
+    )
+    .authn_as(AuthnMode::PrivilegedUser)
+    .execute()
+    .await
+    .expect("expected success")
+    .parsed_body::<Project>()
+    .expect("failed to parse Project from PUT response");
     assert_eq!(project.identity.id, new_project_ids[2]);
     assert_eq!(project.identity.name, "lil-lightnin");
     assert_eq!(project.identity.description, "little lightning");
 
-    client
-        .make_request_error(
-            Method::GET,
-            "/organizations/test-org/projects/simproject3",
-            StatusCode::NOT_FOUND,
-        )
-        .await;
+    NexusRequest::expect_failure(
+        client,
+        StatusCode::NOT_FOUND,
+        Method::GET,
+        "/organizations/test-org/projects/simproject3",
+    )
+    .authn_as(AuthnMode::PrivilegedUser)
+    .execute()
+    .await
+    .expect("expected success");
 
     /*
      * Try to create a project with a name that conflicts with an existing one.

nexus/tests/integration_tests/organizations.rs

@@ -5,8 +5,6 @@
 use nexus_test_utils::http_testing::{AuthnMode, NexusRequest};
 use omicron_nexus::external_api::views::Organization;
 
-use dropshot::test_util::object_delete;
-
 use http::method::Method;
 use http::StatusCode;
 use nexus_test_utils::resource_helpers::{
@@ -168,7 +166,11 @@ async fn test_organizations(cptestctx: &ControlPlaneTestContext) {
     .expect("failed to make request");
 
     // Delete the project, then delete the organization
-    object_delete(&client, &project_url).await;
+    NexusRequest::object_delete(&client, &project_url)
+        .authn_as(AuthnMode::PrivilegedUser)
+        .execute()
+        .await
+        .expect("failed to make request");
     NexusRequest::object_delete(&client, &o2_url)
         .authn_as(AuthnMode::PrivilegedUser)
         .execute()