From 8623f6626b06b6bb50b48bd7eefa89d67392edf5 Mon Sep 17 00:00:00 2001 From: Apoorv Dixit Date: Wed, 22 May 2024 19:31:23 +0530 Subject: [PATCH 01/11] feat(users): add support to verify using recovery code --- config/config.example.toml | 3 +- config/deployments/integration_test.toml | 1 + config/deployments/production.toml | 1 + config/deployments/sandbox.toml | 1 + config/development.toml | 1 + config/docker_compose.toml | 1 + crates/api_models/src/events/user.rs | 3 +- crates/api_models/src/user.rs | 5 ++ crates/router/src/configs/settings.rs | 1 + crates/router/src/consts/user.rs | 1 + crates/router/src/core/user.rs | 57 ++++++++++++++++++- crates/router/src/routes/app.rs | 1 + crates/router/src/routes/lock_utils.rs | 1 + crates/router/src/routes/user.rs | 18 ++++++ crates/router/src/types/domain/user.rs | 4 ++ crates/router/src/utils/user/password.rs | 12 ++++ .../router/src/utils/user/two_factor_auth.rs | 15 +++++ crates/router_env/src/logger/types.rs | 2 + loadtest/config/development.toml | 1 + 19 files changed, 126 insertions(+), 3 deletions(-) diff --git a/config/config.example.toml b/config/config.example.toml index 5d575b5ba095..776ca079dc39 100644 --- a/config/config.example.toml +++ b/config/config.example.toml @@ -351,7 +351,8 @@ email_role_arn = "" # The amazon resource name ( arn ) of the role which sts_role_session_name = "" # An identifier for the assumed role session, used to uniquely identify a session. [user] -password_validity_in_days = 90 # Number of days after which password should be updated +password_validity_in_days = 90 # Number of days after which password should be updated +two_factor_auth_expiry_in_secs = 60 * 5; # Number of seconds after which 2FA should be done again if doing update/change from inside #tokenization configuration which describe token lifetime and payment method for specific connector [tokenization] diff --git a/config/deployments/integration_test.toml b/config/deployments/integration_test.toml index 5b6c5fe152de..93dadc4617fa 100644 --- a/config/deployments/integration_test.toml +++ b/config/deployments/integration_test.toml @@ -113,6 +113,7 @@ slack_invite_url = "https://join.slack.com/t/hyperswitch-io/shared_invite/zt-2aw [user] password_validity_in_days = 90 +two_factor_auth_expiry_in_secs = 60 * 5; [frm] enabled = true diff --git a/config/deployments/production.toml b/config/deployments/production.toml index b5441f0059de..b4931c1591ff 100644 --- a/config/deployments/production.toml +++ b/config/deployments/production.toml @@ -120,6 +120,7 @@ slack_invite_url = "https://join.slack.com/t/hyperswitch-io/shared_invite/zt-2aw [user] password_validity_in_days = 90 +two_factor_auth_expiry_in_secs = 60 * 5; [frm] enabled = false diff --git a/config/deployments/sandbox.toml b/config/deployments/sandbox.toml index e168fab121d2..640596f5fec8 100644 --- a/config/deployments/sandbox.toml +++ b/config/deployments/sandbox.toml @@ -120,6 +120,7 @@ slack_invite_url = "https://join.slack.com/t/hyperswitch-io/shared_invite/zt-2aw [user] password_validity_in_days = 90 +two_factor_auth_expiry_in_secs = 60 * 5; [frm] enabled = true diff --git a/config/development.toml b/config/development.toml index af26d91446f8..7dc0caa6dbae 100644 --- a/config/development.toml +++ b/config/development.toml @@ -269,6 +269,7 @@ sts_role_session_name = "" [user] password_validity_in_days = 90 +two_factor_auth_expiry_in_secs = 60 * 5; [bank_config.eps] stripe = { banks = "arzte_und_apotheker_bank,austrian_anadi_bank_ag,bank_austria,bankhaus_carl_spangler,bankhaus_schelhammer_und_schattera_ag,bawag_psk_ag,bks_bank_ag,brull_kallmus_bank_ag,btv_vier_lander_bank,capital_bank_grawe_gruppe_ag,dolomitenbank,easybank_ag,erste_bank_und_sparkassen,hypo_alpeadriabank_international_ag,hypo_noe_lb_fur_niederosterreich_u_wien,hypo_oberosterreich_salzburg_steiermark,hypo_tirol_bank_ag,hypo_vorarlberg_bank_ag,hypo_bank_burgenland_aktiengesellschaft,marchfelder_bank,oberbank_ag,raiffeisen_bankengruppe_osterreich,schoellerbank_ag,sparda_bank_wien,volksbank_gruppe,volkskreditbank_ag,vr_bank_braunau" } diff --git a/config/docker_compose.toml b/config/docker_compose.toml index 0d615b11b5cf..d0a99254e9c7 100644 --- a/config/docker_compose.toml +++ b/config/docker_compose.toml @@ -53,6 +53,7 @@ recon_admin_api_key = "recon_test_admin" [user] password_validity_in_days = 90 +two_factor_auth_expiry_in_secs = 60 * 5; [locker] host = "" diff --git a/crates/api_models/src/events/user.rs b/crates/api_models/src/events/user.rs index b7d7adbf8e39..c6e3293bca13 100644 --- a/crates/api_models/src/events/user.rs +++ b/crates/api_models/src/events/user.rs @@ -17,7 +17,7 @@ use crate::user::{ RecoveryCodes, ResetPasswordRequest, RotatePasswordRequest, SendVerifyEmailRequest, SignInResponse, SignUpRequest, SignUpWithMerchantIdRequest, SwitchMerchantIdRequest, TokenOrPayloadResponse, TokenResponse, UpdateUserAccountDetailsRequest, UserFromEmailRequest, - UserMerchantCreate, VerifyEmailRequest, VerifyTotpRequest, + UserMerchantCreate, VerifyAccessCodeRequest, VerifyEmailRequest, VerifyTotpRequest, }; impl ApiEventMetric for DashboardEntryResponse { @@ -75,6 +75,7 @@ common_utils::impl_misc_api_event_type!( TokenResponse, UserFromEmailRequest, BeginTotpResponse, + VerifyAccessCodeRequest, VerifyTotpRequest, RecoveryCodes ); diff --git a/crates/api_models/src/user.rs b/crates/api_models/src/user.rs index 7864117856e6..0ed78b601f1f 100644 --- a/crates/api_models/src/user.rs +++ b/crates/api_models/src/user.rs @@ -258,6 +258,11 @@ pub struct VerifyTotpRequest { pub totp: Option>, } +#[derive(Debug, serde::Deserialize, serde::Serialize)] +pub struct VerifyAccessCodeRequest { + pub access_code: Secret, +} + #[derive(Debug, serde::Deserialize, serde::Serialize)] pub struct RecoveryCodes { pub recovery_codes: Vec>, diff --git a/crates/router/src/configs/settings.rs b/crates/router/src/configs/settings.rs index 9564a759ccd8..29188eae9ccc 100644 --- a/crates/router/src/configs/settings.rs +++ b/crates/router/src/configs/settings.rs @@ -395,6 +395,7 @@ pub struct Secrets { #[derive(Debug, Clone, Default, Deserialize)] pub struct UserSettings { pub password_validity_in_days: u16, + pub two_factor_auth_expiry_in_secs: u64, } #[derive(Debug, Deserialize, Clone)] diff --git a/crates/router/src/consts/user.rs b/crates/router/src/consts/user.rs index 33642205d57f..2f347dc3d3eb 100644 --- a/crates/router/src/consts/user.rs +++ b/crates/router/src/consts/user.rs @@ -14,3 +14,4 @@ pub const MAX_PASSWORD_LENGTH: usize = 70; pub const MIN_PASSWORD_LENGTH: usize = 8; pub const TOTP_PREFIX: &str = "TOTP_"; +pub const REDIS_RECOVERY_CODE_PREFIX: &str = "RC_"; diff --git a/crates/router/src/core/user.rs b/crates/router/src/core/user.rs index 7a0ef683e7a6..c3f463c582f5 100644 --- a/crates/router/src/core/user.rs +++ b/crates/router/src/core/user.rs @@ -24,7 +24,10 @@ use crate::{ routes::{app::ReqState, AppState}, services::{authentication as auth, authorization::roles, ApplicationResponse}, types::{domain, transformers::ForeignInto}, - utils, + utils::{ + self, + user::{password, two_factor_auth::insert_access_code_in_redis}, + }, }; pub mod dashboard_metadata; #[cfg(feature = "dummy_connector")] @@ -1766,3 +1769,55 @@ pub async fn generate_recovery_codes( recovery_codes: recovery_codes.into_inner(), })) } + +pub async fn verify_access_code( + state: AppState, + user_token: auth::UserFromSinglePurposeToken, + req: user_api::VerifyAccessCodeRequest, +) -> UserResponse { + let user_from_db: domain::UserFromStorage = state + .store + .find_user_by_id(&user_token.user_id) + .await + .change_context(UserErrors::InternalServerError)? + .into(); + + if user_from_db.get_totp_status() != TotpStatus::Set { + return Err(UserErrors::TotpNotSetup.into()); + } + + let access_codes = user_from_db + .get_recovery_codes() + .ok_or(UserErrors::InternalServerError)?; + + let matching_index = + password::get_correct_access_code_index(req.access_code, access_codes.clone())?; + + if matching_index.is_none() { + return Err(UserErrors::InvalidCredentials.into()); + } + + let mut updated_recovery_codes = access_codes; + if let Some(index) = matching_index { + updated_recovery_codes.remove(index); + } + + state + .store + .update_user_by_user_id( + user_from_db.get_user_id(), + storage_user::UserUpdate::TotpUpdate { + totp_status: None, + totp_secret: None, + totp_recovery_codes: Some(updated_recovery_codes), + }, + ) + .await + .change_context(UserErrors::InternalServerError)?; + + let _ = insert_access_code_in_redis(&state, user_from_db.get_user_id()) + .await + .map_err(|e| logger::error!(?e)); + + Ok(ApplicationResponse::StatusOk) +} diff --git a/crates/router/src/routes/app.rs b/crates/router/src/routes/app.rs index 0e152ec32c1d..54f92e8edef5 100644 --- a/crates/router/src/routes/app.rs +++ b/crates/router/src/routes/app.rs @@ -1212,6 +1212,7 @@ impl User { ) .service(web::resource("/totp/begin").route(web::get().to(totp_begin))) .service(web::resource("/totp/verify").route(web::post().to(totp_verify))) + .service(web::resource("/access_code/verify").route(web::post().to(access_code_verify))) .service( web::resource("/recovery_codes/generate") .route(web::get().to(generate_recovery_codes)), diff --git a/crates/router/src/routes/lock_utils.rs b/crates/router/src/routes/lock_utils.rs index 706726979dc1..e7e5c7a71c45 100644 --- a/crates/router/src/routes/lock_utils.rs +++ b/crates/router/src/routes/lock_utils.rs @@ -215,6 +215,7 @@ impl From for ApiIdentifier { | Flow::UpdateUserAccountDetails | Flow::TotpBegin | Flow::TotpVerify + | Flow::AccessCodeVerify | Flow::GenerateRecoveryCodes => Self::User, Flow::ListRoles diff --git a/crates/router/src/routes/user.rs b/crates/router/src/routes/user.rs index f542c446e498..ef5fb0ba617b 100644 --- a/crates/router/src/routes/user.rs +++ b/crates/router/src/routes/user.rs @@ -666,6 +666,24 @@ pub async fn totp_verify( .await } +pub async fn access_code_verify( + state: web::Data, + req: HttpRequest, + json_payload: web::Json, +) -> HttpResponse { + let flow = Flow::AccessCodeVerify; + Box::pin(api::server_wrap( + flow, + state.clone(), + &req, + json_payload.into_inner(), + |state, user, req_body, _| user_core::verify_access_code(state, user, req_body), + &auth::SinglePurposeJWTAuth(TokenPurpose::TOTP), + api_locking::LockAction::NotApplicable, + )) + .await +} + pub async fn generate_recovery_codes(state: web::Data, req: HttpRequest) -> HttpResponse { let flow = Flow::GenerateRecoveryCodes; Box::pin(api::server_wrap( diff --git a/crates/router/src/types/domain/user.rs b/crates/router/src/types/domain/user.rs index 051e6ccf38ed..0704e51e7813 100644 --- a/crates/router/src/types/domain/user.rs +++ b/crates/router/src/types/domain/user.rs @@ -930,6 +930,10 @@ impl UserFromStorage { self.0.totp_status } + pub fn get_recovery_codes(&self) -> Option>> { + self.0.totp_recovery_codes.clone() + } + pub async fn decrypt_and_get_totp_secret( &self, state: &AppState, diff --git a/crates/router/src/utils/user/password.rs b/crates/router/src/utils/user/password.rs index beb87c325b62..78658dd0d08b 100644 --- a/crates/router/src/utils/user/password.rs +++ b/crates/router/src/utils/user/password.rs @@ -40,6 +40,18 @@ pub fn is_correct_password( .change_context(UserErrors::InternalServerError) } +pub fn get_correct_access_code_index( + candidate: Secret, + access_codes: Vec>, +) -> CustomResult, UserErrors> { + Ok(access_codes + .into_iter() + .map(|access_code| is_correct_password(candidate.clone(), access_code)) + .collect::, _>>()? + .into_iter() + .position(|x| x)) +} + pub fn get_temp_password() -> Secret { let uuid_pass = uuid::Uuid::new_v4().to_string(); let mut rng = rand::thread_rng(); diff --git a/crates/router/src/utils/user/two_factor_auth.rs b/crates/router/src/utils/user/two_factor_auth.rs index 62bcf2f7eb15..ca8954bfdb47 100644 --- a/crates/router/src/utils/user/two_factor_auth.rs +++ b/crates/router/src/utils/user/two_factor_auth.rs @@ -50,3 +50,18 @@ fn get_redis_connection(state: &AppState) -> UserResult .change_context(UserErrors::InternalServerError) .attach_printable("Failed to get redis connection") } + +fn expiry_to_i64(expiry: u64) -> UserResult { + i64::try_from(expiry).change_context(UserErrors::InternalServerError) +} + +pub async fn insert_access_code_in_redis(state: &AppState, user_id: &str) -> UserResult<()> { + let redis_conn = get_redis_connection(state)?; + let key = format!("{}{}", consts::user::REDIS_RECOVERY_CODE_PREFIX, user_id); + let expiry = expiry_to_i64(state.conf.user.two_factor_auth_expiry_in_secs) + .change_context(UserErrors::InternalServerError)?; + redis_conn + .set_key_with_expiry(key.as_str(), true, expiry) + .await + .change_context(UserErrors::InternalServerError) +} diff --git a/crates/router_env/src/logger/types.rs b/crates/router_env/src/logger/types.rs index 0e35aba31741..447fa10af3f3 100644 --- a/crates/router_env/src/logger/types.rs +++ b/crates/router_env/src/logger/types.rs @@ -406,6 +406,8 @@ pub enum Flow { TotpBegin, /// Verify TOTP TotpVerify, + /// Verify Access Code + AccessCodeVerify, /// Generate or Regenerate recovery codes GenerateRecoveryCodes, /// List initial webhook delivery attempts diff --git a/loadtest/config/development.toml b/loadtest/config/development.toml index a104f5760b40..ad1d0e3174e0 100644 --- a/loadtest/config/development.toml +++ b/loadtest/config/development.toml @@ -30,6 +30,7 @@ jwt_secret = "secret" [user] password_validity_in_days = 90 +two_factor_auth_expiry_in_secs = 60 * 5; [locker] host = "" From db64e4aa806ae659767338b430f992c179ad5c36 Mon Sep 17 00:00:00 2001 From: Apoorv Dixit Date: Wed, 22 May 2024 23:20:38 +0530 Subject: [PATCH 02/11] fix: config for 2fa --- config/config.example.toml | 2 +- config/deployments/integration_test.toml | 2 +- config/deployments/production.toml | 2 +- config/deployments/sandbox.toml | 2 +- config/development.toml | 2 +- config/docker_compose.toml | 2 +- loadtest/config/development.toml | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/config/config.example.toml b/config/config.example.toml index 776ca079dc39..3b2f3ec0629b 100644 --- a/config/config.example.toml +++ b/config/config.example.toml @@ -352,7 +352,7 @@ sts_role_session_name = "" # An identifier for the assumed role session, used to [user] password_validity_in_days = 90 # Number of days after which password should be updated -two_factor_auth_expiry_in_secs = 60 * 5; # Number of seconds after which 2FA should be done again if doing update/change from inside +two_factor_auth_expiry_in_secs = 300 # Number of seconds after which 2FA should be done again if doing update/change from inside #tokenization configuration which describe token lifetime and payment method for specific connector [tokenization] diff --git a/config/deployments/integration_test.toml b/config/deployments/integration_test.toml index 93dadc4617fa..9551cd2a80d8 100644 --- a/config/deployments/integration_test.toml +++ b/config/deployments/integration_test.toml @@ -113,7 +113,7 @@ slack_invite_url = "https://join.slack.com/t/hyperswitch-io/shared_invite/zt-2aw [user] password_validity_in_days = 90 -two_factor_auth_expiry_in_secs = 60 * 5; +two_factor_auth_expiry_in_secs = 300 [frm] enabled = true diff --git a/config/deployments/production.toml b/config/deployments/production.toml index b4931c1591ff..5f9408674383 100644 --- a/config/deployments/production.toml +++ b/config/deployments/production.toml @@ -120,7 +120,7 @@ slack_invite_url = "https://join.slack.com/t/hyperswitch-io/shared_invite/zt-2aw [user] password_validity_in_days = 90 -two_factor_auth_expiry_in_secs = 60 * 5; +two_factor_auth_expiry_in_secs = 300 [frm] enabled = false diff --git a/config/deployments/sandbox.toml b/config/deployments/sandbox.toml index 640596f5fec8..d5f63524e6bd 100644 --- a/config/deployments/sandbox.toml +++ b/config/deployments/sandbox.toml @@ -120,7 +120,7 @@ slack_invite_url = "https://join.slack.com/t/hyperswitch-io/shared_invite/zt-2aw [user] password_validity_in_days = 90 -two_factor_auth_expiry_in_secs = 60 * 5; +two_factor_auth_expiry_in_secs = 300 [frm] enabled = true diff --git a/config/development.toml b/config/development.toml index 7dc0caa6dbae..a9f7f4d7b4d8 100644 --- a/config/development.toml +++ b/config/development.toml @@ -269,7 +269,7 @@ sts_role_session_name = "" [user] password_validity_in_days = 90 -two_factor_auth_expiry_in_secs = 60 * 5; +two_factor_auth_expiry_in_secs = 300 [bank_config.eps] stripe = { banks = "arzte_und_apotheker_bank,austrian_anadi_bank_ag,bank_austria,bankhaus_carl_spangler,bankhaus_schelhammer_und_schattera_ag,bawag_psk_ag,bks_bank_ag,brull_kallmus_bank_ag,btv_vier_lander_bank,capital_bank_grawe_gruppe_ag,dolomitenbank,easybank_ag,erste_bank_und_sparkassen,hypo_alpeadriabank_international_ag,hypo_noe_lb_fur_niederosterreich_u_wien,hypo_oberosterreich_salzburg_steiermark,hypo_tirol_bank_ag,hypo_vorarlberg_bank_ag,hypo_bank_burgenland_aktiengesellschaft,marchfelder_bank,oberbank_ag,raiffeisen_bankengruppe_osterreich,schoellerbank_ag,sparda_bank_wien,volksbank_gruppe,volkskreditbank_ag,vr_bank_braunau" } diff --git a/config/docker_compose.toml b/config/docker_compose.toml index d0a99254e9c7..6661d164032d 100644 --- a/config/docker_compose.toml +++ b/config/docker_compose.toml @@ -53,7 +53,7 @@ recon_admin_api_key = "recon_test_admin" [user] password_validity_in_days = 90 -two_factor_auth_expiry_in_secs = 60 * 5; +two_factor_auth_expiry_in_secs = 300 [locker] host = "" diff --git a/loadtest/config/development.toml b/loadtest/config/development.toml index ad1d0e3174e0..2c3e83ad2ba9 100644 --- a/loadtest/config/development.toml +++ b/loadtest/config/development.toml @@ -30,7 +30,7 @@ jwt_secret = "secret" [user] password_validity_in_days = 90 -two_factor_auth_expiry_in_secs = 60 * 5; +two_factor_auth_expiry_in_secs = 300 [locker] host = "" From 7343626f6e3620a252746f61144266cc6da0845a Mon Sep 17 00:00:00 2001 From: Apoorv Dixit Date: Thu, 23 May 2024 01:09:37 +0530 Subject: [PATCH 03/11] fix: rename access code to recovery code --- crates/api_models/src/user.rs | 2 +- crates/router/src/core/user.rs | 12 ++++++------ crates/router/src/routes/app.rs | 2 +- crates/router/src/routes/user.rs | 4 ++-- crates/router/src/utils/user/password.rs | 8 ++++---- crates/router/src/utils/user/two_factor_auth.rs | 2 +- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/crates/api_models/src/user.rs b/crates/api_models/src/user.rs index 0ed78b601f1f..70b3122d1f78 100644 --- a/crates/api_models/src/user.rs +++ b/crates/api_models/src/user.rs @@ -260,7 +260,7 @@ pub struct VerifyTotpRequest { #[derive(Debug, serde::Deserialize, serde::Serialize)] pub struct VerifyAccessCodeRequest { - pub access_code: Secret, + pub recovery_code: Secret, } #[derive(Debug, serde::Deserialize, serde::Serialize)] diff --git a/crates/router/src/core/user.rs b/crates/router/src/core/user.rs index c3f463c582f5..68fadd4a5db1 100644 --- a/crates/router/src/core/user.rs +++ b/crates/router/src/core/user.rs @@ -26,7 +26,7 @@ use crate::{ types::{domain, transformers::ForeignInto}, utils::{ self, - user::{password, two_factor_auth::insert_access_code_in_redis}, + user::{password, two_factor_auth::insert_recovery_code_in_redis}, }, }; pub mod dashboard_metadata; @@ -1770,7 +1770,7 @@ pub async fn generate_recovery_codes( })) } -pub async fn verify_access_code( +pub async fn verify_recovery_code( state: AppState, user_token: auth::UserFromSinglePurposeToken, req: user_api::VerifyAccessCodeRequest, @@ -1786,18 +1786,18 @@ pub async fn verify_access_code( return Err(UserErrors::TotpNotSetup.into()); } - let access_codes = user_from_db + let recovery_codes = user_from_db .get_recovery_codes() .ok_or(UserErrors::InternalServerError)?; let matching_index = - password::get_correct_access_code_index(req.access_code, access_codes.clone())?; + password::get_correct_recovery_code_index(req.recovery_code, recovery_codes.clone())?; if matching_index.is_none() { return Err(UserErrors::InvalidCredentials.into()); } - let mut updated_recovery_codes = access_codes; + let mut updated_recovery_codes = recovery_codes; if let Some(index) = matching_index { updated_recovery_codes.remove(index); } @@ -1815,7 +1815,7 @@ pub async fn verify_access_code( .await .change_context(UserErrors::InternalServerError)?; - let _ = insert_access_code_in_redis(&state, user_from_db.get_user_id()) + let _ = insert_recovery_code_in_redis(&state, user_from_db.get_user_id()) .await .map_err(|e| logger::error!(?e)); diff --git a/crates/router/src/routes/app.rs b/crates/router/src/routes/app.rs index 54f92e8edef5..662f7eec6923 100644 --- a/crates/router/src/routes/app.rs +++ b/crates/router/src/routes/app.rs @@ -1212,7 +1212,7 @@ impl User { ) .service(web::resource("/totp/begin").route(web::get().to(totp_begin))) .service(web::resource("/totp/verify").route(web::post().to(totp_verify))) - .service(web::resource("/access_code/verify").route(web::post().to(access_code_verify))) + .service(web::resource("/recovery_code/verify").route(web::post().to(recovery_code_verify))) .service( web::resource("/recovery_codes/generate") .route(web::get().to(generate_recovery_codes)), diff --git a/crates/router/src/routes/user.rs b/crates/router/src/routes/user.rs index ef5fb0ba617b..e189b3410230 100644 --- a/crates/router/src/routes/user.rs +++ b/crates/router/src/routes/user.rs @@ -666,7 +666,7 @@ pub async fn totp_verify( .await } -pub async fn access_code_verify( +pub async fn recovery_code_verify( state: web::Data, req: HttpRequest, json_payload: web::Json, @@ -677,7 +677,7 @@ pub async fn access_code_verify( state.clone(), &req, json_payload.into_inner(), - |state, user, req_body, _| user_core::verify_access_code(state, user, req_body), + |state, user, req_body, _| user_core::verify_recovery_code(state, user, req_body), &auth::SinglePurposeJWTAuth(TokenPurpose::TOTP), api_locking::LockAction::NotApplicable, )) diff --git a/crates/router/src/utils/user/password.rs b/crates/router/src/utils/user/password.rs index 78658dd0d08b..4ccb6e7ae9b1 100644 --- a/crates/router/src/utils/user/password.rs +++ b/crates/router/src/utils/user/password.rs @@ -40,13 +40,13 @@ pub fn is_correct_password( .change_context(UserErrors::InternalServerError) } -pub fn get_correct_access_code_index( +pub fn get_correct_recovery_code_index( candidate: Secret, - access_codes: Vec>, + recovery_codes: Vec>, ) -> CustomResult, UserErrors> { - Ok(access_codes + Ok(recovery_codes .into_iter() - .map(|access_code| is_correct_password(candidate.clone(), access_code)) + .map(|recovery_code| is_correct_password(candidate.clone(), recovery_code)) .collect::, _>>()? .into_iter() .position(|x| x)) diff --git a/crates/router/src/utils/user/two_factor_auth.rs b/crates/router/src/utils/user/two_factor_auth.rs index ca8954bfdb47..6372cb72e006 100644 --- a/crates/router/src/utils/user/two_factor_auth.rs +++ b/crates/router/src/utils/user/two_factor_auth.rs @@ -55,7 +55,7 @@ fn expiry_to_i64(expiry: u64) -> UserResult { i64::try_from(expiry).change_context(UserErrors::InternalServerError) } -pub async fn insert_access_code_in_redis(state: &AppState, user_id: &str) -> UserResult<()> { +pub async fn insert_recovery_code_in_redis(state: &AppState, user_id: &str) -> UserResult<()> { let redis_conn = get_redis_connection(state)?; let key = format!("{}{}", consts::user::REDIS_RECOVERY_CODE_PREFIX, user_id); let expiry = expiry_to_i64(state.conf.user.two_factor_auth_expiry_in_secs) From 27784d3716c2b5a5c5bdbe617b32605a808285bc Mon Sep 17 00:00:00 2001 From: Apoorv Dixit Date: Thu, 23 May 2024 01:15:13 +0530 Subject: [PATCH 04/11] fix: add error for invalid recovery code --- crates/router/src/core/errors/user.rs | 6 ++++++ crates/router/src/core/user.rs | 2 +- crates/router/src/routes/app.rs | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/router/src/core/errors/user.rs b/crates/router/src/core/errors/user.rs index 2d1c196f5df7..844b0ebe13eb 100644 --- a/crates/router/src/core/errors/user.rs +++ b/crates/router/src/core/errors/user.rs @@ -72,6 +72,8 @@ pub enum UserErrors { InvalidTotp, #[error("TotpRequired")] TotpRequired, + #[error("InvalidRecoveryCode")] + InvalidRecoveryCode, } impl common_utils::errors::ErrorSwitch for UserErrors { @@ -184,6 +186,9 @@ impl common_utils::errors::ErrorSwitch { AER::BadRequest(ApiError::new(sub_code, 38, self.get_error_message(), None)) } + Self::InvalidRecoveryCode => { + AER::BadRequest(ApiError::new(sub_code, 39, self.get_error_message(), None)) + } } } } @@ -223,6 +228,7 @@ impl UserErrors { Self::TotpNotSetup => "TOTP not setup", Self::InvalidTotp => "Invalid TOTP", Self::TotpRequired => "TOTP required", + Self::InvalidRecoveryCode => "Invalid Recovery Code" } } } diff --git a/crates/router/src/core/user.rs b/crates/router/src/core/user.rs index 68fadd4a5db1..70e8559f4a12 100644 --- a/crates/router/src/core/user.rs +++ b/crates/router/src/core/user.rs @@ -1794,7 +1794,7 @@ pub async fn verify_recovery_code( password::get_correct_recovery_code_index(req.recovery_code, recovery_codes.clone())?; if matching_index.is_none() { - return Err(UserErrors::InvalidCredentials.into()); + return Err(UserErrors::InvalidRecoveryCode.into()); } let mut updated_recovery_codes = recovery_codes; diff --git a/crates/router/src/routes/app.rs b/crates/router/src/routes/app.rs index 662f7eec6923..325cfae59361 100644 --- a/crates/router/src/routes/app.rs +++ b/crates/router/src/routes/app.rs @@ -1212,7 +1212,7 @@ impl User { ) .service(web::resource("/totp/begin").route(web::get().to(totp_begin))) .service(web::resource("/totp/verify").route(web::post().to(totp_verify))) - .service(web::resource("/recovery_code/verify").route(web::post().to(recovery_code_verify))) + .service(web::resource("/recovery_codes/verify").route(web::post().to(recovery_code_verify))) .service( web::resource("/recovery_codes/generate") .route(web::get().to(generate_recovery_codes)), From ce598b9da385efcd5b962b937dfc611d1a7c0ddc Mon Sep 17 00:00:00 2001 From: "hyperswitch-bot[bot]" <148525504+hyperswitch-bot[bot]@users.noreply.github.com> Date: Wed, 22 May 2024 19:55:07 +0000 Subject: [PATCH 05/11] chore: run formatter --- crates/router/src/core/errors/user.rs | 2 +- crates/router/src/routes/app.rs | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/router/src/core/errors/user.rs b/crates/router/src/core/errors/user.rs index 844b0ebe13eb..a1f79f8954fc 100644 --- a/crates/router/src/core/errors/user.rs +++ b/crates/router/src/core/errors/user.rs @@ -228,7 +228,7 @@ impl UserErrors { Self::TotpNotSetup => "TOTP not setup", Self::InvalidTotp => "Invalid TOTP", Self::TotpRequired => "TOTP required", - Self::InvalidRecoveryCode => "Invalid Recovery Code" + Self::InvalidRecoveryCode => "Invalid Recovery Code", } } } diff --git a/crates/router/src/routes/app.rs b/crates/router/src/routes/app.rs index 325cfae59361..a1af9e8fc6d3 100644 --- a/crates/router/src/routes/app.rs +++ b/crates/router/src/routes/app.rs @@ -1212,7 +1212,9 @@ impl User { ) .service(web::resource("/totp/begin").route(web::get().to(totp_begin))) .service(web::resource("/totp/verify").route(web::post().to(totp_verify))) - .service(web::resource("/recovery_codes/verify").route(web::post().to(recovery_code_verify))) + .service( + web::resource("/recovery_codes/verify").route(web::post().to(recovery_code_verify)), + ) .service( web::resource("/recovery_codes/generate") .route(web::get().to(generate_recovery_codes)), From 9b4bdfe712b89b3ec4ffc51f843d2bbbe4250dc8 Mon Sep 17 00:00:00 2001 From: Apoorv Dixit Date: Thu, 23 May 2024 01:29:23 +0530 Subject: [PATCH 06/11] fix: rename get index func for recovery code --- crates/router/src/core/user.rs | 2 +- crates/router/src/utils/user/password.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/router/src/core/user.rs b/crates/router/src/core/user.rs index 70e8559f4a12..7012df317502 100644 --- a/crates/router/src/core/user.rs +++ b/crates/router/src/core/user.rs @@ -1791,7 +1791,7 @@ pub async fn verify_recovery_code( .ok_or(UserErrors::InternalServerError)?; let matching_index = - password::get_correct_recovery_code_index(req.recovery_code, recovery_codes.clone())?; + password::get_index_for_correct_recovery_code(req.recovery_code, recovery_codes.clone())?; if matching_index.is_none() { return Err(UserErrors::InvalidRecoveryCode.into()); diff --git a/crates/router/src/utils/user/password.rs b/crates/router/src/utils/user/password.rs index 4ccb6e7ae9b1..cb342841abdf 100644 --- a/crates/router/src/utils/user/password.rs +++ b/crates/router/src/utils/user/password.rs @@ -40,7 +40,7 @@ pub fn is_correct_password( .change_context(UserErrors::InternalServerError) } -pub fn get_correct_recovery_code_index( +pub fn get_index_for_correct_recovery_code( candidate: Secret, recovery_codes: Vec>, ) -> CustomResult, UserErrors> { From 4b260046838d792e12a088e4c2ad487b50c05bdc Mon Sep 17 00:00:00 2001 From: Apoorv Dixit Date: Thu, 23 May 2024 15:19:33 +0530 Subject: [PATCH 07/11] fix: resolve review comments --- config/config.example.toml | 2 +- crates/api_models/src/events/user.rs | 4 +-- crates/api_models/src/user.rs | 2 +- crates/router/src/configs/settings.rs | 2 +- crates/router/src/core/user.rs | 28 ++++++++----------- crates/router/src/routes/app.rs | 15 +++++----- crates/router/src/routes/lock_utils.rs | 2 +- crates/router/src/routes/user.rs | 6 ++-- crates/router/src/utils/user/password.rs | 13 +++++---- .../router/src/utils/user/two_factor_auth.rs | 12 ++++---- crates/router_env/src/logger/types.rs | 2 +- 11 files changed, 41 insertions(+), 47 deletions(-) diff --git a/config/config.example.toml b/config/config.example.toml index 3b2f3ec0629b..d5bdfb5a6a7c 100644 --- a/config/config.example.toml +++ b/config/config.example.toml @@ -352,7 +352,7 @@ sts_role_session_name = "" # An identifier for the assumed role session, used to [user] password_validity_in_days = 90 # Number of days after which password should be updated -two_factor_auth_expiry_in_secs = 300 # Number of seconds after which 2FA should be done again if doing update/change from inside +two_factor_auth_expiry_in_secs = 300 # Number of seconds after which 2FA should be done again if doing update/change from inside #tokenization configuration which describe token lifetime and payment method for specific connector [tokenization] diff --git a/crates/api_models/src/events/user.rs b/crates/api_models/src/events/user.rs index c6e3293bca13..a472b3a76e68 100644 --- a/crates/api_models/src/events/user.rs +++ b/crates/api_models/src/events/user.rs @@ -17,7 +17,7 @@ use crate::user::{ RecoveryCodes, ResetPasswordRequest, RotatePasswordRequest, SendVerifyEmailRequest, SignInResponse, SignUpRequest, SignUpWithMerchantIdRequest, SwitchMerchantIdRequest, TokenOrPayloadResponse, TokenResponse, UpdateUserAccountDetailsRequest, UserFromEmailRequest, - UserMerchantCreate, VerifyAccessCodeRequest, VerifyEmailRequest, VerifyTotpRequest, + UserMerchantCreate, VerifyEmailRequest, VerifyRecoveryCodeRequest, VerifyTotpRequest, }; impl ApiEventMetric for DashboardEntryResponse { @@ -75,7 +75,7 @@ common_utils::impl_misc_api_event_type!( TokenResponse, UserFromEmailRequest, BeginTotpResponse, - VerifyAccessCodeRequest, + VerifyRecoveryCodeRequest, VerifyTotpRequest, RecoveryCodes ); diff --git a/crates/api_models/src/user.rs b/crates/api_models/src/user.rs index 70b3122d1f78..1127767ef6c6 100644 --- a/crates/api_models/src/user.rs +++ b/crates/api_models/src/user.rs @@ -259,7 +259,7 @@ pub struct VerifyTotpRequest { } #[derive(Debug, serde::Deserialize, serde::Serialize)] -pub struct VerifyAccessCodeRequest { +pub struct VerifyRecoveryCodeRequest { pub recovery_code: Secret, } diff --git a/crates/router/src/configs/settings.rs b/crates/router/src/configs/settings.rs index 29188eae9ccc..b8a315f944b8 100644 --- a/crates/router/src/configs/settings.rs +++ b/crates/router/src/configs/settings.rs @@ -395,7 +395,7 @@ pub struct Secrets { #[derive(Debug, Clone, Default, Deserialize)] pub struct UserSettings { pub password_validity_in_days: u16, - pub two_factor_auth_expiry_in_secs: u64, + pub two_factor_auth_expiry_in_secs: i64, } #[derive(Debug, Deserialize, Clone)] diff --git a/crates/router/src/core/user.rs b/crates/router/src/core/user.rs index 7012df317502..ebd721dfaa79 100644 --- a/crates/router/src/core/user.rs +++ b/crates/router/src/core/user.rs @@ -24,10 +24,7 @@ use crate::{ routes::{app::ReqState, AppState}, services::{authentication as auth, authorization::roles, ApplicationResponse}, types::{domain, transformers::ForeignInto}, - utils::{ - self, - user::{password, two_factor_auth::insert_recovery_code_in_redis}, - }, + utils::{self, user::two_factor_auth::insert_recovery_code_in_redis}, }; pub mod dashboard_metadata; #[cfg(feature = "dummy_connector")] @@ -1773,7 +1770,7 @@ pub async fn generate_recovery_codes( pub async fn verify_recovery_code( state: AppState, user_token: auth::UserFromSinglePurposeToken, - req: user_api::VerifyAccessCodeRequest, + req: user_api::VerifyRecoveryCodeRequest, ) -> UserResponse { let user_from_db: domain::UserFromStorage = state .store @@ -1786,21 +1783,20 @@ pub async fn verify_recovery_code( return Err(UserErrors::TotpNotSetup.into()); } - let recovery_codes = user_from_db + let mut recovery_codes = user_from_db .get_recovery_codes() .ok_or(UserErrors::InternalServerError)?; - let matching_index = - password::get_index_for_correct_recovery_code(req.recovery_code, recovery_codes.clone())?; + // let matching_index = + // utils::user::password::get_index_for_correct_recovery_code(req.recovery_code, recovery_codes.clone())?; - if matching_index.is_none() { - return Err(UserErrors::InvalidRecoveryCode.into()); - } + let matching_index = utils::user::password::get_index_for_correct_recovery_code( + req.recovery_code, + recovery_codes.clone(), + )? + .ok_or(UserErrors::InvalidRecoveryCode)?; - let mut updated_recovery_codes = recovery_codes; - if let Some(index) = matching_index { - updated_recovery_codes.remove(index); - } + let _ = recovery_codes.remove(matching_index); state .store @@ -1809,7 +1805,7 @@ pub async fn verify_recovery_code( storage_user::UserUpdate::TotpUpdate { totp_status: None, totp_secret: None, - totp_recovery_codes: Some(updated_recovery_codes), + totp_recovery_codes: Some(recovery_codes), }, ) .await diff --git a/crates/router/src/routes/app.rs b/crates/router/src/routes/app.rs index a1af9e8fc6d3..95e8ca94ea2b 100644 --- a/crates/router/src/routes/app.rs +++ b/crates/router/src/routes/app.rs @@ -1211,14 +1211,13 @@ impl User { .route(web::post().to(set_dashboard_metadata)), ) .service(web::resource("/totp/begin").route(web::get().to(totp_begin))) - .service(web::resource("/totp/verify").route(web::post().to(totp_verify))) - .service( - web::resource("/recovery_codes/verify").route(web::post().to(recovery_code_verify)), - ) - .service( - web::resource("/recovery_codes/generate") - .route(web::get().to(generate_recovery_codes)), - ); + .service(web::resource("/totp/verify").route(web::post().to(totp_verify))); + + route = route.service( + web::scope("/recovery_code") + .service(web::resource("/verify").route(web::post().to(verify_recovery_code))) + .service(web::resource("/generate").route(web::post().to(generate_recovery_codes))), + ); #[cfg(feature = "email")] { diff --git a/crates/router/src/routes/lock_utils.rs b/crates/router/src/routes/lock_utils.rs index e7e5c7a71c45..79421ceca805 100644 --- a/crates/router/src/routes/lock_utils.rs +++ b/crates/router/src/routes/lock_utils.rs @@ -215,7 +215,7 @@ impl From for ApiIdentifier { | Flow::UpdateUserAccountDetails | Flow::TotpBegin | Flow::TotpVerify - | Flow::AccessCodeVerify + | Flow::RecoveryCodeVerify | Flow::GenerateRecoveryCodes => Self::User, Flow::ListRoles diff --git a/crates/router/src/routes/user.rs b/crates/router/src/routes/user.rs index e189b3410230..773ade93acce 100644 --- a/crates/router/src/routes/user.rs +++ b/crates/router/src/routes/user.rs @@ -666,12 +666,12 @@ pub async fn totp_verify( .await } -pub async fn recovery_code_verify( +pub async fn verify_recovery_code( state: web::Data, req: HttpRequest, - json_payload: web::Json, + json_payload: web::Json, ) -> HttpResponse { - let flow = Flow::AccessCodeVerify; + let flow = Flow::RecoveryCodeVerify; Box::pin(api::server_wrap( flow, state.clone(), diff --git a/crates/router/src/utils/user/password.rs b/crates/router/src/utils/user/password.rs index cb342841abdf..33de4c66963c 100644 --- a/crates/router/src/utils/user/password.rs +++ b/crates/router/src/utils/user/password.rs @@ -44,12 +44,13 @@ pub fn get_index_for_correct_recovery_code( candidate: Secret, recovery_codes: Vec>, ) -> CustomResult, UserErrors> { - Ok(recovery_codes - .into_iter() - .map(|recovery_code| is_correct_password(candidate.clone(), recovery_code)) - .collect::, _>>()? - .into_iter() - .position(|x| x)) + for (index, recovery_code) in recovery_codes.into_iter().enumerate() { + let is_match = is_correct_password(candidate.clone(), recovery_code)?; + if is_match { + return Ok(Some(index)); + } + } + return Ok(None); } pub fn get_temp_password() -> Secret { diff --git a/crates/router/src/utils/user/two_factor_auth.rs b/crates/router/src/utils/user/two_factor_auth.rs index 6372cb72e006..c9cfb4d04486 100644 --- a/crates/router/src/utils/user/two_factor_auth.rs +++ b/crates/router/src/utils/user/two_factor_auth.rs @@ -51,17 +51,15 @@ fn get_redis_connection(state: &AppState) -> UserResult .attach_printable("Failed to get redis connection") } -fn expiry_to_i64(expiry: u64) -> UserResult { - i64::try_from(expiry).change_context(UserErrors::InternalServerError) -} - pub async fn insert_recovery_code_in_redis(state: &AppState, user_id: &str) -> UserResult<()> { let redis_conn = get_redis_connection(state)?; let key = format!("{}{}", consts::user::REDIS_RECOVERY_CODE_PREFIX, user_id); - let expiry = expiry_to_i64(state.conf.user.two_factor_auth_expiry_in_secs) - .change_context(UserErrors::InternalServerError)?; redis_conn - .set_key_with_expiry(key.as_str(), true, expiry) + .set_key_with_expiry( + key.as_str(), + common_utils::date_time::now_unix_timestamp(), + state.conf.user.two_factor_auth_expiry_in_secs, + ) .await .change_context(UserErrors::InternalServerError) } diff --git a/crates/router_env/src/logger/types.rs b/crates/router_env/src/logger/types.rs index 447fa10af3f3..b9e2ee9ee562 100644 --- a/crates/router_env/src/logger/types.rs +++ b/crates/router_env/src/logger/types.rs @@ -407,7 +407,7 @@ pub enum Flow { /// Verify TOTP TotpVerify, /// Verify Access Code - AccessCodeVerify, + RecoveryCodeVerify, /// Generate or Regenerate recovery codes GenerateRecoveryCodes, /// List initial webhook delivery attempts From 774b4822bec791f0cf05134a64d10c0edc14dd28 Mon Sep 17 00:00:00 2001 From: Apoorv Dixit Date: Thu, 23 May 2024 15:47:45 +0530 Subject: [PATCH 08/11] fix: change error for verify access code --- crates/router/src/core/errors/user.rs | 6 ++++++ crates/router/src/core/user.rs | 12 ++++-------- crates/router/src/routes/lock_utils.rs | 2 +- crates/router/src/routes/user.rs | 2 +- crates/router_env/src/logger/types.rs | 2 +- 5 files changed, 13 insertions(+), 11 deletions(-) diff --git a/crates/router/src/core/errors/user.rs b/crates/router/src/core/errors/user.rs index a1f79f8954fc..c6a962aba164 100644 --- a/crates/router/src/core/errors/user.rs +++ b/crates/router/src/core/errors/user.rs @@ -74,6 +74,8 @@ pub enum UserErrors { TotpRequired, #[error("InvalidRecoveryCode")] InvalidRecoveryCode, + #[error("TwoFactorAuthNotSetup")] + TwoFactorAuthNotSetup, } impl common_utils::errors::ErrorSwitch for UserErrors { @@ -189,6 +191,9 @@ impl common_utils::errors::ErrorSwitch { AER::BadRequest(ApiError::new(sub_code, 39, self.get_error_message(), None)) } + Self::TwoFactorAuthNotSetup => { + AER::BadRequest(ApiError::new(sub_code, 39, self.get_error_message(), None)) + } } } } @@ -229,6 +234,7 @@ impl UserErrors { Self::InvalidTotp => "Invalid TOTP", Self::TotpRequired => "TOTP required", Self::InvalidRecoveryCode => "Invalid Recovery Code", + Self::TwoFactorAuthNotSetup => "Two factor auth not setup" } } } diff --git a/crates/router/src/core/user.rs b/crates/router/src/core/user.rs index ebd721dfaa79..8852840d47a8 100644 --- a/crates/router/src/core/user.rs +++ b/crates/router/src/core/user.rs @@ -1780,16 +1780,13 @@ pub async fn verify_recovery_code( .into(); if user_from_db.get_totp_status() != TotpStatus::Set { - return Err(UserErrors::TotpNotSetup.into()); + return Err(UserErrors::TwoFactorAuthNotSetup.into()); } let mut recovery_codes = user_from_db .get_recovery_codes() .ok_or(UserErrors::InternalServerError)?; - // let matching_index = - // utils::user::password::get_index_for_correct_recovery_code(req.recovery_code, recovery_codes.clone())?; - let matching_index = utils::user::password::get_index_for_correct_recovery_code( req.recovery_code, recovery_codes.clone(), @@ -1797,6 +1794,9 @@ pub async fn verify_recovery_code( .ok_or(UserErrors::InvalidRecoveryCode)?; let _ = recovery_codes.remove(matching_index); + let _ = insert_recovery_code_in_redis(&state, user_from_db.get_user_id()) + .await + .map_err(|e| logger::error!(?e)); state .store @@ -1811,9 +1811,5 @@ pub async fn verify_recovery_code( .await .change_context(UserErrors::InternalServerError)?; - let _ = insert_recovery_code_in_redis(&state, user_from_db.get_user_id()) - .await - .map_err(|e| logger::error!(?e)); - Ok(ApplicationResponse::StatusOk) } diff --git a/crates/router/src/routes/lock_utils.rs b/crates/router/src/routes/lock_utils.rs index 79421ceca805..b59d30c08412 100644 --- a/crates/router/src/routes/lock_utils.rs +++ b/crates/router/src/routes/lock_utils.rs @@ -216,7 +216,7 @@ impl From for ApiIdentifier { | Flow::TotpBegin | Flow::TotpVerify | Flow::RecoveryCodeVerify - | Flow::GenerateRecoveryCodes => Self::User, + | Flow::RecoveryCodesGenerate => Self::User, Flow::ListRoles | Flow::GetRole diff --git a/crates/router/src/routes/user.rs b/crates/router/src/routes/user.rs index 773ade93acce..0492418caa07 100644 --- a/crates/router/src/routes/user.rs +++ b/crates/router/src/routes/user.rs @@ -685,7 +685,7 @@ pub async fn verify_recovery_code( } pub async fn generate_recovery_codes(state: web::Data, req: HttpRequest) -> HttpResponse { - let flow = Flow::GenerateRecoveryCodes; + let flow = Flow::RecoveryCodesGenerate; Box::pin(api::server_wrap( flow, state.clone(), diff --git a/crates/router_env/src/logger/types.rs b/crates/router_env/src/logger/types.rs index b9e2ee9ee562..250c2d43b12c 100644 --- a/crates/router_env/src/logger/types.rs +++ b/crates/router_env/src/logger/types.rs @@ -409,7 +409,7 @@ pub enum Flow { /// Verify Access Code RecoveryCodeVerify, /// Generate or Regenerate recovery codes - GenerateRecoveryCodes, + RecoveryCodesGenerate, /// List initial webhook delivery attempts WebhookEventInitialDeliveryAttemptList, /// List delivery attempts for a webhook event From ed42420d9e5042e3835d2521766aa69d4ff50382 Mon Sep 17 00:00:00 2001 From: "hyperswitch-bot[bot]" <148525504+hyperswitch-bot[bot]@users.noreply.github.com> Date: Thu, 23 May 2024 10:18:44 +0000 Subject: [PATCH 09/11] chore: run formatter --- crates/router/src/core/errors/user.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/router/src/core/errors/user.rs b/crates/router/src/core/errors/user.rs index c6a962aba164..d42bf1659b8f 100644 --- a/crates/router/src/core/errors/user.rs +++ b/crates/router/src/core/errors/user.rs @@ -234,7 +234,7 @@ impl UserErrors { Self::InvalidTotp => "Invalid TOTP", Self::TotpRequired => "TOTP required", Self::InvalidRecoveryCode => "Invalid Recovery Code", - Self::TwoFactorAuthNotSetup => "Two factor auth not setup" + Self::TwoFactorAuthNotSetup => "Two factor auth not setup", } } } From acf85f4e709d08eb40daa8b6f35eeab76f5f09ad Mon Sep 17 00:00:00 2001 From: Apoorv Dixit Date: Thu, 23 May 2024 16:13:03 +0530 Subject: [PATCH 10/11] fix: change is correct password params to accpet reference --- crates/router/src/core/errors/user.rs | 2 +- crates/router/src/core/user.rs | 16 +++++++--------- crates/router/src/types/domain/user.rs | 4 ++-- crates/router/src/utils/user/password.rs | 22 +++++++++++----------- 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/crates/router/src/core/errors/user.rs b/crates/router/src/core/errors/user.rs index d42bf1659b8f..ccd0d32b2c7a 100644 --- a/crates/router/src/core/errors/user.rs +++ b/crates/router/src/core/errors/user.rs @@ -192,7 +192,7 @@ impl common_utils::errors::ErrorSwitch { - AER::BadRequest(ApiError::new(sub_code, 39, self.get_error_message(), None)) + AER::BadRequest(ApiError::new(sub_code, 40, self.get_error_message(), None)) } } } diff --git a/crates/router/src/core/user.rs b/crates/router/src/core/user.rs index 8852840d47a8..eb2701bcb3ed 100644 --- a/crates/router/src/core/user.rs +++ b/crates/router/src/core/user.rs @@ -178,7 +178,7 @@ pub async fn signin( })? .into(); - user_from_db.compare_password(request.password)?; + user_from_db.compare_password(&request.password)?; let signin_strategy = if let Some(preferred_merchant_id) = user_from_db.get_preferred_merchant_id() { @@ -216,7 +216,7 @@ pub async fn signin_token_only_flow( .to_not_found_response(UserErrors::InvalidCredentials)? .into(); - user_from_db.compare_password(request.password)?; + user_from_db.compare_password(&request.password)?; let next_flow = domain::NextFlow::from_origin(domain::Origin::SignIn, user_from_db.clone(), &state).await?; @@ -340,7 +340,7 @@ pub async fn change_password( .change_context(UserErrors::InternalServerError)? .into(); - user.compare_password(request.old_password.to_owned()) + user.compare_password(&request.old_password) .change_context(UserErrors::InvalidOldPassword)?; if request.old_password == request.new_password { @@ -438,7 +438,7 @@ pub async fn rotate_password( let password = domain::UserPassword::new(request.password.to_owned())?; let hash_password = utils::user::password::generate_password_hash(password.get_secret())?; - if user.compare_password(request.password).is_ok() { + if user.compare_password(&request.password).is_ok() { return Err(UserErrors::ChangePasswordError.into()); } @@ -1788,15 +1788,13 @@ pub async fn verify_recovery_code( .ok_or(UserErrors::InternalServerError)?; let matching_index = utils::user::password::get_index_for_correct_recovery_code( - req.recovery_code, - recovery_codes.clone(), + &req.recovery_code, + &recovery_codes, )? .ok_or(UserErrors::InvalidRecoveryCode)?; + insert_recovery_code_in_redis(&state, user_from_db.get_user_id()).await?; let _ = recovery_codes.remove(matching_index); - let _ = insert_recovery_code_in_redis(&state, user_from_db.get_user_id()) - .await - .map_err(|e| logger::error!(?e)); state .store diff --git a/crates/router/src/types/domain/user.rs b/crates/router/src/types/domain/user.rs index 0704e51e7813..91323b0c16d3 100644 --- a/crates/router/src/types/domain/user.rs +++ b/crates/router/src/types/domain/user.rs @@ -774,8 +774,8 @@ impl UserFromStorage { self.0.user_id.as_str() } - pub fn compare_password(&self, candidate: Secret) -> UserResult<()> { - match password::is_correct_password(candidate, self.0.password.clone()) { + pub fn compare_password(&self, candidate: &Secret) -> UserResult<()> { + match password::is_correct_password(candidate, &self.0.password) { Ok(true) => Ok(()), Ok(false) => Err(UserErrors::InvalidCredentials.into()), Err(e) => Err(e), diff --git a/crates/router/src/utils/user/password.rs b/crates/router/src/utils/user/password.rs index 33de4c66963c..e01181acb9d0 100644 --- a/crates/router/src/utils/user/password.rs +++ b/crates/router/src/utils/user/password.rs @@ -7,7 +7,7 @@ use argon2::{ }; use common_utils::errors::CustomResult; use error_stack::ResultExt; -use masking::{ExposeInterface, Secret}; +use masking::{ExposeInterface, PeekInterface, Secret}; use rand::{seq::SliceRandom, Rng}; use crate::core::errors::UserErrors; @@ -25,13 +25,13 @@ pub fn generate_password_hash( } pub fn is_correct_password( - candidate: Secret, - password: Secret, + candidate: &Secret, + password: &Secret, ) -> CustomResult { - let password = password.expose(); + let password = password.peek(); let parsed_hash = - PasswordHash::new(&password).change_context(UserErrors::InternalServerError)?; - let result = Argon2::default().verify_password(candidate.expose().as_bytes(), &parsed_hash); + PasswordHash::new(password).change_context(UserErrors::InternalServerError)?; + let result = Argon2::default().verify_password(candidate.peek().as_bytes(), &parsed_hash); match result { Ok(_) => Ok(true), Err(argon2Err::Password) => Ok(false), @@ -41,16 +41,16 @@ pub fn is_correct_password( } pub fn get_index_for_correct_recovery_code( - candidate: Secret, - recovery_codes: Vec>, + candidate: &Secret, + recovery_codes: &[Secret], ) -> CustomResult, UserErrors> { - for (index, recovery_code) in recovery_codes.into_iter().enumerate() { - let is_match = is_correct_password(candidate.clone(), recovery_code)?; + for (index, recovery_code) in recovery_codes.iter().enumerate() { + let is_match = is_correct_password(candidate, recovery_code)?; if is_match { return Ok(Some(index)); } } - return Ok(None); + Ok(None) } pub fn get_temp_password() -> Secret { From c12decc4554acb0f6c072f90cbd86a1a951417cc Mon Sep 17 00:00:00 2001 From: Apoorv Dixit Date: Thu, 23 May 2024 18:00:56 +0530 Subject: [PATCH 11/11] fix: resolve conflicts changes --- crates/router/src/core/user.rs | 2 +- crates/router/src/routes/lock_utils.rs | 1 - crates/router/src/utils/user/two_factor_auth.rs | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/router/src/core/user.rs b/crates/router/src/core/user.rs index dee2d6cbaf5e..4fd9fa865cd4 100644 --- a/crates/router/src/core/user.rs +++ b/crates/router/src/core/user.rs @@ -1792,7 +1792,7 @@ pub async fn verify_recovery_code( )? .ok_or(UserErrors::InvalidRecoveryCode)?; - insert_recovery_code_in_redis(&state, user_from_db.get_user_id()).await?; + tfa_utils::insert_recovery_code_in_redis(&state, user_from_db.get_user_id()).await?; let _ = recovery_codes.remove(matching_index); state diff --git a/crates/router/src/routes/lock_utils.rs b/crates/router/src/routes/lock_utils.rs index 801fd52608aa..75821bbd2ba2 100644 --- a/crates/router/src/routes/lock_utils.rs +++ b/crates/router/src/routes/lock_utils.rs @@ -218,7 +218,6 @@ impl From for ApiIdentifier { | Flow::RecoveryCodeVerify | Flow::RecoveryCodesGenerate | Flow::TerminateTwoFactorAuth => Self::User, - Flow::ListRoles | Flow::GetRole | Flow::GetRoleFromToken diff --git a/crates/router/src/utils/user/two_factor_auth.rs b/crates/router/src/utils/user/two_factor_auth.rs index 711154c1cb64..e9e3f66005cf 100644 --- a/crates/router/src/utils/user/two_factor_auth.rs +++ b/crates/router/src/utils/user/two_factor_auth.rs @@ -45,7 +45,7 @@ pub async fn check_totp_in_redis(state: &AppState, user_id: &str) -> UserResult< pub async fn check_recovery_code_in_redis(state: &AppState, user_id: &str) -> UserResult { let redis_conn = get_redis_connection(state)?; - let key = format!("{}{}", consts::user::REDIS_RECOVERY_CODES_PREFIX, user_id); + let key = format!("{}{}", consts::user::REDIS_RECOVERY_CODE_PREFIX, user_id); redis_conn .exists::<()>(&key) .await