From 9a79ef701b05ca9d6eff3cf2e198333f5cfff6ea Mon Sep 17 00:00:00 2001 From: Pierre HUBERT Date: Wed, 27 Mar 2024 19:26:07 +0100 Subject: [PATCH] Need to perform 2FA before modifying factors --- src/constants.rs | 4 +++ src/controllers/login_controller.rs | 7 ++--- src/controllers/two_factor_api.rs | 5 ++++ src/controllers/two_factors_controller.rs | 6 +++-- src/data/critical_route.rs | 32 +++++++++++++++++++++++ src/data/current_user.rs | 12 +++++++++ src/data/from_request_redirect.rs | 32 +++++++++++++++++++++++ src/data/login_redirect.rs | 14 ++++++++++ src/data/mod.rs | 2 ++ 9 files changed, 107 insertions(+), 7 deletions(-) create mode 100644 src/data/critical_route.rs create mode 100644 src/data/from_request_redirect.rs diff --git a/src/constants.rs b/src/constants.rs index 5ee6a30..0b1aec4 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -29,6 +29,10 @@ pub const MAX_SECOND_FACTOR_NAME_LEN: usize = 25; /// exempted from this IP address to use 2FA pub const SECOND_FACTOR_EXEMPTION_AFTER_SUCCESSFUL_LOGIN: u64 = 7 * 24 * 3600; +/// The maximum acceptable interval of time between last two factors authentication of a user and +/// access to a critical route / a critical client +pub const SECOND_FACTOR_EXPIRATION_FOR_CRITICAL_OPERATIONS: u64 = 60 * 10; + /// Minimum password length pub const MIN_PASS_LEN: usize = 4; diff --git a/src/controllers/login_controller.rs b/src/controllers/login_controller.rs index 112a7f7..026d9b7 100644 --- a/src/controllers/login_controller.rs +++ b/src/controllers/login_controller.rs @@ -14,7 +14,7 @@ use crate::controllers::base_controller::{ }; use crate::data::action_logger::{Action, ActionLogger}; use crate::data::force_2fa_auth::Force2FAAuth; -use crate::data::login_redirect::LoginRedirect; +use crate::data::login_redirect::{get_2fa_url, LoginRedirect}; use crate::data::provider::{Provider, ProvidersManager}; use crate::data::session_identity::{SessionIdentity, SessionStatus}; use crate::data::user::User; @@ -129,10 +129,7 @@ pub async fn login_route( } // Check if the user has to validate a second factor else if SessionIdentity(id.as_ref()).need_2fa_auth() { - return redirect_user(&format!( - "/2fa_auth?redirect={}", - query.redirect.get_encoded() - )); + return redirect_user(&get_2fa_url(&query.redirect, false)); } // Check if given login is not acceptable else if req diff --git a/src/controllers/two_factor_api.rs b/src/controllers/two_factor_api.rs index 369e8dc..ce46e05 100644 --- a/src/controllers/two_factor_api.rs +++ b/src/controllers/two_factor_api.rs @@ -7,6 +7,7 @@ use crate::actors::users_actor; use crate::actors::users_actor::UsersActor; use crate::constants::MAX_SECOND_FACTOR_NAME_LEN; use crate::data::action_logger::{Action, ActionLogger}; +use crate::data::critical_route::CriticalRoute; use crate::data::current_user::CurrentUser; use crate::data::totp_key::TotpKey; use crate::data::user::{FactorID, TwoFactor, TwoFactorType}; @@ -29,6 +30,7 @@ pub struct AddTOTPRequest { } pub async fn save_totp_factor( + _critical: CriticalRoute, user: CurrentUser, form: web::Json, users: web::Data>, @@ -77,6 +79,7 @@ pub struct AddWebauthnRequest { } pub async fn save_webauthn_factor( + _critical: CriticalRoute, user: CurrentUser, form: web::Json, users: web::Data>, @@ -121,6 +124,7 @@ pub struct DeleteFactorRequest { } pub async fn delete_factor( + _critical: CriticalRoute, user: CurrentUser, form: web::Json, users: web::Data>, @@ -145,6 +149,7 @@ pub async fn delete_factor( } pub async fn clear_login_history( + _critical: CriticalRoute, user: CurrentUser, users: web::Data>, logger: ActionLogger, diff --git a/src/controllers/two_factors_controller.rs b/src/controllers/two_factors_controller.rs index 93d053f..cd72c9e 100644 --- a/src/controllers/two_factors_controller.rs +++ b/src/controllers/two_factors_controller.rs @@ -9,6 +9,7 @@ use qrcode_generator::QrCodeEcc; use crate::constants::MAX_SECOND_FACTOR_NAME_LEN; use crate::controllers::settings_controller::BaseSettingsPage; use crate::data::app_config::AppConfig; +use crate::data::critical_route::CriticalRoute; use crate::data::current_user::CurrentUser; use crate::data::totp_key::TotpKey; use crate::data::user::User; @@ -43,7 +44,7 @@ struct AddWebauhtnPage<'a> { } /// Manage two factors authentication methods route -pub async fn two_factors_route(user: CurrentUser) -> impl Responder { +pub async fn two_factors_route(_critical: CriticalRoute, user: CurrentUser) -> impl Responder { HttpResponse::Ok().body( TwoFactorsPage { p: BaseSettingsPage::get("Two factor auth", &user, None, None), @@ -56,7 +57,7 @@ pub async fn two_factors_route(user: CurrentUser) -> impl Responder { } /// Configure a new TOTP authentication factor -pub async fn add_totp_factor_route(user: CurrentUser) -> impl Responder { +pub async fn add_totp_factor_route(_critical: CriticalRoute, user: CurrentUser) -> impl Responder { let key = TotpKey::new_random(); let qr_code = qrcode_generator::to_png_to_vec( @@ -87,6 +88,7 @@ pub async fn add_totp_factor_route(user: CurrentUser) -> impl Responder { /// Configure a new security key factor pub async fn add_webauthn_factor_route( + _critical: CriticalRoute, user: CurrentUser, manager: WebAuthManagerReq, ) -> impl Responder { diff --git a/src/data/critical_route.rs b/src/data/critical_route.rs new file mode 100644 index 0000000..0e54e96 --- /dev/null +++ b/src/data/critical_route.rs @@ -0,0 +1,32 @@ +use crate::data::current_user::CurrentUser; +use crate::data::from_request_redirect::FromRequestRedirect; +use crate::data::login_redirect::{get_2fa_url, LoginRedirect}; +use actix_web::dev::Payload; +use actix_web::{FromRequest, HttpRequest}; +use std::future::Future; +use std::pin::Pin; + +pub struct CriticalRoute; + +impl FromRequest for CriticalRoute { + type Error = FromRequestRedirect; + type Future = Pin>>>; + + fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { + let req = req.clone(); + + Box::pin(async move { + let current_user = CurrentUser::from_request(&req, &mut Payload::None) + .await + .expect("Failed to extract user identity!"); + + if current_user.should_request_2fa_for_critical_function() { + let url = get_2fa_url(&LoginRedirect::from_req(&req), true); + + return Err(FromRequestRedirect::new(url)); + } + + Ok(Self) + }) + } +} diff --git a/src/data/current_user.rs b/src/data/current_user.rs index 41adfd0..1a50b73 100644 --- a/src/data/current_user.rs +++ b/src/data/current_user.rs @@ -10,8 +10,10 @@ use actix_web::{web, Error, FromRequest, HttpRequest}; use crate::actors::users_actor; use crate::actors::users_actor::UsersActor; +use crate::constants::SECOND_FACTOR_EXPIRATION_FOR_CRITICAL_OPERATIONS; use crate::data::session_identity::SessionIdentity; use crate::data::user::User; +use crate::utils::time::time; pub struct CurrentUser { user: User, @@ -19,6 +21,16 @@ pub struct CurrentUser { pub last_2fa_auth: Option, } +impl CurrentUser { + pub fn should_request_2fa_for_critical_function(&self) -> bool { + self.user.has_two_factor() + && self + .last_2fa_auth + .map(|t| t + SECOND_FACTOR_EXPIRATION_FOR_CRITICAL_OPERATIONS < time()) + .unwrap_or(true) + } +} + impl From for User { fn from(user: CurrentUser) -> Self { user.user diff --git a/src/data/from_request_redirect.rs b/src/data/from_request_redirect.rs new file mode 100644 index 0000000..8a4c2b7 --- /dev/null +++ b/src/data/from_request_redirect.rs @@ -0,0 +1,32 @@ +use actix_web::body::BoxBody; +use actix_web::http::StatusCode; +use actix_web::{HttpResponse, ResponseError}; +use std::fmt::{Debug, Display, Formatter}; + +#[derive(Debug)] +pub struct FromRequestRedirect { + url: String, +} + +impl FromRequestRedirect { + pub fn new(url: String) -> Self { + Self { url } + } +} + +impl Display for FromRequestRedirect { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "Redirect to {}", self.url) + } +} + +impl ResponseError for FromRequestRedirect { + fn status_code(&self) -> StatusCode { + StatusCode::FOUND + } + fn error_response(&self) -> HttpResponse { + HttpResponse::Found() + .insert_header(("Location", self.url.as_str())) + .body("Redirecting...") + } +} diff --git a/src/data/login_redirect.rs b/src/data/login_redirect.rs index d25e977..fd7469f 100644 --- a/src/data/login_redirect.rs +++ b/src/data/login_redirect.rs @@ -1,7 +1,13 @@ +use actix_web::HttpRequest; + #[derive(serde::Serialize, serde::Deserialize, Debug, Eq, PartialEq, Clone)] pub struct LoginRedirect(String); impl LoginRedirect { + pub fn from_req(req: &HttpRequest) -> Self { + Self(req.uri().to_string()) + } + pub fn get(&self) -> &str { match self.0.starts_with('/') && !self.0.starts_with("//") { true => self.0.as_str(), @@ -19,3 +25,11 @@ impl Default for LoginRedirect { Self("/".to_string()) } } + +/// Get the URL for 2FA authentication +pub fn get_2fa_url(redir: &LoginRedirect, force_2fa: bool) -> String { + format!( + "/2fa_auth?redirect={}&force_2fa={force_2fa}", + redir.get_encoded() + ) +} diff --git a/src/data/mod.rs b/src/data/mod.rs index 20b5826..efdaba3 100644 --- a/src/data/mod.rs +++ b/src/data/mod.rs @@ -3,9 +3,11 @@ pub mod action_logger; pub mod app_config; pub mod client; pub mod code_challenge; +pub mod critical_route; pub mod current_user; pub mod entity_manager; pub mod force_2fa_auth; +pub mod from_request_redirect; pub mod id_token; pub mod jwt_signer; pub mod login_redirect;