Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(users): add support to verify 2FA using recovery code #4737

Merged
merged 12 commits into from
May 23, 2024
3 changes: 2 additions & 1 deletion config/config.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
ThisIsMani marked this conversation as resolved.
Show resolved Hide resolved
two_factor_auth_expiry_in_secs = 300 # Number of seconds after which 2FA should be done again if doing update/change from inside
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo, if we've two different keys in redis for totp and recovery codes then the expiry for them should be different.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can consider this when will be adding other 2FA methods, for now anyways value for all of them will be same


#tokenization configuration which describe token lifetime and payment method for specific connector
[tokenization]
Expand Down
1 change: 1 addition & 0 deletions config/deployments/integration_test.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = 300

[frm]
enabled = true
Expand Down
1 change: 1 addition & 0 deletions config/deployments/production.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = 300

[frm]
enabled = false
Expand Down
1 change: 1 addition & 0 deletions config/deployments/sandbox.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = 300

[frm]
enabled = true
Expand Down
1 change: 1 addition & 0 deletions config/development.toml
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ sts_role_session_name = ""

[user]
password_validity_in_days = 90
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" }
Expand Down
1 change: 1 addition & 0 deletions config/docker_compose.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ recon_admin_api_key = "recon_test_admin"

[user]
password_validity_in_days = 90
two_factor_auth_expiry_in_secs = 300

[locker]
host = ""
Expand Down
3 changes: 2 additions & 1 deletion crates/api_models/src/events/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::user::{
RecoveryCodes, ResetPasswordRequest, RotatePasswordRequest, SendVerifyEmailRequest,
SignInResponse, SignUpRequest, SignUpWithMerchantIdRequest, SwitchMerchantIdRequest,
TokenOrPayloadResponse, TokenResponse, UpdateUserAccountDetailsRequest, UserFromEmailRequest,
UserMerchantCreate, VerifyEmailRequest, VerifyTotpRequest,
UserMerchantCreate, VerifyEmailRequest, VerifyRecoveryCodeRequest, VerifyTotpRequest,
};

impl ApiEventMetric for DashboardEntryResponse {
Expand Down Expand Up @@ -75,6 +75,7 @@ common_utils::impl_misc_api_event_type!(
TokenResponse,
UserFromEmailRequest,
BeginTotpResponse,
VerifyRecoveryCodeRequest,
VerifyTotpRequest,
RecoveryCodes
);
Expand Down
5 changes: 5 additions & 0 deletions crates/api_models/src/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,11 @@ pub struct VerifyTotpRequest {
pub totp: Option<Secret<String>>,
}

#[derive(Debug, serde::Deserialize, serde::Serialize)]
pub struct VerifyRecoveryCodeRequest {
pub recovery_code: Secret<String>,
}

#[derive(Debug, serde::Deserialize, serde::Serialize)]
pub struct RecoveryCodes {
pub recovery_codes: Vec<Secret<String>>,
Expand Down
1 change: 1 addition & 0 deletions crates/router/src/configs/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: i64,
}

#[derive(Debug, Deserialize, Clone)]
Expand Down
2 changes: 1 addition & 1 deletion crates/router/src/consts/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +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_CODES_PREFIX: &str = "RC_";
pub const REDIS_RECOVERY_CODE_PREFIX: &str = "RC_";
10 changes: 8 additions & 2 deletions crates/router/src/core/errors/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ pub enum UserErrors {
InvalidTotp,
#[error("TotpRequired")]
TotpRequired,
#[error("InvalidRecoveryCode")]
InvalidRecoveryCode,
#[error("TwoFactorAuthRequired")]
TwoFactorAuthRequired,
#[error("TwoFactorAuthNotSetup")]
Expand Down Expand Up @@ -188,12 +190,15 @@ impl common_utils::errors::ErrorSwitch<api_models::errors::types::ApiErrorRespon
Self::TotpRequired => {
AER::BadRequest(ApiError::new(sub_code, 38, self.get_error_message(), None))
}
Self::TwoFactorAuthRequired => {
Self::InvalidRecoveryCode => {
AER::BadRequest(ApiError::new(sub_code, 39, self.get_error_message(), None))
}
Self::TwoFactorAuthNotSetup => {
Self::TwoFactorAuthRequired => {
AER::BadRequest(ApiError::new(sub_code, 40, self.get_error_message(), None))
}
Self::TwoFactorAuthNotSetup => {
AER::BadRequest(ApiError::new(sub_code, 41, self.get_error_message(), None))
}
}
}
}
Expand Down Expand Up @@ -233,6 +238,7 @@ impl UserErrors {
Self::TotpNotSetup => "TOTP not setup",
Self::InvalidTotp => "Invalid TOTP",
Self::TotpRequired => "TOTP required",
Self::InvalidRecoveryCode => "Invalid Recovery Code",
Self::TwoFactorAuthRequired => "Two factor auth required",
Self::TwoFactorAuthNotSetup => "Two factor auth not setup",
}
Expand Down
53 changes: 49 additions & 4 deletions crates/router/src/core/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,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() {
Expand Down Expand Up @@ -217,7 +217,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?;
Expand Down Expand Up @@ -341,7 +341,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 {
Expand Down Expand Up @@ -439,7 +439,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());
}

Expand Down Expand Up @@ -1766,6 +1766,51 @@ pub async fn generate_recovery_codes(
}))
}

pub async fn verify_recovery_code(
state: AppState,
user_token: auth::UserFromSinglePurposeToken,
req: user_api::VerifyRecoveryCodeRequest,
) -> UserResponse<user_api::TokenResponse> {
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::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,
)?
.ok_or(UserErrors::InvalidRecoveryCode)?;

tfa_utils::insert_recovery_code_in_redis(&state, user_from_db.get_user_id()).await?;
let _ = recovery_codes.remove(matching_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(recovery_codes),
},
)
.await
.change_context(UserErrors::InternalServerError)?;

Ok(ApplicationResponse::StatusOk)
}

pub async fn terminate_two_factor_auth(
state: AppState,
user_token: auth::UserFromSinglePurposeToken,
Expand Down
10 changes: 6 additions & 4 deletions crates/router/src/routes/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1212,14 +1212,16 @@ 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/generate")
.route(web::get().to(generate_recovery_codes)),
)
.service(
web::resource("/2fa/terminate").route(web::get().to(terminate_two_factor_auth)),
);

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")]
{
route = route
Expand Down
6 changes: 3 additions & 3 deletions crates/router/src/routes/lock_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,9 @@ impl From<Flow> for ApiIdentifier {
| Flow::UpdateUserAccountDetails
| Flow::TotpBegin
| Flow::TotpVerify
| Flow::TerminateTwoFactorAuth
| Flow::GenerateRecoveryCodes => Self::User,

| Flow::RecoveryCodeVerify
| Flow::RecoveryCodesGenerate
| Flow::TerminateTwoFactorAuth => Self::User,
Flow::ListRoles
| Flow::GetRole
| Flow::GetRoleFromToken
Expand Down
20 changes: 19 additions & 1 deletion crates/router/src/routes/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -666,8 +666,26 @@ pub async fn totp_verify(
.await
}

pub async fn verify_recovery_code(
state: web::Data<AppState>,
req: HttpRequest,
json_payload: web::Json<user_api::VerifyRecoveryCodeRequest>,
) -> HttpResponse {
let flow = Flow::RecoveryCodeVerify;
Box::pin(api::server_wrap(
flow,
state.clone(),
&req,
json_payload.into_inner(),
|state, user, req_body, _| user_core::verify_recovery_code(state, user, req_body),
&auth::SinglePurposeJWTAuth(TokenPurpose::TOTP),
api_locking::LockAction::NotApplicable,
))
.await
}

pub async fn generate_recovery_codes(state: web::Data<AppState>, req: HttpRequest) -> HttpResponse {
let flow = Flow::GenerateRecoveryCodes;
let flow = Flow::RecoveryCodesGenerate;
Box::pin(api::server_wrap(
flow,
state.clone(),
Expand Down
4 changes: 2 additions & 2 deletions crates/router/src/types/domain/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -774,8 +774,8 @@ impl UserFromStorage {
self.0.user_id.as_str()
}

pub fn compare_password(&self, candidate: Secret<String>) -> UserResult<()> {
match password::is_correct_password(candidate, self.0.password.clone()) {
pub fn compare_password(&self, candidate: &Secret<String>) -> UserResult<()> {
match password::is_correct_password(candidate, &self.0.password) {
Ok(true) => Ok(()),
Ok(false) => Err(UserErrors::InvalidCredentials.into()),
Err(e) => Err(e),
Expand Down
25 changes: 19 additions & 6 deletions crates/router/src/utils/user/password.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -25,13 +25,13 @@ pub fn generate_password_hash(
}

pub fn is_correct_password(
candidate: Secret<String>,
password: Secret<String>,
candidate: &Secret<String>,
password: &Secret<String>,
) -> CustomResult<bool, UserErrors> {
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),
Expand All @@ -40,6 +40,19 @@ pub fn is_correct_password(
.change_context(UserErrors::InternalServerError)
}

pub fn get_index_for_correct_recovery_code(
candidate: &Secret<String>,
recovery_codes: &[Secret<String>],
) -> CustomResult<Option<usize>, UserErrors> {
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));
}
}
Ok(None)
}

pub fn get_temp_password() -> Secret<String> {
let uuid_pass = uuid::Uuid::new_v4().to_string();
let mut rng = rand::thread_rng();
Expand Down
15 changes: 14 additions & 1 deletion crates/router/src/utils/user/two_factor_auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> {
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
Expand All @@ -59,3 +59,16 @@ fn get_redis_connection(state: &AppState) -> UserResult<Arc<RedisConnectionPool>
.change_context(UserErrors::InternalServerError)
.attach_printable("Failed to get redis connection")
}

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);
redis_conn
.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)
}
4 changes: 3 additions & 1 deletion crates/router_env/src/logger/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,10 @@ pub enum Flow {
TotpBegin,
/// Verify TOTP
TotpVerify,
/// Verify Access Code
RecoveryCodeVerify,
/// Generate or Regenerate recovery codes
GenerateRecoveryCodes,
RecoveryCodesGenerate,
// Terminate two factor authentication
TerminateTwoFactorAuth,
/// List initial webhook delivery attempts
Expand Down
1 change: 1 addition & 0 deletions loadtest/config/development.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ jwt_secret = "secret"

[user]
password_validity_in_days = 90
two_factor_auth_expiry_in_secs = 300

[locker]
host = ""
Expand Down
Loading