Browse Source

Fix 2FA Remember to actually be 30 days

Currently we always regenerate the 2FA Remember token, and always send that back to the client.
This is not the correct way, and in turn causes the remember token to never expire.

While this might be convenient, it is not really safe.
This commit changes the 2FA Remember Tokens from random string to a JWT.
This JWT has a lifetime of 30 days and is validated per device & user combination.

This does mean that once this commit is merged, and users are using this version, all their remember tokens will be invalidated.
From my point of view this isn't a bad thing, since those tokens should have expired already.

Only users who recently checked the remember checkbox within 30 days have to login again, but that is a minor inconvenience I think.

Signed-off-by: BlackDex <black.dex@gmail.com>
pull/6929/head
BlackDex 17 hours ago
parent
commit
d785a25941
No known key found for this signature in database GPG Key ID: 58C80A2AA6C765E1
  1. 21
      src/api/identity.rs
  2. 30
      src/auth.rs
  3. 11
      src/db/models/device.rs

21
src/api/identity.rs

@ -757,7 +757,6 @@ async fn twofactor_auth(
use crate::crypto::ct_eq;
let selected_data = _selected_data(selected_twofactor);
let mut remember = data.two_factor_remember.unwrap_or(0);
match TwoFactorType::from_i32(selected_id) {
Some(TwoFactorType::Authenticator) => {
@ -789,13 +788,23 @@ async fn twofactor_auth(
}
Some(TwoFactorType::Remember) => {
match device.twofactor_remember {
Some(ref code) if !CONFIG.disable_2fa_remember() && ct_eq(code, twofactor_code) => {
remember = 1; // Make sure we also return the token here, otherwise it will only remember the first time
}
// When a 2FA Remember token is used, check and validate this JWT token, if it is valid, just continue
// If it is invalid we need to trigger the 2FA Login prompt
Some(ref token)
if !CONFIG.disable_2fa_remember()
&& (ct_eq(token, twofactor_code)
&& auth::decode_2fa_remember(twofactor_code)
.is_ok_and(|t| t.sub == device.uuid && t.user_uuid == user.uuid)) => {}
_ => {
// Always delete the current twofactor remember token here if it exists
if device.twofactor_remember.is_some() {
device.delete_twofactor_remember();
// We need to save here, since we send a err_json!() which prevents saving `device` at a later stage
device.save(true, conn).await?;
}
err_json!(
_json_err_twofactor(&twofactor_ids, &user.uuid, data, client_version, conn).await?,
"2FA Remember token not provided"
"2FA Remember token not provided or expired"
)
}
}
@ -826,10 +835,10 @@ async fn twofactor_auth(
TwoFactorIncomplete::mark_complete(&user.uuid, &device.uuid, conn).await?;
let remember = data.two_factor_remember.unwrap_or(0);
let two_factor = if !CONFIG.disable_2fa_remember() && remember == 1 {
Some(device.refresh_twofactor_remember())
} else {
device.delete_twofactor_remember();
None
};
Ok(two_factor)

30
src/auth.rs

@ -46,6 +46,7 @@ static JWT_FILE_DOWNLOAD_ISSUER: LazyLock<String> =
LazyLock::new(|| format!("{}|file_download", CONFIG.domain_origin()));
static JWT_REGISTER_VERIFY_ISSUER: LazyLock<String> =
LazyLock::new(|| format!("{}|register_verify", CONFIG.domain_origin()));
static JWT_2FA_REMEMBER_ISSUER: LazyLock<String> = LazyLock::new(|| format!("{}|2faremember", CONFIG.domain_origin()));
static PRIVATE_RSA_KEY: OnceLock<EncodingKey> = OnceLock::new();
static PUBLIC_RSA_KEY: OnceLock<DecodingKey> = OnceLock::new();
@ -160,6 +161,10 @@ pub fn decode_register_verify(token: &str) -> Result<RegisterVerifyClaims, Error
decode_jwt(token, JWT_REGISTER_VERIFY_ISSUER.to_string())
}
pub fn decode_2fa_remember(token: &str) -> Result<TwoFactorRememberClaims, Error> {
decode_jwt(token, JWT_2FA_REMEMBER_ISSUER.to_string())
}
#[derive(Debug, Serialize, Deserialize)]
pub struct LoginJwtClaims {
// Not before
@ -440,6 +445,31 @@ pub fn generate_register_verify_claims(email: String, name: Option<String>, veri
}
}
#[derive(Serialize, Deserialize)]
pub struct TwoFactorRememberClaims {
// Not before
pub nbf: i64,
// Expiration time
pub exp: i64,
// Issuer
pub iss: String,
// Subject
pub sub: DeviceId,
// UserId
pub user_uuid: UserId,
}
pub fn generate_2fa_remember_claims(device_uuid: DeviceId, user_uuid: UserId) -> TwoFactorRememberClaims {
let time_now = Utc::now();
TwoFactorRememberClaims {
nbf: time_now.timestamp(),
exp: (time_now + TimeDelta::try_days(30).unwrap()).timestamp(),
iss: JWT_2FA_REMEMBER_ISSUER.to_string(),
sub: device_uuid,
user_uuid,
}
}
#[derive(Debug, Serialize, Deserialize)]
pub struct BasicJwtClaims {
// Not before

11
src/db/models/device.rs

@ -1,6 +1,6 @@
use chrono::{NaiveDateTime, Utc};
use data_encoding::{BASE64, BASE64URL};
use data_encoding::BASE64URL;
use derive_more::{Display, From};
use serde_json::Value;
@ -67,10 +67,13 @@ impl Device {
}
pub fn refresh_twofactor_remember(&mut self) -> String {
let twofactor_remember = crypto::encode_random_bytes::<180>(&BASE64);
self.twofactor_remember = Some(twofactor_remember.clone());
use crate::auth::{encode_jwt, generate_2fa_remember_claims};
twofactor_remember
let two_factor_remember_claim = generate_2fa_remember_claims(self.uuid.clone(), self.user_uuid.clone());
let two_factor_remember_string = encode_jwt(&two_factor_remember_claim);
self.twofactor_remember = Some(two_factor_remember_string.clone());
two_factor_remember_string
}
pub fn delete_twofactor_remember(&mut self) {

Loading…
Cancel
Save