diff --git a/src/controllers/crate_owner_invitation.rs b/src/controllers/crate_owner_invitation.rs index 10a79cc6aa0..2b90a43f419 100644 --- a/src/controllers/crate_owner_invitation.rs +++ b/src/controllers/crate_owner_invitation.rs @@ -1,76 +1,48 @@ use super::frontend_prelude::*; -use crate::models::{CrateOwnerInvitation, User}; +use crate::controllers::helpers::pagination::{Page, PaginationOptions}; +use crate::controllers::util::AuthenticatedUser; +use crate::models::{Crate, CrateOwnerInvitation, Rights, User}; use crate::schema::{crate_owner_invitations, crates, users}; -use crate::views::{EncodableCrateOwnerInvitationV1, EncodablePublicUser, InvitationResponse}; -use diesel::dsl::any; -use std::collections::HashMap; +use crate::util::errors::{forbidden, internal}; +use crate::views::{ + EncodableCrateOwnerInvitation, EncodableCrateOwnerInvitationV1, EncodablePublicUser, + InvitationResponse, +}; +use chrono::{Duration, Utc}; +use diesel::{pg::Pg, sql_types::Bool}; +use indexmap::IndexMap; +use std::collections::{HashMap, HashSet}; /// Handles the `GET /api/v1/me/crate_owner_invitations` route. pub fn list(req: &mut dyn RequestExt) -> EndpointResult { - // Ensure that the user is authenticated - let user = req.authenticate()?.forbid_api_token_auth()?.user(); + let auth = req.authenticate()?.forbid_api_token_auth()?; + let user_id = auth.user_id(); - // Load all pending invitations for the user - let conn = &*req.db_read_only()?; - let crate_owner_invitations: Vec = crate_owner_invitations::table - .filter(crate_owner_invitations::invited_user_id.eq(user.id)) - .load(&*conn)?; + let PrivateListResponse { + invitations, users, .. + } = prepare_list(req, auth, ListFilter::InviteeId(user_id))?; - // Make a list of all related users - let user_ids: Vec<_> = crate_owner_invitations - .iter() - .map(|invitation| invitation.invited_by_user_id) - .collect(); - - // Load all related users - let users: Vec = users::table - .filter(users::id.eq(any(user_ids))) - .load(conn)?; - - let users: HashMap = users.into_iter().map(|user| (user.id, user)).collect(); - - // Make a list of all related crates - let crate_ids: Vec<_> = crate_owner_invitations - .iter() - .map(|invitation| invitation.crate_id) - .collect(); - - // Load all related crates - let crates: Vec<_> = crates::table - .select((crates::id, crates::name)) - .filter(crates::id.eq(any(crate_ids))) - .load(conn)?; - - let crates: HashMap = crates.into_iter().collect(); - - // Turn `CrateOwnerInvitation` list into `EncodableCrateOwnerInvitation` list - let config = &req.app().config; - let crate_owner_invitations = crate_owner_invitations + // The schema for the private endpoints is converted to the schema used by v1 endpoints. + let crate_owner_invitations = invitations .into_iter() - .filter(|i| !i.is_expired(config)) - .map(|invitation| { - let inviter_id = invitation.invited_by_user_id; - let inviter_name = users - .get(&inviter_id) - .map(|user| user.gh_login.clone()) - .unwrap_or_default(); - - let crate_name = crates - .get(&invitation.crate_id) - .cloned() - .unwrap_or_else(|| String::from("(unknown crate name)")); - - let expires_at = invitation.expires_at(config); - EncodableCrateOwnerInvitationV1::from(invitation, inviter_name, crate_name, expires_at) + .map(|private| { + Ok(EncodableCrateOwnerInvitationV1 { + invited_by_username: users + .iter() + .find(|u| u.id == private.inviter_id) + .ok_or_else(|| internal(&format!("missing user {}", private.inviter_id)))? + .login + .clone(), + invitee_id: private.invitee_id, + inviter_id: private.inviter_id, + crate_name: private.crate_name, + crate_id: private.crate_id, + created_at: private.created_at, + expires_at: private.expires_at, + }) }) - .collect(); - - // Turn `User` list into `EncodablePublicUser` list - let users = users - .into_iter() - .map(|(_, user)| EncodablePublicUser::from(user)) - .collect(); + .collect::>>()?; #[derive(Serialize)] struct R { @@ -83,6 +55,200 @@ pub fn list(req: &mut dyn RequestExt) -> EndpointResult { })) } +/// Handles the `GET /api/private/crate-owner-invitations` route. +pub fn private_list(req: &mut dyn RequestExt) -> EndpointResult { + let auth = req.authenticate()?.forbid_api_token_auth()?; + + let filter = if let Some(crate_name) = req.query().get("crate_name") { + ListFilter::CrateName(crate_name.clone()) + } else if let Some(id) = req.query().get("invitee_id").and_then(|i| i.parse().ok()) { + ListFilter::InviteeId(id) + } else { + return Err(bad_request("missing or invalid filter")); + }; + + let list = prepare_list(req, auth, filter)?; + Ok(req.json(&list)) +} + +enum ListFilter { + CrateName(String), + InviteeId(i32), +} + +fn prepare_list( + req: &mut dyn RequestExt, + auth: AuthenticatedUser, + filter: ListFilter, +) -> AppResult { + let pagination: PaginationOptions = PaginationOptions::builder() + .enable_pages(false) + .enable_seek(true) + .gather(req)?; + + let user = auth.user(); + let conn = req.db_read_only()?; + let config = &req.app().config; + + let mut crate_names = HashMap::new(); + let mut users = IndexMap::new(); + users.insert(user.id, user.clone()); + + let sql_filter: Box> = + match filter { + ListFilter::CrateName(crate_name) => { + // Only allow crate owners to query pending invitations for their crate. + let krate: Crate = Crate::by_name(&crate_name).first(&*conn)?; + let owners = krate.owners(&*conn)?; + if user.rights(req.app(), &owners)? != Rights::Full { + return Err(forbidden()); + } + + // Cache the crate name to avoid querying it from the database again + crate_names.insert(krate.id, krate.name.clone()); + + Box::new(crate_owner_invitations::crate_id.eq(krate.id)) + } + ListFilter::InviteeId(invitee_id) => { + if invitee_id != user.id { + return Err(forbidden()); + } + Box::new(crate_owner_invitations::invited_user_id.eq(invitee_id)) + } + }; + + // Load all the non-expired invitations matching the filter. + let expire_cutoff = Duration::days(config.ownership_invitations_expiration_days as i64); + let query = crate_owner_invitations::table + .filter(sql_filter) + .filter(crate_owner_invitations::created_at.gt((Utc::now() - expire_cutoff).naive_utc())) + .order_by(( + crate_owner_invitations::crate_id, + crate_owner_invitations::invited_user_id, + )) + // We fetch one element over the page limit to then detect whether there is a next page. + .limit(pagination.per_page as i64 + 1); + + // Load and paginate the results. + let mut raw_invitations: Vec = match pagination.page { + Page::Unspecified => query.load(&*conn)?, + Page::Seek(s) => { + let seek_key: (i32, i32) = s.decode()?; + query + .filter( + crate_owner_invitations::crate_id.gt(seek_key.0).or( + crate_owner_invitations::crate_id + .eq(seek_key.0) + .and(crate_owner_invitations::invited_user_id.gt(seek_key.1)), + ), + ) + .load(&*conn)? + } + Page::Numeric(_) => unreachable!("page-based pagination is disabled"), + }; + let next_page = if raw_invitations.len() > pagination.per_page as usize { + // We fetch `per_page + 1` to check if there are records for the next page. Since the last + // element is not what the user wanted it's discarded. + raw_invitations.pop(); + + if let Some(last) = raw_invitations.last() { + let mut params = IndexMap::new(); + params.insert( + "seek".into(), + crate::controllers::helpers::pagination::encode_seek(( + last.crate_id, + last.invited_user_id, + ))?, + ); + Some(req.query_with_params(params)) + } else { + None + } + } else { + None + }; + + // Load all the related crates. + let missing_crate_names = raw_invitations + .iter() + .map(|i| i.crate_id) + .filter(|id| !crate_names.contains_key(id)) + .collect::>(); + if !missing_crate_names.is_empty() { + let new_names: Vec<(i32, String)> = crates::table + .select((crates::id, crates::name)) + .filter(crates::id.eq_any(missing_crate_names)) + .load(&*conn)?; + for (id, name) in new_names.into_iter() { + crate_names.insert(id, name); + } + } + + // Load all the related users. + let missing_users = raw_invitations + .iter() + .flat_map(|invite| { + std::iter::once(invite.invited_user_id) + .chain(std::iter::once(invite.invited_by_user_id)) + }) + .filter(|id| !users.contains_key(id)) + .collect::>(); + if !missing_users.is_empty() { + let new_users: Vec = users::table + .filter(users::id.eq_any(missing_users)) + .load(&*conn)?; + for user in new_users.into_iter() { + users.insert(user.id, user); + } + } + + // Turn `CrateOwnerInvitation`s into `EncodablePrivateCrateOwnerInvitation`. + let config = &req.app().config; + let mut invitations = Vec::new(); + let mut users_in_response = HashSet::new(); + for invitation in raw_invitations.into_iter() { + invitations.push(EncodableCrateOwnerInvitation { + invitee_id: invitation.invited_user_id, + inviter_id: invitation.invited_by_user_id, + crate_id: invitation.crate_id, + crate_name: crate_names + .get(&invitation.crate_id) + .ok_or_else(|| internal(&format!("missing crate with id {}", invitation.crate_id)))? + .clone(), + created_at: invitation.created_at, + expires_at: invitation.expires_at(config), + }); + users_in_response.insert(invitation.invited_user_id); + users_in_response.insert(invitation.invited_by_user_id); + } + + // Provide a stable response for the users list, only including the referenced users with + // stable sorting. + users.retain(|k, _| users_in_response.contains(k)); + users.sort_keys(); + + Ok(PrivateListResponse { + invitations, + users: users + .into_iter() + .map(|(_, user)| EncodablePublicUser::from(user)) + .collect(), + meta: ResponseMeta { next_page }, + }) +} + +#[derive(Serialize)] +struct PrivateListResponse { + invitations: Vec, + users: Vec, + meta: ResponseMeta, +} + +#[derive(Serialize)] +struct ResponseMeta { + next_page: Option, +} + #[derive(Deserialize)] struct OwnerInvitation { crate_owner_invite: InvitationResponse, diff --git a/src/router.rs b/src/router.rs index 91cfb509770..40175e3138d 100644 --- a/src/router.rs +++ b/src/router.rs @@ -126,6 +126,12 @@ pub fn build_router(app: &App) -> RouteBuilder { // Metrics router.get("/api/private/metrics/:kind", C(metrics::prometheus)); + // Crate ownership invitations management in the frontend + router.get( + "/api/private/crate-owner-invitations", + C(crate_owner_invitation::private_list), + ); + // Only serve the local checkout of the git index in development mode. // In production, for crates.io, cargo gets the index from // https://github.com/rust-lang/crates.io-index directly. diff --git a/src/tests/owners.rs b/src/tests/owners.rs index 1d25b4bee93..7873e24d123 100644 --- a/src/tests/owners.rs +++ b/src/tests/owners.rs @@ -8,7 +8,8 @@ use crate::{ use cargo_registry::{ models::Crate, views::{ - EncodableCrateOwnerInvitationV1, EncodableOwner, EncodablePublicUser, InvitationResponse, + EncodableCrateOwnerInvitation, EncodableCrateOwnerInvitationV1, EncodableOwner, + EncodablePublicUser, InvitationResponse, }, Emails, }; @@ -30,6 +31,16 @@ struct InvitationListResponse { crate_owner_invitations: Vec, users: Vec, } +#[derive(Deserialize, Debug, PartialEq, Eq)] +struct CrateOwnerInvitationsResponse { + invitations: Vec, + users: Vec, + meta: CrateOwnerInvitationsMeta, +} +#[derive(Deserialize, Debug, PartialEq, Eq)] +struct CrateOwnerInvitationsMeta { + next_page: Option, +} // Implementing locally for now, unless these are needed elsewhere impl MockCookieUser { @@ -458,7 +469,7 @@ fn invitations_list_v1() { // This value changes with each test run so we can't use a fixed value here expires_at: invitations.crate_owner_invitations[0].expires_at, }], - users: vec![owner.clone().into()], + users: vec![owner.clone().into(), user.as_model().clone().into()], } ); } @@ -493,7 +504,7 @@ fn invitations_list_does_not_include_expired_invites_v1() { // This value changes with each test run so we can't use a fixed value here expires_at: invitations.crate_owner_invitations[0].expires_at, }], - users: vec![owner.clone().into()], + users: vec![owner.clone().into(), user.as_model().clone().into()], } ); } @@ -758,3 +769,319 @@ fn extract_token_from_invite_email(emails: &Emails) -> String { let after_pos = before_pos + (&body[before_pos..]).find(after_token).unwrap(); body[before_pos..after_pos].to_string() } + +// +// Tests for the `GET /api/private/crate-owners-invitations` endpoint +// + +#[track_caller] +fn get_invitations(user: &MockCookieUser, query: &str) -> CrateOwnerInvitationsResponse { + user.get_with_query::( + "/api/private/crate-owner-invitations", + query, + ) + .good() +} + +#[test] +fn invitation_list() { + let (app, _, owner, token) = TestApp::init().with_token(); + + let (crate1, crate2) = app.db(|conn| { + ( + CrateBuilder::new("crate_1", owner.as_model().id).expect_build(conn), + CrateBuilder::new("crate_2", owner.as_model().id).expect_build(conn), + ) + }); + let user1 = app.db_new_user("user_1"); + let user2 = app.db_new_user("user_2"); + token.add_user_owner("crate_1", "user_1"); + token.add_user_owner("crate_1", "user_2"); + token.add_user_owner("crate_2", "user_1"); + + // user1 has invites for both crates + let invitations = get_invitations(&user1, &format!("invitee_id={}", user1.as_model().id)); + assert_eq!( + invitations, + CrateOwnerInvitationsResponse { + invitations: vec![ + EncodableCrateOwnerInvitation { + crate_id: crate1.id, + crate_name: crate1.name.clone(), + invitee_id: user1.as_model().id, + inviter_id: owner.as_model().id, + // The timestamps depend on when the test is run. + created_at: invitations.invitations[0].created_at, + expires_at: invitations.invitations[0].expires_at, + }, + EncodableCrateOwnerInvitation { + crate_id: crate2.id, + crate_name: crate2.name.clone(), + invitee_id: user1.as_model().id, + inviter_id: owner.as_model().id, + // The timestamps depend on when the test is run. + created_at: invitations.invitations[1].created_at, + expires_at: invitations.invitations[1].expires_at, + }, + ], + users: vec![ + owner.as_model().clone().into(), + user1.as_model().clone().into() + ], + meta: CrateOwnerInvitationsMeta { next_page: None }, + } + ); + + // user2 is only invited to a single crate + let invitations = get_invitations(&user2, &format!("invitee_id={}", user2.as_model().id)); + assert_eq!( + invitations, + CrateOwnerInvitationsResponse { + invitations: vec![EncodableCrateOwnerInvitation { + crate_id: crate1.id, + crate_name: crate1.name.clone(), + invitee_id: user2.as_model().id, + inviter_id: owner.as_model().id, + // The timestamps depend on when the test is run. + created_at: invitations.invitations[0].created_at, + expires_at: invitations.invitations[0].expires_at, + }], + users: vec![ + owner.as_model().clone().into(), + user2.as_model().clone().into(), + ], + meta: CrateOwnerInvitationsMeta { next_page: None }, + } + ); + + // owner has no invites + let invitations = get_invitations(&owner, &format!("invitee_id={}", owner.as_model().id)); + assert_eq!( + invitations, + CrateOwnerInvitationsResponse { + invitations: vec![], + users: vec![], + meta: CrateOwnerInvitationsMeta { next_page: None }, + } + ); + + // crate1 has two available invitations + let invitations = get_invitations(&owner, "crate_name=crate_1"); + assert_eq!( + invitations, + CrateOwnerInvitationsResponse { + invitations: vec![ + EncodableCrateOwnerInvitation { + crate_id: crate1.id, + crate_name: crate1.name.clone(), + invitee_id: user1.as_model().id, + inviter_id: owner.as_model().id, + // The timestamps depend on when the test is run. + created_at: invitations.invitations[0].created_at, + expires_at: invitations.invitations[0].expires_at, + }, + EncodableCrateOwnerInvitation { + crate_id: crate1.id, + crate_name: crate1.name, + invitee_id: user2.as_model().id, + inviter_id: owner.as_model().id, + // The timestamps depend on when the test is run. + created_at: invitations.invitations[1].created_at, + expires_at: invitations.invitations[1].expires_at, + }, + ], + users: vec![ + owner.as_model().clone().into(), + user1.as_model().clone().into(), + user2.as_model().clone().into(), + ], + meta: CrateOwnerInvitationsMeta { next_page: None }, + } + ); + + // crate2 has one available invitation + let invitations = get_invitations(&owner, "crate_name=crate_2"); + assert_eq!( + invitations, + CrateOwnerInvitationsResponse { + invitations: vec![EncodableCrateOwnerInvitation { + crate_id: crate2.id, + crate_name: crate2.name, + invitee_id: user1.as_model().id, + inviter_id: owner.as_model().id, + // The timestamps depend on when the test is run. + created_at: invitations.invitations[0].created_at, + expires_at: invitations.invitations[0].expires_at, + }], + users: vec![ + owner.as_model().clone().into(), + user1.as_model().clone().into(), + ], + meta: CrateOwnerInvitationsMeta { next_page: None }, + } + ); +} + +#[test] +fn invitations_list_does_not_include_expired_invites() { + let (app, _, owner, token) = TestApp::init().with_token(); + let user = app.db_new_user("invited_user"); + + let (crate1, crate2) = app.db(|conn| { + ( + CrateBuilder::new("crate_1", owner.as_model().id).expect_build(conn), + CrateBuilder::new("crate_2", owner.as_model().id).expect_build(conn), + ) + }); + token.add_user_owner("crate_1", "invited_user"); + token.add_user_owner("crate_2", "invited_user"); + + // Simulate one of the invitations expiring + expire_invitation(&app, crate1.id); + + // user1 has an invite just for crate 2 + let invitations = get_invitations(&user, &format!("invitee_id={}", user.as_model().id)); + assert_eq!( + invitations, + CrateOwnerInvitationsResponse { + invitations: vec![EncodableCrateOwnerInvitation { + crate_id: crate2.id, + crate_name: crate2.name, + invitee_id: user.as_model().id, + inviter_id: owner.as_model().id, + // The timestamps depend on when the test is run. + created_at: invitations.invitations[0].created_at, + expires_at: invitations.invitations[0].expires_at, + }], + users: vec![ + owner.as_model().clone().into(), + user.as_model().clone().into(), + ], + meta: CrateOwnerInvitationsMeta { next_page: None }, + } + ); +} + +#[test] +fn invitations_list_paginated() { + let (app, _, owner, token) = TestApp::init().with_token(); + let user = app.db_new_user("invited_user"); + + let (crate1, crate2) = app.db(|conn| { + ( + CrateBuilder::new("crate_1", owner.as_model().id).expect_build(conn), + CrateBuilder::new("crate_2", owner.as_model().id).expect_build(conn), + ) + }); + token.add_user_owner("crate_1", "invited_user"); + token.add_user_owner("crate_2", "invited_user"); + + // Fetch the first page of results + let invitations = get_invitations( + &user, + &format!("per_page=1&invitee_id={}", user.as_model().id), + ); + assert_eq!( + invitations, + CrateOwnerInvitationsResponse { + invitations: vec![EncodableCrateOwnerInvitation { + crate_id: crate1.id, + crate_name: crate1.name, + invitee_id: user.as_model().id, + inviter_id: owner.as_model().id, + // The timestamps depend on when the test is run. + created_at: invitations.invitations[0].created_at, + expires_at: invitations.invitations[0].expires_at, + }], + users: vec![ + owner.as_model().clone().into(), + user.as_model().clone().into(), + ], + meta: CrateOwnerInvitationsMeta { + // This unwraps and then wraps again in Some() to ensure it's not None + next_page: Some(invitations.meta.next_page.clone().unwrap()), + }, + } + ); + + // Fetch the second page of results + let invitations = get_invitations( + &user, + invitations.meta.next_page.unwrap().trim_start_matches('?'), + ); + assert_eq!( + invitations, + CrateOwnerInvitationsResponse { + invitations: vec![EncodableCrateOwnerInvitation { + crate_id: crate2.id, + crate_name: crate2.name, + invitee_id: user.as_model().id, + inviter_id: owner.as_model().id, + // The timestamps depend on when the test is run. + created_at: invitations.invitations[0].created_at, + expires_at: invitations.invitations[0].expires_at, + }], + users: vec![ + owner.as_model().clone().into(), + user.as_model().clone().into(), + ], + meta: CrateOwnerInvitationsMeta { next_page: None }, + } + ); +} + +#[test] +fn invitation_list_with_no_filter() { + let (_, _, owner, _) = TestApp::init().with_token(); + + let resp = owner.get::<()>("/api/private/crate-owner-invitations"); + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + assert_eq!( + resp.json(), + json!({ + "errors": [{ + "detail": "missing or invalid filter", + }], + }) + ); +} + +#[test] +fn invitation_list_other_users() { + let (app, _, owner, _) = TestApp::init().with_token(); + let other_user = app.db_new_user("other"); + + // Retrieving our own invitations work. + let resp = owner.get_with_query::<()>( + "/api/private/crate-owner-invitations", + &format!("invitee_id={}", owner.as_model().id), + ); + assert_eq!(resp.status(), StatusCode::OK); + + // Retrieving other users' invitations doesn't work. + let resp = owner.get_with_query::<()>( + "/api/private/crate-owner-invitations", + &format!("invitee_id={}", other_user.as_model().id), + ); + assert_eq!(resp.status(), StatusCode::FORBIDDEN); +} + +#[test] +fn invitation_list_other_crates() { + let (app, _, owner, _) = TestApp::init().with_token(); + let other_user = app.db_new_user("other"); + app.db(|conn| { + CrateBuilder::new("crate_1", owner.as_model().id).expect_build(conn); + CrateBuilder::new("crate_2", other_user.as_model().id).expect_build(conn); + }); + + // Retrieving our own invitations work. + let resp = + owner.get_with_query::<()>("/api/private/crate-owner-invitations", "crate_name=crate_1"); + assert_eq!(resp.status(), StatusCode::OK); + + // Retrieving other users' invitations doesn't work. + let resp = + owner.get_with_query::<()>("/api/private/crate-owner-invitations", "crate_name=crate_2"); + assert_eq!(resp.status(), StatusCode::FORBIDDEN); +} diff --git a/src/views.rs b/src/views.rs index 6fd182d186d..43b8aa06c02 100644 --- a/src/views.rs +++ b/src/views.rs @@ -105,6 +105,18 @@ impl EncodableCrateOwnerInvitationV1 { } } +#[derive(Deserialize, Serialize, Debug, PartialEq, Eq)] +pub struct EncodableCrateOwnerInvitation { + pub invitee_id: i32, + pub inviter_id: i32, + pub crate_id: i32, + pub crate_name: String, + #[serde(with = "rfc3339")] + pub created_at: NaiveDateTime, + #[serde(with = "rfc3339")] + pub expires_at: NaiveDateTime, +} + #[derive(Deserialize, Serialize, Debug, Copy, Clone)] pub struct InvitationResponse { pub crate_id: i32,