Make the compat login use the new password manager

Cargo.lock

@@ -2684,7 +2684,6 @@ name = "mas-cli"
 version = "0.1.0"
 dependencies = [
  "anyhow",
- "argon2",
  "atty",
  "axum",
  "camino",
@@ -3101,21 +3100,16 @@ dependencies = [
 name = "mas-storage"
 version = "0.1.0"
 dependencies = [
- "anyhow",
- "argon2",
  "chrono",
  "mas-data-model",
  "mas-iana",
  "mas-jose",
  "oauth2-types",
- "password-hash",
  "rand 0.8.5",
- "rand_chacha 0.3.1",
  "serde",
  "serde_json",
  "sqlx",
  "thiserror",
- "tokio",
  "tracing",
  "ulid",
  "url",

crates/cli/Cargo.toml

@@ -7,7 +7,6 @@ license = "Apache-2.0"
 
 [dependencies]
 anyhow = "1.0.66"
-argon2 = { version = "0.4.1", features = ["password-hash"] }
 atty = "0.2.14"
 axum = "0.6.1"
 camino = "1.1.1"

crates/handlers/src/compat/login.rs

@@ -18,18 +18,20 @@ use hyper::StatusCode;
 use mas_data_model::{CompatSession, CompatSsoLoginState, Device, TokenType};
 use mas_storage::{
     compat::{
-        add_compat_access_token, add_compat_refresh_token, compat_login,
+        add_compat_access_token, add_compat_refresh_token, get_compat_sso_login_by_token,
-        get_compat_sso_login_by_token, mark_compat_sso_login_as_exchanged,
+        mark_compat_sso_login_as_exchanged, start_compat_session,
     },
+    user::{add_user_password, lookup_user_by_username, lookup_user_password},
     Clock,
 };
 use serde::{Deserialize, Serialize};
 use serde_with::{serde_as, skip_serializing_none, DurationMilliSeconds};
 use sqlx::{PgPool, Postgres, Transaction};
 use thiserror::Error;
+use zeroize::Zeroizing;
 
 use super::{MatrixError, MatrixHomeserver};
-use crate::impl_from_error_for_route;
+use crate::{impl_from_error_for_route, passwords::PasswordManager};
 
 #[derive(Debug, Serialize)]
 #[serde(tag = "type")]
@@ -132,8 +134,14 @@ pub enum RouteError {
     #[error("unsupported login method")]
     Unsupported,
 
-    #[error("login failed")]
+    #[error("user not found")]
-    LoginFailed,
+    UserNotFound,
+
+    #[error("user has no password")]
+    NoPassword,
+
+    #[error("password verification failed")]
+    PasswordVerificationFailed(#[source] anyhow::Error),
 
     #[error("login took too long")]
     LoginTookTooLong,
@@ -158,11 +166,13 @@ impl IntoResponse for RouteError {
                 error: "Invalid login type",
                 status: StatusCode::BAD_REQUEST,
             },
-            Self::LoginFailed => MatrixError {
+            Self::UserNotFound | Self::NoPassword | Self::PasswordVerificationFailed(_) => {
-                errcode: "M_UNAUTHORIZED",
+                MatrixError {
-                error: "Invalid username/password",
+                    errcode: "M_UNAUTHORIZED",
-                status: StatusCode::FORBIDDEN,
+                    error: "Invalid username/password",
-            },
+                    status: StatusCode::FORBIDDEN,
+                }
+            }
             Self::LoginTookTooLong => MatrixError {
                 errcode: "M_UNAUTHORIZED",
                 error: "Login token expired",
@@ -180,6 +190,7 @@ impl IntoResponse for RouteError {
 
 #[tracing::instrument(skip_all, err)]
 pub(crate) async fn post(
+    State(password_manager): State<PasswordManager>,
     State(pool): State<PgPool>,
     State(homeserver): State<MatrixHomeserver>,
     Json(input): Json<RequestBody>,
@@ -190,7 +201,7 @@ pub(crate) async fn post(
         Credentials::Password {
             identifier: Identifier::User { user },
             password,
-        } => user_password_login(&mut txn, user, password).await?,
+        } => user_password_login(&password_manager, &mut txn, user, password).await?,
 
         Credentials::Token { token } => token_login(&mut txn, &clock, &token).await?,
 
@@ -295,16 +306,53 @@ async fn token_login(
 }
 
 async fn user_password_login(
+    password_manager: &PasswordManager,
     txn: &mut Transaction<'_, Postgres>,
     username: String,
     password: String,
 ) -> Result<CompatSession, RouteError> {
     let (clock, mut rng) = crate::clock_and_rng();
 
-    let device = Device::generate(&mut rng);
+    // Find the user
-    let session = compat_login(txn, &mut rng, &clock, &username, &password, device)
+    let user = lookup_user_by_username(&mut *txn, &username)
+        .await?
+        .ok_or(RouteError::UserNotFound)?;
+
+    // Lookup its password
+    let user_password = lookup_user_password(&mut *txn, &user)
+        .await?
+        .ok_or(RouteError::NoPassword)?;
+
+    // Verify the password
+    let password = Zeroizing::new(password.into_bytes());
+
+    let new_password_hash = password_manager
+        .verify_and_upgrade(
+            &mut rng,
+            user_password.version,
+            password,
+            user_password.hashed_password.clone(),
+        )
         .await
-        .map_err(|_| RouteError::LoginFailed)?;
+        .map_err(RouteError::PasswordVerificationFailed)?;
+
+    if let Some((version, hashed_password)) = new_password_hash {
+        // Save the upgraded password if needed
+        add_user_password(
+            &mut *txn,
+            &mut rng,
+            &clock,
+            &user,
+            version,
+            hashed_password,
+            Some(user_password),
+        )
+        .await?;
+    }
+
+    // Now that the user credentials have been verified, start a new compat session
+    let device = Device::generate(&mut rng);
+    let session = start_compat_session(&mut *txn, &mut rng, &clock, user, device).await?;
 
     Ok(session)
 }

crates/handlers/src/lib.rs

@@ -208,6 +208,7 @@ where
     UrlBuilder: FromRef<S>,
     PgPool: FromRef<S>,
     MatrixHomeserver: FromRef<S>,
+    PasswordManager: FromRef<S>,
 {
     Router::new()
         .route(

crates/storage/Cargo.toml

@@ -6,20 +6,15 @@ edition = "2021"
 license = "Apache-2.0"
 
 [dependencies]
-tokio = "1.23.0"
 sqlx = { version = "0.6.2", features = ["runtime-tokio-rustls", "postgres", "migrate", "chrono", "offline", "json", "uuid"] }
 chrono = { version = "0.4.23", features = ["serde"] }
 serde = { version = "1.0.149", features = ["derive"] }
 serde_json = "1.0.89"
 thiserror = "1.0.37"
-anyhow = "1.0.66"
 tracing = "0.1.37"
 
 # Password hashing
-argon2 = { version = "0.4.1", features = ["password-hash"] }
-password-hash = { version = "0.4.2", features = ["std"] }
 rand = "0.8.5"
-rand_chacha = "0.3.1"
 url = { version = "2.3.1", features = ["serde"] }
 uuid = "1.2.2"
 ulid = { version = "1.0.0", features = ["uuid", "serde"] }

crates/storage/src/compat.rs

@@ -12,8 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use anyhow::Context;
-use argon2::{Argon2, PasswordHash};
 use chrono::{DateTime, Duration, Utc};
 use mas_data_model::{
     CompatAccessToken, CompatRefreshToken, CompatSession, CompatSsoLogin, CompatSsoLoginState,
@@ -21,7 +19,6 @@ use mas_data_model::{
 };
 use rand::Rng;
 use sqlx::{Acquire, PgExecutor, Postgres, QueryBuilder};
-use tokio::task;
 use tracing::{info_span, Instrument};
 use ulid::Ulid;
 use url::Url;
@@ -29,7 +26,6 @@ use uuid::Uuid;
 
 use crate::{
     pagination::{process_page, QueryBuilderExt},
-    user::lookup_user_by_username,
     Clock, DatabaseError, DatabaseInconsistencyError, LookupResultExt,
 };
 
@@ -284,91 +280,6 @@ pub async fn lookup_active_compat_refresh_token(
     Ok(Some((refresh_token, access_token, session)))
 }
 
-#[tracing::instrument(
-    skip_all,
-    fields(
-        user.username = username,
-        user.id,
-        compat_session.id,
-        compat_session.device.id = device.as_str(),
-    ),
-    err(Debug),
-)]
-pub async fn compat_login(
-    conn: impl Acquire<'_, Database = Postgres> + Send,
-    mut rng: impl Rng + Send,
-    clock: &Clock,
-    username: &str,
-    password: &str,
-    device: Device,
-) -> Result<CompatSession, anyhow::Error> {
-    // TODO: that should be split and not verify the password hash here
-    let mut txn = conn.begin().await.context("could not start transaction")?;
-
-    // First, lookup the user
-    let user = lookup_user_by_username(&mut txn, username)
-        .await?
-        .context("Could not lookup username")?;
-    tracing::Span::current().record("user.id", tracing::field::display(user.id));
-
-    // Now, fetch the hashed password from the user associated with that session
-    let hashed_password: String = sqlx::query_scalar!(
-        r#"
-            SELECT up.hashed_password
-            FROM user_passwords up
-            WHERE up.user_id = $1
-            ORDER BY up.created_at DESC
-            LIMIT 1
-        "#,
-        Uuid::from(user.id),
-    )
-    .fetch_one(&mut txn)
-    .instrument(tracing::info_span!("Lookup hashed password"))
-    .await?;
-
-    // TODO: pass verifiers list as parameter
-    // Verify the password in a blocking thread to avoid blocking the async executor
-    let password = password.to_owned();
-    task::spawn_blocking(move || {
-        let context = Argon2::default();
-        let hasher = PasswordHash::new(&hashed_password)?;
-        hasher.verify_password(&[&context], &password)
-    })
-    .instrument(tracing::info_span!("Verify hashed password"))
-    .await??;
-
-    let created_at = clock.now();
-    let id = Ulid::from_datetime_with_source(created_at.into(), &mut rng);
-    tracing::Span::current().record("compat_session.id", tracing::field::display(id));
-
-    sqlx::query!(
-        r#"
-            INSERT INTO compat_sessions
-              (compat_session_id, user_id, device_id, created_at)
-            VALUES ($1, $2, $3, $4)
-        "#,
-        Uuid::from(id),
-        Uuid::from(user.id),
-        device.as_str(),
-        created_at,
-    )
-    .execute(&mut txn)
-    .instrument(tracing::info_span!("Insert compat session"))
-    .await
-    .context("could not insert compat session")?;
-
-    let session = CompatSession {
-        id,
-        user,
-        device,
-        created_at,
-        finished_at: None,
-    };
-
-    txn.commit().await.context("could not commit transaction")?;
-    Ok(session)
-}
-
 #[tracing::instrument(
     skip_all,
     fields(
@@ -895,6 +806,48 @@ pub async fn get_compat_sso_login_by_token(
     Ok(Some(res.try_into()?))
 }
 
+#[tracing::instrument(
+    skip_all,
+    fields(
+        %user.id,
+        compat_session.id,
+        compat_session.device.id = device.as_str(),
+    ),
+    err,
+)]
+pub async fn start_compat_session(
+    executor: impl PgExecutor<'_>,
+    mut rng: impl Rng + Send,
+    clock: &Clock,
+    user: User,
+    device: Device,
+) -> Result<CompatSession, DatabaseError> {
+    let created_at = clock.now();
+    let id = Ulid::from_datetime_with_source(created_at.into(), &mut rng);
+    tracing::Span::current().record("compat_session.id", tracing::field::display(id));
+
+    sqlx::query!(
+        r#"
+            INSERT INTO compat_sessions (compat_session_id, user_id, device_id, created_at)
+            VALUES ($1, $2, $3, $4)
+        "#,
+        Uuid::from(id),
+        Uuid::from(user.id),
+        device.as_str(),
+        created_at,
+    )
+    .execute(executor)
+    .await?;
+
+    Ok(CompatSession {
+        id,
+        user,
+        device,
+        created_at,
+        finished_at: None,
+    })
+}
+
 #[tracing::instrument(
     skip_all,
     fields(
@@ -920,31 +873,7 @@ pub async fn fullfill_compat_sso_login(
 
     let mut txn = conn.begin().await?;
 
-    let created_at = clock.now();
+    let session = start_compat_session(&mut txn, &mut rng, clock, user, device).await?;
-    let id = Ulid::from_datetime_with_source(created_at.into(), &mut rng);
-    tracing::Span::current().record("compat_session.id", tracing::field::display(id));
-
-    sqlx::query!(
-        r#"
-            INSERT INTO compat_sessions (compat_session_id, user_id, device_id, created_at)
-            VALUES ($1, $2, $3, $4)
-        "#,
-        Uuid::from(id),
-        Uuid::from(user.id),
-        device.as_str(),
-        created_at,
-    )
-    .execute(&mut txn)
-    .instrument(tracing::info_span!("Insert compat session"))
-    .await?;
-
-    let session = CompatSession {
-        id,
-        user,
-        device,
-        created_at,
-        finished_at: None,
-    };
 
     let fulfilled_at = clock.now();
     sqlx::query!(