From 4b205fa995a6935a2512a9725d84635e889db48e Mon Sep 17 00:00:00 2001 From: Rob Watson Date: Tue, 4 Jun 2019 20:55:17 +0200 Subject: [PATCH] Store password reset requests in database (#610) * Store password reset requests in database Signed-off-by: Rob Watson * Refactor password reset request expiry handling * Integrate sqlite * Fix formatting --- .../down.sql | 1 + .../up.sql | 9 + .../down.sql | 1 + .../up.sql | 9 + plume-models/src/lib.rs | 2 + plume-models/src/password_reset_requests.rs | 164 ++++++++++++++++++ plume-models/src/schema.rs | 10 ++ src/routes/session.rs | 108 ++++-------- .../password_reset_request_expired.rs.html | 9 + 9 files changed, 238 insertions(+), 75 deletions(-) create mode 100644 migrations/postgres/2019-05-30-173029_create_password_reset_requests_table/down.sql create mode 100644 migrations/postgres/2019-05-30-173029_create_password_reset_requests_table/up.sql create mode 100644 migrations/sqlite/2019-06-04-102747_create_password_reset_requests_table/down.sql create mode 100644 migrations/sqlite/2019-06-04-102747_create_password_reset_requests_table/up.sql create mode 100644 plume-models/src/password_reset_requests.rs create mode 100644 templates/session/password_reset_request_expired.rs.html diff --git a/migrations/postgres/2019-05-30-173029_create_password_reset_requests_table/down.sql b/migrations/postgres/2019-05-30-173029_create_password_reset_requests_table/down.sql new file mode 100644 index 0000000..7d4bc8f --- /dev/null +++ b/migrations/postgres/2019-05-30-173029_create_password_reset_requests_table/down.sql @@ -0,0 +1 @@ +DROP TABLE password_reset_requests; diff --git a/migrations/postgres/2019-05-30-173029_create_password_reset_requests_table/up.sql b/migrations/postgres/2019-05-30-173029_create_password_reset_requests_table/up.sql new file mode 100644 index 0000000..b1fffc2 --- /dev/null +++ b/migrations/postgres/2019-05-30-173029_create_password_reset_requests_table/up.sql @@ -0,0 +1,9 @@ +CREATE TABLE password_reset_requests ( + id SERIAL PRIMARY KEY, + email VARCHAR NOT NULL, + token VARCHAR NOT NULL, + expiration_date TIMESTAMP NOT NULL +); + +CREATE INDEX password_reset_requests_token ON password_reset_requests (token); +CREATE UNIQUE INDEX password_reset_requests_email ON password_reset_requests (email); diff --git a/migrations/sqlite/2019-06-04-102747_create_password_reset_requests_table/down.sql b/migrations/sqlite/2019-06-04-102747_create_password_reset_requests_table/down.sql new file mode 100644 index 0000000..7d4bc8f --- /dev/null +++ b/migrations/sqlite/2019-06-04-102747_create_password_reset_requests_table/down.sql @@ -0,0 +1 @@ +DROP TABLE password_reset_requests; diff --git a/migrations/sqlite/2019-06-04-102747_create_password_reset_requests_table/up.sql b/migrations/sqlite/2019-06-04-102747_create_password_reset_requests_table/up.sql new file mode 100644 index 0000000..bf64163 --- /dev/null +++ b/migrations/sqlite/2019-06-04-102747_create_password_reset_requests_table/up.sql @@ -0,0 +1,9 @@ +CREATE TABLE password_reset_requests ( + id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + email VARCHAR NOT NULL, + token VARCHAR NOT NULL, + expiration_date DATETIME NOT NULL +); + +CREATE INDEX password_reset_requests_token ON password_reset_requests (token); +CREATE UNIQUE INDEX password_reset_requests_email ON password_reset_requests (email); diff --git a/plume-models/src/lib.rs b/plume-models/src/lib.rs index fd46f61..2c30cb8 100644 --- a/plume-models/src/lib.rs +++ b/plume-models/src/lib.rs @@ -65,6 +65,7 @@ pub enum Error { Unauthorized, Url, Webfinger, + Expired, } impl From for Error { @@ -367,6 +368,7 @@ pub mod medias; pub mod mentions; pub mod migrations; pub mod notifications; +pub mod password_reset_requests; pub mod plume_rocket; pub mod post_authors; pub mod posts; diff --git a/plume-models/src/password_reset_requests.rs b/plume-models/src/password_reset_requests.rs new file mode 100644 index 0000000..ab1deef --- /dev/null +++ b/plume-models/src/password_reset_requests.rs @@ -0,0 +1,164 @@ +use chrono::{offset::Utc, Duration, NaiveDateTime}; +use diesel::{ExpressionMethods, QueryDsl, RunQueryDsl}; +use schema::password_reset_requests; +use {Connection, Error, Result}; + +#[derive(Clone, Identifiable, Queryable)] +pub struct PasswordResetRequest { + pub id: i32, + pub email: String, + pub token: String, + pub expiration_date: NaiveDateTime, +} + +#[derive(Insertable)] +#[table_name = "password_reset_requests"] +pub struct NewPasswordResetRequest { + pub email: String, + pub token: String, + pub expiration_date: NaiveDateTime, +} + +const TOKEN_VALIDITY_HOURS: i64 = 2; + +impl PasswordResetRequest { + pub fn insert(conn: &Connection, email: &str) -> Result { + // first, delete other password reset tokens associated with this email: + let existing_requests = + password_reset_requests::table.filter(password_reset_requests::email.eq(email)); + diesel::delete(existing_requests).execute(conn)?; + + // now, generate a random token, set the expiry date, + // and insert it into the DB: + let token = plume_common::utils::random_hex(); + let expiration_date = Utc::now() + .naive_utc() + .checked_add_signed(Duration::hours(TOKEN_VALIDITY_HOURS)) + .expect("could not calculate expiration date"); + let new_request = NewPasswordResetRequest { + email: email.to_owned(), + token: token.clone(), + expiration_date, + }; + diesel::insert_into(password_reset_requests::table) + .values(new_request) + .execute(conn) + .map_err(Error::from)?; + + Ok(token) + } + + pub fn find_by_token(conn: &Connection, token: &str) -> Result { + let token = password_reset_requests::table + .filter(password_reset_requests::token.eq(token)) + .first::(conn) + .map_err(Error::from)?; + + if token.expiration_date < Utc::now().naive_utc() { + return Err(Error::Expired); + } + + Ok(token) + } + + pub fn find_and_delete_by_token(conn: &Connection, token: &str) -> Result { + let request = Self::find_by_token(&conn, &token)?; + + let filter = + password_reset_requests::table.filter(password_reset_requests::id.eq(request.id)); + diesel::delete(filter).execute(conn)?; + + Ok(request) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use diesel::Connection; + use tests::db; + use users::tests as user_tests; + + #[test] + fn test_insert_and_find_password_reset_request() { + let conn = db(); + conn.test_transaction::<_, (), _>(|| { + user_tests::fill_database(&conn); + let admin_email = "admin@example.com"; + + let token = PasswordResetRequest::insert(&conn, admin_email) + .expect("couldn't insert new request"); + let request = PasswordResetRequest::find_by_token(&conn, &token) + .expect("couldn't retrieve request"); + + assert!(&token.len() > &32); + assert_eq!(&request.email, &admin_email); + + Ok(()) + }); + } + + #[test] + fn test_insert_delete_previous_password_reset_request() { + let conn = db(); + conn.test_transaction::<_, (), _>(|| { + user_tests::fill_database(&conn); + let admin_email = "admin@example.com"; + + PasswordResetRequest::insert(&conn, &admin_email).expect("couldn't insert new request"); + PasswordResetRequest::insert(&conn, &admin_email) + .expect("couldn't insert second request"); + + let count = password_reset_requests::table.count().get_result(&*conn); + assert_eq!(Ok(1), count); + + Ok(()) + }); + } + + #[test] + fn test_find_password_reset_request_by_token_time() { + let conn = db(); + conn.test_transaction::<_, (), _>(|| { + user_tests::fill_database(&conn); + let admin_email = "admin@example.com"; + let token = "abcdef"; + let now = Utc::now().naive_utc(); + + diesel::insert_into(password_reset_requests::table) + .values(( + password_reset_requests::email.eq(&admin_email), + password_reset_requests::token.eq(&token), + password_reset_requests::expiration_date.eq(now), + )) + .execute(&*conn) + .expect("could not insert request"); + + match PasswordResetRequest::find_by_token(&conn, &token) { + Err(Error::Expired) => (), + _ => panic!("Received unexpected result finding expired token"), + } + + Ok(()) + }); + } + + #[test] + fn test_find_and_delete_password_reset_request() { + let conn = db(); + conn.test_transaction::<_, (), _>(|| { + user_tests::fill_database(&conn); + let admin_email = "admin@example.com"; + + let token = PasswordResetRequest::insert(&conn, &admin_email) + .expect("couldn't insert new request"); + PasswordResetRequest::find_and_delete_by_token(&conn, &token) + .expect("couldn't find and delete request"); + + let count = password_reset_requests::table.count().get_result(&*conn); + assert_eq!(Ok(0), count); + + Ok(()) + }); + } +} diff --git a/plume-models/src/schema.rs b/plume-models/src/schema.rs index df460ce..f2cc833 100644 --- a/plume-models/src/schema.rs +++ b/plume-models/src/schema.rs @@ -141,6 +141,15 @@ table! { } } +table! { + password_reset_requests (id) { + id -> Int4, + email -> Varchar, + token -> Varchar, + expiration_date -> Timestamp, + } +} + table! { post_authors (id) { id -> Int4, @@ -247,6 +256,7 @@ allow_tables_to_appear_in_same_query!( medias, mentions, notifications, + password_reset_requests, post_authors, posts, reshares, diff --git a/src/routes/session.rs b/src/routes/session.rs index 5a299f3..0bcd842 100644 --- a/src/routes/session.rs +++ b/src/routes/session.rs @@ -16,10 +16,10 @@ use validator::{Validate, ValidationError, ValidationErrors}; use mail::{build_mail, Mailer}; use plume_models::{ + password_reset_requests::*, users::{User, AUTH_COOKIE}, Error, PlumeRocket, CONFIG, }; -use routes::errors::ErrorPage; use template_utils::{IntoContext, Ructe}; #[get("/login?")] @@ -164,29 +164,17 @@ pub struct ResetForm { pub fn password_reset_request( mail: State>>, form: Form, - requests: State>>>, rockets: PlumeRocket, ) -> Ructe { - let mut requests = requests.lock().unwrap(); - // Remove outdated requests (more than 1 day old) to avoid the list to grow too much - requests.retain(|r| r.creation_date.elapsed().as_secs() < 24 * 60 * 60); + if User::find_by_email(&*rockets.conn, &form.email).is_ok() { + let token = PasswordResetRequest::insert(&*rockets.conn, &form.email) + .expect("password_reset_request::insert: error"); - if User::find_by_email(&*rockets.conn, &form.email).is_ok() - && !requests.iter().any(|x| x.mail == form.email.clone()) - { - let id = plume_common::utils::random_hex(); - - requests.push(ResetRequest { - mail: form.email.clone(), - id: id.clone(), - creation_date: Instant::now(), - }); - - let link = format!("https://{}/password-reset/{}", CONFIG.base_url, id); + let url = format!("https://{}/password-reset/{}", CONFIG.base_url, token); if let Some(message) = build_mail( form.email.clone(), i18n!(rockets.intl.catalog, "Password reset"), - i18n!(rockets.intl.catalog, "Here is the link to reset your password: {0}"; link), + i18n!(rockets.intl.catalog, "Here is the link to reset your password: {0}"; url), ) { if let Some(ref mut mail) = *mail.lock().unwrap() { mail.send(message.into()) @@ -199,17 +187,10 @@ pub fn password_reset_request( } #[get("/password-reset/")] -pub fn password_reset_form( - token: String, - requests: State>>>, - rockets: PlumeRocket, -) -> Result { - requests - .lock() - .unwrap() - .iter() - .find(|x| x.id == token.clone()) - .ok_or(Error::NotFound)?; +pub fn password_reset_form(token: String, rockets: PlumeRocket) -> Result { + PasswordResetRequest::find_by_token(&*rockets.conn, &token) + .map_err(|err| password_reset_error_response(err, &rockets))?; + Ok(render!(session::password_reset( &rockets.to_context(), &NewPasswordForm::default(), @@ -239,56 +220,33 @@ fn passwords_match(form: &NewPasswordForm) -> Result<(), ValidationError> { #[post("/password-reset/", data = "
")] pub fn password_reset( token: String, - requests: State>>>, form: Form, rockets: PlumeRocket, ) -> Result, Ructe> { form.validate() - .and_then(|_| { - let mut requests = requests.lock().unwrap(); - let req = requests - .iter() - .find(|x| x.id == token.clone()) - .ok_or_else(|| to_validation(0))? - .clone(); - if req.creation_date.elapsed().as_secs() < 60 * 60 * 2 { - // Reset link is only valid for 2 hours - requests.retain(|r| *r != req); - let user = User::find_by_email(&*rockets.conn, &req.mail).map_err(to_validation)?; - user.reset_password(&*rockets.conn, &form.password).ok(); - Ok(Flash::success( - Redirect::to(uri!( - new: m = _ - )), - i18n!( - rockets.intl.catalog, - "Your password was successfully reset." - ), - )) - } else { - Ok(Flash::error( - Redirect::to(uri!( - new: m = _ - )), - i18n!( - rockets.intl.catalog, - "Sorry, but the link expired. Try again" - ), - )) - } - }) - .map_err(|err| render!(session::password_reset(&rockets.to_context(), &form, err))) + .map_err(|err| render!(session::password_reset(&rockets.to_context(), &form, err)))?; + + PasswordResetRequest::find_and_delete_by_token(&*rockets.conn, &token) + .and_then(|request| User::find_by_email(&*rockets.conn, &request.email)) + .and_then(|user| user.reset_password(&*rockets.conn, &form.password)) + .map_err(|err| password_reset_error_response(err, &rockets))?; + + Ok(Flash::success( + Redirect::to(uri!( + new: m = _ + )), + i18n!( + rockets.intl.catalog, + "Your password was successfully reset." + ), + )) } -fn to_validation(_: T) -> ValidationErrors { - let mut errors = ValidationErrors::new(); - errors.add( - "", - ValidationError { - code: Cow::from("server_error"), - message: Some(Cow::from("An unknown error occured")), - params: std::collections::HashMap::new(), - }, - ); - errors +fn password_reset_error_response(err: Error, rockets: &PlumeRocket) -> Ructe { + match err { + Error::Expired => render!(session::password_reset_request_expired( + &rockets.to_context() + )), + _ => render!(errors::not_found(&rockets.to_context())), + } } diff --git a/templates/session/password_reset_request_expired.rs.html b/templates/session/password_reset_request_expired.rs.html new file mode 100644 index 0000000..8766c4b --- /dev/null +++ b/templates/session/password_reset_request_expired.rs.html @@ -0,0 +1,9 @@ +@use template_utils::*; +@use templates::base; + +@(ctx: BaseContext) + +@:base(ctx, i18n!(ctx.1, "Password reset"), {}, {}, { +

@i18n!(ctx.1, "This token has expired")

+

@i18n!(ctx.1, "Please start the process again by clicking") @i18n!(ctx.1, "here").

+})