From 02879a3336394cc0b61356d7fff2c1c1e9a69547 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Thu, 3 Jul 2025 16:43:42 -0400 Subject: [PATCH 01/11] Add an API route for admins to assess crates Connects to #10387. This API route, under `/private/` rather than `v1`, will make it a lot easier to assess whether crates contain useful functionality or are squatting. This is only the backend; some of the functionality described in the issue will be frontend-only, but I wanted to get the API in first. The API route will only return data if the currently authenticated user is an admin. This is important because the crate owner's verified email address is part of the returned data so that the admin can contact the owner if necessary. Another reason this route is limited to admins is that some of the queries may be slow. --- src/controllers.rs | 1 + src/controllers/admin.rs | 124 +++++++++++++++++++++++++++++++ src/router.rs | 2 + src/tests/mod.rs | 12 ++- src/tests/routes/crates/admin.rs | 79 ++++++++++++++++++++ src/tests/routes/crates/mod.rs | 1 + src/tests/util.rs | 10 ++- 7 files changed, 226 insertions(+), 3 deletions(-) create mode 100644 src/controllers/admin.rs create mode 100644 src/tests/routes/crates/admin.rs diff --git a/src/controllers.rs b/src/controllers.rs index cd055a67c18..a58d7b1ab1b 100644 --- a/src/controllers.rs +++ b/src/controllers.rs @@ -1,6 +1,7 @@ pub mod helpers; pub mod util; +pub mod admin; pub mod category; pub mod crate_owner_invitation; pub mod git; diff --git a/src/controllers/admin.rs b/src/controllers/admin.rs new file mode 100644 index 00000000000..a6bee769f0b --- /dev/null +++ b/src/controllers/admin.rs @@ -0,0 +1,124 @@ +use crate::{ + app::AppState, + auth::AuthCheck, + models::{CrateOwner, OwnerKind, User, Version}, + schema::*, + util::errors::{AppResult, custom}, +}; +use axum::{Json, extract::Path}; +use chrono::{DateTime, Utc}; +use diesel::{dsl::count_star, prelude::*}; +use diesel_async::RunQueryDsl; +use http::{StatusCode, request::Parts}; +use serde::Serialize; +use std::collections::HashMap; + +/// Handles the `GET /api/private/admin_list/{username}` endpoint. +pub async fn list( + state: AppState, + Path(username): Path, + req: Parts, +) -> AppResult> { + let mut conn = state.db_write().await?; + + let auth = AuthCheck::default().check(&req, &mut conn).await?; + let logged_in_user = auth.user(); + + if !logged_in_user.is_admin { + return Err(custom( + StatusCode::FORBIDDEN, + "must be an admin to use this route", + )); + } + + let (user, verified, email) = users::table + .left_join(emails::table) + .filter(users::gh_login.eq(username)) + .select(( + User::as_select(), + emails::verified.nullable(), + emails::email.nullable(), + )) + .first::<(User, Option, Option)>(&mut conn) + .await?; + + let crates: Vec<(i32, String, DateTime, i64)> = CrateOwner::by_owner_kind(OwnerKind::User) + .inner_join(crates::table) + .filter(crate_owners::owner_id.eq(user.id)) + .select(( + crates::id, + crates::name, + crates::updated_at, + rev_deps_subquery(), + )) + .order(crates::name.asc()) + .load(&mut conn) + .await?; + + let crate_ids: Vec<_> = crates.iter().map(|(id, ..)| id).collect(); + + let versions: Vec = versions::table + .filter(versions::crate_id.eq_any(crate_ids)) + .select(Version::as_select()) + .load(&mut conn) + .await?; + let mut versions_by_crate_id: HashMap> = HashMap::new(); + for version in versions { + let crate_versions = versions_by_crate_id.entry(version.crate_id).or_default(); + crate_versions.push(version); + } + + let verified = verified.unwrap_or(false); + let crates = crates + .into_iter() + .map(|(crate_id, name, updated_at, num_rev_deps)| { + let versions = versions_by_crate_id.get(&crate_id); + let last_version = versions.and_then(|v| v.last()); + AdminCrateInfo { + name, + updated_at, + num_rev_deps, + num_versions: versions.map(|v| v.len()).unwrap_or(0), + crate_size: last_version.map(|v| v.crate_size).unwrap_or(0), + bin_names: last_version + .map(|v| v.bin_names.clone()) + .unwrap_or_default(), + } + }) + .collect(); + Ok(Json(AdminListResponse { + user_email: verified.then_some(email).flatten(), + crates, + })) +} + +#[derive(Debug, Serialize)] +pub struct AdminListResponse { + user_email: Option, + crates: Vec, +} + +#[derive(Debug, Serialize)] +pub struct AdminCrateInfo { + pub name: String, + pub updated_at: DateTime, + pub num_rev_deps: i64, + pub num_versions: usize, + pub crate_size: i32, + pub bin_names: Option>>, +} + +/// A subquery that returns the number of reverse dependencies of a crate. +/// +/// **Warning:** this is an incorrect reverse dependencies query, since it +/// includes the `dependencies` rows for all versions, not just the +/// "default version" per crate. However, it's good enough for our +/// purposes here. +#[diesel::dsl::auto_type] +fn rev_deps_subquery() -> _ { + dependencies::table + .select(count_star()) + .filter(dependencies::crate_id.eq(crates::id)) + .single_value() + .assume_not_null() +} diff --git a/src/router.rs b/src/router.rs index 254c5e8ced3..91d598f0cb5 100644 --- a/src/router.rs +++ b/src/router.rs @@ -103,6 +103,8 @@ pub fn build_axum_router(state: AppState) -> Router<()> { let mut router = router // Metrics .route("/api/private/metrics/{kind}", get(metrics::prometheus)) + // Listing a user's crates for admin/support purposes + .route("/api/private/admin_list/{username}", get(admin::list)) // Alerts from GitHub scanning for exposed API tokens .route( "/api/github/secret-scanning/verify", diff --git a/src/tests/mod.rs b/src/tests/mod.rs index eba892f98a8..feb4200255a 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -84,7 +84,17 @@ pub struct OkBool { #[allow(dead_code)] ok: bool, } - +#[derive(Deserialize)] +pub struct AdminListResponse { + user_email: Option, + crates: Vec, +} +#[derive(Deserialize)] +pub struct AdminCrateInfo { + name: String, + num_versions: usize, + num_rev_deps: i64, +} #[derive(Deserialize, Debug)] pub struct OwnerResp { // server must include `ok: true` to support old cargo clients diff --git a/src/tests/routes/crates/admin.rs b/src/tests/routes/crates/admin.rs new file mode 100644 index 00000000000..776d7317ddc --- /dev/null +++ b/src/tests/routes/crates/admin.rs @@ -0,0 +1,79 @@ +use crate::{ + schema::users, + tests::{ + builders::{CrateBuilder, VersionBuilder}, + util::{RequestHelper, TestApp}, + }, +}; +use diesel::prelude::*; +use diesel_async::RunQueryDsl; +use insta::assert_snapshot; + +#[tokio::test(flavor = "multi_thread")] +async fn admin_list_by_a_non_admin_fails() { + let (_app, anon, user) = TestApp::init().with_user().await; + + let response = anon.admin_list("anything").await; + assert_snapshot!(response.status(), @"403 Forbidden"); + assert_snapshot!( + response.text(), + @r#"{"errors":[{"detail":"this action requires authentication"}]}"# + ); + + let response = user.admin_list("anything").await; + assert_snapshot!(response.status(), @"403 Forbidden"); + assert_snapshot!( + response.text(), + @r#"{"errors":[{"detail":"must be an admin to use this route"}]}"# + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn index_include_yanked() -> anyhow::Result<()> { + let (app, _anon, user) = TestApp::init().with_user().await; + let mut conn = app.db_conn().await; + let user = user.as_model(); + + let admin = app.db_new_user("admin").await; + + diesel::update(admin.as_model()) + .set(users::is_admin.eq(true)) + .execute(&mut conn) + .await + .unwrap(); + + let crate_1 = CrateBuilder::new("unyanked", user.id) + .version(VersionBuilder::new("0.1.0").yanked(true)) + .version(VersionBuilder::new("1.0.0")) + .version(VersionBuilder::new("2.0.0")) + .expect_build(&mut conn) + .await; + + CrateBuilder::new("all_yanked", user.id) + .version(VersionBuilder::new("1.0.0").yanked(true)) + .version(VersionBuilder::new("2.0.0").yanked(true)) + .expect_build(&mut conn) + .await; + + CrateBuilder::new("someone_elses_crate", admin.as_model().id) + .version(VersionBuilder::new("1.0.0").dependency(&crate_1, None)) + .expect_build(&mut conn) + .await; + + // Include fully yanked (all versions were yanked) crates + let username = &user.gh_login; + let json = admin.admin_list(username).await.good(); + + assert_eq!(json.user_email.unwrap(), "foo@example.com"); + assert_eq!(json.crates.len(), 2); + + assert_eq!(json.crates[0].name, "all_yanked"); + assert_eq!(json.crates[0].num_versions, 2); + assert_eq!(json.crates[0].num_rev_deps, 0); + + assert_eq!(json.crates[1].name, "unyanked"); + assert_eq!(json.crates[1].num_versions, 3); + assert_eq!(json.crates[1].num_rev_deps, 1); + + Ok(()) +} diff --git a/src/tests/routes/crates/mod.rs b/src/tests/routes/crates/mod.rs index 2619dcbfb94..a2160a66431 100644 --- a/src/tests/routes/crates/mod.rs +++ b/src/tests/routes/crates/mod.rs @@ -1,3 +1,4 @@ +mod admin; pub mod downloads; mod following; mod list; diff --git a/src/tests/util.rs b/src/tests/util.rs index c6f4c3766fc..ff8823c64d5 100644 --- a/src/tests/util.rs +++ b/src/tests/util.rs @@ -21,8 +21,8 @@ use crate::models::{ApiToken, User}; use crate::tests::{ - CategoryListResponse, CategoryResponse, CrateList, CrateResponse, GoodCrate, OwnerResp, - OwnersResponse, VersionResponse, + AdminListResponse, CategoryListResponse, CategoryResponse, CrateList, CrateResponse, GoodCrate, + OwnerResp, OwnersResponse, VersionResponse, }; use std::future::Future; @@ -186,6 +186,12 @@ pub trait RequestHelper { self.get_with_query("/api/v1/crates", query).await.good() } + /// Request the JSON used for the admin list page + async fn admin_list(&self, owner: &str) -> Response { + let url = format!("/api/private/admin_list/{owner}"); + self.get(&url).await + } + /// Publish the crate and run background jobs to completion /// /// Background jobs will publish to the git index and sync to the HTTP index. From d851e9d1c688324e897fbfeab1f4edec78ab0304 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Mon, 7 Jul 2025 16:23:36 -0400 Subject: [PATCH 02/11] Admin list only needs read access --- src/controllers/admin.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controllers/admin.rs b/src/controllers/admin.rs index a6bee769f0b..ad5660ac6ad 100644 --- a/src/controllers/admin.rs +++ b/src/controllers/admin.rs @@ -19,7 +19,7 @@ pub async fn list( Path(username): Path, req: Parts, ) -> AppResult> { - let mut conn = state.db_write().await?; + let mut conn = state.db_read().await?; let auth = AuthCheck::default().check(&req, &mut conn).await?; let logged_in_user = auth.user(); From 59a762ef155ac396db4f8442fe1c32e4fbb84a6d Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Mon, 7 Jul 2025 16:46:52 -0400 Subject: [PATCH 03/11] Add description to crate admin list --- src/controllers/admin.rs | 30 +++++++++++++++++------------- src/tests/mod.rs | 1 + src/tests/routes/crates/admin.rs | 17 +++++++++++------ 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/controllers/admin.rs b/src/controllers/admin.rs index ad5660ac6ad..b2f48ceffef 100644 --- a/src/controllers/admin.rs +++ b/src/controllers/admin.rs @@ -42,18 +42,20 @@ pub async fn list( .first::<(User, Option, Option)>(&mut conn) .await?; - let crates: Vec<(i32, String, DateTime, i64)> = CrateOwner::by_owner_kind(OwnerKind::User) - .inner_join(crates::table) - .filter(crate_owners::owner_id.eq(user.id)) - .select(( - crates::id, - crates::name, - crates::updated_at, - rev_deps_subquery(), - )) - .order(crates::name.asc()) - .load(&mut conn) - .await?; + let crates: Vec<(i32, String, Option, DateTime, i64)> = + CrateOwner::by_owner_kind(OwnerKind::User) + .inner_join(crates::table) + .filter(crate_owners::owner_id.eq(user.id)) + .select(( + crates::id, + crates::name, + crates::description, + crates::updated_at, + rev_deps_subquery(), + )) + .order(crates::name.asc()) + .load(&mut conn) + .await?; let crate_ids: Vec<_> = crates.iter().map(|(id, ..)| id).collect(); @@ -71,11 +73,12 @@ pub async fn list( let verified = verified.unwrap_or(false); let crates = crates .into_iter() - .map(|(crate_id, name, updated_at, num_rev_deps)| { + .map(|(crate_id, name, description, updated_at, num_rev_deps)| { let versions = versions_by_crate_id.get(&crate_id); let last_version = versions.and_then(|v| v.last()); AdminCrateInfo { name, + description, updated_at, num_rev_deps, num_versions: versions.map(|v| v.len()).unwrap_or(0), @@ -101,6 +104,7 @@ pub struct AdminListResponse { #[derive(Debug, Serialize)] pub struct AdminCrateInfo { pub name: String, + pub description: Option, pub updated_at: DateTime, pub num_rev_deps: i64, pub num_versions: usize, diff --git a/src/tests/mod.rs b/src/tests/mod.rs index feb4200255a..30b3bdaa040 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -92,6 +92,7 @@ pub struct AdminListResponse { #[derive(Deserialize)] pub struct AdminCrateInfo { name: String, + description: Option, num_versions: usize, num_rev_deps: i64, } diff --git a/src/tests/routes/crates/admin.rs b/src/tests/routes/crates/admin.rs index 776d7317ddc..50bad9a3f85 100644 --- a/src/tests/routes/crates/admin.rs +++ b/src/tests/routes/crates/admin.rs @@ -43,6 +43,7 @@ async fn index_include_yanked() -> anyhow::Result<()> { .unwrap(); let crate_1 = CrateBuilder::new("unyanked", user.id) + .description("My Fun Crate") .version(VersionBuilder::new("0.1.0").yanked(true)) .version(VersionBuilder::new("1.0.0")) .version(VersionBuilder::new("2.0.0")) @@ -67,13 +68,17 @@ async fn index_include_yanked() -> anyhow::Result<()> { assert_eq!(json.user_email.unwrap(), "foo@example.com"); assert_eq!(json.crates.len(), 2); - assert_eq!(json.crates[0].name, "all_yanked"); - assert_eq!(json.crates[0].num_versions, 2); - assert_eq!(json.crates[0].num_rev_deps, 0); + let json_crate_0 = &json.crates[0]; + assert_eq!(json_crate_0.name, "all_yanked"); + assert!(json_crate_0.description.is_none()); + assert_eq!(json_crate_0.num_versions, 2); + assert_eq!(json_crate_0.num_rev_deps, 0); - assert_eq!(json.crates[1].name, "unyanked"); - assert_eq!(json.crates[1].num_versions, 3); - assert_eq!(json.crates[1].num_rev_deps, 1); + let json_crate_1 = &json.crates[1]; + assert_eq!(json_crate_1.name, "unyanked"); + assert_eq!(json_crate_1.description.as_ref().unwrap(), "My Fun Crate"); + assert_eq!(json_crate_1.num_versions, 3); + assert_eq!(json_crate_1.num_rev_deps, 1); Ok(()) } From 1746fbf1678f2a89f337716d56bce34ec9dfbcc9 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Mon, 7 Jul 2025 17:08:50 -0400 Subject: [PATCH 04/11] Add download count to admin list --- src/controllers/admin.rs | 76 +++++++++++++++++++++----------- src/tests/mod.rs | 5 ++- src/tests/routes/crates/admin.rs | 4 ++ 3 files changed, 58 insertions(+), 27 deletions(-) diff --git a/src/controllers/admin.rs b/src/controllers/admin.rs index b2f48ceffef..c1de4fa5990 100644 --- a/src/controllers/admin.rs +++ b/src/controllers/admin.rs @@ -42,20 +42,33 @@ pub async fn list( .first::<(User, Option, Option)>(&mut conn) .await?; - let crates: Vec<(i32, String, Option, DateTime, i64)> = - CrateOwner::by_owner_kind(OwnerKind::User) - .inner_join(crates::table) - .filter(crate_owners::owner_id.eq(user.id)) - .select(( - crates::id, - crates::name, - crates::description, - crates::updated_at, - rev_deps_subquery(), - )) - .order(crates::name.asc()) - .load(&mut conn) - .await?; + let crates: Vec<( + i32, + String, + Option, + DateTime, + Option, + Option, + i64, + )> = CrateOwner::by_owner_kind(OwnerKind::User) + .inner_join(crates::table) + .left_join(crate_downloads::table.on(crates::id.eq(crate_downloads::crate_id))) + .left_join( + recent_crate_downloads::table.on(crates::id.eq(recent_crate_downloads::crate_id)), + ) + .filter(crate_owners::owner_id.eq(user.id)) + .select(( + crates::id, + crates::name, + crates::description, + crates::updated_at, + crate_downloads::downloads.nullable(), + recent_crate_downloads::downloads.nullable(), + rev_deps_subquery(), + )) + .order(crates::name.asc()) + .load(&mut conn) + .await?; let crate_ids: Vec<_> = crates.iter().map(|(id, ..)| id).collect(); @@ -73,21 +86,33 @@ pub async fn list( let verified = verified.unwrap_or(false); let crates = crates .into_iter() - .map(|(crate_id, name, description, updated_at, num_rev_deps)| { - let versions = versions_by_crate_id.get(&crate_id); - let last_version = versions.and_then(|v| v.last()); - AdminCrateInfo { + .map( + |( + crate_id, name, description, updated_at, + downloads, + recent_crate_downloads, num_rev_deps, - num_versions: versions.map(|v| v.len()).unwrap_or(0), - crate_size: last_version.map(|v| v.crate_size).unwrap_or(0), - bin_names: last_version - .map(|v| v.bin_names.clone()) - .unwrap_or_default(), - } - }) + )| { + let versions = versions_by_crate_id.get(&crate_id); + let last_version = versions.and_then(|v| v.last()); + AdminCrateInfo { + name, + description, + updated_at, + downloads: downloads.unwrap_or_default() + + recent_crate_downloads.unwrap_or_default(), + num_rev_deps, + num_versions: versions.map(|v| v.len()).unwrap_or(0), + crate_size: last_version.map(|v| v.crate_size).unwrap_or(0), + bin_names: last_version + .map(|v| v.bin_names.clone()) + .unwrap_or_default(), + } + }, + ) .collect(); Ok(Json(AdminListResponse { user_email: verified.then_some(email).flatten(), @@ -106,6 +131,7 @@ pub struct AdminCrateInfo { pub name: String, pub description: Option, pub updated_at: DateTime, + pub downloads: i64, pub num_rev_deps: i64, pub num_versions: usize, pub crate_size: i32, diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 30b3bdaa040..d7cdd0a6855 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -84,15 +84,16 @@ pub struct OkBool { #[allow(dead_code)] ok: bool, } -#[derive(Deserialize)] +#[derive(Deserialize, Debug)] pub struct AdminListResponse { user_email: Option, crates: Vec, } -#[derive(Deserialize)] +#[derive(Deserialize, Debug)] pub struct AdminCrateInfo { name: String, description: Option, + downloads: i64, num_versions: usize, num_rev_deps: i64, } diff --git a/src/tests/routes/crates/admin.rs b/src/tests/routes/crates/admin.rs index 50bad9a3f85..6505f3f0168 100644 --- a/src/tests/routes/crates/admin.rs +++ b/src/tests/routes/crates/admin.rs @@ -44,6 +44,8 @@ async fn index_include_yanked() -> anyhow::Result<()> { let crate_1 = CrateBuilder::new("unyanked", user.id) .description("My Fun Crate") + .downloads(500) + .recent_downloads(36) .version(VersionBuilder::new("0.1.0").yanked(true)) .version(VersionBuilder::new("1.0.0")) .version(VersionBuilder::new("2.0.0")) @@ -71,12 +73,14 @@ async fn index_include_yanked() -> anyhow::Result<()> { let json_crate_0 = &json.crates[0]; assert_eq!(json_crate_0.name, "all_yanked"); assert!(json_crate_0.description.is_none()); + assert_eq!(json_crate_0.downloads, 0); assert_eq!(json_crate_0.num_versions, 2); assert_eq!(json_crate_0.num_rev_deps, 0); let json_crate_1 = &json.crates[1]; assert_eq!(json_crate_1.name, "unyanked"); assert_eq!(json_crate_1.description.as_ref().unwrap(), "My Fun Crate"); + assert_eq!(json_crate_1.downloads, 536); assert_eq!(json_crate_1.num_versions, 3); assert_eq!(json_crate_1.num_rev_deps, 1); From 30a977c43920b9219c92bc1c4c2e28ad2eaec10c Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Mon, 7 Jul 2025 17:27:08 -0400 Subject: [PATCH 05/11] Add default version num to admin list --- src/controllers/admin.rs | 13 ++++++++++--- src/tests/mod.rs | 1 + src/tests/routes/crates/admin.rs | 4 +++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/controllers/admin.rs b/src/controllers/admin.rs index c1de4fa5990..3d85cd527c9 100644 --- a/src/controllers/admin.rs +++ b/src/controllers/admin.rs @@ -49,6 +49,7 @@ pub async fn list( DateTime, Option, Option, + i32, i64, )> = CrateOwner::by_owner_kind(OwnerKind::User) .inner_join(crates::table) @@ -56,6 +57,7 @@ pub async fn list( .left_join( recent_crate_downloads::table.on(crates::id.eq(recent_crate_downloads::crate_id)), ) + .inner_join(default_versions::table.on(crates::id.eq(default_versions::crate_id))) .filter(crate_owners::owner_id.eq(user.id)) .select(( crates::id, @@ -64,6 +66,7 @@ pub async fn list( crates::updated_at, crate_downloads::downloads.nullable(), recent_crate_downloads::downloads.nullable(), + default_versions::version_id, rev_deps_subquery(), )) .order(crates::name.asc()) @@ -94,10 +97,12 @@ pub async fn list( updated_at, downloads, recent_crate_downloads, + default_version, num_rev_deps, )| { let versions = versions_by_crate_id.get(&crate_id); - let last_version = versions.and_then(|v| v.last()); + let default_version = + versions.and_then(|versions| versions.iter().find(|v| v.id == default_version)); AdminCrateInfo { name, description, @@ -106,8 +111,9 @@ pub async fn list( + recent_crate_downloads.unwrap_or_default(), num_rev_deps, num_versions: versions.map(|v| v.len()).unwrap_or(0), - crate_size: last_version.map(|v| v.crate_size).unwrap_or(0), - bin_names: last_version + default_version_num: default_version.map(|v| v.num.clone()).unwrap_or_default(), + crate_size: default_version.map(|v| v.crate_size).unwrap_or(0), + bin_names: default_version .map(|v| v.bin_names.clone()) .unwrap_or_default(), } @@ -134,6 +140,7 @@ pub struct AdminCrateInfo { pub downloads: i64, pub num_rev_deps: i64, pub num_versions: usize, + pub default_version_num: String, pub crate_size: i32, pub bin_names: Option>>, } diff --git a/src/tests/mod.rs b/src/tests/mod.rs index d7cdd0a6855..52aa5f15faf 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -95,6 +95,7 @@ pub struct AdminCrateInfo { description: Option, downloads: i64, num_versions: usize, + default_version_num: String, num_rev_deps: i64, } #[derive(Deserialize, Debug)] diff --git a/src/tests/routes/crates/admin.rs b/src/tests/routes/crates/admin.rs index 6505f3f0168..8d3f9710ded 100644 --- a/src/tests/routes/crates/admin.rs +++ b/src/tests/routes/crates/admin.rs @@ -48,7 +48,7 @@ async fn index_include_yanked() -> anyhow::Result<()> { .recent_downloads(36) .version(VersionBuilder::new("0.1.0").yanked(true)) .version(VersionBuilder::new("1.0.0")) - .version(VersionBuilder::new("2.0.0")) + .version(VersionBuilder::new("2.0.0").yanked(true)) .expect_build(&mut conn) .await; @@ -75,6 +75,7 @@ async fn index_include_yanked() -> anyhow::Result<()> { assert!(json_crate_0.description.is_none()); assert_eq!(json_crate_0.downloads, 0); assert_eq!(json_crate_0.num_versions, 2); + assert_eq!(json_crate_0.default_version_num, "2.0.0"); assert_eq!(json_crate_0.num_rev_deps, 0); let json_crate_1 = &json.crates[1]; @@ -82,6 +83,7 @@ async fn index_include_yanked() -> anyhow::Result<()> { assert_eq!(json_crate_1.description.as_ref().unwrap(), "My Fun Crate"); assert_eq!(json_crate_1.downloads, 536); assert_eq!(json_crate_1.num_versions, 3); + assert_eq!(json_crate_1.default_version_num, "1.0.0"); assert_eq!(json_crate_1.num_rev_deps, 1); Ok(()) From 4144a3d1125df6c8a2eeecf8f7052c1a6b9a2ef4 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Tue, 8 Jul 2025 13:45:05 -0400 Subject: [PATCH 06/11] Get num versions from default versions table instead --- src/controllers/admin.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/controllers/admin.rs b/src/controllers/admin.rs index 3d85cd527c9..9410790b418 100644 --- a/src/controllers/admin.rs +++ b/src/controllers/admin.rs @@ -50,6 +50,7 @@ pub async fn list( Option, Option, i32, + Option, i64, )> = CrateOwner::by_owner_kind(OwnerKind::User) .inner_join(crates::table) @@ -67,6 +68,7 @@ pub async fn list( crate_downloads::downloads.nullable(), recent_crate_downloads::downloads.nullable(), default_versions::version_id, + default_versions::num_versions, rev_deps_subquery(), )) .order(crates::name.asc()) @@ -98,6 +100,7 @@ pub async fn list( downloads, recent_crate_downloads, default_version, + num_versions, num_rev_deps, )| { let versions = versions_by_crate_id.get(&crate_id); @@ -110,7 +113,7 @@ pub async fn list( downloads: downloads.unwrap_or_default() + recent_crate_downloads.unwrap_or_default(), num_rev_deps, - num_versions: versions.map(|v| v.len()).unwrap_or(0), + num_versions: num_versions.unwrap_or_default() as usize, default_version_num: default_version.map(|v| v.num.clone()).unwrap_or_default(), crate_size: default_version.map(|v| v.crate_size).unwrap_or(0), bin_names: default_version From e8d7a8e3ae164c04f6e0e8fa7b7deb0e78337921 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Tue, 8 Jul 2025 13:52:18 -0400 Subject: [PATCH 07/11] Select the default version directly rather than all versions --- src/controllers/admin.rs | 43 +++++++++++++--------------------------- 1 file changed, 14 insertions(+), 29 deletions(-) diff --git a/src/controllers/admin.rs b/src/controllers/admin.rs index 9410790b418..a5451b67d35 100644 --- a/src/controllers/admin.rs +++ b/src/controllers/admin.rs @@ -1,7 +1,7 @@ use crate::{ app::AppState, auth::AuthCheck, - models::{CrateOwner, OwnerKind, User, Version}, + models::{CrateOwner, OwnerKind, User}, schema::*, util::errors::{AppResult, custom}, }; @@ -11,7 +11,6 @@ use diesel::{dsl::count_star, prelude::*}; use diesel_async::RunQueryDsl; use http::{StatusCode, request::Parts}; use serde::Serialize; -use std::collections::HashMap; /// Handles the `GET /api/private/admin_list/{username}` endpoint. pub async fn list( @@ -43,14 +42,15 @@ pub async fn list( .await?; let crates: Vec<( - i32, String, Option, DateTime, Option, Option, - i32, Option, + String, + i32, + Option>>, i64, )> = CrateOwner::by_owner_kind(OwnerKind::User) .inner_join(crates::table) @@ -59,53 +59,40 @@ pub async fn list( recent_crate_downloads::table.on(crates::id.eq(recent_crate_downloads::crate_id)), ) .inner_join(default_versions::table.on(crates::id.eq(default_versions::crate_id))) + .inner_join(versions::table.on(default_versions::version_id.eq(versions::id))) .filter(crate_owners::owner_id.eq(user.id)) .select(( - crates::id, crates::name, crates::description, crates::updated_at, crate_downloads::downloads.nullable(), recent_crate_downloads::downloads.nullable(), - default_versions::version_id, default_versions::num_versions, + versions::num, + versions::crate_size, + versions::bin_names, rev_deps_subquery(), )) .order(crates::name.asc()) .load(&mut conn) .await?; - let crate_ids: Vec<_> = crates.iter().map(|(id, ..)| id).collect(); - - let versions: Vec = versions::table - .filter(versions::crate_id.eq_any(crate_ids)) - .select(Version::as_select()) - .load(&mut conn) - .await?; - let mut versions_by_crate_id: HashMap> = HashMap::new(); - for version in versions { - let crate_versions = versions_by_crate_id.entry(version.crate_id).or_default(); - crate_versions.push(version); - } - let verified = verified.unwrap_or(false); let crates = crates .into_iter() .map( |( - crate_id, name, description, updated_at, downloads, recent_crate_downloads, - default_version, num_versions, + default_version_num, + crate_size, + bin_names, num_rev_deps, )| { - let versions = versions_by_crate_id.get(&crate_id); - let default_version = - versions.and_then(|versions| versions.iter().find(|v| v.id == default_version)); AdminCrateInfo { name, description, @@ -114,11 +101,9 @@ pub async fn list( + recent_crate_downloads.unwrap_or_default(), num_rev_deps, num_versions: num_versions.unwrap_or_default() as usize, - default_version_num: default_version.map(|v| v.num.clone()).unwrap_or_default(), - crate_size: default_version.map(|v| v.crate_size).unwrap_or(0), - bin_names: default_version - .map(|v| v.bin_names.clone()) - .unwrap_or_default(), + default_version_num, + crate_size, + bin_names, } }, ) From b22def0bbca6b354e2748f1e561c04ada5aa153d Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Tue, 8 Jul 2025 13:55:51 -0400 Subject: [PATCH 08/11] Include whether the default version is yanked Which is only true if all versions are yanked, because otherwise a different version would be the default --- src/controllers/admin.rs | 5 +++++ src/tests/mod.rs | 1 + src/tests/routes/crates/admin.rs | 2 ++ 3 files changed, 8 insertions(+) diff --git a/src/controllers/admin.rs b/src/controllers/admin.rs index a5451b67d35..fcf4463d5bc 100644 --- a/src/controllers/admin.rs +++ b/src/controllers/admin.rs @@ -48,6 +48,7 @@ pub async fn list( Option, Option, Option, + bool, String, i32, Option>>, @@ -68,6 +69,7 @@ pub async fn list( crate_downloads::downloads.nullable(), recent_crate_downloads::downloads.nullable(), default_versions::num_versions, + versions::yanked, versions::num, versions::crate_size, versions::bin_names, @@ -88,6 +90,7 @@ pub async fn list( downloads, recent_crate_downloads, num_versions, + yanked, default_version_num, crate_size, bin_names, @@ -101,6 +104,7 @@ pub async fn list( + recent_crate_downloads.unwrap_or_default(), num_rev_deps, num_versions: num_versions.unwrap_or_default() as usize, + yanked, default_version_num, crate_size, bin_names, @@ -128,6 +132,7 @@ pub struct AdminCrateInfo { pub downloads: i64, pub num_rev_deps: i64, pub num_versions: usize, + pub yanked: bool, pub default_version_num: String, pub crate_size: i32, pub bin_names: Option>>, diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 52aa5f15faf..6cd3a1bf839 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -95,6 +95,7 @@ pub struct AdminCrateInfo { description: Option, downloads: i64, num_versions: usize, + yanked: bool, default_version_num: String, num_rev_deps: i64, } diff --git a/src/tests/routes/crates/admin.rs b/src/tests/routes/crates/admin.rs index 8d3f9710ded..d3dba3a15ba 100644 --- a/src/tests/routes/crates/admin.rs +++ b/src/tests/routes/crates/admin.rs @@ -75,6 +75,7 @@ async fn index_include_yanked() -> anyhow::Result<()> { assert!(json_crate_0.description.is_none()); assert_eq!(json_crate_0.downloads, 0); assert_eq!(json_crate_0.num_versions, 2); + assert!(json_crate_0.yanked); assert_eq!(json_crate_0.default_version_num, "2.0.0"); assert_eq!(json_crate_0.num_rev_deps, 0); @@ -83,6 +84,7 @@ async fn index_include_yanked() -> anyhow::Result<()> { assert_eq!(json_crate_1.description.as_ref().unwrap(), "My Fun Crate"); assert_eq!(json_crate_1.downloads, 536); assert_eq!(json_crate_1.num_versions, 3); + assert!(!json_crate_1.yanked); assert_eq!(json_crate_1.default_version_num, "1.0.0"); assert_eq!(json_crate_1.num_rev_deps, 1); From be6571313f975a4564d0ae0888b188f79e6aff08 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Tue, 8 Jul 2025 14:00:06 -0400 Subject: [PATCH 09/11] Include whether the user email is verified rather than not including unverified emails --- src/controllers/admin.rs | 7 ++++--- src/tests/mod.rs | 1 + src/tests/routes/crates/admin.rs | 1 + 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/controllers/admin.rs b/src/controllers/admin.rs index fcf4463d5bc..06dcc350d46 100644 --- a/src/controllers/admin.rs +++ b/src/controllers/admin.rs @@ -30,7 +30,7 @@ pub async fn list( )); } - let (user, verified, email) = users::table + let (user, verified, user_email) = users::table .left_join(emails::table) .filter(users::gh_login.eq(username)) .select(( @@ -79,7 +79,6 @@ pub async fn list( .load(&mut conn) .await?; - let verified = verified.unwrap_or(false); let crates = crates .into_iter() .map( @@ -113,7 +112,8 @@ pub async fn list( ) .collect(); Ok(Json(AdminListResponse { - user_email: verified.then_some(email).flatten(), + user_email, + user_email_verified: verified.unwrap_or_default(), crates, })) } @@ -121,6 +121,7 @@ pub async fn list( #[derive(Debug, Serialize)] pub struct AdminListResponse { user_email: Option, + user_email_verified: bool, crates: Vec, } diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 6cd3a1bf839..06da818ac09 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -87,6 +87,7 @@ pub struct OkBool { #[derive(Deserialize, Debug)] pub struct AdminListResponse { user_email: Option, + user_email_verified: bool, crates: Vec, } #[derive(Deserialize, Debug)] diff --git a/src/tests/routes/crates/admin.rs b/src/tests/routes/crates/admin.rs index d3dba3a15ba..28c8c54f4a5 100644 --- a/src/tests/routes/crates/admin.rs +++ b/src/tests/routes/crates/admin.rs @@ -68,6 +68,7 @@ async fn index_include_yanked() -> anyhow::Result<()> { let json = admin.admin_list(username).await.good(); assert_eq!(json.user_email.unwrap(), "foo@example.com"); + assert!(json.user_email_verified); assert_eq!(json.crates.len(), 2); let json_crate_0 = &json.crates[0]; From f08ba7c24dc7a5c7c9a994521ffe45b1d9165b0c Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Tue, 8 Jul 2025 14:08:25 -0400 Subject: [PATCH 10/11] Extract a type to describe the select expression that's gotten complicated --- src/controllers/admin.rs | 101 ++++++++++++++++++++++----------------- 1 file changed, 57 insertions(+), 44 deletions(-) diff --git a/src/controllers/admin.rs b/src/controllers/admin.rs index 06dcc350d46..f75e350ccec 100644 --- a/src/controllers/admin.rs +++ b/src/controllers/admin.rs @@ -12,6 +12,43 @@ use diesel_async::RunQueryDsl; use http::{StatusCode, request::Parts}; use serde::Serialize; +#[derive(Debug, Queryable, Selectable)] +#[diesel(check_for_backend(diesel::pg::Pg))] +struct DatabaseCrateInfo { + #[diesel(select_expression = crates::columns::name)] + name: String, + + #[diesel(select_expression = crates::columns::description)] + description: Option, + + #[diesel(select_expression = crates::columns::updated_at)] + updated_at: DateTime, + + #[diesel(select_expression = crate_downloads::columns::downloads.nullable())] + downloads: Option, + + #[diesel(select_expression = recent_crate_downloads::columns::downloads.nullable())] + recent_crate_downloads: Option, + + #[diesel(select_expression = default_versions::columns::num_versions)] + num_versions: Option, + + #[diesel(select_expression = versions::columns::yanked)] + yanked: bool, + + #[diesel(select_expression = versions::columns::num)] + default_version_num: String, + + #[diesel(select_expression = versions::columns::crate_size)] + crate_size: i32, + + #[diesel(select_expression = versions::columns::bin_names)] + bin_names: Option>>, + + #[diesel(select_expression = rev_deps_subquery())] + num_rev_deps: i64, +} + /// Handles the `GET /api/private/admin_list/{username}` endpoint. pub async fn list( state: AppState, @@ -41,19 +78,7 @@ pub async fn list( .first::<(User, Option, Option)>(&mut conn) .await?; - let crates: Vec<( - String, - Option, - DateTime, - Option, - Option, - Option, - bool, - String, - i32, - Option>>, - i64, - )> = CrateOwner::by_owner_kind(OwnerKind::User) + let crates: Vec = CrateOwner::by_owner_kind(OwnerKind::User) .inner_join(crates::table) .left_join(crate_downloads::table.on(crates::id.eq(crate_downloads::crate_id))) .left_join( @@ -62,27 +87,15 @@ pub async fn list( .inner_join(default_versions::table.on(crates::id.eq(default_versions::crate_id))) .inner_join(versions::table.on(default_versions::version_id.eq(versions::id))) .filter(crate_owners::owner_id.eq(user.id)) - .select(( - crates::name, - crates::description, - crates::updated_at, - crate_downloads::downloads.nullable(), - recent_crate_downloads::downloads.nullable(), - default_versions::num_versions, - versions::yanked, - versions::num, - versions::crate_size, - versions::bin_names, - rev_deps_subquery(), - )) + .select(DatabaseCrateInfo::as_select()) .order(crates::name.asc()) .load(&mut conn) .await?; let crates = crates .into_iter() - .map( - |( + .map(|database_crate_info| { + let DatabaseCrateInfo { name, description, updated_at, @@ -94,22 +107,22 @@ pub async fn list( crate_size, bin_names, num_rev_deps, - )| { - AdminCrateInfo { - name, - description, - updated_at, - downloads: downloads.unwrap_or_default() - + recent_crate_downloads.unwrap_or_default(), - num_rev_deps, - num_versions: num_versions.unwrap_or_default() as usize, - yanked, - default_version_num, - crate_size, - bin_names, - } - }, - ) + } = database_crate_info; + + AdminCrateInfo { + name, + description, + updated_at, + downloads: downloads.unwrap_or_default() + + recent_crate_downloads.unwrap_or_default(), + num_rev_deps, + num_versions: num_versions.unwrap_or_default() as usize, + yanked, + default_version_num, + crate_size, + bin_names, + } + }) .collect(); Ok(Json(AdminListResponse { user_email, From e36f29bf23289cf31f732daae1fb5228e91ca8f6 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Wed, 9 Jul 2025 14:08:09 -0400 Subject: [PATCH 11/11] Use insta snapshot rather than custom type+field assertion --- src/tests/mod.rs | 16 --------- src/tests/routes/crates/admin.rs | 32 ++++------------- ...__crates__admin__index_include_yanked.snap | 35 +++++++++++++++++++ src/tests/util.rs | 6 ++-- 4 files changed, 45 insertions(+), 44 deletions(-) create mode 100644 src/tests/routes/crates/snapshots/crates_io__tests__routes__crates__admin__index_include_yanked.snap diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 06da818ac09..6fe0e62148c 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -85,22 +85,6 @@ pub struct OkBool { ok: bool, } #[derive(Deserialize, Debug)] -pub struct AdminListResponse { - user_email: Option, - user_email_verified: bool, - crates: Vec, -} -#[derive(Deserialize, Debug)] -pub struct AdminCrateInfo { - name: String, - description: Option, - downloads: i64, - num_versions: usize, - yanked: bool, - default_version_num: String, - num_rev_deps: i64, -} -#[derive(Deserialize, Debug)] pub struct OwnerResp { // server must include `ok: true` to support old cargo clients ok: bool, diff --git a/src/tests/routes/crates/admin.rs b/src/tests/routes/crates/admin.rs index 28c8c54f4a5..892715d5acd 100644 --- a/src/tests/routes/crates/admin.rs +++ b/src/tests/routes/crates/admin.rs @@ -7,20 +7,20 @@ use crate::{ }; use diesel::prelude::*; use diesel_async::RunQueryDsl; -use insta::assert_snapshot; +use insta::{assert_json_snapshot, assert_snapshot}; #[tokio::test(flavor = "multi_thread")] async fn admin_list_by_a_non_admin_fails() { let (_app, anon, user) = TestApp::init().with_user().await; - let response = anon.admin_list("anything").await; + let response = anon.admin_list::<()>("anything").await; assert_snapshot!(response.status(), @"403 Forbidden"); assert_snapshot!( response.text(), @r#"{"errors":[{"detail":"this action requires authentication"}]}"# ); - let response = user.admin_list("anything").await; + let response = user.admin_list::<()>("anything").await; assert_snapshot!(response.status(), @"403 Forbidden"); assert_snapshot!( response.text(), @@ -65,29 +65,11 @@ async fn index_include_yanked() -> anyhow::Result<()> { // Include fully yanked (all versions were yanked) crates let username = &user.gh_login; - let json = admin.admin_list(username).await.good(); + let response = admin.admin_list::<()>(username).await; - assert_eq!(json.user_email.unwrap(), "foo@example.com"); - assert!(json.user_email_verified); - assert_eq!(json.crates.len(), 2); - - let json_crate_0 = &json.crates[0]; - assert_eq!(json_crate_0.name, "all_yanked"); - assert!(json_crate_0.description.is_none()); - assert_eq!(json_crate_0.downloads, 0); - assert_eq!(json_crate_0.num_versions, 2); - assert!(json_crate_0.yanked); - assert_eq!(json_crate_0.default_version_num, "2.0.0"); - assert_eq!(json_crate_0.num_rev_deps, 0); - - let json_crate_1 = &json.crates[1]; - assert_eq!(json_crate_1.name, "unyanked"); - assert_eq!(json_crate_1.description.as_ref().unwrap(), "My Fun Crate"); - assert_eq!(json_crate_1.downloads, 536); - assert_eq!(json_crate_1.num_versions, 3); - assert!(!json_crate_1.yanked); - assert_eq!(json_crate_1.default_version_num, "1.0.0"); - assert_eq!(json_crate_1.num_rev_deps, 1); + assert_json_snapshot!(response.json(), { + ".crates[].updated_at" => "[datetime]", + }); Ok(()) } diff --git a/src/tests/routes/crates/snapshots/crates_io__tests__routes__crates__admin__index_include_yanked.snap b/src/tests/routes/crates/snapshots/crates_io__tests__routes__crates__admin__index_include_yanked.snap new file mode 100644 index 00000000000..6bc1c1257cb --- /dev/null +++ b/src/tests/routes/crates/snapshots/crates_io__tests__routes__crates__admin__index_include_yanked.snap @@ -0,0 +1,35 @@ +--- +source: src/tests/routes/crates/admin.rs +expression: response.json() +snapshot_kind: text +--- +{ + "crates": [ + { + "bin_names": null, + "crate_size": 0, + "default_version_num": "2.0.0", + "description": null, + "downloads": 0, + "name": "all_yanked", + "num_rev_deps": 0, + "num_versions": 2, + "updated_at": "[datetime]", + "yanked": true + }, + { + "bin_names": null, + "crate_size": 0, + "default_version_num": "1.0.0", + "description": "My Fun Crate", + "downloads": 536, + "name": "unyanked", + "num_rev_deps": 1, + "num_versions": 3, + "updated_at": "[datetime]", + "yanked": false + } + ], + "user_email": "foo@example.com", + "user_email_verified": true +} diff --git a/src/tests/util.rs b/src/tests/util.rs index ff8823c64d5..6b6bfae932f 100644 --- a/src/tests/util.rs +++ b/src/tests/util.rs @@ -21,8 +21,8 @@ use crate::models::{ApiToken, User}; use crate::tests::{ - AdminListResponse, CategoryListResponse, CategoryResponse, CrateList, CrateResponse, GoodCrate, - OwnerResp, OwnersResponse, VersionResponse, + CategoryListResponse, CategoryResponse, CrateList, CrateResponse, GoodCrate, OwnerResp, + OwnersResponse, VersionResponse, }; use std::future::Future; @@ -187,7 +187,7 @@ pub trait RequestHelper { } /// Request the JSON used for the admin list page - async fn admin_list(&self, owner: &str) -> Response { + async fn admin_list(&self, owner: &str) -> Response { let url = format!("/api/private/admin_list/{owner}"); self.get(&url).await }