From 74254aed4a909dc2d69f8172df86a99146d20225 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Wed, 12 Jan 2022 04:18:13 +0900 Subject: [PATCH 01/10] Move require_logins from plume-common to plume --- plume-common/src/utils.rs | 18 +----------------- src/main.rs | 1 + src/routes/blogs.rs | 3 ++- src/routes/likes.rs | 4 ++-- src/routes/notifications.rs | 4 ++-- src/routes/posts.rs | 9 +++++---- src/routes/reshares.rs | 4 ++-- src/routes/user.rs | 13 +++++++------ src/utils.rs | 17 +++++++++++++++++ 9 files changed, 39 insertions(+), 34 deletions(-) create mode 100644 src/utils.rs diff --git a/plume-common/src/utils.rs b/plume-common/src/utils.rs index cae6504de..cbb4db727 100644 --- a/plume-common/src/utils.rs +++ b/plume-common/src/utils.rs @@ -2,10 +2,7 @@ use heck::ToUpperCamelCase; use openssl::rand::rand_bytes; use pulldown_cmark::{html, CodeBlockKind, CowStr, Event, LinkType, Options, Parser, Tag}; use regex_syntax::is_word_character; -use rocket::{ - http::uri::Uri, - response::{Flash, Redirect}, -}; +use rocket::http::uri::Uri; use std::collections::HashSet; use syntect::html::{ClassStyle, ClassedHTMLGenerator}; use syntect::parsing::SyntaxSet; @@ -80,19 +77,6 @@ pub fn iri_percent_encode_seg_char(c: char) -> String { } } -/** -* Redirects to the login page with a given message. -* -* Note that the message should be translated before passed to this function. -*/ -pub fn requires_login>>(message: &str, url: T) -> Flash { - Flash::new( - Redirect::to(format!("/login?m={}", Uri::percent_encode(message))), - "callback", - url.into().to_string(), - ) -} - #[derive(Debug)] enum State { Mention, diff --git a/src/main.rs b/src/main.rs index 4d2366613..32d181f21 100755 --- a/src/main.rs +++ b/src/main.rs @@ -33,6 +33,7 @@ init_i18n!( mod api; mod inbox; mod mail; +mod utils; #[macro_use] mod template_utils; mod routes; diff --git a/src/routes/blogs.rs b/src/routes/blogs.rs index 07a8e8f15..c56b5a6ee 100644 --- a/src/routes/blogs.rs +++ b/src/routes/blogs.rs @@ -11,6 +11,7 @@ use validator::{Validate, ValidationError, ValidationErrors}; use crate::routes::{errors::ErrorPage, Page, RespondOrRedirect}; use crate::template_utils::{IntoContext, Ructe}; +use crate::utils::requires_login; use plume_common::activity_pub::{ActivityStream, ApRequest}; use plume_common::utils; use plume_models::{ @@ -62,7 +63,7 @@ pub fn new(conn: DbConn, rockets: PlumeRocket, _user: User) -> Ructe { #[get("/blogs/new", rank = 2)] pub fn new_auth(i18n: I18n) -> Flash { - utils::requires_login( + requires_login( &i18n!( i18n.catalog, "To create a new blog, you need to be logged in" diff --git a/src/routes/likes.rs b/src/routes/likes.rs index 267bbe45f..25ca68bfd 100644 --- a/src/routes/likes.rs +++ b/src/routes/likes.rs @@ -2,8 +2,8 @@ use rocket::response::{Flash, Redirect}; use rocket_i18n::I18n; use crate::routes::errors::ErrorPage; +use crate::utils::requires_login; use plume_common::activity_pub::broadcast; -use plume_common::utils; use plume_models::{ blogs::Blog, db_conn::DbConn, inbox::inbox, likes, posts::Post, timeline::*, users::User, Error, PlumeRocket, CONFIG, @@ -54,7 +54,7 @@ pub fn create( #[post("/~///like", rank = 2)] pub fn create_auth(blog: String, slug: String, i18n: I18n) -> Flash { - utils::requires_login( + requires_login( &i18n!(i18n.catalog, "To like a post, you need to be logged in"), uri!(create: blog = blog, slug = slug), ) diff --git a/src/routes/notifications.rs b/src/routes/notifications.rs index 1b987336e..2f473ed01 100644 --- a/src/routes/notifications.rs +++ b/src/routes/notifications.rs @@ -3,7 +3,7 @@ use rocket_i18n::I18n; use crate::routes::{errors::ErrorPage, Page}; use crate::template_utils::{IntoContext, Ructe}; -use plume_common::utils; +use crate::utils::requires_login; use plume_models::{db_conn::DbConn, notifications::Notification, users::User, PlumeRocket}; #[get("/notifications?")] @@ -24,7 +24,7 @@ pub fn notifications( #[get("/notifications?", rank = 2)] pub fn notifications_auth(i18n: I18n, page: Option) -> Flash { - utils::requires_login( + requires_login( &i18n!( i18n.catalog, "To see your notifications, you need to be logged in" diff --git a/src/routes/posts.rs b/src/routes/posts.rs index de0f4541b..854a5621a 100644 --- a/src/routes/posts.rs +++ b/src/routes/posts.rs @@ -14,8 +14,9 @@ use crate::routes::{ comments::NewCommentForm, errors::ErrorPage, ContentLen, RemoteForm, RespondOrRedirect, }; use crate::template_utils::{IntoContext, Ructe}; +use crate::utils::requires_login; use plume_common::activity_pub::{broadcast, ActivityStream, ApRequest}; -use plume_common::utils; +use plume_common::utils::md_to_html; use plume_models::{ blogs::*, comments::{Comment, CommentTree}, @@ -120,7 +121,7 @@ pub fn activity_details( #[get("/~//new", rank = 2)] pub fn new_auth(blog: String, i18n: I18n) -> Flash { - utils::requires_login( + requires_login( &i18n!( i18n.catalog, "To write a new post, you need to be logged in" @@ -268,7 +269,7 @@ pub fn update( ) .into() } else { - let (content, mentions, hashtags) = utils::md_to_html( + let (content, mentions, hashtags) = md_to_html( form.content.to_string().as_ref(), Some( &Instance::get_local() @@ -452,7 +453,7 @@ pub fn create( .into()); } - let (content, mentions, hashtags) = utils::md_to_html( + let (content, mentions, hashtags) = md_to_html( form.content.to_string().as_ref(), Some( &Instance::get_local() diff --git a/src/routes/reshares.rs b/src/routes/reshares.rs index 67b2b84f1..b6d11c8a0 100644 --- a/src/routes/reshares.rs +++ b/src/routes/reshares.rs @@ -2,8 +2,8 @@ use rocket::response::{Flash, Redirect}; use rocket_i18n::I18n; use crate::routes::errors::ErrorPage; +use crate::utils::requires_login; use plume_common::activity_pub::broadcast; -use plume_common::utils; use plume_models::{ blogs::Blog, db_conn::DbConn, inbox::inbox, posts::Post, reshares::*, timeline::*, users::User, Error, PlumeRocket, CONFIG, @@ -54,7 +54,7 @@ pub fn create( #[post("/~///reshare", rank = 1)] pub fn create_auth(blog: String, slug: String, i18n: I18n) -> Flash { - utils::requires_login( + requires_login( &i18n!(i18n.catalog, "To reshare a post, you need to be logged in"), uri!(create: blog = blog, slug = slug), ) diff --git a/src/routes/user.rs b/src/routes/user.rs index e7d66de1d..ac8371a37 100644 --- a/src/routes/user.rs +++ b/src/routes/user.rs @@ -14,8 +14,9 @@ use crate::routes::{ email_signups::EmailSignupForm, errors::ErrorPage, Page, RemoteForm, RespondOrRedirect, }; use crate::template_utils::{IntoContext, Ructe}; +use crate::utils::requires_login; use plume_common::activity_pub::{broadcast, ActivityStream, ApRequest, Id}; -use plume_common::utils; +use plume_common::utils::md_to_html; use plume_models::{ blogs::Blog, db_conn::DbConn, follows, headers::Headers, inbox::inbox as local_inbox, instance::Instance, medias::Media, posts::Post, reshares::Reshare, safe_string::SafeString, @@ -26,7 +27,7 @@ use plume_models::{ pub fn me(user: Option) -> RespondOrRedirect { match user { Some(user) => Redirect::to(uri!(details: name = user.username)).into(), - None => utils::requires_login("", uri!(me)).into(), + None => requires_login("", uri!(me)).into(), } } @@ -71,7 +72,7 @@ pub fn dashboard(user: User, conn: DbConn, rockets: PlumeRocket) -> Result Flash { - utils::requires_login( + requires_login( &i18n!( i18n.catalog, "To access your dashboard, you need to be logged in" @@ -187,7 +188,7 @@ pub fn follow_not_connected( #[get("/@//follow?local", rank = 2)] pub fn follow_auth(name: String, i18n: I18n) -> Flash { - utils::requires_login( + requires_login( &i18n!( i18n.catalog, "To subscribe to someone, you need to be logged in" @@ -307,7 +308,7 @@ pub fn edit( #[get("/@//edit", rank = 2)] pub fn edit_auth(name: String, i18n: I18n) -> Flash { - utils::requires_login( + requires_login( &i18n!( i18n.catalog, "To edit your profile, you need to be logged in" @@ -338,7 +339,7 @@ pub fn update( user.email = Some(form.email.clone()); user.summary = form.summary.clone(); user.summary_html = SafeString::new( - &utils::md_to_html( + &md_to_html( &form.summary, None, false, diff --git a/src/utils.rs b/src/utils.rs new file mode 100644 index 000000000..b204814c0 --- /dev/null +++ b/src/utils.rs @@ -0,0 +1,17 @@ +use rocket::{ + http::uri::Uri, + response::{Flash, Redirect}, +}; + +/** +* Redirects to the login page with a given message. +* +* Note that the message should be translated before passed to this function. +*/ +pub fn requires_login>>(message: &str, url: T) -> Flash { + Flash::new( + Redirect::to(format!("/login?m={}", Uri::percent_encode(message))), + "callback", + url.into().to_string(), + ) +} From 6498dbfbb756baf3ec2c516217f806712db70a61 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Wed, 12 Jan 2022 04:28:02 +0900 Subject: [PATCH 02/10] Reuse form values --- src/routes/email_signups.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/routes/email_signups.rs b/src/routes/email_signups.rs index c874ec6ce..82c5ccbda 100644 --- a/src/routes/email_signups.rs +++ b/src/routes/email_signups.rs @@ -206,11 +206,8 @@ pub fn signup( let mut err = ValidationErrors::default(); err.add("email", ValidationError::new("Email couldn't changed")); let form = NewUserForm { - username: form.username.clone(), - password: form.password.clone(), - password_confirmation: form.password_confirmation.clone(), email: signup.email, - token: form.token.clone(), + ..form.into_inner() }; return Ok(Response(render!(email_signups::edit( &(&conn, &rockets).to_context(), From 7c82b0861598ec4cc729b263e6e9e207b10bb6cc Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Wed, 12 Jan 2022 04:36:01 +0900 Subject: [PATCH 03/10] Use into() instead of explicitly wrapping return values --- src/routes/email_signups.rs | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/routes/email_signups.rs b/src/routes/email_signups.rs index 82c5ccbda..057a8d5c2 100644 --- a/src/routes/email_signups.rs +++ b/src/routes/email_signups.rs @@ -70,16 +70,15 @@ pub fn create( conn: DbConn, rockets: PlumeRocket, ) -> Result { - use RespondOrRedirect::{FlashRedirect, Response}; - if !matches!(CONFIG.signup, SignupStrategy::Email) { - return Ok(FlashRedirect(Flash::error( + return Ok(Flash::error( Redirect::to(uri!(super::user::new)), i18n!( rockets.intl.catalog, "Email registrations are not enabled. Please restart." ), - ))); + ) + .into()); } let registration_open = !Instance::get_local() @@ -87,13 +86,14 @@ pub fn create( .unwrap_or(true); if registration_open { - return Ok(FlashRedirect(Flash::error( + return Ok(Flash::error( Redirect::to(uri!(super::user::new)), i18n!( rockets.intl.catalog, "Registrations are closed on this instance." ), - ))); // Actually, it is an error + ) + .into()); // Actually, it is an error } let mut form = form.into_inner(); form.email = form.email.trim().to_owned(); @@ -111,14 +111,10 @@ pub fn create( Error::UserAlreadyExists => { // TODO: Notify to admin (and the user?) warn!("Registration attempted for existing user: {}. Registraion halted and email sending skipped.", &form.email); - Response(render!(email_signups::create( - &(&conn, &rockets).to_context() - ))) - } - Error::NotFound => { - Response(render!(errors::not_found(&(&conn, &rockets).to_context()))) + render!(email_signups::create(&(&conn, &rockets).to_context())).into() } - _ => Response(render!(errors::not_found(&(&conn, &rockets).to_context()))), // FIXME + Error::NotFound => render!(errors::not_found(&(&conn, &rockets).to_context())).into(), + _ => render!(errors::not_found(&(&conn, &rockets).to_context())).into(), // FIXME }); } let token = res.unwrap(); @@ -138,9 +134,7 @@ pub fn create( mailer.send(message.into()).ok(); // TODO: Render error page } - Ok(Response(render!(email_signups::create( - &(&conn, &rockets).to_context() - )))) + Ok(render!(email_signups::create(&(&conn, &rockets).to_context())).into()) } #[get("/email_signups/new")] From b4395bce997e324ad6ba32f86e42ca1c4accf796 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Wed, 12 Jan 2022 08:39:33 +0900 Subject: [PATCH 04/10] Implement request guard to detect enabled sign-up strategy --- plume-models/src/signups.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/plume-models/src/signups.rs b/plume-models/src/signups.rs index 07bdff152..7a520eab4 100644 --- a/plume-models/src/signups.rs +++ b/plume-models/src/signups.rs @@ -1,3 +1,5 @@ +use crate::CONFIG; +use rocket::request::{FromRequest, Outcome, Request}; use std::fmt; use std::str::FromStr; @@ -43,3 +45,28 @@ impl fmt::Display for StrategyError { } impl std::error::Error for StrategyError {} + +pub struct Password(); +pub struct Email(); + +impl<'a, 'r> FromRequest<'a, 'r> for Password { + type Error = (); + + fn from_request(_request: &'a Request<'r>) -> Outcome { + match matches!(CONFIG.signup, Strategy::Password) { + true => Outcome::Success(Self()), + false => Outcome::Forward(()), + } + } +} + +impl<'a, 'r> FromRequest<'a, 'r> for Email { + type Error = (); + + fn from_request(_request: &'a Request<'r>) -> Outcome { + match matches!(CONFIG.signup, Strategy::Email) { + true => Outcome::Success(Self()), + false => Outcome::Forward(()), + } + } +} From 13f7734751a204d2c793b92c8e6c18e872bf5713 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Wed, 12 Jan 2022 08:40:20 +0900 Subject: [PATCH 05/10] Hide email sign-up routings when it's disabled --- src/routes/email_signups.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/routes/email_signups.rs b/src/routes/email_signups.rs index 057a8d5c2..0b93bb1af 100644 --- a/src/routes/email_signups.rs +++ b/src/routes/email_signups.rs @@ -4,8 +4,12 @@ use crate::{ template_utils::{IntoContext, Ructe}, }; use plume_models::{ - db_conn::DbConn, email_signups::EmailSignup, instance::Instance, lettre::Transport, - signups::Strategy as SignupStrategy, Error, PlumeRocket, CONFIG, + db_conn::DbConn, + email_signups::EmailSignup, + instance::Instance, + lettre::Transport, + signups::{self, Strategy as SignupStrategy}, + Error, PlumeRocket, CONFIG, }; use rocket::{ http::Status, @@ -69,6 +73,7 @@ pub fn create( form: LenientForm, conn: DbConn, rockets: PlumeRocket, + _enabled: signups::Email, ) -> Result { if !matches!(CONFIG.signup, SignupStrategy::Email) { return Ok(Flash::error( @@ -138,12 +143,17 @@ pub fn create( } #[get("/email_signups/new")] -pub fn created(conn: DbConn, rockets: PlumeRocket) -> Ructe { +pub fn created(conn: DbConn, rockets: PlumeRocket, _enabled: signups::Email) -> Ructe { render!(email_signups::create(&(&conn, &rockets).to_context())) } #[get("/email_signups/")] -pub fn show(token: String, conn: DbConn, rockets: PlumeRocket) -> Result { +pub fn show( + token: String, + conn: DbConn, + rockets: PlumeRocket, + _enabled: signups::Email, +) -> Result { let signup = EmailSignup::find_by_token(&conn, token.into())?; let confirmation = signup.confirm(&conn); if let Some(err) = confirmation.err() { @@ -179,6 +189,7 @@ pub fn signup( form: LenientForm, conn: DbConn, rockets: PlumeRocket, + _enabled: signups::Email, ) -> Result { use RespondOrRedirect::{FlashRedirect, Response}; From 7de37bc9b7f4451ec4f78e0f0a9bd4c6e255a070 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Wed, 12 Jan 2022 08:46:11 +0900 Subject: [PATCH 06/10] Hide password sign-up routings when it's disabled --- src/routes/user.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/routes/user.rs b/src/routes/user.rs index ac8371a37..576fc3c6d 100644 --- a/src/routes/user.rs +++ b/src/routes/user.rs @@ -18,9 +18,19 @@ use crate::utils::requires_login; use plume_common::activity_pub::{broadcast, ActivityStream, ApRequest, Id}; use plume_common::utils::md_to_html; use plume_models::{ - blogs::Blog, db_conn::DbConn, follows, headers::Headers, inbox::inbox as local_inbox, - instance::Instance, medias::Media, posts::Post, reshares::Reshare, safe_string::SafeString, - signups::Strategy as SignupStrategy, users::*, Error, PlumeRocket, CONFIG, + blogs::Blog, + db_conn::DbConn, + follows, + headers::Headers, + inbox::inbox as local_inbox, + instance::Instance, + medias::Media, + posts::Post, + reshares::Reshare, + safe_string::SafeString, + signups::{self, Strategy as SignupStrategy}, + users::*, + Error, PlumeRocket, CONFIG, }; #[get("/me")] @@ -466,6 +476,7 @@ pub fn create( form: LenientForm, conn: DbConn, rockets: PlumeRocket, + _enabled: signups::Password, ) -> Result, Ructe> { if !Instance::get_local() .map(|i| i.open_registrations) From e31a2238fbcfee17b64e3ecfd4924156a41ffe88 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Wed, 12 Jan 2022 08:58:42 +0900 Subject: [PATCH 07/10] Respond with error status code when error --- src/routes/errors.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/routes/errors.rs b/src/routes/errors.rs index eb1604316..74dc4dd58 100644 --- a/src/routes/errors.rs +++ b/src/routes/errors.rs @@ -1,6 +1,7 @@ use crate::template_utils::{IntoContext, Ructe}; use plume_models::{db_conn::DbConn, Error, PlumeRocket}; use rocket::{ + http::Status, response::{self, Responder}, Request, }; @@ -16,18 +17,13 @@ impl From for ErrorPage { } impl<'r> Responder<'r> for ErrorPage { - fn respond_to(self, req: &Request<'_>) -> response::Result<'r> { - let conn = req.guard::().unwrap(); - let rockets = req.guard::().unwrap(); + fn respond_to(self, _req: &Request<'_>) -> response::Result<'r> { + warn!("{:?}", self.0); match self.0 { - Error::NotFound => { - render!(errors::not_found(&(&conn, &rockets).to_context())).respond_to(req) - } - Error::Unauthorized => { - render!(errors::not_found(&(&conn, &rockets).to_context())).respond_to(req) - } - _ => render!(errors::not_found(&(&conn, &rockets).to_context())).respond_to(req), + Error::NotFound => Err(Status::NotFound), + Error::Unauthorized => Err(Status::NotFound), + _ => Err(Status::InternalServerError), } } } From 5d58b31f1cd2428846f100e0d67cc4a556c41d79 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Wed, 12 Jan 2022 08:59:41 +0900 Subject: [PATCH 08/10] Remove unreachable code --- src/routes/email_signups.rs | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/src/routes/email_signups.rs b/src/routes/email_signups.rs index 0b93bb1af..a9994801f 100644 --- a/src/routes/email_signups.rs +++ b/src/routes/email_signups.rs @@ -4,11 +4,7 @@ use crate::{ template_utils::{IntoContext, Ructe}, }; use plume_models::{ - db_conn::DbConn, - email_signups::EmailSignup, - instance::Instance, - lettre::Transport, - signups::{self, Strategy as SignupStrategy}, + db_conn::DbConn, email_signups::EmailSignup, instance::Instance, lettre::Transport, signups, Error, PlumeRocket, CONFIG, }; use rocket::{ @@ -75,17 +71,6 @@ pub fn create( rockets: PlumeRocket, _enabled: signups::Email, ) -> Result { - if !matches!(CONFIG.signup, SignupStrategy::Email) { - return Ok(Flash::error( - Redirect::to(uri!(super::user::new)), - i18n!( - rockets.intl.catalog, - "Email registrations are not enabled. Please restart." - ), - ) - .into()); - } - let registration_open = !Instance::get_local() .map(|i| i.open_registrations) .unwrap_or(true); From 9bbfc71fc8eaa1776f075c21c92782f206201bc2 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Wed, 12 Jan 2022 09:19:48 +0900 Subject: [PATCH 09/10] Fix registration openess condition mistake --- src/routes/email_signups.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/routes/email_signups.rs b/src/routes/email_signups.rs index a9994801f..fc868eadf 100644 --- a/src/routes/email_signups.rs +++ b/src/routes/email_signups.rs @@ -71,11 +71,11 @@ pub fn create( rockets: PlumeRocket, _enabled: signups::Email, ) -> Result { - let registration_open = !Instance::get_local() + let registration_open = Instance::get_local() .map(|i| i.open_registrations) .unwrap_or(true); - if registration_open { + if !registration_open { return Ok(Flash::error( Redirect::to(uri!(super::user::new)), i18n!( From 43b46a8be4662172b1d17f2946c4b2399ea3e752 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Wed, 12 Jan 2022 09:20:20 +0900 Subject: [PATCH 10/10] Make email_signups::create return ErrorPage on error --- src/routes/email_signups.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/routes/email_signups.rs b/src/routes/email_signups.rs index fc868eadf..7a364dd0f 100644 --- a/src/routes/email_signups.rs +++ b/src/routes/email_signups.rs @@ -70,7 +70,7 @@ pub fn create( conn: DbConn, rockets: PlumeRocket, _enabled: signups::Email, -) -> Result { +) -> Result { let registration_open = Instance::get_local() .map(|i| i.open_registrations) .unwrap_or(true); @@ -87,14 +87,15 @@ pub fn create( } let mut form = form.into_inner(); form.email = form.email.trim().to_owned(); - form.validate().map_err(|err| { - render!(email_signups::new( + if let Err(err) = form.validate() { + return Ok(render!(email_signups::new( &(&conn, &rockets).to_context(), registration_open, &form, err )) - })?; + .into()); + } let res = EmailSignup::start(&conn, &form.email); if let Some(err) = res.as_ref().err() { return Ok(match err {