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

Rework the way 2FA is enabled/disabled (fixes #3309) #3959

Merged
merged 8 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ chrono = { workspace = true }
url = { workspace = true }
wav = "1.0.0"
sitemap-rs = "0.2.0"
totp-rs = { version = "5.0.2", features = ["gen_secret", "otpauth"] }

[dev-dependencies]
serial_test = { workspace = true }
Expand Down
70 changes: 68 additions & 2 deletions crates/api/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use base64::{engine::general_purpose::STANDARD_NO_PAD as base64, Engine};
use captcha::Captcha;
use lemmy_api_common::utils::local_site_to_slur_regex;
use lemmy_db_schema::source::local_site::LocalSite;
use lemmy_db_schema::source::{local_site::LocalSite, local_user::LocalUser};
use lemmy_utils::{
error::{LemmyError, LemmyErrorExt, LemmyErrorType},
error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult},
utils::slurs::check_slurs,
};
use std::io::Cursor;
use totp_rs::{Secret, TOTP};

pub mod comment;
pub mod comment_report;
Expand Down Expand Up @@ -135,3 +136,68 @@ mod tests {
assert_eq!(1, num_deleted);
}
}

pub fn check_totp_2fa_valid(
local_user: &LocalUser,
totp_token: &Option<String>,
site_name: &str,
username: &str,
) -> LemmyResult<()> {
// Check only if they have a totp_secret in the DB
if local_user.totp_2fa_enabled {
// Throw an error if their token is missing
let token = totp_token
.as_deref()
.ok_or(LemmyErrorType::MissingTotpToken)?;
let secret = local_user
.totp_2fa_secret
.as_deref()
.ok_or(LemmyErrorType::MissingTotpSecret)?;

let totp = build_totp_2fa(site_name, username, secret)?;

let check_passed = totp.check_current(token)?;
if !check_passed {
return Err(LemmyErrorType::IncorrectTotpToken.into());
}
}

Ok(())
}

pub fn generate_totp_2fa_secret() -> String {
Secret::generate_secret().to_string()
}

pub fn build_totp_2fa(site_name: &str, username: &str, secret: &str) -> Result<TOTP, LemmyError> {
let sec = Secret::Raw(secret.as_bytes().to_vec());
let sec_bytes = sec
.to_bytes()
.map_err(|_| LemmyErrorType::CouldntParseTotpSecret)?;

TOTP::new(
totp_rs::Algorithm::SHA1,
Copy link
Member

Choose a reason for hiding this comment

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

I see you got this one, good.

6,
1,
30,
sec_bytes,
Some(site_name.to_string()),
username.to_string(),
)
.with_lemmy_type(LemmyErrorType::CouldntGenerateTotp)
}

#[cfg(test)]
mod test {
#![allow(clippy::unwrap_used)]
#![allow(clippy::indexing_slicing)]

use super::*;

#[test]
fn test_build_totp() {
let generated_secret = generate_totp_2fa_secret();
let totp = build_totp_2fa("lemmy", "my_name", &generated_secret);
assert!(totp.is_ok());
}
}
47 changes: 47 additions & 0 deletions crates/api/src/local_user/generate_totp_secret.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
use crate::{build_totp_2fa, generate_totp_2fa_secret};
use activitypub_federation::config::Data;
use actix_web::web::Json;
use lemmy_api_common::{
context::LemmyContext,
person::GenerateTotpSecretResponse,
sensitive::Sensitive,
};
use lemmy_db_schema::{
source::local_user::{LocalUser, LocalUserUpdateForm},
traits::Crud,
};
use lemmy_db_views::structs::{LocalUserView, SiteView};
use lemmy_utils::error::{LemmyError, LemmyErrorType};

/// Generate a new secret for two-factor-authentication. Afterwards you need to call [toggle_totp]
/// to enable it. This can only be called if 2FA is currently disabled.
#[tracing::instrument(skip(context))]
pub async fn generate_totp_secret(
local_user_view: LocalUserView,
context: Data<LemmyContext>,
) -> Result<Json<GenerateTotpSecretResponse>, LemmyError> {
let site_view = SiteView::read_local(&mut context.pool()).await?;

if local_user_view.local_user.totp_2fa_enabled {
return Err(LemmyErrorType::TotpAlreadyEnabled)?;
}

let secret = generate_totp_2fa_secret();
let secret_url =
build_totp_2fa(&site_view.site.name, &local_user_view.person.name, &secret)?.get_url();

let local_user_form = LocalUserUpdateForm {
totp_2fa_secret: Some(Some(secret)),
..Default::default()
};
LocalUser::update(
&mut context.pool(),
local_user_view.local_user.id,
&local_user_form,
)
.await?;

Ok(Json(GenerateTotpSecretResponse {
totp_secret_url: Sensitive::new(secret_url),
}))
}
4 changes: 2 additions & 2 deletions crates/api/src/local_user/login.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::check_totp_2fa_valid;
use actix_web::web::{Data, Json};
use bcrypt::verify;
use lemmy_api_common::{
Expand All @@ -9,7 +10,6 @@ use lemmy_db_views::structs::{LocalUserView, SiteView};
use lemmy_utils::{
claims::Claims,
error::{LemmyError, LemmyErrorExt, LemmyErrorType},
utils::validation::check_totp_2fa_valid,
};

#[tracing::instrument(skip(context))]
Expand Down Expand Up @@ -55,7 +55,7 @@ pub async fn login(

// Check the totp
check_totp_2fa_valid(
&local_user_view.local_user.totp_2fa_secret,
&local_user_view.local_user,
&data.totp_2fa_token,
&site_view.site.name,
&local_user_view.person.name,
Expand Down
2 changes: 2 additions & 0 deletions crates/api/src/local_user/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ pub mod ban_person;
pub mod block;
pub mod change_password;
pub mod change_password_after_reset;
pub mod generate_totp_secret;
pub mod get_captcha;
pub mod list_banned;
pub mod login;
pub mod notifications;
pub mod report_count;
pub mod reset_password;
pub mod save_settings;
pub mod toggle_totp;
pub mod verify_email;
24 changes: 1 addition & 23 deletions crates/api/src/local_user/save_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,7 @@ use lemmy_db_views::structs::SiteView;
use lemmy_utils::{
claims::Claims,
error::{LemmyError, LemmyErrorExt, LemmyErrorType},
utils::validation::{
build_totp_2fa,
generate_totp_2fa_secret,
is_valid_bio_field,
is_valid_display_name,
is_valid_matrix_id,
},
utils::validation::{is_valid_bio_field, is_valid_display_name, is_valid_matrix_id},
};

#[tracing::instrument(skip(context))]
Expand Down Expand Up @@ -105,20 +99,6 @@ pub async fn save_user_settings(
LocalUserLanguage::update(&mut context.pool(), discussion_languages, local_user_id).await?;
}

// If generate_totp is Some(false), this will clear it out from the database.
let (totp_2fa_secret, totp_2fa_url) = if let Some(generate) = data.generate_totp_2fa {
if generate {
let secret = generate_totp_2fa_secret();
let url =
build_totp_2fa(&site_view.site.name, &local_user_view.person.name, &secret)?.get_url();
(Some(Some(secret)), Some(Some(url)))
} else {
(Some(None), Some(None))
}
} else {
(None, None)
};

let local_user_form = LocalUserUpdateForm {
email,
show_avatars: data.show_avatars,
Expand All @@ -134,8 +114,6 @@ pub async fn save_user_settings(
default_listing_type,
theme,
interface_language: data.interface_language.clone(),
totp_2fa_secret,
totp_2fa_url,
open_links_in_new_tab: data.open_links_in_new_tab,
infinite_scroll_enabled: data.infinite_scroll_enabled,
..Default::default()
Expand Down
61 changes: 61 additions & 0 deletions crates/api/src/local_user/toggle_totp.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
use crate::check_totp_2fa_valid;
use actix_web::{
web::{Data, Json},
HttpResponse,
};
use lemmy_api_common::{context::LemmyContext, person::ToggleTotp};
use lemmy_db_schema::{
source::local_user::{LocalUser, LocalUserUpdateForm},
traits::Crud,
};
use lemmy_db_views::structs::{LocalUserView, SiteView};
use lemmy_utils::error::{LemmyError, LemmyErrorType};

/// Enable or disable two-factor-authentication. The current setting is determined from
/// [LocalUser.totp_2fa_enabled].
///
/// To enable, you need to first call [generate_totp_secret] and then pass a valid token to this
/// function.
///
/// Disabling is only possible if 2FA was previously enabled. Again it is necessary to pass a valid
/// token.
#[tracing::instrument(skip(context))]
pub async fn toggle_totp(
data: Json<ToggleTotp>,
local_user_view: LocalUserView,
context: Data<LemmyContext>,
) -> Result<HttpResponse, LemmyError> {
let site_view = SiteView::read_local(&mut context.pool()).await?;

// require valid 2fa token to enable or disable 2fa
if local_user_view.local_user.totp_2fa_secret.is_none() {
return Err(LemmyErrorType::MissingTotpToken.into());
}
check_totp_2fa_valid(
dessalines marked this conversation as resolved.
Show resolved Hide resolved
&local_user_view.local_user,
&Some(data.totp_totp_token.clone()),
&site_view.site.name,
&local_user_view.person.name,
)?;

// toggle the 2fa setting
let new_totp_state = !local_user_view.local_user.totp_2fa_enabled;
let mut local_user_form = LocalUserUpdateForm {
totp_2fa_enabled: Some(new_totp_state),
..Default::default()
};

// clear totp secret if 2fa is being disabled
if !new_totp_state {
local_user_form.totp_2fa_secret = None;
}

LocalUser::update(
&mut context.pool(),
local_user_view.local_user.id,
&local_user_form,
)
.await?;

Ok(HttpResponse::Ok().finish())
dessalines marked this conversation as resolved.
Show resolved Hide resolved
}
18 changes: 14 additions & 4 deletions crates/api_common/src/person.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,6 @@ pub struct SaveUserSettings {
pub show_new_post_notifs: Option<bool>,
/// A list of languages you are able to see discussion in.
pub discussion_languages: Option<Vec<LanguageId>>,
/// Generates a TOTP / 2-factor authentication token.
///
/// None leaves it as is, true will generate or regenerate it, false clears it out.
pub generate_totp_2fa: Option<bool>,
pub auth: Sensitive<String>,
/// Open links in a new tab
pub open_links_in_new_tab: Option<bool>,
Expand Down Expand Up @@ -443,3 +439,17 @@ pub struct VerifyEmail {
#[cfg_attr(feature = "full", ts(export))]
/// A response to verifying your email.
pub struct VerifyEmailResponse {}

#[derive(Debug, Serialize, Deserialize, Clone)]
#[cfg_attr(feature = "full", derive(TS))]
#[cfg_attr(feature = "full", ts(export))]
pub struct GenerateTotpSecretResponse {
pub totp_secret_url: Sensitive<String>,
}

#[derive(Debug, Serialize, Deserialize, Clone)]
#[cfg_attr(feature = "full", derive(TS))]
#[cfg_attr(feature = "full", ts(export))]
pub struct ToggleTotp {
pub totp_totp_token: String,
Copy link
Member

Choose a reason for hiding this comment

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

This needs a comment about how to disable totp also (maybe sending an empty or invalid string)

Copy link
Member Author

Choose a reason for hiding this comment

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

Added enabled param

Copy link
Member

@dessalines dessalines Sep 12, 2023

Choose a reason for hiding this comment

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

This should probably also have a response, probably a simple success: boolean or enabled: boolean

Because right now the responses for enabling / disabling TOTP are the same.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added

}
2 changes: 1 addition & 1 deletion crates/db_schema/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,13 +427,13 @@ diesel::table! {
email_verified -> Bool,
accepted_application -> Bool,
totp_2fa_secret -> Nullable<Text>,
totp_2fa_url -> Nullable<Text>,
open_links_in_new_tab -> Bool,
blur_nsfw -> Bool,
auto_expand -> Bool,
infinite_scroll_enabled -> Bool,
admin -> Bool,
post_listing_mode -> PostListingModeEnum,
totp_2fa_enabled -> Bool,
}
}

Expand Down
7 changes: 3 additions & 4 deletions crates/db_schema/src/source/local_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ pub struct LocalUser {
pub accepted_application: bool,
#[serde(skip)]
pub totp_2fa_secret: Option<String>,
/// A URL to add their 2-factor auth.
pub totp_2fa_url: Option<String>,
/// Open links in a new tab.
pub open_links_in_new_tab: bool,
pub blur_nsfw: bool,
Expand All @@ -62,6 +60,7 @@ pub struct LocalUser {
/// Whether the person is an admin.
pub admin: bool,
pub post_listing_mode: PostListingMode,
pub totp_2fa_enabled: bool,
}

#[derive(Clone, TypedBuilder)]
Expand All @@ -88,13 +87,13 @@ pub struct LocalUserInsertForm {
pub email_verified: Option<bool>,
pub accepted_application: Option<bool>,
pub totp_2fa_secret: Option<Option<String>>,
pub totp_2fa_url: Option<Option<String>>,
pub open_links_in_new_tab: Option<bool>,
pub blur_nsfw: Option<bool>,
pub auto_expand: Option<bool>,
pub infinite_scroll_enabled: Option<bool>,
pub admin: Option<bool>,
pub post_listing_mode: Option<PostListingMode>,
pub totp_2fa_enabled: Option<bool>,
}

#[derive(Clone, Default)]
Expand All @@ -117,11 +116,11 @@ pub struct LocalUserUpdateForm {
pub email_verified: Option<bool>,
pub accepted_application: Option<bool>,
pub totp_2fa_secret: Option<Option<String>>,
pub totp_2fa_url: Option<Option<String>>,
pub open_links_in_new_tab: Option<bool>,
pub blur_nsfw: Option<bool>,
pub auto_expand: Option<bool>,
pub infinite_scroll_enabled: Option<bool>,
pub admin: Option<bool>,
pub post_listing_mode: Option<PostListingMode>,
pub totp_2fa_enabled: Option<bool>,
}
Loading