From 30f236be74f289a7efb242715a161d2d2d94d972 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Mon, 12 Apr 2021 18:56:32 +0200 Subject: [PATCH 1/5] clarify existing crate_owner_invitations APIs are in the v1 namespace --- src/controllers/crate_owner_invitation.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/controllers/crate_owner_invitation.rs b/src/controllers/crate_owner_invitation.rs index f0193331c3..2ccf85b2f1 100644 --- a/src/controllers/crate_owner_invitation.rs +++ b/src/controllers/crate_owner_invitation.rs @@ -6,7 +6,7 @@ use crate::views::{EncodableCrateOwnerInvitation, EncodablePublicUser, Invitatio use diesel::dsl::any; use std::collections::HashMap; -/// Handles the `GET /me/crate_owner_invitations` route. +/// 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(); @@ -88,7 +88,7 @@ struct OwnerInvitation { crate_owner_invite: InvitationResponse, } -/// Handles the `PUT /me/crate_owner_invitations/:crate_id` route. +/// Handles the `PUT /api/v1/me/crate_owner_invitations/:crate_id` route. pub fn handle_invite(req: &mut dyn RequestExt) -> EndpointResult { let mut body = String::new(); req.body().read_to_string(&mut body)?; @@ -117,7 +117,7 @@ pub fn handle_invite(req: &mut dyn RequestExt) -> EndpointResult { })) } -/// Handles the `PUT /me/crate_owner_invitations/accept/:token` route. +/// Handles the `PUT /api/v1/me/crate_owner_invitations/accept/:token` route. pub fn handle_invite_with_token(req: &mut dyn RequestExt) -> EndpointResult { let config = &req.app().config; let conn = req.db_conn()?; From 2069eabe01635ce4d6a7b92f19d344046db10367 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Thu, 1 Jul 2021 15:56:28 +0200 Subject: [PATCH 2/5] helpers/pagination: Implement `enable_pages` flag --- src/controllers/helpers/pagination.rs | 68 +++++++++++++++++---------- 1 file changed, 44 insertions(+), 24 deletions(-) diff --git a/src/controllers/helpers/pagination.rs b/src/controllers/helpers/pagination.rs index c42663ce2b..573222789c 100644 --- a/src/controllers/helpers/pagination.rs +++ b/src/controllers/helpers/pagination.rs @@ -35,6 +35,7 @@ impl PaginationOptions { PaginationOptionsBuilder { limit_page_numbers: None, enable_seek: false, + enable_pages: true, } } @@ -49,6 +50,7 @@ impl PaginationOptions { pub(crate) struct PaginationOptionsBuilder { limit_page_numbers: Option>, + enable_pages: bool, enable_seek: bool, } @@ -58,6 +60,11 @@ impl PaginationOptionsBuilder { self } + pub(crate) fn enable_pages(mut self, enable: bool) -> Self { + self.enable_pages = enable; + self + } + pub(crate) fn enable_seek(mut self, enable: bool) -> Self { self.enable_seek = enable; self @@ -75,34 +82,38 @@ impl PaginationOptionsBuilder { } let page = if let Some(s) = page_param { - let numeric_page = s.parse().map_err(|e| bad_request(&e))?; - if numeric_page < 1 { - return Err(bad_request(&format_args!( - "page indexing starts from 1, page {} is invalid", - numeric_page, - ))); - } + if self.enable_pages { + let numeric_page = s.parse().map_err(|e| bad_request(&e))?; + if numeric_page < 1 { + return Err(bad_request(&format_args!( + "page indexing starts from 1, page {} is invalid", + numeric_page, + ))); + } - if numeric_page > MAX_PAGE_BEFORE_SUSPECTED_BOT { - req.log_metadata("bot", "suspected"); - } + if numeric_page > MAX_PAGE_BEFORE_SUSPECTED_BOT { + req.log_metadata("bot", "suspected"); + } - // Block large offsets for known violators of the crawler policy - if let Some(app) = self.limit_page_numbers { - let config = &app.config; - let user_agent = request_header(req, header::USER_AGENT); - if numeric_page > config.max_allowed_page_offset - && config - .page_offset_ua_blocklist - .iter() - .any(|blocked| user_agent.contains(blocked)) - { - add_custom_metadata(req, "cause", "large page offset"); - return Err(bad_request("requested page offset is too large")); + // Block large offsets for known violators of the crawler policy + if let Some(app) = self.limit_page_numbers { + let config = &app.config; + let user_agent = request_header(req, header::USER_AGENT); + if numeric_page > config.max_allowed_page_offset + && config + .page_offset_ua_blocklist + .iter() + .any(|blocked| user_agent.contains(blocked)) + { + add_custom_metadata(req, "cause", "large page offset"); + return Err(bad_request("requested page offset is too large")); + } } - } - Page::Numeric(numeric_page) + Page::Numeric(numeric_page) + } else { + return Err(bad_request("?page= is not supported for this request")); + } } else if let Some(s) = seek_param { if self.enable_seek { Page::Seek(RawSeekPayload(s.clone())) @@ -348,6 +359,15 @@ mod tests { ); } + #[test] + fn disabled_pages() { + assert_pagination_error( + PaginationOptions::builder().enable_pages(false), + "page=1", + "?page= is not supported for this request", + ); + } + #[test] fn test_seek_encode_and_decode() { // Encoding produces the results we expect From b6bacd44c95b095a9c29af6a6dd79a9a228078fe Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Thu, 1 Jul 2021 15:58:07 +0200 Subject: [PATCH 3/5] views: Rename `EncodableCrateOwnerInvitation` to `EncodableCrateOwnerInvitationV1` --- src/controllers/crate_owner_invitation.rs | 6 +++--- src/tests/owners.rs | 8 ++++---- src/views.rs | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/controllers/crate_owner_invitation.rs b/src/controllers/crate_owner_invitation.rs index 2ccf85b2f1..10a79cc6aa 100644 --- a/src/controllers/crate_owner_invitation.rs +++ b/src/controllers/crate_owner_invitation.rs @@ -2,7 +2,7 @@ use super::frontend_prelude::*; use crate::models::{CrateOwnerInvitation, User}; use crate::schema::{crate_owner_invitations, crates, users}; -use crate::views::{EncodableCrateOwnerInvitation, EncodablePublicUser, InvitationResponse}; +use crate::views::{EncodableCrateOwnerInvitationV1, EncodablePublicUser, InvitationResponse}; use diesel::dsl::any; use std::collections::HashMap; @@ -62,7 +62,7 @@ pub fn list(req: &mut dyn RequestExt) -> EndpointResult { .unwrap_or_else(|| String::from("(unknown crate name)")); let expires_at = invitation.expires_at(config); - EncodableCrateOwnerInvitation::from(invitation, inviter_name, crate_name, expires_at) + EncodableCrateOwnerInvitationV1::from(invitation, inviter_name, crate_name, expires_at) }) .collect(); @@ -74,7 +74,7 @@ pub fn list(req: &mut dyn RequestExt) -> EndpointResult { #[derive(Serialize)] struct R { - crate_owner_invitations: Vec, + crate_owner_invitations: Vec, users: Vec, } Ok(req.json(&R { diff --git a/src/tests/owners.rs b/src/tests/owners.rs index d1feb1069c..03d4038e7c 100644 --- a/src/tests/owners.rs +++ b/src/tests/owners.rs @@ -8,7 +8,7 @@ use crate::{ use cargo_registry::{ models::Crate, views::{ - EncodableCrateOwnerInvitation, EncodableOwner, EncodablePublicUser, InvitationResponse, + EncodableCrateOwnerInvitationV1, EncodableOwner, EncodablePublicUser, InvitationResponse, }, Emails, }; @@ -27,7 +27,7 @@ struct UserResponse { } #[derive(Deserialize, Debug, PartialEq, Eq)] struct InvitationListResponse { - crate_owner_invitations: Vec, + crate_owner_invitations: Vec, users: Vec, } @@ -447,7 +447,7 @@ fn invitations_list() { assert_eq!( invitations, InvitationListResponse { - crate_owner_invitations: vec![EncodableCrateOwnerInvitation { + crate_owner_invitations: vec![EncodableCrateOwnerInvitationV1 { crate_id: krate.id, crate_name: krate.name, invited_by_username: owner.gh_login.clone(), @@ -482,7 +482,7 @@ fn invitations_list_does_not_include_expired_invites() { assert_eq!( invitations, InvitationListResponse { - crate_owner_invitations: vec![EncodableCrateOwnerInvitation { + crate_owner_invitations: vec![EncodableCrateOwnerInvitationV1 { crate_id: krate2.id, crate_name: krate2.name, invited_by_username: owner.gh_login.clone(), diff --git a/src/views.rs b/src/views.rs index b15967ce12..6fd182d186 100644 --- a/src/views.rs +++ b/src/views.rs @@ -74,7 +74,7 @@ pub struct EncodableCategoryWithSubcategories { /// The serialization format for the `CrateOwnerInvitation` model. #[derive(Deserialize, Serialize, Debug, PartialEq, Eq)] -pub struct EncodableCrateOwnerInvitation { +pub struct EncodableCrateOwnerInvitationV1 { pub invitee_id: i32, pub inviter_id: i32, pub invited_by_username: String, @@ -86,7 +86,7 @@ pub struct EncodableCrateOwnerInvitation { pub expires_at: NaiveDateTime, } -impl EncodableCrateOwnerInvitation { +impl EncodableCrateOwnerInvitationV1 { pub fn from( invitation: CrateOwnerInvitation, inviter_name: String, @@ -801,7 +801,7 @@ mod tests { #[test] fn crate_owner_invitation_serializes_to_rfc3339() { - let inv = EncodableCrateOwnerInvitation { + let inv = EncodableCrateOwnerInvitationV1 { invitee_id: 1, inviter_id: 2, invited_by_username: "".to_string(), From cccfc23f3461988555c2cb7471dba9d031bc75e2 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Thu, 1 Jul 2021 16:00:01 +0200 Subject: [PATCH 4/5] tests/owners: Add `v1` suffix to invitation tests --- src/tests/owners.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tests/owners.rs b/src/tests/owners.rs index 03d4038e7c..6d1ef6cff2 100644 --- a/src/tests/owners.rs +++ b/src/tests/owners.rs @@ -414,7 +414,7 @@ fn deleted_ownership_isnt_in_owner_user() { } #[test] -fn invitations_are_empty_by_default() { +fn invitations_are_empty_by_default_v1() { let (_, _, user) = TestApp::init().with_user(); let json = user.list_invitations(); @@ -422,7 +422,7 @@ fn invitations_are_empty_by_default() { } #[test] -fn api_token_cannot_list_invitations() { +fn api_token_cannot_list_invitations_v1() { let (_, _, _, token) = TestApp::init().with_token(); token @@ -431,7 +431,7 @@ fn api_token_cannot_list_invitations() { } #[test] -fn invitations_list() { +fn invitations_list_v1() { let (app, _, owner, token) = TestApp::init().with_token(); let owner = owner.as_model(); @@ -464,7 +464,7 @@ fn invitations_list() { } #[test] -fn invitations_list_does_not_include_expired_invites() { +fn invitations_list_does_not_include_expired_invites_v1() { let (app, _, owner, token) = TestApp::init().with_token(); let owner = owner.as_model(); From 2d1ac260cfc69c9228c760054427873a6774157e Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Tue, 29 Jun 2021 11:47:24 +0200 Subject: [PATCH 5/5] add the /api/private/crate-owner-invitations endpoint The endpoint provides a listing of all the invitations sent to the current user or all the invitations to a crate the user owns. Unauthenticated users or unrelated users won't be able to see others' invitations to prevent abuses. The two ways to query the endpoint are: GET /api/private/crate-owner-invitations?crate_name={name} GET /api/private/crate-owner-invitations?invitee_id={uid} The endpoint is paginated using only seek-based pagination, and the next page field is provided when more results are available. Once the frontend switches to use the new endpoint we can remove safely remove the old "v1" endpoint, as that's only used for the frontend. Because of this, the "v1" endpoint internally uses the same logic as the new one and converts the data to the old schema. --- src/controllers/crate_owner_invitation.rs | 292 +++++++++++++++---- src/router.rs | 6 + src/tests/owners.rs | 333 +++++++++++++++++++++- src/views.rs | 12 + 4 files changed, 577 insertions(+), 66 deletions(-) diff --git a/src/controllers/crate_owner_invitation.rs b/src/controllers/crate_owner_invitation.rs index 10a79cc6aa..2b90a43f41 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 0ca45bc07c..28724f38c1 100644 --- a/src/router.rs +++ b/src/router.rs @@ -127,6 +127,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 6d1ef6cff2..b220ac7cac 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.into_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 6fd182d186..43b8aa06c0 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,