Skip to content

Commit fa16340

Browse files
committed
Require verified email address to accept a crate ownership invitation
1 parent 46b5393 commit fa16340

File tree

3 files changed

+76
-8
lines changed

3 files changed

+76
-8
lines changed

crates/crates_io_database/src/models/crate_owner_invitation.rs

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ use diesel_async::scoped_futures::ScopedFutureExt;
44
use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl};
55
use secrecy::SecretString;
66

7-
use crate::models::CrateOwner;
8-
use crate::schema::{crate_owner_invitations, crates};
7+
use crate::models::{CrateOwner, User};
8+
use crate::schema::{crate_owner_invitations, crates, users};
99

1010
#[derive(Debug)]
1111
pub enum NewCrateOwnerInvitationOutcome {
@@ -89,16 +89,25 @@ impl CrateOwnerInvitation {
8989
}
9090

9191
pub async fn accept(self, conn: &mut AsyncPgConnection) -> Result<(), AcceptError> {
92-
if self.is_expired() {
93-
let crate_name: String = crates::table
94-
.find(self.crate_id)
95-
.select(crates::name)
96-
.first(conn)
97-
.await?;
92+
// Get the crate name for error messages
93+
let crate_name: String = crates::table
94+
.find(self.crate_id)
95+
.select(crates::name)
96+
.first(conn)
97+
.await?;
9898

99+
if self.is_expired() {
99100
return Err(AcceptError::Expired { crate_name });
100101
}
101102

103+
// Get the user and check if they have a verified email
104+
let user: User = users::table.find(self.invited_user_id).first(conn).await?;
105+
106+
let verified_email = user.verified_email(conn).await?;
107+
if verified_email.is_none() {
108+
return Err(AcceptError::EmailNotVerified { crate_name });
109+
}
110+
102111
conn.transaction(|conn| {
103112
async move {
104113
CrateOwner::from_invite(&self).insert(conn).await?;
@@ -132,4 +141,6 @@ pub enum AcceptError {
132141
Diesel(#[from] diesel::result::Error),
133142
#[error("The invitation has expired")]
134143
Expired { crate_name: String },
144+
#[error("Email verification required")]
145+
EmailNotVerified { crate_name: String },
135146
}

src/controllers/crate_owner_invitation.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,14 @@ impl From<AcceptError> for BoxedAppError {
415415

416416
custom(StatusCode::GONE, detail)
417417
}
418+
AcceptError::EmailNotVerified { crate_name } => {
419+
let detail = format!(
420+
"You need to verify your email address before you can accept the invitation \
421+
to become an owner of the {crate_name} crate.",
422+
);
423+
424+
custom(StatusCode::FORBIDDEN, detail)
425+
}
418426
}
419427
}
420428
}

src/tests/owners.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::models::Crate;
2+
use crate::schema::emails;
23
use crate::tests::builders::{CrateBuilder, PublishBuilder};
34
use crate::tests::util::{
45
MockAnonymousUser, MockCookieUser, MockTokenUser, RequestHelper, Response,
@@ -710,6 +711,54 @@ async fn test_decline_expired_invitation() {
710711
assert_eq!(json.users.len(), 1);
711712
}
712713

714+
/// Given a user inviting a different user to be a crate
715+
/// owner, check that the user invited cannot accept their
716+
/// invitation if they don't have a verified email address.
717+
#[tokio::test(flavor = "multi_thread")]
718+
async fn test_accept_invitation_without_verified_email() {
719+
let (app, anon, owner, owner_token) = TestApp::init().with_token().await;
720+
let mut conn = app.db_conn().await;
721+
let owner = owner.as_model();
722+
723+
// Create a user with a verified email (default behavior of db_new_user)
724+
let invited_user = app.db_new_user("user_unverified").await;
725+
726+
// Update the email to be unverified
727+
diesel::update(emails::table)
728+
.filter(emails::user_id.eq(invited_user.as_model().id))
729+
.set(emails::verified.eq(false))
730+
.execute(&mut conn)
731+
.await
732+
.unwrap();
733+
734+
let krate = CrateBuilder::new("foo", owner.id)
735+
.expect_build(&mut conn)
736+
.await;
737+
738+
// Invite the unverified user
739+
owner_token
740+
.add_named_owner("foo", "user_unverified")
741+
.await
742+
.good();
743+
744+
// Attempt to accept the invitation - this should fail
745+
let response = invited_user
746+
.try_accept_ownership_invitation::<()>(&krate.name, krate.id)
747+
.await;
748+
749+
// Verify that the response is a 403 Forbidden with the expected error message
750+
assert_snapshot!(response.status(), @"403 Forbidden");
751+
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."}]}"#);
752+
753+
// Verify that the invitation still exists
754+
let json = invited_user.list_invitations().await;
755+
assert_eq!(json.crate_owner_invitations.len(), 1);
756+
757+
// Verify that the user is not listed as an owner
758+
let json = anon.show_crate_owners("foo").await;
759+
assert_eq!(json.users.len(), 1);
760+
}
761+
713762
#[tokio::test(flavor = "multi_thread")]
714763
async fn test_accept_expired_invitation_by_mail() {
715764
let (app, anon, owner, owner_token) = TestApp::init().with_token().await;

0 commit comments

Comments
 (0)