Split SsoUser::find_by_identifier_or_email

src/api/identity.rs

@@ -162,56 +162,41 @@ async fn _sso_login(data: ConnectData, user_uuid: &mut Option<String>, conn: &mu
     };
 
     let user_infos = sso::exchange_code(code, conn).await?;
-
-    // Will trigger 2FA flow if needed
-    let user_data = match SsoUser::find_by_identifier_or_email(&user_infos.identifier, &user_infos.email, conn).await {
+    let user_with_sso = match SsoUser::find_by_identifier(&user_infos.identifier, conn).await {
+        None => match SsoUser::find_by_mail(&user_infos.email, conn).await {
             None => None,
-        Some((user, None)) if user.private_key.is_some() && !CONFIG.sso_signups_match_email() => {
+            Some((user, Some(_))) => {
                 error!(
-                "Login failure ({}), existing non SSO user ({}) with same email ({}) and association is disabled",
+                    "Login failure ({}), existing SSO user ({}) with same email ({})",
                     user_infos.identifier, user.uuid, user.email
                 );
                 err_silent!(
-                "Existing non SSO user with same email",
+                    "Existing SSO user with same email",
                     ErrorEvent {
                         event: EventType::UserFailedLogIn
                     }
                 )
             }
-        Some((user, Some(sso_user))) if sso_user.identifier != user_infos.identifier => {
+            Some((user, None)) if user.private_key.is_some() && !CONFIG.sso_signups_match_email() => {
                 error!(
-                "Login failure ({}), existing SSO user ({}) with same email ({})",
+                    "Login failure ({}), existing non SSO user ({}) with same email ({}) and association is disabled",
                     user_infos.identifier, user.uuid, user.email
                 );
                 err_silent!(
-                "Existing SSO user with same email",
-                ErrorEvent {
-                    event: EventType::UserFailedLogIn
-                }
-            )
-        }
-        Some((user, _)) if !user.enabled => {
-            err!(
-                "This user has been disabled",
-                format!("IP: {}. Username: {}.", ip.ip, user.name),
+                    "Existing non SSO user with same email",
                     ErrorEvent {
                         event: EventType::UserFailedLogIn
                     }
                 )
             }
-        Some((user, sso_user)) => {
-            let (mut device, new_device) = get_device(&data, conn, &user).await?;
-            let twofactor_token = twofactor_auth(&user, &data, &mut device, ip, conn).await?;
-
-            Some((user, device, new_device, twofactor_token, sso_user))
-        }
+            Some((user, None)) => Some((user, None)),
+        },
+        Some((user, sso_user)) => Some((user, Some(sso_user))),
     };
 
-    // We passed 2FA get full user informations
-    let auth_user = sso::redeem(&user_infos.state, conn).await?;
-
     let now = Utc::now().naive_utc();
-    let (user, mut device, new_device, twofactor_token, sso_user) = match user_data {
+    // Will trigger 2FA flow if needed
+    let (user, mut device, new_device, twofactor_token, sso_user) = match user_with_sso {
         None => {
             if !CONFIG.is_email_domain_allowed(&user_infos.email) {
                 err!(
@@ -247,7 +232,20 @@ async fn _sso_login(data: ConnectData, user_uuid: &mut Option<String>, conn: &mu
 
             (user, device, new_device, None, None)
         }
-        Some((mut user, device, new_device, twofactor_token, sso_user)) if user.private_key.is_none() => {
+        Some((user, _)) if !user.enabled => {
+            err!(
+                "This user has been disabled",
+                format!("IP: {}. Username: {}.", ip.ip, user.name),
+                ErrorEvent {
+                    event: EventType::UserFailedLogIn
+                }
+            )
+        }
+        Some((mut user, sso_user)) => {
+            let (mut device, new_device) = get_device(&data, conn, &user).await?;
+            let twofactor_token = twofactor_auth(&user, &data, &mut device, ip, conn).await?;
+
+            if user.private_key.is_none() {
                 // User was invited a stub was created
                 user.verified_at = Some(now);
                 if let Some(user_name) = user_infos.user_name {
@@ -259,19 +257,22 @@ async fn _sso_login(data: ConnectData, user_uuid: &mut Option<String>, conn: &mu
                 }
 
                 user.save(conn).await?;
-            (user, device, new_device, twofactor_token, sso_user)
             }
-        Some((user, device, new_device, twofactor_token, sso_user)) => {
+
             if user.email != user_infos.email {
                 if CONFIG.mail_enabled() {
                     mail::send_sso_change_email(&user_infos.email).await?;
                 }
                 info!("User {} email changed in SSO provider from {} to {}", user.uuid, user.email, user_infos.email);
             }
+
             (user, device, new_device, twofactor_token, sso_user)
         }
     };
 
+    // We passed 2FA get full user informations
+    let auth_user = sso::redeem(&user_infos.state, conn).await?;
+
     if sso_user.is_none() {
         let user_sso = SsoUser {
             user_uuid: user.uuid.clone(),

src/db/models/user.rs

@@ -1,7 +1,6 @@
 use crate::util::{format_date, get_uuid, retry};
 use chrono::{NaiveDateTime, TimeDelta, Utc};
 use serde_json::Value;
-use std::cmp::Ordering;
 
 use crate::crypto;
 use crate::CONFIG;
@@ -487,41 +486,29 @@ impl SsoUser {
         }
     }
 
-    // Written as an union to make the query more lisible than using an `or_filter`.
-    // If there is a match on identifier and email we want the identifier match.
-    // We sort results in code since UNION does not garanty order and DBs order NULL differently.
-    pub async fn find_by_identifier_or_email(
-        identifier: &str,
-        mail: &str,
-        conn: &DbConn,
-    ) -> Option<(User, Option<SsoUser>)> {
-        let lower_mail = mail.to_lowercase();
-
+    pub async fn find_by_identifier(identifier: &str, conn: &DbConn) -> Option<(User, SsoUser)> {
         db_run! {conn: {
-            let mut res = users::table
+            users::table
                 .inner_join(sso_users::table)
-                .select(<(UserDb, Option<SsoUserDb>)>::as_select())
+                .select(<(UserDb, SsoUserDb)>::as_select())
                 .filter(sso_users::identifier.eq(identifier))
-                .union(
+                .first::<(UserDb, SsoUserDb)>(conn)
+                .ok()
+                .map(|(user, sso_user)| { (user.from_db(), sso_user.from_db()) })
+        }}
+    }
+
+    pub async fn find_by_mail(mail: &str, conn: &DbConn) -> Option<(User, Option<SsoUser>)> {
+        let lower_mail = mail.to_lowercase();
+
+        db_run! {conn: {
             users::table
                 .left_join(sso_users::table)
                 .select(<(UserDb, Option<SsoUserDb>)>::as_select())
                 .filter(users::email.eq(lower_mail))
-                )
-                .load(conn)
-                .expect("Error searching user by SSO identifier and email")
-                .into_iter()
+                .first::<(UserDb, Option<SsoUserDb>)>(conn)
+                .ok()
                 .map(|(user, sso_user)| { (user.from_db(), sso_user.from_db()) })
-                .collect::<Vec<(User, Option<SsoUser>)>>();
-
-            res.sort_by(|(_, sso_user), _| {
-                match sso_user {
-                    Some(db_sso_user) if db_sso_user.identifier == identifier => Ordering::Less,
-                    _ => Ordering::Greater,
-                }
-            });
-
-            res.into_iter().next()
         }}
     }
 }