From 9b9e6ea269caedc800676ae080802f90569442b9 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sun, 22 Jun 2025 14:06:16 +0200 Subject: [PATCH] Require verified email address to accept a crate ownership invitation --- .../src/models/crate_owner_invitation.rs | 24 +++++++-- src/controllers/crate_owner_invitation.rs | 8 +++ src/tests/owners.rs | 49 +++++++++++++++++++ 3 files changed, 76 insertions(+), 5 deletions(-) diff --git a/crates/crates_io_database/src/models/crate_owner_invitation.rs b/crates/crates_io_database/src/models/crate_owner_invitation.rs index 24dd375e72f..a2bb63ebf38 100644 --- a/crates/crates_io_database/src/models/crate_owner_invitation.rs +++ b/crates/crates_io_database/src/models/crate_owner_invitation.rs @@ -4,8 +4,8 @@ use diesel_async::scoped_futures::ScopedFutureExt; use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl}; use secrecy::SecretString; -use crate::models::CrateOwner; -use crate::schema::{crate_owner_invitations, crates}; +use crate::models::{CrateOwner, User}; +use crate::schema::{crate_owner_invitations, crates, users}; #[derive(Debug)] pub enum NewCrateOwnerInvitationOutcome { @@ -89,16 +89,28 @@ impl CrateOwnerInvitation { } pub async fn accept(self, conn: &mut AsyncPgConnection) -> Result<(), AcceptError> { - if self.is_expired() { - let crate_name: String = crates::table + let get_crate_name = async |conn| { + crates::table .find(self.crate_id) .select(crates::name) .first(conn) - .await?; + .await + }; + if self.is_expired() { + let crate_name = get_crate_name(conn).await?; return Err(AcceptError::Expired { crate_name }); } + // Get the user and check if they have a verified email + let user: User = users::table.find(self.invited_user_id).first(conn).await?; + + let verified_email = user.verified_email(conn).await?; + if verified_email.is_none() { + let crate_name = get_crate_name(conn).await?; + return Err(AcceptError::EmailNotVerified { crate_name }); + } + conn.transaction(|conn| { async move { CrateOwner::from_invite(&self).insert(conn).await?; @@ -132,4 +144,6 @@ pub enum AcceptError { Diesel(#[from] diesel::result::Error), #[error("The invitation has expired")] Expired { crate_name: String }, + #[error("Email verification required")] + EmailNotVerified { crate_name: String }, } diff --git a/src/controllers/crate_owner_invitation.rs b/src/controllers/crate_owner_invitation.rs index 24547c0e650..014a0bc577b 100644 --- a/src/controllers/crate_owner_invitation.rs +++ b/src/controllers/crate_owner_invitation.rs @@ -415,6 +415,14 @@ impl From for BoxedAppError { custom(StatusCode::GONE, detail) } + AcceptError::EmailNotVerified { crate_name } => { + let detail = format!( + "You need to verify your email address before you can accept the invitation \ + to become an owner of the {crate_name} crate.", + ); + + custom(StatusCode::FORBIDDEN, detail) + } } } } diff --git a/src/tests/owners.rs b/src/tests/owners.rs index 3c81801bf3d..8777b66ac3b 100644 --- a/src/tests/owners.rs +++ b/src/tests/owners.rs @@ -1,4 +1,5 @@ use crate::models::Crate; +use crate::schema::emails; use crate::tests::builders::{CrateBuilder, PublishBuilder}; use crate::tests::util::{ MockAnonymousUser, MockCookieUser, MockTokenUser, RequestHelper, Response, @@ -710,6 +711,54 @@ async fn test_decline_expired_invitation() { assert_eq!(json.users.len(), 1); } +/// Given a user inviting a different user to be a crate +/// owner, check that the user invited cannot accept their +/// invitation if they don't have a verified email address. +#[tokio::test(flavor = "multi_thread")] +async fn test_accept_invitation_without_verified_email() { + let (app, anon, owner, owner_token) = TestApp::init().with_token().await; + let mut conn = app.db_conn().await; + let owner = owner.as_model(); + + // Create a user with a verified email (default behavior of db_new_user) + let invited_user = app.db_new_user("user_unverified").await; + + // Update the email to be unverified + diesel::update(emails::table) + .filter(emails::user_id.eq(invited_user.as_model().id)) + .set(emails::verified.eq(false)) + .execute(&mut conn) + .await + .unwrap(); + + let krate = CrateBuilder::new("foo", owner.id) + .expect_build(&mut conn) + .await; + + // Invite the unverified user + owner_token + .add_named_owner("foo", "user_unverified") + .await + .good(); + + // Attempt to accept the invitation - this should fail + let response = invited_user + .try_accept_ownership_invitation::<()>(&krate.name, krate.id) + .await; + + // Verify that the response is a 403 Forbidden with the expected error message + assert_snapshot!(response.status(), @"403 Forbidden"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"You need to verify your email address before you can accept the invitation to become an owner of the foo crate."}]}"#); + + // Verify that the invitation still exists + let json = invited_user.list_invitations().await; + assert_eq!(json.crate_owner_invitations.len(), 1); + + // Verify that the user is not listed as an owner + let json = anon.show_crate_owners("foo").await; + assert_eq!(json.users.len(), 1); +} + #[tokio::test(flavor = "multi_thread")] async fn test_accept_expired_invitation_by_mail() { let (app, anon, owner, owner_token) = TestApp::init().with_token().await;