Skip to content

Commit

Permalink
fix: mfa notification only at first method (#441)
Browse files Browse the repository at this point in the history
  • Loading branch information
@filipslezaklab
filipslezaklab authored Nov 29, 2023
1 parent 35360b6 commit 5d7617e
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 58 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ ladle-build
.vscode/
.envrc
.direnv/
*.pem
39 changes: 18 additions & 21 deletions src/handlers/auth.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,19 +313,18 @@ pub async fn webauthn_finish(
let mut webauthn = WebAuthn::new(session.session.user_id, webauth_reg.name, &passkey)?;
webauthn.save(&appstate.pool).await?;
if user.mfa_method == MFAMethod::None {
send_mfa_configured_email(
Some(&session.session),
&user,
&MFAMethod::Webauthn,
&appstate.mail_tx,
)?;
user.set_mfa_method(&appstate.pool, MFAMethod::Webauthn)
.await?;
}

info!("Finished Webauthn registration for user {}", user.username);

send_mfa_configured_email(
Some(&session.session),
&user,
&MFAMethod::Webauthn,
&appstate.mail_tx,
)?;

Ok(ApiResponse {
json: json!(recovery_codes),
status: StatusCode::OK,
Expand Down Expand Up @@ -430,17 +429,16 @@ pub async fn totp_enable(
let recovery_codes = RecoveryCodes::new(user.get_recovery_codes(&appstate.pool).await?);
user.enable_totp(&appstate.pool).await?;
if user.mfa_method == MFAMethod::None {
send_mfa_configured_email(
Some(&session.session),
&user,
&MFAMethod::OneTimePassword,
&appstate.mail_tx,
)?;
user.set_mfa_method(&appstate.pool, MFAMethod::OneTimePassword)
.await?;
}

send_mfa_configured_email(
Some(&session.session),
&user,
&MFAMethod::OneTimePassword,
&appstate.mail_tx,
)?;

info!("Enabled TOTP for user {}", user.username);
Ok(ApiResponse {
json: json!(recovery_codes),
Expand Down Expand Up @@ -540,17 +538,16 @@ pub async fn email_mfa_enable(
let recovery_codes = RecoveryCodes::new(user.get_recovery_codes(&appstate.pool).await?);
user.enable_email_mfa(&appstate.pool).await?;
if user.mfa_method == MFAMethod::None {
send_mfa_configured_email(
Some(&session.session),
&user,
&MFAMethod::Email,
&appstate.mail_tx,
)?;
user.set_mfa_method(&appstate.pool, MFAMethod::Email)
.await?;
}

send_mfa_configured_email(
Some(&session.session),
&user,
&MFAMethod::Email,
&appstate.mail_tx,
)?;

info!("Enabled email MFA for user {}", user.username);
Ok(ApiResponse {
json: json!(recovery_codes),
Expand Down
14 changes: 7 additions & 7 deletions src/handlers/user.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -565,14 +565,14 @@ pub async fn update_wallet(
if mfa_change {
if data.use_for_mfa {
debug!("Wallet {} MFA flag enabled", wallet.address);
// send notification email about enabled MFA
send_mfa_configured_email(
Some(&session.session),
&user,
&MFAMethod::Web3,
&appstate.mail_tx,
)?;
if !user.mfa_enabled {
// send notification email about enabled MFA
send_mfa_configured_email(
Some(&session.session),
&user,
&MFAMethod::Web3,
&appstate.mail_tx,
)?;
user.set_mfa_method(&appstate.pool, MFAMethod::Web3).await?;
let recovery_codes = user.get_recovery_codes(&appstate.pool).await?;
info!("User {} MFA enabled", username);
Expand Down
30 changes: 12 additions & 18 deletions tests/auth.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -980,12 +980,10 @@ async fn test_mfa_method_totp_enabled_mail() {
mail.subject,
"MFA method TOTP was activated on your account"
);
assert_eq!(mail.content.contains("IP Address:</span> 127.0.0.1"), true);
assert_eq!(
mail.content
.contains("Device type:</span> iPhone, OS: iOS 17.1, Mobile Safari"),
true
);
assert!(mail.content.contains("IP Address:</span> 127.0.0.1"));
assert!(mail
.content
.contains("Device type:</span> iPhone, OS: iOS 17.1, Mobile Safari"));
}

#[tokio::test]
Expand All @@ -1011,12 +1009,10 @@ async fn test_new_device_login() {
mail.subject,
"Defguard: new device logged in to your account"
);
assert_eq!(mail.content.contains("IP Address:</span> 127.0.0.1"), true);
assert_eq!(
mail.content
.contains("Device type:</span> iPhone, OS: iOS 17.1, Mobile Safari"),
true
);
assert!(mail.content.contains("IP Address:</span> 127.0.0.1"));
assert!(mail
.content
.contains("Device type:</span> iPhone, OS: iOS 17.1, Mobile Safari"));

let response = client.post("/api/v1/auth/logout").send().await;
assert_eq!(response.status(), StatusCode::OK);
Expand Down Expand Up @@ -1048,10 +1044,8 @@ async fn test_new_device_login() {
mail.subject,
"Defguard: new device logged in to your account"
);
assert_eq!(mail.content.contains("IP Address:</span> 127.0.0.1"), true);
assert_eq!(
mail.content
.contains("Device type:</span> SM-G930VC, OS: Android 7.0, Chrome Mobile WebView"),
true
);
assert!(mail.content.contains("IP Address:</span> 127.0.0.1"));
assert!(mail
.content
.contains("Device type:</span> SM-G930VC, OS: Android 7.0, Chrome Mobile WebView"));
}
10 changes: 4 additions & 6 deletions tests/openid.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -678,12 +678,10 @@ async fn test_openid_flow_new_login_mail() {
let mail = mail_rx.try_recv().unwrap();
assert_eq!(mail.to, "admin@defguard");
assert_eq!(mail.subject, "New login to Test application with defguard");
assert_eq!(mail.content.contains("IP Address:</span> 127.0.0.1"), true);
assert_eq!(
mail.content
.contains("Device type:</span> iPhone, OS: iOS 17.1, Mobile Safari"),
true
);
assert!(mail.content.contains("IP Address:</span> 127.0.0.1"));
assert!(mail
.content
.contains("Device type:</span> iPhone, OS: iOS 17.1, Mobile Safari"));

let response = client
.post(format!(
Expand Down
10 changes: 4 additions & 6 deletions tests/user.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -617,10 +617,8 @@ async fn test_user_add_device() {
let mail = mail_rx.try_recv().unwrap();
assert_eq!(mail.to, "admin@defguard");
assert_eq!(mail.subject, "Defguard: new device added to your account");
assert_eq!(mail.content.contains("IP Address:</span> 127.0.0.1"), true);
assert_eq!(
mail.content
.contains("Device type:</span> iPhone, OS: iOS 17.1, Mobile Safari"),
true
);
assert!(mail.content.contains("IP Address:</span> 127.0.0.1"));
assert!(mail
.content
.contains("Device type:</span> iPhone, OS: iOS 17.1, Mobile Safari"));
}

0 comments on commit 5d7617e

Please sign in to comment.