Skip to content

Commit 4ad8baf

Browse files
authored
fix email as 2fa for sso (#6495)
* fix email as 2fa for sso * allow saving device without updating `updated_at` * check if email is some * allow device to be saved in postgresql * use twofactor_incomplete table * no need to update device.updated_at
1 parent 8f689d8 commit 4ad8baf

File tree

7 files changed

+93
-60
lines changed

7 files changed

+93
-60
lines changed

src/api/core/accounts.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1409,7 +1409,7 @@ async fn put_device_token(device_id: DeviceId, data: Json<PushToken>, headers: H
14091409
}
14101410

14111411
device.push_token = Some(token);
1412-
if let Err(e) = device.save(&conn).await {
1412+
if let Err(e) = device.save(true, &conn).await {
14131413
err!(format!("An error occurred while trying to save the device push token: {e}"));
14141414
}
14151415

src/api/core/two_factor/email.rs

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::{
1010
auth::Headers,
1111
crypto,
1212
db::{
13-
models::{EventType, TwoFactor, TwoFactorType, User, UserId},
13+
models::{DeviceId, EventType, TwoFactor, TwoFactorType, User, UserId},
1414
DbConn,
1515
},
1616
error::{Error, MapResult},
@@ -24,10 +24,12 @@ pub fn routes() -> Vec<Route> {
2424
#[derive(Deserialize)]
2525
#[serde(rename_all = "camelCase")]
2626
struct SendEmailLoginData {
27+
#[serde(alias = "DeviceIdentifier")]
28+
device_identifier: DeviceId,
2729
#[serde(alias = "Email")]
28-
email: String,
30+
email: Option<String>,
2931
#[serde(alias = "MasterPasswordHash")]
30-
master_password_hash: String,
32+
master_password_hash: Option<String>,
3133
}
3234

3335
/// User is trying to login and wants to use email 2FA.
@@ -36,25 +38,40 @@ struct SendEmailLoginData {
3638
async fn send_email_login(data: Json<SendEmailLoginData>, conn: DbConn) -> EmptyResult {
3739
let data: SendEmailLoginData = data.into_inner();
3840

39-
use crate::db::models::User;
41+
if !CONFIG._enable_email_2fa() {
42+
err!("Email 2FA is disabled")
43+
}
4044

4145
// Get the user
42-
let Some(user) = User::find_by_mail(&data.email, &conn).await else {
43-
err!("Username or password is incorrect. Try again.")
46+
let email = match &data.email {
47+
Some(email) if !email.is_empty() => Some(email),
48+
_ => None,
4449
};
50+
let user = if let Some(email) = email {
51+
let Some(master_password_hash) = &data.master_password_hash else {
52+
err!("No password hash has been submitted.")
53+
};
4554

46-
if !CONFIG._enable_email_2fa() {
47-
err!("Email 2FA is disabled")
48-
}
55+
let Some(user) = User::find_by_mail(email, &conn).await else {
56+
err!("Username or password is incorrect. Try again.")
57+
};
4958

50-
// Check password
51-
if !user.check_valid_password(&data.master_password_hash) {
52-
err!("Username or password is incorrect. Try again.")
53-
}
59+
// Check password
60+
if !user.check_valid_password(master_password_hash) {
61+
err!("Username or password is incorrect. Try again.")
62+
}
63+
64+
user
65+
} else {
66+
// SSO login only sends device id, so we get the user by the most recently used device
67+
let Some(user) = User::find_by_device_for_email2fa(&data.device_identifier, &conn).await else {
68+
err!("Username or password is incorrect. Try again.")
69+
};
5470

55-
send_token(&user.uuid, &conn).await?;
71+
user
72+
};
5673

57-
Ok(())
74+
send_token(&user.uuid, &conn).await
5875
}
5976

6077
/// Generate the token, save the data for later verification and send email to user

src/api/identity.rs

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use chrono::{NaiveDateTime, Utc};
1+
use chrono::Utc;
22
use num_traits::FromPrimitive;
33
use rocket::{
44
form::{Form, FromForm},
@@ -148,7 +148,7 @@ async fn _refresh_login(data: ConnectData, conn: &DbConn, ip: &ClientIp) -> Json
148148
}
149149
Ok((mut device, auth_tokens)) => {
150150
// Save to update `device.updated_at` to track usage and toggle new status
151-
device.save(conn).await?;
151+
device.save(true, conn).await?;
152152

153153
let result = json!({
154154
"refresh_token": auth_tokens.refresh_token(),
@@ -274,6 +274,7 @@ async fn _sso_login(
274274
}
275275
Some((mut user, sso_user)) => {
276276
let mut device = get_device(&data, conn, &user).await?;
277+
277278
let twofactor_token = twofactor_auth(&mut user, &data, &mut device, ip, client_version, conn).await?;
278279

279280
if user.private_key.is_none() {
@@ -303,7 +304,7 @@ async fn _sso_login(
303304
// We passed 2FA get auth tokens
304305
let auth_tokens = sso::redeem(&device, &user, data.client_id, sso_user, sso_auth, user_infos, conn).await?;
305306

306-
authenticated_response(&user, &mut device, auth_tokens, twofactor_token, &now, conn, ip).await
307+
authenticated_response(&user, &mut device, auth_tokens, twofactor_token, conn, ip).await
307308
}
308309

309310
async fn _password_login(
@@ -425,20 +426,20 @@ async fn _password_login(
425426

426427
let auth_tokens = auth::AuthTokens::new(&device, &user, AuthMethod::Password, data.client_id);
427428

428-
authenticated_response(&user, &mut device, auth_tokens, twofactor_token, &now, conn, ip).await
429+
authenticated_response(&user, &mut device, auth_tokens, twofactor_token, conn, ip).await
429430
}
430431

431432
async fn authenticated_response(
432433
user: &User,
433434
device: &mut Device,
434435
auth_tokens: auth::AuthTokens,
435436
twofactor_token: Option<String>,
436-
now: &NaiveDateTime,
437437
conn: &DbConn,
438438
ip: &ClientIp,
439439
) -> JsonResult {
440440
if CONFIG.mail_enabled() && device.is_new() {
441-
if let Err(e) = mail::send_new_device_logged_in(&user.email, &ip.ip.to_string(), now, device).await {
441+
let now = Utc::now().naive_utc();
442+
if let Err(e) = mail::send_new_device_logged_in(&user.email, &ip.ip.to_string(), &now, device).await {
442443
error!("Error sending new device email: {e:#?}");
443444

444445
if CONFIG.require_device_email() {
@@ -458,7 +459,7 @@ async fn authenticated_response(
458459
}
459460

460461
// Save to update `device.updated_at` to track usage and toggle new status
461-
device.save(conn).await?;
462+
device.save(true, conn).await?;
462463

463464
let master_password_policy = master_password_policy(user, conn).await;
464465

@@ -575,7 +576,7 @@ async fn _user_api_key_login(
575576
let access_claims = auth::LoginJwtClaims::default(&device, &user, &AuthMethod::UserApiKey, data.client_id);
576577

577578
// Save to update `device.updated_at` to track usage and toggle new status
578-
device.save(conn).await?;
579+
device.save(true, conn).await?;
579580

580581
info!("User {} logged in successfully via API key. IP: {}", user.email, ip.ip);
581582

@@ -638,7 +639,12 @@ async fn get_device(data: &ConnectData, conn: &DbConn, user: &User) -> ApiResult
638639
// Find device or create new
639640
match Device::find_by_uuid_and_user(&device_id, &user.uuid, conn).await {
640641
Some(device) => Ok(device),
641-
None => Device::new(device_id, user.uuid.clone(), device_name, device_type, conn).await,
642+
None => {
643+
let mut device = Device::new(device_id, user.uuid.clone(), device_name, device_type);
644+
// save device without updating `device.updated_at`
645+
device.save(false, conn).await?;
646+
Ok(device)
647+
}
642648
}
643649
}
644650

src/api/push.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ pub async fn register_push_device(device: &mut Device, conn: &DbConn) -> EmptyRe
128128
err!(format!("An error occurred while proceeding registration of a device: {e}"));
129129
}
130130

131-
if let Err(e) = device.save(conn).await {
131+
if let Err(e) = device.save(true, conn).await {
132132
err!(format!("An error occurred while trying to save the (registered) device push uuid: {e}"));
133133
}
134134

src/auth.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1223,7 +1223,7 @@ pub async fn refresh_tokens(
12231223
};
12241224

12251225
// Save to update `updated_at`.
1226-
device.save(conn).await?;
1226+
device.save(true, conn).await?;
12271227

12281228
let user = match User::find_by_uuid(&device.user_uuid, conn).await {
12291229
None => err!("Impossible to find user"),

src/db/models/device.rs

Lines changed: 27 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,25 @@ pub struct Device {
3535

3636
/// Local methods
3737
impl Device {
38+
pub fn new(uuid: DeviceId, user_uuid: UserId, name: String, atype: i32) -> Self {
39+
let now = Utc::now().naive_utc();
40+
41+
Self {
42+
uuid,
43+
created_at: now,
44+
updated_at: now,
45+
46+
user_uuid,
47+
name,
48+
atype,
49+
50+
push_uuid: Some(PushId(get_uuid())),
51+
push_token: None,
52+
refresh_token: crypto::encode_random_bytes::<64>(&BASE64URL),
53+
twofactor_remember: None,
54+
}
55+
}
56+
3857
pub fn to_json(&self) -> Value {
3958
json!({
4059
"id": self.uuid,
@@ -110,62 +129,39 @@ impl DeviceWithAuthRequest {
110129
}
111130
use crate::db::DbConn;
112131

113-
use crate::api::{ApiResult, EmptyResult};
132+
use crate::api::EmptyResult;
114133
use crate::error::MapResult;
115134

116135
/// Database methods
117136
impl Device {
118-
pub async fn new(uuid: DeviceId, user_uuid: UserId, name: String, atype: i32, conn: &DbConn) -> ApiResult<Device> {
119-
let now = Utc::now().naive_utc();
120-
121-
let device = Self {
122-
uuid,
123-
created_at: now,
124-
updated_at: now,
125-
126-
user_uuid,
127-
name,
128-
atype,
129-
130-
push_uuid: Some(PushId(get_uuid())),
131-
push_token: None,
132-
refresh_token: crypto::encode_random_bytes::<64>(&BASE64URL),
133-
twofactor_remember: None,
134-
};
135-
136-
device.inner_save(conn).await.map(|()| device)
137-
}
137+
pub async fn save(&mut self, update_time: bool, conn: &DbConn) -> EmptyResult {
138+
if update_time {
139+
self.updated_at = Utc::now().naive_utc();
140+
}
138141

139-
async fn inner_save(&self, conn: &DbConn) -> EmptyResult {
140142
db_run! { conn:
141143
sqlite, mysql {
142144
crate::util::retry(||
143145
diesel::replace_into(devices::table)
144-
.values(self)
146+
.values(&*self)
145147
.execute(conn),
146148
10,
147149
).map_res("Error saving device")
148150
}
149151
postgresql {
150152
crate::util::retry(||
151153
diesel::insert_into(devices::table)
152-
.values(self)
154+
.values(&*self)
153155
.on_conflict((devices::uuid, devices::user_uuid))
154156
.do_update()
155-
.set(self)
157+
.set(&*self)
156158
.execute(conn),
157159
10,
158160
).map_res("Error saving device")
159161
}
160162
}
161163
}
162164

163-
// Should only be called after user has passed authentication
164-
pub async fn save(&mut self, conn: &DbConn) -> EmptyResult {
165-
self.updated_at = Utc::now().naive_utc();
166-
self.inner_save(conn).await
167-
}
168-
169165
pub async fn delete_all_by_user(user_uuid: &UserId, conn: &DbConn) -> EmptyResult {
170166
db_run! { conn: {
171167
diesel::delete(devices::table.filter(devices::user_uuid.eq(user_uuid)))

src/db/models/user.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::db::schema::{invitations, sso_users, users};
1+
use crate::db::schema::{invitations, sso_users, twofactor_incomplete, users};
22
use chrono::{NaiveDateTime, TimeDelta, Utc};
33
use derive_more::{AsRef, Deref, Display, From};
44
use diesel::prelude::*;
@@ -10,7 +10,7 @@ use super::{
1010
use crate::{
1111
api::EmptyResult,
1212
crypto,
13-
db::DbConn,
13+
db::{models::DeviceId, DbConn},
1414
error::MapResult,
1515
sso::OIDCIdentifier,
1616
util::{format_date, get_uuid, retry},
@@ -386,6 +386,20 @@ impl User {
386386
}}
387387
}
388388

389+
pub async fn find_by_device_for_email2fa(device_uuid: &DeviceId, conn: &DbConn) -> Option<Self> {
390+
if let Some(user_uuid) = db_run! ( conn: {
391+
twofactor_incomplete::table
392+
.filter(twofactor_incomplete::device_uuid.eq(device_uuid))
393+
.order_by(twofactor_incomplete::login_time.desc())
394+
.select(twofactor_incomplete::user_uuid)
395+
.first::<UserId>(conn)
396+
.ok()
397+
}) {
398+
return Self::find_by_uuid(&user_uuid, conn).await;
399+
}
400+
None
401+
}
402+
389403
pub async fn get_all(conn: &DbConn) -> Vec<(Self, Option<SsoUser>)> {
390404
db_run! { conn: {
391405
users::table

0 commit comments

Comments
 (0)