From b38d55f486ec4905653b7f1ed6253ca1c8aedcb5 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Tue, 3 Jan 2023 14:34:55 +0900 Subject: [PATCH 1/3] Check email block list when email sign-up --- plume-models/src/blocklisted_emails.rs | 8 +-- plume-models/src/email_signups.rs | 11 ++++ plume-models/src/follows.rs | 7 +-- plume-models/src/posts.rs | 3 +- plume-models/src/timeline/mod.rs | 16 +---- src/routes/email_signups.rs | 85 ++++++++++++++++++++++++-- 6 files changed, 99 insertions(+), 31 deletions(-) diff --git a/plume-models/src/blocklisted_emails.rs b/plume-models/src/blocklisted_emails.rs index a6889244..ce258c85 100644 --- a/plume-models/src/blocklisted_emails.rs +++ b/plume-models/src/blocklisted_emails.rs @@ -126,11 +126,9 @@ pub(crate) mod tests { .id, various[1].id ); - assert!( - BlocklistedEmail::matches_blocklist(&conn, no_match) - .unwrap() - .is_none() - ); + assert!(BlocklistedEmail::matches_blocklist(&conn, no_match) + .unwrap() + .is_none()); Ok(()) }); } diff --git a/plume-models/src/email_signups.rs b/plume-models/src/email_signups.rs index 6867e7c7..78de4cf9 100644 --- a/plume-models/src/email_signups.rs +++ b/plume-models/src/email_signups.rs @@ -1,4 +1,5 @@ use crate::{ + blocklisted_emails::BlocklistedEmail, db_conn::DbConn, schema::email_signups, users::{NewUser, Role, User}, @@ -60,6 +61,10 @@ pub struct NewEmailSignup<'a> { impl EmailSignup { pub fn start(conn: &DbConn, email: &str) -> Result { + if let Some(x) = BlocklistedEmail::matches_blocklist(conn, email)? { + return Err(Error::Blocklisted(x.notify_user, x.notification_text)); + } + conn.transaction(|| { Self::ensure_user_not_exist_by_email(conn, email)?; let _rows = Self::delete_existings_by_email(conn, email)?; @@ -90,6 +95,9 @@ impl EmailSignup { } pub fn confirm(&self, conn: &DbConn) -> Result<()> { + if let Some(x) = BlocklistedEmail::matches_blocklist(conn, &self.email)? { + return Err(Error::Blocklisted(x.notify_user, x.notification_text)); + } conn.transaction(|| { Self::ensure_user_not_exist_by_email(conn, &self.email)?; if self.expired() { @@ -101,6 +109,9 @@ impl EmailSignup { } pub fn complete(&self, conn: &DbConn, username: String, password: String) -> Result { + if let Some(x) = BlocklistedEmail::matches_blocklist(conn, &self.email)? { + return Err(Error::Blocklisted(x.notify_user, x.notification_text)); + } conn.transaction(|| { Self::ensure_user_not_exist_by_email(conn, &self.email)?; let user = NewUser::new_local( diff --git a/plume-models/src/follows.rs b/plume-models/src/follows.rs index 1d47b2d5..2be7e966 100644 --- a/plume-models/src/follows.rs +++ b/plume-models/src/follows.rs @@ -107,12 +107,7 @@ impl Follow { res.notify(conn)?; let accept = res.build_accept(from, target, follow)?; - broadcast( - target, - accept, - vec![from.clone()], - CONFIG.proxy().cloned(), - ); + broadcast(target, accept, vec![from.clone()], CONFIG.proxy().cloned()); Ok(res) } diff --git a/plume-models/src/posts.rs b/plume-models/src/posts.rs index d76015ac..4d0748e0 100644 --- a/plume-models/src/posts.rs +++ b/plume-models/src/posts.rs @@ -133,7 +133,8 @@ impl Post { .filter(posts::id.eq_any(ids)) .filter(posts::published.eq(true)) .count() - .load(conn)?.first() + .load(conn)? + .first() .cloned() .ok_or(Error::NotFound) } diff --git a/plume-models/src/timeline/mod.rs b/plume-models/src/timeline/mod.rs index a8881392..ae515e97 100644 --- a/plume-models/src/timeline/mod.rs +++ b/plume-models/src/timeline/mod.rs @@ -291,13 +291,7 @@ mod tests { "all".to_owned(), ) .unwrap(); - List::new( - conn, - "languages I speak", - Some(&users[1]), - ListType::Prefix, - ) - .unwrap(); + List::new(conn, "languages I speak", Some(&users[1]), ListType::Prefix).unwrap(); let tl2_u1 = Timeline::new_for_user( conn, users[0].id, @@ -337,18 +331,14 @@ mod tests { let tl_instance = Timeline::list_for_user(conn, None).unwrap(); assert_eq!(3, tl_instance.len()); // there are also the local and federated feed by default - assert!(tl_instance - .iter() - .any(|tl| *tl == tl1_instance)); + assert!(tl_instance.iter().any(|tl| *tl == tl1_instance)); tl1_u1.name = "My Super TL".to_owned(); let new_tl1_u2 = tl1_u2.update(conn).unwrap(); let tl_u2 = Timeline::list_for_user(conn, Some(users[1].id)).unwrap(); assert_eq!(2, tl_u2.len()); // same here - assert!(tl_u2 - .iter() - .any(|tl| *tl == new_tl1_u2)); + assert!(tl_u2.iter().any(|tl| *tl == new_tl1_u2)); Ok(()) }); diff --git a/src/routes/email_signups.rs b/src/routes/email_signups.rs index 7a364dd0..c36b7a91 100644 --- a/src/routes/email_signups.rs +++ b/src/routes/email_signups.rs @@ -3,6 +3,7 @@ use crate::{ routes::{errors::ErrorPage, RespondOrRedirect}, template_utils::{IntoContext, Ructe}, }; + use plume_models::{ db_conn::DbConn, email_signups::EmailSignup, instance::Instance, lettre::Transport, signups, Error, PlumeRocket, CONFIG, @@ -13,7 +14,11 @@ use rocket::{ response::{Flash, Redirect}, State, }; -use std::sync::{Arc, Mutex}; +use std::{ + borrow::Cow, + collections::HashMap, + sync::{Arc, Mutex}, +}; use tracing::warn; use validator::{Validate, ValidationError, ValidationErrors}; @@ -105,6 +110,26 @@ pub fn create( render!(email_signups::create(&(&conn, &rockets).to_context())).into() } Error::NotFound => render!(errors::not_found(&(&conn, &rockets).to_context())).into(), + Error::Blocklisted(show, msg) => { + let mut errors = ValidationErrors::new(); + if *show { + errors.add( + "email", + ValidationError { + code: Cow::from("blocklisted"), + message: Some(Cow::from(msg.clone())), + params: HashMap::new(), + }, + ); + } + render!(email_signups::new( + &(&conn, &rockets).to_context(), + registration_open, + &form, + errors + )) + .into() + } _ => render!(errors::not_found(&(&conn, &rockets).to_context())).into(), // FIXME }); } @@ -153,6 +178,28 @@ pub fn show( ))) } // TODO: Flash and redirect Error::NotFound => return Err(Error::NotFound.into()), + Error::Blocklisted(show, msg) => { + let mut errors = ValidationErrors::new(); + if show { + errors.add( + "email", + ValidationError { + code: Cow::from("blocklisted"), + message: Some(Cow::from(msg)), + params: HashMap::new(), + }, + ); + } + return Ok(render!(email_signups::new( + &(&conn, &rockets).to_context(), + Instance::get_local()?.open_registrations, + &EmailSignupForm { + email: signup.email.clone(), + email_confirmation: signup.email + }, + errors + ))); + } _ => return Err(Error::NotFound.into()), // FIXME } } @@ -207,12 +254,38 @@ pub fn signup( err )))); } - let _user = signup - .complete(&conn, form.username.clone(), form.password.clone()) - .map_err(|e| { + let user = signup.complete(&conn, form.username.clone(), form.password.clone()); + match user { + Err(Error::Blocklisted(show, msg)) => { + let instance = Instance::get_local().map_err(|_| Status::UnprocessableEntity)?; + let mut errors = ValidationErrors::new(); + if show { + errors.add( + "email", + ValidationError { + code: Cow::from("blocklisted"), + message: Some(Cow::from(msg)), + params: HashMap::new(), + }, + ); + } + return Ok(render!(email_signups::new( + &(&conn, &rockets).to_context(), + instance.open_registrations, + &EmailSignupForm { + email: signup.email.clone(), + email_confirmation: signup.email + }, + errors + )) + .into()); + } + Err(e) => { warn!("{:?}", e); - Status::UnprocessableEntity - })?; + return Err(Status::UnprocessableEntity); + } + _ => {} + } Ok(FlashRedirect(Flash::success( Redirect::to(uri!(super::session::new: m = _)), i18n!( From 3b3148fa6b5618d3cd31eda9cde751676f07cf4f Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Tue, 3 Jan 2023 17:41:31 +0900 Subject: [PATCH 2/3] Clippy --- src/routes/blogs.rs | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/routes/blogs.rs b/src/routes/blogs.rs index bff25d01..295d08a8 100644 --- a/src/routes/blogs.rs +++ b/src/routes/blogs.rs @@ -463,20 +463,24 @@ mod tests { Instance::cache_local(conn); instance }); - let mut user = NewUser::default(); - user.instance_id = instance.id; - user.username = random_hex(); - user.ap_url = random_hex(); - user.inbox_url = random_hex(); - user.outbox_url = random_hex(); - user.followers_endpoint = random_hex(); + let user = NewUser { + instance_id: instance.id, + username: random_hex(), + ap_url: random_hex(), + inbox_url: random_hex(), + outbox_url: random_hex(), + followers_endpoint: random_hex(), + ..Default::default() + }; let user = User::insert(conn, user).unwrap(); - let mut blog = NewBlog::default(); - blog.instance_id = instance.id; - blog.actor_id = random_hex(); - blog.ap_url = random_hex(); - blog.inbox_url = random_hex(); - blog.outbox_url = random_hex(); + let blog = NewBlog { + instance_id: instance.id, + actor_id: random_hex(), + ap_url: random_hex(), + inbox_url: random_hex(), + outbox_url: random_hex(), + ..Default::default() + }; let blog = Blog::insert(conn, blog).unwrap(); BlogAuthor::insert( conn, From 832479a7068435b560cb4c8f08af925a73794388 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Tue, 3 Jan 2023 17:49:15 +0900 Subject: [PATCH 3/3] Extract EmailSingup::ensure_email_not_blocked() --- plume-models/src/email_signups.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/plume-models/src/email_signups.rs b/plume-models/src/email_signups.rs index 78de4cf9..55fa1646 100644 --- a/plume-models/src/email_signups.rs +++ b/plume-models/src/email_signups.rs @@ -61,9 +61,7 @@ pub struct NewEmailSignup<'a> { impl EmailSignup { pub fn start(conn: &DbConn, email: &str) -> Result { - if let Some(x) = BlocklistedEmail::matches_blocklist(conn, email)? { - return Err(Error::Blocklisted(x.notify_user, x.notification_text)); - } + Self::ensure_email_not_blocked(conn, email)?; conn.transaction(|| { Self::ensure_user_not_exist_by_email(conn, email)?; @@ -95,9 +93,8 @@ impl EmailSignup { } pub fn confirm(&self, conn: &DbConn) -> Result<()> { - if let Some(x) = BlocklistedEmail::matches_blocklist(conn, &self.email)? { - return Err(Error::Blocklisted(x.notify_user, x.notification_text)); - } + Self::ensure_email_not_blocked(conn, &self.email)?; + conn.transaction(|| { Self::ensure_user_not_exist_by_email(conn, &self.email)?; if self.expired() { @@ -109,9 +106,8 @@ impl EmailSignup { } pub fn complete(&self, conn: &DbConn, username: String, password: String) -> Result { - if let Some(x) = BlocklistedEmail::matches_blocklist(conn, &self.email)? { - return Err(Error::Blocklisted(x.notify_user, x.notification_text)); - } + Self::ensure_email_not_blocked(conn, &self.email)?; + conn.transaction(|| { Self::ensure_user_not_exist_by_email(conn, &self.email)?; let user = NewUser::new_local( @@ -133,6 +129,14 @@ impl EmailSignup { Ok(()) } + fn ensure_email_not_blocked(conn: &DbConn, email: &str) -> Result<()> { + if let Some(x) = BlocklistedEmail::matches_blocklist(conn, email)? { + Err(Error::Blocklisted(x.notify_user, x.notification_text)) + } else { + Ok(()) + } + } + fn ensure_user_not_exist_by_email(conn: &DbConn, email: &str) -> Result<()> { if User::email_used(conn, email)? { let _rows = Self::delete_existings_by_email(conn, email)?;